Set CredentialsMode on Manual-only Platforms #5325

patrickdillon · 2021-10-25T19:12:30Z

CredentialsMode should be set for platforms that only allow manual mode so that no cloud credentials are uploaded to the cluster by the installer. #4416 created logic in the installer to not create cluster credentials when running in manual mode, but unless CredentialsMode: Manual is explicitly set in the install config, that logic would be ignored.

With Azure Stack Hub, IBM, and Alibaba being manual-only platforms, we should use a default credential mode for these platforms to ensure that manual-mode logic is being observed. We should also consider future work of disabling the CCO when manual mode is set.

CredentialsMode should be set for platforms that only allow manual mode so that no cloud credentials are uploaded to the cluster by the installer.

staebler · 2021-10-25T20:37:21Z

It do not think that this is the correct behavior. There are platforms where the credentials stored in kube-system are needed. The CCO is only one potential consumer of the credentials. The CCO running in Manual credentials mode does not preclude other consumers from needing the credentials in kube-system.

patrickdillon · 2021-10-26T15:34:56Z

It do not think that this is the correct behavior. There are platforms where the credentials stored in kube-system are needed. The CCO is only one potential consumer of the credentials. The CCO running in Manual credentials mode does not preclude other consumers from needing the credentials in kube-system.

There are a few issues here to untangle.

In manual mode, all credentials in the cluster should be created by the cluster. The installer is correctly observing this behavior, but there are two (or maybe three) different classes of platforms: manual-mode-only platforms and CCO-supported platforms.

At the moment, the only manual-mode-only platform is Azure Stack Hub. The issue with this type of platform is that we want it to run in manual mode, but unless the user sets CredentialsMode: Manual in the install config, the installer does not know it is in manual mode. (I think this is a flaw in the installer that we allow CredentialsMode to be unset & have no default.) So that is the point of this PR, to set the mode for those platforms (and improve my sloppy implementation for ASH). IBM and Alibaba will be running in manual mode, and they should not have the ability to create a cluster secret, but even so they would benefit from this if we add logic to automatically disable the CCO in manual mode, which I think we should do.

I think the issues you pointed out are more related to the second-kind of clusters that have existing support in the CCO. These platforms do not run in manual mode by default, so #4416 is already preventing the kube-system secret from being written. This PR should not affect those platforms. If the kube-system credential is required by platforms like aws, gcp, or public azure, I think creating it is not covered by our documentation. But I have not tested a manual install and it is unclear to me from reading the code whether there is actually an issue; the machine provider spec for those three platforms seem to use secrets in the machine-api namespace.

staebler · 2021-10-26T19:28:41Z

@patrickdillon My concern was with platforms that do not have any interaction with CCO but that still make use of the creds secret in the kube-system namespace. Some examples are openstack and vsphere. If we set the credentials mode to manual, then the installer will not write out the manifest to create the creds secret. See

installer/pkg/asset/manifests/openshift.go

Lines 240 to 243 in 1b4b561

    
           case awstypes.Name, openstacktypes.Name, vspheretypes.Name, azuretypes.Name, gcptypes.Name, ibmcloudtypes.Name, ovirttypes.Name: 
        
           	if installConfig.Config.CredentialsMode != types.ManualCredentialsMode { 
        
           		assetData["99_cloud-creds-secret.yaml"] = applyTemplateData(cloudCredsSecret.Files()[0].Data, templateData) 
        
           	}

.

But I see after closer inspection of the changes in this PR that the credentials mode is only set for platforms that allow the field to be set at all. I find it surprising that we have some platforms that allow the credentials mode to be set but only to Manual and other platforms that do not allow the credentials mode to be set at all. Is the distinction that for the former there are CredentialsRequests generated but for the latter there are not?

patrickdillon · 2021-10-26T19:53:14Z

