You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "talatuyarer (via GitHub)" <gi...@apache.org> on 2025/12/09 23:18:31 UTC
[PR] Add GCP service account impersonation for credentials. [polaris]
talatuyarer opened a new pull request, #3246:
URL: https://github.com/apache/polaris/pull/3246
This will close this bug: https://github.com/apache/polaris/issues/550
cc @flyrain
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add GCP service account impersonation for credentials. [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #3246:
URL: https://github.com/apache/polaris/pull/3246#discussion_r2608311624
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java:
##########
@@ -123,6 +134,46 @@ public StorageAccessConfig getSubscopedCreds(
return accessConfig.build();
}
+ /**
+ * Returns the credential to be used as the source for downscoping. If a specific service account
+ * is configured, it impersonates that account first.
+ */
+ private GoogleCredentials getBaseCredentials() {
+ if (config().getGcpServiceAccount() != null) {
Review Comment:
Looks like this is in the CLI doc for a while, https://polaris.apache.org/in-dev/unreleased/command-line-interface/#catalogs
```
--service-account (Only for GCS) The service account to use when connecting to GCS
```
Agreed with @dimas-b, not particularly a big concern, but +1 on adding this to the CHANGELOG.md.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add GCP service account impersonation for credentials. [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #3246:
URL: https://github.com/apache/polaris/pull/3246#issuecomment-3643165887
Thanks a lot for adding this, @talatuyarer! Thanks @dimas-b @adnanhemani for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add GCP service account impersonation for credentials. [polaris]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #3246:
URL: https://github.com/apache/polaris/pull/3246#discussion_r2604816225
##########
polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java:
##########
@@ -309,6 +315,63 @@ public void testRefreshCredentialsEndpointIsReturned() throws IOException {
.isEqualTo(REFRESH_ENDPOINT);
}
+ @Test
+ public void testImpersonation() throws IOException {
+ String serviceAccount = "test-sa@project.iam.gserviceaccount.com";
+ GcpStorageConfigurationInfo config =
+ GcpStorageConfigurationInfo.builder()
+ .addAllAllowedLocations(List.of("gs://bucket/path"))
+ .gcpServiceAccount(serviceAccount)
+ .build();
+
+ IamCredentialsClient mockIamClient = Mockito.mock(IamCredentialsClient.class);
+ GenerateAccessTokenResponse mockResponse =
+ GenerateAccessTokenResponse.newBuilder()
+ .setAccessToken("impersonated-token")
+ .setExpireTime(
+ Timestamp.newBuilder().setSeconds(System.currentTimeMillis() / 1000 + 3600).build())
+ .build();
+ Mockito.when(mockIamClient.generateAccessToken(Mockito.any(GenerateAccessTokenRequest.class)))
+ .thenReturn(mockResponse);
+
+ GoogleCredentials mockCreds = Mockito.mock(GoogleCredentials.class);
+ Mockito.when(mockCreds.createScoped(Mockito.any(String.class))).thenReturn(mockCreds);
+
+ GcpCredentialsStorageIntegration integration =
+ new GcpCredentialsStorageIntegration(
+ config,
+ mockCreds,
+ ServiceOptions.getFromServiceLoader(
+ HttpTransportFactory.class, NetHttpTransport::new)) {
+ @Override
+ protected IamCredentialsClient createIamCredentialsClient(GoogleCredentials credentials) {
+ return mockIamClient;
+ }
+
+ @Override
+ protected AccessToken refreshAccessToken(DownscopedCredentials credentials) {
+ return new AccessToken("downscoped-token", new Date());
+ }
+ };
+
+ integration.getSubscopedCreds(
+ EMPTY_REALM_CONFIG,
+ true,
+ Set.of("gs://bucket/path"),
+ Set.of("gs://bucket/path"),
+ Optional.empty());
+
+ Mockito.verify(mockIamClient)
+ .generateAccessToken(
+ Mockito.argThat(
+ request ->
+ request.getName().equals("projects/-/serviceAccounts/" + serviceAccount)
Review Comment:
nit: Would recommend moving these variables out to the main class as public variables and then referencing them here from there so that we don't have magic variables and it will be cleaner to maintain if there are any changes in the future!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add GCP service account impersonation for credentials. [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain merged PR #3246:
URL: https://github.com/apache/polaris/pull/3246
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add GCP service account impersonation for credentials. [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3246:
URL: https://github.com/apache/polaris/pull/3246#discussion_r2604870601
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java:
##########
@@ -123,6 +134,46 @@ public StorageAccessConfig getSubscopedCreds(
return accessConfig.build();
}
+ /**
+ * Returns the credential to be used as the source for downscoping. If a specific service account
+ * is configured, it impersonates that account first.
+ */
+ private GoogleCredentials getBaseCredentials() {
+ if (config().getGcpServiceAccount() != null) {
Review Comment:
Interesting that this config value was defined before, but not used 🤔
Does anyone have insight into why it was this way?
Technically, now that `GcpServiceAccount` config values start affecting Polaris behaviour, this could be a breaking change in existing deployments that may have (possibly accidental and/or incorrect) values in that config property.
I do believe that such a situation is unlikely, so it should be fine to proceed with this PR. However, it probably deserves mentioning as a `Change` in CHANGELOG.md
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org