feat(bigquery): support IAM conditions in datasets in Java client. #3602
Conversation
product-auto-label
bot
added
size: m
product-auto-label
bot
added
size: l
whuffman36
requested review from
Neenu1995,
shollyman and
PhongChuong
and removed request for
Neenu1995
| bqExpr.getExpression(), bqExpr.getTitle(), bqExpr.getDescription(), bqExpr.getLocation()); | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Include toString() as well as the normal methods like equals and hashcode?
There was a problem hiding this comment.
Good point, updated to have toString() method.
| } | ||
| } | ||
| if (accessPolicyVersion != null) { | ||
| return bigquery |
There was a problem hiding this comment.
nit: I worry here is if there's a divergence between this chain and the default execute(). Is it worth doing a single chain of builders with a conditional? Same with this pattern in the other RPCs (create, patch, etc).
There was a problem hiding this comment.
Yes that approach seems cleaner. I updated the conditional to toggle only the access policy version instead of creating an entirely new RPC.
| @Test | ||
| public void testGetDatasetWithAccessPolicyVersion() { | ||
| String accessPolicyDataset = RemoteBigQueryHelper.generateDatasetName(); | ||
| User user = new User("liamhuffman@google.com"); |
There was a problem hiding this comment.
Let's use a managed identity for this kind of thing rather than your own identity, particular for an IT test. Same for the other tests that use your identity directly.
There was a problem hiding this comment.
I've updated the user to use a default bigquery account for now. I found the account in my test project and I'm not sure if every test account follows the same naming pattern. We discussed a bit offline but this works as a workaround for the time being until we figure out how to integrate a managed account.
There was a problem hiding this comment.
It appears that not all projects have the test account, as the integration tests failed due to the account not existing in the project. Looking into https://g3doc.corp.google.com/company/teams/eip-cloud/overground/user-guide/policies/org/iam.allowedPolicyMemberDomains.md?cl=head
There was a problem hiding this comment.
Looks like using the default credentials works in this test project, it was just failing in my personal test project. The tests are using the default managed identity now.
Use service account in integration test instead of hardcoded personal account. Change Database API calls to only have one branch, toggling only the access policy version in a conditional.
google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITBigQueryTest.java
Show resolved
Hide resolved
google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITBigQueryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITBigQueryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITBigQueryTest.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITBigQueryTest.java
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/spi/v2/HttpBigQueryRpc.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/spi/v2/HttpBigQueryRpc.java
Outdated
Show resolved
Hide resolved
google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/spi/v2/HttpBigQueryRpc.java
Outdated
Show resolved
Hide resolved
This PR enables the use of IAM conditions when accessing a dataset through the create, update, and get API endpoints. The following changes are necessary:
Exprtype in theAclclass for expressing the access conditionsDatasetOptionfield to set the access policy versionBuganizer link: b/374156746