From 5dc1abe87ac750805b904d06340461e3fe045dfa Mon Sep 17 00:00:00 2001 From: Chetan Banavikalmutt Date: Tue, 21 May 2024 11:44:55 +0530 Subject: [PATCH] fix: verify DeploymentConfig API when installing keycloak using Template (#1329) * fix: verify DeploymentConfig API when installing keycloak using Template Signed-off-by: Chetan Banavikalmutt * return an error if the DeploymentConfig API is not present on OCP Signed-off-by: Chetan Banavikalmutt * log when the Keycloak instance can't be managed using Template Signed-off-by: Chetan Banavikalmutt --------- Signed-off-by: Chetan Banavikalmutt --- controllers/argocd/keycloak.go | 12 ++++---- controllers/argocd/keycloak_test.go | 45 +++++++++++++++++------------ controllers/argocd/sso.go | 36 +++++++++++++++++------ controllers/argocd/sso_test.go | 32 ++++++++++++++++++++ controllers/argocd/status.go | 2 +- controllers/argocd/status_test.go | 1 + controllers/argocd/util.go | 4 +-- main.go | 5 +++- 8 files changed, 100 insertions(+), 37 deletions(-) diff --git a/controllers/argocd/keycloak.go b/controllers/argocd/keycloak.go index c36b6740d..74157ab4f 100644 --- a/controllers/argocd/keycloak.go +++ b/controllers/argocd/keycloak.go @@ -111,7 +111,7 @@ func getKeycloakContainerImage(cr *argoproj.ArgoCD) string { if img == "" { img = common.ArgoCDKeycloakImage - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { img = common.ArgoCDKeycloakImageForOpenShift } defaultImg = true @@ -123,7 +123,7 @@ func getKeycloakContainerImage(cr *argoproj.ArgoCD) string { if tag == "" { tag = common.ArgoCDKeycloakVersion - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { tag = common.ArgoCDKeycloakVersionForOpenShift } defaultTag = true @@ -910,7 +910,7 @@ func createRealmConfig(cfg *keycloakConfig) ([]byte, error) { // Add OpenShift-v4 as Identity Provider only for OpenShift environment. // No Identity Provider is configured by default for non-openshift environments. - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { baseURL := "https://kubernetes.default.svc.cluster.local" if isProxyCluster() { baseURL = getOpenShiftAPIURL() @@ -1005,7 +1005,7 @@ func (r *ReconcileArgoCD) updateArgoCDConfiguration(cr *argoproj.ArgoCD, kRouteU } // Create openshift OAuthClient - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { oAuthClient := &oauthv1.OAuthClient{ TypeMeta: metav1.TypeMeta{ Kind: "OAuthClient", @@ -1128,7 +1128,7 @@ func handleKeycloakPodDeletion(dc *appsv1.DeploymentConfig) error { func (r *ReconcileArgoCD) reconcileKeycloakConfiguration(cr *argoproj.ArgoCD) error { // TemplateAPI is available, Install keycloak using openshift templates. - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { err := r.reconcileKeycloakForOpenShift(cr) if err != nil { return err @@ -1146,7 +1146,7 @@ func (r *ReconcileArgoCD) reconcileKeycloakConfiguration(cr *argoproj.ArgoCD) er func deleteKeycloakConfiguration(cr *argoproj.ArgoCD) error { // If SSO is installed using OpenShift templates. - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { err := deleteKeycloakConfigForOpenShift(cr) if err != nil { return err diff --git a/controllers/argocd/keycloak_test.go b/controllers/argocd/keycloak_test.go index 66da5b495..6b840e011 100644 --- a/controllers/argocd/keycloak_test.go +++ b/controllers/argocd/keycloak_test.go @@ -84,13 +84,14 @@ func TestKeycloakContainerImage(t *testing.T) { defer removeTemplateAPI() tests := []struct { - name string - setEnvVarFunc func(*testing.T, string) - envVar string - argoCD *argoproj.ArgoCD - updateCrFunc func(cr *argoproj.ArgoCD) - templateAPIFound bool - wantContainerImage string + name string + setEnvVarFunc func(*testing.T, string) + envVar string + argoCD *argoproj.ArgoCD + updateCrFunc func(cr *argoproj.ArgoCD) + templateAPIFound bool + deploymentConfigAPIFound bool + wantContainerImage string }{ { name: "no .spec.sso, no ArgoCDKeycloakImageEnvName env var set", @@ -101,9 +102,10 @@ func TestKeycloakContainerImage(t *testing.T) { Provider: argoproj.SSOProviderTypeKeycloak, } }), - updateCrFunc: nil, - templateAPIFound: false, - wantContainerImage: "quay.io/keycloak/keycloak@sha256:64fb81886fde61dee55091e6033481fa5ccdac62ae30a4fd29b54eb5e97df6a9", + updateCrFunc: nil, + templateAPIFound: false, + deploymentConfigAPIFound: false, + wantContainerImage: "quay.io/keycloak/keycloak@sha256:64fb81886fde61dee55091e6033481fa5ccdac62ae30a4fd29b54eb5e97df6a9", }, { name: "no .spec.sso, no ArgoCDKeycloakImageEnvName env var set - for OCP", @@ -114,9 +116,10 @@ func TestKeycloakContainerImage(t *testing.T) { Provider: argoproj.SSOProviderTypeKeycloak, } }), - updateCrFunc: nil, - templateAPIFound: true, - wantContainerImage: "registry.redhat.io/rh-sso-7/sso76-openshift-rhel8@sha256:ec9f60018694dcc5d431ba47d5536b761b71cb3f66684978fe6bb74c157679ac", + updateCrFunc: nil, + templateAPIFound: true, + deploymentConfigAPIFound: true, + wantContainerImage: "registry.redhat.io/rh-sso-7/sso76-openshift-rhel8@sha256:ec9f60018694dcc5d431ba47d5536b761b71cb3f66684978fe6bb74c157679ac", }, { name: "ArgoCDKeycloakImageEnvName env var set", @@ -129,9 +132,10 @@ func TestKeycloakContainerImage(t *testing.T) { Provider: argoproj.SSOProviderTypeKeycloak, } }), - updateCrFunc: nil, - templateAPIFound: true, - wantContainerImage: "envImage:latest", + updateCrFunc: nil, + templateAPIFound: true, + deploymentConfigAPIFound: true, + wantContainerImage: "envImage:latest", }, { name: "both cr.spec.sso.keycloak.Image and ArgoCDKeycloakImageEnvName are set", @@ -153,14 +157,16 @@ func TestKeycloakContainerImage(t *testing.T) { }, } }, - templateAPIFound: true, - wantContainerImage: "crImage:crVersion", + templateAPIFound: true, + deploymentConfigAPIFound: true, + wantContainerImage: "crImage:crVersion", }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { templateAPIFound = test.templateAPIFound + deploymentConfigAPIFound = test.deploymentConfigAPIFound if test.setEnvVarFunc != nil { test.setEnvVarFunc(t, test.envVar) @@ -244,6 +250,8 @@ func TestNewKeycloakTemplate_testKeycloakContainer(t *testing.T) { // For OpenShift Container Platform. t.Setenv(common.ArgoCDKeycloakImageEnvName, "") templateAPIFound = true + deploymentConfigAPIFound = true + defer removeTemplateAPI() a := makeTestArgoCD() @@ -496,4 +504,5 @@ func TestKeycloak_NodeLabelSelector(t *testing.T) { func removeTemplateAPI() { templateAPIFound = false + deploymentConfigAPIFound = false } diff --git a/controllers/argocd/sso.go b/controllers/argocd/sso.go index 3acd2ab69..fa917d848 100644 --- a/controllers/argocd/sso.go +++ b/controllers/argocd/sso.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" + deploymentConfig "github.com/openshift/api/apps/v1" template "github.com/openshift/api/template/v1" apiErrors "k8s.io/apimachinery/pkg/api/errors" @@ -33,22 +34,29 @@ const ( ) var ( - templateAPIFound = false - ssoConfigLegalStatus string + templateAPIFound = false + deploymentConfigAPIFound = false + ssoConfigLegalStatus string ) -// IsTemplateAPIAvailable returns true if the template API is present. -func IsTemplateAPIAvailable() bool { - return templateAPIFound +// CanUseKeycloakWithTemplate checks if the required APIs are available to +// manage a Keycloak instance using Templates. +func CanUseKeycloakWithTemplate() bool { + return templateAPIFound && deploymentConfigAPIFound } -// verifyTemplateAPI will verify that the template API is present. -func verifyTemplateAPI() error { - found, err := argoutil.VerifyAPI(template.GroupVersion.Group, template.GroupVersion.Version) +func verifyKeycloakTemplateAPIs() error { + var err error + deploymentConfigAPIFound, err = argoutil.VerifyAPI(deploymentConfig.GroupVersion.Group, deploymentConfig.GroupVersion.Version) if err != nil { return err } - templateAPIFound = found + + templateAPIFound, err = argoutil.VerifyAPI(template.GroupVersion.Group, template.GroupVersion.Version) + if err != nil { + return err + } + return nil } @@ -115,6 +123,16 @@ func (r *ReconcileArgoCD) reconcileSSO(cr *argoproj.ArgoCD) error { _ = r.reconcileStatusSSO(cr) return err } + + // DeploymentConfig API is being deprecated with OpenShift 4.14. Users who wish to + // install Keycloak using Template should enable the DeploymentConfig API. + if templateAPIFound && !deploymentConfigAPIFound { + ssoConfigLegalStatus = ssoLegalFailed + if err := r.reconcileStatusSSO(cr); err != nil { + return err + } + return fmt.Errorf("cannot manage Keycloak using Template since the DeploymentConfig API is not found") + } } // case 4 diff --git a/controllers/argocd/sso_test.go b/controllers/argocd/sso_test.go index 08d30e07d..b254576a0 100644 --- a/controllers/argocd/sso_test.go +++ b/controllers/argocd/sso_test.go @@ -33,6 +33,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1" + apierrors "k8s.io/apimachinery/pkg/api/errors" ) func TestReconcile_testKeycloakTemplateInstance(t *testing.T) { @@ -40,6 +41,7 @@ func TestReconcile_testKeycloakTemplateInstance(t *testing.T) { a := makeTestArgoCDForKeycloak() templateAPIFound = true + deploymentConfigAPIFound = true resObjs := []client.Object{a} subresObjs := []client.Object{a} @@ -263,6 +265,36 @@ func TestReconcile_testKeycloakK8sInstance(t *testing.T) { assert.NoError(t, r.reconcileSSO(a)) } +func TestReconcile_KeycloakTemplateWithoutDeploymentConfig(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + a := makeTestArgoCDForKeycloak() + + // Cluster has Template API but no DeploymentConfig API + templateAPIFound = true + deploymentConfigAPIFound = false + defer removeTemplateAPI() + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme, templatev1.Install, oappsv1.Install, routev1.Install) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch) + + assert.NoError(t, createNamespace(r, a.Namespace, "")) + + err := r.reconcileSSO(a) + assert.NotNil(t, err) + expectedErrMsg := "cannot manage Keycloak using Template since the DeploymentConfig API is not found" + assert.Equal(t, err.Error(), expectedErrMsg) + + // Verify that the Template instance is not created. + templateInstance := &templatev1.TemplateInstance{} + err = r.Client.Get(context.TODO(), types.NamespacedName{Name: "rhsso", Namespace: a.Namespace}, templateInstance) + assert.NotNil(t, err) + assert.True(t, apierrors.IsNotFound(err)) +} + func TestReconcile_testKeycloakInstanceResources(t *testing.T) { logf.SetLogger(ZapLogger(true)) a := makeTestArgoCDForKeycloak() diff --git a/controllers/argocd/status.go b/controllers/argocd/status.go index ae143ca1f..08017ad10 100644 --- a/controllers/argocd/status.go +++ b/controllers/argocd/status.go @@ -130,7 +130,7 @@ func (r *ReconcileArgoCD) reconcileStatusDex(cr *argoproj.ArgoCD) error { func (r *ReconcileArgoCD) reconcileStatusKeycloak(cr *argoproj.ArgoCD) error { status := "Unknown" - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { // keycloak is installed using OpenShift templates. dc := &oappsv1.DeploymentConfig{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controllers/argocd/status_test.go b/controllers/argocd/status_test.go index 78ab78b69..119f693b1 100644 --- a/controllers/argocd/status_test.go +++ b/controllers/argocd/status_test.go @@ -67,6 +67,7 @@ func TestReconcileArgoCD_reconcileStatusKeycloak_OpenShift(t *testing.T) { assert.NoError(t, oappsv1.Install(r.Scheme)) templateAPIFound = true + deploymentConfigAPIFound = true defer removeTemplateAPI() dc := getKeycloakDeploymentConfigTemplate(a) diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index e60170ae4..95254f40f 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -616,7 +616,7 @@ func InspectCluster() error { return err } - if err := verifyTemplateAPI(); err != nil { + if err := verifyKeycloakTemplateAPIs(); err != nil { return err } @@ -1078,7 +1078,7 @@ func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResou bldr.Owns(&monitoringv1.ServiceMonitor{}) } - if IsTemplateAPIAvailable() { + if CanUseKeycloakWithTemplate() { // Watch for the changes to Deployment Config bldr.Owns(&oappsv1.DeploymentConfig{}, builder.WithPredicates(deploymentConfigPred)) diff --git a/main.go b/main.go index 5e743cdb8..db5eda82d 100644 --- a/main.go +++ b/main.go @@ -227,7 +227,8 @@ func main() { } // Setup Schemes for SSO if template instance is available. - if argocd.IsTemplateAPIAvailable() { + if argocd.CanUseKeycloakWithTemplate() { + setupLog.Info("Keycloak instance can be managed using OpenShift Template") if err := templatev1.Install(mgr.GetScheme()); err != nil { setupLog.Error(err, "") os.Exit(1) @@ -240,6 +241,8 @@ func main() { setupLog.Error(err, "") os.Exit(1) } + } else { + setupLog.Info("Keycloak instance cannot be managed using OpenShift Template, as DeploymentConfig/Template API is not present") } if err = (&argocd.ReconcileArgoCD{