storage: hmac service account snippets #6088

jkwlui · 2019-08-15T23:14:31Z

pending service account email.
lint

elharo · 2019-08-16T01:22:08Z

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

+
+  /** Example of creating an HMAC key for a service account * */
+  public HmacKey createHmacKey(String serviceAccountEmail) throws StorageException {
+    // [START storage_create_hmac_key]


I'm guessing this is part of some doc set somewhere? (A filed issue explaining all this would be helpful.) Is there documentation for how the example inclusion works?

elharo · 2019-08-16T01:22:31Z

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

+    Storage storage = StorageOptions.getDefaultInstance().getService();
+
+    // The service account email for which the new HMAC key will be created.
+    // String serviceAccountEmail = "service-account@iam.gserviceaccount.com";


Why is this commented out?

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

elharo · 2019-08-16T01:23:44Z

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

+    Storage storage = StorageOptions.getDefaultInstance().getService();
+
+    // The access ID of the HMAC key, e.g. "GOOG0234230X00"
+    // String accessId = "GOOG0234230X00";


Why is this commented out?

The code section between the START and END region tags are extracted and put on the doc site. When copy and pasting the sample into their own project, the developer is expected to uncomment the variable declaration and replace the value with their own.

...mples/src/test/java/com/google/cloud/examples/storage/snippets/ITStorageHmacKeySnippets.java

codecov · 2019-08-16T22:00:39Z

Codecov Report

Merging #6088 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #6088      +/-   ##
============================================
+ Coverage     47.38%    47.4%   +0.01%     
+ Complexity    27182    27123      -59     
============================================
  Files          2523     2501      -22     
  Lines        274581   273249    -1332     
  Branches      31380    31185     -195     
============================================
- Hits         130123   129544     -579     
+ Misses       134850   134174     -676     
+ Partials       9608     9531      -77

Impacted Files	Coverage Δ	Complexity Δ
.../bigtable/admin/v2/BigtableTableAdminSettings.java	`63.63% <0%> (-8.11%)`	`8% <0%> (+1%)`
...gtable/admin/v2/BigtableInstanceAdminSettings.java	`96.55% <0%> (-3.45%)`	`6% <0%> (ø)`
...ud/bigtable/data/v2/stub/EnhancedBigtableStub.java	`95.45% <0%> (-3.02%)`	`21% <0%> (-1%)`
...le/cloud/bigtable/data/v2/models/BulkMutation.java	`97.29% <0%> (-2.71%)`	`9% <0%> (+2%)`
...e/cloud/bigtable/data/v2/BigtableDataSettings.java	`32.14% <0%> (-2.64%)`	`4% <0%> (+1%)`
.../google/cloud/spanner/jdbc/CredentialsService.java	`91.11% <0%> (-2.23%)`	`11% <0%> (-1%)`
...able/gaxx/reframing/ReframingResponseObserver.java	`88.99% <0%> (-1.84%)`	`29% <0%> (-1%)`
...oogle/cloud/spanner/jdbc/SingleUseTransaction.java	`86.5% <0%> (-1.29%)`	`36% <0%> (-4%)`
...table/admin/v2/BaseBigtableTableAdminSettings.java	`10.93% <0%> (-1.14%)`	`2% <0%> (ø)`
.../com/google/cloud/spanner/jdbc/ConnectionImpl.java	`82.58% <0%> (-1.12%)`	`155% <0%> (ø)`
... and 159 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1daf76a...be87a93. Read the comment docs.

frankyn

Overall LGTM.

Please add service account for samples IT to kokoro builds in case they are re-enabled. Marking that as a requested change.

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

This reverts commit e756253.

jkwlui · 2019-09-09T21:43:20Z

@frankyn I reverted the commit to add the env var for integration tests since it's not whitelisted in the Kokoro configs in the backend. This will not block this, we just need an action item for when we add sample test running on Kokoro.

Please take another look :)

frankyn

Small nits. Overall LGTM.

frankyn · 2019-09-09T21:49:33Z

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

+  }
+
+  /** Example of retrieving the metadata of an existing HMAC key. * */
+  public HmacKeyMetadata getHmacKey(String accessId) throws StorageException {


pass in projectId in this example and the others as well. It's a small level of detail but it keeps consistency.

frankyn · 2019-09-09T21:50:20Z

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

+    return newMetadata;
+  }
+
+  public Page<HmacKeyMetadata> listHmacKeys(String serviceAccountEmail) throws StorageException {


use projectId instead of serviceAccountEmail.

jkwlui · 2019-09-09T22:21:15Z

I couldn't find an option to provide projectId for deleteHmacKey or updateHmacKey. Am I missing something here? @JesseLovelace

frankyn

Thanks @jkwlui two nits.

frankyn · 2019-09-10T21:30:00Z

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

+    //
+    // The ID of the project to which the service account belongs.
+    // String projectId = "project-id";
+    HmacKeyMetadata metadata = storage.getHmacKey(accessId);


set projectId here.

frankyn · 2019-09-10T21:30:37Z

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java

+    Storage storage = StorageOptions.getDefaultInstance().getService();
+
+    // The service account to list HMAC keys for.
+    // String serviceAccountEmail = "service-account@iam.gserviceaccount.com";


This deviates a bit from other samples.

in what way?

Node.js: https://github.com/googleapis/nodejs-storage/blob/master/samples/hmacKeysList.js#L24
Ruby: https://github.com/GoogleCloudPlatform/ruby-docs-samples/blob/master/storage/hmac.rb#L15
Python: https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/storage/cloud-client/hmac.py#L22

Service account parameter isn't set.

Gotcha! Nice catch.

jkwlui · 2019-09-13T20:40:44Z

I don't think the test failure is due to this PR. Can someone with admin privileges add kokoro:run?

@JesseLovelace

jkwlui requested a review from a team as a code owner August 15, 2019 23:14

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 15, 2019

jkwlui changed the base branch from hmacsa to master August 15, 2019 23:17

storage: hmac service account snippets

7d67a39

jkwlui force-pushed the hmacsa-samples branch from 3fcde29 to 7d67a39 Compare August 15, 2019 23:27

elharo suggested changes Aug 16, 2019

View reviewed changes

fix expected<>actual; for (:) with bracket

43531d2

jkwlui added 5 commits August 16, 2019 15:01

copyright header

4261331

inline

1d9d2d1

fix cases

4bd83ae

Base64

237481b

reformat printing HMAC key metadata

2fab5ae

frankyn suggested changes Aug 23, 2019

View reviewed changes

JesseLovelace reviewed Aug 29, 2019

View reviewed changes

...cloud-examples/src/main/java/com/google/cloud/examples/storage/snippets/StorageSnippets.java Outdated Show resolved Hide resolved

jkwlui added 4 commits September 3, 2019 14:58

add note about showDeletedKeys

b46d2ea

format code

c925924

add HMAC_KEY_SAMPLES_SERVICE_ACCOUNT to Kokoro

e756253

Revert "add HMAC_KEY_SAMPLES_SERVICE_ACCOUNT to Kokoro"

73c3f84

This reverts commit e756253.

frankyn suggested changes Sep 9, 2019

View reviewed changes

elharo approved these changes Sep 9, 2019

View reviewed changes

jkwlui added 3 commits September 10, 2019 11:44

add projectId as argument to snippets

c6acdc4

reformat

283ad00

reformat

1bdac78

frankyn suggested changes Sep 10, 2019

View reviewed changes

listHmacKeys snippet: do not filter by serviceAccountEmail

be87a93

frankyn approved these changes Sep 10, 2019

View reviewed changes

rahulKQL added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 13, 2019

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 13, 2019

frankyn merged commit 06d2065 into googleapis:master Sep 16, 2019

chingor13 mentioned this pull request Sep 18, 2019

Release google-cloud-java v0.110.0 #6294

Merged

storage: hmac service account snippets #6088

storage: hmac service account snippets #6088

Uh oh!

Conversation

jkwlui commented Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Aug 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkwlui commented Sep 9, 2019

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkwlui commented Sep 9, 2019

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkwlui Sep 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkwlui commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jkwlui commented Aug 15, 2019 •

edited

Loading

codecov bot commented Aug 16, 2019 •

edited

Loading

jkwlui Sep 10, 2019 •

edited

Loading

jkwlui commented Sep 13, 2019 •

edited

Loading