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

Vault 5917 allow patch operations to pki roles issuers #15510

Merged

Conversation

kitography
Copy link
Contributor

PATCH functionality for Roles and Issuers

@kitography kitography marked this pull request as ready for review May 20, 2022 15:12
@cipherboy cipherboy requested review from stevendpclark and cipherboy and removed request for stevendpclark May 20, 2022 16:20
Copy link
Contributor

@cipherboy cipherboy left a comment

Choose a reason for hiding this comment

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

This largely looks good! I have passing tests for role based patching but I think we can add this in a separate PR.

Thank you @kitography!

// When the new name is in use but isn't this name, throw an error.
return logical.ErrorResponse(err.Error()), nil
}
if newName != issuer.Name {
Copy link
Contributor

Choose a reason for hiding this comment

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

Verify if we are writing a new name, if non-zero, that it matches nameMatcher.MatchString(newName) so that we have valid names to be use within URLs.

I'm okay with deferring this to another PR as we are missing it here and within the pathUpdateIssuer handler.

Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

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

👍

@kitography kitography merged commit bdd7d7d into main May 20, 2022
Gabrielopesantos pushed a commit to Gabrielopesantos/vault that referenced this pull request Jun 6, 2022
* Add a warning when Issuing Certificate set on a role does not resolve.

* Ivanka's requests - add a warning on deleting issuer or changing it's name.

* Fix nil checks; reduce number of roles to iterate through; only verify roles after migration.

* Fix semgrep failure, ignore roles deleted behind our back.

* Patch functionality for roles

* Make Patch Roles work again, add back patch issuers.

* Add changelog.

* Fix nil-reversion on empty response.

* Panics are bad. don't do that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants