Change oauth2 to mount rather than load secrets

config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml

@@ -1320,7 +1320,7 @@ spec:
                     type: array
                   gunicorn:
                     description: |-
-                      Settings for the gunicorn server.
+                      Settings for the Gunicorn server.
                       More info: https://docs.gunicorn.org/en/latest/settings.html
                     type: object
                     x-kubernetes-preserve-unknown-fields: true
@@ -1355,34 +1355,59 @@ spec:
                     x-kubernetes-map-type: atomic
                   oauthConfigurations:
                     description: |-
-                      OauthConfigurations allows the user to reference one or more Secrets
-                      containing OAUTH2 configuration settings for pgAdmin.
-                      Each Secret shall contain a single data key called oauth-config
-                      whose value is a JSON object containing the OAUTH2 configuration settings.
+                      Secrets for the `OAUTH2_CONFIG` setting. If there are `OAUTH2_CONFIG` values
+                      in the settings field, they will be combined with the values loaded here.
                       More info: https://www.pgadmin.org/docs/pgadmin4/latest/oauth2.html
                     items:
-                      description: |-
-                        LocalObjectReference contains enough information to let you locate the
-                        referenced object inside the same namespace.
                       properties:
                         name:
-                          default: ""
-                          description: |-
-                            Name of the referent.
-                            This field is effectively required, but due to backwards compatibility is
-                            allowed to be empty. Instances of this type with an empty value here are
-                            almost certainly wrong.
-                            More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                          description: The OAUTH2_NAME of this configuration.
+                          maxLength: 20
+                          minLength: 1
+                          pattern: ^[A-Za-z0-9]+$
                           type: string
+                        secret:
+                          description: A Secret containing the settings of one OAuth2
+                            provider as a JSON object.
+                          properties:
+                            key:
+                              description: Name of the data field within the Secret.
+                              maxLength: 253
+                              minLength: 1
+                              pattern: ^[-._a-zA-Z0-9]+$
+                              type: string
+                              x-kubernetes-validations:
+                              - message: cannot be "." or start with ".."
+                                rule: self != "." && !self.startsWith("..")
+                            name:
+                              description: Name of the Secret.
+                              maxLength: 253
+                              minLength: 1
+                              pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
+                              type: string
+                          required:
+                          - key
+                          - name
+                          type: object
+                          x-kubernetes-map-type: atomic
+                      required:
+                      - name
+                      - secret
                       type: object
                       x-kubernetes-map-type: atomic
+                    maxItems: 10
+                    minItems: 1
                     type: array
+                    x-kubernetes-list-map-keys:
+                    - name
+                    x-kubernetes-list-type: map
                   settings:
                     description: |-
                       Settings for the pgAdmin server process. Keys should be uppercase and
                       values must be constants.
                       More info: https://www.pgadmin.org/docs/pgadmin4/latest/config_py.html
                     type: object
+                    x-kubernetes-map-type: granular
                     x-kubernetes-preserve-unknown-fields: true
                 type: object
               dataVolumeClaimSpec:

internal/controller/standalone_pgadmin/configmap.go

@@ -15,7 +15,6 @@ import (
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/pkg/errors"
 
@@ -29,25 +28,6 @@ import (
 // +kubebuilder:rbac:groups="",resources="configmaps",verbs={get}
 // +kubebuilder:rbac:groups="",resources="configmaps",verbs={create,delete,patch}
 
-// reconcileOauthConfigSecrets reconciles the Oauth Configuration Secrets for pgAdmin.
-func (r *PGAdminReconciler) reconcileOauthConfigSecrets(
-	ctx context.Context, pgadmin *v1beta1.PGAdmin,
-) ([]corev1.Secret, error) {
-
-	var secrets []corev1.Secret
-	for _, secretRef := range pgadmin.Spec.Config.OauthConfigurations {
-		var secret corev1.Secret
-		secretKey := types.NamespacedName{Name: secretRef.Name, Namespace: pgadmin.Namespace}
-
-		if err := r.Get(ctx, secretKey, &secret); err != nil {
-			return nil, fmt.Errorf("failed to get Secret %s: %w", secretRef.Name, err)
-		}
-		secrets = append(secrets, secret)
-	}
-
-	return secrets, nil
-}
-
 // reconcilePGAdminConfigMap writes the ConfigMap for pgAdmin.
 func (r *PGAdminReconciler) reconcilePGAdminConfigMap(
 	ctx context.Context, pgadmin *v1beta1.PGAdmin,

internal/controller/standalone_pgadmin/controller.go

@@ -73,12 +73,6 @@ func (r *PGAdminReconciler) SetupWithManager(mgr ctrl.Manager) error {
 				return runtime.Requests(r.findPGAdminsForSecret(ctx, client.ObjectKeyFromObject(secret))...)
 			}),
 		).
-		Watches(
-			&corev1.Secret{},
-			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, secret client.Object) []ctrl.Request {
-				return runtime.Requests(r.findPGAdminsForOauthSecret(ctx, client.ObjectKeyFromObject(secret))...)
-			}),
-		).
 		Complete(r)
 }
 
