Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
11ffccc
Storage: HMAC service account support
JesseLovelace Jun 3, 2019
fe560aa
Storage: HMAC service account support
JesseLovelace Jun 3, 2019
2375c85
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 17, 2019
42569aa
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 17, 2019
a09334c
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 21, 2019
89d165b
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 21, 2019
7be7c2b
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 21, 2019
4bb6b44
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 21, 2019
91287cd
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 21, 2019
82259e4
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 21, 2019
46f4e29
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 24, 2019
df99b6d
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jun 24, 2019
ab32675
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jul 11, 2019
9e185bb
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jul 11, 2019
6d6fd4b
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jul 12, 2019
46fc073
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Jul 12, 2019
cefd5ea
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 6, 2019
6dc9627
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 6, 2019
6998a83
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 6, 2019
0a8abee
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 6, 2019
16a3c60
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 13, 2019
af8bfd6
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 13, 2019
584015f
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 13, 2019
8760589
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 13, 2019
55f1ef4
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 13, 2019
5cfdea5
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 13, 2019
61187df
Merge branch 'hmacsa' of github.com:googleapis/google-cloud-java into…
JesseLovelace Aug 13, 2019
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
5 changes: 5 additions & 0 deletions .kokoro/continuous/storage-it.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,8 @@ env_vars: {
key: "GOOGLE_APPLICATION_CREDENTIALS"
value: "keystore/73713_java_it_service_account"
}

env_vars: {
key: "IT_SERVICE_ACCOUNT_EMAIL"
value: "[email protected]"
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import com.google.api.services.storage.model.Bucket;
import com.google.api.services.storage.model.BucketAccessControl;
import com.google.api.services.storage.model.HmacKey;
import com.google.api.services.storage.model.HmacKeyMetadata;
import com.google.api.services.storage.model.Notification;
import com.google.api.services.storage.model.ObjectAccessControl;
import com.google.api.services.storage.model.Policy;
Expand Down Expand Up @@ -67,6 +69,7 @@
* <li>checksums, etags
* <li>IAM operations
* <li>BucketLock operations
* <li>HMAC key operations
* </ul>
* </ul>
*/
Expand Down Expand Up @@ -420,6 +423,32 @@ public List<BucketAccessControl> listAcls(String bucket, Map<Option, ?> options)
throw new UnsupportedOperationException();
}

@Override
public HmacKey createHmacKey(String serviceAccountEmail) {
throw new UnsupportedOperationException();
}

@Override
public Tuple<String, Iterable<HmacKeyMetadata>> listHmacKeys(
String serviceAccountEmail, String pageToken, Long maxResults, boolean showDeletedKeys) {
throw new UnsupportedOperationException();
}

@Override
public HmacKeyMetadata updateHmacKey(HmacKeyMetadata hmacKeyMetadata) {
throw new UnsupportedOperationException();
}

@Override
public HmacKeyMetadata getHmacKey(String accessId) {
throw new UnsupportedOperationException();
}

@Override
public void deleteHmacKey(String accessId) {
throw new UnsupportedOperationException();
}

@Override
public ObjectAccessControl getDefaultAcl(String bucket, String entity) {
throw new UnsupportedOperationException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.io.Serializable;
import java.util.Objects;

/** HMAC key for a service account. */
public class HmacKey implements Serializable {

private static final long serialVersionUID = -1809610424373783062L;
Expand All @@ -20,6 +21,7 @@ public static Builder newBuilder(String secretKey) {
return new Builder(secretKey);
}

/** Builder for {@code HmacKey} objects. * */
public static class Builder {
private String secretKey;
Copy link
Member

Choose a reason for hiding this comment

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

Not super familiar with documenting Java code, but can we add a note saying the secret is only available on creation?

Copy link
Contributor Author

@JesseLovelace JesseLovelace Jun 18, 2019

Choose a reason for hiding this comment

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

Yeah--I'm planning to add the documentation once the code is otherwise approved

private HmacKeyMetadata metadata;
Expand All @@ -38,22 +40,25 @@ public Builder setMetadata(HmacKeyMetadata metadata) {
return this;
}

/** Creates an {@code HmacKey} object from this builder. * */
public HmacKey build() {
return new HmacKey(this);
}
}

/** Returns the secret key associated with this HMAC key. * */
public String getSecretKey() {
return secretKey;
}

/** Returns the metadata associated with this HMAC key. * */
public HmacKeyMetadata getMetadata() {
return metadata;
}

@Override
public int hashCode() {
return Objects.hash(secretKey, metadata);
return Objects.hash(secretKey, metadata);
}

@Override
Expand All @@ -65,8 +70,7 @@ public boolean equals(Object obj) {
return false;
}
final HmacKeyMetadata other = (HmacKeyMetadata) obj;
return Objects.equals(this.secretKey, secretKey)
&& Objects.equals(this.metadata, metadata);
return Objects.equals(this.secretKey, secretKey) && Objects.equals(this.metadata, metadata);
}

com.google.api.services.storage.model.HmacKey toPb() {
Expand Down Expand Up @@ -99,6 +103,10 @@ public enum HmacKeyState {
}
}

/**
* The metadata for a service account HMAC key. This class holds all data associated with an HMAC
* key other than the secret key.
*/
public static class HmacKeyMetadata implements Serializable {

private static final long serialVersionUID = 4571684785352640737L;
Expand Down Expand Up @@ -182,38 +190,53 @@ static HmacKeyMetadata fromPb(com.google.api.services.storage.model.HmacKeyMetad
.build();
}

/**
* Returns the access id for this HMAC key. This is the id needed to get or delete the key. *
*/
public String getAccessId() {
return accessId;
}

/**
* Returns HTTP 1.1 Entity tag for this HMAC key.
*
* @see <a href="http://tools.ietf.org/html/rfc2616#section-3.11">Entity Tags</a>
*/
public String getEtag() {
return etag;
}

/** Returns the resource name of this HMAC key. * */
public String getId() {
return id;
}

/** Returns the project id associated with this HMAC key. * */
public String getProjectId() {
return projectId;
}

/** Returns the service account associated with this HMAC key. * */
public ServiceAccount getServiceAccount() {
return serviceAccount;
}

/** Returns the current state of this HMAC key. * */
public HmacKeyState getState() {
return state;
}

/** Returns the creation time of this HMAC key. * */
public Long getCreateTime() {
return createTime;
}

/** Returns the last updated time of this HMAC key. * */
public Long getUpdateTime() {
return updateTime;
}

/** Builder for {@code HmacKeyMetadata} objects. * */
public static class Builder {
private String accessId;
private String etag;
Expand Down Expand Up @@ -274,6 +297,7 @@ public Builder setProjectId(String projectId) {
return this;
}

/** Creates an {@code HmacKeyMetadata} object from this builder. * */
public HmacKeyMetadata build() {
return new HmacKeyMetadata(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2709,17 +2709,117 @@ Blob create(
*/
List<Acl> listAcls(BlobId blob);

/**
* Creates a new HMAC Key for the provided service account, including the secret key. Note that
* the secret key is only returned upon creation via this method.
*
* <p>Example of creating a new HMAC Key.
*
* <pre>{@code
* ServiceAccount serviceAccount = ServiceAccount.of("[email protected]");
*
* HmacKey hmacKey = storage.createHmacKey(serviceAccount);
*
* String secretKey = hmacKey.getSecretKey();
* HmacKey.HmacKeyMetadata metadata = hmacKey.getMetadata();
* }</pre>
*
* @throws StorageException upon failure
*/
HmacKey createHmacKey(ServiceAccount serviceAccount);
Copy link
Contributor

Choose a reason for hiding this comment

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

Include options for userProject.


/**
* Lists HMAC keys for a given service account. Note this returns {@code HmacKeyMetadata} objects,
* which do not contain secret keys.
*
* <p>Example of listing HMAC keys, specifying max results.
*
* <pre>{@code
* ServiceAccount serviceAccount = ServiceAccount.of("[email protected]");
Copy link
Contributor

Choose a reason for hiding this comment

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

serviceAccount isn't used.

*
* Page<HmacKey.HmacKeyMetadata> metadataPage = storage.listHmacKeys(serviceAccount, null, 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add the boolean value of true for Deleted keys?

* for (HmacKey.HmacKeyMetadata hmacKeyMetadata : metadataPage.getValues()) {
* //do something with the metadata
* }
* }</pre>
*
* @param serviceAccount the service account whose HMAC keys to list
* @param pageToken the page from which to start results
* @param maxResults the maximum amount of results that can be returned by this request
* @param showDeletedKeys whether to show keys with the DELETED state in the result
* @throws StorageException upon failure
*/
Page<HmacKeyMetadata> listHmacKeys(
ServiceAccount serviceAccount, String pageToken, Long maxResults);
ServiceAccount serviceAccount, String pageToken, Long maxResults, boolean showDeletedKeys);
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, why the overload for showDeletedKeys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reasoning here is that the most common use case is going to be listHmacKeys(serviceAccount), so offering that along with one other signature to be more precise would meet most needs

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha that makes sense. What about defaulting to not showing deleted keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is already the default I believe

Copy link
Member

Choose a reason for hiding this comment

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

API defaults to false if not given.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @jkwlui and @JesseLovelace!

Copy link
Contributor

@frankyn frankyn Jul 10, 2019

Choose a reason for hiding this comment

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

Based on other RPCs the Storage library has, the following parameters: serviceAccountEmail, pageToken, showDeletedKeys, and maxResults should be listed under an Options optional parameter because they are optional. The only required value is project_id which is provided by client credentials.

Other HmacKey RPCs should have a similar Options parameter for the value userProject. At the moment it's ignored by the service but it's expected to be available. Given it would change the signature of the method I thought it'd be best to do it now rather than wait until later. (I'm open to opinions here).


/**
* Lists HMAC keys for a given service account. Note this returns {@code HmacKeyMetadata} objects,
Copy link
Contributor

Choose a reason for hiding this comment

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

Lists active HMAC keys for a given service account.

* which do not contain secret keys. This is the same as calling {@code
* listHmacKeys(serviceAccount, null, null, false)}.
*
* <p>Example of listing HMAC keys.
*
* <pre>{@code
* ServiceAccount serviceAccount = ServiceAccount.of("[email protected]");
*
* Page<HmacKey.HmacKeyMetadata> metadataPage = storage.listHmacKeys(serviceAccount);
* for (HmacKey.HmacKeyMetadata hmacKeyMetadata : metadataPage.getValues()) {
* //do something with the metadata
* }
* }</pre>
*
* @throws StorageException upon failure
*/
Page<HmacKeyMetadata> listHmacKeys(ServiceAccount serviceAccount);
Copy link
Member

Choose a reason for hiding this comment

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

listHmacKeys should also be able to accept no serviceAccount, which will show HMAC keys from all accounts. Can your implementation accept no serviceAccount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Listing keys from all accounts can be accomplished by calling this with serviceAccount=null. I'll add a note about it in the documentation

Copy link
Member

Choose a reason for hiding this comment

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

Got it. That would really help, thank you!


/**
* Gets an HMAC key given its access id. Note that this returns a {@code HmacKeyMetadata} object,
* which does not contain the secret key.
*
* <p>Example of getting an HMAC key.
*
* <pre>{@code
* String hmacKeyAccessId = "my-access-id";
* HmacKey.HmackeyMetadata hmacKeyMetadata = storage.getHmacKey(hmacKeyAccessId);
* }</pre>
*
* @throws StorageException upon failure
*/
HmacKeyMetadata getHmacKey(String accessId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Include options for userProject.


/**
* Deletes an HMAC key given its access ID. Note that only an {@code INACTIVE} key can be deleted.
* Attempting to delete a key whose {@code HmacKey.HmacKeyState} is anything other than {@code
* INACTIVE} will fail.
*
* <p>Example of updating an HMAC key's state to INACTIVE and then deleting it.
*
* <pre>{@code
* String hmacKeyAccessId = "my-access-id";
* HmacKey.HmacKeyMetadata hmacKeyMetadata = storage.getHmacKey(hmacKeyAccessId);
*
* storage.updateHmacKeyState(hmacKeyMetadata, HmacKey.HmacKeyState.INACTIVE);
* storage.deleteHmacKey(hmacKeyMetadata.getAccessId());
* }</pre>
*
* @throws StorageException upon failure
*/
void deleteHmacKey(String accessId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Include options for userProject.


/**
* Updates the state of an HMAC key and returns the updated metadata.
*
* <p>Example of updating the state of a newly created HMAC key.
*
* <pre>{@code
* ServiceAccount serviceAccount = ServiceAccount.of("[email protected]");
* HmacKey key = storage.createHmacKey(serviceAccount);
Copy link
Contributor

Choose a reason for hiding this comment

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

Perform a Get request instead of a create.

*
* storage.updateHmacKeyState(hmacKey.getMetadata(), HmacKey.HmacKeyState.INACTIVE);
* }</pre>
*
* @throws StorageException upon failure
*/
HmacKeyMetadata updateHmacKeyState(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice helper method! (no action required).

final HmacKeyMetadata hmacKeyMetadata, final HmacKey.HmacKeyState state);
Copy link
Contributor

Choose a reason for hiding this comment

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

Include options for userProject.

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,21 +305,25 @@ private static class HmacKeyMetadataPageFetcher implements NextPageFetcher<HmacK
private final ServiceAccount serviceAccount;
private final String nextPageToken;
private final Long maxResults;
private final boolean showDeletedKeys;

HmacKeyMetadataPageFetcher(
StorageOptions serviceOptions,
ServiceAccount serviceAccount,
String nextPageToken,
Long maxResults) {
Long maxResults,
boolean showDeletedKeys) {
this.serviceOptions = serviceOptions;
this.serviceAccount = serviceAccount;
this.nextPageToken = nextPageToken;
this.maxResults = maxResults;
this.showDeletedKeys = showDeletedKeys;
}

@Override
public Page<HmacKeyMetadata> getNextPage() {
return listHmacKeys(serviceOptions, serviceAccount, nextPageToken, maxResults);
return listHmacKeys(
serviceOptions, serviceAccount, nextPageToken, maxResults, showDeletedKeys);
}
}

Expand Down Expand Up @@ -1209,12 +1213,15 @@ public com.google.api.services.storage.model.HmacKey call() {

@Override
public Page<HmacKeyMetadata> listHmacKeys(
final ServiceAccount serviceAccount, final String pageToken, final Long maxResults) {
return listHmacKeys(getOptions(), serviceAccount, pageToken, maxResults);
final ServiceAccount serviceAccount,
final String pageToken,
final Long maxResults,
final boolean showDeletedKeys) {
return listHmacKeys(getOptions(), serviceAccount, pageToken, maxResults, showDeletedKeys);
}

public Page<HmacKeyMetadata> listHmacKeys(final ServiceAccount serviceAccount) {
return listHmacKeys(getOptions(), serviceAccount, null, null);
return listHmacKeys(getOptions(), serviceAccount, null, null, false);
}

@Override
Expand Down Expand Up @@ -1289,7 +1296,8 @@ private static Page<HmacKeyMetadata> listHmacKeys(
final StorageOptions serviceOptions,
final ServiceAccount serviceAccount,
final String pageToken,
final Long maxResults) {
final Long maxResults,
final boolean showDeletedKeys) {
try {
Tuple<String, Iterable<com.google.api.services.storage.model.HmacKeyMetadata>> result =
runWithRetries(
Expand All @@ -1302,7 +1310,8 @@ private static Page<HmacKeyMetadata> listHmacKeys(
call() {
return serviceOptions
.getStorageRpcV1()
.listHmacKeys(serviceAccount.getEmail(), pageToken, maxResults);
.listHmacKeys(
serviceAccount.getEmail(), pageToken, maxResults, showDeletedKeys);
}
},
serviceOptions.getRetrySettings(),
Expand All @@ -1323,7 +1332,8 @@ public HmacKeyMetadata apply(
}
});
return new PageImpl<>(
new HmacKeyMetadataPageFetcher(serviceOptions, serviceAccount, pageToken, maxResults),
new HmacKeyMetadataPageFetcher(
serviceOptions, serviceAccount, pageToken, maxResults, showDeletedKeys),
cursor,
metadata);
} catch (RetryHelperException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ public HmacKey createHmacKey(String serviceAccountEmail) {

@Override
public Tuple<String, Iterable<HmacKeyMetadata>> listHmacKeys(
String serviceAccountEmail, String pageToken, Long maxResults) {
String serviceAccountEmail, String pageToken, Long maxResults, boolean showDeletedKeys) {
Span span = startSpan(HttpStorageRpcSpans.SPAN_NAME_LIST_HMAC_KEYS);
Scope scope = tracer.withSpan(span);
try {
Expand All @@ -1267,6 +1267,7 @@ public Tuple<String, Iterable<HmacKeyMetadata>> listHmacKeys(
.setServiceAccountEmail(serviceAccountEmail)
.setPageToken(pageToken)
.setMaxResults(maxResults)
.setShowDeletedKeys(showDeletedKeys)
.execute();
return Tuple.<String, Iterable<HmacKeyMetadata>>of(
hmacKeysMetadata.getNextPageToken(), hmacKeysMetadata.getItems());
Expand Down
Loading