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

Entities may have duplicate policies #12812

Merged
merged 5 commits into from Oct 22, 2021
Merged

Entities may have duplicate policies #12812

merged 5 commits into from Oct 22, 2021

Conversation

hghaf099
Copy link
Contributor

@hghaf099 hghaf099 commented Oct 12, 2021 •
edited

Fixes #10847

@vercel vercel bot temporarily deployed to Preview – vault October 12, 2021 22:35 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 12, 2021 22:35 Inactive
ncabatoff
ncabatoff reviewed Oct 13, 2021
View reviewed changes
changelog/12812.txt Outdated Show resolved Hide resolved
ncabatoff
ncabatoff approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@ncabatoff ncabatoff left a comment

Choose a reason for hiding this comment

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

This will prevent new duplicate policies - do we also want to filter out duplicates on query endpoints, so that any existing entity policy dups aren't exposed to the user?

swayne275
swayne275 approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@swayne275 swayne275 left a comment

Choose a reason for hiding this comment

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

i agree with nick's comment

@hghaf099 hghaf099 changed the title Entities may have duplicate plicies Entities may have duplicate policies Oct 13, 2021
vishalnayak
vishalnayak reviewed Oct 13, 2021
View reviewed changes
vault/identity_store_entities.go Outdated
@@ -238,7 +238,8 @@ func (i *IdentityStore) handleEntityUpdateCommon() framework.OperationFunc {
// Update the policies if supplied
entityPoliciesRaw, ok := d.GetOk("policies")
if ok {
entity.Policies = entityPoliciesRaw.([]string)
filteredPolicies := strutil.RemoveDuplicates(entityPoliciesRaw.([]string), false)
Copy link
Member

Choose a reason for hiding this comment

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

filteredPolicies var can be avoided by direct assignment to entity.Policies.

@vercel vercel bot temporarily deployed to Preview – vault-storybook October 13, 2021 16:23 Inactive
@vercel vercel bot temporarily deployed to Preview – vault October 13, 2021 16:23 Inactive
@hghaf099
Copy link
Contributor Author

Thanks for the comments. I am trying to fix the query endpoints, and I have found handleEntityReadCommon and a line in mergeEntity. And some other ones are using policyutil.ParsePolicies which already removes the duplicates. Where else I could look for those endpoints?

@hghaf099 hghaf099 force-pushed the remove-duplicate-policies-on-creating-entities branch from 7f1cc67 to e8da9be Compare October 14, 2021 00:16
@vercel vercel bot temporarily deployed to Preview – vault October 14, 2021 00:18 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook October 14, 2021 00:27 Inactive
@hghaf099 hghaf099 merged commit 6c03006 into main Oct 22, 2021
@hghaf099 hghaf099 deleted the remove-duplicate-policies-on-creating-entities branch October 22, 2021 23:28
qk4l pushed a commit to qk4l/vault that referenced this pull request Feb 4, 2022
@hghaf099
Entities may have duplicate policies (hashicorp#12812)
e91aa6a 
* Entities may have duplicate plicies

* Adding changelog

* removing duplicates on reading entity policies

* fix changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vishalnayak vishalnayak vishalnayak approved these changes

@ncabatoff ncabatoff ncabatoff approved these changes

@swayne275 swayne275 swayne275 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Entities may have duplicate policies
4 participants
@hghaf099 @vishalnayak @ncabatoff @swayne275