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

Add method to set the default issuer #1198

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

stephan2012
Copy link

Add the missing method hvac.api.secrets_engines.pki.set_issuers() to configure the default PKI issuer.

Fixes #1197

Signed-off-by: Stephan Austermühle <[email protected]>
Signed-off-by: Stephan Austermühle <[email protected]>
Signed-off-by: Stephan Austermühle <[email protected]>
Signed-off-by: Stephan Austermühle <[email protected]>
@stephan2012 stephan2012 requested a review from a team as a code owner December 5, 2024 14:20
docs/conf.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@@ -798,6 +798,30 @@ def list_issuers(self, mount_point=DEFAULT_MOUNT_POINT):
url=api_path,
)

def set_issuers(self, params, mount_point=DEFAULT_MOUNT_POINT):
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should be probably be named set_issuers_configuration since it corresponds to that API: https://developer.hashicorp.com/vault/api-docs/secret/pki#set-issuers-configuration

We also don't want to take a generic params parameter. Instead we should accept a parameter called default (or maybe default_issuer to be more specific), and a default_follows_latest_issuer parameter so that these can be validated and documented properly.

The tests should then reflect these changes and test for success and failure conditions.

Copy link
Author

Choose a reason for hiding this comment

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

Using set_issuers_configuration vs. set_issuers is somewhat inconsistent with list_issuers (titled Read issuers configuration in the API docs, by the way).

Copy link
Author

Choose a reason for hiding this comment

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

default_follows_latest_issuer is optional in the HTTP API. Taking it as an argument with an implicit default would probably unintentionally change the configured value.

@briantist briantist added enhancement a new feature or addition secrets engines generally related to a Vault secrets engine pki Related to the PKI secrets engine labels Dec 5, 2024
@briantist briantist added this to the 2.4.0 milestone Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a new feature or addition pki Related to the PKI secrets engine secrets engines generally related to a Vault secrets engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add method to set the default issuer
2 participants