Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make user-gcp-sa secret optional on GCP #2589

Closed
2 of 5 tasks
Bobgy opened this issue Nov 11, 2019 · 8 comments
Closed
2 of 5 tasks

Make user-gcp-sa secret optional on GCP #2589

Bobgy opened this issue Nov 11, 2019 · 8 comments

Comments

@Bobgy
Copy link
Contributor

Bobgy commented Nov 11, 2019

UPDATE:
When GKE provides application default credentials with enough permission through one of the following three ways:

  • Project default service account
  • User specified GCE default service account (cluster or node pool level fine grained control)
  • Workload identity (kubernetes service account level fine grained control)

There's no need to set user-gcp-sa in https://github.com/kubeflow/pipelines/blob/master/manifests/gcp_marketplace/guide.md#gcp-service-account-credentials. This allows zero config on-boarding experience.

This issue tracks various efforts towards making user-gcp-sa optional.

Pipeline samples:

  • If user-gcp-sa is not present, drop GOOGLE_APPLICATION_CREDENTIALS env in pipelines api server before running it. Drop GCP credentials env if user-gcp-sa secret is not present #2643
    With the above done, we no longer need to change all pipeline samples with GCP auth usage
    This PR is abandoned to avoid making short-term workarounds that we need to remove in the future too.
Hidden links to samples * [ ] https://github.com/kubeflow/pipelines/blob/d5e27e291fbe09cfa63a9de0f5b4fdea65af7c6f/sdk/python/kfp/gcp.py#L18
* [ ] https://github.com/kubeflow/pipelines/tree/master/samples/core/xgboost_training_cm
* [ ] https://github.com/kubeflow/pipelines/tree/master/samples/contrib/parameterized_tfx_oss
* [ ] [tfx sample](https://github.com/tensorflow/tfx/blob/6b6a2f9b69c93ce8a889c48d9e02354ed52c3f89/tfx/orchestration/kubeflow/kubeflow_dag_runner.py#L109)

Servers

  • frontend node server should not use user-gcp-sa on Kubeflow deployment
  • CloudSqlProxy /credentials/gcp-sa-token on marketplace
  • Minio /etc/credentials/gcp-sa-token on marketplace
  • TensorBoardViewer
@Bobgy
Copy link
Contributor Author

Bobgy commented Nov 11, 2019

/cc @rmgogogo

@Bobgy Bobgy changed the title Make user-gcp-sa optional on GCP Make user-gcp-sa secret optional on GCP Nov 11, 2019
@Ark-kun
Copy link
Contributor

Ark-kun commented Nov 11, 2019

The SDK extension method is probably still useful, but I agree that the samples should be cleaned.
We need to verify that everything works out of the box on all deployments:

  • Marketplace
  • Standalone
  • Kubeflow

@IronPan
Copy link
Member

IronPan commented Nov 12, 2019

Using default SA is not considered a good practice https://cloud.google.com/kubernetes-engine/docs/tutorials/authenticating-to-cloud-platform#why_use_service_accounts

  • No visibility on how it's setup
  • Harder to revoke permission
  • It leads to a misleading habit for user to setup and will be a debt to remove when supporting workload identity.

@Bobgy
Copy link
Contributor Author

Bobgy commented Nov 14, 2019

Efforts are paused because we agreed that KFP will eventually move all deployments to workload identity. When that happens, we have no need for taking credentials.

On-prem will still need this, but it's not our priority. We can revisit the issue later.

@Bobgy
Copy link
Contributor Author

Bobgy commented Nov 21, 2019

Updated description to better reflect current status. Bumped to p0 because it blocks both multi user efforts and marketplace efforts.

@Bobgy
Copy link
Contributor Author

Bobgy commented Dec 30, 2019

Migrate standalone deployment to workload identity on GCP #2619 removes all use_gcp_secret usages in samples, so they can be run in both workload identity and GCP hosted ML pipelines

@stale
Copy link

stale bot commented Jun 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added lifecycle/stale The issue / pull request is stale, any activities remove this label. and removed lifecycle/stale The issue / pull request is stale, any activities remove this label. labels Jun 25, 2020
@Bobgy
Copy link
Contributor Author

Bobgy commented Jun 29, 2020

We are generally recommending workload identity now because it reached GA. Closing the issue.

@Bobgy Bobgy closed this as completed Jun 29, 2020
magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this issue Oct 22, 2023
* adding prometheus serverless and model name labels

Signed-off-by: alexagriffith <agriffith50@bloomberg.net>

* adding sanitize func

Signed-off-by: alexagriffith <agriffith50@bloomberg.net>

* cleaning up tests

Signed-off-by: alexagriffith <agriffith50@bloomberg.net>

* remove error log

Signed-off-by: alexagriffith <agriffith50@bloomberg.net>

* go mod tidy

Signed-off-by: alexagriffith <agriffith50@bloomberg.net>

Signed-off-by: alexagriffith <agriffith50@bloomberg.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants