Skip to content

Commit e484f5d

Browse files
authored
Merge pull request #9 from cbandy/pgadmin-oauth-secrets
Change oauth2 to mount rather than load secrets
2 parents 29196d4 + bf5e48f commit e484f5d

File tree

12 files changed

+231
-224
lines changed

12 files changed

+231
-224
lines changed

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

+40-15
Original file line numberDiff line numberDiff line change
@@ -1320,7 +1320,7 @@ spec:
13201320
type: array
13211321
gunicorn:
13221322
description: |-
1323-
Settings for the gunicorn server.
1323+
Settings for the Gunicorn server.
13241324
More info: https://docs.gunicorn.org/en/latest/settings.html
13251325
type: object
13261326
x-kubernetes-preserve-unknown-fields: true
@@ -1355,34 +1355,59 @@ spec:
13551355
x-kubernetes-map-type: atomic
13561356
oauthConfigurations:
13571357
description: |-
1358-
OauthConfigurations allows the user to reference one or more Secrets
1359-
containing OAUTH2 configuration settings for pgAdmin.
1360-
Each Secret shall contain a single data key called oauth-config
1361-
whose value is a JSON object containing the OAUTH2 configuration settings.
1358+
Secrets for the `OAUTH2_CONFIG` setting. If there are `OAUTH2_CONFIG` values
1359+
in the settings field, they will be combined with the values loaded here.
13621360
More info: https://www.pgadmin.org/docs/pgadmin4/latest/oauth2.html
13631361
items:
1364-
description: |-
1365-
LocalObjectReference contains enough information to let you locate the
1366-
referenced object inside the same namespace.
13671362
properties:
13681363
name:
1369-
default: ""
1370-
description: |-
1371-
Name of the referent.
1372-
This field is effectively required, but due to backwards compatibility is
1373-
allowed to be empty. Instances of this type with an empty value here are
1374-
almost certainly wrong.
1375-
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
1364+
description: The OAUTH2_NAME of this configuration.
1365+
maxLength: 20
1366+
minLength: 1
1367+
pattern: ^[A-Za-z0-9]+$
13761368
type: string
1369+
secret:
1370+
description: A Secret containing the settings of one OAuth2
1371+
provider as a JSON object.
1372+
properties:
1373+
key:
1374+
description: Name of the data field within the Secret.
1375+
maxLength: 253
1376+
minLength: 1
1377+
pattern: ^[-._a-zA-Z0-9]+$
1378+
type: string
1379+
x-kubernetes-validations:
1380+
- message: cannot be "." or start with ".."
1381+
rule: self != "." && !self.startsWith("..")
1382+
name:
1383+
description: Name of the Secret.
1384+
maxLength: 253
1385+
minLength: 1
1386+
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?([.][a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
1387+
type: string
1388+
required:
1389+
- key
1390+
- name
1391+
type: object
1392+
x-kubernetes-map-type: atomic
1393+
required:
1394+
- name
1395+
- secret
13771396
type: object
13781397
x-kubernetes-map-type: atomic
1398+
maxItems: 10
1399+
minItems: 1
13791400
type: array
1401+
x-kubernetes-list-map-keys:
1402+
- name
1403+
x-kubernetes-list-type: map
13801404
settings:
13811405
description: |-
13821406
Settings for the pgAdmin server process. Keys should be uppercase and
13831407
values must be constants.
13841408
More info: https://www.pgadmin.org/docs/pgadmin4/latest/config_py.html
13851409
type: object
1410+
x-kubernetes-map-type: granular
13861411
x-kubernetes-preserve-unknown-fields: true
13871412
type: object
13881413
dataVolumeClaimSpec:

internal/controller/standalone_pgadmin/configmap.go

-20
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"strings"
1616

1717
corev1 "k8s.io/api/core/v1"
18-
"k8s.io/apimachinery/pkg/types"
1918

2019
"github.com/pkg/errors"
2120

@@ -29,25 +28,6 @@ import (
2928
// +kubebuilder:rbac:groups="",resources="configmaps",verbs={get}
3029
// +kubebuilder:rbac:groups="",resources="configmaps",verbs={create,delete,patch}
3130

32-
// reconcileOauthConfigSecrets reconciles the Oauth Configuration Secrets for pgAdmin.
33-
func (r *PGAdminReconciler) reconcileOauthConfigSecrets(
34-
ctx context.Context, pgadmin *v1beta1.PGAdmin,
35-
) ([]corev1.Secret, error) {
36-
37-
var secrets []corev1.Secret
38-
for _, secretRef := range pgadmin.Spec.Config.OauthConfigurations {
39-
var secret corev1.Secret
40-
secretKey := types.NamespacedName{Name: secretRef.Name, Namespace: pgadmin.Namespace}
41-
42-
if err := r.Get(ctx, secretKey, &secret); err != nil {
43-
return nil, fmt.Errorf("failed to get Secret %s: %w", secretRef.Name, err)
44-
}
45-
secrets = append(secrets, secret)
46-
}
47-
48-
return secrets, nil
49-
}
50-
5131
// reconcilePGAdminConfigMap writes the ConfigMap for pgAdmin.
5232
func (r *PGAdminReconciler) reconcilePGAdminConfigMap(
5333
ctx context.Context, pgadmin *v1beta1.PGAdmin,

internal/controller/standalone_pgadmin/controller.go

+5-15
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,6 @@ func (r *PGAdminReconciler) SetupWithManager(mgr ctrl.Manager) error {
7373
return runtime.Requests(r.findPGAdminsForSecret(ctx, client.ObjectKeyFromObject(secret))...)
7474
}),
7575
).
76-
Watches(
77-
&corev1.Secret{},
78-
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, secret client.Object) []ctrl.Request {
79-
return runtime.Requests(r.findPGAdminsForOauthSecret(ctx, client.ObjectKeyFromObject(secret))...)
80-
}),
81-
).
8276
Complete(r)
8377
}
8478