@patrickdillon My concern was with platforms that do not have any interaction with CCO but that still make use of the creds secret in the kube-system namespace. Some examples are openstack and vsphere. If we set the credentials mode to manual, then the installer will not write out the manifest to create the creds secret. See

installer/pkg/asset/manifests/openshift.go

Lines 240 to 243 in 1b4b561

case awstypes.Name, openstacktypes.Name, vspheretypes.Name, azuretypes.Name, gcptypes.Name, ibmcloudtypes.Name, ovirttypes.Name:

if installConfig.Config.CredentialsMode != types.ManualCredentialsMode {

assetData["99_cloud-creds-secret.yaml"] = applyTemplateData(cloudCredsSecret.Files()[0].Data, templateData)

}

.

But I see after closer inspection of the changes in this PR that the credentials mode is only set for platforms that allow the field to be set at all. I find it surprising that we have some platforms that allow the credentials mode to be set but only to Manual and other platforms that do not allow the credentials mode to be set at all. Is the distinction that for the former there are CredentialsRequests generated but for the latter there are not?

I would assume for those where credentialsmode cannot be set, they can only run in mint mode (I could be very wrong).

I see credenetialsrequests for vsphere:

$ oc adm release extract quay.io/openshift-release-dev/ocp-release:4.9.0-x86_64 --credentials-requests --cloud=vsphere
$ ls
0000_30_machine-api-operator_00_credentials-request.yaml  0000_50_cluster-storage-operator_03_credentials_request_vsphere_csi.yaml  0000_50_cluster-storage-operator_03_credentials_request_vsphere_detector.yaml

and openstack:

$ oc adm release extract quay.io/openshift-release-dev/ocp-release:4.9.0-x86_64 --credentials-requests --cloud=openstack
$ ls
0000_26_cloud-controller-manager-operator_13_credentialsrequest-openstack.yaml          0000_50_cluster-storage-operator_03_credentials_request_cinder.yaml
0000_30_machine-api-operator_00_credentials-request.yaml                                0000_50_cluster-storage-operator_03_credentials_request_manila.yaml
0000_50_cluster-image-registry-operator_01-registry-credentials-request-openstack.yaml  0000_70_cluster-network-operator_01_credentialsrequest.yaml

staebler · 2021-10-26T20:34:17Z

After consultation with the CCO team, the on-prem platforms are running in passthrough mode. Should we set the mode to passthrough by default for those platforms rather than leave it empty?

openshift-ci · 2022-03-25T18:45:57Z

@patrickdillon: PR needs rebase.

Details

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-ci · 2022-03-25T18:45:58Z

@patrickdillon: PR needs rebase.

Details

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-ci · 2022-03-25T18:45:58Z

@patrickdillon: PR needs rebase.

Details

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.

sadasu · 2022-07-25T15:57:41Z

@patrickdillon is this still an issue we want to solve?

patrickdillon · 2022-09-12T20:36:07Z

/close

patrickdillon · 2022-09-12T20:36:26Z

/reopen

patrickdillon · 2022-09-12T20:36:52Z

/refresh

openshift-ci · 2022-09-12T20:38:12Z

@patrickdillon: Closed this PR.

Details

In response to this:

/close

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-ci · 2022-09-12T20:38:21Z

@patrickdillon: Reopened this PR.

Details

In response to this:

/reopen

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-ci · 2022-09-12T20:43:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SHAKY404
Once this PR has been reviewed and has the lgtm label, please ask for approval from patrickdillon by writing /assign @patrickdillon in a comment. For more information see:The Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

openshift-ci · 2022-09-12T20:43:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SHAKY404
Once this PR has been reviewed and has the lgtm label, please ask for approval from patrickdillon by writing /assign @patrickdillon in a comment. For more information see:The Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

jstuever · 2022-10-12T16:07:25Z

/cc @patrickdillon
/uncc

openshift-ci · 2022-10-12T16:08:42Z

@jstuever: GitHub didn't allow me to request PR reviews from the following users: patrickdillon.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @patrickdillon
/uncc

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-ci · 2023-01-12T16:44:21Z

