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] 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