@@ -122,19 +116,15 @@ func (r *PGAdminReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
122116
pgAdmin.Default()
123117

124118
var (
125-
configmap *corev1.ConfigMap
126-
dataVolume *corev1.PersistentVolumeClaim
127-
clusters map[string][]*v1beta1.PostgresCluster
128-
oauthSecrets []corev1.Secret
129-
_ *corev1.Service
119+
configmap *corev1.ConfigMap
120+
dataVolume *corev1.PersistentVolumeClaim
121+
clusters map[string][]*v1beta1.PostgresCluster
122+
_ *corev1.Service
130123
)
131124

132125
if err == nil {
133126
clusters, err = r.getClustersForPGAdmin(ctx, pgAdmin)
134127
}
135-
if err == nil {
136-
oauthSecrets, err = r.reconcileOauthConfigSecrets(ctx, pgAdmin)
137-
}
138128
if err == nil {
139129
configmap, err = r.reconcilePGAdminConfigMap(ctx, pgAdmin, clusters)
140130
}
@@ -145,7 +135,7 @@ func (r *PGAdminReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
145135
err = r.reconcilePGAdminService(ctx, pgAdmin)
146136
}
147137
if err == nil {
148-
err = r.reconcilePGAdminStatefulSet(ctx, pgAdmin, configmap, dataVolume, oauthSecrets)
138+
err = r.reconcilePGAdminStatefulSet(ctx, pgAdmin, configmap, dataVolume)
149139
}
150140
if err == nil {
151141
err = r.reconcilePGAdminUsers(ctx, pgAdmin)

internal/controller/standalone_pgadmin/pod.go

+37-38
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ const (
2828
configDatabaseURIPath = "~postgres-operator/config-database-uri"
2929
ldapFilePath = "~postgres-operator/ldap-bind-password"
3030
gunicornConfigFilePath = "~postgres-operator/" + gunicornConfigKey
31-
oauthConfigDir = "~postgres-operator/oauth-config/"
31+
oauthConfigDir = "~postgres-operator/oauth-config"
32+
oauthAbsolutePath = configMountPath + "/" + oauthConfigDir
3233

3334
// scriptMountPath is where to mount a temporary directory that is only
3435
// writable during Pod initialization.
@@ -49,15 +50,14 @@ func pod(
4950
inConfigMap *corev1.ConfigMap,
5051
outPod *corev1.PodSpec,
5152
pgAdminVolume *corev1.PersistentVolumeClaim,
52-
oauthSecrets []corev1.Secret,
5353
) {
5454
// create the projected volume of config maps for use in
5555
// 1. dynamic server discovery
5656
// 2. adding the config variables during pgAdmin startup
5757
configVolume := corev1.Volume{Name: "pgadmin-config"}
5858
configVolume.VolumeSource = corev1.VolumeSource{
5959
Projected: &corev1.ProjectedVolumeSource{
60-
Sources: podConfigFiles(inConfigMap, *inPGAdmin, oauthSecrets),
60+
Sources: podConfigFiles(inConfigMap, *inPGAdmin),
6161
},
6262
}
6363

@@ -187,8 +187,7 @@ func pod(
187187

188188
// podConfigFiles returns projections of pgAdmin's configuration files to
189189
// include in the configuration volume.
190-
func podConfigFiles(configmap *corev1.ConfigMap, pgadmin v1beta1.PGAdmin,
191-
oauthSecrets []corev1.Secret) []corev1.VolumeProjection {
190+
func podConfigFiles(configmap *corev1.ConfigMap, pgadmin v1beta1.PGAdmin) []corev1.VolumeProjection {
192191

193192
config := append(append([]corev1.VolumeProjection{}, pgadmin.Spec.Config.Files...),
194193
[]corev1.VolumeProjection{
@@ -215,22 +214,15 @@ func podConfigFiles(configmap *corev1.ConfigMap, pgadmin v1beta1.PGAdmin,
215214
},
216215
}...)
217216

218-
if pgadmin.Spec.Config.OauthConfigurations != nil {
219-
for _, secret := range oauthSecrets {
220-
config = append(config, corev1.VolumeProjection{
221-
Secret: &corev1.SecretProjection{
222-
LocalObjectReference: corev1.LocalObjectReference{
223-
Name: secret.Name,
224-
},
225-
Items: []corev1.KeyToPath{
226-
{
227-
Key: "oauth-config",
228-
Path: fmt.Sprintf("%s%s.json", oauthConfigDir, secret.Name),
229-
},
230-
},
231-
},
232-
})
233-
}
217+
for i, oauth := range pgadmin.Spec.Config.OAuthConfigurations {
218+
// Safely encode the OAUTH2_NAME in the file name. Prepend the index so
219+
// the files can be loaded in the order they are defined in the spec.
220+
mountPath := fmt.Sprintf(
221+
"%s/%02d-%s.json", oauthConfigDir, i, shell.CleanFileName(oauth.Name),
222+
)
223+
config = append(config, corev1.VolumeProjection{
224+
Secret: initialize.Pointer(oauth.Secret.AsProjection(mountPath)),
225+
})
234226
}
235227

236228
if pgadmin.Spec.Config.ConfigDatabaseURI != nil {
@@ -332,15 +324,17 @@ loadServerCommand
332324
// descriptor and uses the timeout of the builtin `read` to wait. That same
333325
// descriptor gets closed and reopened to use the builtin `[ -nt` to check mtimes.
334326
// - https://unix.stackexchange.com/a/407383
335-
// In order to get gunicorn to reload the logging config
336-
// we need to send a KILL rather than a HUP signal.
327+
//
328+
// Gunicorn needs a SIGTERM rather than SIGHUP to reload its logging config.
329+
// This also causes pgAdmin to restart when its configuration changes.
337330
// - https://github.com/benoitc/gunicorn/issues/3353
331+
//
338332
// Right now the config file is on the same configMap as the cluster file
339333
// so if the mtime changes for any of those files, it will change for all.
340334
var reloadScript = `
341335
exec {fd}<> <(:||:)
342336
while read -r -t 5 -u "${fd}" ||:; do
343-
if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand && kill -KILL $(head -1 ${PGADMIN4_PIDFILE?});
337+
if [[ "${cluster_file}" -nt "/proc/self/fd/${fd}" ]] && loadServerCommand && kill -TERM $(head -1 ${PGADMIN4_PIDFILE?});
344338
then
345339
exec {fd}>&- && exec {fd}<> <(:||:)
346340
stat --format='Loaded shared servers dated %y' "${cluster_file}"
@@ -394,28 +388,33 @@ import glob, json, re, os
394388
DEFAULT_BINARY_PATHS = {'pg': sorted([''] + glob.glob('/usr/pgsql-*/bin')).pop()}
395389
with open('` + configMountPath + `/` + configFilePath + `') as _f:
396390
_conf, _data = re.compile(r'[A-Z_0-9]+'), json.load(_f)
397-
folder_path = '` + configMountPath + `/` + oauthConfigDir + `'
398-
if os.path.isdir(folder_path):
399-
for filename in os.listdir(folder_path):
400-
with open(os.path.join(folder_path, filename), "r", encoding="utf-8") as f:
401-
try:
402-
oath = json.load(f)
403-
if oath.get("OAUTH2_NAME") not in [
404-
o.get("OAUTH2_NAME") for o in _data.get("OAUTH2_CONFIG")]:
405-
_data.get("OAUTH2_CONFIG").append(oath)
406-
for o in _data.get("OAUTH2_CONFIG"):
407-
if o.get("OAUTH2_NAME") == oath.get("OAUTH2_NAME"):
408-
o.update(oath)
409-
except Exception as e:
410-
print(f"An unexpected error occurred: {e}")
411391
if type(_data) is dict:
412392
globals().update({k: v for k, v in _data.items() if _conf.fullmatch(k)})
393+
if 'OAUTH2_CONFIG' in globals() and type(OAUTH2_CONFIG) is list:
394+
OAUTH2_CONFIG = [_conf for _conf in OAUTH2_CONFIG if type(_conf) is dict and 'OAUTH2_NAME' in _conf]
395+
for _f in reversed(glob.glob('` + oauthAbsolutePath + `/[0-9][0-9]-*.json')):
396+
if 'OAUTH2_CONFIG' not in globals() or type(OAUTH2_CONFIG) is not list:
397+
OAUTH2_CONFIG = []
398+
try:
399+
with open(_f) as _f:
400+
_data, _name = json.load(_f), os.path.basename(_f.name)[3:-5]
401+
_data, _next = { 'OAUTH2_NAME': _name } | _data, []
402+
for _conf in OAUTH2_CONFIG:
403+
if _data['OAUTH2_NAME'] == _conf.get('OAUTH2_NAME'):
404+
_data = _conf | _data
405+
else:
406+
_next.append(_conf)
407+
OAUTH2_CONFIG = [_data] + _next
408+
del _next
409+
except:
410+
pass
413411
if os.path.isfile('` + ldapPasswordAbsolutePath + `'):
414412
with open('` + ldapPasswordAbsolutePath + `') as _f:
415413
LDAP_BIND_PASSWORD = _f.read()
416414
if os.path.isfile('` + configDatabaseURIPathAbsolutePath + `'):
417415
with open('` + configDatabaseURIPathAbsolutePath + `') as _f:
418416
CONFIG_DATABASE_URI = _f.read()
417+
del _conf, _data, _f
419418
`
420419

421420
// Gunicorn reads from the `/etc/pgadmin/gunicorn_config.py` file during startup

0 commit comments

Comments
 (0)