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

HIVE-1159: Cloud Credentials Operator (CCO) support for deleting GCP root creds post-install #28974

Merged
merged 1 commit into from
Feb 22, 2021
Merged

HIVE-1159: Cloud Credentials Operator (CCO) support for deleting GCP root creds post-install #28974

merged 1 commit into from
Feb 22, 2021

Conversation

jeana-redhat
Copy link
Contributor

@jeana-redhat jeana-redhat commented Jan 29, 2021 •
edited

Primarily an effort to cover HIVE-1159: Cloud Credentials Operator (CCO) support for deleting GCP root creds post-install, but also clarifying some sections and bringing them into parallel construction, and elaborating on the credentials rotation process.

Previews

@jeana-redhat jeana-redhat added this to the Future Release milestone Jan 29, 2021
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 29, 2021
@netlify
Copy link

netlify bot commented Jan 29, 2021 •
edited

Deploy preview for osdocs ready!

Built with commit 6dc493b

https://deploy-preview-28974--osdocs.netlify.app

@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch 7 times, most recently from d3ffb8a to 1e057cd Compare February 5, 2021 00:07
@jeana-redhat jeana-redhat changed the title [WIP] Co 1159 delete gcp root creds post install CO-1159 delete gcp root creds post install Feb 5, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 5, 2021
@jeana-redhat jeana-redhat changed the title CO-1159 delete gcp root creds post install HIVE-1159: Cloud Credentials Operator (CCO) support for deleting GCP root creds post-install Feb 5, 2021
jeana-redhat
jeana-redhat commented Feb 5, 2021
View reviewed changes
Copy link
Contributor Author

@jeana-redhat jeana-redhat left a comment

Choose a reason for hiding this comment

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

(Adding a few minor edits I noticed while re-reviewing previews, will grab as part of SME revisions)

post_installation_configuration/cluster-tasks.adoc Show resolved Hide resolved
operators/operator-reference.adoc Show resolved Hide resolved
modules/mint-mode-with-removal-of-admin-credential.adoc Outdated Show resolved Hide resolved
@jeana-redhat
Copy link
Contributor Author

@akhil-rane this PR has revisions to cover cred removal on GCP, and some related changes to support that addition. PTAL when you can 🙏

akhil-rane
akhil-rane reviewed Feb 5, 2021
View reviewed changes
Copy link

@akhil-rane akhil-rane left a comment

Choose a reason for hiding this comment

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

@jeana-redhat In the section Rotating cloud provider credentials manually, it is mentioned that supported platforms are AWS, Azure, and GCP. But I think other platforms are also supported. Only thing is that they are not supported for Mint mode (as mentioned in the support matrix).

@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch from 1e057cd to 8979363 Compare February 8, 2021 16:11
@jeana-redhat
Copy link
Contributor Author

jeana-redhat commented Feb 8, 2021 •
edited

@jeana-redhat In the section Rotating cloud provider credentials manually, it is mentioned that supported platforms are AWS, Azure, and GCP. But I think other platforms are also supported. Only thing is that they are not supported for Mint mode (as mentioned in the support matrix).

Updated rotation task prereq to split out mint vs passthrough supported status. Thank you!

@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch from 8979363 to 33e418c Compare February 8, 2021 16:16
@akhil-rane
Copy link

Here are some of the things that I noticed during the review:

  1. when the user is extracting credentials requests from the release image using the command oc adm release extract quay.io/openshift-release-dev/ocp-release:4.y.z-x86_64 --credentials-requests --cloud=azure, nothing gets displayed in stdout (in 4.7) as mentioned in the docs. Instead yaml files are created for each credentials request object.

  2. In the section Removing cloud provider credentials, I am not sure why step 2 (Deleting IAM users) and step 3 (Deleting component secrets) are required?

  3. For azure and gcp sections for Manually creating IAM it is mentioned that we can set the credentialsMode parameter for the CCO to Manual in the install-config.yaml when installing OpenShift Container Platform. But unlike AWS this step is missing from GCP and Azure.

  4. Docs say that removal of admin credentials are only supported in AWS and GCP. I am not sure why that is not possible in Azure.

@joelddiaz your input on some of the above topics will help a lot in case I miss something :)

@joelddiaz
Copy link
Contributor

