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

[step_ca] Rework existing cert variables to be more readable and clear #308

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maxhoesel
Copy link
Collaborator

This change modifies the variables for initializing the CA with an existing key and certificate and makes them more readable.
This also removes the old, confusing behavior that was introduced when the role gained the ability to use both remote and local certificate files as a source. To simplify things, the names of the parameters have been updated and made more explicit.

Tests have also been added to ensure that all three paths (existing local, remote and no previous certificate) all work as expected.

Since this PR changes the names of variables without a fallback, this is a breaking change.
As this only affects initial CA setup, any impact on existing users should be minimal.
Updating the parameter names and values is all that is required for migrating to this new system

@maxhoesel maxhoesel added the pr-major This PR introduces a breaking change! A major release will be required label Jul 25, 2023
- If the file is present on the controller instead of the target node, set `step_ca_existing_<root/key>_is_local`, to `true`.
- Default: not set (will generate a new certificate)

##### `step_ca_existing_<root/key>_is_local`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of is_local, perhaps a _mode parameter would be better, with the default behavior of none and choices for local and remote. This would allow for easy extendability in the future

@maxhoesel maxhoesel added this to the version-1 milestone Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-major This PR introduces a breaking change! A major release will be required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant