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

secrets/db: enable skip auto import rotation of static roles #29093

Merged
merged 10 commits into from
Dec 12, 2024

Conversation

fairclothjm
Copy link
Contributor

@fairclothjm fairclothjm commented Dec 4, 2024

Description

This is the CE component of adding support to the DB engine for skipping auto rotation of static roles on import (VAULT-31657). Once this is merged there will be an Enterprise PR that enable usage of the feature and adds tests. We will be backporting this but I am going to wait until I can get the Enterprise pieces in place, tested and merged.

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

@fairclothjm fairclothjm requested a review from a team as a code owner December 4, 2024 18:32
@fairclothjm fairclothjm added this to the 1.18.3 milestone Dec 4, 2024
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Dec 4, 2024
Copy link

github-actions bot commented Dec 4, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Dec 4, 2024

Build Results:
All builds succeeded! ✅

Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

Looks great so far! Had a question around returning the param on a config read

@kpcraig
Copy link
Contributor

kpcraig commented Dec 4, 2024

Does this want a 1.18 backport tag if it's in the 1.18.3 milestone?

@fairclothjm
Copy link
Contributor Author

@vinay-gopalan @kpcraig You two are really on top of things! Thanks!

This wasn't quite ready for review so I didn't add any reviewers yet. I wanted to run the tests. But I think it should be ready now.

Does this want a 1.18 backport tag if it's in the 1.18.3 milestone?

We will be backporting this but I am going to wait until I can get the Enterprise pieces in place, tested and merged.

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>
builtin/logical/database/backend.go Dismissed Show dismissed Hide dismissed
builtin/logical/database/path_roles.go Dismissed Show dismissed Hide dismissed
vinay-gopalan
vinay-gopalan previously approved these changes Dec 5, 2024
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

LGTM!

vinay-gopalan
vinay-gopalan previously approved these changes Dec 10, 2024
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

LGTM!

builtin/logical/database/path_roles.go Outdated Show resolved Hide resolved
@fairclothjm fairclothjm removed this from the 1.18.3 milestone Dec 11, 2024
@fairclothjm fairclothjm enabled auto-merge (squash) December 12, 2024 01:17
@fairclothjm fairclothjm merged commit d411a44 into main Dec 12, 2024
92 checks passed
@fairclothjm fairclothjm deleted the VAULT-31658/ce/db-skip-static-import-rotation branch December 12, 2024 01:39
@fairclothjm fairclothjm added this to the 1.19.0-rc milestone Dec 12, 2024
Monkeychip pushed a commit that referenced this pull request Dec 18, 2024
* secrets/db: enable skip auto import rotation of static roles

* fix panic due to empty role name causing role to not be stored

* fix role upgrade test

* Apply suggestions from code review

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>

* use password in favor of self_managed_password

* add deprecated to self_managed_password field

* fix bug with allowing updates to password

---------

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>
fairclothjm added a commit that referenced this pull request Jan 7, 2025
* secrets/db: enable skip auto import rotation of static roles

* fix panic due to empty role name causing role to not be stored

* fix role upgrade test

* Apply suggestions from code review

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>

* use password in favor of self_managed_password

* add deprecated to self_managed_password field

* fix bug with allowing updates to password

---------

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>
fairclothjm added a commit that referenced this pull request Jan 8, 2025
* secrets/db: enable skip auto import rotation of static roles (#29093)

* secrets/db: enable skip auto import rotation of static roles

* fix panic due to empty role name causing role to not be stored

* fix role upgrade test

* Apply suggestions from code review

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>

* use password in favor of self_managed_password

* add deprecated to self_managed_password field

* fix bug with allowing updates to password

---------

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>

* db: fix skip-import-rotation/rootless integration (#29202)

* db: fix skip-import-rotation/rootless integration

* prevent setting both password and self_managed_password

* move func call and add comment

* db: allow updates to self_managed_password (#29283)

* test: fix ce/ent diff (#29307)

---------

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Co-authored-by: kpcraig <3031348+kpcraig@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants