adds support for cors on storage accounts#193
Merged
Conversation
mob-galtryn
force-pushed
the
2778-enable-cors-stroage-account-module
branch
3 times, most recently
from
df5f53f to
371fdfd
Compare
RMcVelia
reviewed
May 20, 2026
| allowed_methods = cors_rule.value.allowed_methods | ||
| allowed_origins = cors_rule.value.allowed_origins | ||
| exposed_headers = cors_rule.value.exposed_headers | ||
| max_age_in_seconds = cors_rule.value.max_age_in_seconds |
Contributor
There was a problem hiding this comment.
Should we hardcode, or restrict any of these values? e.g. exposed_headers might be something we could hardcode depending on what TV require, as that might be common for all services that want to use this?
for allowed_origins maybe we want to stop * if that's possible?
mob-galtryn
force-pushed
the
2778-enable-cors-stroage-account-module
branch
6 times, most recently
from
ff6bd19 to
eea1eae
Compare
mob-galtryn
force-pushed
the
2778-enable-cors-stroage-account-module
branch
3 times, most recently
from
98bd3e6 to
4c2c7fc
Compare
| })) | ||
| description = "A list of CORS rules to apply to the stroage account" | ||
| default = [] | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
The defaults look to be good ones to me
hopep-mobilise
approved these changes
May 29, 2026
hopep-mobilise
left a comment
hopep-mobilise
left a comment
Contributor
There was a problem hiding this comment.
The defaults look to be good choices to me
RMcVelia
reviewed
Jun 2, 2026
| type = list(object({ | ||
| allowed_headers = optional(list(string), ["Content-Type", "Content-MD5", "Content-Disposition", "x-ms-blob-content-disposition", "x-ms-blob-type"]), | ||
| allowed_methods = optional(list(string), ["PUT"]), | ||
| allowed_origins = optional(list(string)), |
Contributor
There was a problem hiding this comment.
can we stop * for allowed origins using a validation rule? i.e. https://developer.hashicorp.com/terraform/language/block/variable#validation
| allowed_headers = optional(list(string), ["Content-Type", "Content-MD5", "Content-Disposition", "x-ms-blob-content-disposition", "x-ms-blob-type"]), | ||
| allowed_methods = optional(list(string), ["PUT"]), | ||
| allowed_origins = optional(list(string)), | ||
| exposed_headers = optional(list(string), ["*"]), |
Contributor
There was a problem hiding this comment.
same here, we don't want to allow *. Think we should restrict allowed_headers to specifics (and allowed_headers) using a validation rule if possible
mob-galtryn
force-pushed
the
2778-enable-cors-stroage-account-module
branch
2 times, most recently
from
e72da6b to
100b6b1
Compare
mob-galtryn
force-pushed
the
2778-enable-cors-stroage-account-module
branch
3 times, most recently
from
ec67939 to
d642022
Compare
mob-galtryn
force-pushed
the
2778-enable-cors-stroage-account-module
branch
from
d642022 to
4f4aba7
Compare
RMcVelia
approved these changes
Jun 10, 2026
10 tasks
scruti
added a commit
to DFE-Digital/teaching-vacancies
that referenced
this pull request
Jun 12, 2026
Setup CORS rules through Terraform that will set & stay between deployments. They will allow Direct Uploads (Browser -> Storage server) in all our environments. Infra team has enabled for us the support for CORS config on the storage accounts: DFE-Digital/terraform-modules#193
scruti
added a commit
to DFE-Digital/teaching-vacancies
that referenced
this pull request
Jun 12, 2026
Setup CORS rules through Terraform that will set & stay between deployments. They will allow Direct Uploads (Browser -> Storage server) in all our environments. Infra team has enabled for us the support for CORS config on the storage accounts: DFE-Digital/terraform-modules#193
scruti
added a commit
to DFE-Digital/teaching-vacancies
that referenced
this pull request
Jun 17, 2026
## Trello card URL https://trello.com/c/IdNp4YfN/2667-file-attachment-error-in-candidate-message-section-ats-evaluation-march-2026 ## Changes in this PR: Set up CORS rules through Terraform that will set & stay between deployments. They will allow Direct Uploads (Browser -> Storage server) in all our environments. Infra team has enabled for us the support for CORS config on the storage accounts: DFE-Digital/terraform-modules#193 ## Screenshots of UI changes: ### Before <img width="2478" height="478" alt="Screenshot From 2026-06-12 16-55-33" src="https://github.com/user-attachments/assets/668470e6-7645-4816-b78a-f5917daa2d29" /> ### After <img width="2525" height="476" alt="Screenshot From 2026-06-12 16-53-57" src="https://github.com/user-attachments/assets/1d684749-f4f0-4c5a-a79a-c5fdd541c4a2" /> <img width="2532" height="539" alt="image" src="https://github.com/user-attachments/assets/c5fe47fd-c520-4317-9d25-e82194f2740a" /> ## Checklists: ### Data & Schema Changes If this PR modifies data structures or validations, check the following: - [ ] Adds/removes model validations - [ ] Adds/removes database fields - [ ] Modifies Vacancy enumerables (phases, working patterns, job roles, key stages, etc.) <details> <summary>If any of the above options has changed then the author must check/resolve all of the following...</summary> ### Integration Impact Does this change affect any of these integrations? - [ ] DfE Analytics platform - [ ] Legacy imports mappings - [ ] DWP Find a Job export mappings - [ ] Publisher ATS API (may require mapping updates or API versioning) ### User Experience & Data Integrity Could this change impact: - [ ] Existing subscription alerts (will legacy subscription search filters break?) - [ ] Legacy vacancy copying (will copied vacancies fail new validations?) - [ ] In-progress drafts for Vacancies or Job Applications </details>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Adds the ability to set cors rules on storage accounts via the terraform module.
Changes proposed in this pull request
Adds the a dynamic block for cors_rules to the blob_properties of the stroage account reasource.
Guidance to review
Can be tested by deploying a review app that uses a storage account and adding to cors_rule block as described in the readme.
Before merging
After merging
Checklist
Devopslabel