Combine the beta and regular provisioner commands #673
Conversation
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
github-actions
bot
added
the
needs triage
maraino
requested changes
May 2, 2022
go.mod
Outdated
Comment on lines
194
to
195
| replace github.com/smallstep/certificates => ../certificates | ||
|
|
There was a problem hiding this comment.
Remember to change this, you can also point to a branch version of certificates so the CI can compile it.
dopey
added
needs docs
and removed
needs triage
github-actions
bot
added
the
needs triage
github-actions
bot
added
the
needs triage
maraino
requested changes
May 13, 2022
Comment on lines
78
to
80
| if err := client.auth.ReloadAdminResources(ctx); err != nil { | ||
| return nil, errors.Wrapf(err, "error loading provisioners from config") | ||
| } |
There was a problem hiding this comment.
Will this fail if we want to change a bad OIDC configuration present in the ca.json?
There was a problem hiding this comment.
Good catch! I think we have two general options here:
- I can build the provisioner collection my self and then set it on the Authority using something like With ProvisionerCollection - OR -
- I can lazy load the provisioner - which could have the benefit of fixing the bug where the CA becomes unusable if, for some reason, the OIDC configuration changes and breaks the currently stored one.
maraino
reviewed
May 18, 2022
maraino
approved these changes
May 20, 2022
- add back provisionerbeta with deprecation warnings - add deprecation warnings to adminbeta - place admin in new position
- additional cleanup and cleanup around loadProvisioner
- fix durationFlags defaulting to 0s values - remove no-private-key flag in favor of private-key="" - fix deprecation warnings for adminbeta and provisionerbeta
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.
Name of feature:
Improvement to provisioner CRUD commands
Pain or issue this feature alleviates:
Use the same commands to perform CRUD on provisioners - rather than having a separate
betasubcommand with different flags.Why is this important to the project (if not answered above):
Uniform implementation and execution. Prevents further drift and provides a unified experience for operators. Also, prevents the need for different sets of documentation that currently confuses users.
Is there documentation on how to use this feature? If so, where?
Yes, the existing provisioner documentation.
In what environments or workflows is this feature supported?
All of them.
In what environments or workflows is this feature explicitly NOT supported (if any)?
N/A
Supporting links/other PRs/issues:
Fixes smallstep/certificates#886
Fixes #676
💔Thank you!