Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added CORS fragments to nginx_vhost role #370

Merged
merged 3 commits into from
Jun 5, 2020

Conversation

vjrj
Copy link
Collaborator

@vjrj vjrj commented Feb 13, 2020

Following this conversation in slack this fix #225.

Tested in a demo site.

@vjrj vjrj requested a review from ansell February 13, 2020 09:42
Copy link
Contributor

@ansell ansell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The template names need to match the fragment names on disk after they are rendered, and we need to be able to tune the header names/origins/methods per server as necessary.

ansible/roles/nginx_vhost/templates/ala_cors.j2 Outdated Show resolved Hide resolved
ansible/roles/nginx_vhost/tasks/main.yml Show resolved Hide resolved
ansible/roles/nginx_vhost/tasks/main.yml Show resolved Hide resolved
@vjrj vjrj requested a review from ansell March 13, 2020 10:07
@ansell ansell self-assigned this May 14, 2020
@vjrj
Copy link
Collaborator Author

vjrj commented Jun 4, 2020

Since I pushed this PR we have manually applied (*) and tested in several LA deployments (Tanzania, Guatemala, Royal Gardens Kew demo, Vermont). I think is safe to merge it if you don't have any other comment.

Thanks,

(*) With people not very comfortable with git, this is quite a difficult task. Yesterday I spend more than an hour (looking my chat, from 15:41 to 17:35) just trying to help someone in slack, to apply this PR, to solve the CORS errors in a LA site. And it's not the first time this happens to me.

@vjrj vjrj merged commit 01e47c6 into AtlasOfLivingAustralia:master Jun 5, 2020
@vjrj
Copy link
Collaborator Author

vjrj commented Jun 5, 2020

Merged after talking with @ansell , thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CORS header support to nginx locations
2 participants