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

Fixed ACL user comparison. Resolves #33754. #33776

Merged
merged 2 commits into from
Jun 6, 2016

Conversation

danslimmon
Copy link
Contributor

What does this PR do?

Fixes the change detection of states.boto_s3_bucket.present.

What issues does this PR fix or reference?

This PR hopefully resolves #33754, but as there are other possible causes for that problem, the reporter will need to confirm. In my particular case at least, it fixes the problem described in #33754.

Previous Behavior

states.boto_s3_bucket.present was idempotent with respect to actual bucket settings, but in certain cases it would detect changes when there were none. This resulted in unnecessary API requests and incorrect output.

The problem lay in bucket ACL descriptions. When specifying an ACL to the AWS API, each user description must include a Type key (usually set to CanonicalUser). But when you query the API for an ACL, it sometimes omits the Type key. So the comparison between expected and received ACL descriptions was failing.

New Behavior

We normalize all user descriptions before comparison, removing the Type key from them.

Tests written?

Yes (just modified the unit test test_present_when_bucket_exists_no_mods that was already present)

@cachedout
Copy link
Contributor

Really nice fix here. Thanks, @danslimmon

@cachedout cachedout merged commit 94f98b4 into saltstack:2016.3 Jun 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants