diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 3c492fea32..5594e80b3d 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -87,8 +87,8 @@ "group": "build" }, { - "label": "Update csv bundle files", - "command": "./olm/update-nightly-bundle.sh", + "label": "Update OLM bundle files", + "command": "./olm/update-crd-files.sh && ./olm/update-nightly-bundle.sh", "type": "shell", "args": [], "problemMatcher": [ diff --git a/Dockerfile b/Dockerfile index 5ba1b6af3f..5e724a7ef7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -26,7 +26,7 @@ RUN export ARCH="$(uname -m)" && if [[ ${ARCH} == "x86_64" ]]; then export ARCH= GOOS=linux GOARCH=${ARCH} CGO_ENABLED=0 go build -mod=vendor -o /tmp/che-operator/che-operator cmd/manager/main.go # https://access.redhat.com/containers/?tab=tags#/registry.access.redhat.com/ubi8-minimal -FROM registry.access.redhat.com/ubi8-minimal:8.3-201 +FROM registry.access.redhat.com/ubi8-minimal:8.3-230 COPY --from=builder /tmp/che-operator/che-operator /usr/local/bin/che-operator COPY --from=builder /che-operator/templates/keycloak_provision /tmp/keycloak_provision diff --git a/README.md b/README.md index bdc1a4a656..c0ec08c5c1 100644 --- a/README.md +++ b/README.md @@ -326,7 +326,7 @@ Sometimes, during development, you need to modify some YAML definitions in the ` - Che cluster custom resource definition `pkg/apis/org/v1/che_types.go`. For example you want to fix some properties description or apply new Che type properties with default values. These changes affect CRD `deploy/crds/org_v1_che_crd.yaml`. - add Openshift ui annotations for Che types properties (`pkg/apis/org/v1/che_types.go`) to display information or interactive elements on the Openshift user interface. -For all these cases it's a necessary to generate a new OLM bundle to make these changes working with OLM. Run the VSCode tasks `Update csv bundle files` or use the terminal: +For all these cases it's a necessary to generate a new OLM bundle to make these changes working with OLM. Run the VSCode tasks `Update OLM bundle files` or use the terminal: ```bash $ olm/update-nightly-bundle.sh diff --git a/deploy/crds/org_v1_che_cr.yaml b/deploy/crds/org_v1_che_cr.yaml index 27a15510cc..5dbd12365c 100644 --- a/deploy/crds/org_v1_che_cr.yaml +++ b/deploy/crds/org_v1_che_cr.yaml @@ -124,9 +124,6 @@ spec: identityProviderRealm: '' # id of a keycloak client. This client will be created, when externalIdentityProvider is false, otherwise passed to Che server identityProviderClientId: '' - # instructs an Operator to enable OpenShift v3 identity provider in Keycloak, - # as well as create respective oAuthClient and configure Che configMap accordingly - openShiftoAuth: true # name of oAuthClient used in OpenShift v3 identity provider in Keycloak realm. Auto generated if left blank oAuthClientName: '' # secret used in oAuthClient. Auto generated if left blank diff --git a/deploy/crds/org_v1_che_crd.yaml b/deploy/crds/org_v1_che_crd.yaml index a81bf1c1e7..d28649f1b7 100644 --- a/deploy/crds/org_v1_che_crd.yaml +++ b/deploy/crds/org_v1_che_crd.yaml @@ -159,12 +159,15 @@ spec: type: string openShiftoAuth: description: 'Enables the integration of the identity provider (Keycloak - / RHSSO) with OpenShift OAuth. Enabled by default on OpenShift. - This will allow users to directly login with their Openshift user - through the Openshift login, and have their workspaces created - under personal OpenShift namespaces. WARNING: the `kubeadmin` - user is NOT supported, and logging through it will NOT allow accessing - the Che Dashboard.' + / RHSSO) with OpenShift OAuth. Empty value on the OpenShift platform + by default. If user changes this empty value to true/false, then + che-operator respect this value. Otherwise che-operator tries + to auto detect if Openshift oAuth can be enabled and change empty + value, correspondly to auto-detection result. This property allows + users to directly login with their Openshift user through the + Openshift login, and have their workspaces created under personal + OpenShift namespaces. WARNING: the `kubeadmin` user is NOT supported, + and logging through it will NOT allow accessing the Che Dashboard.' type: boolean updateAdminPassword: description: Forces the default `admin` Che user to update password diff --git a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml index 89e106b059..3654c936d1 100644 --- a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml @@ -84,13 +84,13 @@ metadata: categories: Developer Tools certified: "false" containerImage: quay.io/eclipse/che-operator:nightly - createdAt: "2020-12-14T15:43:45Z" + createdAt: "2020-12-17T10:09:13Z" description: A Kube-native development solution that delivers portable and collaborative developer workspaces. operatorframework.io/suggested-namespace: eclipse-che repository: https://github.com/eclipse/che-operator support: Eclipse Foundation - name: eclipse-che-preview-kubernetes.v7.23.0-48.nightly + name: eclipse-che-preview-kubernetes.v7.24.0-52.nightly namespace: placeholder spec: apiservicedefinitions: {} @@ -322,7 +322,7 @@ spec: - name: RELATED_IMAGE_che_tls_secrets_creation_job value: quay.io/eclipse/che-tls-secret-creator:alpine-d1ed4ad - name: RELATED_IMAGE_pvc_jobs - value: registry.access.redhat.com/ubi8-minimal:8.3-201 + value: registry.access.redhat.com/ubi8-minimal:8.3-230 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_keycloak @@ -494,4 +494,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.23.0-48.nightly + version: 7.24.0-52.nightly diff --git a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml index a81bf1c1e7..d28649f1b7 100644 --- a/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-kubernetes/manifests/org_v1_che_crd.yaml @@ -159,12 +159,15 @@ spec: type: string openShiftoAuth: description: 'Enables the integration of the identity provider (Keycloak - / RHSSO) with OpenShift OAuth. Enabled by default on OpenShift. - This will allow users to directly login with their Openshift user - through the Openshift login, and have their workspaces created - under personal OpenShift namespaces. WARNING: the `kubeadmin` - user is NOT supported, and logging through it will NOT allow accessing - the Che Dashboard.' + / RHSSO) with OpenShift OAuth. Empty value on the OpenShift platform + by default. If user changes this empty value to true/false, then + che-operator respect this value. Otherwise che-operator tries + to auto detect if Openshift oAuth can be enabled and change empty + value, correspondly to auto-detection result. This property allows + users to directly login with their Openshift user through the + Openshift login, and have their workspaces created under personal + OpenShift namespaces. WARNING: the `kubeadmin` user is NOT supported, + and logging through it will NOT allow accessing the Che Dashboard.' type: boolean updateAdminPassword: description: Forces the default `admin` Che user to update password diff --git a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml index 43a3d8efba..e2e8f6c7bb 100644 --- a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml @@ -20,8 +20,7 @@ metadata: "identityProviderRealm": "", "identityProviderURL": "", "oAuthClientName": "", - "oAuthSecret": "", - "openShiftoAuth": true + "oAuthSecret": "" }, "database": { "chePostgresDb": "", @@ -76,13 +75,13 @@ metadata: categories: Developer Tools, OpenShift Optional certified: "false" containerImage: quay.io/eclipse/che-operator:nightly - createdAt: "2020-12-14T15:43:55Z" + createdAt: "2020-12-17T10:09:20Z" description: A Kube-native development solution that delivers portable and collaborative developer workspaces in OpenShift. operatorframework.io/suggested-namespace: eclipse-che repository: https://github.com/eclipse/che-operator support: Eclipse Foundation - name: eclipse-che-preview-openshift.v7.23.0-48.nightly + name: eclipse-che-preview-openshift.v7.24.0-52.nightly namespace: placeholder spec: apiservicedefinitions: {} @@ -336,7 +335,7 @@ spec: - name: RELATED_IMAGE_devfile_registry value: quay.io/eclipse/che-devfile-registry:nightly - name: RELATED_IMAGE_pvc_jobs - value: registry.access.redhat.com/ubi8-minimal:8.3-201 + value: registry.access.redhat.com/ubi8-minimal:8.3-230 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_keycloak @@ -514,4 +513,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.23.0-48.nightly + version: 7.24.0-52.nightly diff --git a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml index 9c19122be0..8249516f19 100644 --- a/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml +++ b/deploy/olm-catalog/eclipse-che-preview-openshift/manifests/org_v1_che_crd.yaml @@ -160,12 +160,15 @@ spec: type: string openShiftoAuth: description: 'Enables the integration of the identity provider (Keycloak - / RHSSO) with OpenShift OAuth. Enabled by default on OpenShift. - This will allow users to directly login with their Openshift user - through the Openshift login, and have their workspaces created - under personal OpenShift namespaces. WARNING: the `kubeadmin` - user is NOT supported, and logging through it will NOT allow accessing - the Che Dashboard.' + / RHSSO) with OpenShift OAuth. Empty value on the OpenShift platform + by default. If user changes this empty value to true/false, then + che-operator respect this value. Otherwise che-operator tries + to auto detect if Openshift oAuth can be enabled and change empty + value, correspondly to auto-detection result. This property allows + users to directly login with their Openshift user through the + Openshift login, and have their workspaces created under personal + OpenShift namespaces. WARNING: the `kubeadmin` user is NOT supported, + and logging through it will NOT allow accessing the Che Dashboard.' type: boolean updateAdminPassword: description: Forces the default `admin` Che user to update password diff --git a/deploy/operator-local.yaml b/deploy/operator-local.yaml index 342689d710..6c92fcee1c 100644 --- a/deploy/operator-local.yaml +++ b/deploy/operator-local.yaml @@ -54,7 +54,7 @@ spec: - name: RELATED_IMAGE_che_tls_secrets_creation_job value: quay.io/eclipse/che-tls-secret-creator:alpine-d1ed4ad - name: RELATED_IMAGE_pvc_jobs - value: registry.access.redhat.com/ubi8-minimal:8.3-201 + value: registry.access.redhat.com/ubi8-minimal:8.3-230 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_keycloak diff --git a/deploy/operator.yaml b/deploy/operator.yaml index 00bf90d291..130d0f4ff5 100644 --- a/deploy/operator.yaml +++ b/deploy/operator.yaml @@ -53,7 +53,7 @@ spec: - name: RELATED_IMAGE_che_tls_secrets_creation_job value: quay.io/eclipse/che-tls-secret-creator:alpine-d1ed4ad - name: RELATED_IMAGE_pvc_jobs - value: registry.access.redhat.com/ubi8-minimal:8.3-201 + value: registry.access.redhat.com/ubi8-minimal:8.3-230 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_keycloak diff --git a/e2e/create.go b/e2e/create.go index 64fa08d732..8a3d495583 100644 --- a/e2e/create.go +++ b/e2e/create.go @@ -13,6 +13,7 @@ package main import ( orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" + "github.com/eclipse/che-operator/pkg/util" "github.com/sirupsen/logrus" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -122,6 +123,9 @@ func newCheCluster() (cr *orgv1.CheCluster) { Server: orgv1.CheClusterSpecServer{ UseInternalClusterSVCNames: true, }, + Auth: orgv1.CheClusterSpecAuth{ + OpenShiftoAuth: util.NewBoolPointer(true), + }, }, } return cr diff --git a/pkg/apis/org/v1/che_types.go b/pkg/apis/org/v1/che_types.go index 4382782c68..0b4ca090f2 100644 --- a/pkg/apis/org/v1/che_types.go +++ b/pkg/apis/org/v1/che_types.go @@ -405,12 +405,16 @@ type CheClusterSpecAuth struct { // Forces the default `admin` Che user to update password on first login. Defaults to `false`. // +optional UpdateAdminPassword bool `json:"updateAdminPassword"` - // Enables the integration of the identity provider (Keycloak / RHSSO) with OpenShift OAuth. Enabled by default on OpenShift. - // This will allow users to directly login with their Openshift user through the Openshift login, + // Enables the integration of the identity provider (Keycloak / RHSSO) with OpenShift OAuth. + // Empty value on the OpenShift platform by default. + // If user changes this empty value to true/false, then che-operator respect this value. + // Otherwise che-operator tries to auto detect if Openshift oAuth can be enabled and change empty value, + // correspondly to auto-detection result. + // This property allows users to directly login with their Openshift user through the Openshift login, // and have their workspaces created under personal OpenShift namespaces. // WARNING: the `kubeadmin` user is NOT supported, and logging through it will NOT allow accessing the Che Dashboard. // +optional - OpenShiftoAuth bool `json:"openShiftoAuth"` + OpenShiftoAuth *bool `json:"openShiftoAuth"` // Name of the OpenShift `OAuthClient` resource used to setup identity federation on the OpenShift side. Auto-generated if left blank. // See also the `OpenShiftoAuth` field. // +optional diff --git a/pkg/apis/org/v1/zz_generated.deepcopy.go b/pkg/apis/org/v1/zz_generated.deepcopy.go index a3f514ad75..67e5208527 100644 --- a/pkg/apis/org/v1/zz_generated.deepcopy.go +++ b/pkg/apis/org/v1/zz_generated.deepcopy.go @@ -75,7 +75,7 @@ func (in *CheClusterSpec) DeepCopyInto(out *CheClusterSpec) { *out = *in in.Server.DeepCopyInto(&out.Server) out.Database = in.Database - out.Auth = in.Auth + in.Auth.DeepCopyInto(&out.Auth) out.Storage = in.Storage out.Metrics = in.Metrics out.K8s = in.K8s @@ -96,6 +96,11 @@ func (in *CheClusterSpec) DeepCopy() *CheClusterSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CheClusterSpecAuth) DeepCopyInto(out *CheClusterSpecAuth) { *out = *in + if in.OpenShiftoAuth != nil { + in, out := &in.OpenShiftoAuth, &out.OpenShiftoAuth + *out = new(bool) + **out = **in + } out.IdentityProviderIngress = in.IdentityProviderIngress out.IdentityProviderRoute = in.IdentityProviderRoute return diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 3207b63d9e..f682494eb3 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -340,55 +340,21 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e host, err := getDefaultCheHost(deployContext) if host == "" { return reconcile.Result{RequeueAfter: 1 * time.Second}, err - } else { - deployContext.DefaultCheHost = host } + deployContext.DefaultCheHost = host } } - if isOpenShift && instance.Spec.Auth.OpenShiftoAuth { - if isOpenShift4 { - oauthv1 := &oauthv1.OAuth{} - if err := r.nonCachedClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, oauthv1); err != nil { - getOAuthV1ErrMsg := failedUnableToGetOAuth + " Cause: " + err.Error() - logrus.Errorf(getOAuthV1ErrMsg) - if err := r.SetStatusDetails(instance, request, failedNoOpenshiftUserReason, getOAuthV1ErrMsg, ""); err != nil { - return reconcile.Result{}, err - } - return reconcile.Result{}, err - } - if len(oauthv1.Spec.IdentityProviders) < 1 { - logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4) - instance.Spec.Auth.OpenShiftoAuth = false - if err := r.UpdateCheCRSpec(instance, "OpenShiftoAuth", strconv.FormatBool(false)); err != nil { - return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err - } - } - } else { - users := &userv1.UserList{} - listOptions := &client.ListOptions{} - if err := r.nonCachedClient.List(context.TODO(), users, listOptions); err != nil { - getUsersErrMsg := failedUnableToGetOpenshiftUsers + " Cause: " + err.Error() - logrus.Errorf(getUsersErrMsg) - if err := r.SetStatusDetails(instance, request, failedNoOpenshiftUserReason, getUsersErrMsg, ""); err != nil { - return reconcile.Result{}, err - } - return reconcile.Result{}, err - } - if len(users.Items) < 1 { - logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3) - instance.Spec.Auth.OpenShiftoAuth = false - if err := r.UpdateCheCRSpec(instance, "OpenShiftoAuth", strconv.FormatBool(false)); err != nil { - return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err - } - } + if isOpenShift && instance.Spec.Auth.OpenShiftoAuth == nil { + if reconcileResult, err := r.autoEnableOAuth(instance, request, isOpenShift4); err != nil { + return reconcileResult, err } + } - // delete oAuthClient before CR is deleted - if instance.Spec.Auth.OpenShiftoAuth { - if err := r.ReconcileFinalizer(instance); err != nil { - return reconcile.Result{}, err - } + // delete oAuthClient before CR is deleted + if util.IsOAuthEnabled(instance) { + if err := r.ReconcileFinalizer(instance); err != nil { + return reconcile.Result{}, err } } @@ -431,7 +397,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e // To use Openshift v4 OAuth, the OAuth endpoints are served from a namespace // and NOT from the Openshift API Master URL (as in v3) // So we also need the self-signed certificate to access them (same as the Che server) - (isOpenShift4 && instance.Spec.Auth.OpenShiftoAuth && !instance.Spec.Server.TlsSupport) { + (isOpenShift4 && util.IsOAuthEnabled(instance) && !instance.Spec.Server.TlsSupport) { if err := deploy.CreateTLSSecretFromEndpoint(deployContext, "", deploy.CheTLSSelfSignedCertificateSecretName); err != nil { return reconcile.Result{}, err } @@ -447,7 +413,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } } - if instance.Spec.Auth.OpenShiftoAuth { + if util.IsOAuthEnabled(instance) { // create a secret with OpenShift API crt to be added to keystore that RH SSO will consume baseURL, err := util.GetClusterPublicHostname(isOpenShift4) if err != nil { @@ -1123,3 +1089,50 @@ func isTrustedBundleConfigMap(mgr manager.Manager, obj handler.MapObject) (bool, }, } } + +func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.Request, isOpenShift4 bool) (reconcile.Result, error) { + var message, reason string + oauth := false + if isOpenShift4 { + oauthv1 := &oauthv1.OAuth{} + if err := r.nonCachedClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, oauthv1); err != nil { + getOAuthV1ErrMsg := failedUnableToGetOAuth + " Cause: " + err.Error() + logrus.Errorf(getOAuthV1ErrMsg) + message = getOAuthV1ErrMsg + reason = failedNoOpenshiftUserReason + } else { + oauth = len(oauthv1.Spec.IdentityProviders) >= 1 + if !oauth { + logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4) + } + } + // openshift 3 + } else { + users := &userv1.UserList{} + listOptions := &client.ListOptions{} + if err := r.nonCachedClient.List(context.TODO(), users, listOptions); err != nil { + getUsersErrMsg := failedUnableToGetOpenshiftUsers + " Cause: " + err.Error() + logrus.Errorf(getUsersErrMsg) + message = getUsersErrMsg + reason = failedNoOpenshiftUserReason + } else { + oauth = len(users.Items) >= 1 + if !oauth { + logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3) + } + } + } + + cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(oauth) + if err := r.UpdateCheCRSpec(cr, "OpenShiftoAuth", strconv.FormatBool(oauth)); err != nil { + return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err + } + + if message != "" && reason != "" { + if err := r.SetStatusDetails(cr, request, message, reason, ""); err != nil { + return reconcile.Result{}, err + } + } + + return reconcile.Result{}, nil +} diff --git a/pkg/controller/che/che_controller_test.go b/pkg/controller/che/che_controller_test.go index cc4c0cc282..9872f738d9 100644 --- a/pkg/controller/che/che_controller_test.go +++ b/pkg/controller/che/che_controller_test.go @@ -29,6 +29,7 @@ import ( orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1" "github.com/eclipse/che-operator/pkg/util" + oauth_config "github.com/openshift/api/config/v1" oauth "github.com/openshift/api/oauth/v1" routev1 "github.com/openshift/api/route/v1" userv1 "github.com/openshift/api/user/v1" @@ -146,6 +147,40 @@ var ( Name: csvName, }, } + nonEmptyUserList = &userv1.UserList{ + Items: []userv1.User{ + userv1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user1", + }, + }, + userv1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user2", + }, + }, + }, + } + oAuthClient = &oauth.OAuthClient{} + oAuthWithNoIdentityProviders = &oauth_config.OAuth{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Namespace: namespace, + }, + } + oAuthWithIdentityProvider = &oauth_config.OAuth{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Namespace: namespace, + }, + Spec: oauth_config.OAuthSpec{ + IdentityProviders: []oauth_config.IdentityProvider{ + { + Name: "htpasswd", + }, + }, + }, + } ) func init() { @@ -159,6 +194,199 @@ func init() { } } +func TestCaseAutoDetectOAuth(t *testing.T) { + type testCase struct { + name string + initObjects []runtime.Object + openshiftVersion string + initialOAuthValue *bool + oAuthExpected *bool + } + + testCases := []testCase{ + { + name: "che-operator should auto enable oAuth when Che CR with oAuth nil value on the Openshift 3 with users > 0", + initObjects: []runtime.Object{ + nonEmptyUserList, + &oauth.OAuthClient{}, + }, + openshiftVersion: "3", + initialOAuthValue: nil, + oAuthExpected: util.NewBoolPointer(true), + }, + { + name: "che-operator should auto disable oAuth when Che CR with nil oAuth on the Openshift 3 with no users", + initObjects: []runtime.Object{ + &userv1.UserList{}, + &oauth.OAuthClient{}, + }, + openshiftVersion: "3", + initialOAuthValue: util.NewBoolPointer(false), + oAuthExpected: util.NewBoolPointer(false), + }, + { + name: "che-operator should respect oAuth = true even if there no users on the Openshift 3", + initObjects: []runtime.Object{ + &userv1.UserList{}, + &oauth.OAuthClient{}, + }, + openshiftVersion: "3", + initialOAuthValue: util.NewBoolPointer(true), + oAuthExpected: util.NewBoolPointer(true), + }, + { + name: "che-operator should respect oAuth = true even if there are some users on the Openshift 3", + initObjects: []runtime.Object{ + nonEmptyUserList, + &oauth.OAuthClient{}, + }, + openshiftVersion: "3", + initialOAuthValue: util.NewBoolPointer(true), + oAuthExpected: util.NewBoolPointer(true), + }, + { + name: "che-operator should respect oAuth = false even if there are some users on the Openshift 3", + initObjects: []runtime.Object{ + nonEmptyUserList, + &oauth.OAuthClient{}, + }, + openshiftVersion: "3", + initialOAuthValue: util.NewBoolPointer(false), + oAuthExpected: util.NewBoolPointer(false), + }, + { + name: "che-operator should respect oAuth = false even if no users on the Openshift 3", + initObjects: []runtime.Object{ + &userv1.UserList{}, + &oauth.OAuthClient{}, + }, + openshiftVersion: "3", + initialOAuthValue: util.NewBoolPointer(false), + oAuthExpected: util.NewBoolPointer(false), + }, + { + name: "che-operator should auto enable oAuth when Che CR with nil value on the Openshift 4 with identity providers", + initObjects: []runtime.Object{ + oAuthWithIdentityProvider, + }, + openshiftVersion: "4", + initialOAuthValue: nil, + oAuthExpected: util.NewBoolPointer(true), + }, + { + name: "che-operator should auto enable oAuth when Che CR with nil value on the Openshift 4 with identity providers", + initObjects: []runtime.Object{ + oAuthWithNoIdentityProviders, + }, + openshiftVersion: "4", + initialOAuthValue: nil, + oAuthExpected: util.NewBoolPointer(false), + }, + { + name: "che-operator should respect oAuth = true even if there no indentity providers on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithNoIdentityProviders, + }, + openshiftVersion: "4", + initialOAuthValue: util.NewBoolPointer(true), + oAuthExpected: util.NewBoolPointer(true), + }, + { + name: "che-operator should respect oAuth = true even if there are some users on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithIdentityProvider, + }, + openshiftVersion: "4", + initialOAuthValue: util.NewBoolPointer(true), + oAuthExpected: util.NewBoolPointer(true), + }, + + { + name: "che-operator should respect oAuth = false even if there no indentity providers on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithNoIdentityProviders, + }, + openshiftVersion: "4", + initialOAuthValue: util.NewBoolPointer(false), + oAuthExpected: util.NewBoolPointer(false), + }, + { + name: "che-operator should respect oAuth = false even if there are some users on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithIdentityProvider, + }, + openshiftVersion: "4", + initialOAuthValue: util.NewBoolPointer(false), + oAuthExpected: util.NewBoolPointer(false), + }, + { + name: "che-operator should auto disable oAuth on error retieve identity providers", + initObjects: []runtime.Object{}, + openshiftVersion: "4", + initialOAuthValue: nil, + oAuthExpected: util.NewBoolPointer(false), + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + logf.SetLogger(zap.LoggerTo(os.Stdout, true)) + + scheme := scheme.Scheme + orgv1.SchemeBuilder.AddToScheme(scheme) + scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuthClient) + scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.UserList{}, &userv1.User{}) + scheme.AddKnownTypes(oauth_config.SchemeGroupVersion, &oauth_config.OAuth{}) + + initCR := InitCheWithSimpleCR() + initCR.Spec.Auth.OpenShiftoAuth = testCase.initialOAuthValue + testCase.initObjects = append(testCase.initObjects, initCR) + + cli := fake.NewFakeClientWithScheme(scheme, testCase.initObjects...) + nonCachedClient := fake.NewFakeClientWithScheme(scheme, testCase.initObjects...) + clientSet := fakeclientset.NewSimpleClientset() + fakeDiscovery, ok := clientSet.Discovery().(*fakeDiscovery.FakeDiscovery) + fakeDiscovery.Fake.Resources = []*metav1.APIResourceList{} + + if !ok { + t.Fatal("Error creating fake discovery client") + } + + r := &ReconcileChe{ + client: cli, + nonCachedClient: nonCachedClient, + discoveryClient: fakeDiscovery, + scheme: scheme, + } + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + os.Setenv("OPENSHIFT_VERSION", testCase.openshiftVersion) + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("Error reconciling: %v", err) + } + + cheCR := &orgv1.CheCluster{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + if cheCR.Spec.Auth.OpenShiftoAuth == nil { + t.Error("OAuth should not stay with nil value.") + } + + if *cheCR.Spec.Auth.OpenShiftoAuth != *testCase.oAuthExpected { + t.Errorf("Openshift oAuth should be %t", *testCase.oAuthExpected) + } + }) + } +} + func TestImagePullerConfiguration(t *testing.T) { type testCase struct { name string @@ -432,6 +660,7 @@ func TestImagePullerConfiguration(t *testing.T) { } func TestCheController(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) @@ -531,7 +760,7 @@ func TestCheController(t *testing.T) { } // update CR and make sure Che configmap has been updated - cheCR.Spec.Auth.OpenShiftoAuth = true + cheCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(true) if err := cl.Update(context.TODO(), cheCR); err != nil { t.Error("Failed to update CheCluster custom resource") } @@ -653,6 +882,7 @@ func TestCheController(t *testing.T) { } func TestConfiguringLabelsForRoutes(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) @@ -705,6 +935,8 @@ func TestConfiguringLabelsForRoutes(t *testing.T) { } func TestConfiguringInternalNetworkTest(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") + // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) @@ -856,6 +1088,21 @@ func TestConfiguringInternalNetworkTest(t *testing.T) { } func Init() (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { + objs, ds, scheme := createAPIObjects() + + oAuthClient := &oauth.OAuthClient{} + users := &userv1.UserList{} + user := &userv1.User{} + + // Register operator types with the runtime scheme + scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuthClient) + scheme.AddKnownTypes(userv1.SchemeGroupVersion, users, user) + + // Create a fake client to mock API calls + return fake.NewFakeClient(objs...), ds, scheme +} + +func createAPIObjects() ([]runtime.Object, discovery.DiscoveryInterface, runtime.Scheme) { pgPod := &corev1.Pod{ TypeMeta: metav1.TypeMeta{ Kind: "Pod", @@ -871,33 +1118,7 @@ func Init() (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { } // A CheCluster custom resource with metadata and spec - cheCR := &orgv1.CheCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - }, - Spec: orgv1.CheClusterSpec{ - // todo add some spec to check controller ifs like external db, ssl etc - Server: orgv1.CheClusterSpecServer{ - CheWorkspaceClusterRole: "cluster-admin", - }, - }, - } - - userList := &userv1.UserList{ - Items: []userv1.User{ - userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "user1", - }, - }, - userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "user2", - }, - }, - }, - } + cheCR := InitCheWithSimpleCR() route := &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ @@ -915,19 +1136,13 @@ func Init() (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { // Objects to track in the fake client. objs := []runtime.Object{ - cheCR, pgPod, userList, route, packageManifest, + cheCR, pgPod, route, packageManifest, } - oAuthClient := &oauth.OAuthClient{} - users := &userv1.UserList{} - user := &userv1.User{} - // Register operator types with the runtime scheme scheme := scheme.Scheme scheme.AddKnownTypes(orgv1.SchemeGroupVersion, cheCR) scheme.AddKnownTypes(routev1.SchemeGroupVersion, route) - scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuthClient) - scheme.AddKnownTypes(userv1.SchemeGroupVersion, users, user) scheme.AddKnownTypes(console.GroupVersion, &console.ConsoleLink{}) chev1alpha1.AddToScheme(scheme) packagesv1.AddToScheme(scheme) @@ -942,7 +1157,22 @@ func Init() (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { } // Create a fake client to mock API calls - return fake.NewFakeClient(objs...), fakeDiscovery, *scheme + return objs, fakeDiscovery, *scheme +} + +func InitCheWithSimpleCR() *orgv1.CheCluster { + return &orgv1.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: orgv1.CheClusterSpec{ + // todo add some spec to check controller ifs like external db, ssl etc + Server: orgv1.CheClusterSpecServer{ + CheWorkspaceClusterRole: "cluster-admin", + }, + }, + } } func InitCheCRWithImagePullerEnabled() *orgv1.CheCluster { diff --git a/pkg/controller/che/update.go b/pkg/controller/che/update.go index 2d131b5628..deb8f9fd0c 100644 --- a/pkg/controller/che/update.go +++ b/pkg/controller/che/update.go @@ -46,7 +46,7 @@ func (r *ReconcileChe) UpdateCheCRSpec(instance *orgv1.CheCluster, updatedField } func (r *ReconcileChe) ReconcileIdentityProvider(instance *orgv1.CheCluster, isOpenShift4 bool) (deleted bool, err error) { - if instance.Spec.Auth.OpenShiftoAuth == false && instance.Status.OpenShiftoAuthProvisioned == true { + if !util.IsOAuthEnabled(instance) && instance.Status.OpenShiftoAuthProvisioned == true { keycloakDeployment := &appsv1.Deployment{} if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "keycloak", Namespace: instance.Namespace}, keycloakDeployment); err != nil { logrus.Errorf("Deployment %s not found: %s", keycloakDeployment.Name, err) diff --git a/pkg/deploy/identity-provider/identity_provider.go b/pkg/deploy/identity-provider/identity_provider.go index 5ad181b826..35fb84802b 100644 --- a/pkg/deploy/identity-provider/identity_provider.go +++ b/pkg/deploy/identity-provider/identity_provider.go @@ -127,12 +127,9 @@ func SyncIdentityProviderToCluster(deployContext *deploy.DeployContext) (bool, e } } - if isOpenShift { - doInstallOpenShiftoAuthProvider := instance.Spec.Auth.OpenShiftoAuth - if doInstallOpenShiftoAuthProvider { - if err := SyncIdentityProviderItems(deployContext, cheFlavor); err != nil { - return false, err - } + if isOpenShift && util.IsOAuthEnabled(instance) { + if err := SyncIdentityProviderItems(deployContext, cheFlavor); err != nil { + return false, err } } diff --git a/pkg/deploy/server/che_configmap.go b/pkg/deploy/server/che_configmap.go index 5139b584b7..2314c24b2f 100644 --- a/pkg/deploy/server/che_configmap.go +++ b/pkg/deploy/server/che_configmap.go @@ -115,9 +115,8 @@ func GetCheConfigMapData(deployContext *deploy.DeployContext) (cheEnv map[string } tls := "false" openShiftIdentityProviderId := "NULL" - openshiftOAuth := deployContext.CheCluster.Spec.Auth.OpenShiftoAuth defaultTargetNamespaceDefault := deployContext.CheCluster.Namespace // By default Che SA has right in the namespace where Che in installed ... - if openshiftOAuth && isOpenShift { + if isOpenShift && util.IsOAuthEnabled(deployContext.CheCluster) { // ... But if the workspace is created under the openshift identity of the end-user, // Then we'll have rights to create any new namespace defaultTargetNamespaceDefault = "-" + cheFlavor diff --git a/pkg/deploy/server/che_configmap_test.go b/pkg/deploy/server/che_configmap_test.go index a89716b71c..05defdd59e 100644 --- a/pkg/deploy/server/che_configmap_test.go +++ b/pkg/deploy/server/che_configmap_test.go @@ -28,7 +28,7 @@ func TestNewCheConfigMap(t *testing.T) { cr := &orgv1.CheCluster{} cr.Spec.Server.CheHost = "myhostname.com" cr.Spec.Server.TlsSupport = true - cr.Spec.Auth.OpenShiftoAuth = true + cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(true) deployContext := &deploy.DeployContext{ CheCluster: cr, Proxy: &deploy.Proxy{}, @@ -58,7 +58,7 @@ func TestConfigMapOverride(t *testing.T) { cr.Spec.Server.CustomCheProperties = map[string]string{ "CHE_WORKSPACE_NO_PROXY": "myproxy.myhostname.com", } - cr.Spec.Auth.OpenShiftoAuth = true + cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(true) deployContext := &deploy.DeployContext{ CheCluster: cr, Proxy: &deploy.Proxy{}, diff --git a/pkg/util/util.go b/pkg/util/util.go index 795b2bf1a8..9b6072f312 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -24,6 +24,7 @@ import ( "regexp" "runtime" "sort" + "strconv" "strings" "time" @@ -92,22 +93,29 @@ func MapToKeyValuePairs(m map[string]string) string { func DetectOpenShift() (isOpenshift bool, isOpenshift4 bool, anError error) { tests := IsTestMode() - if !tests { - apiGroups, err := getApiList() - if err != nil { - return false, false, err + if tests { + openshiftVersionEnv := os.Getenv("OPENSHIFT_VERSION") + openshiftVersion, err := strconv.ParseInt(openshiftVersionEnv, 0, 64) + if err == nil && openshiftVersion == 4 { + return true, true, nil } - for _, apiGroup := range apiGroups { - if apiGroup.Name == "route.openshift.io" { - isOpenshift = true - } - if apiGroup.Name == "config.openshift.io" { - isOpenshift4 = true - } + return true, false, nil + } + + apiGroups, err := getApiList() + if err != nil { + return false, false, err + } + for _, apiGroup := range apiGroups { + if apiGroup.Name == "route.openshift.io" { + isOpenshift = true + } + if apiGroup.Name == "config.openshift.io" { + isOpenshift4 = true } - return } - return true, false, nil + + return isOpenshift, isOpenshift4, nil } func getDiscoveryClient() (*discovery.DiscoveryClient, error) { @@ -330,3 +338,18 @@ func GetArchitectureDependentEnv(env string) string { return env } + +// NewBoolPointer returns `bool` pointer to value in the memory. +// Unfortunately golang hasn't got syntax to create `bool` pointer. +func NewBoolPointer(value bool) *bool { + variable := value + return &variable +} + +// IsOAuthEnabled return true when oAuth is enable for CheCluster resource, otherwise false. +func IsOAuthEnabled(c *orgv1.CheCluster) bool { + if c.Spec.Auth.OpenShiftoAuth != nil && *c.Spec.Auth.OpenShiftoAuth { + return true + } + return false +}