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

Implement BucketPolicyOnly #4404

Merged
merged 14 commits into from
Feb 6, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,12 @@ public Builder setRetentionPeriod(Long retentionPeriod) {
return this;
}

@Override
public Builder setIamConfiguration(IamConfiguration iamConfiguration) {
infoBuilder.setIamConfiguration(iamConfiguration);
return this;
}

@Override
public Bucket build() {
return new Bucket(storage, infoBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,118 @@ public com.google.api.services.storage.model.Bucket apply(BucketInfo bucketInfo)
private final Long retentionEffectiveTime;
private final Boolean retentionPolicyIsLocked;
private final Long retentionPeriod;
private final IamConfiguration iamConfiguration;

/**
* The Bucket's IAM Configuration.
*
* @see <a href="https://cloud.google.com/storage/docs/bucket-policy-only">Bucket Policy Only</a>
*/
public static class IamConfiguration implements Serializable {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add unit tests for IamConfiguration?

private static final long serialVersionUID = -8671736104909424616L;

private Boolean isBucketPolicyOnlyEnabled;
private Long bucketPolicyOnlyLockedTime;

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) {
return false;
}
IamConfiguration other = (IamConfiguration) o;
return Objects.equals(toPb(), other.toPb());
}

@Override
public int hashCode() {
return Objects.hash(isBucketPolicyOnlyEnabled, bucketPolicyOnlyLockedTime);
}

private IamConfiguration(Builder builder) {
this.isBucketPolicyOnlyEnabled = builder.isBucketPolicyOnlyEnabled;
this.bucketPolicyOnlyLockedTime = builder.bucketPolicyOnlyLockedTime;
}

public static Builder newBuilder() {
return new Builder();
}

public Builder toBuilder() {
Builder builder = new Builder();
builder.isBucketPolicyOnlyEnabled = isBucketPolicyOnlyEnabled;
builder.bucketPolicyOnlyLockedTime = bucketPolicyOnlyLockedTime;
return builder;
}

public Boolean isBucketPolicyOnlyEnabled() {
return isBucketPolicyOnlyEnabled;
}

public Long getBucketPolicyOnlyLockedTime() {
return bucketPolicyOnlyLockedTime;
}

Bucket.IamConfiguration toPb() {
Bucket.IamConfiguration iamConfiguration = new Bucket.IamConfiguration();

Bucket.IamConfiguration.BucketPolicyOnly bucketPolicyOnly =
new Bucket.IamConfiguration.BucketPolicyOnly();
bucketPolicyOnly.setEnabled(isBucketPolicyOnlyEnabled);
bucketPolicyOnly.setLockedTime(
bucketPolicyOnlyLockedTime == null ? null : new DateTime(bucketPolicyOnlyLockedTime));

iamConfiguration.setBucketPolicyOnly(bucketPolicyOnly);

return iamConfiguration;
}

static IamConfiguration fromPb(Bucket.IamConfiguration iamConfiguration) {
Bucket.IamConfiguration.BucketPolicyOnly bucketPolicyOnly =
iamConfiguration.getBucketPolicyOnly();
DateTime lockedTime = bucketPolicyOnly.getLockedTime();

return newBuilder()
.setIsBucketPolicyOnlyEnabled(bucketPolicyOnly.getEnabled())
.setLockedTime(lockedTime == null ? null : lockedTime.getValue())
.build();
}

/** Builder for {@code IamConfiguration} */
public static class Builder {
private Boolean isBucketPolicyOnlyEnabled;
private Long bucketPolicyOnlyLockedTime;

/**
* Sets whether BucketPolicyOnly is enabled for this bucket. When this is enabled, access to
* the bucket will be configured through IAM, and legacy ACL policies will not work. When this
* is first enabled, {@code lockedTime} will be set by the API automatically. This field can
Copy link
Member

Choose a reason for hiding this comment

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

Could you use bucketPolicyOnly.lockedTime as it will help scope what this belongs within IamConfiguration.

* then be disabled until the time specified, after which it will become immutable and calls
* to change it will fail. If this is enabled, calls to access legacy ACL information will
* fail.
*/
public Builder setIsBucketPolicyOnlyEnabled(boolean isBucketPolicyOnlyEnabled) {
Copy link
Member

Choose a reason for hiding this comment

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

Used the boxed type Boolean

this.isBucketPolicyOnlyEnabled = isBucketPolicyOnlyEnabled;
return this;
}

/**
* Sets the deadline for switching {@code enabled} back to false. After this time passes,
Copy link
Member

Choose a reason for hiding this comment

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

Could you use bucketPolicyOnly.enabled

* calls to do so will fail. This is package-private, since in general this field should never
* be set by a user--it's automatically set by the backend when {@code enabled} is set to
* true.
*/
Builder setLockedTime(Long bucketPolicyOnlyLockedTime) {
Copy link
Member

Choose a reason for hiding this comment

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

setBucketPolicyOnlyLockedTime

this.bucketPolicyOnlyLockedTime = bucketPolicyOnlyLockedTime;
return this;
}

/** Builds an {@code IamConfiguration} object */
public IamConfiguration build() {
return new IamConfiguration(this);
}
}
}

/**
* Lifecycle rule for a bucket. Allows supported Actions, such as deleting and changing storage
Expand Down Expand Up @@ -786,6 +898,15 @@ public abstract static class Builder {
@BetaApi
public abstract Builder setRetentionPeriod(Long retentionPeriod);

/**
* Sets the IamConfiguration to specify whether IAM access should be enabled.
*
* @see <a href="https://cloud.google.com/storage/docs/bucket-policy-only">Bucket Policy
* Only</a>
*/
@BetaApi
public abstract Builder setIamConfiguration(IamConfiguration iamConfiguration);

/** Creates a {@code BucketInfo} object. */
public abstract BucketInfo build();
}
Expand Down Expand Up @@ -816,6 +937,7 @@ static final class BuilderImpl extends Builder {
private Long retentionEffectiveTime;
private Boolean retentionPolicyIsLocked;
private Long retentionPeriod;
private IamConfiguration iamConfiguration;

BuilderImpl(String name) {
this.name = name;
Expand Down Expand Up @@ -846,6 +968,7 @@ static final class BuilderImpl extends Builder {
retentionEffectiveTime = bucketInfo.retentionEffectiveTime;
retentionPolicyIsLocked = bucketInfo.retentionPolicyIsLocked;
retentionPeriod = bucketInfo.retentionPeriod;
iamConfiguration = bucketInfo.iamConfiguration;
}

@Override
Expand Down Expand Up @@ -998,6 +1121,12 @@ public Builder setRetentionPeriod(Long retentionPeriod) {
return this;
}

@Override
public Builder setIamConfiguration(IamConfiguration iamConfiguration) {
this.iamConfiguration = iamConfiguration;
return this;
}

@Override
public BucketInfo build() {
checkNotNull(name);
Expand Down Expand Up @@ -1030,6 +1159,7 @@ public BucketInfo build() {
retentionEffectiveTime = builder.retentionEffectiveTime;
retentionPolicyIsLocked = builder.retentionPolicyIsLocked;
retentionPeriod = builder.retentionPeriod;
iamConfiguration = builder.iamConfiguration;
}

/** Returns the service-generated id for the bucket. */
Expand Down Expand Up @@ -1268,6 +1398,12 @@ public Long getRetentionPeriod() {
return retentionPeriod;
}

/** Returns the IAM configuration */
@BetaApi
public IamConfiguration getIamConfiguration() {
return iamConfiguration;
}

/** Returns a builder for the current bucket. */
public Builder toBuilder() {
return new BuilderImpl(this);
Expand Down Expand Up @@ -1405,6 +1541,9 @@ public Rule apply(LifecycleRule lifecycleRule) {
bucketPb.setRetentionPolicy(retentionPolicy);
}
}
if (iamConfiguration != null) {
bucketPb.setIamConfiguration(iamConfiguration.toPb());
}

return bucketPb;
}
Expand Down Expand Up @@ -1526,6 +1665,10 @@ public DeleteRule apply(Rule rule) {
builder.setRetentionPeriod(retentionPolicy.getRetentionPeriod());
}
}
Bucket.IamConfiguration iamConfiguration = bucketPb.getIamConfiguration();
if (iamConfiguration != null) {
builder.setIamConfiguration(IamConfiguration.fromPb(iamConfiguration));
}
return builder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ enum BucketField implements FieldSelector {
ENCRYPTION("encryption"),
BILLING("billing"),
DEFAULT_EVENT_BASED_HOLD("defaultEventBasedHold"),
RETENTION_POLICY("retentionPolicy");
RETENTION_POLICY("retentionPolicy"),
IAMCONFIGURATION("iamConfiguration");

static final List<? extends FieldSelector> REQUIRED_FIELDS = ImmutableList.of(NAME);

Expand Down Expand Up @@ -204,6 +205,14 @@ public static BucketTargetOption metagenerationNotMatch() {
public static BucketTargetOption userProject(String userProject) {
return new BucketTargetOption(StorageRpc.Option.USER_PROJECT, userProject);
}

/**
* Returns an option to define the projection in the API request. In some cases this option may be needed to be
* set to `noAcl` to omit ACL data from the response.
Copy link
Member

Choose a reason for hiding this comment

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

State the default value is "full" as well. Add the reference link as well. https://cloud.google.com/storage/docs/json_api/v1/buckets/patch

*/
public static BucketTargetOption projection(String projection) {
return new BucketTargetOption(StorageRpc.Option.PROJECTION, projection);
}
}

/** Class for specifying bucket source options. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@

public class HttpStorageRpc implements StorageRpc {
public static final String DEFAULT_PROJECTION = "full";
public static final String NO_ACL_PROJECTION = "noAcl";
private static final String ENCRYPTION_KEY_PREFIX = "x-goog-encryption-";
private static final String SOURCE_ENCRYPTION_KEY_PREFIX = "x-goog-copy-source-encryption-";

Expand Down Expand Up @@ -450,10 +451,22 @@ public Bucket patch(Bucket bucket, Map<Option, ?> options) {
Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_PATCH_BUCKET);
Scope scope = tracer.withSpan(span);
try {
String projection = Option.PROJECTION.getString(options);

if (bucket.getIamConfiguration() != null && bucket.getIamConfiguration().getBucketPolicyOnly() != null
&& bucket.getIamConfiguration().getBucketPolicyOnly().getEnabled()) {
//If BucketPolicyOnly is enabled, patch calls will fail if ACL information is included in the request
bucket.setDefaultObjectAcl(null);
bucket.setAcl(null);

if (projection == null) {
projection = NO_ACL_PROJECTION;
}
}
return storage
.buckets()
.patch(bucket.getName(), bucket)
.setProjection(DEFAULT_PROJECTION)
.setProjection(projection == null ? DEFAULT_PROJECTION : projection)
.setPredefinedAcl(Option.PREDEFINED_ACL.getString(options))
.setPredefinedDefaultObjectAcl(Option.PREDEFINED_DEFAULT_OBJECT_ACL.getString(options))
.setIfMetagenerationMatch(Option.IF_METAGENERATION_MATCH.getLong(options))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ enum Option {
IF_SOURCE_GENERATION_NOT_MATCH("ifSourceGenerationNotMatch"),
IF_DISABLE_GZIP_CONTENT("disableGzipContent"),
PREFIX("prefix"),
PROJECTION("projection"),
MAX_RESULTS("maxResults"),
PAGE_TOKEN("pageToken"),
DELIMITER("delimiter"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@

import static com.google.cloud.storage.Acl.Project.ProjectRole.VIEWERS;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import com.google.api.services.storage.model.Bucket;
import com.google.api.services.storage.model.Bucket.Lifecycle.Rule;
import com.google.cloud.storage.Acl.Project;
import com.google.cloud.storage.Acl.Role;
Expand Down Expand Up @@ -64,6 +66,11 @@ public class BucketInfoTest {
LifecycleAction.newDeleteAction(),
LifecycleCondition.newBuilder().setAge(5).build()));
private static final String INDEX_PAGE = "index.html";
private static final BucketInfo.IamConfiguration IAM_CONFIGURATION =
BucketInfo.IamConfiguration.newBuilder()
.setIsBucketPolicyOnlyEnabled(true)
.setLockedTime(System.currentTimeMillis())
.build();
private static final String NOT_FOUND_PAGE = "error.html";
private static final String LOCATION = "ASIA";
private static final StorageClass STORAGE_CLASS = StorageClass.STANDARD;
Expand All @@ -90,6 +97,7 @@ public class BucketInfoTest {
.setDeleteRules(DELETE_RULES)
.setLifecycleRules(LIFECYCLE_RULES)
.setIndexPage(INDEX_PAGE)
.setIamConfiguration(IAM_CONFIGURATION)
.setNotFoundPage(NOT_FOUND_PAGE)
.setLocation(LOCATION)
.setStorageClass(STORAGE_CLASS)
Expand Down Expand Up @@ -139,6 +147,7 @@ public void testBuilder() {
assertEquals(DEFAULT_ACL, BUCKET_INFO.getDefaultAcl());
assertEquals(DELETE_RULES, BUCKET_INFO.getDeleteRules());
assertEquals(INDEX_PAGE, BUCKET_INFO.getIndexPage());
assertEquals(IAM_CONFIGURATION, BUCKET_INFO.getIamConfiguration());
assertEquals(NOT_FOUND_PAGE, BUCKET_INFO.getNotFoundPage());
assertEquals(LOCATION, BUCKET_INFO.getLocation());
assertEquals(STORAGE_CLASS, BUCKET_INFO.getStorageClass());
Expand Down Expand Up @@ -174,6 +183,7 @@ private void compareBuckets(BucketInfo expected, BucketInfo value) {
assertEquals(expected.getDeleteRules(), value.getDeleteRules());
assertEquals(expected.getLifecycleRules(), value.getLifecycleRules());
assertEquals(expected.getIndexPage(), value.getIndexPage());
assertEquals(expected.getIamConfiguration(), value.getIamConfiguration());
assertEquals(expected.getNotFoundPage(), value.getNotFoundPage());
assertEquals(expected.getLocation(), value.getLocation());
assertEquals(expected.getStorageClass(), value.getStorageClass());
Expand Down Expand Up @@ -244,4 +254,16 @@ public void testLifecycleRules() {
assertTrue(setStorageClassLifecycleRule.getCondition().getIsLive());
assertEquals(10, setStorageClassLifecycleRule.getCondition().getNumNewerVersions().intValue());
}

@Test
public void testIamConfiguration() {
Bucket.IamConfiguration iamConfiguration = BucketInfo.IamConfiguration.newBuilder()
.setIsBucketPolicyOnlyEnabled(true)
.setLockedTime(System.currentTimeMillis())
.build()
.toPb();

assertEquals(Boolean.TRUE, iamConfiguration.getBucketPolicyOnly().getEnabled());
assertNotNull(iamConfiguration.getBucketPolicyOnly().getLockedTime());
JesseLovelace marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading