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.

Sign up for GitHub

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

Jump to bottom

VAULT-13763 normalize activity log mount paths #19343

Merged
merged 3 commits into from Feb 24, 2023
Merged

VAULT-13763 normalize activity log mount paths #19343

merged 3 commits into from Feb 24, 2023

Conversation

miagilepner
Copy link
Contributor

Mount paths "auth/foo/" and "auth/foo" should both be represented as "auth/foo/" by the activity log responses, with the clients in those paths getting correctly deduplicated.

I didn't create a changelog entry since there's already one for the original change: #18916

@miagilepner miagilepner added pr/no-changelog backport/1.13.x Backport changes to `release/1.13.x` labels Feb 24, 2023
@miagilepner miagilepner added this to the 1.13.0 milestone Feb 24, 2023
mpalmi
mpalmi approved these changes Feb 24, 2023
View reviewed changes
Copy link
Contributor

@mpalmi mpalmi left a comment

Choose a reason for hiding this comment

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

Looks good to me!

hellobontempo
hellobontempo reviewed Feb 24, 2023
View reviewed changes
Copy link
Contributor

@hellobontempo hellobontempo left a comment

Choose a reason for hiding this comment

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

I'm curious about the comment on line 269 - this is no longer true since the mount accessor doesn't replace the path anymore, right? Just want to confirm my understanding! 😁

@miagilepner
Copy link
Contributor Author

I'm curious about the comment on line 269 - this is no longer true since the mount accessor doesn't replace the path anymore, right? Just want to confirm my understanding! 😁

The comment on mountAccessorToMountPath is still true - the mount accessor still gets transformed to a mount path, and that's returned. But that mount path will always have a / on the end of it, so we won't have that issue where we end up with both auth/token and auth/token/ in the mounts array.

@miagilepner miagilepner merged commit a9e17c2 into main Feb 24, 2023
Merged
@raskchanky raskchanky deleted the miagilepner/VAULT-13763-mount-path-activitylog branch February 24, 2023 17:07
jayant07-yb pushed a commit to jayant07-yb/hashicorp-vault-integrations that referenced this pull request Mar 15, 2023
@miagilepner
VAULT-13763 normalize activity log mount paths (hashicorp#19343)
9752307 
* add slashes to mount paths in activity log

* cleanup test

* fix test
mpalmi pushed a commit that referenced this pull request Jun 13, 2023
@miagilepner @mpalmi
VAULT-13763 normalize activity log mount paths (#19343)
72c8730 
* add slashes to mount paths in activity log

* cleanup test

* fix test

(cherry picked from commit a9e17c2)
This was referenced Jun 13, 2023
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hellobontempo hellobontempo hellobontempo left review comments

@mpalmi mpalmi mpalmi approved these changes

@raskchanky raskchanky Awaiting requested review from raskchanky

@ncabatoff ncabatoff Awaiting requested review from ncabatoff

Assignees
No one assigned
Labels
backport/1.13.x Backport changes to `release/1.13.x` pr/no-changelog
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@miagilepner @mpalmi @hellobontempo