-
Notifications
You must be signed in to change notification settings - Fork 9
[WIP] Introduce Bootstrap secrets #125
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
Closed
Closed
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
3f0e3d8
Initial concept of boostrap secrets
9beb5f9
Update determine_pattern_dir to allow defaults
4d69e10
Fix markdown and extend retries a bit
8a5fbae
Be more aggressive about timers and show a bit more information while…
b913b75
Update docs and fix linter error
aadf421
Enhance super-linter, fix README, and armor ansible python interprete…
a520daa
Separate VALUES_SECRET_BOOTSTRAP override for bootstrap secrets
b8789ea
Single file bootstrap solution
9823c80
Re-order secrets loading so that it happens after pattern is applied
0d6fb00
Consolidate to single file and update tests
5c6eaf4
Don't fail non-bootstrap secrets in bootstrap phase
19cf684
Only consider bootstrap secrets
1985d41
Fix display secrets info playbook
392baaf
Split phases more explicitly
fc8e80a
Remove duplicate playbook to remove confusion
371089e
Remove reference to never-released VALUES_SECRET_BOOTSTRAP variable u…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,24 @@ | ||
| --- | ||
| # Resolves pattern_dir the same way as the pattern_settings role (extra-vars, PATTERN_DIR, PWD, pwd), | ||
| # then fails if still unset. Used by display_secrets_info, load_values_global, load_bootstrap_secrets, etc. | ||
| - name: Determine pattern dir | ||
| hosts: localhost | ||
| connection: local | ||
| gather_facts: false | ||
| become: false | ||
| vars: | ||
| pattern_dir: '' | ||
| tasks: | ||
| - name: Fail if directory is not set | ||
| - name: Resolve pattern_dir from extra-vars, PATTERN_DIR, PWD, or pwd | ||
| ansible.builtin.include_role: | ||
| name: pattern_settings | ||
| tasks_from: resolve_overrides.yml | ||
|
|
||
| - name: Fail if pattern directory is not set after resolution | ||
| ansible.builtin.fail: | ||
| msg: "pattern_dir variable must be set" | ||
| when: pattern_dir | length == 0 | ||
| msg: >- | ||
| pattern_dir is not set. Pass -e pattern_dir=/path/to/pattern, export PATTERN_DIR to that path, | ||
| or run the playbook from the pattern directory so PWD is correct. | ||
| when: pattern_dir | default('') | string | trim | length == 0 | ||
|
|
||
| - name: Set pattern_dir fact for future plays | ||
| ansible.builtin.set_fact: | ||
| pattern_dir: '{{ pattern_dir }}' | ||
| pattern_dir: "{{ pattern_dir | string | trim }}" |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| --- | ||
| # Inject only bootstrap-tagged secrets from the primary values-secret file (none backend, with retries). | ||
| # Does not load secrets into Vault or the primary Kubernetes backend. Does not honor | ||
| # secretLoader.disabled from values-global. Fails if no primary file exists or there are no | ||
| # bootstrap-tagged v2 entries. | ||
| # Optional extra-vars: vp_secrets_bootstrap_retry_max, vp_secrets_bootstrap_retry_delay (seconds). | ||
| - name: Determine pattern directory | ||
| ansible.builtin.import_playbook: ./determine_pattern_dir.yml | ||
|
|
||
| - name: Determine pattern name | ||
| ansible.builtin.import_playbook: ./determine_pattern_name.yml | ||
|
|
||
| - name: Load bootstrap secrets | ||
| hosts: localhost | ||
| connection: local | ||
| gather_facts: false | ||
| become: false | ||
| roles: | ||
| - pattern_settings | ||
|
|
||
| tasks: | ||
| - name: Run bootstrap-only secrets load | ||
| ansible.builtin.include_role: | ||
| name: load_secrets | ||
| tasks_from: bootstrap_only.yml |
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make more sense to just have a separate "bootstrap_secrets" section and drop the bootstrap field entirely. If for whatever odd reason a user needs the same secret in both k8s and vault he can just duplicate them in both sections. Also code wise this is a lot simpler to reason about (especially for the UI as well). You also have less risk of breaking things, because the load_secrets bit can stay mostly unchanged and you just need to do some validation on the bootstrap secrets and then create them as k8s secrets normally. And it is a bit simpler for the user to reason about as well.
What would be nice is to spell out somewhere the exact use cases for all this and how this will work exactly and why they should/must be bootstrap secrets. Right now we have:
So today for case 1. we pass this to a pattern CR:
I assume that we will need to add docs to add an example to the
bootstrap_secretssection for the aboveprivate-reposecret. It'd be nice to have some more concrete examples somewhere. So the example could be:It will also need a label/annotations as well (no matter how we do that). I do wonder if we shouldn't come up with a more user-friendly to define the non secret bits of this (e.g. type, url)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am opposed to creating a separate section of the file for this. I think in enough cases, people will also want to inject these secrets into their designated secret store, and duplicating them (by handling this with separate sections) is an invitation for the declarations to drift. Labels and annotations are already supported for the none injector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've had more internal/offline conversatons about this, and I'm now not opposed to creating a separate section. More work incoming :)