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

Separate buckets for tls certs #2885

Merged
merged 11 commits into from
Jan 19, 2024
Merged

Separate buckets for tls certs #2885

merged 11 commits into from
Jan 19, 2024

Conversation

jmgrady
Copy link
Collaborator

@jmgrady jmgrady commented Jan 17, 2024

This PR enables the cert-proxy-client and cert-proxy-server charts to manage certificates for NUCs and local installations that are stored in different S3 buckets instead of them all being in a single bucket.

The client and server helm charts are changed as follows:

  • the ansible configuration changes from being a string of space-separated hostnames for which certificates are to be generated to a list of objects with a hostname and a bucket attribute;
  • the environment variable created for the cert-proxy-server chart is a string of space-separated values of the form hostname@bucket
  • the environment variable created for the cert-proxy-client chart is a string of the form hostname@bucket
  • the Python script, monitor.py, is updated to support the new environment variable format.

This PR also makes the following "cleanup" changes:

  1. Remove .vagrant from .gitignore - vagrant has not been used on The Combine for many years.
  2. Replace template with include in Helm chart files - see: https://stackoverflow.com/questions/71086697/how-does-template-and-include-differ-in-helm

This change is Reviewable

maintenance/scripts/monitor.py Dismissed Show dismissed Hide dismissed
@codecov-commenter
Copy link

codecov-commenter commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (896bade) 72.94% compared to head (bbd9aec) 72.94%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2885   +/-   ##
=======================================
  Coverage   72.94%   72.94%           
=======================================
  Files         264      264           
  Lines       10012    10012           
  Branches     1181     1181           
=======================================
  Hits         7303     7303           
  Misses       2367     2367           
  Partials      342      342           
Flag Coverage Δ
backend 83.35% <ø> (ø)
frontend 63.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmgrady jmgrady marked this pull request as ready for review January 19, 2024 15:34
Copy link
Collaborator Author

@jmgrady jmgrady left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 21 files reviewed, 1 unresolved discussion (waiting on @Github-advanced-security[bot] and @jmgrady)


maintenance/scripts/monitor.py line 40 at r1 (raw file):

Previously, github-advanced-security[bot] wrote…

Clear-text logging of sensitive information

This expression logs sensitive data (secret) as clear text.

Show more details

Done.

Copy link
Collaborator Author

@jmgrady jmgrady left a comment

Choose a reason for hiding this comment

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

Dismissed @Github-advanced-security[bot] from a discussion.
Reviewable status: 0 of 21 files reviewed, all discussions resolved (waiting on @jmgrady)

Copy link
Collaborator

@imnasnainaec imnasnainaec left a comment

Choose a reason for hiding this comment

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

Reviewed 16 of 21 files at r1, 4 of 5 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jmgrady)

@jmgrady jmgrady merged commit 5123aab into master Jan 19, 2024
17 checks passed
@jmgrady jmgrady deleted the separate-buckets-for-tls-certs branch January 19, 2024 21:57
jmgrady added a commit that referenced this pull request Mar 6, 2024
* Update helm charts to allow separate AWS buckets for proxied TLS certificates
* Prefer 'include' to 'template' in helm charts
* Remove vagrant files from .gitignore
* Change become:no to become:false
* Remove support for `microk8s`
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.

3 participants