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 4 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,186 @@ 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;

/**
* A Bucket's IAM Configuration. Allows specification of authorization policies via IAM instead of legacy ACL.
Copy link
Member

Choose a reason for hiding this comment

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

I'd leave this simplified to: The bucket's IAM configuration. per the discovery document.
https://www.googleapis.com/discovery/v1/apis/storage/v1/rest

*
* @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 final BucketPolicyOnly bucketPolicyOnly;

@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(bucketPolicyOnly.toPb(), other.getBucketPolicyOnly().toPb());
}

@Override
public int hashCode() {
return Objects.hash(bucketPolicyOnly);
}

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

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

public Builder toBuilder() {
Builder builder = new Builder();
builder.bucketPolicyOnly = this.bucketPolicyOnly;
return builder;
}

public BucketPolicyOnly getBucketPolicyOnly() {
return bucketPolicyOnly;
}

Bucket.IamConfiguration toPb() {
Bucket.IamConfiguration iamConfiguration = new Bucket.IamConfiguration();
iamConfiguration.setBucketPolicyOnly(this.bucketPolicyOnly.toPb());

return iamConfiguration;
}

static IamConfiguration fromPb(Bucket.IamConfiguration iamConfiguration) {
return newBuilder()
.setBucketPolicyOnly(BucketPolicyOnly.fromPb(iamConfiguration.getBucketPolicyOnly()))
.build();
}

/** Builder for {@code IamConfiguration} */
public static class Builder {
private BucketPolicyOnly bucketPolicyOnly;

/**
* Sets the BucketPolicyOnly object for this configuration. This object will determine whether IAM is enforced
* or not.
*/
public Builder setBucketPolicyOnly(BucketPolicyOnly bucketPolicyOnly) {
this.bucketPolicyOnly = bucketPolicyOnly;
return this;
}

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

/**
* Configuration of BucketPolicyOnly for a bucket. When this is configured to enable BucketPolicyOnly and included
* in an {@code IamConfiguration}, legacy ACL access to the bucket will be disabled, and all authorization policies
* will be configured through IAM.
*
* @see <a href="https://cloud.google.com/storage/docs/bucket-policy-only">Bucket Policy Only</a>
*/
public static class BucketPolicyOnly implements Serializable {
private static final long serialVersionUID = 2260445211318576550L;
private final Boolean enabled;
private final DateTime lockedTime;
Copy link
Member

Choose a reason for hiding this comment

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

For existing Date/time values the client uses a Long. Also DateTime is an Apiary object that I'd rather not surface out in the client). I just realized that it was surfaced in OLM 😞.

Please change it in this case for Bucket Policy Only. We will need to revisit OLM at a later time.


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

@Override
public int hashCode() {
return Objects.hash(enabled, lockedTime);
}

public Boolean getEnabled() {
return enabled;
}

public DateTime getLockedTime() {
return lockedTime;
}

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

Bucket.IamConfiguration.BucketPolicyOnly toPb() {
Bucket.IamConfiguration.BucketPolicyOnly bucketPolicyOnly = new Bucket.IamConfiguration.BucketPolicyOnly();

bucketPolicyOnly.setEnabled(enabled);
bucketPolicyOnly.setLockedTime(lockedTime);

return bucketPolicyOnly;
}

static BucketPolicyOnly fromPb(Bucket.IamConfiguration.BucketPolicyOnly bucketPolicyOnly) {
return newBuilder()
.setEnabled(bucketPolicyOnly.getEnabled())
.setLockedTime(bucketPolicyOnly.getLockedTime())
.build();
}

private BucketPolicyOnly(Builder builder) {
this.enabled = builder.enabled;
this.lockedTime = builder.lockedTime;
}

public Builder toBuilder() {
Builder builder = new Builder();
builder.setLockedTime(lockedTime);
builder.setEnabled(enabled);
return builder;
}

/** Builder for {@code BucketPolicyOnly} */
public static class Builder {
private Boolean enabled;
private DateTime lockedTime;

private Builder() {}

/** 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. All legacy ACL policies must be removed from
* the bucket's info before this can be enabled. When this is first enabled, {@code lockedTime} will be set by
* the API automatically. This field can 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.
Copy link
Member

Choose a reason for hiding this comment

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

The following is no longer true:

All legacy ACL policies must be removed from the bucket's info before this can be enabled.

Are you experiencing something different? If you're referring to the following:

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;
    }
}

setAcl(null) and setDefaultObjectAcl(null) is required if these fields are set in the metadata resource when performing a patch. If they weren't set then this would succeed. It's more of a client case.

*/
public Builder setEnabled(Boolean enabled) {
this.enabled = enabled;
return this;
}

/**
* Sets the deadline for switching {@code enabled} back to false. After this time passes, 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(DateTime lockedTime) {
this.lockedTime = lockedTime;
return this;
}

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

/**
* Lifecycle rule for a bucket. Allows supported Actions, such as deleting and changing storage
Expand Down Expand Up @@ -786,6 +966,14 @@ 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 +1004,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 +1035,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 +1188,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 +1226,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 +1465,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 +1608,9 @@ public Rule apply(LifecycleRule lifecycleRule) {
bucketPb.setRetentionPolicy(retentionPolicy);
}
}
if (iamConfiguration != null) {
bucketPb.setIamConfiguration(iamConfiguration.toPb());
}

return bucketPb;
}
Expand Down Expand Up @@ -1526,6 +1732,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
Loading