You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/04/19 15:51:46 UTC

[GitHub] [beam] kennknowles opened a new pull request, #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

kennknowles opened a new pull request, #17394:
URL: https://github.com/apache/beam/pull/17394

   Adding this parameter, which is standardizing across GCP. It is a chain of accounts where the user invoking the API call has delegation permissions and each account has permissions to the next, until we reach the target principal.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
    - [x] Update `CHANGES.md` with noteworthy changes.
    - [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on a diff in pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles commented on code in PR #17394:
URL: https://github.com/apache/beam/pull/17394#discussion_r889045192


##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java:
##########
@@ -169,6 +169,25 @@ public interface GcpOptions extends GoogleApiDebugOptions, PipelineOptions {
 
   void setGcpCredential(Credentials value);
 
+  /**
+   * All API requests will be made as the given service account or target service account in an
+   * impersonation delegation chain instead of the currently selected account. You can specify
+   * either a single service account as the impersonator, or a comma-separated list of service
+   * accounts to create an impersonation delegation chain.
+   */
+  @Description(
+      "All API requests will be made as the given service account or"
+          + " target service account in an impersonation delegation chain"
+          + " instead of the currently selected account. You can specify"
+          + " either a single service account as the impersonator, or a"
+          + " comma-separated list of service accounts to create an"
+          + " impersonation delegation chain.")
+  @JsonIgnore
+  @Nullable
+  String getImpersonateServiceAccount();
+
+  void setImpersonateServiceAccount(String impersonateServiceAccount);

Review Comment:
   Ah, I did not realize this. That could be a useful follow-up. We do want the format of the parameter to be the same across SDKs and across Beam and gcloud, etc. But it seems that comma-separated strings will be the same across all of them.



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17394:
URL: https://github.com/apache/beam/pull/17394#issuecomment-1119158298

   run java postcommit


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17394:
URL: https://github.com/apache/beam/pull/17394#issuecomment-1119158199

   PTAL. Tests are green. Most of the work was in the GCP setup. Now the pipeline option is not shipped to workers, so that solves that problem for this PR.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles merged pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles merged PR #17394:
URL: https://github.com/apache/beam/pull/17394


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17394:
URL: https://github.com/apache/beam/pull/17394#issuecomment-1102817482

   R: @ryanthompson591 
   
   I'm figuring I can piggyback an integration test for this on whatever you come up with for you PR #17244.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17394:
URL: https://github.com/apache/beam/pull/17394#issuecomment-1119078565

   Noting that everything is green, aka I didn't break anything. And if you believe my comments and code, this works. So now I am going to continue with changes within the Google Cloud Console and a new commit that might break things again.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17394:
URL: https://github.com/apache/beam/pull/17394#issuecomment-1119745179

   Agree. I will do some set up of the `apache-beam-testing` project to get this tested.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] aaltay commented on pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #17394:
URL: https://github.com/apache/beam/pull/17394#issuecomment-1112496846

   What is the next step on this PR?


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] kennknowles commented on pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
kennknowles commented on PR #17394:
URL: https://github.com/apache/beam/pull/17394#issuecomment-1119047075

   Added a test, but unfortunately Jenkins testing runs as the same service account as the Dataflow workers (both are the default GCE service account) so I cannot remove permissions from the account and ensure the test is meaningful. I have run the test as myself locally to confirm, but more work is needed to make sure CI can truly keep this feature healthy.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lukecwik commented on a diff in pull request #17394: [BEAM-14014] Add parameter for service account impersonation in GCP credentials

Posted by GitBox <gi...@apache.org>.
lukecwik commented on code in PR #17394:
URL: https://github.com/apache/beam/pull/17394#discussion_r888490254


##########
sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java:
##########
@@ -169,6 +169,25 @@ public interface GcpOptions extends GoogleApiDebugOptions, PipelineOptions {
 
   void setGcpCredential(Credentials value);
 
+  /**
+   * All API requests will be made as the given service account or target service account in an
+   * impersonation delegation chain instead of the currently selected account. You can specify
+   * either a single service account as the impersonator, or a comma-separated list of service
+   * accounts to create an impersonation delegation chain.
+   */
+  @Description(
+      "All API requests will be made as the given service account or"
+          + " target service account in an impersonation delegation chain"
+          + " instead of the currently selected account. You can specify"
+          + " either a single service account as the impersonator, or a"
+          + " comma-separated list of service accounts to create an"
+          + " impersonation delegation chain.")
+  @JsonIgnore
+  @Nullable
+  String getImpersonateServiceAccount();
+
+  void setImpersonateServiceAccount(String impersonateServiceAccount);

Review Comment:
   It seems odd to take a string that we parse instead of List<String>/String[] since PipelineOptionsFactory can do this already for us. See https://github.com/apache/beam/blob/937e22ffcb56c0722b558e7c5c1c30fda5116be8/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java#L1423



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org