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.

Sign up for GitHub

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

Jump to bottom

[KFP] Update authenticating to GCP doc about KF 1.0 and hosted #1832

Merged
merged 12 commits into from
Apr 10, 2020
Merged

[KFP] Update authenticating to GCP doc about KF 1.0 and hosted #1832

merged 12 commits into from
Apr 10, 2020

Conversation

Bobgy
Copy link
Contributor

@Bobgy Bobgy commented Mar 23, 2020 •
edited by jlewi
Loading

/assign @joeliedtke
/cc @rmgogogo

Changes:

  • Documented for Kubeflow 1.0, workload identity is used by default
  • Documented for GCP Hosted Pipelines, instructions to use workload identity

This change is Reviewable

@k8s-ci-robot k8s-ci-robot requested a review from rmgogogo March 23, 2020 03:47
This was referenced Mar 23, 2020
Closed
Closed
@joeliedtke
Copy link
Member

/retest

@joeliedtke
Copy link
Member

/test

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 24, 2020

Looks like there was something wrong with netlify. I will push a commit to retrigger it

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 24, 2020

@joeliedtke would you have time taking a look again?

@joeliedtke
Copy link
Member

Preview:
https://deploy-preview-1832--competent-brattain-de2d6d.netlify.com/docs/gke/pipelines/authentication-pipelines/

https://deploy-preview-1832--competent-brattain-de2d6d.netlify.com/docs/pipelines/installation/standalone-deployment/

joeliedtke
joeliedtke reviewed Mar 25, 2020
View reviewed changes
Copy link
Member

@joeliedtke joeliedtke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this doc. I've added a few suggestions.

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 26, 2020

Thanks @joeliedtke for the review! I will fix these

Bobgy
Bobgy commented Mar 26, 2020
View reviewed changes
Copy link
Contributor Author

@Bobgy Bobgy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Joe for the review!
I've updated accordingly

@Bobgy
Copy link
Contributor Author

Bobgy commented Mar 31, 2020

@joeliedtke I've updated again. PTAL
thanks!

Bobgy
Bobgy commented Apr 2, 2020
View reviewed changes
Copy link
Contributor Author

@Bobgy Bobgy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joeliedtke Is there any further requests? Can we merge this in now?

@rmgogogo
Copy link
Contributor

rmgogogo commented Apr 8, 2020

/lgtm

@Bobgy
Copy link
Contributor Author

Bobgy commented Apr 8, 2020

FYI, the doc updated here is referenced by https://cloud.google.com/ai-platform/pipelines/docs/configure-gke-cluster#grant-access

@k8s-ci-robot k8s-ci-robot removed the lgtm label Apr 8, 2020
@rmgogogo
Copy link
Contributor

rmgogogo commented Apr 8, 2020

/lgtm

@Bobgy
Copy link
Contributor Author

Bobgy commented Apr 8, 2020

@joeliedtke for approval

@Bobgy
Copy link
Contributor Author

Bobgy commented Apr 10, 2020 •
edited
Loading

@joeliedtke reminder, I synced with @rmgogogo and we decided to not include documentation for manually setting up workload identity for marketplace, because that's not what marketplace targets. It should provide one click setup experience.

Standalone will be the choice for customizations like workload identity.

Can you double check the new update and whether we can merge this in?

@joeliedtke
Copy link
Member

/approve

Thanks for updating this doc @Bobgy!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joeliedtke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 3c7748a into kubeflow:master Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmgogogo rmgogogo rmgogogo left review comments

@numerology numerology numerology left review comments

@joeliedtke joeliedtke joeliedtke left review comments

Assignees

@rmgogogo rmgogogo

@joeliedtke joeliedtke

Labels
approved lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Bobgy @joeliedtke @rmgogogo @k8s-ci-robot @numerology