@@ -122,19 +116,15 @@ func (r *PGAdminReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	pgAdmin.Default()
 
 	var (
-		configmap    *corev1.ConfigMap
-		dataVolume   *corev1.PersistentVolumeClaim
-		clusters     map[string][]*v1beta1.PostgresCluster
-		oauthSecrets []corev1.Secret
-		_            *corev1.Service
+		configmap  *corev1.ConfigMap
+		dataVolume *corev1.PersistentVolumeClaim
+		clusters   map[string][]*v1beta1.PostgresCluster
+		_          *corev1.Service
 	)
 
 	if err == nil {
 		clusters, err = r.getClustersForPGAdmin(ctx, pgAdmin)
 	}
-	if err == nil {
-		oauthSecrets, err = r.reconcileOauthConfigSecrets(ctx, pgAdmin)
-	}
 	if err == nil {
 		configmap, err = r.reconcilePGAdminConfigMap(ctx, pgAdmin, clusters)
 	}
@@ -145,7 +135,7 @@ func (r *PGAdminReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		err = r.reconcilePGAdminService(ctx, pgAdmin)
 	}
 	if err == nil {
-		err = r.reconcilePGAdminStatefulSet(ctx, pgAdmin, configmap, dataVolume, oauthSecrets)
+		err = r.reconcilePGAdminStatefulSet(ctx, pgAdmin, configmap, dataVolume)
 	}
 	if err == nil {
 		err = r.reconcilePGAdminUsers(ctx, pgAdmin)

internal/controller/standalone_pgadmin/pod.go

@@ -28,7 +28,8 @@ const (
 	configDatabaseURIPath  = "~postgres-operator/config-database-uri"
 	ldapFilePath           = "~postgres-operator/ldap-bind-password"
 	gunicornConfigFilePath = "~postgres-operator/" + gunicornConfigKey
-	oauthConfigDir         = "~postgres-operator/oauth-config/"
+	oauthConfigDir         = "~postgres-operator/oauth-config"
+	oauthAbsolutePath      = configMountPath + "/" + oauthConfigDir
 
 	// scriptMountPath is where to mount a temporary directory that is only
 	// writable during Pod initialization.
@@ -49,15 +50,14 @@ func pod(
 	inConfigMap *corev1.ConfigMap,
 	outPod *corev1.PodSpec,
 	pgAdminVolume *corev1.PersistentVolumeClaim,
-	oauthSecrets []corev1.Secret,
 ) {
 	// create the projected volume of config maps for use in
 	// 1. dynamic server discovery
 	// 2. adding the config variables during pgAdmin startup
 	configVolume := corev1.Volume{Name: "pgadmin-config"}
 	configVolume.VolumeSource = corev1.VolumeSource{
 		Projected: &corev1.ProjectedVolumeSource{
-			Sources: podConfigFiles(inConfigMap, *inPGAdmin, oauthSecrets),
+			Sources: podConfigFiles(inConfigMap, *inPGAdmin),
 		},
 	}
 
@@ -187,8 +187,7 @@ func pod(
 
 // podConfigFiles returns projections of pgAdmin's configuration files to
 // include in the configuration volume.
-func podConfigFiles(configmap *corev1.ConfigMap, pgadmin v1beta1.PGAdmin,
-	oauthSecrets []corev1.Secret) []corev1.VolumeProjection {
+func podConfigFiles(configmap *corev1.ConfigMap, pgadmin v1beta1.PGAdmin) []corev1.VolumeProjection {
 
 	config := append(append([]corev1.VolumeProjection{}, pgadmin.Spec.Config.Files...),
 		[]corev1.VolumeProjection{
@@ -215,22 +214,15 @@ func podConfigFiles(configmap *corev1.ConfigMap, pgadmin v1beta1.PGAdmin,
 			},
 		}...)
 
-	if pgadmin.Spec.Config.OauthConfigurations != nil {
-		for _, secret := range oauthSecrets {
-			config = append(config, corev1.VolumeProjection{
-				Secret: &corev1.SecretProjection{
-					LocalObjectReference: corev1.LocalObjectReference{
-						Name: secret.Name,
-					},
-					Items: []corev1.KeyToPath{
-						{
-							Key:  "oauth-config",
-							Path: fmt.Sprintf("%s%s.json", oauthConfigDir, secret.Name),
-						},
-					},
-				},
-			})
-		}
+	for i, oauth := range pgadmin.Spec.Config.OAuthConfigurations {
+		// Safely encode the OAUTH2_NAME in the file name. Prepend the index so
+		// the files can be loaded in the order they are defined in the spec.
+		mountPath := fmt.Sprintf(
+			"%s/%02d-%s.json", oauthConfigDir, i, shell.CleanFileName(oauth.Name),
+		)
+		config = append(config, corev1.VolumeProjection{
+			Secret: initialize.Pointer(oauth.Secret.AsProjection(mountPath)),
+		})
 	}
 
 	if pgadmin.Spec.Config.ConfigDatabaseURI != nil {
@@ -332,15 +324,17 @@ loadServerCommand
 	// descriptor and uses the timeout of the builtin `read` to wait. That same
 	// descriptor gets closed and reopened to use the builtin `[ -nt` to check mtimes.
 	// - https://unix.stackexchange.com/a/407383
-	// In order to get gunicorn to reload the logging config
-	// we need to send a KILL rather than a HUP signal.
+	//
+	// Gunicorn needs a SIGTERM rather than SIGHUP to reload its logging config.
+	// This also causes pgAdmin to restart when its configuration changes.
 	// - https://github.com/benoitc/gunicorn/issues/3353
+	//
 	// Right now the config file is on the same configMap as the cluster file
 	// so if the mtime changes for any of those files, it will change for all.
 	var reloadScript = `
 exec {fd}<> <(:||:)
 while read -r -t 5 -u "${fd}" ||:; do
-    if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand && kill -KILL $(head -1 ${PGADMIN4_PIDFILE?});
+    if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand && kill -TERM $(head -1 ${PGADMIN4_PIDFILE?});
     then
         exec {fd}>&- && exec {fd}<> <(:||:)
         stat --format='Loaded shared servers dated %y' "${cluster_file}"
@@ -394,28 +388,33 @@ import glob, json, re, os
 DEFAULT_BINARY_PATHS = {'pg': sorted([''] + glob.glob('/usr/pgsql-*/bin')).pop()}
 with open('` + configMountPath + `/` + configFilePath + `') as _f:
     _conf, _data = re.compile(r'[A-Z_0-9]+'), json.load(_f)
-    folder_path = '` + configMountPath + `/` + oauthConfigDir + `'
-    if os.path.isdir(folder_path):
-        for filename in os.listdir(folder_path):
-            with open(os.path.join(folder_path, filename), "r", encoding="utf-8") as f:
-                try:
-                    oath = json.load(f)
-                    if oath.get("OAUTH2_NAME") not in [
-                        o.get("OAUTH2_NAME") for o in _data.get("OAUTH2_CONFIG")]:
-                        _data.get("OAUTH2_CONFIG").append(oath)
-                    for o in _data.get("OAUTH2_CONFIG"):
-                        if o.get("OAUTH2_NAME") == oath.get("OAUTH2_NAME"):
-                            o.update(oath)
-                except Exception as e:
-                    print(f"An unexpected error occurred: {e}")
     if type(_data) is dict:
         globals().update({k: v for k, v in _data.items() if _conf.fullmatch(k)})
+if 'OAUTH2_CONFIG' in globals() and type(OAUTH2_CONFIG) is list:
+    OAUTH2_CONFIG = [_conf for _conf in OAUTH2_CONFIG if type(_conf) is dict and 'OAUTH2_NAME' in _conf]
+for _f in reversed(glob.glob('` + oauthAbsolutePath + `/[0-9][0-9]-*.json')):
+    if 'OAUTH2_CONFIG' not in globals() or type(OAUTH2_CONFIG) is not list:
+        OAUTH2_CONFIG = []
+    try:
+        with open(_f) as _f:
+            _data, _name = json.load(_f), os.path.basename(_f.name)[3:-5]
+            _data, _next = { 'OAUTH2_NAME': _name } | _data, []
+            for _conf in OAUTH2_CONFIG:
+                if _data['OAUTH2_NAME'] == _conf.get('OAUTH2_NAME'):
+                    _data = _conf | _data
+                else:
+                    _next.append(_conf)
+            OAUTH2_CONFIG = [_data] + _next
+            del _next
+    except:
+        pass
 if os.path.isfile('` + ldapPasswordAbsolutePath + `'):
     with open('` + ldapPasswordAbsolutePath + `') as _f:
         LDAP_BIND_PASSWORD = _f.read()
 if os.path.isfile('` + configDatabaseURIPathAbsolutePath + `'):
     with open('` + configDatabaseURIPathAbsolutePath + `') as _f:
         CONFIG_DATABASE_URI = _f.read()
+del _conf, _data, _f
 `
 
 		// Gunicorn reads from the `/etc/pgadmin/gunicorn_config.py` file during startup