Secrets as resources review fixes #425
Merged
Conversation
This file contains 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
|
@DazWorrall I believe this addresses all of @KnVerey's review comments on #424. |
Unifying the constant used for simple resources of this type is left as an exercise for another change, mostly because the name of such a thing may be controversial and I don't want to block merging this on detailed review.
Doesn't test anything more than test_create_and_update_secrets_from_ejson
benlangfeld
force-pushed
the
secrets-as-resources-review-fixes
branch
from
9919ad7
to
2fbaf64
Compare
benlangfeld
force-pushed
the
secrets-as-resources-review-fixes
branch
from
2fbaf64
to
a975c3f
Compare
|
Thanks @benlangfeld, this is a great help ❤️ |
DazWorrall
pushed a commit
that referenced
this pull request
Feb 27, 2019
Thanks so much @benlangfeld for your help with these: * Remove excess logging * Stop referring to EJSON secrets as generically "managed" * Consistent timeout for Secret resources Unifying the constant used for simple resources of this type is left as an exercise for another change, mostly because the name of such a thing may be controversial and I don't want to block merging this on detailed review. * Give secrets the same statsd tags as other resources * Removes duplicate spec Doesn't test anything more than test_create_and_update_secrets_from_ejson * Replaces log assertion * Point out breaking changes in CHANGELOG * Include ejson generated secrets in discovery log
DazWorrall
pushed a commit
that referenced
this pull request
Feb 28, 2019
Thanks so much @benlangfeld for your help with these: * Remove excess logging * Stop referring to EJSON secrets as generically "managed" * Consistent timeout for Secret resources Unifying the constant used for simple resources of this type is left as an exercise for another change, mostly because the name of such a thing may be controversial and I don't want to block merging this on detailed review. * Give secrets the same statsd tags as other resources * Removes duplicate spec Doesn't test anything more than test_create_and_update_secrets_from_ejson * Replaces log assertion * Point out breaking changes in CHANGELOG * Include ejson generated secrets in discovery log
DazWorrall
pushed a commit
that referenced
this pull request
Mar 1, 2019
Thanks so much @benlangfeld for your help with these: * Remove excess logging * Stop referring to EJSON secrets as generically "managed" * Consistent timeout for Secret resources Unifying the constant used for simple resources of this type is left as an exercise for another change, mostly because the name of such a thing may be controversial and I don't want to block merging this on detailed review. * Give secrets the same statsd tags as other resources * Removes duplicate spec Doesn't test anything more than test_create_and_update_secrets_from_ejson * Replaces log assertion * Point out breaking changes in CHANGELOG * Include ejson generated secrets in discovery log
DazWorrall
pushed a commit
that referenced
this pull request
Mar 1, 2019
Thanks so much @benlangfeld for your help with these: * Remove excess logging * Stop referring to EJSON secrets as generically "managed" * Consistent timeout for Secret resources Unifying the constant used for simple resources of this type is left as an exercise for another change, mostly because the name of such a thing may be controversial and I don't want to block merging this on detailed review. * Give secrets the same statsd tags as other resources * Removes duplicate spec Doesn't test anything more than test_create_and_update_secrets_from_ejson * Replaces log assertion * Point out breaking changes in CHANGELOG * Include ejson generated secrets in discovery log
KnVerey
pushed a commit
that referenced
this pull request
Mar 1, 2019
* Refactor EjsonSecretProvisioner to provide resources * Add `Secret` resource * Refactor kubectl sensitive output config * Deploy resources from EjsonSecretProvisioner * Secrets as resources review fixes (#425) Thanks so much @benlangfeld for your help with these: * Remove excess logging * Stop referring to EJSON secrets as generically "managed" * Consistent timeout for Secret resources Unifying the constant used for simple resources of this type is left as an exercise for another change, mostly because the name of such a thing may be controversial and I don't want to block merging this on detailed review. * Give secrets the same statsd tags as other resources * Removes duplicate spec Doesn't test anything more than test_create_and_update_secrets_from_ejson * Replaces log assertion * Point out breaking changes in CHANGELOG * Include ejson generated secrets in discovery log * Misc review feedback * Rebase on master * Fix changelog after rebase * Remove unnecessary logging * Fix unknown Secret status * Update test name to not include 'update' * Write new unrecognized resource test * Hide some validation and deploy output when deploying sensitive resources * Better logging when handling failures with sensitive resources * Typo * Catch more specific Kubectl error output * Suppresses error messages on invalid sensitive templates * This is not flaky any more (finer-grained assertion on kubectl error) * Minor review fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Addresses some code review comments from #424