diff --git a/Makefile b/Makefile index 7d24eedae..0c1ca008d 100644 --- a/Makefile +++ b/Makefile @@ -76,7 +76,13 @@ $(call build-image,ocp-cloud-credential-operator,$(IMAGE_REGISTRY)/ocp/4.5:cloud $(call add-crd-gen,cloudcredential-manifests,./pkg/apis/cloudcredential/v1,./manifests,./manifests) $(call add-crd-gen,cloudcredential-bindata,./pkg/apis/cloudcredential/v1,./bindata/bootstrap,./bindata/bootstrap) -update: update-codegen +update: update-vendored-crds update-codegen update-bindata + +update-vendored-crds: + # copy config CRD from openshift/api + cp vendor/github.com/openshift/api/operator/v1/0000_40_cloud-credential-operator_00_config.crd.yaml ./manifests/00-config-crd.yaml + # ...and into where we generate bindata from + cp vendor/github.com/openshift/api/operator/v1/0000_40_cloud-credential-operator_00_config.crd.yaml ./bindata/bootstrap/cloudcredential_v1_operator_config_crd.yaml update-codegen: update-codegen-crds ./hack/update-codegen.sh diff --git a/bindata/bootstrap/cloudcredential_v1_credentialsrequest.yaml b/bindata/bootstrap/cloudcredential_v1_credentialsrequest_crd.yaml similarity index 100% rename from bindata/bootstrap/cloudcredential_v1_credentialsrequest.yaml rename to bindata/bootstrap/cloudcredential_v1_credentialsrequest_crd.yaml diff --git a/bindata/bootstrap/cloudcredential_v1_operator_config_crd.yaml b/bindata/bootstrap/cloudcredential_v1_operator_config_crd.yaml new file mode 100644 index 000000000..1723c2337 --- /dev/null +++ b/bindata/bootstrap/cloudcredential_v1_operator_config_crd.yaml @@ -0,0 +1,155 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: cloudcredentials.operator.openshift.io +spec: + scope: Cluster + preserveUnknownFields: false + group: operator.openshift.io + names: + kind: CloudCredential + listKind: CloudCredentialList + plural: cloudcredentials + singular: cloudcredential + subresources: + status: {} + versions: + - name: v1 + served: true + storage: true + validation: + openAPIV3Schema: + description: CloudCredential provides a means to configure an operator to manage + CredentialsRequests. + type: object + required: + - spec + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: CloudCredentialSpec is the specification of the desired behavior + of the cloud-credential-operator. + type: object + properties: + credentialsMode: + description: CredentialsMode allows informing CCO that it should not + attempt to dynamically determine the root cloud credentials capabilities, + and it should just run in the specified mode. It also allows putting + the operator into "manual" mode if desired. Leaving the field in default + mode runs CCO so that the cluster's cloud credentials will be dynamically + probed for capabilities (on supported clouds/platforms). + type: string + enum: + - "" + - Manual + - Mint + - Passthrough + logLevel: + description: logLevel is an intent based logging for an overall component. It + does not give fine grained control, but it is a simple way to manage + coarse grained logging choices that operators have to interpret for + their operands. + type: string + managementState: + description: managementState indicates whether and how the operator + should manage the component + type: string + pattern: ^(Managed|Unmanaged|Force|Removed)$ + observedConfig: + description: observedConfig holds a sparse config that controller has + observed from the cluster state. It exists in spec because it is + an input to the level for the operator + type: object + nullable: true + x-kubernetes-preserve-unknown-fields: true + operatorLogLevel: + description: operatorLogLevel is an intent based logging for the operator + itself. It does not give fine grained control, but it is a simple + way to manage coarse grained logging choices that operators have to + interpret for themselves. + type: string + unsupportedConfigOverrides: + description: 'unsupportedConfigOverrides holds a sparse config that + will override any previously set options. It only needs to be the + fields to override it will end up overlaying in the following order: + 1. hardcoded defaults 2. observedConfig 3. unsupportedConfigOverrides' + type: object + nullable: true + x-kubernetes-preserve-unknown-fields: true + status: + description: CloudCredentialStatus defines the observed status of the cloud-credential-operator. + type: object + properties: + conditions: + description: conditions is a list of conditions and their status + type: array + items: + description: OperatorCondition is just the standard condition fields. + type: object + properties: + lastTransitionTime: + type: string + format: date-time + message: + type: string + reason: + type: string + status: + type: string + type: + type: string + generations: + description: generations are used to determine when an item needs to + be reconciled or has changed in a way that needs a reaction. + type: array + items: + description: GenerationStatus keeps track of the generation for a + given resource so that decisions about forced updates can be made. + type: object + properties: + group: + description: group is the group of the thing you're tracking + type: string + hash: + description: hash is an optional field set for resources without + generation that are content sensitive like secrets and configmaps + type: string + lastGeneration: + description: lastGeneration is the last generation of the workload + controller involved + type: integer + format: int64 + name: + description: name is the name of the thing you're tracking + type: string + namespace: + description: namespace is where the thing you're tracking is + type: string + resource: + description: resource is the resource type of the thing you're + tracking + type: string + observedGeneration: + description: observedGeneration is the last generation change you've + dealt with + type: integer + format: int64 + readyReplicas: + description: readyReplicas indicates how many replicas are ready and + at the desired state + type: integer + format: int32 + version: + description: version is the level this availability applies to + type: string diff --git a/manifests/01-operator-config.yaml b/manifests/01-operator-config.yaml new file mode 100644 index 000000000..80644ebd8 --- /dev/null +++ b/manifests/01-operator-config.yaml @@ -0,0 +1,6 @@ +apiVersion: operator.openshift.io/v1 +kind: CloudCredential +metadata: + name: cluster + annotations: + release.openshift.io/create-only: "true" diff --git a/manifests/01-operator_configmap.yaml b/manifests/01-operator_configmap.yaml deleted file mode 100644 index 92d68f041..000000000 --- a/manifests/01-operator_configmap.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: v1 -kind: ConfigMap -metadata: - name: cloud-credential-operator-config - namespace: openshift-cloud-credential-operator - annotations: - release.openshift.io/create-only: "true" -data: - disabled: "false" diff --git a/pkg/assets/bootstrap/bindata.go b/pkg/assets/bootstrap/bindata.go index 21df9df9b..a8caab0f7 100644 --- a/pkg/assets/bootstrap/bindata.go +++ b/pkg/assets/bootstrap/bindata.go @@ -1,6 +1,7 @@ // Code generated for package bootstrap by go-bindata DO NOT EDIT. (@generated) // sources: -// bindata/bootstrap/cloudcredential_v1_credentialsrequest.yaml +// bindata/bootstrap/cloudcredential_v1_credentialsrequest_crd.yaml +// bindata/bootstrap/cloudcredential_v1_operator_config_crd.yaml // bindata/bootstrap/namespace.yaml package bootstrap @@ -55,7 +56,7 @@ func (fi bindataFileInfo) Sys() interface{} { return nil } -var _bootstrapCloudcredential_v1_credentialsrequestYaml = []byte(`apiVersion: apiextensions.k8s.io/v1beta1 +var _bootstrapCloudcredential_v1_credentialsrequest_crdYaml = []byte(`apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: name: credentialsrequests.cloudcredential.openshift.io @@ -203,17 +204,189 @@ status: storedVersions: [] `) -func bootstrapCloudcredential_v1_credentialsrequestYamlBytes() ([]byte, error) { - return _bootstrapCloudcredential_v1_credentialsrequestYaml, nil +func bootstrapCloudcredential_v1_credentialsrequest_crdYamlBytes() ([]byte, error) { + return _bootstrapCloudcredential_v1_credentialsrequest_crdYaml, nil } -func bootstrapCloudcredential_v1_credentialsrequestYaml() (*asset, error) { - bytes, err := bootstrapCloudcredential_v1_credentialsrequestYamlBytes() +func bootstrapCloudcredential_v1_credentialsrequest_crdYaml() (*asset, error) { + bytes, err := bootstrapCloudcredential_v1_credentialsrequest_crdYamlBytes() if err != nil { return nil, err } - info := bindataFileInfo{name: "bootstrap/cloudcredential_v1_credentialsrequest.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + info := bindataFileInfo{name: "bootstrap/cloudcredential_v1_credentialsrequest_crd.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var _bootstrapCloudcredential_v1_operator_config_crdYaml = []byte(`apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: cloudcredentials.operator.openshift.io +spec: + scope: Cluster + preserveUnknownFields: false + group: operator.openshift.io + names: + kind: CloudCredential + listKind: CloudCredentialList + plural: cloudcredentials + singular: cloudcredential + subresources: + status: {} + versions: + - name: v1 + served: true + storage: true + validation: + openAPIV3Schema: + description: CloudCredential provides a means to configure an operator to manage + CredentialsRequests. + type: object + required: + - spec + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: CloudCredentialSpec is the specification of the desired behavior + of the cloud-credential-operator. + type: object + properties: + credentialsMode: + description: CredentialsMode allows informing CCO that it should not + attempt to dynamically determine the root cloud credentials capabilities, + and it should just run in the specified mode. It also allows putting + the operator into "manual" mode if desired. Leaving the field in default + mode runs CCO so that the cluster's cloud credentials will be dynamically + probed for capabilities (on supported clouds/platforms). + type: string + enum: + - "" + - Manual + - Mint + - Passthrough + logLevel: + description: logLevel is an intent based logging for an overall component. It + does not give fine grained control, but it is a simple way to manage + coarse grained logging choices that operators have to interpret for + their operands. + type: string + managementState: + description: managementState indicates whether and how the operator + should manage the component + type: string + pattern: ^(Managed|Unmanaged|Force|Removed)$ + observedConfig: + description: observedConfig holds a sparse config that controller has + observed from the cluster state. It exists in spec because it is + an input to the level for the operator + type: object + nullable: true + x-kubernetes-preserve-unknown-fields: true + operatorLogLevel: + description: operatorLogLevel is an intent based logging for the operator + itself. It does not give fine grained control, but it is a simple + way to manage coarse grained logging choices that operators have to + interpret for themselves. + type: string + unsupportedConfigOverrides: + description: 'unsupportedConfigOverrides holds a sparse config that + will override any previously set options. It only needs to be the + fields to override it will end up overlaying in the following order: + 1. hardcoded defaults 2. observedConfig 3. unsupportedConfigOverrides' + type: object + nullable: true + x-kubernetes-preserve-unknown-fields: true + status: + description: CloudCredentialStatus defines the observed status of the cloud-credential-operator. + type: object + properties: + conditions: + description: conditions is a list of conditions and their status + type: array + items: + description: OperatorCondition is just the standard condition fields. + type: object + properties: + lastTransitionTime: + type: string + format: date-time + message: + type: string + reason: + type: string + status: + type: string + type: + type: string + generations: + description: generations are used to determine when an item needs to + be reconciled or has changed in a way that needs a reaction. + type: array + items: + description: GenerationStatus keeps track of the generation for a + given resource so that decisions about forced updates can be made. + type: object + properties: + group: + description: group is the group of the thing you're tracking + type: string + hash: + description: hash is an optional field set for resources without + generation that are content sensitive like secrets and configmaps + type: string + lastGeneration: + description: lastGeneration is the last generation of the workload + controller involved + type: integer + format: int64 + name: + description: name is the name of the thing you're tracking + type: string + namespace: + description: namespace is where the thing you're tracking is + type: string + resource: + description: resource is the resource type of the thing you're + tracking + type: string + observedGeneration: + description: observedGeneration is the last generation change you've + dealt with + type: integer + format: int64 + readyReplicas: + description: readyReplicas indicates how many replicas are ready and + at the desired state + type: integer + format: int32 + version: + description: version is the level this availability applies to + type: string +`) + +func bootstrapCloudcredential_v1_operator_config_crdYamlBytes() ([]byte, error) { + return _bootstrapCloudcredential_v1_operator_config_crdYaml, nil +} + +func bootstrapCloudcredential_v1_operator_config_crdYaml() (*asset, error) { + bytes, err := bootstrapCloudcredential_v1_operator_config_crdYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "bootstrap/cloudcredential_v1_operator_config_crd.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -296,8 +469,9 @@ func AssetNames() []string { // _bindata is a table, holding each asset generator, mapped to its name. var _bindata = map[string]func() (*asset, error){ - "bootstrap/cloudcredential_v1_credentialsrequest.yaml": bootstrapCloudcredential_v1_credentialsrequestYaml, - "bootstrap/namespace.yaml": bootstrapNamespaceYaml, + "bootstrap/cloudcredential_v1_credentialsrequest_crd.yaml": bootstrapCloudcredential_v1_credentialsrequest_crdYaml, + "bootstrap/cloudcredential_v1_operator_config_crd.yaml": bootstrapCloudcredential_v1_operator_config_crdYaml, + "bootstrap/namespace.yaml": bootstrapNamespaceYaml, } // AssetDir returns the file names below a certain @@ -342,7 +516,8 @@ type bintree struct { var _bintree = &bintree{nil, map[string]*bintree{ "bootstrap": {nil, map[string]*bintree{ - "cloudcredential_v1_credentialsrequest.yaml": {bootstrapCloudcredential_v1_credentialsrequestYaml, map[string]*bintree{}}, + "cloudcredential_v1_credentialsrequest_crd.yaml": {bootstrapCloudcredential_v1_credentialsrequest_crdYaml, map[string]*bintree{}}, + "cloudcredential_v1_operator_config_crd.yaml": {bootstrapCloudcredential_v1_operator_config_crdYaml, map[string]*bintree{}}, "namespace.yaml": {bootstrapNamespaceYaml, map[string]*bintree{}}, }}, }} diff --git a/pkg/aws/actuator/actuator.go b/pkg/aws/actuator/actuator.go index 200765a3f..2cd2736ff 100644 --- a/pkg/aws/actuator/actuator.go +++ b/pkg/aws/actuator/actuator.go @@ -24,21 +24,13 @@ import ( log "github.com/sirupsen/logrus" - minterv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1" - ccaws "github.com/openshift/cloud-credential-operator/pkg/aws" - minteraws "github.com/openshift/cloud-credential-operator/pkg/aws" - "github.com/openshift/cloud-credential-operator/pkg/operator/constants" - actuatoriface "github.com/openshift/cloud-credential-operator/pkg/operator/credentialsrequest/actuator" - awsannotator "github.com/openshift/cloud-credential-operator/pkg/operator/secretannotator/aws" - "github.com/openshift/cloud-credential-operator/pkg/operator/utils" - awsutils "github.com/openshift/cloud-credential-operator/pkg/operator/utils/aws" - - configv1 "github.com/openshift/api/config/v1" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/iam" + configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,6 +38,15 @@ import ( "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" "sigs.k8s.io/controller-runtime/pkg/client" + + minterv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1" + ccaws "github.com/openshift/cloud-credential-operator/pkg/aws" + minteraws "github.com/openshift/cloud-credential-operator/pkg/aws" + "github.com/openshift/cloud-credential-operator/pkg/operator/constants" + actuatoriface "github.com/openshift/cloud-credential-operator/pkg/operator/credentialsrequest/actuator" + awsannotator "github.com/openshift/cloud-credential-operator/pkg/operator/secretannotator/aws" + "github.com/openshift/cloud-credential-operator/pkg/operator/utils" + awsutils "github.com/openshift/cloud-credential-operator/pkg/operator/utils/aws" ) const ( @@ -225,19 +226,31 @@ func (a *AWSActuator) needsUpdate(ctx context.Context, cr *minterv1.CredentialsR } else { // for passthrough creds, just see if we have the permissions requested in the credentialsrequest - region, err := awsutils.LoadInfrastructureRegion(a.Client, logger) + + // but for the case where the operator mode is non-default, then we will avoid performing any + // policy simulations and assume that the passthrough creds must be good enough + mode, _, err := utils.GetOperatorConfiguration(a.Client, logger) if err != nil { return true, err } - simParams := &ccaws.SimulateParams{ - Region: region, - } - goodEnough, err := ccaws.CheckPermissionsUsingQueryClient(readAWSClient, awsClient, awsSpec.StatementEntries, simParams, logger) - if err != nil { - return true, fmt.Errorf("error validating whether current creds are good enough: %v", err) - } - if !goodEnough { - return true, nil + if mode == operatorv1.CloudCredentialsModePassthrough { + logger.Debug("will not perform permissions simulation because operator in mode %q", mode) + } else { + region, err := awsutils.LoadInfrastructureRegion(a.Client, logger) + if err != nil { + return true, err + } + simParams := &ccaws.SimulateParams{ + Region: region, + } + + goodEnough, err := ccaws.CheckPermissionsUsingQueryClient(readAWSClient, awsClient, awsSpec.StatementEntries, simParams, logger) + if err != nil { + return true, fmt.Errorf("error validating whether current creds are good enough: %v", err) + } + if !goodEnough { + return true, nil + } } } diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go index 207480301..b34cc5583 100644 --- a/pkg/cmd/render/render.go +++ b/pkg/cmd/render/render.go @@ -17,17 +17,22 @@ limitations under the License. package render import ( + "bytes" "fmt" "io/ioutil" "os" "path/filepath" + "text/template" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" yaml "k8s.io/apimachinery/pkg/util/yaml" + operatorv1 "github.com/openshift/api/operator/v1" + minterv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1" assets "github.com/openshift/cloud-credential-operator/pkg/assets/bootstrap" "github.com/openshift/cloud-credential-operator/pkg/operator/constants" @@ -35,6 +40,8 @@ import ( ) const ( + podYamlFilename = "cloud-credential-operator-pod.yaml" + podTemplate = `apiVersion: v1 kind: Pod metadata: @@ -66,11 +73,25 @@ const ( manifestsDir = "manifests" bootstrapManifestsDir = "bootstrap-manifests" defaultLogLevel = "info" + + installConfigNamespace = "kube-system" + installConfigName = "cluster-config-v1" + installConfigKeyName = "install-config" + + operatorConfigFilename = "cco-operator-config.yaml" ) var ( + operatorConfigTemplate = template.Must(template.New("operatorConfig").Parse(`apiVersion: operator.openshift.io/v1 +kind: CloudCredential +metadata: + name: cluster +spec: + credentialsMode: "{{ .CredentialsMode }}"`)) + renderAssets = []string{ - "bootstrap/cloudcredential_v1_credentialsrequest.yaml", + "bootstrap/cloudcredential_v1_operator_config_crd.yaml", + "bootstrap/cloudcredential_v1_credentialsrequest_crd.yaml", "bootstrap/namespace.yaml", } @@ -89,6 +110,10 @@ var ( } ) +type operatorTemplateVars struct { + CredentialsMode operatorv1.CloudCredentialsMode +} + func NewRenderCommand() *cobra.Command { renderCmd.Flags().StringVar(&renderOpts.manifestsDir, "manifests-dir", "", "The directory where the install-time manifests are located.") renderCmd.Flags().StringVar(&renderOpts.destinationDir, "dest-dir", "", "The destination directory where CCO writes the manifests.") @@ -109,54 +134,110 @@ func runRenderCmd(cmd *cobra.Command, args []string) { log.SetLevel(level) log.Debug("debug logging enabled") - operatorDisabled := isDisabled() + err = render() + if err != nil { + log.WithError(err).Fatal("failed while rendering bootstrap assets") + } +} + +func render() error { + operatorDisabledViaConfigmap := isDisabledViaConfigmap() + + installConfigMode, err := getModeFromInstallConfig() + if err != nil { + return err + } + if !isValidMode(installConfigMode) { + return fmt.Errorf("invalid mode defined: %s", installConfigMode) + } + + effectiveMode, conflict := utils.GetEffectiveOperatorMode(operatorDisabledViaConfigmap, installConfigMode) + + if conflict { + return fmt.Errorf("config map asking for CCO to be disabled, and install-config asking for %q mode", installConfigMode) + } log.Infof("Rendering files to %s", renderOpts.destinationDir) ccoRenderDir := renderOpts.destinationDir - // render manifests + // render manifests from bindata if err := os.MkdirAll(filepath.Join(ccoRenderDir, manifestsDir), 0775); err != nil { - log.WithError(err).Fatal("error creating manifests directory") + return errors.Wrap(err, "error creating manifests directory") } for _, assetName := range renderAssets { asset, err := assets.Asset(assetName) if err != nil { - log.WithError(err).Fatal("failed to read static asset") + return errors.Wrap(err, "failed to read static asset") } - assetRenderPath := filepath.Join(ccoRenderDir, "manifests", "cco-"+filepath.Base(assetName)) - log.Infof("Writing file: %s", assetRenderPath) - err = ioutil.WriteFile(assetRenderPath, asset, 0644) - if err != nil { - log.WithError(err).Fatal("failed to write file") + assetRenderPath := filepath.Join(ccoRenderDir, manifestsDir, "cco-"+filepath.Base(assetName)) + if err := writeFile(assetRenderPath, asset); err != nil { + return err } } + // render operator config + var operatorConfig bytes.Buffer + templateVars := operatorTemplateVars{ + CredentialsMode: effectiveMode, + } + + if err := operatorConfigTemplate.Execute(&operatorConfig, templateVars); err != nil { + return errors.Wrap(err, "failed to execute operator config template") + } + assetRenderPath := filepath.Join(ccoRenderDir, manifestsDir, operatorConfigFilename) + if err := writeFile(assetRenderPath, operatorConfig.Bytes()); err != nil { + return err + } + // need at least the empty dir so the installer bootkube.sh script works as expected if err := os.Mkdir(filepath.Join(ccoRenderDir, bootstrapManifestsDir), 0775); err != nil { - log.WithError(err).Fatal("error creating bootstrap-manifests directory") + errors.Wrap(err, "error creating bootstrap-manifests directory") } - if !operatorDisabled { + if effectiveMode != operatorv1.CloudCredentialsModeManual { log.Info("Rendering static pod") - podPath := filepath.Join(ccoRenderDir, bootstrapManifestsDir, "cloud-credential-operator-pod.yaml") + podPath := filepath.Join(ccoRenderDir, bootstrapManifestsDir, podYamlFilename) podContent := fmt.Sprintf(podTemplate, renderOpts.ccoImage) log.Infof("writing file: %s", podPath) - err = ioutil.WriteFile(podPath, []byte(podContent), 0644) + err := ioutil.WriteFile(podPath, []byte(podContent), 0644) if err != nil { - log.WithError(err).Fatal("failed to write file") + return errors.Wrap(err, "failed to write file") } } else { log.Info("CCO disabled, skipping static pod manifest.") } + + return nil +} + +func writeFile(filePath string, fileData []byte) error { + log.Infof("Writing file: %s", filePath) + err := ioutil.WriteFile(filePath, fileData, 0644) + if err != nil { + return errors.Wrap(err, "failed to write file") + } + return nil +} + +func isValidMode(mode operatorv1.CloudCredentialsMode) bool { + switch mode { + case operatorv1.CloudCredentialsModeDefault, + operatorv1.CloudCredentialsModeManual, + operatorv1.CloudCredentialsModeMint, + operatorv1.CloudCredentialsModePassthrough: + return true + default: + return false + } } // isDisabled will search through all the files in destinationDir (which also contains -// the source manifests) for a configmap indicating whether or not CCO is disabled. In +// the source manifests) for the deprecated configmap indicating whether or not CCO is disabled. In // the absence of any configmap, it will return the default disabled setting (which is // that the operator is enabled by default) for the operator. -func isDisabled() bool { +func isDisabledViaConfigmap() bool { // if were were not provided a place to search for the install-time manifests, // just return the default CCO operator enabled/disabled value @@ -164,25 +245,80 @@ func isDisabled() bool { return utils.OperatorDisabledDefault } - files, err := ioutil.ReadDir(renderOpts.manifestsDir) + cm, err := getConfigMap(renderOpts.manifestsDir, minterv1.CloudCredOperatorNamespace, constants.CloudCredOperatorConfigMap) + if err != nil { + log.WithError(err).Warnf("errored while searching for ConfigMap %s/%s, using defaults", minterv1.CloudCredOperatorNamespace, constants.CloudCredOperatorConfigMap) + return utils.OperatorDisabledDefault + } + if cm == nil { + log.Debugf("configmap %s/%s not found, using default mode", minterv1.CloudCredOperatorNamespace, constants.CloudCredOperatorConfigMap) + return utils.OperatorDisabledDefault + } + + disabled, err := utils.CCODisabledCheck(cm, log.WithFields(nil)) if err != nil { - log.WithError(err).Errorf("failed to list files in %s, using defualt operator settings", renderOpts.destinationDir) return utils.OperatorDisabledDefault } + return disabled +} + +type basicInstallConfig struct { + CredentialsMode operatorv1.CloudCredentialsMode `json:"credentialsMode"` +} + +func getModeFromInstallConfig() (operatorv1.CloudCredentialsMode, error) { + + // if we were not provided a place to search for the install-time manifests, + // just return the default cloudCredentialsMode (empty string) + if renderOpts.manifestsDir == "" { + return "", nil + } + + cm, err := getConfigMap(renderOpts.manifestsDir, installConfigNamespace, installConfigName) + if err != nil { + return "", errors.Wrapf(err, "failed to find configmap %s/%s in manifests", installConfigNamespace, installConfigName) + } + if cm == nil { + return "", fmt.Errorf("failed to find configmap %s/%s in manifests", installConfigNamespace, installConfigName) + } + + data, ok := cm.Data[installConfigKeyName] + if !ok { + return "", fmt.Errorf("did not find key %s in configmap %s/%s", installConfigKeyName, installConfigNamespace, installConfigName) + } + + decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader([]byte(data)), 4096) + instConf := &basicInstallConfig{} + if err := decoder.Decode(instConf); err != nil { + return "", errors.Wrap(err, "failed to decode install config") + } + log.Debugf("install-config contains CredentialsMode: %s", instConf.CredentialsMode) + return instConf.CredentialsMode, nil +} + +func getConfigMap(manifestsDir, namespace, name string) (*corev1.ConfigMap, error) { + + files, err := ioutil.ReadDir(manifestsDir) + if err != nil { + log.WithError(err).Errorf("failed to list files in %s", manifestsDir) + return nil, err + } + for _, fInfo := range files { - // non-recursive checking of all files where the the source manifests are located + // non-recursive checking of all files where the source manifests are located if fInfo.IsDir() { continue } - fullPath := filepath.Join(renderOpts.manifestsDir, fInfo.Name()) + fullPath := filepath.Join(manifestsDir, fInfo.Name()) log.Debugf("checking file: %s", fullPath) file, err := os.Open(fullPath) if err != nil { log.WithError(err).Warn("failed to open file while searching for configmap") continue } + decoder := yaml.NewYAMLOrJSONDecoder(file, 4096) configMap := &corev1.ConfigMap{} if err := decoder.Decode(configMap); err != nil { @@ -190,17 +326,14 @@ func isDisabled() bool { continue } - if configMap.Namespace == minterv1.CloudCredOperatorNamespace && configMap.Name == constants.CloudCredOperatorConfigMap { - logger := log.New() - logger.SetLevel(log.GetLevel()) - disabled, err := utils.CCODisabledCheck(configMap, logger) - if err != nil { - return utils.OperatorDisabledDefault - } - - return disabled + if configMap.APIVersion == corev1.SchemeGroupVersion.Identifier() && + configMap.Kind == "ConfigMap" && + configMap.Namespace == namespace && + configMap.Name == name { + return configMap, nil } } - return utils.OperatorDisabledDefault + // if we made it this far, then the file was not found + return nil, nil } diff --git a/pkg/cmd/render/render_test.go b/pkg/cmd/render/render_test.go new file mode 100644 index 000000000..497487240 --- /dev/null +++ b/pkg/cmd/render/render_test.go @@ -0,0 +1,184 @@ +package render + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/util/yaml" +) + +const ( + testDestinationDirName = "renderHere" +) + +type testFile struct { + name string + data string +} + +func TestRender(t *testing.T) { + tests := []struct { + name string + existingFiles []*testFile + expectPodFile bool + expectMode string + expectError bool + }{ + { + name: "install config default", + existingFiles: []*testFile{ + testInstallConfig(""), + }, + expectPodFile: true, + // default is no setting in the config + expectMode: "", + }, + { + name: "install config manual mode", + existingFiles: []*testFile{ + testInstallConfig(string(operatorv1.CloudCredentialsModeManual)), + }, + expectPodFile: false, + expectMode: string(operatorv1.CloudCredentialsModeManual), + }, + { + name: "deprecated configmap disables cco", + existingFiles: []*testFile{ + testInstallConfig(""), + testConfigMap("true"), + }, + expectPodFile: false, + expectMode: string(operatorv1.CloudCredentialsModeManual), + }, + { + name: "install config mint mode", + existingFiles: []*testFile{ + testInstallConfig(string(operatorv1.CloudCredentialsModeMint)), + }, + expectPodFile: true, + expectMode: string(operatorv1.CloudCredentialsModeMint), + }, + { + name: "configmap and installconfig conflict", + existingFiles: []*testFile{ + testInstallConfig(string(operatorv1.CloudCredentialsModeMint)), + testConfigMap("true"), + }, + expectError: true, + }, + { + name: "configmap and installconfig concur", + existingFiles: []*testFile{ + testInstallConfig(string(operatorv1.CloudCredentialsModeManual)), + testConfigMap("true"), + }, + expectPodFile: false, + expectMode: string(operatorv1.CloudCredentialsModeManual), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + manifestsDir, err := ioutil.TempDir("/tmp", "rendertestmanifests") + require.NoError(t, err, "errored setting up test") + defer os.RemoveAll(manifestsDir) + + for _, file := range test.existingFiles { + filePath := filepath.Join(manifestsDir, file.name) + err := ioutil.WriteFile(filePath, []byte(file.data), 0644) + require.NoError(t, err, "failed writing out manifests for test") + } + destDir, err := ioutil.TempDir("/tmp", "rendertestdestination") + require.NoError(t, err, "errored setting up test") + defer os.RemoveAll(destDir) + + destDirRenderPath := filepath.Join(destDir, testDestinationDirName) + + renderOpts.manifestsDir = manifestsDir + renderOpts.destinationDir = destDirRenderPath + renderOpts.ccoImage = "testCCOImage" + renderOpts.logLevel = "debug" + + err = render() + + if test.expectError { + require.Error(t, err, "expected error for test case") + } else { + require.NoError(t, err, "unexpected error") + + verifyPodFile(t, destDirRenderPath, test.expectPodFile) + + verifyConfigMode(t, destDirRenderPath, test.expectMode) + } + }) + } +} + +func testInstallConfig(credMode string) *testFile { + instConfFile := testFile{ + name: "install-config.yaml", + } + + installConfigData := `apiVersion: v1 +kind: ConfigMap +metadata: + name: cluster-config-v1 + namespace: kube-system +data: + install-config: | + baseDomain: test.openshift.io + credentialsMode: %s` + + instConfFile.data = fmt.Sprintf(installConfigData, credMode) + + return &instConfFile +} + +func verifyPodFile(t *testing.T, destDir string, expectFileToExist bool) { + podFilePath := filepath.Join(destDir, bootstrapManifestsDir, podYamlFilename) + _, err := os.Stat(podFilePath) + + if expectFileToExist { + assert.NoError(t, err, "expected pod yaml to be rendered") + } else { + assert.True(t, os.IsNotExist(err), "expect pod yaml to not be rendered") + } +} + +func verifyConfigMode(t *testing.T, destDir, expectMode string) { + configFilePath := filepath.Join(destDir, manifestsDir, operatorConfigFilename) + + file, err := os.Open(configFilePath) + require.NoError(t, err, "error reading in rendered config file") + + conf := operatorv1.CloudCredential{} + decoder := yaml.NewYAMLOrJSONDecoder(file, 4096) + err = decoder.Decode(&conf) + require.NoError(t, err, "error decoding rendered config") + + assert.Equal(t, expectMode, string(conf.Spec.CredentialsMode), "config file has unexpected mode set") +} + +func testConfigMap(disabled string) *testFile { + configMapFile := testFile{ + name: "configmap.yaml", + } + + data := `apiVersion: v1 +kind: ConfigMap +metadata: + name: cloud-credential-operator-config + namespace: openshift-cloud-credential-operator +data: + disabled: "%s"` + + configMapFile.data = fmt.Sprintf(data, disabled) + + return &configMapFile +} diff --git a/pkg/operator/credentialsrequest/credentialsrequest_controller_gcp_test.go b/pkg/operator/credentialsrequest/credentialsrequest_controller_gcp_test.go index 40917b368..a93088d8f 100644 --- a/pkg/operator/credentialsrequest/credentialsrequest_controller_gcp_test.go +++ b/pkg/operator/credentialsrequest/credentialsrequest_controller_gcp_test.go @@ -109,6 +109,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "new credential", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testGCPCredsSecret("kube-system", constants.GCPCloudCredSecretName, testRootGCPAuth), @@ -164,6 +165,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "new credential cluster has no infra name", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testGCPCredentialsRequest(t), @@ -219,6 +221,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "new credential no root creds available", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testGCPCredentialsRequest(t), @@ -250,6 +253,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "cred missing access key exists", // expect old key(s) deleted, new key created/saved existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testGCPCredentialsRequest(t), @@ -290,6 +294,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "cred exists access key missing", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testGCPCredentialsRequest(t), @@ -330,6 +335,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "cred deletion", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testGCPCredentialsRequestWithDeletionTimestamp(t), @@ -356,6 +362,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "failed to mint condition", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testGCPCredentialsRequest(t), testGCPCredsSecret("kube-system", constants.GCPCloudCredSecretName, testRootGCPAuth), @@ -389,6 +396,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "cred deletion failure condition", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testGCPCredentialsRequestWithDeletionTimestamp(t), testGCPCredsSecret("kube-system", constants.GCPCloudCredSecretName, testRootGCPAuth), @@ -414,6 +422,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "new cred passthrough", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testGCPCredentialsRequest(t), testGCPCredsSecretPassthrough("kube-system", constants.GCPCloudCredSecretName, testRootGCPAuth), @@ -447,6 +456,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "new cred passthrough fail permissions", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testGCPCredentialsRequest(t), testGCPCredsSecretPassthrough("kube-system", constants.GCPCloudCredSecretName, testRootGCPAuth), @@ -483,6 +493,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "existing cr up to date", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testGCPPassthroughCredentialsRequest(t), testGCPCredsSecretPassthrough("kube-system", constants.GCPCloudCredSecretName, testRootGCPAuth), @@ -515,6 +526,7 @@ func TestCredentialsRequestGCPReconcile(t *testing.T) { { name: "existing secret has old secret content", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testGCPPassthroughCredentialsRequest(t), testGCPCredsSecretPassthrough("kube-system", constants.GCPCloudCredSecretName, testRootGCPAuth), diff --git a/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go b/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go index 85ab1e3f9..532a734df 100644 --- a/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go +++ b/pkg/operator/credentialsrequest/credentialsrequest_controller_test.go @@ -28,6 +28,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/iam" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -49,10 +53,6 @@ import ( "github.com/openshift/cloud-credential-operator/pkg/operator/utils" schemeutils "github.com/openshift/cloud-credential-operator/pkg/util" "github.com/openshift/cloud-credential-operator/pkg/util/clusteroperator" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/service/iam" ) var c client.Client @@ -131,6 +131,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "add finalizer", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), func() *minterv1.CredentialsRequest { @@ -157,6 +158,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "new credential", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -212,6 +214,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "new credential cluster has no infra name", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -268,6 +271,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { // This tests the case where we create our own read only creds initially: name: "new credential no read-only creds available", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -317,6 +321,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { // This indicates an error state. name: "new credential no root creds available", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -359,6 +364,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "cred and secret exist user tagged", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -394,6 +400,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "cred and secret exist user missing tag", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -434,6 +441,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "cred and secret exist no root creds", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -482,6 +490,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "cred missing access key exists", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -519,6 +528,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "cred exists access key missing", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -557,6 +567,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "cred deletion", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequestWithDeletionTimestamp(t), @@ -596,6 +607,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "new passthrough credential", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), testInfrastructure(testInfraName), createTestNamespace(testSecretNamespace), @@ -623,9 +635,86 @@ func TestCredentialsRequestReconcile(t *testing.T) { assert.True(t, cr.Status.Provisioned) }, }, + { + name: "existing passthrough credential", + existing: []runtime.Object{ + testOperatorConfig(""), + createTestNamespace(testNamespace), + testInfrastructure(testInfraName), + createTestNamespace(testSecretNamespace), + testPassthroughCredentialsRequest(t), + testAWSCredsSecret("openshift-cloud-credential-operator", "cloud-credential-operator-iam-ro-creds", testReadAWSAccessKeyID, testReadAWSSecretAccessKey), + testAWSCredsSecret(testSecretNamespace, testSecretName, testAWSAccessKeyID, testAWSSecretAccessKey), + }, + mockRootAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { + mockAWSClient := mockaws.NewMockClient(mockCtrl) + return mockAWSClient + }, + mockReadAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { + mockAWSClient := mockaws.NewMockClient(mockCtrl) + mockGetUser(mockAWSClient) + // will simulate to check that the creds in the target secret actually satisfy the requested perms + mockSimulatePrincipalPolicyPagesSuccess(mockAWSClient) + return mockAWSClient + }, + mockSecretAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { + mockAWSClient := mockaws.NewMockClient(mockCtrl) + mockGetUser(mockAWSClient) + return mockAWSClient + }, + validate: func(c client.Client, t *testing.T) { + targetSecret := getSecret(c) + if assert.NotNil(t, targetSecret) { + assert.Equal(t, testAWSAccessKeyID, + string(targetSecret.Data["aws_access_key_id"])) + assert.Equal(t, testAWSSecretAccessKey, + string(targetSecret.Data["aws_secret_access_key"])) + } + cr := getCR(c) + assert.True(t, cr.Status.Provisioned) + }, + }, + { + name: "existing passthrough credential bypassing simulations", + existing: []runtime.Object{ + testOperatorConfig(operatorv1.CloudCredentialsModePassthrough), // indicates that CCO should not perform permissions simulations + createTestNamespace(testNamespace), + testInfrastructure(testInfraName), + createTestNamespace(testSecretNamespace), + testPassthroughCredentialsRequest(t), + testAWSCredsSecret("openshift-cloud-credential-operator", "cloud-credential-operator-iam-ro-creds", testReadAWSAccessKeyID, testReadAWSSecretAccessKey), + testAWSCredsSecret(testSecretNamespace, testSecretName, testAWSAccessKeyID, testAWSSecretAccessKey), + }, + mockRootAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { + mockAWSClient := mockaws.NewMockClient(mockCtrl) + return mockAWSClient + }, + mockReadAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { + mockAWSClient := mockaws.NewMockClient(mockCtrl) + mockGetUser(mockAWSClient) + return mockAWSClient + }, + mockSecretAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { + mockAWSClient := mockaws.NewMockClient(mockCtrl) + mockGetUser(mockAWSClient) + return mockAWSClient + }, + validate: func(c client.Client, t *testing.T) { + targetSecret := getSecret(c) + if assert.NotNil(t, targetSecret) { + assert.Equal(t, testAWSAccessKeyID, + string(targetSecret.Data["aws_access_key_id"])) + assert.Equal(t, testAWSSecretAccessKey, + string(targetSecret.Data["aws_secret_access_key"])) + } + cr := getCR(c) + assert.True(t, cr.Status.Provisioned) + }, + }, { name: "passthrough cred deletion", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), testInfrastructure(testInfraName), createTestNamespace(testSecretNamespace), @@ -642,6 +731,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "no namespace condition", existing: []runtime.Object{ + testOperatorConfig(""), testInfrastructure(testInfraName), testCredentialsRequest(t), }, @@ -660,6 +750,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "insufficient creds", existing: []runtime.Object{ + testOperatorConfig(""), testInfrastructure(testInfraName), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -681,6 +772,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "failed to mint condition", existing: []runtime.Object{ + testOperatorConfig(""), testInfrastructure(testInfraName), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -709,6 +801,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "cred deletion failure condition", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testInfrastructure(testInfraName), testCredentialsRequestWithDeletionTimestamp(t), @@ -737,6 +830,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "skip AWS if recently synced", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), testInfrastructure(testInfraName), createTestNamespace(testSecretNamespace), @@ -755,6 +849,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "regenerate secret if missing", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), testInfrastructure(testInfraName), testClusterVersion(), @@ -786,6 +881,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { name: "recently synced but modified", existing: func() []runtime.Object { objects := []runtime.Object{} + objects = append(objects, testOperatorConfig("")) objects = append(objects, createTestNamespace(testNamespace)) objects = append(objects, createTestNamespace(testSecretNamespace)) @@ -820,6 +916,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "skip nonAWS credreq", existing: []runtime.Object{ + testOperatorConfig(""), testGCPCredentialsRequest(t), }, mockRootAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { @@ -840,6 +937,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "clear conditions when ignoring cred request", existing: []runtime.Object{ + testOperatorConfig(""), func() runtime.Object { cr := testGCPCredentialsRequest(t) for _, cond := range minterv1.FailureConditionTypes { @@ -869,6 +967,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "pass along any existing permissions boundary", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testCredentialsRequest(t), @@ -911,6 +1010,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "new credential but operator disabled via configmap", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testOperatorConfigMap("true"), @@ -1011,6 +1111,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "recently synced but with error condition set", existing: []runtime.Object{ + testOperatorConfig(""), func() *minterv1.CredentialsRequest { cr := testCredentialsRequestWithRecentLastSync(t) cr.Status.Conditions = utils.SetCredentialsRequestCondition(cr.Status.Conditions, @@ -1043,6 +1144,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "recently synced but not provisioned", existing: []runtime.Object{ + testOperatorConfig(""), func() *minterv1.CredentialsRequest { cr := testCredentialsRequestWithRecentLastSync(t) cr.Status.Provisioned = false @@ -1070,6 +1172,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "provision with aws policy condition", existing: []runtime.Object{ + testOperatorConfig(""), func() *minterv1.CredentialsRequest { cr := testCredentialsRequest(t) awsProvSpec, err := codec.EncodeProviderSpec( @@ -1135,6 +1238,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { { name: "already provisioned with aws policy condition", existing: []runtime.Object{ + testOperatorConfig(""), func() *minterv1.CredentialsRequest { cr := testCredentialsRequest(t) awsProvSpec, err := codec.EncodeProviderSpec( @@ -1210,6 +1314,17 @@ func TestCredentialsRequestReconcile(t *testing.T) { assert.NotNil(t, cr.Status.LastSyncTimestamp) }, }, + { + name: "error reconcile without operator config", + existing: []runtime.Object{ + testCredentialsRequest(t), + }, + mockRootAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { + mockAWSClient := mockaws.NewMockClient(mockCtrl) + return mockAWSClient + }, + expectErr: true, + }, } for _, test := range tests { @@ -1263,7 +1378,7 @@ func TestCredentialsRequestReconcile(t *testing.T) { } if err != nil && !test.expectErr { - t.Errorf("Unexpected error: %v", err) + require.NoError(t, err, "Unexpected error: %v", err) } if err == nil && test.expectErr { t.Errorf("Expected error but got none") @@ -1272,14 +1387,14 @@ func TestCredentialsRequestReconcile(t *testing.T) { cr := getCR(fakeClient) for _, condition := range test.expectedConditions { foundCondition := utils.FindCredentialsRequestCondition(cr.Status.Conditions, condition.conditionType) - assert.NotNil(t, foundCondition) + require.NotNil(t, foundCondition, "unexpected unable to find condition") assert.Exactly(t, condition.status, foundCondition.Status) assert.Exactly(t, condition.reason, foundCondition.Reason) } for _, condition := range test.expectedCOConditions { co := getClusterOperator(fakeClient) - assert.NotNil(t, co) + require.NotNil(t, co) foundCondition := findClusterOperatorCondition(co.Status.Conditions, condition.conditionType) assert.NotNil(t, foundCondition) assert.Equal(t, string(condition.status), string(foundCondition.Status), "condition %s had unexpected status", condition.conditionType) @@ -1301,7 +1416,7 @@ const ( testSecretName = "test-secret" testSecretNamespace = "myproject" testAWSUser = "mycluster-test-aws-user" - testAWSARN = "some:fake:ARN:1234" + testAWSARN = "arn:aws:iam::1234:user/testuser" testAWSUserID = "FAKEAWSUSERID" testAWSAccessKeyID = "FAKEAWSACCESSKEYID" testAWSAccessKeyID2 = "FAKEAWSACCESSKEYID2" @@ -1662,6 +1777,10 @@ func testClusterVersion() *configv1.ClusterVersion { } } +func mockSimulatePrincipalPolicyPagesSuccess(mockAWSClient *mockaws.MockClient) { + mockAWSClient.EXPECT().SimulatePrincipalPolicyPages(gomock.Any(), gomock.Any()).Return(nil) +} + func mockSimulatePrincipalPolicySuccess(mockAWSClient *mockaws.MockClient) { mockAWSClient.EXPECT().SimulatePrincipalPolicy(gomock.Any()).Return(&iam.SimulatePolicyResponse{ EvaluationResults: []*iam.EvaluationResult{ diff --git a/pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go b/pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go index 580bec249..f0510e588 100644 --- a/pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go +++ b/pkg/operator/credentialsrequest/credentialsrequest_controller_vsphere_test.go @@ -78,6 +78,7 @@ func TestCredentialsRequestVSphereReconcile(t *testing.T) { { name: "new credentialsrequest passthrough", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testVSphereCredsSecretPassthrough(), @@ -113,6 +114,7 @@ func TestCredentialsRequestVSphereReconcile(t *testing.T) { { name: "new credential no root creds available", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testVSphereCredentialsRequest(t), @@ -138,6 +140,7 @@ func TestCredentialsRequestVSphereReconcile(t *testing.T) { { name: "cred deletion", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testNamespace), createTestNamespace(testSecretNamespace), testVSphereCredentialsRequestWithDeletionTimestamp(t), @@ -152,6 +155,7 @@ func TestCredentialsRequestVSphereReconcile(t *testing.T) { { name: "existing cr up to date", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testVSphereCredentialsRequest(t), testVSphereCredsSecretPassthrough(), @@ -172,6 +176,7 @@ func TestCredentialsRequestVSphereReconcile(t *testing.T) { { name: "existing secret has old secret content", existing: []runtime.Object{ + testOperatorConfig(""), createTestNamespace(testSecretNamespace), testVSphereCredentialsRequest(t), testVSphereCredsSecretPassthrough(), diff --git a/pkg/operator/credentialsrequest/status_test.go b/pkg/operator/credentialsrequest/status_test.go index 32c896bef..d61f9bf9d 100644 --- a/pkg/operator/credentialsrequest/status_test.go +++ b/pkg/operator/credentialsrequest/status_test.go @@ -264,7 +264,8 @@ func TestClusterOperatorVersion(t *testing.T) { t.Run(test.name, func(t *testing.T) { existingCO := testClusterOperator("4.0.0-5", twentyHoursAgo) - existing := []runtime.Object{existingCO} + operatorConfig := testOperatorConfig("") + existing := []runtime.Object{existingCO, operatorConfig} fakeClient := fake.NewFakeClient(existing...) rcr := &ReconcileCredentialsRequest{ diff --git a/pkg/operator/secretannotator/aws/reconciler_test.go b/pkg/operator/secretannotator/aws/reconciler_test.go index 2fc0528bc..fb5490ded 100644 --- a/pkg/operator/secretannotator/aws/reconciler_test.go +++ b/pkg/operator/secretannotator/aws/reconciler_test.go @@ -94,8 +94,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue string }{ { - name: "cred minter mode", - existing: []runtime.Object{testSecret()}, + name: "cred minter mode", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -106,8 +109,12 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue: constants.MintAnnotation, }, { - name: "operator disabled via configmap", - existing: []runtime.Object{testSecret(), testOperatorConfigMap("true")}, + name: "operator disabled via configmap", + existing: []runtime.Object{ + testSecret(), + testOperatorConfigMap("true"), + testOperatorConfig(""), + }, }, { name: "operator disabled", @@ -117,8 +124,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { }, }, { - name: "detect root user creds", - existing: []runtime.Object{testSecret()}, + name: "detect root user creds", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetRootUser(mockAWSClient) @@ -128,8 +138,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { expectErr: true, }, { - name: "cred passthrough mode", - existing: []runtime.Object{testSecret()}, + name: "cred passthrough mode", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -143,8 +156,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue: constants.PassthroughAnnotation, }, { - name: "cred passthrough mode wrong region permission", - existing: []runtime.Object{testSecret()}, + name: "cred passthrough mode wrong region permission", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -158,8 +174,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue: constants.InsufficientAnnotation, }, { - name: "useless creds", - existing: []runtime.Object{testSecret()}, + name: "useless creds", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockAWSClient: func(mockCtrl *gomock.Controller) *mockaws.MockClient { mockAWSClient := mockaws.NewMockClient(mockCtrl) mockGetUser(mockAWSClient) @@ -173,22 +192,28 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue: constants.InsufficientAnnotation, }, { - name: "missing secret", + name: "missing secret", + existing: []runtime.Object{ + testOperatorConfig(""), + }, expectErr: true, }, { name: "secret missing key", expectErr: true, - existing: []runtime.Object{&corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: testSecretName, - Namespace: testNamespace, - }, - Data: map[string][]byte{ - annaws.AwsAccessKeyName: []byte(testAWSAccessKeyID), - "not_aws_secret_access_key": []byte(testAWSSecretAccessKey), + existing: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testSecretName, + Namespace: testNamespace, + }, + Data: map[string][]byte{ + annaws.AwsAccessKeyName: []byte(testAWSAccessKeyID), + "not_aws_secret_access_key": []byte(testAWSSecretAccessKey), + }, }, - }}, + testOperatorConfig(""), + }, }, { name: "annotation matches forced mode", @@ -206,6 +231,13 @@ func TestSecretAnnotatorReconcile(t *testing.T) { }, expectErr: true, }, + { + name: "error on missing config CR", + existing: []runtime.Object{ + testSecret(), + }, + expectErr: true, + }, } for _, test := range tests { diff --git a/pkg/operator/secretannotator/azure/reconciler_test.go b/pkg/operator/secretannotator/azure/reconciler_test.go index a334483f9..a7c77ac13 100644 --- a/pkg/operator/secretannotator/azure/reconciler_test.go +++ b/pkg/operator/secretannotator/azure/reconciler_test.go @@ -18,6 +18,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -83,8 +84,9 @@ func TestAzureSecretAnnotatorReconcile(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { base := getInputSecret() + operatorConfig := testOperatorConfig("") infra := &configv1.Infrastructure{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} - fakeClient := fake.NewFakeClient(base, infra) + fakeClient := fake.NewFakeClient(base, operatorConfig, infra) mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() mockAdalClient := mock.NewMockAdalService(mockCtrl) @@ -142,6 +144,19 @@ func getInputSecret() *corev1.Secret { } +func testOperatorConfig(mode operatorv1.CloudCredentialsMode) *operatorv1.CloudCredential { + conf := &operatorv1.CloudCredential{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.CloudCredOperatorConfig, + }, + Spec: operatorv1.CloudCredentialSpec{ + CredentialsMode: mode, + }, + } + + return conf +} + func setupAdalMock(r *mock.MockAdalServiceMockRecorder, roles []string) { gomock.InOrder( // these methdods are extensivly tested in azure codebase diff --git a/pkg/operator/secretannotator/gcp/reconciler_test.go b/pkg/operator/secretannotator/gcp/reconciler_test.go index 08f249a8d..96b1096bc 100644 --- a/pkg/operator/secretannotator/gcp/reconciler_test.go +++ b/pkg/operator/secretannotator/gcp/reconciler_test.go @@ -85,8 +85,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue string }{ { - name: "cred minter mode", - existing: []runtime.Object{testSecret()}, + name: "cred minter mode", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) mockGetProjectName(mockGCPClient) @@ -98,8 +101,12 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue: constants.MintAnnotation, }, { - name: "operator disabled via configmap", - existing: []runtime.Object{testSecret(), testOperatorConfigMap("true")}, + name: "operator disabled via configmap", + existing: []runtime.Object{ + testSecret(), + testOperatorConfigMap("true"), + testOperatorConfig(""), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) return mockGCPClient @@ -113,8 +120,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { }, }, { - name: "cred passthrough mode", - existing: []runtime.Object{testSecret()}, + name: "cred passthrough mode", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) mockGetProjectName(mockGCPClient) @@ -128,8 +138,11 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue: constants.PassthroughAnnotation, }, { - name: "useless creds", - existing: []runtime.Object{testSecret()}, + name: "useless creds", + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, mockGCPClient: func(mockCtrl *gomock.Controller) *mockgcp.MockClient { mockGCPClient := mockgcp.NewMockClient(mockCtrl) mockGetProjectName(mockGCPClient) @@ -142,7 +155,10 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue: constants.InsufficientAnnotation, }, { - name: "missing secret", + name: "missing secret", + existing: []runtime.Object{ + testOperatorConfig(""), + }, expectErr: true, }, { @@ -157,6 +173,7 @@ func TestSecretAnnotatorReconcile(t *testing.T) { "not-the-right-key": []byte(testGCPAuthJSON), }, }, + testOperatorConfig(""), }, validateAnnotationValue: constants.InsufficientAnnotation, }, @@ -272,6 +289,19 @@ func testOperatorConfigMap(disabled string) *corev1.ConfigMap { } } +func testOperatorConfig(mode operatorv1.CloudCredentialsMode) *operatorv1.CloudCredential { + conf := &operatorv1.CloudCredential{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.CloudCredOperatorConfig, + }, + Spec: operatorv1.CloudCredentialSpec{ + CredentialsMode: mode, + }, + } + + return conf +} + func mockGetProjectName(mockGCPClient *mockgcp.MockClient) { mockGCPClient.EXPECT().GetProjectName().AnyTimes().Return("test-GCP-project") } @@ -306,16 +336,3 @@ func mockListMintServicesEnabledSuccess(mockGCPClient *mockgcp.MockClient) { func mockListPassthroughServicesEnabledSuccess(mockGCPClient *mockgcp.MockClient) { mockGCPClient.EXPECT().ListServicesEnabled().Return(passthroughServicesEnabledMap, nil) } - -func testOperatorConfig(mode operatorv1.CloudCredentialsMode) *operatorv1.CloudCredential { - conf := &operatorv1.CloudCredential{ - ObjectMeta: metav1.ObjectMeta{ - Name: constants.CloudCredOperatorConfig, - }, - Spec: operatorv1.CloudCredentialSpec{ - CredentialsMode: mode, - }, - } - - return conf -} diff --git a/pkg/operator/secretannotator/vsphere/reconciler_test.go b/pkg/operator/secretannotator/vsphere/reconciler_test.go index bad7442c2..79a41764d 100644 --- a/pkg/operator/secretannotator/vsphere/reconciler_test.go +++ b/pkg/operator/secretannotator/vsphere/reconciler_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -61,17 +62,28 @@ func TestSecretAnnotatorReconcile(t *testing.T) { validateAnnotationValue string }{ { - name: "operator disabled", - existing: []runtime.Object{testSecret(), testOperatorConfigMap("true")}, + name: "operator disabled", + existing: []runtime.Object{ + testSecret(), + testOperatorConfigMap("true"), + testOperatorConfig(""), + }, }, { - name: "operator enabled", - existing: []runtime.Object{testSecret(), testOperatorConfigMap("false")}, + name: "operator enabled", + existing: []runtime.Object{ + testSecret(), + testOperatorConfigMap("false"), + testOperatorConfig(""), + }, }, { name: "annotate passthrough mode", // right now only passthrough mode is supported so any secret works - existing: []runtime.Object{testSecret()}, + existing: []runtime.Object{ + testSecret(), + testOperatorConfig(""), + }, validateAnnotationValue: constants.PassthroughAnnotation, }, { @@ -148,6 +160,19 @@ func testOperatorConfigMap(disabled string) *corev1.ConfigMap { } } +func testOperatorConfig(mode operatorv1.CloudCredentialsMode) *operatorv1.CloudCredential { + conf := &operatorv1.CloudCredential{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.CloudCredOperatorConfig, + }, + Spec: operatorv1.CloudCredentialSpec{ + CredentialsMode: mode, + }, + } + + return conf +} + func validateSecretAnnotation(c client.Client, t *testing.T, value string) { secret := getCredSecret(c) validateAnnotation(t, secret, value) diff --git a/pkg/operator/utils/utils.go b/pkg/operator/utils/utils.go index ef44ef8c2..6b3f0318e 100644 --- a/pkg/operator/utils/utils.go +++ b/pkg/operator/utils/utils.go @@ -149,10 +149,9 @@ func GenerateNameWithFieldLimits(infraName string, infraNameMaxLen int, crName s return fmt.Sprintf("%s%s", infraPrefix, crName), nil } -// IsOperatorDisabledViaConfigmap checks the cloud-credential-operator-config ConfigMap for a -// "disabled" property set to true. If the configmap or property do not exist, we assume -// false and continue normal operation. This should be used in all controllers to shutdown -// functionality in environments where admins want to manage their credentials themselves. +// isOperatorDisabledViaConfigmap checks the cloud-credential-operator-config ConfigMap for a +// "disabled" property set to true. If the configmap or property does not exist, we assume +// false and continue normal operation. // DEPRECATED (and unexported), use GetOperatorConfiguration to determine disabled state. func isOperatorDisabledViaConfigmap(kubeClient client.Client, logger log.FieldLogger) (bool, error) { cm, err := GetLegacyConfigMap(kubeClient) @@ -181,14 +180,14 @@ func GetLegacyConfigMap(kubeClient client.Client) (*corev1.ConfigMap, error) { // and whether there is a conflict between the legacy ConfigMap and CCO config (in the even of a conflict, the // operator mode will be reported to reflect the actual value in the operator config). func GetOperatorConfiguration(kubeClient client.Client, logger log.FieldLogger) ( - operatorMode operatorv1.CloudCredentialsMode, + effectiveOperatorMode operatorv1.CloudCredentialsMode, configurationConflict bool, err error) { + var operatorMode operatorv1.CloudCredentialsMode operatorMode, err = getOperatorMode(kubeClient, logger) if err != nil { return } - disabledViaOperatorConfig := operatorMode == operatorv1.CloudCredentialsModeManual var disabledViaConfigMap bool disabledViaConfigMap, err = isOperatorDisabledViaConfigmap(kubeClient, logger) @@ -196,26 +195,35 @@ func GetOperatorConfiguration(kubeClient client.Client, logger log.FieldLogger) return } - if operatorMode == operatorv1.CloudCredentialsModeDefault { - // if no mode is set, then only the value in the configmap can end up disabling CCO - // so report back that the operator mode is manual (via the configmap setting) - if disabledViaConfigMap { - operatorMode = operatorv1.CloudCredentialsModeManual + effectiveOperatorMode, configurationConflict = GetEffectiveOperatorMode(disabledViaConfigMap, operatorMode) + if configurationConflict { + log.Errorf("legacy configmap disabled set to %v conflicts with operator CR mode of %s", + disabledViaConfigMap, operatorMode) + } + + return +} + +// GetEffectiveOperatorMode will take the legacy configmap and the value in the operator config, and return +// the effective CCO mode and whether there is a conflict between the legacy and operator config values. +func GetEffectiveOperatorMode(configMapDisabledValue bool, operatorConfigMode operatorv1.CloudCredentialsMode) (operatorv1.CloudCredentialsMode, bool) { + + // if no mode is set, then only the value in the configmap can end up disabling CCO + if operatorConfigMode == "" { + if configMapDisabledValue { + return operatorv1.CloudCredentialsModeManual, false } - return + return operatorConfigMode, false } // else see if there is a disconnect between the operator mode and the // opt-in value of 'disabled: "true"' in the ConfigMap - if disabledViaConfigMap && disabledViaConfigMap != disabledViaOperatorConfig { - log.Errorf("legacy configmap disabled set to %v conflict with operator CR mode of %s", - disabledViaConfigMap, operatorMode) - // Allow the actual CCO config to be returned as that should take - // precedence for the purpose of reporting metrics data, but indicate the conflict. - configurationConflict = true + disabledViaOperatorConfig := operatorConfigMode == operatorv1.CloudCredentialsModeManual + if configMapDisabledValue && !disabledViaOperatorConfig { + return operatorConfigMode, true } - return + return operatorConfigMode, false } @@ -227,14 +235,15 @@ func getOperatorMode(kubeClient client.Client, logger log.FieldLogger) (operator Name: constants.CloudCredOperatorConfig, }, conf) if err != nil { - // SHOULD WE BE WATCHING FOR THIS ERROR??? + // TODO: is it valuable to watch for this error, or just return the error + // at the bottom of this block??? if metaerrors.IsNoMatchError(err) { logger.WithError(err).Debug("no config CRD found") - return operatorv1.CloudCredentialsModeDefault, nil + return "", err } if errors.IsNotFound(err) { - logger.Debugf("%s CCO operator config does not exist, assuming default behavior", constants.CloudCredOperatorConfig) - return operatorv1.CloudCredentialsModeDefault, nil + logger.Debugf("%s CCO operator config does not exist", constants.CloudCredOperatorConfig) + return "", err } return "", err }