From f02c945f0d79f9e3edd1893e3fa1323d980521b2 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 3 Dec 2020 22:45:26 +0200 Subject: [PATCH 01/14] Improve oauth handling. Signed-off-by: Oleksandr Andriienko --- deploy/crds/org_v1_che_cr.yaml | 3 - pkg/apis/org/v1/che_types.go | 2 +- pkg/apis/org/v1/zz_generated.deepcopy.go | 7 +- pkg/controller/che/che_controller.go | 103 ++++++++++-------- pkg/controller/che/che_controller_test.go | 2 +- pkg/controller/che/update.go | 2 +- .../identity-provider/identity_provider.go | 9 +- pkg/deploy/server/che_configmap.go | 2 +- pkg/deploy/server/che_configmap_test.go | 4 +- pkg/util/util.go | 7 ++ 10 files changed, 80 insertions(+), 61 deletions(-) diff --git a/deploy/crds/org_v1_che_cr.yaml b/deploy/crds/org_v1_che_cr.yaml index 5f313d4708..934e630ad3 100644 --- a/deploy/crds/org_v1_che_cr.yaml +++ b/deploy/crds/org_v1_che_cr.yaml @@ -125,9 +125,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/pkg/apis/org/v1/che_types.go b/pkg/apis/org/v1/che_types.go index 0c84b2327d..79875acfa2 100644 --- a/pkg/apis/org/v1/che_types.go +++ b/pkg/apis/org/v1/che_types.go @@ -406,7 +406,7 @@ type CheClusterSpecAuth struct { // 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 a73dbe4bbc..16cddc5c01 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 @@ -95,6 +95,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 801aef2476..8a2a0e15af 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -319,55 +319,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 instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth { + if err := r.ReconcileFinalizer(instance); err != nil { + return reconcile.Result{}, err } } @@ -409,7 +375,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 && instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth && !instance.Spec.Server.TlsSupport) { if err := deploy.CreateTLSSecretFromEndpoint(deployContext, "", deploy.CheTLSSelfSignedCertificateSecretName); err != nil { return reconcile.Result{}, err } @@ -425,7 +391,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } } - if instance.Spec.Auth.OpenShiftoAuth { + if instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth { // 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 { @@ -1101,3 +1067,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 + 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 + cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + } else { + cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(len(oauthv1.Spec.IdentityProviders) >= 1) + if !*cr.Spec.Auth.OpenShiftoAuth { + 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 + cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + } + + cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(len(users.Items) >= 1) + if !*cr.Spec.Auth.OpenShiftoAuth { + logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3) + } + } + + if err := r.UpdateCheCRSpec(cr, "OpenShiftoAuth", strconv.FormatBool(*cr.Spec.Auth.OpenShiftoAuth)); 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 f35d357d2e..f9aa25cc61 100644 --- a/pkg/controller/che/che_controller_test.go +++ b/pkg/controller/che/che_controller_test.go @@ -162,7 +162,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.GetBoolPointer(true) if err := cl.Update(context.TODO(), cheCR); err != nil { t.Error("Failed to update CheCluster custom resource") } diff --git a/pkg/controller/che/update.go b/pkg/controller/che/update.go index 2d131b5628..d5e0f2d6e2 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 instance.Spec.Auth.OpenShiftoAuth != nil && !*instance.Spec.Auth.OpenShiftoAuth && 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..ccfeb1f628 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 && instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth { + 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 7e4a94be45..44c143dea4 100644 --- a/pkg/deploy/server/che_configmap.go +++ b/pkg/deploy/server/che_configmap.go @@ -117,7 +117,7 @@ func GetCheConfigMapData(deployContext *deploy.DeployContext) (cheEnv map[string 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 openshiftOAuth != nil && *openshiftOAuth && isOpenShift { // ... 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..beb123afdb 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 = 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 = true deployContext := &deploy.DeployContext{ CheCluster: cr, Proxy: &deploy.Proxy{}, diff --git a/pkg/util/util.go b/pkg/util/util.go index 795b2bf1a8..b291dbd798 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -330,3 +330,10 @@ func GetArchitectureDependentEnv(env string) string { return env } + +// GetBoolPointer returns `bool` pointer to value in the memory. +// Unfortunately golang hasn't got syntax to create `bool` pointer. +func GetBoolPointer(value bool) *bool { + variable := value + return &variable +} From 1db9db2873478f4f01401f84dee9cecfe7dc21e9 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Fri, 11 Dec 2020 23:15:37 +0200 Subject: [PATCH 02/14] Cover code by tests. Signed-off-by: Oleksandr Andriienko --- pkg/controller/che/che_controller.go | 6 +- pkg/controller/che/che_controller_test.go | 473 ++++++++++++++++++++-- pkg/deploy/server/che_configmap_test.go | 4 +- pkg/util/util.go | 34 +- 4 files changed, 472 insertions(+), 45 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 8a2a0e15af..9f60c4c0ae 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -293,8 +293,8 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } deployContext := &deploy.DeployContext{ - ClusterAPI: clusterAPI, - CheCluster: instance, + ClusterAPI: clusterAPI, + CheCluster: instance, InternalService: deploy.InternalService{}, } @@ -1084,7 +1084,7 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4) } } - // openshift 3 + // openshift 3 } else { users := &userv1.UserList{} listOptions := &client.ListOptions{} diff --git a/pkg/controller/che/che_controller_test.go b/pkg/controller/che/che_controller_test.go index f9aa25cc61..42aa294bf2 100644 --- a/pkg/controller/che/che_controller_test.go +++ b/pkg/controller/che/che_controller_test.go @@ -25,6 +25,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" @@ -63,10 +64,11 @@ func init() { } func TestCheController(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) - cl, scheme := Init() + cl, scheme := CreateOpenshift3Client(true) // Create a ReconcileChe object with the scheme and fake client r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} @@ -284,10 +286,11 @@ 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)) - cl, scheme := Init() + cl, scheme := CreateOpenshift3Client(true) // Create a ReconcileChe object with the scheme and fake client r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} @@ -336,10 +339,12 @@ 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)) - cl, scheme := Init() + cl, scheme := CreateOpenshift3Client(true) // Create a ReconcileChe object with the scheme and fake client r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} @@ -486,7 +491,442 @@ func TestConfiguringInternalNetworkTest(t *testing.T) { } } -func Init() (client.Client, runtime.Scheme) { +func TestAutoEnableOAuthForOpenshift4(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "4") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + cl, scheme := CreateOpenshift4Client(true) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + if cheCR.Spec.Auth.OpenShiftoAuth != nil { + t.Fatal("Openshift oAuth should not be set up by default") + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(true) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be enabled") + } +} + +func TestAutoEnableOAuthForOpenshift3(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + cl, scheme := CreateOpenshift3Client(true) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + if cheCR.Spec.Auth.OpenShiftoAuth != nil { + t.Fatal("Openshift oAuth should not be set up by default") + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(true) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be enabled") + } +} + +func TestAutoDisableOAuthForOpenshift3(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + cl, scheme := CreateOpenshift3Client(false) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + if cheCR.Spec.Auth.OpenShiftoAuth != nil { + t.Fatal("Openshift oAuth should not be set up by default") + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(false) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be enabled") + } +} + +func TestRespectEnablingOAuthFromUserOpenshift3(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + cl, scheme := CreateOpenshift3Client(false) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // update CR + cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(true) + if err := cl.Update(context.TODO(), cheCR); err != nil { + t.Error("Failed to update CheCluster custom resource") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(true) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be enabled") + } +} + +func TestRespectDisablingOAuthFromUserOpenshift3(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "3") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + cl, scheme := CreateOpenshift3Client(true) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // update CR + cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + if err := cl.Update(context.TODO(), cheCR); err != nil { + t.Error("Failed to update CheCluster custom resource") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(false) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be enabled") + } +} + +func TestAutoDisableOAuthForOpenshift4(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "4") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + cl, scheme := CreateOpenshift4Client(false) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(false) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be disabled") + } +} + +func TestRespectEnablingOAuthFromUserOpenshift4(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "4") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + // Create fake client which returns no identity providers. + cl, scheme := CreateOpenshift4Client(false) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // update CR + cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(true) + if err := cl.Update(context.TODO(), cheCR); err != nil { + t.Error("Failed to update CheCluster custom resource") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(true) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be enabled") + } +} + +func TestRespectDisablingOAuthFromUserOpenshift4(t *testing.T) { + os.Setenv("OPENSHIFT_VERSION", "4") + + // Set the logger to development mode for verbose logs. + logf.SetLogger(logf.ZapLogger(true)) + + // Create fake client which returns non zero quantity identity providers. + cl, scheme := CreateOpenshift4Client(true) + + // Create a ReconcileChe object with the scheme and fake client + r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, tests: true} + + // get CR + cheCR := &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + // update CR + cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + if err := cl.Update(context.TODO(), cheCR); err != nil { + t.Error("Failed to update CheCluster custom resource") + } + + // Mock request to simulate Reconcile() being called on an event for a + // watched resource . + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + + _, err := r.Reconcile(req) + if err != nil { + t.Fatalf("reconcile: (%v)", err) + } + + cheCR = &orgv1.CheCluster{} + if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { + t.Errorf("CR not found") + } + + value := util.GetBoolPointer(false) + if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { + t.Errorf("Openshift oAuth should be enabled") + } +} + +func CreateOpenshift4Client(withUsers bool) (client.Client, runtime.Scheme) { + objs, scheme := createAPIObjects() + + oAuth := &oauth_config.OAuth{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Namespace: namespace, + }, + Spec: oauth_config.OAuthSpec{}, + } + + if withUsers { + oAuth.Spec.IdentityProviders = []oauth_config.IdentityProvider{ + { + Name: "htpasswd", + }, + } + } + + objs = append(objs, oAuth) + scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuth) + + // Create a fake client to mock API calls + return fake.NewFakeClient(objs...), scheme +} + +func CreateOpenshift3Client(withUsers bool) (client.Client, runtime.Scheme) { + objs, scheme := createAPIObjects() + + userList := &userv1.UserList{ + Items: []userv1.User{}, + } + + if withUsers { + userList.Items = append(userList.Items, userv1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user1", + }, + }) + userList.Items = append(userList.Items, userv1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user2", + }, + }) + } + + // Objects to track in the fake client. + objs = append(objs, userList) + + 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...), scheme +} + +func createAPIObjects() ([]runtime.Object, runtime.Scheme) { pgPod := &corev1.Pod{ TypeMeta: metav1.TypeMeta{ Kind: "Pod", @@ -515,21 +955,6 @@ func Init() (client.Client, runtime.Scheme) { }, } - userList := &userv1.UserList{ - Items: []userv1.User{ - userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "user1", - }, - }, - userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "user2", - }, - }, - }, - } - route := &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ Name: deploy.DefaultCheFlavor(cheCR), @@ -538,20 +963,14 @@ func Init() (client.Client, runtime.Scheme) { } // Objects to track in the fake client. objs := []runtime.Object{ - cheCR, pgPod, userList, route, + cheCR, pgPod, route, } - 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{}) - // Create a fake client to mock API calls - return fake.NewFakeClient(objs...), *scheme + return objs, *scheme } diff --git a/pkg/deploy/server/che_configmap_test.go b/pkg/deploy/server/che_configmap_test.go index beb123afdb..dd95806136 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.GetBoolPointer(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.GetBoolPointer(true) deployContext := &deploy.DeployContext{ CheCluster: cr, Proxy: &deploy.Proxy{}, diff --git a/pkg/util/util.go b/pkg/util/util.go index b291dbd798..e99e9677fb 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -24,6 +24,7 @@ import ( "regexp" "runtime" "sort" + "strconv" "strings" "time" @@ -92,21 +93,28 @@ 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 } @@ -331,7 +339,7 @@ func GetArchitectureDependentEnv(env string) string { return env } -// GetBoolPointer returns `bool` pointer to value in the memory. +// GetBoolPointer returns `bool` pointer to value in the memory. // Unfortunately golang hasn't got syntax to create `bool` pointer. func GetBoolPointer(value bool) *bool { variable := value From 055cb0bf8882a3ff788d7518eee07c4865380533 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Tue, 15 Dec 2020 00:14:16 +0200 Subject: [PATCH 03/14] Fix openshift version detection. Signed-off-by: Oleksandr Andriienko --- pkg/util/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/util.go b/pkg/util/util.go index e99e9677fb..dd326ffb06 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -115,7 +115,7 @@ func DetectOpenShift() (isOpenshift bool, isOpenshift4 bool, anError error) { } } - return true, false, nil + return isOpenshift, isOpenshift4, nil } func getDiscoveryClient() (*discovery.DiscoveryClient, error) { From b69645ea72562941b1b19bfddc8b20dd5360004b Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Tue, 15 Dec 2020 14:11:24 +0200 Subject: [PATCH 04/14] Enable oAuth for e2e test. e2e tests fails to create CheCR programatically with nil value. But with yamls it works... Signed-off-by: Oleksandr Andriienko --- e2e/create.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e/create.go b/e2e/create.go index 64fa08d732..d8c7d8f919 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.GetBoolPointer(true), + }, }, } return cr From 0ea5fb2d74a569db2228e73882a701a9598154ed Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Tue, 15 Dec 2020 16:24:51 +0200 Subject: [PATCH 05/14] Add nightly bundle Signed-off-by: Oleksandr Andriienko --- .../manifests/che-operator.clusterserviceversion.yaml | 6 +++--- .../manifests/che-operator.clusterserviceversion.yaml | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) 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..ffb1574151 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-15T14:08:40Z" 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-50.nightly namespace: placeholder spec: apiservicedefinitions: {} @@ -494,4 +494,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.23.0-48.nightly + version: 7.24.0-50.nightly 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..855d086177 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-15T14:08:47Z" 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-50.nightly namespace: placeholder spec: apiservicedefinitions: {} @@ -514,4 +513,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.23.0-48.nightly + version: 7.24.0-50.nightly From 9d8c34c147ddabdd735aa7346b5896cb8bb3b8c9 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Wed, 16 Dec 2020 20:49:44 +0200 Subject: [PATCH 06/14] Handle code review requested changes. Signed-off-by: Oleksandr Andriienko --- e2e/create.go | 2 +- pkg/controller/che/che_controller.go | 17 ++++++------ pkg/controller/che/che_controller_test.go | 26 +++++++++---------- pkg/controller/che/update.go | 2 +- .../identity-provider/identity_provider.go | 2 +- pkg/deploy/server/che_configmap.go | 3 +-- pkg/deploy/server/che_configmap_test.go | 4 +-- pkg/util/util.go | 13 ++++++++-- 8 files changed, 39 insertions(+), 30 deletions(-) diff --git a/e2e/create.go b/e2e/create.go index d8c7d8f919..8a3d495583 100644 --- a/e2e/create.go +++ b/e2e/create.go @@ -124,7 +124,7 @@ func newCheCluster() (cr *orgv1.CheCluster) { UseInternalClusterSVCNames: true, }, Auth: orgv1.CheClusterSpecAuth{ - OpenShiftoAuth: util.GetBoolPointer(true), + OpenShiftoAuth: util.NewBoolPointer(true), }, }, } diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 441ceaeb33..72fe5c491a 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -352,7 +352,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } // delete oAuthClient before CR is deleted - if instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth { + if util.IsOAuthEnabled(instance) { if err := r.ReconcileFinalizer(instance); err != nil { return reconcile.Result{}, err } @@ -397,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 != nil && *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 } @@ -413,7 +413,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e } } - if instance.Spec.Auth.OpenShiftoAuth != nil && *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 { @@ -1099,9 +1099,9 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Errorf(getOAuthV1ErrMsg) message = getOAuthV1ErrMsg reason = failedNoOpenshiftUserReason - cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) } else { - cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(len(oauthv1.Spec.IdentityProviders) >= 1) + cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(len(oauthv1.Spec.IdentityProviders) >= 1) if !*cr.Spec.Auth.OpenShiftoAuth { logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4) } @@ -1115,16 +1115,17 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Errorf(getUsersErrMsg) message = getUsersErrMsg reason = failedNoOpenshiftUserReason - cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) } - cr.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(len(users.Items) >= 1) + cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(len(users.Items) >= 1) if !*cr.Spec.Auth.OpenShiftoAuth { logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3) } } - if err := r.UpdateCheCRSpec(cr, "OpenShiftoAuth", strconv.FormatBool(*cr.Spec.Auth.OpenShiftoAuth)); err != nil { + oauth := *cr.Spec.Auth.OpenShiftoAuth + if err := r.UpdateCheCRSpec(cr, "OpenShiftoAuth", strconv.FormatBool(oauth)); err != nil { return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err } diff --git a/pkg/controller/che/che_controller_test.go b/pkg/controller/che/che_controller_test.go index 30fe8f87d8..63285a96b8 100644 --- a/pkg/controller/che/che_controller_test.go +++ b/pkg/controller/che/che_controller_test.go @@ -533,7 +533,7 @@ func TestCheController(t *testing.T) { } // update CR and make sure Che configmap has been updated - cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(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") } @@ -900,7 +900,7 @@ func TestAutoEnableOAuthForOpenshift4(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(true) + value := util.NewBoolPointer(true) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be enabled") } @@ -946,7 +946,7 @@ func TestAutoEnableOAuthForOpenshift3(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(true) + value := util.NewBoolPointer(true) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be enabled") } @@ -992,7 +992,7 @@ func TestAutoDisableOAuthForOpenshift3(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(false) + value := util.NewBoolPointer(false) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be enabled") } @@ -1016,7 +1016,7 @@ func TestRespectEnablingOAuthFromUserOpenshift3(t *testing.T) { } // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(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") } @@ -1040,7 +1040,7 @@ func TestRespectEnablingOAuthFromUserOpenshift3(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(true) + value := util.NewBoolPointer(true) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be enabled") } @@ -1064,7 +1064,7 @@ func TestRespectDisablingOAuthFromUserOpenshift3(t *testing.T) { } // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + cheCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) if err := cl.Update(context.TODO(), cheCR); err != nil { t.Error("Failed to update CheCluster custom resource") } @@ -1088,7 +1088,7 @@ func TestRespectDisablingOAuthFromUserOpenshift3(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(false) + value := util.NewBoolPointer(false) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be enabled") } @@ -1130,7 +1130,7 @@ func TestAutoDisableOAuthForOpenshift4(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(false) + value := util.NewBoolPointer(false) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be disabled") } @@ -1155,7 +1155,7 @@ func TestRespectEnablingOAuthFromUserOpenshift4(t *testing.T) { } // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(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") } @@ -1179,7 +1179,7 @@ func TestRespectEnablingOAuthFromUserOpenshift4(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(true) + value := util.NewBoolPointer(true) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be enabled") } @@ -1204,7 +1204,7 @@ func TestRespectDisablingOAuthFromUserOpenshift4(t *testing.T) { } // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.GetBoolPointer(false) + cheCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) if err := cl.Update(context.TODO(), cheCR); err != nil { t.Error("Failed to update CheCluster custom resource") } @@ -1228,7 +1228,7 @@ func TestRespectDisablingOAuthFromUserOpenshift4(t *testing.T) { t.Errorf("CR not found") } - value := util.GetBoolPointer(false) + value := util.NewBoolPointer(false) if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { t.Errorf("Openshift oAuth should be enabled") } diff --git a/pkg/controller/che/update.go b/pkg/controller/che/update.go index d5e0f2d6e2..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 != nil && !*instance.Spec.Auth.OpenShiftoAuth && 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 ccfeb1f628..35fb84802b 100644 --- a/pkg/deploy/identity-provider/identity_provider.go +++ b/pkg/deploy/identity-provider/identity_provider.go @@ -127,7 +127,7 @@ func SyncIdentityProviderToCluster(deployContext *deploy.DeployContext) (bool, e } } - if isOpenShift && instance.Spec.Auth.OpenShiftoAuth != nil && *instance.Spec.Auth.OpenShiftoAuth { + 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 9fe2a699af..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 != nil && *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 dd95806136..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 = util.GetBoolPointer(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 = util.GetBoolPointer(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 dd326ffb06..e5beea0b00 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -339,9 +339,18 @@ func GetArchitectureDependentEnv(env string) string { return env } -// GetBoolPointer returns `bool` pointer to value in the memory. +// NewBoolPointer returns `bool` pointer to value in the memory. // Unfortunately golang hasn't got syntax to create `bool` pointer. -func GetBoolPointer(value bool) *bool { +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 +} From c2a1742eee055960d93a847bb7a39d8d783a1cc9 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 17 Dec 2020 12:11:49 +0200 Subject: [PATCH 07/14] Update openshiftOAuth property description. Update OLM bundle. Improve Update Olm bundle dev task. Signed-off-by: Oleksandr Andriienko --- .vscode/tasks.json | 4 ++-- Dockerfile | 2 +- README.md | 2 +- deploy/crds/org_v1_che_crd.yaml | 15 +++++++++------ .../che-operator.clusterserviceversion.yaml | 8 ++++---- .../manifests/org_v1_che_crd.yaml | 15 +++++++++------ .../che-operator.clusterserviceversion.yaml | 8 ++++---- .../manifests/org_v1_che_crd.yaml | 15 +++++++++------ deploy/operator-local.yaml | 2 +- deploy/operator.yaml | 2 +- pkg/apis/org/v1/che_types.go | 8 ++++++-- 11 files changed, 47 insertions(+), 34 deletions(-) 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_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 ffb1574151..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-15T14:08:40Z" + 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.24.0-50.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.24.0-50.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 855d086177..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 @@ -75,13 +75,13 @@ metadata: categories: Developer Tools, OpenShift Optional certified: "false" containerImage: quay.io/eclipse/che-operator:nightly - createdAt: "2020-12-15T14:08:47Z" + 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.24.0-50.nightly + name: eclipse-che-preview-openshift.v7.24.0-52.nightly namespace: placeholder spec: apiservicedefinitions: {} @@ -335,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 @@ -513,4 +513,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.24.0-50.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/pkg/apis/org/v1/che_types.go b/pkg/apis/org/v1/che_types.go index defd571fb9..e19cab1ce2 100644 --- a/pkg/apis/org/v1/che_types.go +++ b/pkg/apis/org/v1/che_types.go @@ -405,8 +405,12 @@ 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 From 5794c82a854076eddbd5864d03b2d57045d06920 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 17 Dec 2020 14:37:30 +0200 Subject: [PATCH 08/14] Update pkg/controller/che/che_controller.go Co-authored-by: Anatolii Bazko --- pkg/controller/che/che_controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 72fe5c491a..e06b003cb8 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -1092,6 +1092,7 @@ 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 := *cr.Spec.Auth.OpenShiftoAuth if isOpenShift4 { oauthv1 := &oauthv1.OAuth{} if err := r.nonCachedClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, oauthv1); err != nil { From eb03b0484613a9bfd4b16b7f639aa818e52bf7d5 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 17 Dec 2020 14:37:40 +0200 Subject: [PATCH 09/14] Update pkg/controller/che/che_controller.go Co-authored-by: Anatolii Bazko --- pkg/controller/che/che_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index e06b003cb8..8c9a1be1fd 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -1100,7 +1100,7 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Errorf(getOAuthV1ErrMsg) message = getOAuthV1ErrMsg reason = failedNoOpenshiftUserReason - cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) + oauth = util.NewBoolPointer(false) } else { cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(len(oauthv1.Spec.IdentityProviders) >= 1) if !*cr.Spec.Auth.OpenShiftoAuth { From 16bf3023b9cce808eb8d155a016e4736f89e977a Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 17 Dec 2020 14:55:45 +0200 Subject: [PATCH 10/14] Handle code review suggestions Signed-off-by: Oleksandr Andriienko --- pkg/controller/che/che_controller.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 8c9a1be1fd..537267da47 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -1091,8 +1091,7 @@ 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 := *cr.Spec.Auth.OpenShiftoAuth + var message, reason string; var oauth bool if isOpenShift4 { oauthv1 := &oauthv1.OAuth{} if err := r.nonCachedClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, oauthv1); err != nil { @@ -1100,10 +1099,10 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Errorf(getOAuthV1ErrMsg) message = getOAuthV1ErrMsg reason = failedNoOpenshiftUserReason - oauth = util.NewBoolPointer(false) + oauth = false } else { - cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(len(oauthv1.Spec.IdentityProviders) >= 1) - if !*cr.Spec.Auth.OpenShiftoAuth { + oauth =len(oauthv1.Spec.IdentityProviders) >= 1 + if !oauth { logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4) } } @@ -1116,16 +1115,16 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Errorf(getUsersErrMsg) message = getUsersErrMsg reason = failedNoOpenshiftUserReason - cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) + oauth = false } - cr.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(len(users.Items) >= 1) - if !*cr.Spec.Auth.OpenShiftoAuth { + oauth = len(users.Items) >= 1 + if !oauth { logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3) } } - oauth := *cr.Spec.Auth.OpenShiftoAuth + 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 } From 2c908892bc89ba0c5cc54fd48cccf44826c51b89 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 17 Dec 2020 17:51:13 +0200 Subject: [PATCH 11/14] Update pkg/controller/che/che_controller.go Co-authored-by: Anatolii Bazko --- pkg/controller/che/che_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 537267da47..258ea95c0a 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -1101,7 +1101,7 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R reason = failedNoOpenshiftUserReason oauth = false } else { - oauth =len(oauthv1.Spec.IdentityProviders) >= 1 + oauth = len(oauthv1.Spec.IdentityProviders) >= 1 if !oauth { logrus.Warn(warningNoIdentityProvidersMessage, " ", howToAddIdentityProviderLinkOS4) } From 1da8eb0ddda40b74b71c56123b6534aa9378ed82 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 17 Dec 2020 17:58:08 +0200 Subject: [PATCH 12/14] Handle pull request feedback. Signed-off-by: Oleksandr Andriienko --- pkg/controller/che/che_controller.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 258ea95c0a..988161dea0 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -1099,7 +1099,6 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Errorf(getOAuthV1ErrMsg) message = getOAuthV1ErrMsg reason = failedNoOpenshiftUserReason - oauth = false } else { oauth = len(oauthv1.Spec.IdentityProviders) >= 1 if !oauth { @@ -1115,12 +1114,11 @@ func (r *ReconcileChe) autoEnableOAuth(cr *orgv1.CheCluster, request reconcile.R logrus.Errorf(getUsersErrMsg) message = getUsersErrMsg reason = failedNoOpenshiftUserReason - oauth = false - } - - oauth = len(users.Items) >= 1 - if !oauth { - logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3) + } else { + oauth = len(users.Items) >= 1 + if !oauth { + logrus.Warn(warningNoRealUsersMessage, " ", howToConfigureOAuthLinkOS3) + } } } From 279b0254b9b8013e65ce42cf45194a991aa9f4a7 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Thu, 17 Dec 2020 18:01:53 +0200 Subject: [PATCH 13/14] Handle pull request feedback. Signed-off-by: Oleksandr Andriienko --- pkg/apis/org/v1/che_types.go | 2 +- pkg/controller/che/che_controller.go | 3 ++- pkg/controller/che/che_controller_test.go | 2 +- pkg/util/util.go | 1 - 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/apis/org/v1/che_types.go b/pkg/apis/org/v1/che_types.go index e19cab1ce2..0b4ca090f2 100644 --- a/pkg/apis/org/v1/che_types.go +++ b/pkg/apis/org/v1/che_types.go @@ -407,7 +407,7 @@ type CheClusterSpecAuth struct { UpdateAdminPassword bool `json:"updateAdminPassword"` // 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. + // 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, diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index 988161dea0..f682494eb3 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -1091,7 +1091,8 @@ 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; var oauth bool + 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 { diff --git a/pkg/controller/che/che_controller_test.go b/pkg/controller/che/che_controller_test.go index 63285a96b8..d88d6b378a 100644 --- a/pkg/controller/che/che_controller_test.go +++ b/pkg/controller/che/che_controller_test.go @@ -713,7 +713,7 @@ func TestConfiguringInternalNetworkTest(t *testing.T) { // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) - cl, discoveryClient ,scheme := CreateOpenshift3Client(true) // Todo rename, it's not only client... + cl, discoveryClient, scheme := CreateOpenshift3Client(true) // Todo rename, it's not only client... // Create a ReconcileChe object with the scheme and fake client r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: discoveryClient, scheme: &scheme, tests: true} diff --git a/pkg/util/util.go b/pkg/util/util.go index e5beea0b00..9b6072f312 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -346,7 +346,6 @@ func NewBoolPointer(value bool) *bool { 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 { From adf5e50bea35fccdf235bdd079fd85647a7f6046 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Sun, 20 Dec 2020 00:22:28 +0200 Subject: [PATCH 14/14] Use test suite for oAuth tests. Signed-off-by: Oleksandr Andriienko --- pkg/controller/che/che_controller_test.go | 683 ++++++++-------------- 1 file changed, 247 insertions(+), 436 deletions(-) diff --git a/pkg/controller/che/che_controller_test.go b/pkg/controller/che/che_controller_test.go index d88d6b378a..9872f738d9 100644 --- a/pkg/controller/che/che_controller_test.go +++ b/pkg/controller/che/che_controller_test.go @@ -147,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() { @@ -160,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 @@ -437,7 +664,7 @@ func TestCheController(t *testing.T) { // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) - cl, dc, scheme := CreateOpenshift3Client(true) + cl, dc, scheme := Init() // Create a ReconcileChe object with the scheme and fake client r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, discoveryClient: dc, tests: true} @@ -659,7 +886,7 @@ func TestConfiguringLabelsForRoutes(t *testing.T) { // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) - cl, dc, scheme := CreateOpenshift3Client(true) + cl, dc, scheme := Init() // Create a ReconcileChe object with the scheme and fake client r := &ReconcileChe{client: cl, nonCachedClient: cl, scheme: &scheme, discoveryClient: dc, tests: true} @@ -713,7 +940,7 @@ func TestConfiguringInternalNetworkTest(t *testing.T) { // Set the logger to development mode for verbose logs. logf.SetLogger(logf.ZapLogger(true)) - cl, discoveryClient, scheme := CreateOpenshift3Client(true) // Todo rename, it's not only client... + cl, discoveryClient, scheme := Init() // Create a ReconcileChe object with the scheme and fake client r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: discoveryClient, scheme: &scheme, tests: true} @@ -860,429 +1087,9 @@ func TestConfiguringInternalNetworkTest(t *testing.T) { } } -func TestAutoEnableOAuthForOpenshift4(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "4") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - cl, ds, scheme := CreateOpenshift4Client(true) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - if cheCR.Spec.Auth.OpenShiftoAuth != nil { - t.Fatal("Openshift oAuth should not be set up by default") - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(true) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be enabled") - } -} - -func TestAutoEnableOAuthForOpenshift3(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "3") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - cl, ds, scheme := CreateOpenshift3Client(true) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - if cheCR.Spec.Auth.OpenShiftoAuth != nil { - t.Fatal("Openshift oAuth should not be set up by default") - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(true) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be enabled") - } -} - -func TestAutoDisableOAuthForOpenshift3(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "3") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - cl, ds, scheme := CreateOpenshift3Client(false) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - if cheCR.Spec.Auth.OpenShiftoAuth != nil { - t.Fatal("Openshift oAuth should not be set up by default") - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(false) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be enabled") - } -} - -func TestRespectEnablingOAuthFromUserOpenshift3(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "3") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - cl, ds, scheme := CreateOpenshift3Client(false) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(true) - if err := cl.Update(context.TODO(), cheCR); err != nil { - t.Error("Failed to update CheCluster custom resource") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(true) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be enabled") - } -} - -func TestRespectDisablingOAuthFromUserOpenshift3(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "3") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - cl, ds, scheme := CreateOpenshift3Client(true) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) - if err := cl.Update(context.TODO(), cheCR); err != nil { - t.Error("Failed to update CheCluster custom resource") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(false) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be enabled") - } -} - -func TestAutoDisableOAuthForOpenshift4(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "4") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - cl, ds, scheme := CreateOpenshift4Client(false) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(false) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be disabled") - } -} - -func TestRespectEnablingOAuthFromUserOpenshift4(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "4") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - // Create fake client which returns no identity providers. - cl, ds, scheme := CreateOpenshift4Client(false) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(true) - if err := cl.Update(context.TODO(), cheCR); err != nil { - t.Error("Failed to update CheCluster custom resource") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(true) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be enabled") - } -} - -func TestRespectDisablingOAuthFromUserOpenshift4(t *testing.T) { - os.Setenv("OPENSHIFT_VERSION", "4") - - // Set the logger to development mode for verbose logs. - logf.SetLogger(logf.ZapLogger(true)) - - // Create fake client which returns non zero quantity identity providers. - cl, ds, scheme := CreateOpenshift4Client(true) - - // Create a ReconcileChe object with the scheme and fake client - r := &ReconcileChe{client: cl, nonCachedClient: cl, discoveryClient: ds, scheme: &scheme, tests: true} - - // get CR - cheCR := &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - // update CR - cheCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) - if err := cl.Update(context.TODO(), cheCR); err != nil { - t.Error("Failed to update CheCluster custom resource") - } - - // Mock request to simulate Reconcile() being called on an event for a - // watched resource . - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: name, - Namespace: namespace, - }, - } - - _, err := r.Reconcile(req) - if err != nil { - t.Fatalf("reconcile: (%v)", err) - } - - cheCR = &orgv1.CheCluster{} - if err := cl.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } - - value := util.NewBoolPointer(false) - if cheCR.Spec.Auth.OpenShiftoAuth == nil || *cheCR.Spec.Auth.OpenShiftoAuth != *value { - t.Errorf("Openshift oAuth should be enabled") - } -} - -func CreateOpenshift4Client(withUsers bool) (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { +func Init() (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { objs, ds, scheme := createAPIObjects() - oAuth := &oauth_config.OAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - Namespace: namespace, - }, - Spec: oauth_config.OAuthSpec{}, - } - - if withUsers { - oAuth.Spec.IdentityProviders = []oauth_config.IdentityProvider{ - { - Name: "htpasswd", - }, - } - } - - objs = append(objs, oAuth) - scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuth) - - // Create a fake client to mock API calls - return fake.NewFakeClient(objs...), ds, scheme -} - -func CreateOpenshift3Client(withUsers bool) (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { - objs, ds, scheme := createAPIObjects() - - userList := &userv1.UserList{ - Items: []userv1.User{}, - } - - if withUsers { - userList.Items = append(userList.Items, userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "user1", - }, - }) - userList.Items = append(userList.Items, userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: "user2", - }, - }) - } - - // Objects to track in the fake client. - objs = append(objs, userList) - oAuthClient := &oauth.OAuthClient{} users := &userv1.UserList{} user := &userv1.User{} @@ -1311,18 +1118,7 @@ func createAPIObjects() ([]runtime.Object, discovery.DiscoveryInterface, runtime } // 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", - }, - }, - } + cheCR := InitCheWithSimpleCR() route := &routev1.Route{ ObjectMeta: metav1.ObjectMeta{ @@ -1364,6 +1160,21 @@ func createAPIObjects() ([]runtime.Object, discovery.DiscoveryInterface, runtime 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 { return &orgv1.CheCluster{ ObjectMeta: metav1.ObjectMeta{