@patrickdillon: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-workers-rhel8	`b7487d0`	link	false	`/test e2e-aws-workers-rhel8`
ci/prow/e2e-aws-workers-rhel7	`b7487d0`	link	false	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-aws-single-node	`b7487d0`	link	false	`/test e2e-aws-single-node`
ci/prow/e2e-crc	`b7487d0`	link	false	`/test e2e-crc`
ci/prow/e2e-openstack-kuryr	`b7487d0`	link	false	`/test e2e-openstack-kuryr`
ci/prow/e2e-alibaba	`b7487d0`	link	true	`/test e2e-alibaba`
ci/prow/openstack-manifests	`b7487d0`	link	true	`/test openstack-manifests`
ci/prow/e2e-gcp-upgrade	`b7487d0`	link	true	`/test e2e-gcp-upgrade`
ci/prow/e2e-aws-upgrade	`b7487d0`	link	true	`/test e2e-aws-upgrade`
ci/prow/e2e-azure-upi	`b7487d0`	link	true	`/test e2e-azure-upi`
ci/prow/e2e-gcp-upi	`b7487d0`	link	true	`/test e2e-gcp-upi`
ci/prow/e2e-aws-upi	`b7487d0`	link	true	`/test e2e-aws-upi`
ci/prow/e2e-azure	`b7487d0`	link	true	`/test e2e-azure`
ci/prow/e2e-vsphere	`b7487d0`	link	true	`/test e2e-vsphere`
ci/prow/e2e-gcp	`b7487d0`	link	true	`/test e2e-gcp`
ci/prow/e2e-aws	`b7487d0`	link	true	`/test e2e-aws`
ci/prow/okd-e2e-gcp-ovn-upgrade	`b7487d0`	link	false	`/test okd-e2e-gcp-ovn-upgrade`
ci/prow/e2e-openstack	`b7487d0`	link	false	`/test e2e-openstack`
ci/prow/images	`b7487d0`	link	true	`/test images`
ci/prow/gofmt	`b7487d0`	link	true	`/test gofmt`
ci/prow/aro-unit	`b7487d0`	link	true	`/test aro-unit`
ci/prow/shellcheck	`b7487d0`	link	true	`/test shellcheck`
ci/prow/okd-images	`b7487d0`	link	true	`/test okd-images`
ci/prow/verify-vendor	`b7487d0`	link	true	`/test verify-vendor`
ci/prow/tf-fmt	`b7487d0`	link	false	`/test tf-fmt`
ci/prow/tf-lint	`b7487d0`	link	true	`/test tf-lint`
ci/prow/e2e-metal-assisted	`b7487d0`	link	false	`/test e2e-metal-assisted`
ci/prow/unit	`b7487d0`	link	true	`/test unit`
ci/prow/okd-verify-codegen	`b7487d0`	link	true	`/test okd-verify-codegen`
ci/prow/golint	`b7487d0`	link	true	`/test golint`
ci/prow/e2e-libvirt	`b7487d0`	link	false	`/test e2e-libvirt`
ci/prow/okd-e2e-aws-ovn	`b7487d0`	link	false	`/test okd-e2e-aws-ovn`
ci/prow/okd-unit	`b7487d0`	link	true	`/test okd-unit`
ci/prow/e2e-ibmcloud-ovn	`b7487d0`	link	false	`/test e2e-ibmcloud-ovn`
ci/prow/yaml-lint	`b7487d0`	link	true	`/test yaml-lint`
ci/prow/verify-codegen	`b7487d0`	link	true	`/test verify-codegen`
ci/prow/e2e-ovirt	`b7487d0`	link	false	`/test e2e-ovirt`
ci/prow/govet	`b7487d0`	link	true	`/test govet`
ci/prow/e2e-metal-ipi	`b7487d0`	link	false	`/test e2e-metal-ipi`
ci/prow/e2e-aws-ovn	`b7487d0`	link	true	`/test e2e-aws-ovn`
ci/prow/e2e-azure-ovn	`b7487d0`	link	true	`/test e2e-azure-ovn`
ci/prow/e2e-vsphere-ovn	`b7487d0`	link	true	`/test e2e-vsphere-ovn`
ci/prow/e2e-gcp-ovn	`b7487d0`	link	true	`/test e2e-gcp-ovn`
ci/prow/okd-scos-images	`b7487d0`	link	true	`/test okd-scos-images`
ci/prow/okd-scos-verify-codegen	`b7487d0`	link	true	`/test okd-scos-verify-codegen`
ci/prow/okd-scos-unit	`b7487d0`	link	true	`/test okd-scos-unit`
ci/prow/e2e-agent-compact-ipv4	`b7487d0`	link	true	`/test e2e-agent-compact-ipv4`
ci/prow/agent-integration-tests	`b7487d0`	link	true	`/test agent-integration-tests`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

