Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

Better way to get S3 bucket owner #3

Merged
merged 1 commit into from
Jun 22, 2017
Merged

Better way to get S3 bucket owner #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 0 additions & 8 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,6 @@ public final class RemoteOptions extends OptionsBase {
)
public String s3CacheBucket;

@Option(
name = "s3_full_control_userid",
defaultValue = "null",
category = "remote",
help = "An AWS canonical user id for a user who should have full control over uploaded objects"
)
public String s3FullControlUserId;

@Option(name = "remote_fallback_strategy",
allowMultiple = true,
converter = AssignmentConverter.class,
Expand Down
32 changes: 18 additions & 14 deletions src/main/java/com/google/devtools/build/lib/remote/S3ActionCache2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,14 @@

package com.google.devtools.build.lib.remote;

import com.amazonaws.AbortedException;
import com.amazonaws.AmazonClientException;
import com.amazonaws.auth.DefaultAWSCredentialsProviderChain;
import com.amazonaws.services.s3.AmazonS3;
import com.amazonaws.services.s3.AmazonS3Client;
import com.amazonaws.services.s3.model.*;
import com.google.common.hash.HashCode;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputFileCache;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.remote.RemoteProtocol.ContentDigest;
import com.google.devtools.build.lib.vfs.Path;
import com.google.protobuf.ByteString;

import java.io.ByteArrayInputStream;
import java.io.IOException;
Expand All @@ -36,16 +31,15 @@
import java.io.FileOutputStream;
import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
import java.util.Collection;

/**
* xcxc
*/
@ThreadSafe
public final class S3ActionCache2 {
private final String bucketName;
private final String s3userId;
private final boolean debug;
private String bucketOwner;

private static volatile int numConsecutiveErrors;
private static volatile long disableUntilTimeMillis;
Expand All @@ -58,7 +52,6 @@ public final class S3ActionCache2 {
*/
public S3ActionCache2(RemoteOptions options) {
this.bucketName = options.s3CacheBucket;
this.s3userId = options.s3FullControlUserId;
this.debug = options.remoteCacheDebug;
}

Expand Down Expand Up @@ -200,19 +193,30 @@ public void put(String key, byte[] blob) {
putObject(key, new PutObjectRequest(bucketName, key, new ByteArrayInputStream(blob), new ObjectMetadata()));
}

private synchronized String getBucketOwner() {
Copy link

Choose a reason for hiding this comment

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

Does is make sense to cache this for the lifetime of the worker? Otherwise you'll be doing two round-trips to S3 for each put, right?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I'll fix that

if (bucketOwner == null) {
try {
bucketOwner = client.getBucketAcl(bucketName).getOwner().getId();
recordCacheSuccessfulOperation();
} catch (AmazonClientException e) {
recordCacheFailedOperation(e);
}
}
return bucketOwner;
}

private void putObject(String key, PutObjectRequest object) {
if (!isCacheEnabled()) {
return;
}

long t0 = System.currentTimeMillis();
try {
if (s3userId != null) {
Grantee grantee = new CanonicalGrantee(s3userId);
AccessControlList acl = new AccessControlList();
acl.grantPermission(grantee, Permission.FullControl);
object = object.withAccessControlList(acl);
}
// Make sure the bucket owner has full control of the uploaded object
Grantee grantee = new CanonicalGrantee(getBucketOwner());
AccessControlList acl = new AccessControlList();
acl.grantPermission(grantee, Permission.FullControl);
object = object.withAccessControlList(acl);

client.putObject(object);
if (debug) {
Expand Down