Add host attribute to SSO provider spec (#1306)

* Add host attribute to SSO provider spec Signed-off-by: Rizwana777 <[email protected]> * Fix changes in tests and minor fixes Signed-off-by: Rizwana777 <[email protected]> * Host attribute unit test updates Signed-off-by: Jonathan West <[email protected]> --------- Signed-off-by: Rizwana777 <[email protected]> Signed-off-by: Jonathan West <[email protected]> Co-authored-by: Jonathan West <[email protected]>
argoproj-labs · May 22, 2024 · a960d98 · a960d98
1 parent 1ff45e5
commit a960d98
Show file tree

Hide file tree

Showing 14 changed files with 225 additions and 6 deletions.
diff --git a/api/v1alpha1/argocd_conversion.go b/api/v1alpha1/argocd_conversion.go
@@ -36,6 +36,7 @@ func (src *ArgoCD) ConvertTo(dstRaw conversion.Hub) error {
 			sso.Keycloak.Version = src.Spec.SSO.Version
 			sso.Keycloak.VerifyTLS = src.Spec.SSO.VerifyTLS
 			sso.Keycloak.Resources = src.Spec.SSO.Resources
+			sso.Keycloak.Host = src.Spec.SSO.Keycloak.Host
 		}
 	}
 

diff --git a/api/v1alpha1/argocd_conversion_test.go b/api/v1alpha1/argocd_conversion_test.go
@@ -188,6 +188,7 @@ func TestAlphaToBetaConversion(t *testing.T) {
 					Provider: SSOProviderTypeKeycloak,
 					Keycloak: &ArgoCDKeycloakSpec{
 						RootCA: "__CA__",
+						Host:   "test-keycloak-host",
 					},
 					VerifyTLS: tls,
 				}
@@ -200,6 +201,7 @@ func TestAlphaToBetaConversion(t *testing.T) {
 					Keycloak: &v1beta1.ArgoCDKeycloakSpec{
 						RootCA:    "__CA__",
 						VerifyTLS: tls,
+						Host:      "test-keycloak-host",
 					},
 				}
 			}),
@@ -209,12 +211,16 @@ func TestAlphaToBetaConversion(t *testing.T) {
 			input: makeTestArgoCDAlpha(func(cr *ArgoCD) {
 				cr.Spec.SSO = &ArgoCDSSOSpec{
 					Image: "test-image",
+					Keycloak: &ArgoCDKeycloakSpec{
+						Host: "test-host",
+					},
 				}
 			}),
 			expectedOutput: makeTestArgoCDBeta(func(cr *v1beta1.ArgoCD) {
 				cr.Spec.SSO = &v1beta1.ArgoCDSSOSpec{
 					Keycloak: &v1beta1.ArgoCDKeycloakSpec{
 						Image: "test-image",
+						Host:  "test-host",
 					},
 				}
 			}),

diff --git a/api/v1alpha1/argocd_types.go b/api/v1alpha1/argocd_types.go
@@ -302,6 +302,9 @@ type ArgoCDKeycloakSpec struct {
 
 	// VerifyTLS set to false disables strict TLS validation.
 	VerifyTLS *bool `json:"verifyTLS,omitempty"`
+
+	// Host is the hostname to use for Ingress/Route resources.
+	Host string `json:"host,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/api/v1beta1/argocd_types.go b/api/v1beta1/argocd_types.go
@@ -328,6 +328,9 @@ type ArgoCDKeycloakSpec struct {
 
 	// VerifyTLS set to false disables strict TLS validation.
 	VerifyTLS *bool `json:"verifyTLS,omitempty"`
+
+	// Host is the hostname to use for Ingress/Route resources.
+	Host string `json:"host,omitempty"`
 }
 
 //+kubebuilder:object:root=true

diff --git a/bundle/manifests/argoproj.io_argocds.yaml b/bundle/manifests/argoproj.io_argocds.yaml
@@ -6693,6 +6693,10 @@ spec:
                     description: Keycloak contains the configuration for Argo CD keycloak
                       authentication
                     properties:
+                      host:
+                        description: Host is the hostname to use for Ingress/Route
+                          resources.
+                        type: string
                       image:
                         description: Image is the Keycloak container image.
                         type: string
@@ -13704,6 +13708,10 @@ spec:
                     description: Keycloak contains the configuration for Argo CD keycloak
                       authentication
                     properties:
+                      host:
+                        description: Host is the hostname to use for Ingress/Route
+                          resources.
+                        type: string
                       image:
                         description: Image is the Keycloak container image.
                         type: string

diff --git a/config/crd/bases/argoproj.io_argocds.yaml b/config/crd/bases/argoproj.io_argocds.yaml
@@ -6684,6 +6684,10 @@ spec:
                     description: Keycloak contains the configuration for Argo CD keycloak
                       authentication
                     properties:
+                      host:
+                        description: Host is the hostname to use for Ingress/Route
+                          resources.
+                        type: string
                       image:
                         description: Image is the Keycloak container image.
                         type: string
@@ -13695,6 +13699,10 @@ spec:
                     description: Keycloak contains the configuration for Argo CD keycloak
                       authentication
                     properties:
+                      host:
+                        description: Host is the hostname to use for Ingress/Route
+                          resources.
+                        type: string
                       image:
                         description: Image is the Keycloak container image.
                         type: string

diff --git a/controllers/argocd/keycloak.go b/controllers/argocd/keycloak.go
@@ -394,7 +394,7 @@ func getKeycloakServiceTemplate(ns string) *corev1.Service {
 	}
 }
 
-func getKeycloakRouteTemplate(ns string) *routev1.Route {
+func getKeycloakRouteTemplate(ns string, cr argoproj.ArgoCD) *routev1.Route {
 	return &routev1.Route{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels:      map[string]string{"application": "${APPLICATION_NAME}"},
@@ -404,6 +404,7 @@ func getKeycloakRouteTemplate(ns string) *routev1.Route {
 		},
 		TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Route"},
 		Spec: routev1.RouteSpec{
+			Host: getKeycloakOpenshiftHost(cr.Spec.SSO.Keycloak),
 			TLS: &routev1.TLSConfig{
 				Termination: "reencrypt",
 			},
@@ -437,7 +438,7 @@ func newKeycloakTemplate(cr *argoproj.ArgoCD) (template.Template, error) {
 	secretTemplate := getKeycloakSecretTemplate(ns)
 	deploymentConfigTemplate := getKeycloakDeploymentConfigTemplate(cr)
 	serviceTemplate := getKeycloakServiceTemplate(ns)
-	routeTemplate := getKeycloakRouteTemplate(ns)
+	routeTemplate := getKeycloakRouteTemplate(ns, *cr)
 
 	configMap, err := json.Marshal(configMapTemplate)
 	if err != nil {
@@ -534,7 +535,7 @@ func newKeycloakIngress(cr *argoproj.ArgoCD) *networkingv1.Ingress {
 			},
 			Rules: []networkingv1.IngressRule{
 				{
-					Host: keycloakIngressHost,
+					Host: getKeycloakIngressHost(cr.Spec.SSO.Keycloak),
 					IngressRuleValue: networkingv1.IngressRuleValue{
 						HTTP: &networkingv1.HTTPIngressRuleValue{
 							Paths: []networkingv1.HTTPIngressPath{

diff --git a/controllers/argocd/keycloak_test.go b/controllers/argocd/keycloak_test.go
@@ -331,13 +331,39 @@ func TestNewKeycloakTemplate_testService(t *testing.T) {
 }
 
 func TestNewKeycloakTemplate_testRoute(t *testing.T) {
-	route := getKeycloakRouteTemplate(fakeNs)
+	a := makeTestArgoCDForKeycloak()
+	a.Spec.SSO = &argoproj.ArgoCDSSOSpec{
+		Keycloak: &argoproj.ArgoCDKeycloakSpec{
+			Host: "sso.test.example.com",
+		},
+		Provider: "keycloak",
+	}
+	route := getKeycloakRouteTemplate(fakeNs, *a)
+	assert.Equal(t, route.Name, "${APPLICATION_NAME}")
+	assert.Equal(t, route.Namespace, fakeNs)
+	assert.Equal(t, route.Spec.To,
+		routev1.RouteTargetReference{Name: "${APPLICATION_NAME}"})
+	assert.Equal(t, route.Spec.TLS,
+		&routev1.TLSConfig{Termination: "reencrypt"})
+	assert.Equal(t, route.Spec.Host, a.Spec.SSO.Keycloak.Host)
+}
+
+func TestNewKeycloakTemplate_testRouteWhenHostIsEmpty(t *testing.T) {
+	a := makeTestArgoCDForKeycloak()
+	a.Spec.SSO = &argoproj.ArgoCDSSOSpec{
+		Provider: "keycloak",
+	}
+
+	assert.True(t, a.Spec.SSO.Keycloak == nil || a.Spec.SSO.Keycloak.Host == "", "host must be empty, or keycloak must be nil (which implies host is empty)")
+
+	route := getKeycloakRouteTemplate(fakeNs, *a)
 	assert.Equal(t, route.Name, "${APPLICATION_NAME}")
 	assert.Equal(t, route.Namespace, fakeNs)
 	assert.Equal(t, route.Spec.To,
 		routev1.RouteTargetReference{Name: "${APPLICATION_NAME}"})
 	assert.Equal(t, route.Spec.TLS,
 		&routev1.TLSConfig{Termination: "reencrypt"})
+	assert.Equal(t, route.Spec.Host, "")
 }
 
 func TestKeycloak_testRealmConfigCreation(t *testing.T) {

diff --git a/controllers/argocd/sso_test.go b/controllers/argocd/sso_test.go
@@ -17,6 +17,7 @@ package argocd
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 
 	oappsv1 "github.com/openshift/api/apps/v1"
@@ -26,7 +27,7 @@ import (
 	k8sappsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -326,7 +327,7 @@ func TestReconcile_testKeycloakInstanceResources(t *testing.T) {
 	}
 	assert.Equal(t, deployment.Labels, testLabels)
 
-	testSelector := &v1.LabelSelector{
+	testSelector := &metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app": defaultKeycloakIdentifier,
 		},
@@ -401,3 +402,119 @@ func TestReconcile_testKeycloakInstanceResources(t *testing.T) {
 
 	assert.Equal(t, ing.Spec.Rules, testRules)
 }
+
+func TestReconcile_testKeycloakIngressHost(t *testing.T) {
+	logf.SetLogger(ZapLogger(true))
+	a := makeTestArgoCDForKeycloak()
+	a.Spec.SSO.Keycloak = &argoproj.ArgoCDKeycloakSpec{
+		Host: "sso.test.example.com",
+	}
+
+	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, ""))
+
+	assert.NoError(t, r.reconcileSSO(a))
+
+	// Keycloak Ingress
+	ing := &networkingv1.Ingress{}
+	testPathType := networkingv1.PathTypeImplementationSpecific
+	err := r.Client.Get(context.TODO(), types.NamespacedName{Name: defaultKeycloakIdentifier, Namespace: a.Namespace}, ing)
+	assert.NoError(t, err)
+
+	assert.Equal(t, ing.Name, defaultKeycloakIdentifier)
+	assert.Equal(t, ing.Namespace, a.Namespace)
+
+	testTLS := []networkingv1.IngressTLS{
+		{
+			Hosts: []string{keycloakIngressHost},
+		},
+	}
+	assert.Equal(t, ing.Spec.TLS, testTLS)
+
+	testRules := []networkingv1.IngressRule{
+		{
+			Host: "sso.test.example.com",
+			IngressRuleValue: networkingv1.IngressRuleValue{
+				HTTP: &networkingv1.HTTPIngressRuleValue{
+					Paths: []networkingv1.HTTPIngressPath{
+						{
+							Path: "/",
+							Backend: networkingv1.IngressBackend{
+								Service: &networkingv1.IngressServiceBackend{
+									Name: defaultKeycloakIdentifier,
+									Port: networkingv1.ServiceBackendPort{
+										Name: "http",
+									},
+								},
+							},
+							PathType: &testPathType,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	assert.Equal(t, ing.Spec.Rules, testRules)
+
+}
+
+func TestReconcile_testKeycloakRouteHost(t *testing.T) {
+	logf.SetLogger(ZapLogger(true))
+	a := makeTestArgoCDForKeycloak()
+	a.Spec.SSO.Keycloak = &argoproj.ArgoCDKeycloakSpec{
+		Host: "sso.test.example.com",
+	}
+
+	// Set templateAPIFound to true, to simulate running on an OpenShift machine
+	templateAPIFound = true
+	deploymentConfigAPIFound = true
+	ssoConfigLegalStatus = ""
+	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, ""))
+
+	assert.NoError(t, r.reconcileSSO(a))
+
+	// Calls to reconcileSSO will create a TemplateInstance
+	templ := templatev1.TemplateInstance{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      defaultTemplateIdentifier,
+			Namespace: a.Namespace,
+		},
+	}
+	err := r.Client.Get(context.Background(), client.ObjectKeyFromObject(&templ), &templ)
+	assert.NoError(t, err)
+
+	// TemplateInstance contains a set of Objects, but we only care about the Route
+	matchFound := false
+	for _, obj := range templ.Spec.Template.Objects {
+
+		strVal := string(obj.Raw)
+
+		// Look for the Route object within the TemplateInstance
+		if strings.Contains(strVal, "\"kind\":\"Route\"") {
+
+			// Make sure the Route object contains the host
+			assert.Contains(t, strVal, "sso.test.example.com", "the Route portion of the template should contain the host value from above")
+			matchFound = true
+		}
+
+	}
+
+	assert.True(t, matchFound)
+
+}
diff --git a/controllers/argocd/status_test.go b/controllers/argocd/status_test.go
@@ -105,12 +105,28 @@ func TestReconcileArgoCD_reconcileStatusSSO(t *testing.T) {
 	}{
 		{
 			name: "both dex and keycloak configured",
+			argoCD: makeTestArgoCD(func(cr *argoproj.ArgoCD) {
+				cr.Spec.SSO = &argoproj.ArgoCDSSOSpec{
+					Provider: argoproj.SSOProviderTypeKeycloak,
+					Keycloak: &argoproj.ArgoCDKeycloakSpec{
+						Host: "sso.test.example.com",
+					},
+					Dex: &argoproj.ArgoCDDexSpec{
+						OpenShiftOAuth: true,
+					},
+				}
+			}),
+			wantSSOStatus: "Failed",
+		},
+		{
+			name: "both dex and keycloak configured, and keycloak host is empty",
 			argoCD: makeTestArgoCD(func(cr *argoproj.ArgoCD) {
 				cr.Spec.SSO = &argoproj.ArgoCDSSOSpec{
 					Provider: argoproj.SSOProviderTypeKeycloak,
 					Dex: &argoproj.ArgoCDDexSpec{
 						OpenShiftOAuth: true,
 					},
+					Keycloak: &argoproj.ArgoCDKeycloakSpec{},
 				}
 			}),
 			wantSSOStatus: "Failed",

diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go
@@ -271,6 +271,23 @@ func getArgoServerHost(cr *argoproj.ArgoCD) string {
 	return host
 }
 
+// getKeycloakIngressHost will return the host for the given ArgoCD.
+func getKeycloakIngressHost(cr *argoproj.ArgoCDKeycloakSpec) string {
+	if cr != nil && len(cr.Host) > 0 {
+		return cr.Host
+	}
+	// If cr is nil or cr.Host is empty, return a default value or handle it accordingly.
+	return keycloakIngressHost
+}
+
+// getKeycloakIngressHost will return the host for the given ArgoCD.
+func getKeycloakOpenshiftHost(cr *argoproj.ArgoCDKeycloakSpec) string {
+	if cr != nil && len(cr.Host) > 0 {
+		return cr.Host
+	}
+	return ""
+}
+
 // getArgoServerResources will return the ResourceRequirements for the Argo CD server container.
 func getArgoServerResources(cr *argoproj.ArgoCD) corev1.ResourceRequirements {
 	resources := corev1.ResourceRequirements{}