Can't remove creds in Azure b/c there is no read-only credentials defined for Azure so that CCO can verify that things are working.

@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch from 33e418c to aaccc0d Compare February 9, 2021 18:16
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 9, 2021
@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch 2 times, most recently from 927b631 to 5d4426f Compare February 9, 2021 18:21
@openshift-ci-robot openshift-ci-robot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 9, 2021
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 9, 2021
@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch from 5d4426f to 1db92f6 Compare February 9, 2021 21:14
@jeana-redhat
Copy link
Contributor Author

Ok - I think this most recent push resolves previous issues, but PTAL @akhil-rane & @joelddiaz 🙏 Preview links in fist comment have auto-updated for easier reading :)

@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch 2 times, most recently from 477e9ec to 5f92d46 Compare February 10, 2021 15:32
@jeana-redhat
Copy link
Contributor Author

@lwan-wanglin PTAL at the procedures here when you can - some changes from what you sent me (including doing some of it in the GUI instead of CLI). Preview links are in the top comment on the PR.

@jeana-redhat jeana-redhat added the peer-review-needed Signifies that the peer review team needs to review this PR label Feb 16, 2021
@jeana-redhat jeana-redhat mentioned this pull request Feb 16, 2021
Merged
adellape
adellape reviewed Feb 18, 2021
View reviewed changes
Copy link
Contributor

@adellape adellape left a comment

Choose a reason for hiding this comment

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

Real good! Just a few minor suggestions.

post_installation_configuration/cluster-tasks.adoc Outdated Show resolved Hide resolved
modules/manually-rotating-cloud-creds.adoc Outdated Show resolved Hide resolved
modules/manually-removing-cloud-creds.adoc Outdated Show resolved Hide resolved
modules/alternatives-to-storing-admin-secrets-in-kube-system.adoc Outdated Show resolved Hide resolved
modules/manually-removing-cloud-creds.adoc Outdated Show resolved Hide resolved
modules/manually-removing-cloud-creds.adoc Outdated Show resolved Hide resolved
modules/mint-mode-with-removal-of-admin-credential.adoc Outdated Show resolved Hide resolved
installing/installing_aws/manually-creating-iam.adoc Outdated Show resolved Hide resolved
installing/installing_azure/manually-creating-iam-azure.adoc Outdated Show resolved Hide resolved
installing/installing_gcp/manually-creating-iam-gcp.adoc Outdated Show resolved Hide resolved
@adellape adellape added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Feb 18, 2021
@jeana-redhat jeana-redhat force-pushed the CO-1159_delete_GCP_root_creds_post-install branch from 5f92d46 to 6dc493b Compare February 18, 2021 14:59
@jeana-redhat
Copy link
Contributor Author

Status update:
✔️ SME review
✔️ Editorial review
And I see that Lin is back (welcome back!) and has moved the Jira to QE assigned 👍

@lwan-wanglin
Copy link

Status update:
SME review
Editorial review
And I see that Lin is back (welcome back!) and has moved the Jira to QE assigned

LGTM! The feature works well on my test and doc looks good to me.

@jeana-redhat jeana-redhat merged commit 30e7b9c into openshift:master Feb 22, 2021
@jeana-redhat
Copy link
Contributor Author

/cherrypick enterprise-4.7

@openshift-cherrypick-robot
Copy link

@jeana-redhat: new pull request created: #29658

In response to this:

/cherrypick enterprise-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Feb 22, 2021
Merged
@jeana-redhat jeana-redhat deleted the CO-1159_delete_GCP_root_creds_post-install branch July 16, 2021 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adellape adellape adellape left review comments

@joelddiaz joelddiaz joelddiaz left review comments

@akhil-rane akhil-rane akhil-rane left review comments

@codyhoag codyhoag codyhoag approved these changes

@aburdenthehand aburdenthehand Awaiting requested review from aburdenthehand

@vikram-redhat vikram-redhat Awaiting requested review from vikram-redhat

Assignees
No one assigned
Labels
branch/enterprise-4.7 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 4.7 GA
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@jeana-redhat @akhil-rane @joelddiaz @lwan-wanglin @openshift-cherrypick-robot @codyhoag @adellape @vikram-redhat @openshift-ci-robot