From 2da8dac3bba7f0fbe52cd390dd10120d5545da34 Mon Sep 17 00:00:00 2001 From: Jaideep Raghunath Rao Date: Wed, 22 Dec 2021 15:00:09 -0500 Subject: [PATCH] fix: Revert "feat: Migrate dex to spec.sso (#488)" (#527) * Revert "feat: Migrate dex to spec.sso (#488)" This reverts commit 6de5fc2585acdcb595c04cf400139fac48eda431. * fix lint error --- api/v1alpha1/argocd_types.go | 35 ++-- api/v1alpha1/zz_generated.deepcopy.go | 38 ++-- ...argocd-operator.clusterserviceversion.yaml | 64 +++---- bundle/manifests/argoproj.io_argocds.yaml | 178 +++++++++--------- config/crd/bases/argoproj.io_argocds.yaml | 178 +++++++++--------- ...argocd-operator.clusterserviceversion.yaml | 64 +++---- controllers/argocd/argocd_controller.go | 2 +- controllers/argocd/configmap.go | 26 +-- controllers/argocd/configmap_test.go | 35 +++- controllers/argocd/deployment.go | 24 ++- controllers/argocd/deployment_test.go | 54 ++---- controllers/argocd/keycloak.go | 27 +-- controllers/argocd/keycloak_test.go | 6 +- controllers/argocd/role.go | 7 +- controllers/argocd/role_test.go | 19 +- controllers/argocd/rolebinding.go | 13 +- controllers/argocd/rolebinding_test.go | 19 +- controllers/argocd/service.go | 7 +- controllers/argocd/service_account.go | 10 +- controllers/argocd/service_account_test.go | 19 +- controllers/argocd/service_test.go | 31 +-- controllers/argocd/sso.go | 109 ++--------- controllers/argocd/sso_test.go | 6 + controllers/argocd/status.go | 24 +++ controllers/argocd/status_test.go | 45 +++++ controllers/argocd/testing.go | 35 +++- controllers/argocd/util.go | 46 ++--- controllers/argocd/util_test.go | 26 +-- ...operator.v0.2.0.clusterserviceversion.yaml | 64 +++---- .../0.2.0/argoproj.io_argocds.yaml | 178 +++++++++--------- tests/e2e/argocd-tests/01-assert.yaml | 17 ++ 31 files changed, 646 insertions(+), 760 deletions(-) create mode 100644 controllers/argocd/status_test.go diff --git a/api/v1alpha1/argocd_types.go b/api/v1alpha1/argocd_types.go index 4c8eea5c4..d202f5cea 100644 --- a/api/v1alpha1/argocd_types.go +++ b/api/v1alpha1/argocd_types.go @@ -174,21 +174,6 @@ type ArgoCDDexSpec struct { Version string `json:"version,omitempty"` } -// ArgoCDKeycloakSpec defines the desired state for the Keycloak component. -type ArgoCDKeycloakSpec struct { - // Image is the Keycloak container image. - Image string `json:"image,omitempty"` - - // Resources defines the Compute Resources required by the container for Keycloak. - Resources *corev1.ResourceRequirements `json:"resources,omitempty"` - - // Version is the Keycloak container image tag. - Version string `json:"version,omitempty"` - - // VerifyTLS set to false disables strict TLS validation. - VerifyTLS *bool `json:"verifyTLS,omitempty"` -} - // ArgoCDDexOAuthSpec defines the desired state for the Dex OAuth configuration. type ArgoCDDexOAuthSpec struct { // Enabled will toggle OAuth support for the Dex server. @@ -495,21 +480,20 @@ const ( // SSOProviderTypeKeycloak means keycloak will be Installed and Integrated with Argo CD. A new realm with name argocd // will be created in this keycloak. This realm will have a client with name argocd that uses OpenShift v4 as Identity Provider. SSOProviderTypeKeycloak SSOProviderType = "keycloak" - - // SSOProviderTypeDex means dex will be Installed and Integrated with Argo CD. - SSOProviderTypeDex SSOProviderType = "dex" ) // ArgoCDSSOSpec defines SSO provider. type ArgoCDSSOSpec struct { - // Dex contains the configuration for Argo CD dex authentication (previously found under cr.spec.Dex) - Dex ArgoCDDexSpec `json:"dex,omitempty"` - - // Keycloak contains the configuration for Argo CD keycloak authentication (previously found under cr.spec.sso) - Keycloak ArgoCDKeycloakSpec `json:"keycloak,omitempty"` - + // Image is the SSO container image. + Image string `json:"image,omitempty"` // Provider installs and configures the given SSO Provider with Argo CD. Provider SSOProviderType `json:"provider,omitempty"` + // Resources defines the Compute Resources required by the container for SSO. + Resources *corev1.ResourceRequirements `json:"resources,omitempty"` + // VerifyTLS set to false disables strict TLS validation. + VerifyTLS *bool `json:"verifyTLS,omitempty"` + // Version is the SSO container image tag. + Version string `json:"version,omitempty"` } // KustomizeVersionSpec is used to specify information about a kustomize version to be used within ArgoCD. @@ -546,6 +530,9 @@ type ArgoCDSpec struct { // Controller defines the Application Controller options for ArgoCD. Controller ArgoCDApplicationControllerSpec `json:"controller,omitempty"` + // Dex defines the Dex server options for ArgoCD. + Dex ArgoCDDexSpec `json:"dex,omitempty"` + // DisableAdmin will disable the admin user. DisableAdmin bool `json:"disableAdmin,omitempty"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 84bf64e6a..c2469740d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -425,31 +425,6 @@ func (in *ArgoCDIngressSpec) DeepCopy() *ArgoCDIngressSpec { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ArgoCDKeycloakSpec) DeepCopyInto(out *ArgoCDKeycloakSpec) { - *out = *in - if in.Resources != nil { - in, out := &in.Resources, &out.Resources - *out = new(v1.ResourceRequirements) - (*in).DeepCopyInto(*out) - } - if in.VerifyTLS != nil { - in, out := &in.VerifyTLS, &out.VerifyTLS - *out = new(bool) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArgoCDKeycloakSpec. -func (in *ArgoCDKeycloakSpec) DeepCopy() *ArgoCDKeycloakSpec { - if in == nil { - return nil - } - out := new(ArgoCDKeycloakSpec) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArgoCDList) DeepCopyInto(out *ArgoCDList) { *out = *in @@ -683,8 +658,16 @@ func (in *ArgoCDRouteSpec) DeepCopy() *ArgoCDRouteSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArgoCDSSOSpec) DeepCopyInto(out *ArgoCDSSOSpec) { *out = *in - in.Dex.DeepCopyInto(&out.Dex) - in.Keycloak.DeepCopyInto(&out.Keycloak) + if in.Resources != nil { + in, out := &in.Resources, &out.Resources + *out = new(v1.ResourceRequirements) + (*in).DeepCopyInto(*out) + } + if in.VerifyTLS != nil { + in, out := &in.VerifyTLS, &out.VerifyTLS + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArgoCDSSOSpec. @@ -794,6 +777,7 @@ func (in *ArgoCDSpec) DeepCopyInto(out *ArgoCDSpec) { (*in).DeepCopyInto(*out) } in.Controller.DeepCopyInto(&out.Controller) + in.Dex.DeepCopyInto(&out.Dex) in.Grafana.DeepCopyInto(&out.Grafana) in.HA.DeepCopyInto(&out.HA) if in.Import != nil { diff --git a/bundle/manifests/argocd-operator.clusterserviceversion.yaml b/bundle/manifests/argocd-operator.clusterserviceversion.yaml index 33c42f0a3..ad992955b 100644 --- a/bundle/manifests/argocd-operator.clusterserviceversion.yaml +++ b/bundle/manifests/argocd-operator.clusterserviceversion.yaml @@ -324,6 +324,38 @@ spec: x-descriptors: - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Controller - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: Config is the dex connector configuration. + displayName: Configuration + path: dex.config + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text + - description: Image is the Dex container image. + displayName: Image + path: dex.image + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text + - description: OpenShiftOAuth enables OpenShift OAuth authentication for the + Dex server. + displayName: OpenShift OAuth Enabled' + path: dex.openShiftOAuth + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:booleanSwitch + - description: Resources defines the Compute Resources required by the container + for Dex. + displayName: Resource Requirements' + path: dex.resources + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: Version is the Dex container image tag. + displayName: Version + path: dex.version + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text - description: GAAnonymizeUsers toggles user IDs being hashed before sending to google analytics. displayName: Google Analytics Anonymize Users' @@ -617,38 +649,6 @@ spec: x-descriptors: - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Server - urn:alm:descriptor:com.tectonic.ui:text - - description: Config is the dex connector configuration. - displayName: Configuration - path: sso.dex.config - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - - description: Image is the Dex container image. - displayName: Image - path: sso.dex.image - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - - description: OpenShiftOAuth enables OpenShift OAuth authentication for the - Dex server. - displayName: OpenShift OAuth Enabled' - path: sso.dex.openShiftOAuth - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:booleanSwitch - - description: Resources defines the Compute Resources required by the container - for Dex. - displayName: Resource Requirements' - path: sso.dex.resources - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:resourceRequirements - - description: Version is the Dex container image tag. - displayName: Version - path: sso.dex.version - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - description: StatusBadgeEnabled toggles application status badge feature. displayName: Status Badge Enabled' path: statusBadgeEnabled diff --git a/bundle/manifests/argoproj.io_argocds.yaml b/bundle/manifests/argoproj.io_argocds.yaml index e86842c87..5eca67043 100644 --- a/bundle/manifests/argoproj.io_argocds.yaml +++ b/bundle/manifests/argoproj.io_argocds.yaml @@ -277,6 +277,56 @@ spec: type: integer type: object type: object + dex: + description: Dex defines the Dex server options for ArgoCD. + properties: + config: + description: Config is the dex connector configuration. + type: string + groups: + description: Optional list of required groups a user must be a + member of + items: + type: string + type: array + image: + description: Image is the Dex container image. + type: string + openShiftOAuth: + description: OpenShiftOAuth enables OpenShift OAuth authentication + for the Dex server. + type: boolean + resources: + description: Resources defines the Compute Resources required + by the container for Dex. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute + resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute + resources required. If Requests is omitted for a container, + it defaults to Limits if that is explicitly specified, otherwise + to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + version: + description: Version is the Dex container image tag. + type: string + type: object disableAdmin: description: DisableAdmin will disable the admin user. type: boolean @@ -4132,103 +4182,45 @@ spec: description: SSO defines the Single Sign-on configuration for Argo CD properties: - dex: - description: Dex contains the configuration for Argo CD dex authentication - (previously found under cr.spec.Dex) + image: + description: Image is the SSO container image. + type: string + provider: + description: Provider installs and configures the given SSO Provider + with Argo CD. + type: string + resources: + description: Resources defines the Compute Resources required + by the container for SSO. properties: - config: - description: Config is the dex connector configuration. - type: string - groups: - description: Optional list of required groups a user must - be a member of - items: - type: string - type: array - image: - description: Image is the Dex container image. - type: string - openShiftOAuth: - description: OpenShiftOAuth enables OpenShift OAuth authentication - for the Dex server. - type: boolean - resources: - description: Resources defines the Compute Resources required - by the container for Dex. - properties: - limits: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Limits describes the maximum amount of compute - resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object - requests: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Requests describes the minimum amount of - compute resources required. If Requests is omitted for - a container, it defaults to Limits if that is explicitly - specified, otherwise to an implementation-defined value. - More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute + resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' type: object - version: - description: Version is the Dex container image tag. - type: string - type: object - keycloak: - description: Keycloak contains the configuration for Argo CD keycloak - authentication (previously found under cr.spec.sso) - properties: - image: - description: Image is the Keycloak container image. - type: string - resources: - description: Resources defines the Compute Resources required - by the container for Keycloak. - properties: - limits: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Limits describes the maximum amount of compute - resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object - requests: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Requests describes the minimum amount of - compute resources required. If Requests is omitted for - a container, it defaults to Limits if that is explicitly - specified, otherwise to an implementation-defined value. - More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute + resources required. If Requests is omitted for a container, + it defaults to Limits if that is explicitly specified, otherwise + to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' type: object - verifyTLS: - description: VerifyTLS set to false disables strict TLS validation. - type: boolean - version: - description: Version is the Keycloak container image tag. - type: string type: object - provider: - description: Provider installs and configures the given SSO Provider - with Argo CD. + verifyTLS: + description: VerifyTLS set to false disables strict TLS validation. + type: boolean + version: + description: Version is the SSO container image tag. type: string type: object statusBadgeEnabled: diff --git a/config/crd/bases/argoproj.io_argocds.yaml b/config/crd/bases/argoproj.io_argocds.yaml index d6f6df3ec..b99536f59 100644 --- a/config/crd/bases/argoproj.io_argocds.yaml +++ b/config/crd/bases/argoproj.io_argocds.yaml @@ -279,6 +279,56 @@ spec: type: integer type: object type: object + dex: + description: Dex defines the Dex server options for ArgoCD. + properties: + config: + description: Config is the dex connector configuration. + type: string + groups: + description: Optional list of required groups a user must be a + member of + items: + type: string + type: array + image: + description: Image is the Dex container image. + type: string + openShiftOAuth: + description: OpenShiftOAuth enables OpenShift OAuth authentication + for the Dex server. + type: boolean + resources: + description: Resources defines the Compute Resources required + by the container for Dex. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute + resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute + resources required. If Requests is omitted for a container, + it defaults to Limits if that is explicitly specified, otherwise + to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + version: + description: Version is the Dex container image tag. + type: string + type: object disableAdmin: description: DisableAdmin will disable the admin user. type: boolean @@ -4134,103 +4184,45 @@ spec: description: SSO defines the Single Sign-on configuration for Argo CD properties: - dex: - description: Dex contains the configuration for Argo CD dex authentication - (previously found under cr.spec.Dex) + image: + description: Image is the SSO container image. + type: string + provider: + description: Provider installs and configures the given SSO Provider + with Argo CD. + type: string + resources: + description: Resources defines the Compute Resources required + by the container for SSO. properties: - config: - description: Config is the dex connector configuration. - type: string - groups: - description: Optional list of required groups a user must - be a member of - items: - type: string - type: array - image: - description: Image is the Dex container image. - type: string - openShiftOAuth: - description: OpenShiftOAuth enables OpenShift OAuth authentication - for the Dex server. - type: boolean - resources: - description: Resources defines the Compute Resources required - by the container for Dex. - properties: - limits: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Limits describes the maximum amount of compute - resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object - requests: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Requests describes the minimum amount of - compute resources required. If Requests is omitted for - a container, it defaults to Limits if that is explicitly - specified, otherwise to an implementation-defined value. - More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute + resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' type: object - version: - description: Version is the Dex container image tag. - type: string - type: object - keycloak: - description: Keycloak contains the configuration for Argo CD keycloak - authentication (previously found under cr.spec.sso) - properties: - image: - description: Image is the Keycloak container image. - type: string - resources: - description: Resources defines the Compute Resources required - by the container for Keycloak. - properties: - limits: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Limits describes the maximum amount of compute - resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object - requests: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Requests describes the minimum amount of - compute resources required. If Requests is omitted for - a container, it defaults to Limits if that is explicitly - specified, otherwise to an implementation-defined value. - More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute + resources required. If Requests is omitted for a container, + it defaults to Limits if that is explicitly specified, otherwise + to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' type: object - verifyTLS: - description: VerifyTLS set to false disables strict TLS validation. - type: boolean - version: - description: Version is the Keycloak container image tag. - type: string type: object - provider: - description: Provider installs and configures the given SSO Provider - with Argo CD. + verifyTLS: + description: VerifyTLS set to false disables strict TLS validation. + type: boolean + version: + description: Version is the SSO container image tag. type: string type: object statusBadgeEnabled: diff --git a/config/manifests/bases/argocd-operator.clusterserviceversion.yaml b/config/manifests/bases/argocd-operator.clusterserviceversion.yaml index 6e18d0742..ee0407e69 100644 --- a/config/manifests/bases/argocd-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/argocd-operator.clusterserviceversion.yaml @@ -201,6 +201,38 @@ spec: x-descriptors: - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Controller - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: Config is the dex connector configuration. + displayName: Configuration + path: dex.config + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text + - description: Image is the Dex container image. + displayName: Image + path: dex.image + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text + - description: OpenShiftOAuth enables OpenShift OAuth authentication for the + Dex server. + displayName: OpenShift OAuth Enabled' + path: dex.openShiftOAuth + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:booleanSwitch + - description: Resources defines the Compute Resources required by the container + for Dex. + displayName: Resource Requirements' + path: dex.resources + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: Version is the Dex container image tag. + displayName: Version + path: dex.version + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text - description: GAAnonymizeUsers toggles user IDs being hashed before sending to google analytics. displayName: Google Analytics Anonymize Users' @@ -494,38 +526,6 @@ spec: x-descriptors: - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Server - urn:alm:descriptor:com.tectonic.ui:text - - description: Config is the dex connector configuration. - displayName: Configuration - path: sso.dex.config - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - - description: Image is the Dex container image. - displayName: Image - path: sso.dex.image - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - - description: OpenShiftOAuth enables OpenShift OAuth authentication for the - Dex server. - displayName: OpenShift OAuth Enabled' - path: sso.dex.openShiftOAuth - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:booleanSwitch - - description: Resources defines the Compute Resources required by the container - for Dex. - displayName: Resource Requirements' - path: sso.dex.resources - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:resourceRequirements - - description: Version is the Dex container image tag. - displayName: Version - path: sso.dex.version - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - description: StatusBadgeEnabled toggles application status badge feature. displayName: Status Badge Enabled' path: statusBadgeEnabled diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index 01cc3399c..0d6a617b1 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -123,6 +123,6 @@ func (r *ReconcileArgoCD) Reconcile(ctx context.Context, request ctrl.Request) ( // SetupWithManager sets up the controller with the Manager. func (r *ReconcileArgoCD) SetupWithManager(mgr ctrl.Manager) error { bldr := ctrl.NewControllerManagedBy(mgr) - r.setResourceWatches(bldr, r.clusterResourceMapper, r.tlsSecretMapper, r.namespaceResourceMapper) + setResourceWatches(bldr, r.clusterResourceMapper, r.tlsSecretMapper, r.namespaceResourceMapper) return bldr.Complete(r) } diff --git a/controllers/argocd/configmap.go b/controllers/argocd/configmap.go index 5595e61d1..e1e989a98 100644 --- a/controllers/argocd/configmap.go +++ b/controllers/argocd/configmap.go @@ -73,8 +73,8 @@ func getConfigManagementPlugins(cr *argoprojv1a1.ArgoCD) string { func getDexConfig(cr *argoprojv1a1.ArgoCD) string { config := common.ArgoCDDefaultDexConfig - if len(cr.Spec.SSO.Dex.Config) > 0 { - config = cr.Spec.SSO.Dex.Config + if len(cr.Spec.Dex.Config) > 0 { + config = cr.Spec.Dex.Config } return config } @@ -307,9 +307,7 @@ func (r *ReconcileArgoCD) reconcileCAConfigMap(cr *argoprojv1a1.ArgoCD) error { func (r *ReconcileArgoCD) reconcileArgoConfigMap(cr *argoprojv1a1.ArgoCD) error { cm := newConfigMapWithName(common.ArgoCDConfigMapName, cr) if argoutil.IsObjectFound(r.Client, cr.Namespace, cm.Name, cm) { - - // Reconcile dex configuration in configmap only if dex is configured - if cr.Spec.SSO != nil && cr.Spec.SSO.Provider == argoprojv1a1.SSOProviderTypeDex { + if cr.Spec.SSO == nil { if err := r.reconcileDexConfiguration(cm, cr); err != nil { return err } @@ -348,16 +346,18 @@ func (r *ReconcileArgoCD) reconcileArgoConfigMap(cr *argoprojv1a1.ArgoCD) error cm.Data[common.ArgoCDKeyServerURL] = r.getArgoServerURI(cr) cm.Data[common.ArgoCDKeyUsersAnonymousEnabled] = fmt.Sprint(cr.Spec.UsersAnonymousEnabled) - if cr.Spec.SSO != nil && cr.Spec.SSO.Provider == argoprojv1a1.SSOProviderTypeDex { + if !isDexDisabled() { dexConfig := getDexConfig(cr) - if dexConfig == "" && cr.Spec.SSO.Dex.OpenShiftOAuth { - cfg, err := r.getOpenShiftDexConfig(cr) - if err != nil { - return err + if cr.Spec.SSO == nil { + if dexConfig == "" && cr.Spec.Dex.OpenShiftOAuth { + cfg, err := r.getOpenShiftDexConfig(cr) + if err != nil { + return err + } + dexConfig = cfg } - dexConfig = cfg + cm.Data[common.ArgoCDKeyDexConfig] = dexConfig } - cm.Data[common.ArgoCDKeyDexConfig] = dexConfig } if err := controllerutil.SetControllerReference(cr, cm, r.Scheme); err != nil { @@ -370,7 +370,7 @@ func (r *ReconcileArgoCD) reconcileArgoConfigMap(cr *argoprojv1a1.ArgoCD) error func (r *ReconcileArgoCD) reconcileDexConfiguration(cm *corev1.ConfigMap, cr *argoprojv1a1.ArgoCD) error { actual := cm.Data[common.ArgoCDKeyDexConfig] desired := getDexConfig(cr) - if len(desired) <= 0 && cr.Spec.SSO.Dex.OpenShiftOAuth { + if len(desired) <= 0 && cr.Spec.Dex.OpenShiftOAuth { cfg, err := r.getOpenShiftDexConfig(cr) if err != nil { return err diff --git a/controllers/argocd/configmap_test.go b/controllers/argocd/configmap_test.go index 43d5138fa..4f15f1ab3 100644 --- a/controllers/argocd/configmap_test.go +++ b/controllers/argocd/configmap_test.go @@ -17,6 +17,7 @@ package argocd import ( "context" "fmt" + "os" "reflect" "testing" @@ -101,6 +102,7 @@ func TestReconcileArgoCD_reconcileArgoConfigMap(t *testing.T) { "application.instanceLabelKey": common.ArgoCDDefaultApplicationInstanceLabelKey, "admin.enabled": "true", "configManagementPlugins": "", + "dex.config": "", "ga.anonymizeusers": "false", "ga.trackingid": "", "help.chatText": "Chat now!", @@ -212,12 +214,7 @@ func TestReconcileArgoCD_reconcileArgoConfigMap_withDexConnector(t *testing.T) { restoreEnv(t) logf.SetLogger(ZapLogger(true)) a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } + a.Spec.Dex.OpenShiftOAuth = true }) sa := &corev1.ServiceAccount{ TypeMeta: metav1.TypeMeta{Kind: "ServiceAccount", APIVersion: "v1"}, @@ -257,14 +254,13 @@ func TestReconcileArgoCD_reconcileArgoConfigMap_withDexConnector(t *testing.T) { assert.Equal(t, config.(map[interface{}]interface{})["clientID"], "system:serviceaccount:argocd:argocd-argocd-dex-server") } -func TestReconcileArgoCD_reconcileArgoConfigMap_withDexNotConfigured(t *testing.T) { +func TestReconcileArgoCD_reconcileArgoConfigMap_withDexDisabled(t *testing.T) { restoreEnv(t) logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{} - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) + os.Setenv("DISABLE_DEX", "true") err := r.reconcileArgoConfigMap(a) assert.NilError(t, err) @@ -279,6 +275,25 @@ func TestReconcileArgoCD_reconcileArgoConfigMap_withDexNotConfigured(t *testing. t.Fatalf("reconcileArgoConfigMap failed, dex.config = %q", c) } } +func TestReconcileArgoCD_reconcileArgoConfigMap_withMultipleSSOConfigured(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCDForKeycloakWithDex() + r := makeTestReconciler(t, a) + + err := r.reconcileArgoConfigMap(a) + assert.NilError(t, err) + + cm := &corev1.ConfigMap{} + err = r.Client.Get(context.TODO(), types.NamespacedName{ + Name: common.ArgoCDConfigMapName, + Namespace: testNamespace, + }, cm) + assert.NilError(t, err) + + if c, ok := cm.Data["dex.openShiftOAuth"]; !ok && len(c) != 0 { + t.Fatalf("reconcileArgoConfigMap didn't skip setting dex when keycloak is configured, dex.openShiftOAuth = %q", c) + } +} func TestReconcileArgoCD_reconcileArgoConfigMap_withKustomizeVersions(t *testing.T) { logf.SetLogger(ZapLogger(true)) diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go index 1a58d632e..0a2b28b89 100644 --- a/controllers/argocd/deployment.go +++ b/controllers/argocd/deployment.go @@ -319,8 +319,12 @@ func newDeploymentWithSuffix(suffix string, component string, cr *argoprojv1a1.A // reconcileDeployments will ensure that all Deployment resources are present for the given ArgoCD. func (r *ReconcileArgoCD) reconcileDeployments(cr *argoprojv1a1.ArgoCD) error { + err := r.reconcileDexDeployment(cr) + if err != nil { + return err + } - err := r.reconcileRedisDeployment(cr) + err = r.reconcileRedisDeployment(cr) if err != nil { return err } @@ -401,11 +405,15 @@ func (r *ReconcileArgoCD) reconcileDexDeployment(cr *argoprojv1a1.ArgoCD) error EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }} + dexDisabled := isDexDisabled() + if dexDisabled { + log.Info("reconciling for dex, but dex is disabled") + } existing := newDeploymentWithSuffix("dex-server", "dex-server", cr) if argoutil.IsObjectFound(r.Client, cr.Namespace, existing.Name, existing) { - if cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex { - log.Info("deleting the existing Dex deployment because dex is not configured") + if dexDisabled { + log.Info("deleting the existing dex deployment because dex is disabled") // Deployment exists but enabled flag has been set to false, delete the Deployment return r.Client.Delete(context.TODO(), existing) } @@ -450,8 +458,7 @@ func (r *ReconcileArgoCD) reconcileDexDeployment(cr *argoprojv1a1.ArgoCD) error return nil // Deployment found with nothing to do, move along... } - // Dex deployment not found. Keep it that way if any of the following conditions are true - if cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex { + if dexDisabled { return nil } @@ -1176,6 +1183,13 @@ func caseInsensitiveGetenv(s string) (string, string) { return "", "" } +func isDexDisabled() bool { + if v := os.Getenv("DISABLE_DEX"); v != "" { + return strings.ToLower(v) == "true" + } + return false +} + // to update nodeSelector and tolerations in reconciler func updateNodePlacement(existing *appsv1.Deployment, deploy *appsv1.Deployment, changed *bool) { if !reflect.DeepEqual(existing.Spec.Template.Spec.NodeSelector, deploy.Spec.Template.Spec.NodeSelector) { diff --git a/controllers/argocd/deployment_test.go b/controllers/argocd/deployment_test.go index 1fbbc4f45..5b5068cb7 100644 --- a/controllers/argocd/deployment_test.go +++ b/controllers/argocd/deployment_test.go @@ -34,6 +34,7 @@ const ( var ( deploymentNames = []string{ "argocd-repo-server", + "argocd-dex-server", "argocd-grafana", "argocd-redis", "argocd-server"} @@ -445,13 +446,13 @@ func TestReconcileArgoCD_reconcileRepoDeployment_command(t *testing.T) { assert.Equal(t, "debug", deployment.Spec.Template.Spec.Containers[0].Command[6]) } -func TestReconcileArgoCD_reconcileDexDeployment_with_dex_not_configured(t *testing.T) { +func TestReconcileArgoCD_reconcileDexDeployment_with_dex_disabled(t *testing.T) { restoreEnv(t) logf.SetLogger(ZapLogger(true)) a := makeTestArgoCD() r := makeTestReconciler(t, a) - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{} + os.Setenv("DISABLE_DEX", "true") assert.NoError(t, r.reconcileDexDeployment(a)) deployment := &appsv1.Deployment{} @@ -459,34 +460,20 @@ func TestReconcileArgoCD_reconcileDexDeployment_with_dex_not_configured(t *testi assert.True(t, apierrors.IsNotFound(err)) } -// When Dex is configured Dex deployment should be present -// When Dex is not configured the Dex Deployment should be removed. -func TestReconcileArgoCD_reconcileDexDeployment_with_configuration(t *testing.T) { +// When Dex is disabled, the Dex Deployment should be removed. +func TestReconcileArgoCD_reconcileDexDeployment_removes_dex_when_disabled(t *testing.T) { restoreEnv(t) logf.SetLogger(ZapLogger(true)) a := makeTestArgoCD() r := makeTestReconciler(t, a) - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } + os.Setenv("DISABLE_DEX", "true") assert.NoError(t, r.reconcileDexDeployment(a)) - deployment := &appsv1.Deployment{} - assertNoError(t, r.Client.Get( - context.TODO(), - types.NamespacedName{ - Name: "argocd-dex-server", - Namespace: a.Namespace, - }, - deployment)) - a.Spec.SSO = nil + a = makeTestArgoCD() assert.NoError(t, r.reconcileDexDeployment(a)) - deployment = &appsv1.Deployment{} + deployment := &appsv1.Deployment{} assertNotFound(t, r.Client.Get( context.TODO(), types.NamespacedName{ @@ -763,14 +750,7 @@ func deploymentDefaultTolerations() []corev1.Toleration { func TestReconcileArgoCD_reconcileDexDeployment(t *testing.T) { logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) assert.NoError(t, r.reconcileDexDeployment(a)) @@ -842,22 +822,15 @@ func TestReconcileArgoCD_reconcileDexDeployment(t *testing.T) { func TestReconcileArgoCD_reconcileDexDeployment_withUpdate(t *testing.T) { logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) // Creates the deployment and then changes the CR and rereconciles. assert.NoError(t, r.reconcileDexDeployment(a)) a.Spec.Image = "justatest" a.Spec.Version = "latest" - a.Spec.SSO.Dex.Image = "testdex" - a.Spec.SSO.Dex.Version = "v0.0.1" + a.Spec.Dex.Image = "testdex" + a.Spec.Dex.Version = "v0.0.1" assert.NoError(t, r.reconcileDexDeployment(a)) deployment := &appsv1.Deployment{} @@ -1170,7 +1143,8 @@ func TestReconcileArgoCD_reconcileRedisDeployment_with_error(t *testing.T) { func restoreEnv(t *testing.T) { keys := []string{ "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", - "http_proxy", "https_proxy", "no_proxy"} + "http_proxy", "https_proxy", "no_proxy", + "DISABLE_DEX"} env := map[string]string{} for _, v := range keys { env[v] = os.Getenv(v) diff --git a/controllers/argocd/keycloak.go b/controllers/argocd/keycloak.go index 6ab6cdc9c..d7e2b1c28 100644 --- a/controllers/argocd/keycloak.go +++ b/controllers/argocd/keycloak.go @@ -124,7 +124,7 @@ type oidcConfig struct { // common.ArgoCDKeycloakImageName. func getKeycloakContainerImage(cr *argoprojv1a1.ArgoCD) string { defaultImg, defaultTag := false, false - img := cr.Spec.SSO.Keycloak.Image + img := cr.Spec.SSO.Image if img == "" { img = common.ArgoCDKeycloakImage if IsTemplateAPIAvailable() { @@ -133,7 +133,7 @@ func getKeycloakContainerImage(cr *argoprojv1a1.ArgoCD) string { defaultImg = true } - tag := cr.Spec.SSO.Keycloak.Version + tag := cr.Spec.SSO.Version if tag == "" { tag = common.ArgoCDKeycloakVersion if IsTemplateAPIAvailable() { @@ -202,8 +202,8 @@ func getKeycloakResources(cr *argoprojv1a1.ArgoCD) corev1.ResourceRequirements { resources := defaultKeycloakResources() // Allow override of resource requirements from CR - if cr.Spec.SSO.Keycloak.Resources != nil { - resources = *cr.Spec.SSO.Keycloak.Resources + if cr.Spec.SSO.Resources != nil { + resources = *cr.Spec.SSO.Resources return resources } @@ -761,7 +761,7 @@ func (r *ReconcileArgoCD) prepareKeycloakConfig(cr *argoprojv1a1.ArgoCD) (*keycl } // By default TLS Verification should be enabled. - if cr.Spec.SSO.Keycloak.VerifyTLS == nil || *cr.Spec.SSO.Keycloak.VerifyTLS { + if cr.Spec.SSO.VerifyTLS == nil || *cr.Spec.SSO.VerifyTLS { tlsVerification = true } @@ -1089,23 +1089,6 @@ func handleKeycloakPodDeletion(dc *oappsv1.DeploymentConfig) error { return nil } -func deleteKeycloakConfiguration(cr *argoprojv1a1.ArgoCD) error { - - // If Keycloak is installed using OpenShift templates. - if IsTemplateAPIAvailable() { - err := deleteKeycloakConfigForOpenShift(cr) - if err != nil { - return err - } - } else { - err := deleteKeycloakConfigForK8s(cr) - if err != nil { - return err - } - } - return nil -} - // Delete Keycloak configuration for OpenShift func deleteKeycloakConfigForOpenShift(cr *argoprojv1a1.ArgoCD) error { cfg, err := config.GetConfig() diff --git a/controllers/argocd/keycloak_test.go b/controllers/argocd/keycloak_test.go index 358286371..c662272fe 100644 --- a/controllers/argocd/keycloak_test.go +++ b/controllers/argocd/keycloak_test.go @@ -94,8 +94,8 @@ func TestKeycloakContainerImage(t *testing.T) { assert.Equal(t, testImage, "envImage:latest") // when both cr.spec.sso.Image and ArgoCDKeycloakImageEnvName are set. - cr.Spec.SSO.Keycloak.Image = "crImage" - cr.Spec.SSO.Keycloak.Version = "crVersion" + cr.Spec.SSO.Image = "crImage" + cr.Spec.SSO.Version = "crVersion" testImage = getKeycloakContainerImage(cr) assert.Equal(t, testImage, "crImage:crVersion") @@ -177,7 +177,7 @@ func TestKeycloakResources(t *testing.T) { // Verify resource requirements are overridden by ArgoCD CR(.spec.SSO.Resources) fR := getFakeKeycloakResources() - a.Spec.SSO.Keycloak.Resources = &fR + a.Spec.SSO.Resources = &fR kc = getKeycloakContainer(a) assert.DeepEqual(t, kc.Resources, getFakeKeycloakResources()) diff --git a/controllers/argocd/role.go b/controllers/argocd/role.go index c2d524324..0c99b98d4 100644 --- a/controllers/argocd/role.go +++ b/controllers/argocd/role.go @@ -120,8 +120,8 @@ func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule continue // skip creating default role if custom cluster role is provided } roles = append(roles, role) - if name == dexServer && (cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex) { - continue // Dex is not configured, do nothing + if name == dexServer && isDexDisabled() { + continue // Dex is disabled, do nothing } // Only set ownerReferences for roles in same namespace as ArgoCD CR @@ -144,8 +144,7 @@ func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule continue } - if name == dexServer && (cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex) { - log.Info("deleting the existing Dex role because dex is not configured") + if name == dexServer && isDexDisabled() { // Delete any existing Role created for Dex if err := r.Client.Delete(context.TODO(), &existingRole); err != nil { return nil, err diff --git a/controllers/argocd/role_test.go b/controllers/argocd/role_test.go index f637fdcae..654450ae8 100644 --- a/controllers/argocd/role_test.go +++ b/controllers/argocd/role_test.go @@ -12,7 +12,6 @@ import ( "k8s.io/apimachinery/pkg/types" logf "sigs.k8s.io/controller-runtime/pkg/log" - argoprojv1alpha1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1" "github.com/argoproj-labs/argocd-operator/common" ) @@ -48,30 +47,24 @@ func TestReconcileArgoCD_reconcileRole(t *testing.T) { assert.DeepEqual(t, expectedRules, reconciledRole.Rules) } -func TestReconcileArgoCD_reconcileRole_dex_configuration_and_removal(t *testing.T) { +func TestReconcileArgoCD_reconcileRole_dex_disabled(t *testing.T) { logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) assert.NilError(t, createNamespace(r, a.Namespace, "")) rules := policyRuleForDexServer() role := newRole(dexServer, rules, a) - // Dex is configured + // Dex is enabled _, err := r.reconcileRole(dexServer, rules, a) assert.NilError(t, err) assert.NilError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: role.Name, Namespace: a.Namespace}, role)) assert.DeepEqual(t, rules, role.Rules) - // Disable configuration removed - a.Spec.SSO = nil + // Disable Dex + os.Setenv("DISABLE_DEX", "true") + defer os.Unsetenv("DISABLE_DEX") _, err = r.reconcileRole(dexServer, rules, a) assert.NilError(t, err) diff --git a/controllers/argocd/rolebinding.go b/controllers/argocd/rolebinding.go index 0e303c382..3348c5827 100644 --- a/controllers/argocd/rolebinding.go +++ b/controllers/argocd/rolebinding.go @@ -9,7 +9,6 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" - apiErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -72,7 +71,6 @@ func (r *ReconcileArgoCD) reconcileRoleBindings(cr *argoprojv1a1.ArgoCD) error { if err := r.reconcileRoleBinding(applicationController, policyRuleForApplicationController(), cr); err != nil { return fmt.Errorf("error reconciling roleBinding for %q: %w", applicationController, err) } - if err := r.reconcileRoleBinding(dexServer, policyRuleForDexServer(), cr); err != nil { return fmt.Errorf("error reconciling roleBinding for %q: %w", dexServer, err) } @@ -126,8 +124,8 @@ func (r *ReconcileArgoCD) reconcileRoleBinding(name string, rules []v1.PolicyRul if !errors.IsNotFound(err) { return fmt.Errorf("failed to get the rolebinding associated with %s : %s", name, err) } - if name == dexServer && (cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex) { - continue // Dex is not configured, do nothing + if name == dexServer && isDexDisabled() { + continue // Dex is disabled, do nothing } roleBindingExists = false } @@ -156,13 +154,10 @@ func (r *ReconcileArgoCD) reconcileRoleBinding(name string, rules []v1.PolicyRul } if roleBindingExists { - if name == dexServer && (cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex) { - log.Info("deleting the existing Dex rolebinding because dex is not configured") + if name == dexServer && isDexDisabled() { // Delete any existing RoleBinding created for Dex if err = r.Client.Delete(context.TODO(), existingRoleBinding); err != nil { - if !apiErrors.IsNotFound(err) { - return err - } + return err } continue } diff --git a/controllers/argocd/rolebinding_test.go b/controllers/argocd/rolebinding_test.go index 887ea0deb..6523054c1 100644 --- a/controllers/argocd/rolebinding_test.go +++ b/controllers/argocd/rolebinding_test.go @@ -6,7 +6,6 @@ import ( "os" "testing" - argoprojv1alpha1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1" "github.com/argoproj-labs/argocd-operator/common" "gotest.tools/assert" corev1 "k8s.io/api/core/v1" @@ -46,28 +45,22 @@ func TestReconcileArgoCD_reconcileRoleBinding(t *testing.T) { assert.NilError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: a.Namespace}, roleBinding)) } -func TestReconcileArgoCD_reconcileRoleBinding_dex_configuration_and_removal(t *testing.T) { +func TestReconcileArgoCD_reconcileRoleBinding_dex_disabled(t *testing.T) { logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) assert.NilError(t, createNamespace(r, a.Namespace, "")) rules := policyRuleForDexServer() rb := newRoleBindingWithname(dexServer, a) - // Dex is configured, creates a role binding + // Dex is enabled, creates a role binding assert.NilError(t, r.reconcileRoleBinding(dexServer, rules, a)) assert.NilError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: rb.Name, Namespace: a.Namespace}, rb)) - // Dex configuration removed, deletes the existing role binding - a.Spec.SSO = nil + // Disable Dex, deletes the existing role binding + os.Setenv("DISABLE_DEX", "true") + defer os.Unsetenv("DISABLE_DEX") _, err := r.reconcileRole(dexServer, rules, a) assert.NilError(t, err) diff --git a/controllers/argocd/service.go b/controllers/argocd/service.go index d5c101232..9eb0715d1 100644 --- a/controllers/argocd/service.go +++ b/controllers/argocd/service.go @@ -69,16 +69,15 @@ func newServiceWithSuffix(suffix string, component string, cr *argoprojv1a1.Argo func (r *ReconcileArgoCD) reconcileDexService(cr *argoprojv1a1.ArgoCD) error { svc := newServiceWithSuffix("dex-server", "dex-server", cr) if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) { - if cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex { - log.Info("deleting the existing Dex service because dex is not configured") + if isDexDisabled() { // Service exists but enabled flag has been set to false, delete the Service return r.Client.Delete(context.TODO(), svc) } return nil } - if cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex { - return nil // Dex is not configured, do nothing + if isDexDisabled() { + return nil // Dex is disabled, do nothing } svc.Spec.Selector = map[string]string{ diff --git a/controllers/argocd/service_account.go b/controllers/argocd/service_account.go index 971af7ecf..5f8264e2d 100644 --- a/controllers/argocd/service_account.go +++ b/controllers/argocd/service_account.go @@ -17,7 +17,6 @@ package argocd import ( "context" "fmt" - "reflect" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/rbac/v1" @@ -97,7 +96,7 @@ func (r *ReconcileArgoCD) reconcileServiceAccounts(cr *argoprojv1a1.ArgoCD) erro // reconcileDexServiceAccount will ensure that the Dex ServiceAccount is configured properly for OpenShift OAuth. func (r *ReconcileArgoCD) reconcileDexServiceAccount(cr *argoprojv1a1.ArgoCD) error { - if cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex || reflect.DeepEqual(cr.Spec.SSO.Dex, argoprojv1a1.ArgoCDDexSpec{}) || !cr.Spec.SSO.Dex.OpenShiftOAuth { + if !cr.Spec.Dex.OpenShiftOAuth { return nil // OpenShift OAuth not enabled, move along... } @@ -158,14 +157,13 @@ func (r *ReconcileArgoCD) reconcileServiceAccount(name string, cr *argoprojv1a1. if !errors.IsNotFound(err) { return nil, err } - if name == dexServer && (cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex) { - return sa, nil // Dex is not configured, do nothing + if name == dexServer && isDexDisabled() { + return sa, nil // Dex is disabled, do nothing } exists = false } if exists { - if name == dexServer && (cr.Spec.SSO == nil || cr.Spec.SSO.Provider != argoprojv1a1.SSOProviderTypeDex) { - log.Info("deleting the existing Dex service account because dex is not configured") + if name == dexServer && isDexDisabled() { // Delete any existing Service Account created for Dex return sa, r.Client.Delete(context.TODO(), sa) } diff --git a/controllers/argocd/service_account_test.go b/controllers/argocd/service_account_test.go index 5ed5c289a..2714c5053 100644 --- a/controllers/argocd/service_account_test.go +++ b/controllers/argocd/service_account_test.go @@ -20,7 +20,6 @@ import ( "os" "testing" - argoprojv1alpha1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1" "gotest.tools/assert" corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/rbac/v1" @@ -123,26 +122,20 @@ func TestReconcileArgoCD_reconcileServiceAccountClusterPermissions(t *testing.T) assert.ErrorContains(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedClusterRoleName}, reconcileClusterRole), "not found") } -func TestReconcileArgoCD_reconcileServiceAccount_dex_configuration_and_removal(t *testing.T) { +func TestReconcileArgoCD_reconcileServiceAccount_dex_disabled(t *testing.T) { logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) assert.NilError(t, createNamespace(r, a.Namespace, "")) - // Dex is configured, creates a new Service Account for it + // Dex is enabled, creates a new Service Account for it sa, err := r.reconcileServiceAccount(dexServer, a) assert.NilError(t, err) assert.NilError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: sa.Name, Namespace: a.Namespace}, sa)) - // Dex configuration is removed, deletes any existing Service Account for it - a.Spec.SSO = nil + //Disable dex, deletes any existing Service Account for it + os.Setenv("DISABLE_DEX", "true") + defer os.Unsetenv("DISABLE_DEX") sa, err = r.reconcileServiceAccount(dexServer, a) assert.NilError(t, err) diff --git a/controllers/argocd/service_test.go b/controllers/argocd/service_test.go index 3d99d1e1c..32d11d5a7 100644 --- a/controllers/argocd/service_test.go +++ b/controllers/argocd/service_test.go @@ -2,9 +2,9 @@ package argocd import ( "context" + "os" "testing" - argoprojv1alpha1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1" "github.com/argoproj-labs/argocd-operator/common" "gotest.tools/assert" "k8s.io/apimachinery/pkg/types" @@ -13,14 +13,7 @@ import ( func TestReconcileArgoCD_reconcileDexService_Dex_Enabled(t *testing.T) { logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) s := newServiceWithSuffix("dex-server", "dex-server", a) @@ -29,26 +22,22 @@ func TestReconcileArgoCD_reconcileDexService_Dex_Enabled(t *testing.T) { assert.NilError(t, r.Client.Get(context.TODO(), types.NamespacedName{Namespace: s.Namespace, Name: s.Name}, s)) } -func TestReconcileArgoCD_reconcileDexService_dex_configruation_and_removal(t *testing.T) { +func TestReconcileArgoCD_reconcileDexService_Dex_Disabled(t *testing.T) { logf.SetLogger(ZapLogger(true)) - a := makeTestArgoCD(func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }) + a := makeTestArgoCD() r := makeTestReconciler(t, a) s := newServiceWithSuffix("dex-server", "dex-server", a) - // Dex configured, Create Service for Dex + // Create Service for Dex assert.NilError(t, r.reconcileDexService(a)) assert.NilError(t, r.Client.Get(context.TODO(), types.NamespacedName{Namespace: s.Namespace, Name: s.Name}, s)) - // Dex configuration removed, existing service should be deleted - a.Spec.SSO = nil + // Disable Dex, existing service should be deleted + os.Setenv("DISABLE_DEX", "true") + t.Cleanup(func() { + os.Unsetenv("DISABLE_DEX") + }) assert.NilError(t, r.reconcileDexService(a)) assert.ErrorContains(t, r.Client.Get(context.TODO(), types.NamespacedName{Namespace: s.Namespace, Name: s.Name}, s), "not found") diff --git a/controllers/argocd/sso.go b/controllers/argocd/sso.go index e40714a9f..82eb05c23 100644 --- a/controllers/argocd/sso.go +++ b/controllers/argocd/sso.go @@ -15,16 +15,12 @@ package argocd import ( - "errors" + e "errors" "fmt" - "reflect" - - apiErrors "k8s.io/apimachinery/pkg/api/errors" template "github.com/openshift/api/template/v1" argoprojv1a1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1" - "github.com/argoproj-labs/argocd-operator/common" "github.com/argoproj-labs/argocd-operator/controllers/argoutil" ) @@ -49,16 +45,10 @@ func verifyTemplateAPI() error { func (r *ReconcileArgoCD) reconcileSSO(cr *argoprojv1a1.ArgoCD) error { if cr.Spec.SSO.Provider == argoprojv1a1.SSOProviderTypeKeycloak { - // Ensure SSO provider type and provided configuration are compatible - if !reflect.DeepEqual(cr.Spec.SSO.Dex, argoprojv1a1.ArgoCDDexSpec{}) { - err := errors.New("incorrect SSO configuration") - log.Error(err, fmt.Sprintf("cannot supply dex spec when provider is set to: %s", cr.Spec.SSO.Provider)) - return err - } - - // Trigger reconciliation of Dex resources so they get deleted - if err := r.reconcileDexResources(cr); err != nil && !apiErrors.IsNotFound(err) { - log.Error(err, "Unable to reconcile necessary resources for uninstallation of Dex") + if cr.Spec.Dex.OpenShiftOAuth || cr.Spec.Dex.Config != "" { + err := e.New("multiple SSO configuration") + log.Error(err, fmt.Sprintf("Installation of multiple SSO providers is not permitted. Please choose a single provider for Argo CD %s in namespace %s.", + cr.Name, cr.Namespace)) return err } @@ -74,94 +64,21 @@ func (r *ReconcileArgoCD) reconcileSSO(cr *argoprojv1a1.ArgoCD) error { return err } } - } else if cr.Spec.SSO.Provider == argoprojv1a1.SSOProviderTypeDex { - - // Ensure SSO provider type and provided configuration are compatible - if !reflect.DeepEqual(cr.Spec.SSO.Keycloak, argoprojv1a1.ArgoCDKeycloakSpec{}) { - err := errors.New("incorrect SSO configuration") - log.Error(err, fmt.Sprintf("cannot supply keycloak spec when provider is set to: %s", cr.Spec.SSO.Provider)) - return err - } - - // Ensure Dex spec is supplied - if reflect.DeepEqual(cr.Spec.SSO.Dex, argoprojv1a1.ArgoCDDexSpec{}) { - err := errors.New("incorrect SSO configuration") - log.Error(err, fmt.Sprintf("Must supply dex spec for provider: %s", cr.Spec.SSO.Provider)) - return err - } - - // Delete any lingering keycloak artifacts before Dex is configured as this is not handled by the reconcilliation loop - if err := deleteKeycloakConfiguration(cr); err != nil && !apiErrors.IsNotFound(err) { - if !apiErrors.IsNotFound(err) { - log.Error(err, "Unable to delete existing SSO configuration before configuring Dex") - return err - } - } - - // Trigger reconciliation of Dex resources so they get created - if err := r.reconcileDexResources(cr); err != nil { - log.Error(err, "Unable to reconcile necessary resources for installation of Dex") - return err - } - } return nil } -// reconcileResources will trigger reconciliation of necessary resources after changes to Dex SSO configuration -func (r *ReconcileArgoCD) reconcileDexResources(cr *argoprojv1a1.ArgoCD) error { +func deleteSSOConfiguration(cr *argoprojv1a1.ArgoCD) error { - if _, err := r.reconcileRole(dexServer, policyRuleForDexServer(), cr); err != nil { - return err - } - - if err := r.reconcileRoleBinding(dexServer, policyRuleForDexServer(), cr); err != nil { - return fmt.Errorf("error reconciling roleBinding for %q: %w", dexServer, err) - } - - if err := r.reconcileServiceAccountPermissions(common.ArgoCDDexServerComponent, policyRuleForDexServer(), cr); err != nil { - return err - } - - // specialized handling for dex - if err := r.reconcileDexServiceAccount(cr); err != nil { - return err - } - - // Reconcile dex config in argocd-cm - if err := r.reconcileArgoConfigMap(cr); err != nil { - return err - } - - if err := r.reconcileDexService(cr); err != nil { - return err - } - - if err := r.reconcileDexDeployment(cr); err != nil { - return err - } - - if err := r.reconcileStatusDex(cr); err != nil { - return err - } - - return nil -} - -func (r *ReconcileArgoCD) deleteSSOConfiguration(cr *argoprojv1a1.ArgoCD, oldCRSSO *argoprojv1a1.ArgoCDSSOSpec) error { - - log.Info("uninstalling existing SSO configuration") - if oldCRSSO.Provider == argoprojv1a1.SSOProviderTypeKeycloak { - if err := deleteKeycloakConfiguration(cr); err != nil { - log.Error(err, "Unable to delete existing keycloak configuration") + // If SSO is installed using OpenShift templates. + if IsTemplateAPIAvailable() { + err := deleteKeycloakConfigForOpenShift(cr) + if err != nil { return err } - - } else if oldCRSSO.Provider == argoprojv1a1.SSOProviderTypeDex { - - // Trigger reconciliation of Dex resources so they get deleted - if err := r.reconcileDexResources(cr); err != nil { - log.Error(err, "Unable to reconcile necessary resources for uninstallation of Dex") + } else { + err := deleteKeycloakConfigForK8s(cr) + if err != nil { return err } } diff --git a/controllers/argocd/sso_test.go b/controllers/argocd/sso_test.go index 21e28de52..53d007921 100644 --- a/controllers/argocd/sso_test.go +++ b/controllers/argocd/sso_test.go @@ -70,6 +70,12 @@ func TestReconcile_testKeycloakTemplateInstance(t *testing.T) { }, templateInstance)) } +func TestReconcile_testKeycloakTemplateWithDexInstance(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCDForKeycloakWithDex() + r := makeFakeReconciler(t, a) + assert.Error(t, r.reconcileSSO(a), "multiple SSO configuration") +} func TestReconcile_noTemplateInstance(t *testing.T) { logf.SetLogger(ZapLogger(true)) diff --git a/controllers/argocd/status.go b/controllers/argocd/status.go index 32ef2b3ce..876937929 100644 --- a/controllers/argocd/status.go +++ b/controllers/argocd/status.go @@ -16,6 +16,7 @@ package argocd import ( "context" + "reflect" argoprojv1a1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1" "github.com/argoproj-labs/argocd-operator/controllers/argoutil" @@ -31,6 +32,10 @@ func (r *ReconcileArgoCD) reconcileStatus(cr *argoprojv1a1.ArgoCD) error { return err } + if err := r.reconcileStatusSSOConfig(cr); err != nil { + return err + } + if err := r.reconcileStatusPhase(cr); err != nil { return err } @@ -93,6 +98,25 @@ func (r *ReconcileArgoCD) reconcileStatusDex(cr *argoprojv1a1.ArgoCD) error { return nil } +// reconcileStatusSSOConfig will ensure that the SSOConfig status is updated for the given ArgoCD. +func (r *ReconcileArgoCD) reconcileStatusSSOConfig(cr *argoprojv1a1.ArgoCD) error { + status := "Unknown" + + if cr.Spec.SSO != nil && !reflect.DeepEqual(cr.Spec.Dex, argoprojv1a1.ArgoCDDexSpec{}) { + // set state to "Failed" when both keycloak and Dex are configured + status = "Failed" + } else if (cr.Spec.SSO != nil && reflect.DeepEqual(cr.Spec.Dex, argoprojv1a1.ArgoCDDexSpec{})) || (cr.Spec.SSO == nil && !reflect.DeepEqual(cr.Spec.Dex, argoprojv1a1.ArgoCDDexSpec{})) { + // set state to "Success" when only keycloak or only Dex is configured + status = "Success" + } + + if cr.Status.SSOConfig != status { + cr.Status.SSOConfig = status + return r.Client.Status().Update(context.TODO(), cr) + } + return nil +} + // reconcileStatusPhase will ensure that the Status Phase is updated for the given ArgoCD. func (r *ReconcileArgoCD) reconcileStatusPhase(cr *argoprojv1a1.ArgoCD) error { phase := "Unknown" diff --git a/controllers/argocd/status_test.go b/controllers/argocd/status_test.go new file mode 100644 index 000000000..973a17a8d --- /dev/null +++ b/controllers/argocd/status_test.go @@ -0,0 +1,45 @@ +package argocd + +import ( + "testing" + + "gotest.tools/assert" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +func TestReconcileArgoCD_reconcileStatusSSOConfig_multi_sso_configured(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCDForKeycloakWithDex() + + templateAPIFound = true + r := makeTestReconciler(t, a) + assert.NilError(t, r.reconcileStatusSSOConfig(a)) + assert.Equal(t, a.Status.SSOConfig, "Failed") +} +func TestReconcileArgoCD_reconcileStatusSSOConfig_only_keycloak_configured(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCDForKeycloak() + + templateAPIFound = true + r := makeTestReconciler(t, a) + assert.NilError(t, r.reconcileStatusSSOConfig(a)) + assert.Equal(t, a.Status.SSOConfig, "Success") +} +func TestReconcileArgoCD_reconcileStatusSSOConfig_only_dex_configured(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCDWithResources() + + templateAPIFound = true + r := makeTestReconciler(t, a) + assert.NilError(t, r.reconcileStatusSSOConfig(a)) + assert.Equal(t, a.Status.SSOConfig, "Success") +} +func TestReconcileArgoCD_reconcileStatusSSOConfig_no_sso_configured(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCD() + + templateAPIFound = true + r := makeTestReconciler(t, a) + assert.NilError(t, r.reconcileStatusSSOConfig(a)) + assert.Equal(t, a.Status.SSOConfig, "Unknown") +} diff --git a/controllers/argocd/testing.go b/controllers/argocd/testing.go index 98fba8a7a..95ace45dc 100644 --- a/controllers/argocd/testing.go +++ b/controllers/argocd/testing.go @@ -97,6 +97,32 @@ func makeTestArgoCDForKeycloak(opts ...argoCDOpt) *argoprojv1alpha1.ArgoCD { } return a } +func makeTestArgoCDForKeycloakWithDex(opts ...argoCDOpt) *argoprojv1alpha1.ArgoCD { + a := &argoprojv1alpha1.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{ + Name: testArgoCDName, + Namespace: testNamespace, + }, + Spec: argoprojv1alpha1.ArgoCDSpec{ + SSO: &argoprojv1alpha1.ArgoCDSSOSpec{ + Provider: "keycloak", + }, + Dex: argoprojv1alpha1.ArgoCDDexSpec{ + OpenShiftOAuth: true, + Resources: makeTestDexResources(), + }, + Server: argoprojv1alpha1.ArgoCDServerSpec{ + Route: argoprojv1alpha1.ArgoCDRouteSpec{ + Enabled: true, + }, + }, + }, + } + for _, o := range opts { + o(a) + } + return a +} func makeTestArgoCDWithResources(opts ...argoCDOpt) *argoprojv1alpha1.ArgoCD { a := &argoprojv1alpha1.ArgoCD{ @@ -111,15 +137,12 @@ func makeTestArgoCDWithResources(opts ...argoCDOpt) *argoprojv1alpha1.ArgoCD { HA: argoprojv1alpha1.ArgoCDHASpec{ Resources: makeTestHAResources(), }, + Dex: argoprojv1alpha1.ArgoCDDexSpec{ + Resources: makeTestDexResources(), + }, Controller: argoprojv1alpha1.ArgoCDApplicationControllerSpec{ Resources: makeTestControllerResources(), }, - SSO: &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - Resources: makeTestDexResources(), - }, - }, }, } for _, o := range opts { diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index 060222d64..a2256503b 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -317,19 +317,16 @@ func getArgoControllerParellismLimit(cr *argoprojv1a1.ArgoCD) int32 { // common.ArgoCDDefaultDexImage. func getDexContainerImage(cr *argoprojv1a1.ArgoCD) string { defaultImg, defaultTag := false, false - img, tag := "", "" - if cr.Spec.SSO != nil && !reflect.DeepEqual(cr.Spec.SSO.Dex, argoprojv1a1.ArgoCDDexSpec{}) { - img = cr.Spec.SSO.Dex.Image - if img == "" { - img = common.ArgoCDDefaultDexImage - defaultImg = true - } + img := cr.Spec.Dex.Image + if img == "" { + img = common.ArgoCDDefaultDexImage + defaultImg = true + } - tag = cr.Spec.SSO.Dex.Version - if tag == "" { - tag = common.ArgoCDDefaultDexVersion - defaultTag = true - } + tag := cr.Spec.Dex.Version + if tag == "" { + tag = common.ArgoCDDefaultDexVersion + defaultTag = true } if e := os.Getenv(common.ArgoCDDexImageEnvName); e != "" && (defaultTag && defaultImg) { return e @@ -377,10 +374,8 @@ func getDexResources(cr *argoprojv1a1.ArgoCD) corev1.ResourceRequirements { resources := corev1.ResourceRequirements{} // Allow override of resource requirements from CR - if cr.Spec.SSO != nil && !reflect.DeepEqual(cr.Spec.SSO.Dex, argoprojv1a1.ArgoCDDexSpec{}) { - if cr.Spec.SSO.Dex.Resources != nil { - resources = *cr.Spec.SSO.Dex.Resources - } + if cr.Spec.Dex.Resources != nil { + resources = *cr.Spec.Dex.Resources } return resources @@ -435,7 +430,7 @@ func (r *ReconcileArgoCD) getOpenShiftDexConfig(cr *argoprojv1a1.ArgoCD) (string "clientSecret": *clientSecret, "redirectURI": r.getDexOAuthRedirectURI(cr), "insecureCA": true, // TODO: Configure for openshift CA, - "groups": cr.Spec.SSO.Dex.Groups, + "groups": cr.Spec.Dex.Groups, }, } @@ -896,7 +891,7 @@ func removeString(slice []string, s string) []string { } // setResourceWatches will register Watches for each of the supported Resources. -func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretMapper, namespaceResourceMapper handler.MapFunc) *builder.Builder { +func setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretMapper, namespaceResourceMapper handler.MapFunc) *builder.Builder { deploymentConfigPred := predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { @@ -940,21 +935,10 @@ func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResou if !ok { return false } - - // Handle deletion of SSO from Argo CD custom resource if !reflect.DeepEqual(oldCR.Spec.SSO, newCR.Spec.SSO) && newCR.Spec.SSO == nil { - err := r.deleteSSOConfiguration(newCR, oldCR.Spec.SSO) - if err != nil { - log.Error(err, fmt.Sprintf("Failed to delete existing SSO Configuration for ArgoCD %s in namespace %s", - newCR.Name, newCR.Namespace)) - } - } - - // Trigger reconciliation of SSO on update event - if !reflect.DeepEqual(oldCR.Spec.SSO, newCR.Spec.SSO) && newCR.Spec.SSO != nil && oldCR.Spec.SSO != nil { - err := r.reconcileSSO(newCR) + err := deleteSSOConfiguration(newCR) if err != nil { - log.Error(err, fmt.Sprintf("Failed to update existing SSO Configuration for ArgoCD %s in namespace %s", + log.Error(err, fmt.Sprintf("Failed to delete SSO Configuration for ArgoCD %s in namespace %s", newCR.Name, newCR.Namespace)) } } diff --git a/controllers/argocd/util_test.go b/controllers/argocd/util_test.go index 01c596675..de97bd0d2 100644 --- a/controllers/argocd/util_test.go +++ b/controllers/argocd/util_test.go @@ -39,28 +39,14 @@ var imageTests = []struct { name: "dex default configuration", imageFunc: getDexContainerImage, want: argoutil.CombineImageTag(common.ArgoCDDefaultDexImage, common.ArgoCDDefaultDexVersion), - opts: []argoCDOpt{func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }}, }, { name: "dex spec configuration", imageFunc: getDexContainerImage, want: dexTestImage, opts: []argoCDOpt{func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - Image: "testing/dex", - Version: "latest", - }, - } + a.Spec.Dex.Image = "testing/dex" + a.Spec.Dex.Version = "latest" }}, }, { @@ -74,14 +60,6 @@ var imageTests = []struct { }) os.Setenv(common.ArgoCDDexImageEnvName, dexTestImage) }, - opts: []argoCDOpt{func(a *argoprojv1alpha1.ArgoCD) { - a.Spec.SSO = &argoprojv1alpha1.ArgoCDSSOSpec{ - Provider: argoprojv1alpha1.SSOProviderTypeDex, - Dex: argoprojv1alpha1.ArgoCDDexSpec{ - OpenShiftOAuth: true, - }, - } - }}, }, { name: "argo default configuration", diff --git a/deploy/olm-catalog/argocd-operator/0.2.0/argocd-operator.v0.2.0.clusterserviceversion.yaml b/deploy/olm-catalog/argocd-operator/0.2.0/argocd-operator.v0.2.0.clusterserviceversion.yaml index 33c42f0a3..ad992955b 100644 --- a/deploy/olm-catalog/argocd-operator/0.2.0/argocd-operator.v0.2.0.clusterserviceversion.yaml +++ b/deploy/olm-catalog/argocd-operator/0.2.0/argocd-operator.v0.2.0.clusterserviceversion.yaml @@ -324,6 +324,38 @@ spec: x-descriptors: - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Controller - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: Config is the dex connector configuration. + displayName: Configuration + path: dex.config + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text + - description: Image is the Dex container image. + displayName: Image + path: dex.image + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text + - description: OpenShiftOAuth enables OpenShift OAuth authentication for the + Dex server. + displayName: OpenShift OAuth Enabled' + path: dex.openShiftOAuth + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:booleanSwitch + - description: Resources defines the Compute Resources required by the container + for Dex. + displayName: Resource Requirements' + path: dex.resources + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:resourceRequirements + - description: Version is the Dex container image tag. + displayName: Version + path: dex.version + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex + - urn:alm:descriptor:com.tectonic.ui:text - description: GAAnonymizeUsers toggles user IDs being hashed before sending to google analytics. displayName: Google Analytics Anonymize Users' @@ -617,38 +649,6 @@ spec: x-descriptors: - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Server - urn:alm:descriptor:com.tectonic.ui:text - - description: Config is the dex connector configuration. - displayName: Configuration - path: sso.dex.config - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - - description: Image is the Dex container image. - displayName: Image - path: sso.dex.image - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - - description: OpenShiftOAuth enables OpenShift OAuth authentication for the - Dex server. - displayName: OpenShift OAuth Enabled' - path: sso.dex.openShiftOAuth - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:booleanSwitch - - description: Resources defines the Compute Resources required by the container - for Dex. - displayName: Resource Requirements' - path: sso.dex.resources - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:resourceRequirements - - description: Version is the Dex container image tag. - displayName: Version - path: sso.dex.version - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:fieldGroup:Dex - - urn:alm:descriptor:com.tectonic.ui:text - description: StatusBadgeEnabled toggles application status badge feature. displayName: Status Badge Enabled' path: statusBadgeEnabled diff --git a/deploy/olm-catalog/argocd-operator/0.2.0/argoproj.io_argocds.yaml b/deploy/olm-catalog/argocd-operator/0.2.0/argoproj.io_argocds.yaml index e86842c87..5eca67043 100644 --- a/deploy/olm-catalog/argocd-operator/0.2.0/argoproj.io_argocds.yaml +++ b/deploy/olm-catalog/argocd-operator/0.2.0/argoproj.io_argocds.yaml @@ -277,6 +277,56 @@ spec: type: integer type: object type: object + dex: + description: Dex defines the Dex server options for ArgoCD. + properties: + config: + description: Config is the dex connector configuration. + type: string + groups: + description: Optional list of required groups a user must be a + member of + items: + type: string + type: array + image: + description: Image is the Dex container image. + type: string + openShiftOAuth: + description: OpenShiftOAuth enables OpenShift OAuth authentication + for the Dex server. + type: boolean + resources: + description: Resources defines the Compute Resources required + by the container for Dex. + properties: + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute + resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute + resources required. If Requests is omitted for a container, + it defaults to Limits if that is explicitly specified, otherwise + to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' + type: object + type: object + version: + description: Version is the Dex container image tag. + type: string + type: object disableAdmin: description: DisableAdmin will disable the admin user. type: boolean @@ -4132,103 +4182,45 @@ spec: description: SSO defines the Single Sign-on configuration for Argo CD properties: - dex: - description: Dex contains the configuration for Argo CD dex authentication - (previously found under cr.spec.Dex) + image: + description: Image is the SSO container image. + type: string + provider: + description: Provider installs and configures the given SSO Provider + with Argo CD. + type: string + resources: + description: Resources defines the Compute Resources required + by the container for SSO. properties: - config: - description: Config is the dex connector configuration. - type: string - groups: - description: Optional list of required groups a user must - be a member of - items: - type: string - type: array - image: - description: Image is the Dex container image. - type: string - openShiftOAuth: - description: OpenShiftOAuth enables OpenShift OAuth authentication - for the Dex server. - type: boolean - resources: - description: Resources defines the Compute Resources required - by the container for Dex. - properties: - limits: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Limits describes the maximum amount of compute - resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object - requests: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Requests describes the minimum amount of - compute resources required. If Requests is omitted for - a container, it defaults to Limits if that is explicitly - specified, otherwise to an implementation-defined value. - More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Limits describes the maximum amount of compute + resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' type: object - version: - description: Version is the Dex container image tag. - type: string - type: object - keycloak: - description: Keycloak contains the configuration for Argo CD keycloak - authentication (previously found under cr.spec.sso) - properties: - image: - description: Image is the Keycloak container image. - type: string - resources: - description: Resources defines the Compute Resources required - by the container for Keycloak. - properties: - limits: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Limits describes the maximum amount of compute - resources allowed. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object - requests: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'Requests describes the minimum amount of - compute resources required. If Requests is omitted for - a container, it defaults to Limits if that is explicitly - specified, otherwise to an implementation-defined value. - More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' - type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'Requests describes the minimum amount of compute + resources required. If Requests is omitted for a container, + it defaults to Limits if that is explicitly specified, otherwise + to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/' type: object - verifyTLS: - description: VerifyTLS set to false disables strict TLS validation. - type: boolean - version: - description: Version is the Keycloak container image tag. - type: string type: object - provider: - description: Provider installs and configures the given SSO Provider - with Argo CD. + verifyTLS: + description: VerifyTLS set to false disables strict TLS validation. + type: boolean + version: + description: Version is the SSO container image tag. type: string type: object statusBadgeEnabled: diff --git a/tests/e2e/argocd-tests/01-assert.yaml b/tests/e2e/argocd-tests/01-assert.yaml index a897fe14e..61fdd9cdc 100644 --- a/tests/e2e/argocd-tests/01-assert.yaml +++ b/tests/e2e/argocd-tests/01-assert.yaml @@ -20,6 +20,13 @@ status: --- apiVersion: apps/v1 kind: Deployment +metadata: + name: example-argocd-dex-server +status: + readyReplicas: 1 +--- +apiVersion: apps/v1 +kind: Deployment metadata: name: example-argocd-redis status: @@ -54,6 +61,11 @@ apiVersion: v1 metadata: name: example-argocd-argocd-redis-ha --- +kind: ServiceAccount +apiVersion: v1 +metadata: + name: example-argocd-argocd-dex-server +--- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: @@ -69,6 +81,11 @@ apiVersion: rbac.authorization.k8s.io/v1 metadata: name: example-argocd-argocd-redis-ha --- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: example-argocd-argocd-dex-server +--- apiVersion: apps/v1 kind: Deployment metadata: