feat: add IAM Conditions support by frankyn · Pull Request #120 · googleapis/java-storage · GitHub

Add IAM Conditions Support.

Depends on:
googleapis/java-core#110

Pending Review List:

@jkwlui (IAM Conditions Review)
@JesseLovelace (Java + Storage Review)

Surface looks good. LGTM!
I'll let @chingor13 and others review Java aspects.

Mostly nits, but snapshot dependencies are a hard no.

no reason to put null first; it's a leftover convention from C that doesn't make sense in Java

delete it if it isn't relevant. or fix it if it is.

don't comment out code

Conditionals around asserts are a code smell. This should be two tests, one with and one without requesterPays.

Or two helper methods

i++ is more common

No snapshot dependencies.

Doing it temporary until dependency PR is merged/released.

Or two helper methods

hard veto on this; we must not depend on snapshots. If this means we have to wait for a release of google core so be it.

I'm doing this only for development*

Codecov Report

Merging #120 into master will increase coverage by 0.06%.
The diff coverage is 44.73%.

@@             Coverage Diff              @@
##             master     #120      +/-   ##
============================================
+ Coverage      63.4%   63.46%   +0.06%     
  Complexity      537      537              
============================================
  Files            30       30              
  Lines          4752     4752              
  Branches        427      427              
============================================
+ Hits           3013     3016       +3     
+ Misses         1579     1576       -3     
  Partials        160      160

Impacted Files	Coverage Δ	Complexity Δ
...rc/main/java/com/google/cloud/storage/Storage.java	80.57% <0%> (ø)	0 <0> (ø)	⬇️
...om/google/cloud/storage/spi/v1/HttpStorageRpc.java	1.63% <0%> (ø)	1 <0> (ø)	⬇️
...va/com/google/cloud/storage/spi/v1/StorageRpc.java	64.86% <100%> (ø)	0 <0> (ø)	⬇️
...in/java/com/google/cloud/storage/PolicyHelper.java	64.86% <55.55%> (ø)	5 <2> (ø)	⬇️
...gle/cloud/storage/testing/RemoteStorageHelper.java	64.46% <0%> (+2.47%)	9% <0%> (ø)	⬇️

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 2627a93...900a991. Read the comment docs.

@elharo could you help cut a release for libraries-bom, @chingor13 released google-cloud-bom which updated java-core dependency to latest release which has IAM Conditions support.

It's blocking the Linkage Monitor.

Add IAM Conditions support

187e98c

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 6, 2020

frankyn requested review from JesseLovelace, chingor13 and jkwlui February 6, 2020 18:38

Frank Natividad added 3 commits February 6, 2020 10:41

format

ef9a53d

wip

f7e1609

set version in mock policies

1160ad5

This was referenced Feb 7, 2020

Iam condition samples googleapis/google-cloud-java#7127

Closed

Iam condition samples googleapis/google-cloud-java#7128

Closed

jkwlui approved these changes Feb 11, 2020

View reviewed changes

elharo suggested changes Feb 11, 2020

View reviewed changes

wip

938c9fe

frankyn requested review from elharo and removed request for chingor13 February 26, 2020 23:13

Frank Natividad added 5 commits February 26, 2020 15:13

address comments

d7a4077

Merge branch 'master' into iam-conditions-support

f2013df

fix pom.

8ad5043

revert http-client-bom

6284c40

address feedback

7117481

elharo suggested changes Feb 27, 2020

View reviewed changes

address integration tests feedback

8606ab7

frankyn requested a review from elharo February 27, 2020 19:40

JesseLovelace approved these changes Feb 27, 2020

View reviewed changes

Frank Natividad added 3 commits February 27, 2020 13:53

Merge branch 'master' into iam-conditions-support

cd2ead7

update core

3941f5c

lint

1dd8343

Frank Natividad added 2 commits February 27, 2020 14:36

Merge branch 'master' into iam-conditions-support

1bd1916

clean up ubla

cff4242

Frank Natividad added 2 commits February 27, 2020 20:09

format

95a17db

Merge branch 'master' into iam-conditions-support

900a991

frankyn added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 28, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 28, 2020

elharo approved these changes Feb 28, 2020

View reviewed changes

frankyn merged commit 8256f6d into master Feb 28, 2020

frankyn deleted the iam-conditions-support branch February 28, 2020 21:12

+                    Bindings apiBinding = new Bindings();
+                    apiBinding.setRole(binding.getRole());
+                    apiBinding.setMembers(new ArrayList<>(binding.getMembers()));
+                    if (null != binding.getCondition()) {

                 public void testBucketPolicy() {
                   testBucketPolicyRequesterPays(true);
                   testBucketPolicyRequesterPays(false);
+                  // testBucketPolicyV3RequesterPays(true);

                   <github.global.server>github</github.global.server>
                   <site.installationModule>google-cloud-storage-parent</site.installationModule>
-                  <google.core.version>1.91.3</google.core.version>
+                  <google.core.version>1.92.3-SNAPSHOT</google.core.version>

                   <github.global.server>github</github.global.server>
                   <site.installationModule>google-cloud-storage-parent</site.installationModule>
-                  <google.core.version>1.92.5</google.core.version>
+                  <google.core.version>1.92.6-SNAPSHOT</google.core.version>

-                  assertEquals(libPolicy, actualLibPolicy);
-                  assertTrue(new ApiPolicyMatcher(apiPolicy).matches(actualApiPolicy));
+              //    Policy actualLibPolicy = PolicyHelper.convertFromApiPolicy(apiPolicy);

                 }
+                private void testBucketPolicyV3RequesterPays(boolean requesterPays) {
+                  if (requesterPays) {

+                  // Remove a member
+                  List<com.google.cloud.Binding> updatedBindings = new ArrayList(updatedPolicy.getBindingsList());
+                  for (int i = 0; i < updatedBindings.size(); ++i) {

feat: add IAM Conditions support by frankyn · Pull Request #120 · googleapis/java-storage · GitHub

Uh oh!

feat: add IAM Conditions support - #120

feat: add IAM Conditions support#120frankyn merged 18 commits intomastergoogleapis/java-storage:masterfrom iam-conditions-supportgoogleapis/java-storage:iam-conditions-supportCopy head branch name to clipboard

Conversation

frankyn commented Feb 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkwlui left a comment

Choose a reason for hiding this comment

Uh oh!

elharo 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

frankyn commented Feb 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

feat: add IAM Conditions support#120
frankyn merged 18 commits into
masterfrom
iam-conditions-support

frankyn commented Feb 6, 2020 •
edited
Loading

codecov Bot commented Feb 27, 2020 •
edited
Loading