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

feat: implement GrpcStorageImpl#getDefaultAcl #1802

Merged
merged 1 commit into from
Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ public List<Acl> listAcls() {
*
* @throws StorageException upon failure
*/
@TransportCompatibility({Transport.HTTP})
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
public Acl getDefaultAcl(Entity entity) {
return storage.getDefaultAcl(getName(), entity);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,10 @@ private Acl.Entity entityDecode(String from) {
private Acl objectAclDecode(ObjectAccessControl from) {
Acl.Role role = Acl.Role.valueOf(from.getRole());
Acl.Entity entity = entityCodec.decode(from.getEntity());
Acl.Builder to = Acl.newBuilder(entity, role).setId(from.getId());
Acl.Builder to = Acl.newBuilder(entity, role);
if (!from.getId().isEmpty()) {
to.setId(from.getId());
}
if (!from.getEtag().isEmpty()) {
to.setEtag(from.getEtag());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.cloud.storage.ByteSizeConstants._16MiB;
import static com.google.cloud.storage.ByteSizeConstants._256KiB;
import static com.google.cloud.storage.GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes;
import static com.google.cloud.storage.StorageV2ProtoUtils.objectAclEntityOrAltEq;
import static com.google.cloud.storage.Utils.bucketNameCodec;
import static com.google.cloud.storage.Utils.ifNonNull;
import static com.google.cloud.storage.Utils.projectNameCodec;
Expand Down Expand Up @@ -54,6 +55,7 @@
import com.google.cloud.storage.UnifiedOpts.BucketListOpt;
import com.google.cloud.storage.UnifiedOpts.BucketSourceOpt;
import com.google.cloud.storage.UnifiedOpts.BucketTargetOpt;
import com.google.cloud.storage.UnifiedOpts.Fields;
import com.google.cloud.storage.UnifiedOpts.HmacKeyListOpt;
import com.google.cloud.storage.UnifiedOpts.HmacKeySourceOpt;
import com.google.cloud.storage.UnifiedOpts.HmacKeyTargetOpt;
Expand Down Expand Up @@ -89,6 +91,7 @@
import com.google.storage.v2.ListObjectsRequest;
import com.google.storage.v2.LockBucketRetentionPolicyRequest;
import com.google.storage.v2.Object;
import com.google.storage.v2.ObjectAccessControl;
import com.google.storage.v2.ProjectName;
import com.google.storage.v2.ReadObjectRequest;
import com.google.storage.v2.RewriteObjectRequest;
Expand Down Expand Up @@ -125,6 +128,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.Spliterator;
import java.util.Spliterators.AbstractSpliterator;
Expand Down Expand Up @@ -903,7 +907,45 @@ public List<Acl> listAcls(String bucket) {

@Override
public Acl getDefaultAcl(String bucket, Entity entity) {
return throwNotYetImplemented(fmtMethodName("getDefaultAcl", String.class, Entity.class));
// Specify the read-mask to explicitly include defaultObjectAcl
Fields fields =
UnifiedOpts.fields(
ImmutableSet.of(
BucketField.ACL, // workaround for b/261771961
BucketField.DEFAULT_OBJECT_ACL));
GrpcCallContext grpcCallContext = GrpcCallContext.createDefault();
GetBucketRequest req =
fields
.getBucket()
.apply(GetBucketRequest.newBuilder())
.setName(bucketNameCodec.encode(bucket))
.build();
try {
com.google.storage.v2.Bucket resp =
Retrying.run(
getOptions(),
retryAlgorithmManager.getFor(req),
() -> storageClient.getBucketCallable().call(req, grpcCallContext),
Decoder.identity());

Predicate<ObjectAccessControl> entityPredicate =
objectAclEntityOrAltEq(codecs.entity().encode(entity));

//noinspection DataFlowIssue
Optional<ObjectAccessControl> first =
resp.getDefaultObjectAclList().stream().filter(entityPredicate).findFirst();

// HttpStorageRpc defaults to null if Not Found
return first.map(codecs.objectAcl()::decode).orElse(null);
} catch (NotFoundException e) {
return null;
} catch (StorageException se) {
if (se.getCode() == 404) {
return null;
} else {
throw se;
}
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3487,7 +3487,7 @@ PostPolicyV4 generateSignedPostPolicyV4(
*
* @throws StorageException upon failure
*/
@TransportCompatibility({Transport.HTTP})
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
Acl getDefaultAcl(String bucket, Entity entity);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import com.google.protobuf.MessageOrBuilder;
import com.google.protobuf.util.JsonFormat;
import com.google.protobuf.util.JsonFormat.Printer;
import com.google.storage.v2.ObjectAccessControl;
import com.google.storage.v2.ReadObjectRequest;
import java.util.function.Predicate;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand Down Expand Up @@ -82,4 +84,12 @@ static String fmtProto(@NonNull final MessageOrBuilder msg) {
throw new RuntimeException(e);
}
}

/**
* When evaluating an {@link ObjectAccessControl} entity, look at both {@code entity} (generally
* project number format) and {@code entity_alt} (generally project id format).
*/
static Predicate<ObjectAccessControl> objectAclEntityOrAltEq(String s) {
return oAcl -> oAcl.getEntity().equals(s) || oAcl.getEntityAlt().equals(s);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import static org.junit.Assert.assertThrows;

import com.google.cloud.storage.jqwik.StorageArbitraries;
import com.google.storage.v2.ObjectAccessControl;
import com.google.storage.v2.ReadObjectRequest;
import java.util.function.Predicate;
import net.jqwik.api.Arbitraries;
import net.jqwik.api.Arbitrary;
import net.jqwik.api.Combinators;
Expand Down Expand Up @@ -66,6 +68,20 @@ void validation_limit_gteq_0() {
() -> seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), null, -1L));
}

@Example
void objectAclEntityIdOrAltEq() {
String entity = "project-viewer-123123";
Predicate<ObjectAccessControl> p = StorageV2ProtoUtils.objectAclEntityOrAltEq(entity);

ObjectAccessControl inAlt =
ObjectAccessControl.newBuilder().setEntity("something").setEntityAlt(entity).build();
ObjectAccessControl inPrimary =
ObjectAccessControl.newBuilder().setEntity(entity).setEntityAlt("something-else").build();

assertThat(p.test(inAlt)).isTrue();
assertThat(p.test(inPrimary)).isTrue();
}

@Property(tries = 100_000)
void seek(@ForAll("seekCases") SeekCase srr) {
Long offset = srr.offset;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.Storage.BlobTargetOption;
import com.google.cloud.storage.Storage.BucketField;
import com.google.cloud.storage.Storage.BucketGetOption;
import com.google.cloud.storage.Storage.BucketSourceOption;
import com.google.cloud.storage.Storage.BucketTargetOption;
import com.google.cloud.storage.StorageException;
Expand Down Expand Up @@ -125,11 +126,43 @@ private void testBucketAclRequesterPays(
assertNull(storage.getAcl(bucket.getName(), User.ofAllAuthenticatedUsers(), bucketOptions));
}

@Test
public void bucket_defaultAcl_get() {
String bucketName = bucket.getName();
// lookup an entity from the bucket which is known to exist
Bucket bucketWithAcls =
storage.get(
bucketName, BucketGetOption.fields(BucketField.ACL, BucketField.DEFAULT_OBJECT_ACL));

Acl actual = bucketWithAcls.getDefaultAcl().iterator().next();

Acl acl = retry429s(() -> storage.getDefaultAcl(bucketName, actual.getEntity()), storage);

assertThat(acl).isEqualTo(actual);
}

/** When a bucket does exist, but an acl for the specified entity is not defined return null */
@Test
public void bucket_defaultAcl_get_notFoundReturnsNull() {
Acl acl = retry429s(() -> storage.getDefaultAcl(bucket.getName(), User.ofAllUsers()), storage);

assertThat(acl).isNull();
}

/** When a bucket doesn't exist, return null for the acl value */
@Test
public void bucket_defaultAcl_get_bucket404() {
Acl acl =
retry429s(() -> storage.getDefaultAcl(bucket.getName() + "x", User.ofAllUsers()), storage);

assertThat(acl).isNull();
}

@Test
@CrossRun.Ignore(transports = Transport.GRPC)
public void testBucketDefaultAcl() {
// TODO: break this test up into each of the respective scenarios
// 1. get default ACL for specific entity
// DONE ~1. get default ACL for specific entity~
BenWhitehead marked this conversation as resolved.
Show resolved Hide resolved
// 2. Delete a default ACL for a specific entity
// 3. Create a default ACL for specific entity
// 4. Update default ACL to change role of a specific entity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public ImmutableList<?> parameters() {
BucketField.DEFAULT_OBJECT_ACL,
(jsonT, grpcT) -> {
assertThat(jsonT.getDefaultAcl()).isNotEmpty();
assertThat(grpcT.getDefaultAcl()).isNull();
assertThat(grpcT.getDefaultAcl()).isNull(); // workaround for b/261771961
}),
new Args<>(BucketField.ENCRYPTION, LazyAssertion.equal()),
new Args<>(
Expand Down