From e1b4541ed27cb36652b45326e932120f993b062b Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Thu, 4 Nov 2021 17:42:49 +0200 Subject: [PATCH] chore: Refactoring OpenShift Initial user reconciler (#1152) * chore: refactoring OpenShift OAuth User Signed-off-by: Anatolii Bazko Signed-off-by: Oleksandr Andriienko Co-authored-by: Oleksandr Andriienko --- api/v1/checluster_types.go | 13 + .../che-operator.clusterserviceversion.yaml | 6 +- .../che-operator.clusterserviceversion.yaml | 6 +- .../che-operator.clusterserviceversion.yaml | 6 +- config/manager/manager.yaml | 2 +- controllers/che/checluster_controller.go | 88 ++--- controllers/che/checluster_controller_test.go | 220 +++++------- controllers/che/checluster_validator.go | 5 +- .../che/oauth_initial_htpasswd_provider.go | 304 ----------------- .../oauth_initial_htpasswd_provider_test.go | 223 ------------ go.mod | 1 + .../oauth_initial_htpasswd_provider_mock.go | 65 ---- .../identity-provider/identity_provider.go | 4 +- .../imagepuller.go} | 64 ++-- .../imagepuller_test.go} | 7 +- pkg/deploy/image-puller/init_test.go | 21 ++ pkg/deploy/openshift-oauth/init_test.go | 21 ++ .../openshift-oauth/openshiftoauth_user.go | 320 ++++++++++++++++++ .../openshiftoauth_user_test.go | 168 +++++++++ .../openshift-oauth/openshiftoauth_util.go | 27 ++ pkg/deploy/reconcile_manager.go | 4 +- pkg/deploy/reconcile_manager_test.go | 4 +- pkg/deploy/server/server_configmap.go | 2 +- pkg/deploy/test_util.go | 11 + pkg/util/util.go | 17 +- vendor/modules.txt | 1 + 26 files changed, 744 insertions(+), 866 deletions(-) delete mode 100644 controllers/che/oauth_initial_htpasswd_provider.go delete mode 100644 controllers/che/oauth_initial_htpasswd_provider_test.go delete mode 100644 mocks/controllers/oauth_initial_htpasswd_provider_mock.go rename pkg/deploy/{kubernetes_image_puller.go => image-puller/imagepuller.go} (91%) rename pkg/deploy/{kubernetes_image_puller_test.go => image-puller/imagepuller_test.go} (99%) create mode 100644 pkg/deploy/image-puller/init_test.go create mode 100644 pkg/deploy/openshift-oauth/init_test.go create mode 100644 pkg/deploy/openshift-oauth/openshiftoauth_user.go create mode 100644 pkg/deploy/openshift-oauth/openshiftoauth_user_test.go create mode 100644 pkg/deploy/openshift-oauth/openshiftoauth_util.go diff --git a/api/v1/checluster_types.go b/api/v1/checluster_types.go index 77691184b..84275a30b 100644 --- a/api/v1/checluster_types.go +++ b/api/v1/checluster_types.go @@ -781,3 +781,16 @@ func (c *CheCluster) IsImagePullerImagesEmpty() bool { func (c *CheCluster) IsInternalClusterSVCNamesEnabled() bool { return c.Spec.Server.DisableInternalClusterSVCNames == nil || !*c.Spec.Server.DisableInternalClusterSVCNames } + +// IsInitialOpenShiftOAuthUserEnabled returns true when initial Openshift oAuth user is enabled for CheCluster resource, otherwise false. +func (c *CheCluster) IsOpenShiftOAuthUserConfigured() bool { + return c.Spec.Auth.InitialOpenShiftOAuthUser != nil && *c.Spec.Auth.InitialOpenShiftOAuthUser +} + +func (c *CheCluster) IsOpenShiftOAuthUserMustBeDeleted() bool { + return c.Spec.Auth.InitialOpenShiftOAuthUser != nil && !*c.Spec.Auth.InitialOpenShiftOAuthUser +} + +func (c *CheCluster) IsOpenShiftOAuthEnabled() bool { + return c.Spec.Auth.OpenShiftoAuth != nil && *c.Spec.Auth.OpenShiftoAuth +} diff --git a/bundle/next-all-namespaces/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml b/bundle/next-all-namespaces/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml index 23fd25f25..686e44cf8 100644 --- a/bundle/next-all-namespaces/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml +++ b/bundle/next-all-namespaces/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml @@ -114,7 +114,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che-preview-openshift.v7.38.0-365.next + name: eclipse-che-preview-openshift.v7.38.0-366.next namespace: placeholder spec: apiservicedefinitions: {} @@ -1084,7 +1084,7 @@ spec: - name: RELATED_IMAGE_devfile_registry value: quay.io/eclipse/che-devfile-registry:next - name: RELATED_IMAGE_pvc_jobs - value: registry.access.redhat.com/ubi8-minimal:8.4-210 + value: registry.access.redhat.com/ubi8-minimal:8.4-212 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_postgres_13_3 @@ -1414,4 +1414,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.38.0-365.next + version: 7.38.0-366.next diff --git a/bundle/next/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml b/bundle/next/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml index 9733c75cd..934f23cc9 100644 --- a/bundle/next/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml +++ b/bundle/next/eclipse-che-preview-kubernetes/manifests/che-operator.clusterserviceversion.yaml @@ -121,7 +121,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che-preview-kubernetes.v7.38.0-326.next + name: eclipse-che-preview-kubernetes.v7.38.0-327.next namespace: placeholder spec: apiservicedefinitions: {} @@ -1073,7 +1073,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.4-210 + value: registry.access.redhat.com/ubi8-minimal:8.4-212 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_postgres_13_3 @@ -1381,4 +1381,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.38.0-326.next + version: 7.38.0-327.next diff --git a/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml b/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml index f608da4aa..635521639 100644 --- a/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml +++ b/bundle/next/eclipse-che-preview-openshift/manifests/che-operator.clusterserviceversion.yaml @@ -114,7 +114,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: https://github.com/eclipse-che/che-operator support: Eclipse Foundation - name: eclipse-che-preview-openshift.v7.38.0-333.next + name: eclipse-che-preview-openshift.v7.38.0-334.next namespace: placeholder spec: apiservicedefinitions: {} @@ -1084,7 +1084,7 @@ spec: - name: RELATED_IMAGE_devfile_registry value: quay.io/eclipse/che-devfile-registry:next - name: RELATED_IMAGE_pvc_jobs - value: registry.access.redhat.com/ubi8-minimal:8.4-210 + value: registry.access.redhat.com/ubi8-minimal:8.4-212 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_postgres_13_3 @@ -1414,4 +1414,4 @@ spec: maturity: stable provider: name: Eclipse Foundation - version: 7.38.0-333.next + version: 7.38.0-334.next diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 793f979b0..1f9b00a79 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -68,7 +68,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.4-210 + value: registry.access.redhat.com/ubi8-minimal:8.4-212 - name: RELATED_IMAGE_postgres value: quay.io/eclipse/che--centos--postgresql-96-centos7:9.6-b681d78125361519180a6ac05242c296f8906c11eab7e207b5ca9a89b6344392 - name: RELATED_IMAGE_postgres_13_3 diff --git a/controllers/che/checluster_controller.go b/controllers/che/checluster_controller.go index 48e75cbf5..b680145b2 100644 --- a/controllers/che/checluster_controller.go +++ b/controllers/che/checluster_controller.go @@ -24,6 +24,8 @@ import ( devworkspace "github.com/eclipse-che/che-operator/pkg/deploy/dev-workspace" "github.com/eclipse-che/che-operator/pkg/deploy/devfileregistry" "github.com/eclipse-che/che-operator/pkg/deploy/gateway" + imagepuller "github.com/eclipse-che/che-operator/pkg/deploy/image-puller" + openshiftoauth "github.com/eclipse-che/che-operator/pkg/deploy/openshift-oauth" "github.com/eclipse-che/che-operator/pkg/deploy/pluginregistry" "github.com/eclipse-che/che-operator/pkg/deploy/postgres" "github.com/eclipse-che/che-operator/pkg/deploy/server" @@ -80,10 +82,10 @@ 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 - userHandler OpenShiftOAuthUserHandler - reconcileManager *deploy.ReconcileManager + discoveryClient discovery.DiscoveryInterface + tests bool + openShiftOAuthUser openshiftoauth.IOpenShiftOAuthUser + reconcileManager *deploy.ReconcileManager // the namespace to which to limit the reconciliation. If empty, all namespaces are considered namespace string } @@ -102,18 +104,21 @@ func NewReconciler( if !util.IsTestMode() { reconcileManager.RegisterReconciler(NewCheClusterValidator()) } - reconcileManager.RegisterReconciler(deploy.NewImagePuller()) + reconcileManager.RegisterReconciler(imagepuller.NewImagePuller()) + + openShiftOAuthUser := openshiftoauth.NewOpenShiftOAuthUser() + reconcileManager.RegisterReconciler(openShiftOAuthUser) return &CheClusterReconciler{ Scheme: scheme, Log: ctrl.Log.WithName("controllers").WithName("CheCluster"), - client: k8sclient, - nonCachedClient: noncachedClient, - discoveryClient: discoveryClient, - userHandler: NewOpenShiftOAuthUserHandler(noncachedClient), - namespace: namespace, - reconcileManager: reconcileManager, + client: k8sclient, + nonCachedClient: noncachedClient, + discoveryClient: discoveryClient, + openShiftOAuthUser: openShiftOAuthUser, + namespace: namespace, + reconcileManager: reconcileManager, } } @@ -297,43 +302,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.IsOpenShift4 && util.IsDeleteOAuthInitialUser(checluster) { - if err := r.userHandler.DeleteOAuthInitialUser(deployContext); err != nil { - logrus.Errorf("Unable to delete initial OpenShift OAuth user from a cluster. Cause: %s", err.Error()) - checluster.Spec.Auth.InitialOpenShiftOAuthUser = nil - err := deploy.UpdateCheCRSpec(deployContext, "initialOpenShiftOAuthUser", "nil") - return reconcile.Result{}, err - } - - checluster.Spec.Auth.OpenShiftoAuth = nil - checluster.Spec.Auth.InitialOpenShiftOAuthUser = nil - updateFields := map[string]string{ - "openShiftoAuth": "nil", - "initialOpenShiftOAuthUser": "nil", - } - - if err := deploy.UpdateCheCRSpecByFields(deployContext, updateFields); err != nil { - return reconcile.Result{}, err - } - - return ctrl.Result{}, nil - } - - // Update status if OpenShift initial user is deleted (in the previous step) - if checluster.Spec.Auth.InitialOpenShiftOAuthUser == nil && checluster.Status.OpenShiftOAuthUserCredentialsSecret != "" { - secret := &corev1.Secret{} - exists, err := getOpenShiftOAuthUserCredentialsSecret(deployContext, secret) - if err != nil { - // We should `Requeue` since we deal with cluster scope objects - return ctrl.Result{RequeueAfter: time.Second}, err - } else if !exists { - checluster.Status.OpenShiftOAuthUserCredentialsSecret = "" - if err := deploy.UpdateCheCRStatus(deployContext, "openShiftOAuthUserCredentialsSecret", ""); err != nil { - return reconcile.Result{}, err - } - } - } - if util.IsOpenShift && checluster.Spec.DevWorkspace.Enable && checluster.Spec.Auth.NativeUserMode == nil { newNativeUserModeValue := util.NewBoolPointer(true) checluster.Spec.Auth.NativeUserMode = newNativeUserModeValue @@ -393,13 +361,13 @@ func (r *CheClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) // 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) - (util.IsOpenShift4 && util.IsOAuthEnabled(checluster) && !checluster.Spec.Server.TlsSupport) { + (util.IsOpenShift4 && checluster.IsOpenShiftOAuthEnabled() && !checluster.Spec.Server.TlsSupport) { if err := deploy.CreateTLSSecretFromEndpoint(deployContext, "", deploy.CheTLSSelfSignedCertificateSecretName); err != nil { return ctrl.Result{}, err } } - if util.IsOAuthEnabled(checluster) { + if util.IsOpenShift && checluster.IsOpenShiftOAuthEnabled() { // create a secret with OpenShift API crt to be added to keystore that RH SSO will consume apiUrl, apiInternalUrl, err := util.GetOpenShiftAPIUrls() if err != nil { @@ -648,7 +616,7 @@ func (r *CheClusterReconciler) autoEnableOAuth(deployContext *deploy.DeployConte oauth := false cr := deployContext.CheCluster if util.IsOpenShift4 { - openshitOAuth, err := GetOpenshiftOAuth(deployContext.ClusterAPI.NonCachedClient) + openshitOAuth, err := openshiftoauth.GetOpenshiftOAuth(deployContext) if err != nil { logrus.Error("Unable to get Openshift oAuth. Cause: " + err.Error()) } else { @@ -658,8 +626,8 @@ func (r *CheClusterReconciler) autoEnableOAuth(deployContext *deploy.DeployConte // enable OpenShift OAuth without adding initial OpenShift OAuth user // since kubeadmin is a valid user for native user mode oauth = true - } else if util.IsInitialOpenShiftOAuthUserEnabled(cr) { - provisioned, err := r.userHandler.SyncOAuthInitialUser(openshitOAuth, deployContext) + } 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) @@ -674,12 +642,6 @@ func (r *CheClusterReconciler) autoEnableOAuth(deployContext *deploy.DeployConte return reconcile.Result{}, err } oauth = true - if deployContext.CheCluster.Status.OpenShiftOAuthUserCredentialsSecret == "" { - deployContext.CheCluster.Status.OpenShiftOAuthUserCredentialsSecret = openShiftOAuthUserCredentialsSecret - if err := deploy.UpdateCheCRStatus(deployContext, "openShiftOAuthUserCredentialsSecret", openShiftOAuthUserCredentialsSecret); err != nil { - return reconcile.Result{}, err - } - } } } } @@ -708,18 +670,12 @@ func (r *CheClusterReconciler) autoEnableOAuth(deployContext *deploy.DeployConte } func (r *CheClusterReconciler) reconcileFinalizers(deployContext *deploy.DeployContext) { - if util.IsOpenShift && util.IsOAuthEnabled(deployContext.CheCluster) { + if util.IsOpenShift && deployContext.CheCluster.IsOpenShiftOAuthEnabled() { if err := deploy.ReconcileOAuthClientFinalizer(deployContext); err != nil { logrus.Error(err) } } - if util.IsOpenShift4 && util.IsInitialOpenShiftOAuthUserEnabled(deployContext.CheCluster) { - if !deployContext.CheCluster.ObjectMeta.DeletionTimestamp.IsZero() { - r.userHandler.DeleteOAuthInitialUser(deployContext) - } - } - if util.IsNativeUserModeEnabled(deployContext.CheCluster) { 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 0607608ad..d58806979 100644 --- a/controllers/che/checluster_controller_test.go +++ b/controllers/che/checluster_controller_test.go @@ -14,11 +14,11 @@ package che import ( "context" "fmt" - "io/ioutil" "os" "strconv" mocks "github.com/eclipse-che/che-operator/mocks" + "github.com/stretchr/testify/assert" "reflect" "time" @@ -37,7 +37,7 @@ import ( console "github.com/openshift/api/console/v1" orgv1 "github.com/eclipse-che/che-operator/api/v1" - oauth "github.com/openshift/api/oauth/v1" + oauthv1 "github.com/openshift/api/oauth/v1" routev1 "github.com/openshift/api/route/v1" userv1 "github.com/openshift/api/user/v1" operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" @@ -48,8 +48,8 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbac "k8s.io/api/rbac/v1" + "k8s.io/utils/pointer" - che_mocks "github.com/eclipse-che/che-operator/mocks/controllers" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -65,7 +65,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/yaml" "testing" ) @@ -86,7 +85,6 @@ var ( }, }, } - oAuthClient = &oauth.OAuthClient{} oAuthWithNoIdentityProviders = &configv1.OAuth{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster", @@ -106,7 +104,6 @@ var ( }, }, } - route = &routev1.Route{} proxy = &configv1.Proxy{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster", @@ -114,17 +111,6 @@ var ( } ) -func init() { - operator := &appsv1.Deployment{} - data, err := ioutil.ReadFile("../../config/manager/manager.yaml") - yaml.Unmarshal(data, operator) - if err == nil { - for _, env := range operator.Spec.Template.Spec.Containers[0].Env { - os.Setenv(env.Name, env.Value) - } - } -} - func TestNativeUserModeEnabled(t *testing.T) { type testCase struct { name string @@ -133,7 +119,6 @@ func TestNativeUserModeEnabled(t *testing.T) { devworkspaceEnabled bool initialNativeUserValue *bool expectedNativeUserValue *bool - mockFunction func(ctrl *gomock.Controller, crNamespace string, usernamePrefix string) *che_mocks.MockOpenShiftOAuthUserHandler } testCases := []testCase{ @@ -142,21 +127,21 @@ func TestNativeUserModeEnabled(t *testing.T) { isOpenshift: true, devworkspaceEnabled: true, initialNativeUserValue: nil, - expectedNativeUserValue: util.NewBoolPointer(true), + expectedNativeUserValue: pointer.BoolPtr(true), }, { name: "che-operator should use nativeUserMode value from initial CR", isOpenshift: true, devworkspaceEnabled: true, - initialNativeUserValue: util.NewBoolPointer(false), - expectedNativeUserValue: util.NewBoolPointer(false), + initialNativeUserValue: pointer.BoolPtr(false), + expectedNativeUserValue: pointer.BoolPtr(false), }, { name: "che-operator should use nativeUserMode value from initial CR", isOpenshift: true, devworkspaceEnabled: true, - initialNativeUserValue: util.NewBoolPointer(true), - expectedNativeUserValue: util.NewBoolPointer(true), + initialNativeUserValue: pointer.BoolPtr(true), + expectedNativeUserValue: pointer.BoolPtr(true), }, { name: "che-operator should not modify nativeUserMode when not on openshift", @@ -180,8 +165,8 @@ func TestNativeUserModeEnabled(t *testing.T) { scheme := scheme.Scheme orgv1.SchemeBuilder.AddToScheme(scheme) - scheme.AddKnownTypes(routev1.GroupVersion, route) - scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuthClient) + scheme.AddKnownTypes(routev1.GroupVersion, &routev1.Route{}) + scheme.AddKnownTypes(oauthv1.SchemeGroupVersion, &oauthv1.OAuthClient{}) scheme.AddKnownTypes(configv1.SchemeGroupVersion, &configv1.Proxy{}) scheme.AddKnownTypes(crdv1.SchemeGroupVersion, &crdv1.CustomResourceDefinition{}) @@ -244,14 +229,12 @@ func TestNativeUserModeEnabled(t *testing.T) { func TestCaseAutoDetectOAuth(t *testing.T) { type testCase struct { - name string - initObjects []runtime.Object - isOpenshift3 bool - initialOAuthValue *bool - oAuthExpected *bool - initialOpenShiftOAuthUserEnabled *bool - OpenShiftOAuthUserCredentialsSecret string - mockFunction func(ctrl *gomock.Controller, crNamespace string, usernamePrefix string) *che_mocks.MockOpenShiftOAuthUserHandler + name string + initObjects []runtime.Object + isOpenshift4 bool + initialOAuthValue *bool + oAuthExpected *bool + initialOpenShiftOAuthUserEnabled *bool } testCases := []testCase{ @@ -259,61 +242,61 @@ func TestCaseAutoDetectOAuth(t *testing.T) { 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{}, + &oauthv1.OAuthClient{}, }, - isOpenshift3: true, + isOpenshift4: false, initialOAuthValue: nil, - oAuthExpected: util.NewBoolPointer(true), + 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{}, - &oauth.OAuthClient{}, + &oauthv1.OAuthClient{}, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(false), - oAuthExpected: util.NewBoolPointer(false), + 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{}, - &oauth.OAuthClient{}, + &oauthv1.OAuthClient{}, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(true), - oAuthExpected: util.NewBoolPointer(true), + 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, - &oauth.OAuthClient{}, + &oauthv1.OAuthClient{}, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(true), - oAuthExpected: util.NewBoolPointer(true), + 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, - &oauth.OAuthClient{}, + &oauthv1.OAuthClient{}, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(false), - oAuthExpected: util.NewBoolPointer(false), + 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{}, - &oauth.OAuthClient{}, + &oauthv1.OAuthClient{}, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(false), - oAuthExpected: util.NewBoolPointer(false), + 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", @@ -321,9 +304,9 @@ func TestCaseAutoDetectOAuth(t *testing.T) { oAuthWithIdentityProvider, proxy, }, - isOpenshift3: false, + isOpenshift4: true, initialOAuthValue: nil, - oAuthExpected: util.NewBoolPointer(true), + oAuthExpected: pointer.BoolPtr(true), }, { name: "che-operator should respect oAuth = true even if there no indentity providers on the Openshift 4", @@ -331,21 +314,10 @@ func TestCaseAutoDetectOAuth(t *testing.T) { oAuthWithNoIdentityProviders, proxy, }, - isOpenshift3: false, - initialOAuthValue: util.NewBoolPointer(true), - oAuthExpected: util.NewBoolPointer(true), - initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(true), - }, - { - name: "che-operator should not create initial user and enable oAuth, when oAuth = true, initialOpenShiftOAuthUserEnabled = true and there no indentity providers on the Openshift 4", - initObjects: []runtime.Object{ - oAuthWithNoIdentityProviders, - proxy, - }, - isOpenshift3: false, - initialOAuthValue: util.NewBoolPointer(true), - oAuthExpected: util.NewBoolPointer(true), - initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(false), + 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", @@ -353,10 +325,10 @@ func TestCaseAutoDetectOAuth(t *testing.T) { oAuthWithIdentityProvider, proxy, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(true), - oAuthExpected: util.NewBoolPointer(true), - initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(true), + 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", @@ -364,9 +336,9 @@ func TestCaseAutoDetectOAuth(t *testing.T) { oAuthWithNoIdentityProviders, proxy, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(false), - oAuthExpected: util.NewBoolPointer(false), + 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", @@ -374,55 +346,35 @@ func TestCaseAutoDetectOAuth(t *testing.T) { oAuthWithIdentityProvider, proxy, }, - isOpenshift3: true, - initialOAuthValue: util.NewBoolPointer(false), - oAuthExpected: util.NewBoolPointer(false), + 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{}, - isOpenshift3: true, + isOpenshift4: false, initialOAuthValue: nil, - initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(true), - oAuthExpected: util.NewBoolPointer(false), + 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))) - scheme := scheme.Scheme - orgv1.SchemeBuilder.AddToScheme(scheme) - scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuthClient) - scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.UserList{}, &userv1.User{}) - scheme.AddKnownTypes(configv1.SchemeGroupVersion, &configv1.OAuth{}, &configv1.Proxy{}) - scheme.AddKnownTypes(routev1.GroupVersion, route) - scheme.AddKnownTypes(configv1.GroupVersion, &configv1.Proxy{}) initCR := InitCheWithSimpleCR().DeepCopy() initCR.Spec.Auth.OpenShiftoAuth = testCase.initialOAuthValue - testCase.initObjects = append(testCase.initObjects, initCR) initCR.Spec.Auth.InitialOpenShiftOAuthUser = testCase.initialOpenShiftOAuthUserEnabled - 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{} + deployContext := deploy.GetTestDeployContext(initCR, testCase.initObjects) - if !ok { - t.Fatal("Error creating fake discovery client") - } - - // prepare mocks - var userHandlerMock *che_mocks.MockOpenShiftOAuthUserHandler - if testCase.mockFunction != nil { - ctrl := gomock.NewController(t) - userHandlerMock = testCase.mockFunction(ctrl, initCR.Namespace, deploy.DefaultCheFlavor(initCR)) - defer ctrl.Finish() - } - - r := NewReconciler(cli, nonCachedClient, fakeDiscovery, scheme, "") - r.userHandler = userHandlerMock + r := NewReconciler( + deployContext.ClusterAPI.Client, + deployContext.ClusterAPI.NonCachedClient, + deployContext.ClusterAPI.DiscoveryClient, + deployContext.ClusterAPI.Client.Scheme(), + "") r.tests = true req := reconcile.Request{ @@ -433,29 +385,17 @@ func TestCaseAutoDetectOAuth(t *testing.T) { } util.IsOpenShift = true - util.IsOpenShift4 = !testCase.isOpenshift3 + util.IsOpenShift4 = testCase.isOpenshift4 _, err := r.Reconcile(context.TODO(), req) - if err != nil { - t.Fatalf("Error reconciling: %v", err) - } + assert.Nil(t, err) cheCR := &orgv1.CheCluster{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: os.Getenv("CHE_FLAVOR"), Namespace: namespace}, cheCR); err != nil { - t.Errorf("CR not found") - } + err = r.client.Get(context.TODO(), types.NamespacedName{Name: os.Getenv("CHE_FLAVOR"), Namespace: namespace}, cheCR) + assert.Nil(t, err) - 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) - } - - if cheCR.Status.OpenShiftOAuthUserCredentialsSecret != testCase.OpenShiftOAuthUserCredentialsSecret { - t.Errorf("Expected initial openshift oAuth user secret %s in the CR status", testCase.OpenShiftOAuthUserCredentialsSecret) - } + assert.NotNil(t, cheCR.Spec.Auth.OpenShiftoAuth) + assert.Equal(t, *testCase.oAuthExpected, *cheCR.Spec.Auth.OpenShiftoAuth) }) } } @@ -524,6 +464,8 @@ func TestEnsureServerExposureStrategy(t *testing.T) { }, } + util.IsOpenShift = true + util.IsOpenShift4 = false _, err := r.Reconcile(context.TODO(), req) if err != nil { t.Fatalf("Error reconciling: %v", err) @@ -823,7 +765,7 @@ func TestCheController(t *testing.T) { } oAuthClientName := cheCR.Spec.Auth.OAuthClientName oauthSecret := cheCR.Spec.Auth.OAuthSecret - oAuthClient := &oauth.OAuthClient{} + oAuthClient := &oauthv1.OAuthClient{} if err = r.client.Get(context.TODO(), types.NamespacedName{Name: oAuthClientName, Namespace: ""}, oAuthClient); err != nil { t.Errorf("Failed to Get oAuthClient %s: %s", oAuthClient.Name, err) } @@ -896,7 +838,7 @@ func TestCheController(t *testing.T) { t.Fatal("Failed to reconcile oAuthClient") } oauthClientName := cheCR.Spec.Auth.OAuthClientName - oauthClient := &oauth.OAuthClient{} + oauthClient := &oauthv1.OAuthClient{} err = r.nonCachedClient.Get(context.TODO(), types.NamespacedName{Name: oAuthClientName}, oauthClient) if err == nil { t.Fatalf("OauthClient %s has not been deleted", oauthClientName) @@ -1021,13 +963,13 @@ func TestShouldDelegatePermissionsForCheWorkspaces(t *testing.T) { scheme := scheme.Scheme orgv1.SchemeBuilder.AddToScheme(scheme) - scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuthClient) + scheme.AddKnownTypes(oauthv1.SchemeGroupVersion, &oauthv1.OAuthClient{}) scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.UserList{}, &userv1.User{}) scheme.AddKnownTypes(configv1.SchemeGroupVersion, &configv1.OAuth{}, &configv1.Proxy{}) - scheme.AddKnownTypes(routev1.GroupVersion, route) + scheme.AddKnownTypes(routev1.GroupVersion, &routev1.Route{}) initCR := testCase.checluster - initCR.Spec.Auth.OpenShiftoAuth = util.NewBoolPointer(false) + initCR.Spec.Auth.OpenShiftoAuth = pointer.BoolPtr(false) testCase.initObjects = append(testCase.initObjects, initCR) cli := fake.NewFakeClientWithScheme(scheme, testCase.initObjects...) @@ -1119,12 +1061,12 @@ func TestShouldDelegatePermissionsForCheWorkspaces(t *testing.T) { func Init() (client.Client, discovery.DiscoveryInterface, runtime.Scheme) { objs, ds, scheme := createAPIObjects() - oAuthClient := &oauth.OAuthClient{} + oAuthClient := &oauthv1.OAuthClient{} users := &userv1.UserList{} user := &userv1.User{} // Register operator types with the runtime scheme - scheme.AddKnownTypes(oauth.SchemeGroupVersion, oAuthClient) + scheme.AddKnownTypes(oauthv1.SchemeGroupVersion, oAuthClient) scheme.AddKnownTypes(userv1.SchemeGroupVersion, users, user) scheme.AddKnownTypes(configv1.SchemeGroupVersion, &configv1.Proxy{}) @@ -1201,7 +1143,7 @@ func InitCheWithSimpleCR() *orgv1.CheCluster { CheWorkspaceClusterRole: "cluster-admin", }, Auth: orgv1.CheClusterSpecAuth{ - OpenShiftoAuth: util.NewBoolPointer(false), + OpenShiftoAuth: pointer.BoolPtr(false), }, }, } diff --git a/controllers/che/checluster_validator.go b/controllers/che/checluster_validator.go index 770a8de21..7d045189f 100644 --- a/controllers/che/checluster_validator.go +++ b/controllers/che/checluster_validator.go @@ -31,6 +31,7 @@ const ( // - self-contradictory configurations // - configurations with which it is impossible to deploy Che type CheClusterValidator struct { + deploy.Reconcilable } func NewCheClusterValidator() *CheClusterValidator { @@ -56,6 +57,6 @@ func (v *CheClusterValidator) Reconcile(ctx *deploy.DeployContext) (reconcile.Re return reconcile.Result{}, true, nil } -func (v *CheClusterValidator) Finalize(ctx *deploy.DeployContext) (bool, error) { - return true, nil +func (v *CheClusterValidator) Finalize(ctx *deploy.DeployContext) error { + return nil } diff --git a/controllers/che/oauth_initial_htpasswd_provider.go b/controllers/che/oauth_initial_htpasswd_provider.go deleted file mode 100644 index fe84d9fca..000000000 --- a/controllers/che/oauth_initial_htpasswd_provider.go +++ /dev/null @@ -1,304 +0,0 @@ -// -// 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 che - -import ( - "context" - - errorMsg "errors" - - "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/config/v1" - userv1 "github.com/openshift/api/user/v1" - "github.com/sirupsen/logrus" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -const ( - htpasswdIdentityProviderName = "htpasswd-eclipse-che" - htpasswdSecretName = "htpasswd-eclipse-che" - ocConfigNamespace = "openshift-config" - openShiftOAuthUserCredentialsSecret = "openshift-oauth-user-credentials" - openshiftOauthUserFinalizerName = "openshift-oauth-user.finalizers.che.eclipse.org" -) - -var ( - password = util.GeneratePasswd(6) - htpasswdFileContent string -) - -// OpenShiftOAuthUserHandler - handler to create or delete new Openshift oAuth user. -type OpenShiftOAuthUserHandler interface { - SyncOAuthInitialUser(openshiftOAuth *oauthv1.OAuth, deployContext *deploy.DeployContext) (bool, error) - DeleteOAuthInitialUser(deployContext *deploy.DeployContext) error -} - -// OpenShiftOAuthUserOperatorHandler - OpenShiftOAuthUserHandler implementation. -type OpenShiftOAuthUserOperatorHandler struct { - OpenShiftOAuthUserHandler - runtimeClient client.Client - runnable util.Runnable -} - -// NewOpenShiftOAuthUserHandler - create new OpenShiftOAuthUserHandler instance -func NewOpenShiftOAuthUserHandler(runtimeClient client.Client) OpenShiftOAuthUserHandler { - return &OpenShiftOAuthUserOperatorHandler{ - runtimeClient: runtimeClient, - // real process implementation. In the test we are using mock. - runnable: util.NewRunnable(), - } -} - -// SyncOAuthInitialUser - creates new htpasswd provider with initial user with Che flavor name -// if Openshift cluster hasn't got identity providers, otherwise do nothing. -// It usefull for good first user experience. -// User can't use kube:admin or system:admin user in the Openshift oAuth. That's why we provide -// initial user for good first meeting with Eclipse Che. -func (iuh *OpenShiftOAuthUserOperatorHandler) SyncOAuthInitialUser(openshiftOAuth *oauthv1.OAuth, deployContext *deploy.DeployContext) (bool, error) { - cr := deployContext.CheCluster - userName := deploy.DefaultCheFlavor(cr) - if htpasswdFileContent == "" { - var err error - if htpasswdFileContent, err = iuh.generateHtPasswdUserInfo(userName, password); err != nil { - return false, err - } - } - - var storedPassword string - - // read existed password from the secret (operator has been restarted case) - secret := &corev1.Secret{} - exists, err := getOpenShiftOAuthUserCredentialsSecret(deployContext, secret) - if err != nil { - return false, err - } else if exists { - storedPassword = string(secret.Data["password"]) - } - - if storedPassword != "" && password != storedPassword { - password = storedPassword - if htpasswdFileContent, err = iuh.generateHtPasswdUserInfo(userName, password); err != nil { - return false, err - } - } - - initialUserSecretData := map[string][]byte{"user": []byte(userName), "password": []byte(password)} - done, err := deploy.SyncSecretToCluster(deployContext, openShiftOAuthUserCredentialsSecret, ocConfigNamespace, initialUserSecretData) - if !done { - return false, err - } - - htpasswdFileSecretData := map[string][]byte{"htpasswd": []byte(htpasswdFileContent)} - done, err = deploy.SyncSecretToCluster(deployContext, htpasswdSecretName, ocConfigNamespace, htpasswdFileSecretData) - if !done { - return false, err - } - - if err := appendIdentityProvider(openshiftOAuth, iuh.runtimeClient); err != nil { - return false, err - } - - if err := deploy.AppendFinalizer(deployContext, openshiftOauthUserFinalizerName); err != nil { - return false, err - } - - return true, nil -} - -// DeleteOAuthInitialUser - removes initial user, htpasswd provider, htpasswd secret and Che secret with username and password. -func (iuh *OpenShiftOAuthUserOperatorHandler) DeleteOAuthInitialUser(deployContext *deploy.DeployContext) error { - oAuth, err := GetOpenshiftOAuth(iuh.runtimeClient) - if err != nil { - return err - } - - cr := deployContext.CheCluster - userName := deploy.DefaultCheFlavor(cr) - - if err := deleteUser(iuh.runtimeClient, userName); err != nil { - return err - } - - if err := deleteUserIdentity(iuh.runtimeClient, userName); err != nil { - return err - } - - if err := deleteIdentityProvider(oAuth, iuh.runtimeClient); err != nil { - return err - } - - _, err = deploy.Delete(deployContext, types.NamespacedName{Name: htpasswdSecretName, Namespace: ocConfigNamespace}, &corev1.Secret{}) - if err != nil { - return err - } - - // legacy secret in the current namespace - _, err = deploy.DeleteNamespacedObject(deployContext, openShiftOAuthUserCredentialsSecret, &corev1.Secret{}) - if err != nil { - return err - } - - _, err = deploy.Delete(deployContext, types.NamespacedName{Name: openShiftOAuthUserCredentialsSecret, Namespace: ocConfigNamespace}, &corev1.Secret{}) - if err != nil { - return err - } - - if err := deploy.DeleteFinalizer(deployContext, openshiftOauthUserFinalizerName); err != nil { - return err - } - - return nil -} - -func (iuh *OpenShiftOAuthUserOperatorHandler) generateHtPasswdUserInfo(userName string, password string) (string, error) { - logrus.Info("Generate initial user httpasswd info") - - err := iuh.runnable.Run("htpasswd", "-nbB", userName, password) - if err != nil { - return "", err - } - - if len(iuh.runnable.GetStdErr()) > 0 { - return "", errorMsg.New("Failed to generate data for HTPasswd identity provider: " + iuh.runnable.GetStdErr()) - } - return iuh.runnable.GetStdOut(), nil -} - -// GetOpenshiftOAuth returns Openshift oAuth object. -func GetOpenshiftOAuth(runtimeClient client.Client) (*oauthv1.OAuth, error) { - oAuth := &oauthv1.OAuth{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, oAuth); err != nil { - return nil, err - } - return oAuth, nil -} - -func identityProviderExists(providerName string, oAuth *oauthv1.OAuth) bool { - if len(oAuth.Spec.IdentityProviders) == 0 { - return false - } - for _, identityProvider := range oAuth.Spec.IdentityProviders { - if identityProvider.Name == providerName { - return true - } - } - return false -} - -// read from the secret from `openshift-config` namespace -// and then from the legacy secret from the current namespace -func getOpenShiftOAuthUserCredentialsSecret(deployContext *deploy.DeployContext, secret *corev1.Secret) (bool, error) { - exists, err := deploy.Get(deployContext, types.NamespacedName{Name: openShiftOAuthUserCredentialsSecret, Namespace: ocConfigNamespace}, secret) - if err != nil { - return false, err - } else if exists { - return true, nil - } - - return deploy.GetNamespacedObject(deployContext, openShiftOAuthUserCredentialsSecret, secret) -} - -func appendIdentityProvider(oAuth *oauthv1.OAuth, runtimeClient client.Client) error { - logrus.Info("Add initial user httpasswd provider to the oAuth") - - htpasswdProvider := newHtpasswdProvider() - if !identityProviderExists(htpasswdProvider.Name, oAuth) { - oauthPatch := client.MergeFrom(oAuth.DeepCopy()) - - oAuth.Spec.IdentityProviders = append(oAuth.Spec.IdentityProviders, *htpasswdProvider) - - if err := runtimeClient.Patch(context.TODO(), oAuth, oauthPatch); err != nil { - return err - } - } - - return nil -} - -func newHtpasswdProvider() *oauthv1.IdentityProvider { - return &oauthv1.IdentityProvider{ - Name: htpasswdIdentityProviderName, - MappingMethod: configv1.MappingMethodClaim, - IdentityProviderConfig: oauthv1.IdentityProviderConfig{ - Type: "HTPasswd", - HTPasswd: &oauthv1.HTPasswdIdentityProvider{ - FileData: oauthv1.SecretNameReference{Name: htpasswdSecretName}, - }, - }, - } -} - -func deleteUser(runtimeClient client.Client, userName string) error { - logrus.Infof("Delete initial user: %s", userName) - - user := &userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: userName, - }, - } - - if err := runtimeClient.Delete(context.TODO(), user); err != nil { - if errors.IsNotFound(err) { - return nil - } - return err - } - - return nil -} - -func deleteUserIdentity(runtimeClient client.Client, userName string) error { - identityName := htpasswdIdentityProviderName + ":" + userName - logrus.Infof("Delete initial user identity: %s", identityName) - - identity := &userv1.Identity{ - ObjectMeta: metav1.ObjectMeta{ - Name: identityName, - }, - } - - if err := runtimeClient.Delete(context.TODO(), identity); err != nil { - if errors.IsNotFound(err) { - return nil - } - return err - } - - return nil -} - -func deleteIdentityProvider(oAuth *configv1.OAuth, runtimeClient client.Client) error { - logrus.Info("Delete initial user httpasswd provider from the oAuth") - - oauthPatch := client.MergeFrom(oAuth.DeepCopy()) - ips := oAuth.Spec.IdentityProviders - for i, ip := range ips { - if ip.Name == htpasswdIdentityProviderName { - // remove provider from slice - oAuth.Spec.IdentityProviders = append(ips[:i], ips[i+1:]...) - break - } - } - - if err := runtimeClient.Patch(context.TODO(), oAuth, oauthPatch); err != nil { - return err - } - - return nil -} diff --git a/controllers/che/oauth_initial_htpasswd_provider_test.go b/controllers/che/oauth_initial_htpasswd_provider_test.go deleted file mode 100644 index 6a668715e..000000000 --- a/controllers/che/oauth_initial_htpasswd_provider_test.go +++ /dev/null @@ -1,223 +0,0 @@ -// -// 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 che - -import ( - "context" - "os" - "testing" - - orgv1 "github.com/eclipse-che/che-operator/api/v1" - util_mocks "github.com/eclipse-che/che-operator/mocks/pkg/util" - "github.com/eclipse-che/che-operator/pkg/deploy" - "github.com/eclipse-che/che-operator/pkg/util" - "github.com/golang/mock/gomock" - oauth_config "github.com/openshift/api/config/v1" - userv1 "github.com/openshift/api/user/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" -) - -const ( - testNamespace = "test-namespace" - testUserName = "test" -) - -var ( - testCR = &orgv1.CheCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "eclipse-che", - Namespace: testNamespace, - }, - Spec: orgv1.CheClusterSpec{ - Server: orgv1.CheClusterSpecServer{ - CheFlavor: testUserName, - }, - }, - } -) - -func TestCreateInitialUser(t *testing.T) { - type testCase struct { - name string - oAuth *oauth_config.OAuth - initObjects []runtime.Object - } - - oAuth := &oauth_config.OAuth{ - ObjectMeta: v1.ObjectMeta{ - Name: "cluster", - }, - Spec: oauth_config.OAuthSpec{IdentityProviders: []oauth_config.IdentityProvider{}}, - } - - logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) - - scheme := scheme.Scheme - orgv1.SchemeBuilder.AddToScheme(scheme) - scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.UserList{}, &userv1.User{}) - scheme.AddKnownTypes(oauth_config.SchemeGroupVersion, &oauth_config.OAuth{}) - - runtimeClient := fake.NewFakeClientWithScheme(scheme, oAuth, testCR) - - ctrl := gomock.NewController(t) - m := util_mocks.NewMockRunnable(ctrl) - m.EXPECT().Run("htpasswd", "-nbB", gomock.Any(), gomock.Any()).Return(nil) - m.EXPECT().GetStdOut().Return("test-string") - m.EXPECT().GetStdErr().Return("") - defer ctrl.Finish() - - initialUserHandler := &OpenShiftOAuthUserOperatorHandler{ - runtimeClient: runtimeClient, - runnable: m, - } - dc := &deploy.DeployContext{ - CheCluster: testCR, - ClusterAPI: deploy.ClusterAPI{Client: runtimeClient, NonCachedClient: runtimeClient, DiscoveryClient: nil, Scheme: scheme}, - } - provisined, err := initialUserHandler.SyncOAuthInitialUser(oAuth, dc) - if err != nil { - t.Errorf("Failed to create user: %s", err.Error()) - } - if !provisined { - t.Error("Unexpected error") - } - - // Check created objects - expectedCheSecret := &corev1.Secret{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: openShiftOAuthUserCredentialsSecret, Namespace: ocConfigNamespace}, expectedCheSecret); err != nil { - t.Errorf("Initial user secret should exists") - } - - expectedHtpasswsSecret := &corev1.Secret{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: htpasswdSecretName, Namespace: ocConfigNamespace}, expectedHtpasswsSecret); err != nil { - t.Errorf("Initial user secret should exists") - } - - expectedOAuth := &oauth_config.OAuth{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, expectedOAuth); err != nil { - t.Errorf("Initial oAuth should exists") - } - - if len(expectedOAuth.Spec.IdentityProviders) < 0 { - t.Error("List identity providers should not be an empty") - } - - if !util.ContainsString(testCR.Finalizers, openshiftOauthUserFinalizerName) { - t.Error("Finaizer hasn't been added") - } -} - -func TestDeleteInitialUser(t *testing.T) { - logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) - - scheme := scheme.Scheme - orgv1.SchemeBuilder.AddToScheme(scheme) - scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.UserList{}, &userv1.User{}) - scheme.AddKnownTypes(oauth_config.SchemeGroupVersion, &oauth_config.OAuth{}) - scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.Identity{}) - scheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.Secret{}) - scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.User{}) - - oAuth := &oauth_config.OAuth{ - ObjectMeta: v1.ObjectMeta{ - Name: "cluster", - }, - Spec: oauth_config.OAuthSpec{IdentityProviders: []oauth_config.IdentityProvider{*newHtpasswdProvider()}}, - } - cheSecret := &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - Kind: "Secret", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: openShiftOAuthUserCredentialsSecret, - Namespace: ocConfigNamespace, - }, - } - htpasswdSecret := &corev1.Secret{ - TypeMeta: metav1.TypeMeta{ - Kind: "Secret", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: htpasswdSecretName, - Namespace: ocConfigNamespace, - }, - } - userIdentity := &userv1.Identity{ - ObjectMeta: metav1.ObjectMeta{ - Name: htpasswdIdentityProviderName + ":" + testUserName, - }, - } - user := &userv1.User{ - ObjectMeta: metav1.ObjectMeta{ - Name: testUserName, - }, - } - - runtimeClient := fake.NewFakeClientWithScheme(scheme, oAuth, cheSecret, htpasswdSecret, userIdentity, user, testCR) - - initialUserHandler := &OpenShiftOAuthUserOperatorHandler{ - runtimeClient: runtimeClient, - } - - dc := &deploy.DeployContext{ - CheCluster: testCR, - ClusterAPI: deploy.ClusterAPI{Client: runtimeClient, NonCachedClient: runtimeClient, DiscoveryClient: nil, Scheme: scheme}, - } - if err := initialUserHandler.DeleteOAuthInitialUser(dc); err != nil { - t.Errorf("Unable to delete initial user: %s", err.Error()) - } - - expectedCheSecret := &corev1.Secret{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: openShiftOAuthUserCredentialsSecret, Namespace: ocConfigNamespace}, expectedCheSecret); !errors.IsNotFound(err) { - t.Errorf("Initial user secret should be deleted") - } - - expectedHtpasswsSecret := &corev1.Secret{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: htpasswdSecretName, Namespace: ocConfigNamespace}, expectedHtpasswsSecret); !errors.IsNotFound(err) { - t.Errorf("Initial user secret should be deleted") - } - - expectedUserIdentity := &userv1.Identity{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: htpasswdIdentityProviderName + ":" + testUserName}, expectedUserIdentity); !errors.IsNotFound(err) { - t.Errorf("Initial user identity should be deleted") - } - - expectedUser := &userv1.User{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: testUserName}, expectedUser); !errors.IsNotFound(err) { - t.Errorf("Initial user should be deleted") - } - - expectedOAuth := &oauth_config.OAuth{} - if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, expectedOAuth); err != nil { - t.Errorf("OAuth should exists") - } - - if len(expectedOAuth.Spec.IdentityProviders) != 0 { - t.Error("List identity providers should be an empty") - } - - if util.ContainsString(testCR.Finalizers, openshiftOauthUserFinalizerName) { - t.Error("Finalizer hasn't been removed") - } -} diff --git a/go.mod b/go.mod index f91ae5261..4ab66689e 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/openshift/api v3.9.1-0.20190924102528-32369d4db2ad+incompatible github.com/operator-framework/api v0.10.0 github.com/operator-framework/operator-lifecycle-manager v0.18.1 + github.com/pkg/errors v0.9.1 github.com/sirupsen/logrus v1.7.0 github.com/stretchr/testify v1.7.0 go.uber.org/zap v1.18.1 diff --git a/mocks/controllers/oauth_initial_htpasswd_provider_mock.go b/mocks/controllers/oauth_initial_htpasswd_provider_mock.go deleted file mode 100644 index d200b0e41..000000000 --- a/mocks/controllers/oauth_initial_htpasswd_provider_mock.go +++ /dev/null @@ -1,65 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: pkg/controller/che/oauth_initial_htpasswd_provider.go - -// Package mock_che is a generated GoMock package. -package mock_controllers - -import ( - reflect "reflect" - - deploy "github.com/eclipse-che/che-operator/pkg/deploy" - gomock "github.com/golang/mock/gomock" - v1 "github.com/openshift/api/config/v1" -) - -// MockOpenShiftOAuthUserHandler is a mock of OpenShiftOAuthUserHandler interface -type MockOpenShiftOAuthUserHandler struct { - ctrl *gomock.Controller - recorder *MockOpenShiftOAuthUserHandlerMockRecorder -} - -// MockOpenShiftOAuthUserHandlerMockRecorder is the mock recorder for MockOpenShiftOAuthUserHandler -type MockOpenShiftOAuthUserHandlerMockRecorder struct { - mock *MockOpenShiftOAuthUserHandler -} - -// NewMockOpenShiftOAuthUserHandler creates a new mock instance -func NewMockOpenShiftOAuthUserHandler(ctrl *gomock.Controller) *MockOpenShiftOAuthUserHandler { - mock := &MockOpenShiftOAuthUserHandler{ctrl: ctrl} - mock.recorder = &MockOpenShiftOAuthUserHandlerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockOpenShiftOAuthUserHandler) EXPECT() *MockOpenShiftOAuthUserHandlerMockRecorder { - return m.recorder -} - -// SyncOAuthInitialUser mocks base method -func (m *MockOpenShiftOAuthUserHandler) SyncOAuthInitialUser(openshiftOAuth *v1.OAuth, deployContext *deploy.DeployContext) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SyncOAuthInitialUser", openshiftOAuth, deployContext) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// SyncOAuthInitialUser indicates an expected call of SyncOAuthInitialUser -func (mr *MockOpenShiftOAuthUserHandlerMockRecorder) SyncOAuthInitialUser(openshiftOAuth, deployContext interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncOAuthInitialUser", reflect.TypeOf((*MockOpenShiftOAuthUserHandler)(nil).SyncOAuthInitialUser), openshiftOAuth, deployContext) -} - -// DeleteOAuthInitialUser mocks base method -func (m *MockOpenShiftOAuthUserHandler) DeleteOAuthInitialUser(deployContext *deploy.DeployContext) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteOAuthInitialUser", deployContext) - ret0, _ := ret[0].(error) - return ret0 -} - -// DeleteOAuthInitialUser indicates an expected call of DeleteOAuthInitialUser -func (mr *MockOpenShiftOAuthUserHandlerMockRecorder) DeleteOAuthInitialUser(deployContext interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOAuthInitialUser", reflect.TypeOf((*MockOpenShiftOAuthUserHandler)(nil).DeleteOAuthInitialUser), deployContext) -} diff --git a/pkg/deploy/identity-provider/identity_provider.go b/pkg/deploy/identity-provider/identity_provider.go index 99cd638bb..63da63535 100644 --- a/pkg/deploy/identity-provider/identity_provider.go +++ b/pkg/deploy/identity-provider/identity_provider.go @@ -148,7 +148,7 @@ func syncKeycloakResources(deployContext *deploy.DeployContext) (bool, error) { func syncOpenShiftIdentityProvider(deployContext *deploy.DeployContext) (bool, error) { cr := deployContext.CheCluster - if util.IsOpenShift && util.IsOAuthEnabled(cr) { + if util.IsOpenShift && cr.IsOpenShiftOAuthEnabled() { return SyncOpenShiftIdentityProviderItems(deployContext) } return true, nil @@ -327,7 +327,7 @@ func SyncGitHubOAuth(deployContext *deploy.DeployContext) (bool, error) { } func ReconcileIdentityProvider(deployContext *deploy.DeployContext) (deleted bool, err error) { - if !util.IsOAuthEnabled(deployContext.CheCluster) && deployContext.CheCluster.Status.OpenShiftoAuthProvisioned == true { + if !deployContext.CheCluster.IsOpenShiftOAuthEnabled() && deployContext.CheCluster.Status.OpenShiftoAuthProvisioned == true { keycloakDeployment := &appsv1.Deployment{} if err := deployContext.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: deploy.IdentityProviderName, Namespace: deployContext.CheCluster.Namespace}, keycloakDeployment); err != nil { logrus.Errorf("Deployment %s not found: %s", keycloakDeployment.Name, err.Error()) diff --git a/pkg/deploy/kubernetes_image_puller.go b/pkg/deploy/image-puller/imagepuller.go similarity index 91% rename from pkg/deploy/kubernetes_image_puller.go rename to pkg/deploy/image-puller/imagepuller.go index e9c828c9d..46c6b2b7f 100644 --- a/pkg/deploy/kubernetes_image_puller.go +++ b/pkg/deploy/image-puller/imagepuller.go @@ -9,7 +9,7 @@ // Contributors: // Red Hat, Inc. - initial API and implementation // -package deploy +package imagepuller import ( "context" @@ -19,6 +19,7 @@ import ( chev1alpha1 "github.com/che-incubator/kubernetes-image-puller-operator/api/v1alpha1" 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" operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -46,23 +47,24 @@ type ImageAndName struct { } type ImagePuller struct { + deploy.Reconcilable } func NewImagePuller() *ImagePuller { return &ImagePuller{} } -func (ip *ImagePuller) Reconcile(ctx *DeployContext) (reconcile.Result, bool, error) { +func (ip *ImagePuller) Reconcile(ctx *deploy.DeployContext) (reconcile.Result, bool, error) { return ReconcileImagePuller(ctx) } -func (ip *ImagePuller) Finalize(ctx *DeployContext) (bool, error) { +func (ip *ImagePuller) Finalize(ctx *deploy.DeployContext) error { return DeleteImagePullerOperatorAndFinalizer(ctx) } // Reconcile the imagePuller section of the CheCluster CR. If imagePuller.enable is set to true, install the Kubernetes Image Puller operator and create // a KubernetesImagePuller CR. Add a finalizer to the CheCluster CR. If false, remove the KubernetesImagePuller CR, uninstall the operator, and remove the finalizer. -func ReconcileImagePuller(ctx *DeployContext) (reconcile.Result, bool, error) { +func ReconcileImagePuller(ctx *deploy.DeployContext) (reconcile.Result, bool, error) { // Determine what server groups the API Server knows about foundPackagesAPI, foundOperatorsAPI, _, err := CheckNeededImagePullerApis(ctx) if err != nil { @@ -193,8 +195,8 @@ func ReconcileImagePuller(ctx *DeployContext) (reconcile.Result, bool, error) { } else { if foundOperatorsAPI && foundPackagesAPI { - done, err := DeleteImagePullerOperatorAndFinalizer(ctx) - if !done { + err := DeleteImagePullerOperatorAndFinalizer(ctx) + if err != nil { return reconcile.Result{}, false, err } else { return reconcile.Result{}, true, nil @@ -204,21 +206,21 @@ func ReconcileImagePuller(ctx *DeployContext) (reconcile.Result, bool, error) { return reconcile.Result{}, true, nil } -func DeleteImagePullerOperatorAndFinalizer(ctx *DeployContext) (bool, error) { +func DeleteImagePullerOperatorAndFinalizer(ctx *deploy.DeployContext) error { if _, err := GetImagePullerOperator(ctx); err == nil { if _, err := UninstallImagePullerOperator(ctx); err != nil { logrus.Errorf("Error uninstalling Image Puller: %v", err) - return false, err + return err } } if HasImagePullerFinalizer(ctx.CheCluster) { if err := DeleteImagePullerFinalizer(ctx); err != nil { logrus.Errorf("Error deleting finalizer: %v", err) - return false, err + return err } } - return true, nil + return nil } func HasImagePullerFinalizer(instance *orgv1.CheCluster) bool { @@ -231,13 +233,13 @@ func HasImagePullerFinalizer(instance *orgv1.CheCluster) bool { return false } -func ReconcileImagePullerFinalizer(ctx *DeployContext) (err error) { +func ReconcileImagePullerFinalizer(ctx *deploy.DeployContext) (err error) { instance := ctx.CheCluster if instance.ObjectMeta.DeletionTimestamp.IsZero() { - return AppendFinalizer(ctx, imagePullerFinalizerName) + return deploy.AppendFinalizer(ctx, imagePullerFinalizerName) } else { if util.ContainsString(instance.ObjectMeta.Finalizers, imagePullerFinalizerName) { - clusterServiceVersionName := DefaultKubernetesImagePullerOperatorCSV() + clusterServiceVersionName := deploy.DefaultKubernetesImagePullerOperatorCSV() logrus.Infof("Custom resource %s is being deleted. Deleting ClusterServiceVersion %s first", instance.Name, clusterServiceVersionName) clusterServiceVersion := &operatorsv1alpha1.ClusterServiceVersion{} err := ctx.ClusterAPI.NonCachedClient.Get(context.TODO(), types.NamespacedName{Namespace: instance.Namespace, Name: clusterServiceVersionName}, clusterServiceVersion) @@ -250,13 +252,13 @@ func ReconcileImagePullerFinalizer(ctx *DeployContext) (err error) { return err } - return DeleteFinalizer(ctx, imagePullerFinalizerName) + return deploy.DeleteFinalizer(ctx, imagePullerFinalizerName) } return nil } } -func DeleteImagePullerFinalizer(ctx *DeployContext) (err error) { +func DeleteImagePullerFinalizer(ctx *deploy.DeployContext) (err error) { instance := ctx.CheCluster instance.ObjectMeta.Finalizers = util.DoRemoveString(instance.ObjectMeta.Finalizers, imagePullerFinalizerName) logrus.Infof("Removing image puller finalizer on %s CR", instance.Name) @@ -284,7 +286,7 @@ func SubscriptionsAreEqual(expected *operatorsv1alpha1.Subscription, actual *ope // foundOperatorsAPI - true if the server discovers the operators.coreos.com API // foundKubernetesImagePullerAPI - true if the server discovers the che.eclipse.org API // error - any error returned by the call to discoveryClient.ServerGroups() -func CheckNeededImagePullerApis(ctx *DeployContext) (bool, bool, bool, error) { +func CheckNeededImagePullerApis(ctx *deploy.DeployContext) (bool, bool, bool, error) { groupList, resourcesList, err := ctx.ClusterAPI.DiscoveryClient.ServerGroupsAndResources() if err != nil { return false, false, false, err @@ -312,7 +314,7 @@ func CheckNeededImagePullerApis(ctx *DeployContext) (bool, bool, bool, error) { } // Search for the kubernetes-imagepuller-operator PackageManifest -func GetPackageManifest(ctx *DeployContext) (*packagesv1.PackageManifest, error) { +func GetPackageManifest(ctx *deploy.DeployContext) (*packagesv1.PackageManifest, error) { packageManifest := &packagesv1.PackageManifest{} err := ctx.ClusterAPI.NonCachedClient.Get(context.TODO(), types.NamespacedName{Namespace: ctx.CheCluster.Namespace, Name: "kubernetes-imagepuller-operator"}, packageManifest) if err != nil { @@ -323,7 +325,7 @@ func GetPackageManifest(ctx *DeployContext) (*packagesv1.PackageManifest, error) // Create an OperatorGroup in the CheCluster namespace if it does not exist. Returns true if the // OperatorGroup was created, and any error returned during the List and Create operation -func CreateOperatorGroupIfNotFound(ctx *DeployContext) (bool, error) { +func CreateOperatorGroupIfNotFound(ctx *deploy.DeployContext) (bool, error) { operatorGroupList := &operatorsv1.OperatorGroupList{} err := ctx.ClusterAPI.NonCachedClient.List(context.TODO(), operatorGroupList, &client.ListOptions{Namespace: ctx.CheCluster.Namespace}) if err != nil { @@ -354,7 +356,7 @@ func CreateOperatorGroupIfNotFound(ctx *DeployContext) (bool, error) { return false, nil } -func CreateImagePullerSubscription(ctx *DeployContext, packageManifest *packagesv1.PackageManifest) (bool, error) { +func CreateImagePullerSubscription(ctx *deploy.DeployContext, packageManifest *packagesv1.PackageManifest) (bool, error) { imagePullerOperatorSubscription := &operatorsv1alpha1.Subscription{} err := ctx.ClusterAPI.NonCachedClient.Get(context.TODO(), types.NamespacedName{ Name: "kubernetes-imagepuller-operator", @@ -374,7 +376,7 @@ func CreateImagePullerSubscription(ctx *DeployContext, packageManifest *packages return false, nil } -func GetExpectedSubscription(ctx *DeployContext, packageManifest *packagesv1.PackageManifest) *operatorsv1alpha1.Subscription { +func GetExpectedSubscription(ctx *deploy.DeployContext, packageManifest *packagesv1.PackageManifest) *operatorsv1alpha1.Subscription { return &operatorsv1alpha1.Subscription{ ObjectMeta: metav1.ObjectMeta{ Name: "kubernetes-imagepuller-operator", @@ -393,7 +395,7 @@ func GetExpectedSubscription(ctx *DeployContext, packageManifest *packagesv1.Pac } } -func GetActualSubscription(ctx *DeployContext) (*operatorsv1alpha1.Subscription, error) { +func GetActualSubscription(ctx *deploy.DeployContext) (*operatorsv1alpha1.Subscription, error) { actual := &operatorsv1alpha1.Subscription{} err := ctx.ClusterAPI.NonCachedClient.Get(context.TODO(), types.NamespacedName{Namespace: ctx.CheCluster.Namespace, Name: "kubernetes-imagepuller-operator"}, actual) if err != nil { @@ -404,7 +406,7 @@ func GetActualSubscription(ctx *DeployContext) (*operatorsv1alpha1.Subscription, // Update the CheCluster ImagePuller spec if the default values are not set // returns the updated spec and an error during update -func UpdateImagePullerSpecIfEmpty(ctx *DeployContext) (orgv1.CheClusterSpecImagePuller, error) { +func UpdateImagePullerSpecIfEmpty(ctx *deploy.DeployContext) (orgv1.CheClusterSpecImagePuller, error) { if ctx.CheCluster.Spec.ImagePuller.Spec.DeploymentName == "" { ctx.CheCluster.Spec.ImagePuller.Spec.DeploymentName = "kubernetes-image-puller" } @@ -418,7 +420,7 @@ func UpdateImagePullerSpecIfEmpty(ctx *DeployContext) (orgv1.CheClusterSpecImage return ctx.CheCluster.Spec.ImagePuller, nil } -func SetDefaultImages(ctx *DeployContext) error { +func SetDefaultImages(ctx *deploy.DeployContext) error { defaultImages := GetDefaultImages() if len(defaultImages) == 0 { return nil @@ -518,7 +520,7 @@ func IsRFC1123Char(ch byte) bool { return len(errs) == 0 } -func CreateKubernetesImagePuller(ctx *DeployContext) (bool, error) { +func CreateKubernetesImagePuller(ctx *deploy.DeployContext) (bool, error) { imagePuller := GetExpectedKubernetesImagePuller(ctx) err := ctx.ClusterAPI.Client.Create(context.TODO(), imagePuller, &client.CreateOptions{}) if err != nil { @@ -527,7 +529,7 @@ func CreateKubernetesImagePuller(ctx *DeployContext) (bool, error) { return true, nil } -func GetExpectedKubernetesImagePuller(ctx *DeployContext) *chev1alpha1.KubernetesImagePuller { +func GetExpectedKubernetesImagePuller(ctx *deploy.DeployContext) *chev1alpha1.KubernetesImagePuller { return &chev1alpha1.KubernetesImagePuller{ ObjectMeta: metav1.ObjectMeta{ Name: ctx.CheCluster.Name + "-image-puller", @@ -546,7 +548,7 @@ func GetExpectedKubernetesImagePuller(ctx *DeployContext) *chev1alpha1.Kubernete } // UpdateDefaultImagesIfNeeded updates the default images from `spec.images` if needed -func UpdateDefaultImagesIfNeeded(ctx *DeployContext) error { +func UpdateDefaultImagesIfNeeded(ctx *deploy.DeployContext) error { specImages := StringToImageSlice(ctx.CheCluster.Spec.ImagePuller.Spec.Images) defaultImages := GetDefaultImages() if UpdateSpecImages(specImages, defaultImages) { @@ -578,15 +580,15 @@ func UpdateSpecImages(specImages []ImageAndName, defaultImages []ImageAndName) b } // SetImages sets the provided images to the imagePuller spec -func SetImages(ctx *DeployContext, images []ImageAndName) error { +func SetImages(ctx *deploy.DeployContext, images []ImageAndName) error { imagesStr := ImageSliceToString(images) ctx.CheCluster.Spec.ImagePuller.Spec.Images = imagesStr - return UpdateCheCRSpec(ctx, "Kubernetes Image Puller images", imagesStr) + return deploy.UpdateCheCRSpec(ctx, "Kubernetes Image Puller images", imagesStr) } // Uninstall the CSV, OperatorGroup, Subscription, KubernetesImagePuller, and update the CheCluster to remove // the image puller spec. Returns true if the CheCluster was updated -func UninstallImagePullerOperator(ctx *DeployContext) (bool, error) { +func UninstallImagePullerOperator(ctx *deploy.DeployContext) (bool, error) { updated := false _, hasOperatorsAPIs, hasImagePullerAPIs, err := CheckNeededImagePullerApis(ctx) if err != nil { @@ -610,7 +612,7 @@ func UninstallImagePullerOperator(ctx *DeployContext) (bool, error) { if hasOperatorsAPIs { // Delete the ClusterServiceVersion csv := &operatorsv1alpha1.ClusterServiceVersion{} - err = ctx.ClusterAPI.NonCachedClient.Get(context.TODO(), types.NamespacedName{Namespace: ctx.CheCluster.Namespace, Name: DefaultKubernetesImagePullerOperatorCSV()}, csv) + err = ctx.ClusterAPI.NonCachedClient.Get(context.TODO(), types.NamespacedName{Namespace: ctx.CheCluster.Namespace, Name: deploy.DefaultKubernetesImagePullerOperatorCSV()}, csv) if err != nil && !errors.IsNotFound(err) { return updated, err } @@ -668,7 +670,7 @@ func UninstallImagePullerOperator(ctx *DeployContext) (bool, error) { } // GetImagePullerOperator returns the current kubernetes-imagepuller-operator if exists -func GetImagePullerOperator(ctx *DeployContext) (*chev1alpha1.KubernetesImagePuller, error) { +func GetImagePullerOperator(ctx *deploy.DeployContext) (*chev1alpha1.KubernetesImagePuller, error) { imagePuller := &chev1alpha1.KubernetesImagePuller{} err := ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Namespace: ctx.CheCluster.Namespace, Name: ctx.CheCluster.Name + "-image-puller"}, imagePuller) if err != nil { diff --git a/pkg/deploy/kubernetes_image_puller_test.go b/pkg/deploy/image-puller/imagepuller_test.go similarity index 99% rename from pkg/deploy/kubernetes_image_puller_test.go rename to pkg/deploy/image-puller/imagepuller_test.go index 0f087cfa3..58f6a52cf 100644 --- a/pkg/deploy/kubernetes_image_puller_test.go +++ b/pkg/deploy/image-puller/imagepuller_test.go @@ -9,7 +9,7 @@ // Contributors: // Red Hat, Inc. - initial API and implementation // -package deploy +package imagepuller import ( "context" @@ -27,6 +27,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "sigs.k8s.io/yaml" + "github.com/eclipse-che/che-operator/pkg/deploy" "github.com/eclipse-che/che-operator/pkg/util" orgv1 "github.com/eclipse-che/che-operator/api/v1" @@ -276,7 +277,7 @@ func TestImagePullerConfiguration(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) - deployContext := GetTestDeployContext(testCase.initCR, []runtime.Object{}) + deployContext := deploy.GetTestDeployContext(testCase.initCR, []runtime.Object{}) orgv1.SchemeBuilder.AddToScheme(deployContext.ClusterAPI.Scheme) packagesv1.AddToScheme(deployContext.ClusterAPI.Scheme) @@ -320,7 +321,7 @@ func TestImagePullerConfiguration(t *testing.T) { var err error if testCase.shouldDelete { - _, err = DeleteImagePullerOperatorAndFinalizer(deployContext) + err = DeleteImagePullerOperatorAndFinalizer(deployContext) if err != nil { t.Fatalf("Error reconciling: %v", err) } diff --git a/pkg/deploy/image-puller/init_test.go b/pkg/deploy/image-puller/init_test.go new file mode 100644 index 000000000..fe7d0318f --- /dev/null +++ b/pkg/deploy/image-puller/init_test.go @@ -0,0 +1,21 @@ +// +// Copyright (c) 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 imagepuller + +import "github.com/eclipse-che/che-operator/pkg/deploy" + +func init() { + err := deploy.InitTestDefaultsFromDeployment("../../../config/manager/manager.yaml") + if err != nil { + panic(err) + } +} diff --git a/pkg/deploy/openshift-oauth/init_test.go b/pkg/deploy/openshift-oauth/init_test.go new file mode 100644 index 000000000..b80bea36f --- /dev/null +++ b/pkg/deploy/openshift-oauth/init_test.go @@ -0,0 +1,21 @@ +// +// Copyright (c) 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 "github.com/eclipse-che/che-operator/pkg/deploy" + +func init() { + err := deploy.InitTestDefaultsFromDeployment("../../../config/manager/manager.yaml") + if err != nil { + panic(err) + } +} diff --git a/pkg/deploy/openshift-oauth/openshiftoauth_user.go b/pkg/deploy/openshift-oauth/openshiftoauth_user.go new file mode 100644 index 000000000..c85591bd2 --- /dev/null +++ b/pkg/deploy/openshift-oauth/openshiftoauth_user.go @@ -0,0 +1,320 @@ +// +// 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" + "fmt" + + "github.com/eclipse-che/che-operator/pkg/deploy" + "github.com/eclipse-che/che-operator/pkg/util" + configv1 "github.com/openshift/api/config/v1" + userv1 "github.com/openshift/api/user/v1" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const ( + HtpasswdIdentityProviderName = "htpasswd-eclipse-che" + HtpasswdSecretName = "htpasswd-eclipse-che" + OcConfigNamespace = "openshift-config" + OpenShiftOAuthUserCredentialsSecret = "openshift-oauth-user-credentials" + OpenshiftOauthUserFinalizerName = "openshift-oauth-user.finalizers.che.eclipse.org" +) + +var ( + password = util.GeneratePasswd(6) + htpasswdFileContent string +) + +type IOpenShiftOAuthUser interface { + Create(ctx *deploy.DeployContext) (bool, error) + Delete(ctx *deploy.DeployContext) error +} + +type OpenShiftOAuthUser struct { + runnable util.Runnable + + deploy.Reconcilable + IOpenShiftOAuthUser +} + +func NewOpenShiftOAuthUser() *OpenShiftOAuthUser { + return &OpenShiftOAuthUser{ + // real process, mock for tests + runnable: util.NewRunnable(), + } +} + +func (oou *OpenShiftOAuthUser) Reconcile(ctx *deploy.DeployContext) (reconcile.Result, bool, error) { + if !util.IsOpenShift4 { + return reconcile.Result{}, true, nil + } + + if ctx.CheCluster.IsOpenShiftOAuthUserConfigured() && ctx.CheCluster.IsOpenShiftOAuthEnabled() { + done, err := oou.Create(ctx) + if !done { + return reconcile.Result{Requeue: true}, false, err + } + return reconcile.Result{}, true, nil + } + + if ctx.CheCluster.IsOpenShiftOAuthUserMustBeDeleted() { + if err := oou.Delete(ctx); err != nil { + return reconcile.Result{}, false, errors.Wrap(err, "Unable to delete initial OpenShift OAuth user from a cluster") + } + + return reconcile.Result{}, true, nil + } + + return reconcile.Result{}, true, nil +} + +func (oou *OpenShiftOAuthUser) Finalize(ctx *deploy.DeployContext) error { + if util.IsOpenShift4 { + return oou.Delete(ctx) + } + + return nil +} + +// Creates new htpasswd provider with initial user with Che flavor name +// if Openshift cluster hasn't got identity providers then does nothing. +// It usefull for good first user experience. +// User can't use kube:admin or system:admin user in the Openshift oAuth when DevWorkspace engine disabled. +// That's why we provide initial user for good first meeting with Eclipse Che. +func (oou *OpenShiftOAuthUser) Create(ctx *deploy.DeployContext) (bool, error) { + userName := deploy.DefaultCheFlavor(ctx.CheCluster) + if htpasswdFileContent == "" { + var err error + if htpasswdFileContent, err = oou.generateHtPasswdUserInfo(userName, password); err != nil { + return false, err + } + } + + var storedPassword string + + // read existed password from the secret (operator has been restarted case) + secret, err := oou.getOpenShiftOAuthUserCredentialsSecret(ctx) + if err != nil { + return false, err + } else if secret != nil { + storedPassword = string(secret.Data["password"]) + } + + if storedPassword != "" && password != storedPassword { + password = storedPassword + if htpasswdFileContent, err = oou.generateHtPasswdUserInfo(userName, password); err != nil { + return false, err + } + } + + initialUserSecretData := map[string][]byte{"user": []byte(userName), "password": []byte(password)} + done, err := deploy.SyncSecretToCluster(ctx, OpenShiftOAuthUserCredentialsSecret, OcConfigNamespace, initialUserSecretData) + if !done { + return false, err + } + + htpasswdFileSecretData := map[string][]byte{"htpasswd": []byte(htpasswdFileContent)} + done, err = deploy.SyncSecretToCluster(ctx, HtpasswdSecretName, OcConfigNamespace, htpasswdFileSecretData) + if !done { + return false, err + } + + oAuth, err := GetOpenshiftOAuth(ctx) + if err != nil { + return false, err + } + + if err := appendIdentityProvider(oAuth, ctx.ClusterAPI.NonCachedClient); err != nil { + return false, err + } + + ctx.CheCluster.Status.OpenShiftOAuthUserCredentialsSecret = OpenShiftOAuthUserCredentialsSecret + if err := deploy.UpdateCheCRStatus(ctx, "openShiftOAuthUserCredentialsSecret", OpenShiftOAuthUserCredentialsSecret); err != nil { + return false, err + } + + if err := deploy.AppendFinalizer(ctx, OpenshiftOauthUserFinalizerName); err != nil { + return false, err + } + + return true, nil +} + +// Removes initial user, htpasswd provider, htpasswd secret and Che secret with username and password. +func (oou *OpenShiftOAuthUser) Delete(ctx *deploy.DeployContext) error { + oAuth, err := GetOpenshiftOAuth(ctx) + if err != nil { + return err + } + + userName := deploy.DefaultCheFlavor(ctx.CheCluster) + if _, err := deploy.Delete(ctx, types.NamespacedName{Name: userName}, &userv1.User{}); err != nil { + return err + } + + identityName := HtpasswdIdentityProviderName + ":" + userName + if _, err := deploy.Delete(ctx, types.NamespacedName{Name: identityName}, &userv1.Identity{}); err != nil { + return err + } + + if err := deleteIdentityProvider(oAuth, ctx.ClusterAPI.NonCachedClient); err != nil { + return err + } + + _, err = deploy.Delete(ctx, types.NamespacedName{Name: HtpasswdSecretName, Namespace: OcConfigNamespace}, &corev1.Secret{}) + if err != nil { + return err + } + + // legacy secret in the current namespace + _, err = deploy.DeleteNamespacedObject(ctx, OpenShiftOAuthUserCredentialsSecret, &corev1.Secret{}) + if err != nil { + return err + } + + _, err = deploy.Delete(ctx, types.NamespacedName{Name: OpenShiftOAuthUserCredentialsSecret, Namespace: OcConfigNamespace}, &corev1.Secret{}) + if err != nil { + return err + } + + if err := deploy.DeleteFinalizer(ctx, OpenshiftOauthUserFinalizerName); err != nil { + return err + } + + ctx.CheCluster.Status.OpenShiftOAuthUserCredentialsSecret = "" + if err := deploy.UpdateCheCRStatus(ctx, "openShiftOAuthUserCredentialsSecret", ""); err != nil { + return err + } + + // set 'openShiftoAuth:nil` to reenable on the following reconcile loop (if possible) + ctx.CheCluster.Spec.Auth.InitialOpenShiftOAuthUser = nil + ctx.CheCluster.Spec.Auth.OpenShiftoAuth = nil + updateFields := map[string]string{ + "openShiftoAuth": "nil", + "initialOpenShiftOAuthUser": "nil", + } + + if err := deploy.UpdateCheCRSpecByFields(ctx, updateFields); err != nil { + return err + } + + return nil +} + +func (oou *OpenShiftOAuthUser) generateHtPasswdUserInfo(userName string, password string) (string, error) { + if util.IsTestMode() { + return "", nil + } + logrus.Info("Generate initial user httpasswd info") + + err := oou.runnable.Run("htpasswd", "-nbB", userName, password) + if err != nil { + return "", err + } + + if len(oou.runnable.GetStdErr()) > 0 { + return "", fmt.Errorf("Failed to generate data for HTPasswd identity provider: %s", oou.runnable.GetStdErr()) + } + return oou.runnable.GetStdOut(), nil +} + +// Gets OpenShift user credentials secret from from the secret from: +// - openshift-config namespace +// - eclipse-che namespace +func (oou *OpenShiftOAuthUser) getOpenShiftOAuthUserCredentialsSecret(ctx *deploy.DeployContext) (*corev1.Secret, error) { + secret := &corev1.Secret{} + + exists, err := deploy.Get(ctx, types.NamespacedName{Name: OpenShiftOAuthUserCredentialsSecret, Namespace: OcConfigNamespace}, secret) + if err != nil { + return nil, err + } else if exists { + return secret, nil + } + + exists, err = deploy.GetNamespacedObject(ctx, OpenShiftOAuthUserCredentialsSecret, secret) + if err != nil { + return nil, err + } else if exists { + return secret, nil + } + + return nil, nil +} + +func identityProviderExists(providerName string, oAuth *configv1.OAuth) bool { + if len(oAuth.Spec.IdentityProviders) == 0 { + return false + } + for _, identityProvider := range oAuth.Spec.IdentityProviders { + if identityProvider.Name == providerName { + return true + } + } + return false +} + +func appendIdentityProvider(oAuth *configv1.OAuth, runtimeClient client.Client) error { + logrus.Info("Add initial user httpasswd provider to the oAuth") + + htpasswdProvider := newHtpasswdProvider() + if !identityProviderExists(htpasswdProvider.Name, oAuth) { + oauthPatch := client.MergeFrom(oAuth.DeepCopy()) + + oAuth.Spec.IdentityProviders = append(oAuth.Spec.IdentityProviders, *htpasswdProvider) + + if err := runtimeClient.Patch(context.TODO(), oAuth, oauthPatch); err != nil { + return err + } + } + + return nil +} + +func newHtpasswdProvider() *configv1.IdentityProvider { + return &configv1.IdentityProvider{ + Name: HtpasswdIdentityProviderName, + MappingMethod: configv1.MappingMethodClaim, + IdentityProviderConfig: configv1.IdentityProviderConfig{ + Type: "HTPasswd", + HTPasswd: &configv1.HTPasswdIdentityProvider{ + FileData: configv1.SecretNameReference{Name: HtpasswdSecretName}, + }, + }, + } +} + +func deleteIdentityProvider(oAuth *configv1.OAuth, runtimeClient client.Client) error { + logrus.Info("Delete initial user httpasswd provider from the oAuth") + + oauthPatch := client.MergeFrom(oAuth.DeepCopy()) + ips := oAuth.Spec.IdentityProviders + for i, ip := range ips { + if ip.Name == HtpasswdIdentityProviderName { + // remove provider from slice + oAuth.Spec.IdentityProviders = append(ips[:i], ips[i+1:]...) + break + } + } + + if err := runtimeClient.Patch(context.TODO(), oAuth, oauthPatch); err != nil { + return err + } + + return nil +} diff --git a/pkg/deploy/openshift-oauth/openshiftoauth_user_test.go b/pkg/deploy/openshift-oauth/openshiftoauth_user_test.go new file mode 100644 index 000000000..e22da917a --- /dev/null +++ b/pkg/deploy/openshift-oauth/openshiftoauth_user_test.go @@ -0,0 +1,168 @@ +// +// 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" + "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" + oauth_config "github.com/openshift/api/config/v1" + userv1 "github.com/openshift/api/user/v1" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +const ( + testNamespace = "test-namespace" + testUserName = "test" +) + +func TestCreateInitialUser(t *testing.T) { + checluster := &orgv1.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eclipse-che", + Namespace: testNamespace, + }, + Spec: orgv1.CheClusterSpec{ + Server: orgv1.CheClusterSpecServer{ + CheFlavor: testUserName, + }, + }, + } + oAuth := &oauth_config.OAuth{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Spec: oauth_config.OAuthSpec{IdentityProviders: []oauth_config.IdentityProvider{}}, + } + logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) + ctx := deploy.GetTestDeployContext(checluster, []runtime.Object{oAuth}) + + openShiftOAuthUser := NewOpenShiftOAuthUser() + done, err := openShiftOAuthUser.Create(ctx) + assert.Nil(t, err) + assert.True(t, done) + + // Check created objects + expectedCheSecret := &corev1.Secret{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: OpenShiftOAuthUserCredentialsSecret, Namespace: OcConfigNamespace}, expectedCheSecret) + assert.Nil(t, err) + + expectedHtpasswsSecret := &corev1.Secret{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: HtpasswdSecretName, Namespace: OcConfigNamespace}, expectedHtpasswsSecret) + assert.Nil(t, err) + + expectedOAuth := &oauth_config.OAuth{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, expectedOAuth) + assert.Nil(t, err) + assert.Equal(t, len(expectedOAuth.Spec.IdentityProviders), 1) + assert.True(t, util.ContainsString(checluster.Finalizers, OpenshiftOauthUserFinalizerName)) + + assert.Equal(t, checluster.Status.OpenShiftOAuthUserCredentialsSecret, OpenShiftOAuthUserCredentialsSecret) +} + +func TestDeleteInitialUser(t *testing.T) { + checluster := &orgv1.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eclipse-che", + Namespace: testNamespace, + }, + Spec: orgv1.CheClusterSpec{ + Server: orgv1.CheClusterSpecServer{ + CheFlavor: testUserName, + }, + }, + Status: orgv1.CheClusterStatus{ + OpenShiftOAuthUserCredentialsSecret: "some-secret", + }, + } + oAuth := &oauth_config.OAuth{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Spec: oauth_config.OAuthSpec{IdentityProviders: []oauth_config.IdentityProvider{*newHtpasswdProvider()}}, + } + cheSecret := &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: OpenShiftOAuthUserCredentialsSecret, + Namespace: OcConfigNamespace, + }, + } + htpasswdSecret := &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: HtpasswdSecretName, + Namespace: OcConfigNamespace, + }, + } + userIdentity := &userv1.Identity{ + ObjectMeta: metav1.ObjectMeta{ + Name: HtpasswdIdentityProviderName + ":" + testUserName, + }, + } + user := &userv1.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUserName, + }, + } + + logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true))) + ctx := deploy.GetTestDeployContext(checluster, []runtime.Object{oAuth, cheSecret, htpasswdSecret, userIdentity, user}) + + openShiftOAuthUser := &OpenShiftOAuthUser{} + err := openShiftOAuthUser.Delete(ctx) + assert.Nil(t, err) + + expectedCheSecret := &corev1.Secret{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: OpenShiftOAuthUserCredentialsSecret, Namespace: OcConfigNamespace}, expectedCheSecret) + assert.True(t, errors.IsNotFound(err)) + + expectedHtpasswsSecret := &corev1.Secret{} + if err := ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: HtpasswdSecretName, Namespace: OcConfigNamespace}, expectedHtpasswsSecret); !errors.IsNotFound(err) { + t.Errorf("Initial user secret should be deleted") + } + assert.True(t, errors.IsNotFound(err)) + + expectedUserIdentity := &userv1.Identity{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: HtpasswdIdentityProviderName + ":" + testUserName}, expectedUserIdentity) + assert.True(t, errors.IsNotFound(err)) + + expectedUser := &userv1.User{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: testUserName}, expectedUser) + assert.True(t, errors.IsNotFound(err)) + + expectedOAuth := &oauth_config.OAuth{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, expectedOAuth) + assert.Nil(t, err) + assert.Equal(t, len(expectedOAuth.Spec.IdentityProviders), 0) + assert.False(t, util.ContainsString(checluster.Finalizers, OpenshiftOauthUserFinalizerName)) + assert.Empty(t, checluster.Status.OpenShiftOAuthUserCredentialsSecret) +} diff --git a/pkg/deploy/openshift-oauth/openshiftoauth_util.go b/pkg/deploy/openshift-oauth/openshiftoauth_util.go new file mode 100644 index 000000000..1f5019400 --- /dev/null +++ b/pkg/deploy/openshift-oauth/openshiftoauth_util.go @@ -0,0 +1,27 @@ +// +// 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 ( + "github.com/eclipse-che/che-operator/pkg/deploy" + oauthv1 "github.com/openshift/api/config/v1" +) + +// Gets OpenShift OAuth. +func GetOpenshiftOAuth(ctx *deploy.DeployContext) (*oauthv1.OAuth, error) { + oAuth := &oauthv1.OAuth{} + if done, err := deploy.GetClusterObject(ctx, "cluster", oAuth); !done { + return nil, err + } + return oAuth, nil +} diff --git a/pkg/deploy/reconcile_manager.go b/pkg/deploy/reconcile_manager.go index 52f7641dc..2215dbd06 100644 --- a/pkg/deploy/reconcile_manager.go +++ b/pkg/deploy/reconcile_manager.go @@ -23,7 +23,7 @@ type Reconcilable interface { // Reconcile object. Reconcile(ctx *DeployContext) (result reconcile.Result, done bool, err error) // Does finalization (removes cluster scope objects, etc) - Finalize(ctx *DeployContext) (done bool, err error) + Finalize(ctx *DeployContext) error } type ReconcileManager struct { @@ -69,7 +69,7 @@ func (manager *ReconcileManager) ReconcileAll(ctx *DeployContext) (reconcile.Res func (manager *ReconcileManager) FinalizeAll(ctx *DeployContext) { for _, reconciler := range manager.reconcilers { - _, err := reconciler.Finalize(ctx) + err := reconciler.Finalize(ctx) if err != nil { reconcilerName := runtime.FuncForPC(reflect.ValueOf(reconciler).Pointer()).Name() logrus.Errorf("Finalization failed for reconciler: `%s`, cause: %v", reconcilerName, err) diff --git a/pkg/deploy/reconcile_manager_test.go b/pkg/deploy/reconcile_manager_test.go index af1d37faf..7f751ca59 100644 --- a/pkg/deploy/reconcile_manager_test.go +++ b/pkg/deploy/reconcile_manager_test.go @@ -39,8 +39,8 @@ func (tr *TestReconcilable) Reconcile(ctx *DeployContext) (reconcile.Result, boo } } -func (tr *TestReconcilable) Finalize(ctx *DeployContext) (bool, error) { - return true, nil +func (tr *TestReconcilable) Finalize(ctx *DeployContext) error { + return nil } func TestShouldUpdateAndCleanStatus(t *testing.T) { diff --git a/pkg/deploy/server/server_configmap.go b/pkg/deploy/server/server_configmap.go index 6b2be53ea..afefc5338 100644 --- a/pkg/deploy/server/server_configmap.go +++ b/pkg/deploy/server/server_configmap.go @@ -110,7 +110,7 @@ func (s *Server) getCheConfigMapData() (cheEnv map[string]string, err error) { } tls := "false" openShiftIdentityProviderId := "NULL" - if util.IsOpenShift && util.IsOAuthEnabled(s.deployContext.CheCluster) { + if util.IsOpenShift && s.deployContext.CheCluster.IsOpenShiftOAuthEnabled() { openShiftIdentityProviderId = "openshift-v3" if util.IsOpenShift4 { openShiftIdentityProviderId = "openshift-v4" diff --git a/pkg/deploy/test_util.go b/pkg/deploy/test_util.go index 7fc0aa06d..2949de270 100644 --- a/pkg/deploy/test_util.go +++ b/pkg/deploy/test_util.go @@ -13,10 +13,16 @@ package deploy import ( orgv1 "github.com/eclipse-che/che-operator/api/v1" + oauthv1 "github.com/openshift/api/oauth/v1" + routev1 "github.com/openshift/api/route/v1" + userv1 "github.com/openshift/api/user/v1" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" + corev1 "k8s.io/api/core/v1" crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + + configv1 "github.com/openshift/api/config/v1" fakeDiscovery "k8s.io/client-go/discovery/fake" fakeclientset "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/scheme" @@ -44,6 +50,11 @@ func GetTestDeployContext(cheCluster *orgv1.CheCluster, initObjs []runtime.Objec scheme.AddKnownTypes(operatorsv1alpha1.SchemeGroupVersion, &operatorsv1alpha1.Subscription{}) scheme.AddKnownTypes(crdv1.SchemeGroupVersion, &crdv1.CustomResourceDefinition{}) scheme.AddKnownTypes(operatorsv1alpha1.SchemeGroupVersion, &operatorsv1alpha1.Subscription{}) + scheme.AddKnownTypes(oauthv1.SchemeGroupVersion, &oauthv1.OAuthClient{}) + scheme.AddKnownTypes(userv1.SchemeGroupVersion, &userv1.UserList{}, &userv1.User{}, &userv1.Identity{}) + scheme.AddKnownTypes(configv1.SchemeGroupVersion, &configv1.OAuth{}, &configv1.Proxy{}) + scheme.AddKnownTypes(routev1.GroupVersion, &routev1.Route{}) + scheme.AddKnownTypes(corev1.SchemeGroupVersion, &corev1.Secret{}) initObjs = append(initObjs, cheCluster) cli := fake.NewFakeClientWithScheme(scheme, initObjs...) diff --git a/pkg/util/util.go b/pkg/util/util.go index f3723ff7c..dd24e15a7 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -484,16 +484,6 @@ func IsNativeUserModeEnabled(c *orgv1.CheCluster) bool { return IsOpenShift && c.Spec.Auth.NativeUserMode != nil && *c.Spec.Auth.NativeUserMode } -// IsOAuthEnabled returns true when oAuth is enable for CheCluster resource, otherwise false. -func IsOAuthEnabled(c *orgv1.CheCluster) bool { - return IsOpenShift && c.Spec.Auth.OpenShiftoAuth != nil && *c.Spec.Auth.OpenShiftoAuth -} - -// IsInitialOpenShiftOAuthUserEnabled returns true when initial Openshift oAuth user is enabled for CheCluster resource, otherwise false. -func IsInitialOpenShiftOAuthUserEnabled(c *orgv1.CheCluster) bool { - return c.Spec.Auth.InitialOpenShiftOAuthUser != nil && *c.Spec.Auth.InitialOpenShiftOAuthUser -} - // 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 { @@ -504,17 +494,12 @@ func GetWorkspaceNamespaceDefault(cr *orgv1.CheCluster) string { } workspaceNamespaceDefault := cr.Namespace - if IsOpenShift && IsOAuthEnabled(cr) { + if IsOpenShift && cr.IsOpenShiftOAuthEnabled() { workspaceNamespaceDefault = "-" + cr.Spec.Server.CheFlavor } return GetValue(cr.Spec.Server.WorkspaceNamespaceDefault, workspaceNamespaceDefault) } -// IsDeleteOAuthInitialUser - returns true when initial Openshfit oAuth user must be deleted. -func IsDeleteOAuthInitialUser(cr *orgv1.CheCluster) bool { - return cr.Spec.Auth.InitialOpenShiftOAuthUser != nil && !*cr.Spec.Auth.InitialOpenShiftOAuthUser && cr.Status.OpenShiftOAuthUserCredentialsSecret != "" -} - func GetResourceQuantity(value string, defaultValue string) resource.Quantity { if value != "" { return resource.MustParse(value) diff --git a/vendor/modules.txt b/vendor/modules.txt index c9d439431..2bc556d6f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -134,6 +134,7 @@ github.com/operator-framework/operator-registry/pkg/api github.com/operator-framework/operator-registry/pkg/image github.com/operator-framework/operator-registry/pkg/registry # github.com/pkg/errors v0.9.1 => github.com/pkg/errors v0.0.0-20200114194744-614d223910a1 +## explicit github.com/pkg/errors # github.com/pmezard/go-difflib v1.0.0 => github.com/pmezard/go-difflib v1.0.0 github.com/pmezard/go-difflib/difflib