From 10ec4aeaf1ae7b1451a0c4b7e3503145e300a363 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 8 Nov 2021 11:21:40 +0200 Subject: [PATCH] Refactoring OpenShiftOAuth Signed-off-by: Anatolii Bazko --- api/v1/checluster_types.go | 4 + controllers/che/checluster_controller.go | 106 +------- controllers/che/checluster_controller_test.go | 214 +--------------- controllers/che/create.go | 12 +- controllers/che/gateway_permission.go | 2 +- pkg/deploy/dashboard/dashboard.go | 2 +- pkg/deploy/gateway/gateway.go | 16 +- .../identity-provider/identity_provider.go | 4 +- pkg/deploy/openshift-oauth/openshiftoauth.go | 117 +++++++++ .../openshift-oauth/openshiftoauth_test.go | 234 ++++++++++++++++++ pkg/deploy/server/server_deployment.go | 2 +- pkg/util/util.go | 5 - 12 files changed, 390 insertions(+), 328 deletions(-) create mode 100644 pkg/deploy/openshift-oauth/openshiftoauth.go create mode 100644 pkg/deploy/openshift-oauth/openshiftoauth_test.go diff --git a/api/v1/checluster_types.go b/api/v1/checluster_types.go index 84275a30b7..d01dc551c5 100644 --- a/api/v1/checluster_types.go +++ b/api/v1/checluster_types.go @@ -794,3 +794,7 @@ func (c *CheCluster) IsOpenShiftOAuthUserMustBeDeleted() bool { func (c *CheCluster) IsOpenShiftOAuthEnabled() bool { return c.Spec.Auth.OpenShiftoAuth != nil && *c.Spec.Auth.OpenShiftoAuth } + +func (c *CheCluster) IsNativeUserModeEnabled() bool { + return c.Spec.Auth.NativeUserMode != nil && *c.Spec.Auth.NativeUserMode +} diff --git a/controllers/che/checluster_controller.go b/controllers/che/checluster_controller.go index b680145b29..6771a4c5fd 100644 --- a/controllers/che/checluster_controller.go +++ b/controllers/che/checluster_controller.go @@ -14,8 +14,6 @@ package che import ( "context" - "reflect" - "strconv" "strings" "time" @@ -50,22 +48,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" orgv1 "github.com/eclipse-che/che-operator/api/v1" - userv1 "github.com/openshift/api/user/v1" networking "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/errors" ) -const ( - warningNoIdentityProvidersMessage = "No Openshift identity providers." - - AddIdentityProviderMessage = "Openshift oAuth was disabled. How to add identity provider read in the Help Link:" - warningNoRealUsersMessage = "No real users. Openshift oAuth was disabled. How to add new user read in the Help Link:" - failedUnableToGetOpenshiftUsers = "Unable to get users on the OpenShift cluster." - - howToAddIdentityProviderLinkOS4 = "https://docs.openshift.com/container-platform/latest/authentication/understanding-identity-provider.html#identity-provider-overview_understanding-identity-provider" - howToConfigureOAuthLinkOS3 = "https://docs.openshift.com/container-platform/3.11/install_config/configuring_authentication.html" -) - // CheClusterReconciler reconciles a CheCluster object type CheClusterReconciler struct { Log logr.Logger @@ -82,10 +68,9 @@ type CheClusterReconciler struct { nonCachedClient client.Client // A discovery client to check for the existence of certain APIs registered // in the API Server - discoveryClient discovery.DiscoveryInterface - tests bool - openShiftOAuthUser openshiftoauth.IOpenShiftOAuthUser - reconcileManager *deploy.ReconcileManager + discoveryClient discovery.DiscoveryInterface + tests bool + reconcileManager *deploy.ReconcileManager // the namespace to which to limit the reconciliation. If empty, all namespaces are considered namespace string } @@ -108,17 +93,17 @@ func NewReconciler( openShiftOAuthUser := openshiftoauth.NewOpenShiftOAuthUser() reconcileManager.RegisterReconciler(openShiftOAuthUser) + reconcileManager.RegisterReconciler(openshiftoauth.NewOpenShiftOAuth(openShiftOAuthUser)) return &CheClusterReconciler{ Scheme: scheme, Log: ctrl.Log.WithName("controllers").WithName("CheCluster"), - client: k8sclient, - nonCachedClient: noncachedClient, - discoveryClient: discoveryClient, - openShiftOAuthUser: openShiftOAuthUser, - namespace: namespace, - reconcileManager: reconcileManager, + client: k8sclient, + nonCachedClient: noncachedClient, + discoveryClient: discoveryClient, + namespace: namespace, + reconcileManager: reconcileManager, } } @@ -302,20 +287,6 @@ func (r *CheClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) // TODO remove in favor of r.reconcileManager.FinalizeAll(deployContext) r.reconcileFinalizers(deployContext) - if util.IsOpenShift && checluster.Spec.DevWorkspace.Enable && checluster.Spec.Auth.NativeUserMode == nil { - newNativeUserModeValue := util.NewBoolPointer(true) - checluster.Spec.Auth.NativeUserMode = newNativeUserModeValue - if err := deploy.UpdateCheCRSpec(deployContext, "nativeUserMode", strconv.FormatBool(*newNativeUserModeValue)); err != nil { - return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err - } - } - - if util.IsOpenShift && checluster.Spec.Auth.OpenShiftoAuth == nil { - if reconcileResult, err := r.autoEnableOAuth(deployContext); err != nil { - return reconcileResult, err - } - } - // Reconcile Dev Workspace Operator done, err := devworkspace.ReconcileDevWorkspace(deployContext) if !done { @@ -612,63 +583,6 @@ func (r *CheClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } -func (r *CheClusterReconciler) autoEnableOAuth(deployContext *deploy.DeployContext) (reconcile.Result, error) { - oauth := false - cr := deployContext.CheCluster - if util.IsOpenShift4 { - openshitOAuth, err := openshiftoauth.GetOpenshiftOAuth(deployContext) - if err != nil { - logrus.Error("Unable to get Openshift oAuth. Cause: " + err.Error()) - } else { - if len(openshitOAuth.Spec.IdentityProviders) > 0 { - oauth = true - } else if util.IsNativeUserModeEnabled(deployContext.CheCluster) { - // enable OpenShift OAuth without adding initial OpenShift OAuth user - // since kubeadmin is a valid user for native user mode - oauth = true - } else if cr.IsOpenShiftOAuthUserConfigured() { - provisioned, err := r.openShiftOAuthUser.Create(deployContext) - if err != nil { - logrus.Error(warningNoIdentityProvidersMessage + " Operator tried to create initial OpenShift OAuth user for HTPasswd identity provider, but failed. Cause: " + err.Error()) - logrus.Info("To enable OpenShift OAuth, please add identity provider first: " + howToAddIdentityProviderLinkOS4) - // Don't try to create initial user any more, che-operator shouldn't hang on this step. - cr.Spec.Auth.InitialOpenShiftOAuthUser = nil - if err := deploy.UpdateCheCRStatus(deployContext, "initialOpenShiftOAuthUser", ""); err != nil { - return reconcile.Result{}, err - } - oauth = false - } else { - if !provisioned { - return reconcile.Result{}, err - } - oauth = true - } - } - } - } else { // Openshift 3 - users := &userv1.UserList{} - listOptions := &client.ListOptions{} - if err := r.nonCachedClient.List(context.TODO(), users, listOptions); err != nil { - logrus.Error(failedUnableToGetOpenshiftUsers + " Cause: " + err.Error()) - } else { - oauth = len(users.Items) >= 1 - if !oauth { - logrus.Warn(warningNoRealUsersMessage + " " + howToConfigureOAuthLinkOS3) - } - } - } - - newOAuthValue := util.NewBoolPointer(oauth) - if !reflect.DeepEqual(newOAuthValue, cr.Spec.Auth.OpenShiftoAuth) { - cr.Spec.Auth.OpenShiftoAuth = newOAuthValue - if err := deploy.UpdateCheCRSpec(deployContext, "openShiftoAuth", strconv.FormatBool(oauth)); err != nil { - return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err - } - } - - return reconcile.Result{}, nil -} - func (r *CheClusterReconciler) reconcileFinalizers(deployContext *deploy.DeployContext) { if util.IsOpenShift && deployContext.CheCluster.IsOpenShiftOAuthEnabled() { if err := deploy.ReconcileOAuthClientFinalizer(deployContext); err != nil { @@ -676,7 +590,7 @@ func (r *CheClusterReconciler) reconcileFinalizers(deployContext *deploy.DeployC } } - if util.IsNativeUserModeEnabled(deployContext.CheCluster) { + if util.IsOpenShift && deployContext.CheCluster.IsNativeUserModeEnabled() { if _, err := r.reconcileGatewayPermissionsFinalizers(deployContext); err != nil { logrus.Error(err) } diff --git a/controllers/che/checluster_controller_test.go b/controllers/che/checluster_controller_test.go index d588069797..ee6cb0d3e8 100644 --- a/controllers/che/checluster_controller_test.go +++ b/controllers/che/checluster_controller_test.go @@ -18,7 +18,6 @@ import ( "strconv" mocks "github.com/eclipse-che/che-operator/mocks" - "github.com/stretchr/testify/assert" "reflect" "time" @@ -70,45 +69,7 @@ import ( ) var ( - namespace = "eclipse-che" - nonEmptyUserList = &userv1.UserList{ - Items: []userv1.User{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "user1", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "user2", - }, - }, - }, - } - oAuthWithNoIdentityProviders = &configv1.OAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - // Namespace: namespace, - }, - } - oAuthWithIdentityProvider = &configv1.OAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - // Namespace: namespace, - }, - Spec: configv1.OAuthSpec{ - IdentityProviders: []configv1.IdentityProvider{ - { - Name: "htpasswd", - }, - }, - }, - } - proxy = &configv1.Proxy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - }, - } + namespace = "eclipse-che" ) func TestNativeUserModeEnabled(t *testing.T) { @@ -227,179 +188,6 @@ func TestNativeUserModeEnabled(t *testing.T) { } } -func TestCaseAutoDetectOAuth(t *testing.T) { - type testCase struct { - name string - initObjects []runtime.Object - isOpenshift4 bool - initialOAuthValue *bool - oAuthExpected *bool - initialOpenShiftOAuthUserEnabled *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, - &oauthv1.OAuthClient{}, - }, - isOpenshift4: false, - initialOAuthValue: nil, - oAuthExpected: pointer.BoolPtr(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{}, - &oauthv1.OAuthClient{}, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(false), - oAuthExpected: pointer.BoolPtr(false), - }, - { - name: "che-operator should respect oAuth = true even if there no users on the Openshift 3", - initObjects: []runtime.Object{ - &userv1.UserList{}, - &oauthv1.OAuthClient{}, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(true), - oAuthExpected: pointer.BoolPtr(true), - }, - { - name: "che-operator should respect oAuth = true even if there are some users on the Openshift 3", - initObjects: []runtime.Object{ - nonEmptyUserList, - &oauthv1.OAuthClient{}, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(true), - oAuthExpected: pointer.BoolPtr(true), - }, - { - name: "che-operator should respect oAuth = false even if there are some users on the Openshift 3", - initObjects: []runtime.Object{ - nonEmptyUserList, - &oauthv1.OAuthClient{}, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(false), - oAuthExpected: pointer.BoolPtr(false), - }, - { - name: "che-operator should respect oAuth = false even if no users on the Openshift 3", - initObjects: []runtime.Object{ - &userv1.UserList{}, - &oauthv1.OAuthClient{}, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(false), - oAuthExpected: pointer.BoolPtr(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, - proxy, - }, - isOpenshift4: true, - initialOAuthValue: nil, - oAuthExpected: pointer.BoolPtr(true), - }, - { - name: "che-operator should respect oAuth = true even if there no indentity providers on the Openshift 4", - initObjects: []runtime.Object{ - oAuthWithNoIdentityProviders, - proxy, - }, - isOpenshift4: true, - initialOAuthValue: pointer.BoolPtr(true), - oAuthExpected: pointer.BoolPtr(true), - initialOpenShiftOAuthUserEnabled: pointer.BoolPtr(true), - }, - { - name: "che-operator should respect oAuth = true even if there are some users on the Openshift 4", - initObjects: []runtime.Object{ - oAuthWithIdentityProvider, - proxy, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(true), - oAuthExpected: pointer.BoolPtr(true), - initialOpenShiftOAuthUserEnabled: pointer.BoolPtr(true), - }, - { - name: "che-operator should respect oAuth = false even if there no indentity providers on the Openshift 4", - initObjects: []runtime.Object{ - oAuthWithNoIdentityProviders, - proxy, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(false), - oAuthExpected: pointer.BoolPtr(false), - }, - { - name: "che-operator should respect oAuth = false even if there are some users on the Openshift 4", - initObjects: []runtime.Object{ - oAuthWithIdentityProvider, - proxy, - }, - isOpenshift4: false, - initialOAuthValue: pointer.BoolPtr(false), - oAuthExpected: pointer.BoolPtr(false), - }, - { - name: "che-operator should auto disable oAuth on error retieve identity providers", - initObjects: []runtime.Object{}, - isOpenshift4: false, - initialOAuthValue: nil, - initialOpenShiftOAuthUserEnabled: pointer.BoolPtr(true), - oAuthExpected: pointer.BoolPtr(false), - }, - } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) - - initCR := InitCheWithSimpleCR().DeepCopy() - initCR.Spec.Auth.OpenShiftoAuth = testCase.initialOAuthValue - initCR.Spec.Auth.InitialOpenShiftOAuthUser = testCase.initialOpenShiftOAuthUserEnabled - - deployContext := deploy.GetTestDeployContext(initCR, testCase.initObjects) - - r := NewReconciler( - deployContext.ClusterAPI.Client, - deployContext.ClusterAPI.NonCachedClient, - deployContext.ClusterAPI.DiscoveryClient, - deployContext.ClusterAPI.Client.Scheme(), - "") - r.tests = true - - req := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: os.Getenv("CHE_FLAVOR"), - Namespace: namespace, - }, - } - - util.IsOpenShift = true - util.IsOpenShift4 = testCase.isOpenshift4 - - _, err := r.Reconcile(context.TODO(), req) - assert.Nil(t, err) - - cheCR := &orgv1.CheCluster{} - err = r.client.Get(context.TODO(), types.NamespacedName{Name: os.Getenv("CHE_FLAVOR"), Namespace: namespace}, cheCR) - assert.Nil(t, err) - - assert.NotNil(t, cheCR.Spec.Auth.OpenShiftoAuth) - assert.Equal(t, *testCase.oAuthExpected, *cheCR.Spec.Auth.OpenShiftoAuth) - }) - } -} - func TestEnsureServerExposureStrategy(t *testing.T) { type testCase struct { name string diff --git a/controllers/che/create.go b/controllers/che/create.go index 6896bce8b4..7fa483cf51 100644 --- a/controllers/che/create.go +++ b/controllers/che/create.go @@ -12,6 +12,8 @@ package che import ( + "strconv" + "github.com/eclipse-che/che-operator/pkg/deploy" "github.com/eclipse-che/che-operator/pkg/util" "github.com/sirupsen/logrus" @@ -137,7 +139,7 @@ func (r *CheClusterReconciler) GenerateAndSaveFields(deployContext *deploy.Deplo } } - if !util.IsNativeUserModeEnabled(deployContext.CheCluster) { + if !util.IsOpenShift || !deployContext.CheCluster.IsNativeUserModeEnabled() { keycloakRealm := util.GetValue(deployContext.CheCluster.Spec.Auth.IdentityProviderRealm, cheFlavor) if len(deployContext.CheCluster.Spec.Auth.IdentityProviderRealm) < 1 { deployContext.CheCluster.Spec.Auth.IdentityProviderRealm = keycloakRealm @@ -246,5 +248,13 @@ func (r *CheClusterReconciler) GenerateAndSaveFields(deployContext *deploy.Deplo } } + if util.IsOpenShift && deployContext.CheCluster.Spec.DevWorkspace.Enable && deployContext.CheCluster.Spec.Auth.NativeUserMode == nil { + newNativeUserModeValue := util.NewBoolPointer(true) + deployContext.CheCluster.Spec.Auth.NativeUserMode = newNativeUserModeValue + if err := deploy.UpdateCheCRSpec(deployContext, "nativeUserMode", strconv.FormatBool(*newNativeUserModeValue)); err != nil { + return err + } + } + return nil } diff --git a/controllers/che/gateway_permission.go b/controllers/che/gateway_permission.go index aedf5edd19..ca9aba1511 100644 --- a/controllers/che/gateway_permission.go +++ b/controllers/che/gateway_permission.go @@ -26,7 +26,7 @@ const ( ) func (r *CheClusterReconciler) reconcileGatewayPermissions(deployContext *deploy.DeployContext) (bool, error) { - if util.IsNativeUserModeEnabled(deployContext.CheCluster) { + if util.IsOpenShift && deployContext.CheCluster.IsNativeUserModeEnabled() { name := gatewayPermisisonsName(deployContext.CheCluster) if _, err := deploy.SyncClusterRoleToCluster(deployContext, name, getGatewayClusterRoleRules()); err != nil { return false, err diff --git a/pkg/deploy/dashboard/dashboard.go b/pkg/deploy/dashboard/dashboard.go index 9bcecf5102..cc07e85828 100644 --- a/pkg/deploy/dashboard/dashboard.go +++ b/pkg/deploy/dashboard/dashboard.go @@ -122,7 +122,7 @@ func (d *Dashboard) createGatewayConfig() *gateway.TraefikConfig { 10, "http://"+d.component+":8080", []string{}) - if util.IsNativeUserModeEnabled(d.deployContext.CheCluster) { + if util.IsOpenShift && d.deployContext.CheCluster.IsNativeUserModeEnabled() { cfg.AddAuthHeaderRewrite(d.component) } return cfg diff --git a/pkg/deploy/gateway/gateway.go b/pkg/deploy/gateway/gateway.go index e4a1646c8b..2e8db1e769 100644 --- a/pkg/deploy/gateway/gateway.go +++ b/pkg/deploy/gateway/gateway.go @@ -87,7 +87,7 @@ func syncAll(deployContext *deploy.DeployContext) error { return err } - if util.IsNativeUserModeEnabled(instance) { + if util.IsOpenShift && deployContext.CheCluster.IsNativeUserModeEnabled() { if oauthSecret, err := getGatewaySecretSpec(deployContext); err == nil { if _, err := deploy.Sync(deployContext, oauthSecret, secretDiffOpts); err != nil { return err @@ -273,7 +273,7 @@ func getGatewayServerConfigSpec(deployContext *deploy.DeployContext) (corev1.Con "http://"+deploy.CheServiceName+":8080", []string{}) - if util.IsNativeUserModeEnabled(deployContext.CheCluster) { + if util.IsOpenShift && deployContext.CheCluster.IsNativeUserModeEnabled() { cfg.AddAuthHeaderRewrite(serverComponentName) // native user mode is currently only available on OpenShift but let's be defensive here so that // this doesn't break once we enable it on Kubernetes, too. Token check will have to work @@ -428,7 +428,7 @@ func skipAuthConfig(instance *orgv1.CheCluster) string { if !instance.Spec.Server.ExternalDevfileRegistry { skipAuthPaths = append(skipAuthPaths, "^/"+deploy.DevfileRegistryName) } - if util.IsNativeUserModeEnabled(instance) { + if util.IsOpenShift && instance.IsNativeUserModeEnabled() { skipAuthPaths = append(skipAuthPaths, "/healthz$") } if len(skipAuthPaths) > 0 { @@ -499,7 +499,7 @@ func getGatewayHeaderRewritePluginConfigSpec(instance *orgv1.CheCluster) (*corev func getGatewayTraefikConfigSpec(instance *orgv1.CheCluster) corev1.ConfigMap { traefikPort := GatewayServicePort - if util.IsNativeUserModeEnabled(instance) { + if util.IsOpenShift && instance.IsNativeUserModeEnabled() { traefikPort = 8081 } data := fmt.Sprintf(` @@ -522,7 +522,7 @@ providers: log: level: "INFO"`, traefikPort) - if util.IsNativeUserModeEnabled(instance) { + if util.IsOpenShift && instance.IsNativeUserModeEnabled() { data += ` experimental: localPlugins: @@ -631,7 +631,7 @@ func getContainersSpec(instance *orgv1.CheCluster) []corev1.Container { }, } - if util.IsNativeUserModeEnabled(instance) { + if util.IsOpenShift && instance.IsNativeUserModeEnabled() { containers = append(containers, corev1.Container{ Name: "oauth-proxy", @@ -683,7 +683,7 @@ func getTraefikContainerVolumeMounts(instance *orgv1.CheCluster) []corev1.Volume MountPath: "/dynamic-config", }, } - if util.IsNativeUserModeEnabled(instance) { + if util.IsOpenShift && instance.IsNativeUserModeEnabled() { mounts = append(mounts, corev1.VolumeMount{ Name: "header-rewrite-traefik-plugin", MountPath: "/plugins-local/src/github.com/che-incubator/header-rewrite-traefik-plugin", @@ -713,7 +713,7 @@ func getVolumesSpec(instance *orgv1.CheCluster) []corev1.Volume { }, } - if util.IsNativeUserModeEnabled(instance) { + if util.IsOpenShift && instance.IsNativeUserModeEnabled() { volumes = append(volumes, corev1.Volume{ Name: "oauth-proxy-config", VolumeSource: corev1.VolumeSource{ diff --git a/pkg/deploy/identity-provider/identity_provider.go b/pkg/deploy/identity-provider/identity_provider.go index 63da63535d..ba48e1d181 100644 --- a/pkg/deploy/identity-provider/identity_provider.go +++ b/pkg/deploy/identity-provider/identity_provider.go @@ -49,7 +49,7 @@ var ( // the provisioning is complete, false if requeue of the reconcile request is needed. func SyncIdentityProviderToCluster(deployContext *deploy.DeployContext) (bool, error) { cr := deployContext.CheCluster - if util.IsNativeUserModeEnabled(cr) { + if util.IsOpenShift && deployContext.CheCluster.IsNativeUserModeEnabled() { return syncNativeIdentityProviderItems(deployContext) } else if cr.Spec.Auth.ExternalIdentityProvider { return true, nil @@ -368,7 +368,7 @@ func createGatewayConfig(cheCluster *orgv1.CheCluster) *gateway.TraefikConfig { "http://"+deploy.IdentityProviderName+":8080", []string{}) - if util.IsNativeUserModeEnabled(cheCluster) { + if util.IsOpenShift && cheCluster.IsNativeUserModeEnabled() { cfg.AddAuthHeaderRewrite(deploy.IdentityProviderName) } diff --git a/pkg/deploy/openshift-oauth/openshiftoauth.go b/pkg/deploy/openshift-oauth/openshiftoauth.go new file mode 100644 index 0000000000..9d47e48e61 --- /dev/null +++ b/pkg/deploy/openshift-oauth/openshiftoauth.go @@ -0,0 +1,117 @@ +// +// Copyright (c) 2012-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package openshiftoauth + +import ( + "context" + "reflect" + "strconv" + + "github.com/eclipse-che/che-operator/pkg/deploy" + "github.com/eclipse-che/che-operator/pkg/util" + userv1 "github.com/openshift/api/user/v1" + "github.com/sirupsen/logrus" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + warningNoIdentityProvidersMessage = "No Openshift identity providers." + + AddIdentityProviderMessage = "Openshift oAuth was disabled. How to add identity provider read in the Help Link:" + warningNoRealUsersMessage = "No real users. Openshift oAuth was disabled. How to add new user read in the Help Link:" + failedUnableToGetOpenshiftUsers = "Unable to get users on the OpenShift cluster." + + howToAddIdentityProviderLinkOS4 = "https://docs.openshift.com/container-platform/latest/authentication/understanding-identity-provider.html#identity-provider-overview_understanding-identity-provider" + howToConfigureOAuthLinkOS3 = "https://docs.openshift.com/container-platform/3.11/install_config/configuring_authentication.html" +) + +type OpenShiftOAuth struct { + openShiftOAuthUser *OpenShiftOAuthUser +} + +func NewOpenShiftOAuth(openShiftOAuthUser *OpenShiftOAuthUser) *OpenShiftOAuth { + return &OpenShiftOAuth{ + openShiftOAuthUser, + } +} + +func (oo *OpenShiftOAuth) Reconcile(ctx *deploy.DeployContext) (reconcile.Result, bool, error) { + if util.IsOpenShift && ctx.CheCluster.Spec.Auth.OpenShiftoAuth == nil { + return oo.enableOpenShiftOAuth(ctx) + } + + return reconcile.Result{}, true, nil +} + +func (oo *OpenShiftOAuth) Finalize(ctx *deploy.DeployContext) error { + return nil +} + +func (oo *OpenShiftOAuth) enableOpenShiftOAuth(ctx *deploy.DeployContext) (reconcile.Result, bool, error) { + oauth := false + if util.IsOpenShift4 { + openshitOAuth, err := GetOpenshiftOAuth(ctx) + if err != nil { + logrus.Error("Unable to get Openshift oAuth. Cause: " + err.Error()) + } else { + if len(openshitOAuth.Spec.IdentityProviders) > 0 { + oauth = true + } else if ctx.CheCluster.IsNativeUserModeEnabled() { + // enable OpenShift OAuth without adding initial OpenShift OAuth user + // since kubeadmin is a valid user for native user mode + oauth = true + } else if ctx.CheCluster.IsOpenShiftOAuthUserConfigured() { + provisioned, err := oo.openShiftOAuthUser.Create(ctx) + if err != nil { + logrus.Error(warningNoIdentityProvidersMessage + " Operator tried to create initial OpenShift OAuth user for HTPasswd identity provider, but failed. Cause: " + err.Error()) + logrus.Info("To enable OpenShift OAuth, please add identity provider first: " + howToAddIdentityProviderLinkOS4) + + // Don't try to create initial user any more, che-operator shouldn't hang on this step. + ctx.CheCluster.Spec.Auth.InitialOpenShiftOAuthUser = nil + if err := deploy.UpdateCheCRStatus(ctx, "initialOpenShiftOAuthUser", ""); err != nil { + return reconcile.Result{}, false, err + } + oauth = false + } else { + if !provisioned { + // let's wait some time + return reconcile.Result{}, false, err + } + oauth = true + } + } + } + } else { // Openshift 3 + users := &userv1.UserList{} + listOptions := &client.ListOptions{} + if err := ctx.ClusterAPI.NonCachedClient.List(context.TODO(), users, listOptions); err != nil { + logrus.Error(failedUnableToGetOpenshiftUsers + " Cause: " + err.Error()) + } else { + oauth = len(users.Items) >= 1 + if !oauth { + logrus.Warn(warningNoRealUsersMessage + " " + howToConfigureOAuthLinkOS3) + } + } + } + + newOAuthValue := util.NewBoolPointer(oauth) + if !reflect.DeepEqual(newOAuthValue, ctx.CheCluster.Spec.Auth.OpenShiftoAuth) { + ctx.CheCluster.Spec.Auth.OpenShiftoAuth = newOAuthValue + if err := deploy.UpdateCheCRSpec(ctx, "openShiftoAuth", strconv.FormatBool(oauth)); err != nil { + return reconcile.Result{Requeue: true}, false, err + } + } + + return reconcile.Result{}, true, nil +} diff --git a/pkg/deploy/openshift-oauth/openshiftoauth_test.go b/pkg/deploy/openshift-oauth/openshiftoauth_test.go new file mode 100644 index 0000000000..31ebc03c1c --- /dev/null +++ b/pkg/deploy/openshift-oauth/openshiftoauth_test.go @@ -0,0 +1,234 @@ +// +// Copyright (c) 2012-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package openshiftoauth + +import ( + "os" + "testing" + + orgv1 "github.com/eclipse-che/che-operator/api/v1" + "github.com/eclipse-che/che-operator/pkg/deploy" + "github.com/eclipse-che/che-operator/pkg/util" + configv1 "github.com/openshift/api/config/v1" + oauthv1 "github.com/openshift/api/oauth/v1" + userv1 "github.com/openshift/api/user/v1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +var ( + nonEmptyUserList = &userv1.UserList{ + Items: []userv1.User{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "user1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "user2", + }, + }, + }, + } + oAuthWithNoIdentityProviders = &configv1.OAuth{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + } + oAuthWithIdentityProvider = &configv1.OAuth{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: configv1.OAuthSpec{ + IdentityProviders: []configv1.IdentityProvider{ + { + Name: "htpasswd", + }, + }, + }, + } + proxy = &configv1.Proxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + } +) + +func TestCaseAutoDetectOAuth(t *testing.T) { + type testCase struct { + name string + initObjects []runtime.Object + isOpenshift4 bool + initialOAuthValue *bool + oAuthExpected *bool + initialOpenShiftOAuthUserEnabled *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, + &oauthv1.OAuthClient{}, + }, + isOpenshift4: false, + initialOAuthValue: nil, + oAuthExpected: pointer.BoolPtr(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{}, + &oauthv1.OAuthClient{}, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(false), + oAuthExpected: pointer.BoolPtr(false), + }, + { + name: "che-operator should respect oAuth = true even if there no users on the Openshift 3", + initObjects: []runtime.Object{ + &userv1.UserList{}, + &oauthv1.OAuthClient{}, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(true), + oAuthExpected: pointer.BoolPtr(true), + }, + { + name: "che-operator should respect oAuth = true even if there are some users on the Openshift 3", + initObjects: []runtime.Object{ + nonEmptyUserList, + &oauthv1.OAuthClient{}, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(true), + oAuthExpected: pointer.BoolPtr(true), + }, + { + name: "che-operator should respect oAuth = false even if there are some users on the Openshift 3", + initObjects: []runtime.Object{ + nonEmptyUserList, + &oauthv1.OAuthClient{}, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(false), + oAuthExpected: pointer.BoolPtr(false), + }, + { + name: "che-operator should respect oAuth = false even if no users on the Openshift 3", + initObjects: []runtime.Object{ + &userv1.UserList{}, + &oauthv1.OAuthClient{}, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(false), + oAuthExpected: pointer.BoolPtr(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, + proxy, + }, + isOpenshift4: true, + initialOAuthValue: nil, + oAuthExpected: pointer.BoolPtr(true), + }, + { + name: "che-operator should respect oAuth = true even if there no indentity providers on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithNoIdentityProviders, + proxy, + }, + isOpenshift4: true, + initialOAuthValue: pointer.BoolPtr(true), + oAuthExpected: pointer.BoolPtr(true), + initialOpenShiftOAuthUserEnabled: pointer.BoolPtr(true), + }, + { + name: "che-operator should respect oAuth = true even if there are some users on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithIdentityProvider, + proxy, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(true), + oAuthExpected: pointer.BoolPtr(true), + initialOpenShiftOAuthUserEnabled: pointer.BoolPtr(true), + }, + { + name: "che-operator should respect oAuth = false even if there no indentity providers on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithNoIdentityProviders, + proxy, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(false), + oAuthExpected: pointer.BoolPtr(false), + }, + { + name: "che-operator should respect oAuth = false even if there are some users on the Openshift 4", + initObjects: []runtime.Object{ + oAuthWithIdentityProvider, + proxy, + }, + isOpenshift4: false, + initialOAuthValue: pointer.BoolPtr(false), + oAuthExpected: pointer.BoolPtr(false), + }, + { + name: "che-operator should auto disable oAuth on error retieve identity providers", + initObjects: []runtime.Object{}, + isOpenshift4: false, + initialOAuthValue: nil, + initialOpenShiftOAuthUserEnabled: pointer.BoolPtr(true), + oAuthExpected: pointer.BoolPtr(false), + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) + + checluster := &orgv1.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: os.Getenv("CHE_FLAVOR"), + Namespace: "eclipse-che", + }, + Spec: orgv1.CheClusterSpec{ + Auth: orgv1.CheClusterSpecAuth{ + OpenShiftoAuth: testCase.initialOAuthValue, + InitialOpenShiftOAuthUser: testCase.initialOpenShiftOAuthUserEnabled, + }, + }, + } + + util.IsOpenShift = true + util.IsOpenShift4 = testCase.isOpenshift4 + deployContext := deploy.GetTestDeployContext(checluster, testCase.initObjects) + + openShiftOAuth := NewOpenShiftOAuth(NewOpenShiftOAuthUser()) + _, done, err := openShiftOAuth.Reconcile(deployContext) + assert.Nil(t, err) + assert.True(t, done) + + assert.NotNil(t, deployContext.CheCluster.Spec.Auth.OpenShiftoAuth) + assert.Equal(t, *testCase.oAuthExpected, *deployContext.CheCluster.Spec.Auth.OpenShiftoAuth) + }) + } +} diff --git a/pkg/deploy/server/server_deployment.go b/pkg/deploy/server/server_deployment.go index 4b08d5b18a..2178e471ab 100644 --- a/pkg/deploy/server/server_deployment.go +++ b/pkg/deploy/server/server_deployment.go @@ -161,7 +161,7 @@ func (s Server) getDeploymentSpec() (*appsv1.Deployment, error) { FieldPath: "metadata.namespace"}}, }) - if util.IsNativeUserModeEnabled(s.deployContext.CheCluster) { + if util.IsOpenShift && s.deployContext.CheCluster.IsNativeUserModeEnabled() { cheEnv = append(cheEnv, corev1.EnvVar{ Name: "CHE_AUTH_NATIVEUSER", Value: "true", diff --git a/pkg/util/util.go b/pkg/util/util.go index dd24e15a74..4f902a509a 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -479,11 +479,6 @@ func NewBoolPointer(value bool) *bool { return &variable } -func IsNativeUserModeEnabled(c *orgv1.CheCluster) bool { - // Native user mode is now available only on openshift - return IsOpenShift && c.Spec.Auth.NativeUserMode != nil && *c.Spec.Auth.NativeUserMode -} - // GetWorkspaceNamespaceDefault - returns workspace namespace default strategy, which points on the namespaces used for workspaces execution. func GetWorkspaceNamespaceDefault(cr *orgv1.CheCluster) string { if cr.Spec.Server.CustomCheProperties != nil {