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

fix: do not cause a failure when encountering no bindings #1177

Merged
merged 2 commits into from Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -34,8 +34,8 @@ class PolicyHelper {
static Policy convertFromApiPolicy(com.google.api.services.storage.model.Policy apiPolicy) {
Policy.Builder policyBuilder = Policy.newBuilder();
List<Bindings> bindings = apiPolicy.getBindings();
ImmutableList.Builder<Binding> coreBindings = ImmutableList.builder();
if (null != bindings && !bindings.isEmpty()) {
ImmutableList.Builder<Binding> coreBindings = ImmutableList.builder();
for (Bindings binding : bindings) {
Binding.Builder bindingBuilder = Binding.newBuilder();
bindingBuilder.setRole(binding.getRole());
Expand All @@ -49,10 +49,8 @@ static Policy convertFromApiPolicy(com.google.api.services.storage.model.Policy
}
coreBindings.add(bindingBuilder.build());
}
policyBuilder.setBindings(coreBindings.build());
} else {
throw new IllegalStateException("Missing required bindings.");
}
policyBuilder.setBindings(coreBindings.build());
return policyBuilder.setEtag(apiPolicy.getEtag()).setVersion(apiPolicy.getVersion()).build();
}

Expand Down
Expand Up @@ -3540,6 +3540,10 @@ HmacKeyMetadata updateHmacKeyState(
/**
* Gets the IAM policy for the provided bucket.
*
* <p>It's possible for bindings to be empty and instead have permissions inherited through
* Project or Organization IAM Policies. To prevent corrupting policies when performing a {@code
Copy link
Collaborator

Choose a reason for hiding this comment

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

"To prevent corrupting policies when you update an IAM policy with {@code Storage.setIamPolicy, the ETAG value..."

* Storage.setIamPolicy}, the ETAG value is used to perform optimistic concurrency.
*
* <p>Example of getting the IAM policy for a bucket.
*
* <pre>{@code
Expand All @@ -3556,6 +3560,9 @@ HmacKeyMetadata updateHmacKeyState(
/**
* Updates the IAM policy on the specified bucket.
*
* <p>To prevent corrupting policies when performing a {@code Storage.setIamPolicy}, the ETAG
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as above

* value is used to perform optimistic concurrency.
*
* <p>Example of updating the IAM policy on a bucket.
*
* <pre>{@code
Expand Down
Expand Up @@ -65,11 +65,15 @@ public void testEquivalence() {
assertTrue(new ApiPolicyMatcher(apiPolicy).matches(actualApiPolicy));
}

@Test(expected = IllegalStateException.class)
@Test
public void testApiPolicyWithoutBinding() {
List<Bindings> bindings = null;
com.google.api.services.storage.model.Policy apiPolicy =
new com.google.api.services.storage.model.Policy().setBindings(bindings).setEtag(ETAG);
PolicyHelper.convertFromApiPolicy(apiPolicy);
new com.google.api.services.storage.model.Policy()
.setBindings(bindings)
.setEtag(ETAG)
.setVersion(1);
Policy policy = PolicyHelper.convertFromApiPolicy(apiPolicy);
assertEquals(policy.getBindings().size(), 0);
}
}