Skip to content

stop considering Mint mode as supported on Azure #5496

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

Closed
joelddiaz wants to merge 1 commit into from
Closed

stop considering Mint mode as supported on Azure #5496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions data/data/install.openshift.io_installconfigs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -979,9 +979,9 @@ spec:
\"Manual\": CredentialsRequests must be handled manually by the user
\n For each of the following platforms, the field can set to the specified
values. For all other platforms, the field must not be set. AWS: \"Mint\",
\"Passthrough\", \"Manual\" Azure: \"Mint\", \"Passthrough\", \"Manual\"
AzureStack: \"Manual\" GCP: \"Mint\", \"Passthrough\", \"Manual\" IBMCloud:
\"Manual\" AlibabaCloud: \"Manual\""
\"Passthrough\", \"Manual\" Azure: \"Passthrough\", \"Manual\" AzureStack:
\"Manual\" GCP: \"Mint\", \"Passthrough\", \"Manual\" IBMCloud: \"Manual\"
AlibabaCloud: \"Manual\""
enum:
- ""
- Mint
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/installconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ type InstallConfig struct {
// For each of the following platforms, the field can set to the specified values. For all other platforms, the
// field must not be set.
// AWS: "Mint", "Passthrough", "Manual"
// Azure: "Mint", "Passthrough", "Manual"
// Azure: "Passthrough", "Manual"
// AzureStack: "Manual"
// GCP: "Mint", "Passthrough", "Manual"
// IBMCloud: "Manual"
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/validation/installconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ func validateCloudCredentialsMode(mode types.CredentialsMode, fldPath *field.Pat
}
allErrs := field.ErrorList{}

allowedAzureModes := []types.CredentialsMode{types.MintCredentialsMode, types.PassthroughCredentialsMode, types.ManualCredentialsMode}
allowedAzureModes := []types.CredentialsMode{types.PassthroughCredentialsMode, types.ManualCredentialsMode}
Copy link
Contributor

@patrickdillon patrickdillon Dec 20, 2021

Choose a reason for hiding this comment

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

This seems reasonable to me. #5325 is a start to clean up some loose ends in how the installer handles credential modes (I think we want to move toward choosing defaults rather than allowing credentialmode to be ""). #5325 would need to handle this case as well, where we have two modes.

Copy link
Contributor

@staebler staebler Feb 1, 2022

Choose a reason for hiding this comment

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

The issue with choosing a default instead of an empty string is that they mean different things. When an explicit selection is made, it (1) tells the cloud-credential-operator to use that mode and (2) skips permissions validation. When an empty string is used, it tells the cloud-credential-operator to inspect the credentials provided and make an opinionated decision about which mode to use based upon what is allowed with the credentials.

Copy link
Author

@joelddiaz joelddiaz Feb 1, 2022

Choose a reason for hiding this comment

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

Of course, now with Azure there is only Passthrough. So no more intelligent querying of the creds.

if platform.Azure != nil && platform.Azure.CloudName == azure.StackCloud {
allowedAzureModes = []types.CredentialsMode{types.ManualCredentialsMode}
}
Expand Down