From 3c83ca9f1d102d251617c8b0bc04f5328463f6bf Mon Sep 17 00:00:00 2001 From: Francisco Barros <francisco.borges.aurindo.barros@cern.ch> Date: Tue, 3 Oct 2023 16:24:06 +0200 Subject: [PATCH 1/5] First working prototype of SSO proxy --- controllers/drupalsite_controller.go | 26 ++ controllers/drupalsite_resources.go | 251 ++++++++++++++++-- .../drupalsite_update_controller_utils.go | 2 +- controllers/reconciler_common.go | 42 ++- main.go | 1 + 5 files changed, 291 insertions(+), 31 deletions(-) diff --git a/controllers/drupalsite_controller.go b/controllers/drupalsite_controller.go index fa0ac7ca..c7fec93a 100644 --- a/controllers/drupalsite_controller.go +++ b/controllers/drupalsite_controller.go @@ -49,6 +49,8 @@ const ( debugAnnotation = "debug" adminPauseAnnotation = "admin-pause-reconcile" oidcSecretName = "oidc-client-secret" + + SSOProxyLabel = "drupal.okd.cern.ch/full-sso" ) var ( @@ -58,6 +60,8 @@ var ( PhpFpmExporterImage string // WebDAVImage refers to the webdav image name WebDAVImage string + // SSOProxyImage refers to the sso proxy image link + SSOProxyImage string // SMTPHost used by Drupal server pods to send emails SMTPHost string // VeleroNamespace refers to the namespace of the velero server to create backups @@ -224,6 +228,28 @@ func (r *DrupalSiteReconciler) Reconcile(ctx context.Context, req ctrl.Request) return r.updateCRStatusOrFailReconcile(ctx, log, drupalSite) } + // Mirror SSO Proxy label from namespace, if it exists + namespace := &corev1.Namespace{} + err = r.Get(ctx, types.NamespacedName{Name: drupalSite.Namespace}, namespace) + if err != nil { + if k8sapierrors.IsNotFound(err) { + // Request object not found, could have been deleted after reconcile request. + // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. + // Return and don't requeue + log.V(3).Info("Namespace resource not found. Ignoring since object must be deleted") + return ctrl.Result{}, nil + } + // Error reading the object - requeue the request. + log.Error(err, "Failed to get Namespace") + } + if drupalSite.Labels == nil { + drupalSite.Labels = map[string]string{} + } + if namespace.Labels[SSOProxyLabel] == "true" && drupalSite.Labels[SSOProxyLabel] != "true" { + drupalSite.Labels[SSOProxyLabel] = "true" + return r.updateCRorFailReconcile(ctx, log, drupalSite) + } + // 2. Check all conditions and update them if needed update := false diff --git a/controllers/drupalsite_resources.go b/controllers/drupalsite_resources.go index d8961b75..b5b9aa6d 100644 --- a/controllers/drupalsite_resources.go +++ b/controllers/drupalsite_resources.go @@ -24,7 +24,6 @@ import ( "io/ioutil" "math/rand" "net/url" - "path" "strconv" "time" @@ -61,6 +60,8 @@ const ( // Variable to set the used Memory for all Jobs generated by the Operator jobMemoryRequest string = "512Mi" jobMemoryLimit string = "4Gi" + // ProxyPort servin port + ssoProxyPort = 8989 ) var ( @@ -743,8 +744,17 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st } currentobject.Annotations["alpha.image.policy.openshift.io/resolve-names"] = "*" + ssoProxyEnabled := false + if d.Labels[SSOProxyLabel] == "true" { + ssoProxyEnabled = true + } + if currentobject.CreationTimestamp.IsZero() { - currentobject.Spec.Template.Spec.Containers = []corev1.Container{{Name: "nginx"}, {Name: "php-fpm"}, {Name: "php-fpm-exporter"}, {Name: "webdav"}, {Name: "cron"}, {Name: "drupal-logs"}} + if ssoProxyEnabled { + currentobject.Spec.Template.Spec.Containers = []corev1.Container{{Name: "nginx"}, {Name: "php-fpm"}, {Name: "php-fpm-exporter"}, {Name: "webdav"}, {Name: "cron"}, {Name: "drupal-logs"}, {Name: "sso-proxy"}} + } else { + currentobject.Spec.Template.Spec.Containers = []corev1.Container{{Name: "nginx"}, {Name: "php-fpm"}, {Name: "php-fpm-exporter"}, {Name: "webdav"}, {Name: "cron"}, {Name: "drupal-logs"}} + } } else { containerExists("nginx", currentobject) containerExists("php-fpm", currentobject) @@ -752,6 +762,11 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st containerExists("webdav", currentobject) containerExists("cron", currentobject) containerExists("drupal-logs", currentobject) + if ssoProxyEnabled { + containerExists("sso-proxy", currentobject) + } else { + containerRemove("sso-proxy", currentobject) + } } // Settings only on creation (not enforced) @@ -998,6 +1013,88 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st MountPath: "/var/run/", }, } + case "sso-proxy": + // Set to always due to https://gitlab.cern.ch/drupal/paas/drupalsite-operator/-/issues/54 + currentobject.Spec.Template.Spec.Containers[i].ImagePullPolicy = "IfNotPresent" + currentobject.Spec.Template.Spec.Containers[i].Ports = []corev1.ContainerPort{{ + ContainerPort: ssoProxyPort, + Name: "sso-proxy", + Protocol: "TCP", + }} + currentobject.Spec.Template.Spec.Containers[i].Env = []corev1.EnvVar{ + { + Name: "OAUTH2_PROXY_HTTP_ADDRESS", + Value: ":" + fmt.Sprint(ssoProxyPort), + }, + { + Name: "OAUTH2_PROXY_PROVIDER", + Value: "oidc", + }, + { + Name: "OAUTH2_PROXY_SCOPE", + Value: "openid", + }, + { + Name: "OAUTH2_PROXY_REVERSE_PROXY", + Value: "true", + }, + { + Name: "OAUTH2_PROXY_UPSTREAMS", + Value: "http://localhost:8080/", + }, + { + Name: "OAUTH2_PROXY_EMAIL_DOMAINS", + Value: "*", + }, + { + Name: "OAUTH2_PROXY_WHITELIST_DOMAINS", + Value: ".cern.ch", + }, + { + Name: "OAUTH2_PROXY_COOKIE_PATH", + Value: "/", + }, + { + Name: "OAUTH2_PROXY_SILENCE_PING_LOGGING", + Value: "true", + }, + { + Name: "OAUTH2_PROXY_CLIENT_ID", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "clientID", + }, + }, + }, + { + Name: "OAUTH2_PROXY_CLIENT_SECRET", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "clientSecret", + }, + }, + }, + { + Name: "OAUTH2_PROXY_OIDC_ISSUER_URL", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "issuerURL", + }, + }, + }, + { + Name: "OAUTH2_PROXY_COOKIE_SECRET", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "suggestedCookieSecret", + }, + }, + }, + } } } } @@ -1102,7 +1199,98 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st MountPath: "/var/run/", }, } + + case "sso-proxy": + currentobject.Spec.Template.Spec.Containers[i].Image = SSOProxyImage + // currentobject.Spec.Template.Spec.Containers[i].Command = []string{"sh"} + currentobject.Spec.Template.Spec.Containers[i].Resources = config.nginxResources + // Set to always due to https://gitlab.cern.ch/drupal/paas/drupalsite-operator/-/issues/54 + currentobject.Spec.Template.Spec.Containers[i].ImagePullPolicy = "IfNotPresent" + currentobject.Spec.Template.Spec.Containers[i].Ports = []corev1.ContainerPort{{ + ContainerPort: ssoProxyPort, + Name: "sso-proxy", + Protocol: "TCP", + }} + currentobject.Spec.Template.Spec.Containers[i].Env = []corev1.EnvVar{ + { + Name: "OAUTH2_PROXY_HTTP_ADDRESS", + Value: ":" + fmt.Sprint(ssoProxyPort), + }, + { + Name: "OAUTH2_PROXY_PROVIDER", + Value: "oidc", + }, + { + Name: "OAUTH2_PROXY_SCOPE", + Value: "openid", + }, + { + Name: "OAUTH2_PROXY_REVERSE_PROXY", + Value: "true", + }, + { + Name: "OAUTH2_PROXY_UPSTREAMS", + Value: "http://localhost:8080/", + }, + { + Name: "OAUTH2_PROXY_EMAIL_DOMAINS", + Value: "*", + }, + { + Name: "OAUTH2_PROXY_WHITELIST_DOMAINS", + Value: ".cern.ch", + }, + { + Name: "OAUTH2_PROXY_COOKIE_PATH", + Value: "/", + }, + { + Name: "OAUTH2_PROXY_SILENCE_PING_LOGGING", + Value: "true", + }, + { + Name: "OAUTH2_PROXY_SKIP_PROVIDER_BUTTON", + Value: "true", + }, + { + Name: "OAUTH2_PROXY_CLIENT_ID", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "clientID", + }, + }, + }, + { + Name: "OAUTH2_PROXY_CLIENT_SECRET", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "clientSecret", + }, + }, + }, + { + Name: "OAUTH2_PROXY_OIDC_ISSUER_URL", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "issuerURL", + }, + }, + }, + { + Name: "OAUTH2_PROXY_COOKIE_SECRET", + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{Name: "oidc-client-secret"}, + Key: "suggestedCookieSecret", + }, + }, + }, + } } + } currentobject.Spec.Replicas = &config.replicas // Add an annotation to be able to verify what releaseID of pod is running. Did not use labels, as it will affect the labelselector for the deployment and might cause downtime @@ -1213,19 +1401,35 @@ func serviceForDrupalSite(currentobject *corev1.Service, d *webservicesv1a1.Drup addOwnerRefToObject(currentobject, asOwner(d)) currentobject.Spec.Selector = ls - currentobject.Spec.Ports = []corev1.ServicePort{ - { - TargetPort: intstr.FromInt(8080), - Name: "nginx", - Port: 80, - Protocol: "TCP", - }, - { - TargetPort: intstr.FromInt(9253), - Name: "php-fpm-exporter", - Port: 9253, - Protocol: "TCP", - }} + if d.Labels[SSOProxyLabel] == "true" { + currentobject.Spec.Ports = []corev1.ServicePort{ + { + TargetPort: intstr.FromInt(ssoProxyPort), + Name: "sso-proxy", + Port: 80, + Protocol: "TCP", + }, + { + TargetPort: intstr.FromInt(9253), + Name: "php-fpm-exporter", + Port: 9253, + Protocol: "TCP", + }} + } else { + currentobject.Spec.Ports = []corev1.ServicePort{ + { + TargetPort: intstr.FromInt(8080), + Name: "nginx", + Port: 80, + Protocol: "TCP", + }, + { + TargetPort: intstr.FromInt(9253), + Name: "php-fpm-exporter", + Port: 9253, + Protocol: "TCP", + }} + } return nil } @@ -1241,8 +1445,19 @@ func routeForDrupalSite(currentobject *routev1.Route, d *webservicesv1a1.DrupalS Name: d.Name, Weight: pointer.Int32Ptr(100), } - currentobject.Spec.Port = &routev1.RoutePort{ - TargetPort: intstr.FromInt(8080), + ssoProxyEnabled := false + if d.Labels[SSOProxyLabel] == "true" { + ssoProxyEnabled = true + } + + if ssoProxyEnabled == true { + currentobject.Spec.Port = &routev1.RoutePort{ + TargetPort: intstr.FromInt(ssoProxyPort), + } + } else { + currentobject.Spec.Port = &routev1.RoutePort{ + TargetPort: intstr.FromInt(8080), + } } if currentobject.Annotations == nil { @@ -1286,7 +1501,7 @@ func newOidcReturnURI(currentobject *authz.OidcReturnURI, d *webservicesv1a1.Dru } // This will append `/openid-connect/*` to the URL, guaranteeing all subpaths of the link can be redirected - url.Path = path.Join(url.Path, "openid-connect") + // url.Path = path.Join(url.Path, "openid-connect") if http { returnURI = "http://" + url.String() + "/*" // Hardcoded since with path.Join method creates `%2A` which will not work in the AuthzAPI, and the prefix `http` } else { diff --git a/controllers/drupalsite_update_controller_utils.go b/controllers/drupalsite_update_controller_utils.go index 41f39e55..cac1e742 100644 --- a/controllers/drupalsite_update_controller_utils.go +++ b/controllers/drupalsite_update_controller_utils.go @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, diff --git a/controllers/reconciler_common.go b/controllers/reconciler_common.go index b37a2721..109c9be5 100644 --- a/controllers/reconciler_common.go +++ b/controllers/reconciler_common.go @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, @@ -172,16 +172,17 @@ func ResourceRequestLimit(memReq, cpuReq, memLim, cpuLim string) (corev1.Resourc // reqLimDict returns the resource requests and limits for a given QoS class and container. // TODO: this should be part of operator configuration, read from a YAML file with format // defaultResources: -// critical: -// phpFpm: -// resources: -// # normal K8s req/lim -// nginx: -// # ... -// standard: -// # ... -// eco: -// # ... +// +// critical: +// phpFpm: +// resources: +// # normal K8s req/lim +// nginx: +// # ... +// standard: +// # ... +// eco: +// # ... func reqLimDict(container string, qosClass webservicesv1a1.QoSClass) (corev1.ResourceRequirements, error) { switch container { case "php-fpm": @@ -302,7 +303,7 @@ func addGitlabWebhookToStatus(ctx context.Context, drp *webservicesv1a1.DrupalSi return false } -//validateSpec validates the spec against the DrupalSiteSpec definition +// validateSpec validates the spec against the DrupalSiteSpec definition func validateSpec(drpSpec webservicesv1a1.DrupalSiteSpec) reconcileError { _, err := govalidator.ValidateStruct(drpSpec) if err != nil { @@ -496,6 +497,21 @@ func containerExists(name string, currentobject *appsv1.Deployment) { } } +// containerExists checks if a container exists on the deployment +// if it doesn't exists, it adds it +func containerRemove(name string, currentobject *appsv1.Deployment) { + for index, container := range currentobject.Spec.Template.Spec.Containers { + if container.Name == name { + if index == len(currentobject.Spec.Template.Spec.Containers)-1 { + currentobject.Spec.Template.Spec.Containers = currentobject.Spec.Template.Spec.Containers[:index] + } else { + currentobject.Spec.Template.Spec.Containers = append(currentobject.Spec.Template.Spec.Containers[:index], currentobject.Spec.Template.Spec.Containers[index+1]) + } + break + } + } +} + // getDeploymentConfiguration precalculates all the configuration that the server deployment needs, including: // pod replicas, resource req/lim // NOTE: this includes the default resource limits for PHP @@ -619,12 +635,14 @@ func (r *Reconciler) updateCRStatusOrFailReconcile(ctx context.Context, log logr // // Example: // ```` +// // sout, serr, err := r.execToServerPod(ctx, drp, "php-fpm", nil, "sh", "-c", "drush version; ls") // sout, serr, err := r.execToServerPod(ctx, drp, "php-fpm", nil, "drush", "version") // if err != nil { // log.Error(err, "Error while exec into pod") // } // log.Info("EXEC", "stdout", sout, "stderr", serr) +// // ```` func (r *Reconciler) execToServerPod(ctx context.Context, d *webservicesv1a1.DrupalSite, containerName string, stdin io.Reader, command ...string) (stdout string, stderr string, err error) { pod, err := r.getRunningPodForVersion(ctx, d, releaseID(d)) diff --git a/main.go b/main.go index 94892db8..617efd3c 100644 --- a/main.go +++ b/main.go @@ -79,6 +79,7 @@ func main() { flag.StringVar(&controllers.SiteBuilderImage, "sitebuilder-image", "gitlab-registry.cern.ch/drupal/paas/cern-drupal-distribution/site-builder", "The sitebuilder source image name.") flag.StringVar(&controllers.PhpFpmExporterImage, "php-fpm-exporter-image", "gitlab-registry.cern.ch/drupal/paas/php-fpm-prometheus-exporter:RELEASE.2021.06.02T09-41-38Z", "The php-fpm-exporter source image name.") flag.StringVar(&controllers.WebDAVImage, "webdav-image", "gitlab-registry.cern.ch/drupal/paas/sabredav/webdav:RELEASE-2021.10.12T17-55-06Z", "The webdav source image name.") + flag.StringVar(&controllers.SSOProxyImage, "ssoproxy-image", "quay.io/oauth2-proxy/oauth2-proxy:latest", "The sso proxy source image name.") // TODO: Get real image flag.StringVar(&controllers.SMTPHost, "smtp-host", "cernmx.cern.ch", "SMTP host used by Drupal server pods to send emails.") flag.StringVar(&controllers.VeleroNamespace, "velero-namespace", "openshift-cern-drupal", "The namespace of the Velero server to create backups") flag.IntVar(&controllers.ParallelThreadCount, "parallel-thread-count", 1, "The default number of parallel threads executed by the DrupalSite Operator controllers") -- GitLab From 5655dd11f1a67ea562d3ed8344911d716f278e6d Mon Sep 17 00:00:00 2001 From: Francisco Barros <francisco.borges.aurindo.barros@cern.ch> Date: Tue, 5 Dec 2023 15:02:52 +0100 Subject: [PATCH 2/5] Added new comments and changed code based on review --- controllers/drupalsite_resources.go | 8 +++++--- controllers/reconciler_common.go | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/controllers/drupalsite_resources.go b/controllers/drupalsite_resources.go index b5b9aa6d..5808d79f 100644 --- a/controllers/drupalsite_resources.go +++ b/controllers/drupalsite_resources.go @@ -750,10 +750,9 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st } if currentobject.CreationTimestamp.IsZero() { + currentobject.Spec.Template.Spec.Containers = []corev1.Container{{Name: "nginx"}, {Name: "php-fpm"}, {Name: "php-fpm-exporter"}, {Name: "webdav"}, {Name: "cron"}, {Name: "drupal-logs"}} if ssoProxyEnabled { currentobject.Spec.Template.Spec.Containers = []corev1.Container{{Name: "nginx"}, {Name: "php-fpm"}, {Name: "php-fpm-exporter"}, {Name: "webdav"}, {Name: "cron"}, {Name: "drupal-logs"}, {Name: "sso-proxy"}} - } else { - currentobject.Spec.Template.Spec.Containers = []corev1.Container{{Name: "nginx"}, {Name: "php-fpm"}, {Name: "php-fpm-exporter"}, {Name: "webdav"}, {Name: "cron"}, {Name: "drupal-logs"}} } } else { containerExists("nginx", currentobject) @@ -1014,13 +1013,13 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st }, } case "sso-proxy": - // Set to always due to https://gitlab.cern.ch/drupal/paas/drupalsite-operator/-/issues/54 currentobject.Spec.Template.Spec.Containers[i].ImagePullPolicy = "IfNotPresent" currentobject.Spec.Template.Spec.Containers[i].Ports = []corev1.ContainerPort{{ ContainerPort: ssoProxyPort, Name: "sso-proxy", Protocol: "TCP", }} + // Values can be inspired from: https://gitlab.cern.ch/paas-tools/okd4-deployment/cern-auth-proxy/-/blob/master/cern-auth-proxy/templates/deployment.yaml?ref_type=heads currentobject.Spec.Template.Spec.Containers[i].Env = []corev1.EnvVar{ { Name: "OAUTH2_PROXY_HTTP_ADDRESS", @@ -1046,6 +1045,7 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st Name: "OAUTH2_PROXY_EMAIL_DOMAINS", Value: "*", }, + // NOTE: This SSO feature will not work with .cern domains! { Name: "OAUTH2_PROXY_WHITELIST_DOMAINS", Value: ".cern.ch", @@ -1211,6 +1211,7 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st Name: "sso-proxy", Protocol: "TCP", }} + // Values can be inspired from: https://gitlab.cern.ch/paas-tools/okd4-deployment/cern-auth-proxy/-/blob/master/cern-auth-proxy/templates/deployment.yaml?ref_type=heads currentobject.Spec.Template.Spec.Containers[i].Env = []corev1.EnvVar{ { Name: "OAUTH2_PROXY_HTTP_ADDRESS", @@ -1236,6 +1237,7 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st Name: "OAUTH2_PROXY_EMAIL_DOMAINS", Value: "*", }, + // NOTE: This SSO feature will not work with .cern domains! { Name: "OAUTH2_PROXY_WHITELIST_DOMAINS", Value: ".cern.ch", diff --git a/controllers/reconciler_common.go b/controllers/reconciler_common.go index 109c9be5..9ddd1b87 100644 --- a/controllers/reconciler_common.go +++ b/controllers/reconciler_common.go @@ -497,8 +497,8 @@ func containerExists(name string, currentobject *appsv1.Deployment) { } } -// containerExists checks if a container exists on the deployment -// if it doesn't exists, it adds it +// containerRemove checks if a container exists on the deployment +// if it does exists, removes it func containerRemove(name string, currentobject *appsv1.Deployment) { for index, container := range currentobject.Spec.Template.Spec.Containers { if container.Name == name { -- GitLab From 197a52a797d79d840ba4aa633857ee260f49e18c Mon Sep 17 00:00:00 2001 From: Francisco Barros <francisco.borges.aurindo.barros@cern.ch> Date: Tue, 5 Dec 2023 17:43:58 +0100 Subject: [PATCH 3/5] Implemented some comments/suggestions --- controllers/drupalsite_controller.go | 6 +-- controllers/drupalsite_resources.go | 66 ++++++++++------------------ 2 files changed, 27 insertions(+), 45 deletions(-) diff --git a/controllers/drupalsite_controller.go b/controllers/drupalsite_controller.go index c7fec93a..715687b9 100644 --- a/controllers/drupalsite_controller.go +++ b/controllers/drupalsite_controller.go @@ -50,7 +50,7 @@ const ( adminPauseAnnotation = "admin-pause-reconcile" oidcSecretName = "oidc-client-secret" - SSOProxyLabel = "drupal.okd.cern.ch/full-sso" + ssoProxyLabel = "drupal.okd.cern.ch/full-sso" ) var ( @@ -245,8 +245,8 @@ func (r *DrupalSiteReconciler) Reconcile(ctx context.Context, req ctrl.Request) if drupalSite.Labels == nil { drupalSite.Labels = map[string]string{} } - if namespace.Labels[SSOProxyLabel] == "true" && drupalSite.Labels[SSOProxyLabel] != "true" { - drupalSite.Labels[SSOProxyLabel] = "true" + if namespace.Labels[ssoProxyLabel] == "true" && drupalSite.Labels[ssoProxyLabel] != "true" { + drupalSite.Labels[ssoProxyLabel] = "true" return r.updateCRorFailReconcile(ctx, log, drupalSite) } diff --git a/controllers/drupalsite_resources.go b/controllers/drupalsite_resources.go index 5808d79f..b4f25a24 100644 --- a/controllers/drupalsite_resources.go +++ b/controllers/drupalsite_resources.go @@ -745,7 +745,7 @@ func deploymentForDrupalSite(currentobject *appsv1.Deployment, databaseSecret st currentobject.Annotations["alpha.image.policy.openshift.io/resolve-names"] = "*" ssoProxyEnabled := false - if d.Labels[SSOProxyLabel] == "true" { + if d.Labels[ssoProxyLabel] == "true" { ssoProxyEnabled = true } @@ -1403,35 +1403,25 @@ func serviceForDrupalSite(currentobject *corev1.Service, d *webservicesv1a1.Drup addOwnerRefToObject(currentobject, asOwner(d)) currentobject.Spec.Selector = ls - if d.Labels[SSOProxyLabel] == "true" { - currentobject.Spec.Ports = []corev1.ServicePort{ - { - TargetPort: intstr.FromInt(ssoProxyPort), - Name: "sso-proxy", - Port: 80, - Protocol: "TCP", - }, - { - TargetPort: intstr.FromInt(9253), - Name: "php-fpm-exporter", - Port: 9253, - Protocol: "TCP", - }} - } else { - currentobject.Spec.Ports = []corev1.ServicePort{ - { - TargetPort: intstr.FromInt(8080), - Name: "nginx", - Port: 80, - Protocol: "TCP", - }, - { - TargetPort: intstr.FromInt(9253), - Name: "php-fpm-exporter", - Port: 9253, - Protocol: "TCP", - }} + serverName := "nginx" + serverPort := 8080 + if d.Labels[ssoProxyLabel] == "true" { + serverName = "sso-proxy" + serverPort = ssoProxyPort } + currentobject.Spec.Ports = []corev1.ServicePort{ + { + TargetPort: intstr.FromInt(serverPort), + Name: serverName, + Port: 80, + Protocol: "TCP", + }, + { + TargetPort: intstr.FromInt(9253), + Name: "php-fpm-exporter", + Port: 9253, + Protocol: "TCP", + }} return nil } @@ -1447,21 +1437,13 @@ func routeForDrupalSite(currentobject *routev1.Route, d *webservicesv1a1.DrupalS Name: d.Name, Weight: pointer.Int32Ptr(100), } - ssoProxyEnabled := false - if d.Labels[SSOProxyLabel] == "true" { - ssoProxyEnabled = true + port := 8080 + if d.Labels[ssoProxyLabel] == "true" { + port = ssoProxyPort } - - if ssoProxyEnabled == true { - currentobject.Spec.Port = &routev1.RoutePort{ - TargetPort: intstr.FromInt(ssoProxyPort), - } - } else { - currentobject.Spec.Port = &routev1.RoutePort{ - TargetPort: intstr.FromInt(8080), - } + currentobject.Spec.Port = &routev1.RoutePort{ + TargetPort: intstr.FromInt(port), } - if currentobject.Annotations == nil { currentobject.Annotations = map[string]string{} } -- GitLab From 5b47ab465f1a7c5fe1baf78983f86167164f2f50 Mon Sep 17 00:00:00 2001 From: Francisco Barros <francisco.borges.aurindo.barros@cern.ch> Date: Wed, 6 Dec 2023 15:35:32 +0100 Subject: [PATCH 4/5] Fix bug on container removal --- controllers/reconciler_common.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/controllers/reconciler_common.go b/controllers/reconciler_common.go index 9ddd1b87..ab2a65e0 100644 --- a/controllers/reconciler_common.go +++ b/controllers/reconciler_common.go @@ -23,6 +23,7 @@ import ( "fmt" "io" "reflect" + "slices" "time" "github.com/asaskevich/govalidator" @@ -499,17 +500,10 @@ func containerExists(name string, currentobject *appsv1.Deployment) { // containerRemove checks if a container exists on the deployment // if it does exists, removes it -func containerRemove(name string, currentobject *appsv1.Deployment) { - for index, container := range currentobject.Spec.Template.Spec.Containers { - if container.Name == name { - if index == len(currentobject.Spec.Template.Spec.Containers)-1 { - currentobject.Spec.Template.Spec.Containers = currentobject.Spec.Template.Spec.Containers[:index] - } else { - currentobject.Spec.Template.Spec.Containers = append(currentobject.Spec.Template.Spec.Containers[:index], currentobject.Spec.Template.Spec.Containers[index+1]) - } - break - } - } +func containerRemove(name string, d *appsv1.Deployment) { + d.Spec.Template.Spec.Containers = slices.DeleteFunc(d.Spec.Template.Spec.Containers, func(container corev1.Container) bool { + return container.Name == name + }) } // getDeploymentConfiguration precalculates all the configuration that the server deployment needs, including: -- GitLab From 16757d551ec0a52910ff7d85157541c7119a6a36 Mon Sep 17 00:00:00 2001 From: Francisco Barros <francisco.borges.aurindo.barros@cern.ch> Date: Thu, 7 Dec 2023 14:01:07 +0100 Subject: [PATCH 5/5] Have ssoProxyImage flag available --- chart/drupalsite-operator/templates/manager-deploy.yaml | 1 + chart/drupalsite-operator/values.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/chart/drupalsite-operator/templates/manager-deploy.yaml b/chart/drupalsite-operator/templates/manager-deploy.yaml index b953eed5..887b9396 100644 --- a/chart/drupalsite-operator/templates/manager-deploy.yaml +++ b/chart/drupalsite-operator/templates/manager-deploy.yaml @@ -26,6 +26,7 @@ spec: - --sitebuilder-image={{.Values.drupalsiteOperator.sitebuilderImage}} - --php-fpm-exporter-image={{.Values.drupalsiteOperator.phpFpmExporterImage}} - --webdav-image={{.Values.drupalsiteOperator.webdavImage}} + - --ssoproxy-image={{.Values.drupalsiteOperator.ssoProxyImage}} - --zap-stacktrace-level={{.Values.drupalsiteOperator.logStacktraceLevel}} - --zap-log-level={{.Values.drupalsiteOperator.logLevel}} - --parallel-thread-count={{.Values.drupalsiteOperator.parallelThreadCount}} diff --git a/chart/drupalsite-operator/values.yaml b/chart/drupalsite-operator/values.yaml index d08e4c74..1ca025a9 100644 --- a/chart/drupalsite-operator/values.yaml +++ b/chart/drupalsite-operator/values.yaml @@ -19,6 +19,7 @@ drupalsiteOperator: sitebuilderImage: "gitlab-registry.cern.ch/drupal/paas/cern-drupal-distribution/site-builder" phpFpmExporterImage: "gitlab-registry.cern.ch/drupal/paas/php-fpm-prometheus-exporter:RELEASE.2021.06.02T09-41-38Z" webdavImage: "gitlab-registry.cern.ch/drupal/paas/sabredav/webdav:RELEASE-2021.10.12T17-55-06Z" + ssoProxyImage: "quay.io/oauth2-proxy/oauth2-proxy:latest" # Zap Level to configure the verbosity of logging. Can be one of 'debug', 'info', 'error', or any integer value > 0 which corresponds to custom debug levels of increasing verbosity logLevel: "3" # Zap Level at and above which stacktraces are captured (one of 'info', 'error') -- GitLab