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

update local nginx to use mkcert #21455

Merged
merged 5 commits into from
Jun 4, 2019
Merged

update local nginx to use mkcert #21455

merged 5 commits into from
Jun 4, 2019

Conversation

akash1810
Copy link
Member

@akash1810 akash1810 commented May 28, 2019

What does this change?

Currently we require Identity credentials to setup nginx as we're storing the certificate in S3. This is problematic in a few ways:

  • A reliance on S3 to setup nginx
  • A reliance on having the correct Janus credentials
  • We need to update the certificate once a year as it expires
  • Certificate renewal is handled by another team
  • Every machine is using the same certificate; if its compromised once, its compromised everywhere

mkcert provides:

  • Security as each machine acts as their own CA
  • No reliance on S3 or Identity Janus credentials
  • Frictionless certificate renewal - we just re-run the script

Also writes nginx site config to /usr/local/etc/nginx/servers/ directory as is preferred with latest version of nginx.

Related to https://github.com/guardian/dev-nginx-old/pull/37. Ideally the dev-nginx scripts would be shared rather than copy pasted, however that repo is private at the moment - once it's public we can migrate.

Screenshots

image

image

What is the value of this and can you measure success?

  • A simplified process to develop on frontend with https
  • No reliance on the Identity credentials

Checklist

Does this affect other platforms?

  • AMP
  • Apps
  • Other (please specify)

Does this affect GLabs Paid Content Pages? Should it have support for Paid Content?

  • No
  • Yes (please give details)

Does this change break ad-free?

  • No
  • It did, but tests caught it and I fixed it
  • It did, but there was no test coverage so I added that then fixed it

Does this change update the version of CAPI we're using?

Accessibility test checklist

Tested

  • Locally
  • On CODE (optional)

@akash1810
Copy link
Member Author

akash1810 commented May 28, 2019

I think this means we can remove the need for Identity credentials from frontend now too? 🤞 cc @guardian/identity https://github.com/guardian/janus/pull/1822

nginx/setup.sh Outdated Show resolved Hide resolved
nginx/setup.sh Show resolved Hide resolved
@jfsoul
Copy link
Contributor

jfsoul commented May 30, 2019

Thought: does this work the same for a fresh install as over the top of an existing nginx setup?

@akash1810
Copy link
Member Author

Thought: does this work the same for a fresh install as over the top of an existing nginx setup?

Yes 🎉 . The slightly contentious part is we're now symlinking the nginx site config to servers as opposed to sited-enabled. My justification is:

@akash1810 akash1810 force-pushed the aa-mkcert branch 2 times, most recently from 767aa14 to aaf47b2 Compare May 31, 2019 13:03
Copy link
Contributor

@nicl nicl left a comment

Choose a reason for hiding this comment

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

Thanks @akash1810

echo "Restarting Nginx"
sudo nginx -s stop
echo -e "🚀 ${YELLOW}Restarting nginx, Requires sudo - enter password when prompted.${NC}"
if pgrep 'nginx' > /dev/null; then
Copy link
Member Author

Choose a reason for hiding this comment

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

@nicl we're now restarting nginx more gracefully - if running, stop, finally start.

Currently we require `Identity` credentials to setup nginx as we're storing the certificate in S3. This is problematic in a few ways:
- A reliance on S3 to setup nginx
- A reliance on having the correct Janus credentials
- We need to update the certificate once a year as it expires
- Certificate renewal is handled by another team
- Every machine is using the same certificate; if its compromised once, its compromised everywhere

mkcert provides:
- Security as each machine acts as their own CA
- No reliance on S3 or Identity Janus credentials
- Frictionless certificate renewal - we just re-run the script

Also writes nginx site config to `/usr/local/etc/nginx/servers/` directory as is preferred with latest version of nginx.
trying to stop nginx when it is not running fails...
@prout-bot
Copy link
Collaborator

Overdue on PROD (merged by @akash1810 30 minutes and 9 seconds ago) What's gone wrong?

@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @akash1810 2 hours, 19 minutes and 48 seconds ago)

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

Successfully merging this pull request may close these issues.

5 participants