openshift-bot · 2023-04-13T09:00:38Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2023-05-14T00:30:15Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2023-06-13T08:00:29Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2023-06-13T08:02:28Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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-ci bot requested review from jstuever and rna-afk October 25, 2021 19:14

patrickdillon mentioned this pull request Oct 25, 2021

[wip] Alibaba recommitted #5291

Closed

Set CredentialsMode on Manual-only Platforms

b7487d0

CredentialsMode should be set for platforms that only allow manual mode so that no cloud credentials are uploaded to the cluster by the installer.

patrickdillon force-pushed the default-credentialMode branch from a55fa47 to b7487d0 Compare October 25, 2021 19:29

patrickdillon mentioned this pull request Dec 20, 2021

stop considering Mint mode as supported on Azure #5496

Closed

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 25, 2022

SHAKY404 approved these changes Jun 29, 2022

View reviewed changes

patrickdillon closed this Sep 12, 2022

patrickdillon reopened this Sep 12, 2022

openshift-ci bot closed this Sep 12, 2022

openshift-ci bot reopened this Sep 12, 2022

openshift-ci bot removed the request for review from jstuever October 12, 2022 16:07

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 13, 2023

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 14, 2023

openshift-ci bot closed this Jun 13, 2023

Set CredentialsMode on Manual-only Platforms #5325

Set CredentialsMode on Manual-only Platforms #5325

Uh oh!

Conversation

patrickdillon commented Oct 25, 2021

Uh oh!

staebler commented Oct 25, 2021

Uh oh!

patrickdillon commented Oct 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staebler commented Oct 26, 2021

Uh oh!

patrickdillon commented Oct 26, 2021

Uh oh!

staebler commented Oct 26, 2021

Uh oh!

openshift-ci bot commented Mar 25, 2022

Uh oh!

openshift-ci bot commented Mar 25, 2022

Uh oh!

openshift-ci bot commented Mar 25, 2022

Uh oh!

sadasu commented Jul 25, 2022

Uh oh!

patrickdillon commented Sep 12, 2022

Uh oh!

patrickdillon commented Sep 12, 2022

Uh oh!

patrickdillon commented Sep 12, 2022

Uh oh!

openshift-ci bot commented Sep 12, 2022

Uh oh!

openshift-ci bot commented Sep 12, 2022

Uh oh!

openshift-ci bot commented Sep 12, 2022

Uh oh!

openshift-ci bot commented Sep 12, 2022

Uh oh!

jstuever commented Oct 12, 2022

Uh oh!

openshift-ci bot commented Oct 12, 2022

Uh oh!

openshift-ci bot commented Jan 12, 2023

Uh oh!

openshift-bot commented Apr 13, 2023

Uh oh!

openshift-bot commented May 14, 2023

Uh oh!

openshift-bot commented Jun 13, 2023

Uh oh!

openshift-ci bot commented Jun 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

patrickdillon commented Oct 26, 2021 •

edited

Loading