Skip to content

Commit 5d9ee07

Browse files
tjmoore4tjmoore4
committed
Updates to support changes starting in pgAdmin 9.3
Changes in the flags used by pgAdmin's setup.py for user managment start in pgAdmin 9.3. This update addresses those changes while allowing PGO to remain backward compatible. Issue: PGO-2686
1 parent b3d57df commit 5d9ee07

8 files changed

Lines changed: 131 additions & 59 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2720,6 +2720,10 @@ spec:
27202720
description: MajorVersion represents the major version of the running
27212721
pgAdmin.
27222722
type: integer
2723+
minorVersion:
2724+
description: MinorVersion represents the minor version of the running
2725+
pgAdmin.
2726+
type: string
27232727
observedGeneration:
27242728
description: observedGeneration represents the .metadata.generation
27252729
on which the status was based.

‎internal/controller/standalone_pgadmin/users.go‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -80,40 +80,55 @@ func (r *PGAdminReconciler) reconcilePGAdminUsers(ctx context.Context, pgadmin *
8080
return nil
8181
}
8282

83-
// If the pgAdmin version is not in the status or the image SHA has changed, get
84-
// the pgAdmin version and store it in the status.
85-
var pgadminVersion int
86-
if pgadmin.Status.MajorVersion == 0 || pgadmin.Status.ImageSHA != pgAdminImageSha {
87-
pgadminVersion, err = r.reconcilePGAdminMajorVersion(ctx, podExecutor)
83+
// If the pgAdmin major or minor version is not in the status or the image
84+
// SHA has changed, get the pgAdmin version and store it in the status.
85+
var pgadminMajorVersion int
86+
if pgadmin.Status.MajorVersion == 0 || pgadmin.Status.MinorVersion == "" ||
87+
pgadmin.Status.ImageSHA != pgAdminImageSha {
88+
89+
pgadminMinorVersion, err := r.reconcilePGAdminVersion(ctx, podExecutor)
8890
if err != nil {
8991
return err
9092
}
91-
pgadmin.Status.MajorVersion = pgadminVersion
93+
94+
// ensure minor version is valid before storing in status
95+
parsedMinorVersion, err := strconv.ParseFloat(pgadminMinorVersion, 64)
96+
if err != nil {
97+
return err
98+
}
99+
100+
// Note: "When converting a floating-point number to an integer, the
101+
// fraction is discarded (truncation towards zero)."
102+
// - https://go.dev/ref/spec#Conversions
103+
pgadminMajorVersion = int(parsedMinorVersion)
104+
105+
pgadmin.Status.MinorVersion = pgadminMinorVersion
106+
pgadmin.Status.MajorVersion = pgadminMajorVersion
92107
pgadmin.Status.ImageSHA = pgAdminImageSha
93108
} else {
94-
pgadminVersion = pgadmin.Status.MajorVersion
109+
pgadminMajorVersion = pgadmin.Status.MajorVersion
95110
}
96111

97112
// If the pgAdmin version is not v8 or higher, return early as user management is
98113
// only supported for pgAdmin v8 and higher.
99-
if pgadminVersion < 8 {
114+
if pgadminMajorVersion < 8 {
100115
// If pgAdmin version is less than v8 and user management is being attempted,
101116
// log a message clarifying that it is only supported for pgAdmin v8 and higher.
102117
if len(pgadmin.Spec.Users) > 0 {
103118
log.Info("User management is only supported for pgAdmin v8 and higher.",
104-
"pgadminVersion", pgadminVersion)
119+
"pgadminVersion", pgadminMajorVersion)
105120
}
106121
return err
107122
}
108123

109124
return r.writePGAdminUsers(ctx, pgadmin, podExecutor)
110125
}
111126

112-
// reconcilePGAdminMajorVersion execs into the pgAdmin pod and retrieves the pgAdmin major version
113-
func (r *PGAdminReconciler) reconcilePGAdminMajorVersion(ctx context.Context, exec Executor) (int, error) {
127+
// reconcilePGAdminVersion execs into the pgAdmin pod and retrieves the pgAdmin minor version
128+
func (r *PGAdminReconciler) reconcilePGAdminVersion(ctx context.Context, exec Executor) (string, error) {
114129
script := fmt.Sprintf(`
115130
PGADMIN_DIR=%s
116-
cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_RELEASE)"
131+
cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_VERSION)"
117132
`, pgAdminDir)
118133

119134
var stdin, stdout, stderr bytes.Buffer
@@ -122,10 +137,10 @@ cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_RELEASE)"
122137
[]string{"bash", "-ceu", "--", script}...)
123138

124139
if err != nil {
125-
return 0, err
140+
return "", err
126141
}
127142

128-
return strconv.Atoi(strings.TrimSpace(stdout.String()))
143+
return strings.TrimSpace(stdout.String()), nil
129144
}
130145

131146
// writePGAdminUsers takes the users in the pgAdmin spec and writes (adds or updates) their data
@@ -171,10 +186,23 @@ cd $PGADMIN_DIR
171186
for _, user := range existingUsersArr {
172187
existingUsersMap[user.Username] = user
173188
}
189+
190+
var olderThan9_3 bool
191+
versionFloat, err := strconv.ParseFloat(pgadmin.Status.MinorVersion, 32)
192+
if err != nil {
193+
return err
194+
}
195+
if versionFloat < 9.3 {
196+
olderThan9_3 = true
197+
}
198+
174199
intentUsers := []pgAdminUserForJson{}
175200
for _, user := range pgadmin.Spec.Users {
176201
var stdin, stdout, stderr bytes.Buffer
177-
typeFlag := "--nonadmin"
202+
typeFlag := "--role User"
203+
if olderThan9_3 {
204+
typeFlag = "--nonadmin"
205+
}
178206
isAdmin := false
179207
if user.Role == "Administrator" {
180208
typeFlag = "--admin"

‎internal/controller/standalone_pgadmin/users_test.go‎

Lines changed: 61 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,16 @@ func TestReconcilePGAdminUsers(t *testing.T) {
110110
assert.Equal(t, namespace, pgadmin.Namespace)
111111
assert.Equal(t, container, naming.ContainerPGAdmin)
112112

113-
// Simulate a v7 version of pgAdmin by setting stdout to "7" for
114-
// podexec call in reconcilePGAdminMajorVersion
115-
_, _ = stdout.Write([]byte("7"))
113+
// Simulate a v7.1 version of pgAdmin by setting stdout to "7.1"
114+
// for podexec call in reconcilePGAdminVersion
115+
_, _ = stdout.Write([]byte("7.1"))
116116
return nil
117117
}
118118

119119
assert.NilError(t, r.reconcilePGAdminUsers(ctx, pgadmin))
120120
assert.Equal(t, calls, 1, "PodExec should be called once")
121121
assert.Equal(t, pgadmin.Status.MajorVersion, 7)
122+
assert.Equal(t, pgadmin.Status.MinorVersion, "7.1")
122123
assert.Equal(t, pgadmin.Status.ImageSHA, "fakeSHA")
123124
})
124125

@@ -145,20 +146,58 @@ func TestReconcilePGAdminUsers(t *testing.T) {
145146
) error {
146147
calls++
147148

148-
// Simulate a v7 version of pgAdmin by setting stdout to "7" for
149-
// podexec call in reconcilePGAdminMajorVersion
150-
_, _ = stdout.Write([]byte("7"))
149+
// Simulate a v7.1 version of pgAdmin by setting stdout to "7.1"
150+
// for podexec call in reconcilePGAdminVersion
151+
_, _ = stdout.Write([]byte("7.1"))
151152
return nil
152153
}
153154

154155
assert.NilError(t, r.reconcilePGAdminUsers(ctx, pgadmin))
155156
assert.Equal(t, calls, 1, "PodExec should be called once")
156157
assert.Equal(t, pgadmin.Status.MajorVersion, 7)
158+
assert.Equal(t, pgadmin.Status.MinorVersion, "7.1")
157159
assert.Equal(t, pgadmin.Status.ImageSHA, "newFakeSHA")
158160
})
161+
162+
t.Run("PodHealthyBadVersion", func(t *testing.T) {
163+
pgadmin := pgadmin.DeepCopy()
164+
pod := pod.DeepCopy()
165+
166+
pod.DeletionTimestamp = nil
167+
pod.Status.ContainerStatuses =
168+
[]corev1.ContainerStatus{{Name: naming.ContainerPGAdmin}}
169+
pod.Status.ContainerStatuses[0].State.Running =
170+
new(corev1.ContainerStateRunning)
171+
pod.Status.ContainerStatuses[0].ImageID = "fakeSHA"
172+
173+
r := new(PGAdminReconciler)
174+
r.Reader = fake.NewClientBuilder().WithObjects(pod).Build()
175+
176+
calls := 0
177+
r.PodExec = func(
178+
ctx context.Context, namespace, pod, container string,
179+
stdin io.Reader, stdout, stderr io.Writer, command ...string,
180+
) error {
181+
calls++
182+
183+
assert.Equal(t, pod, "pgadmin-123-0")
184+
assert.Equal(t, namespace, pgadmin.Namespace)
185+
assert.Equal(t, container, naming.ContainerPGAdmin)
186+
187+
// set expected version to something completely wrong
188+
_, _ = stdout.Write([]byte("woot"))
189+
return nil
190+
}
191+
192+
assert.ErrorContains(t, r.reconcilePGAdminUsers(ctx, pgadmin), "strconv.ParseFloat: parsing \"woot\": invalid syntax")
193+
assert.Equal(t, calls, 1, "PodExec should be called once")
194+
assert.Equal(t, pgadmin.Status.MajorVersion, 0)
195+
assert.Equal(t, pgadmin.Status.MinorVersion, "")
196+
assert.Equal(t, pgadmin.Status.ImageSHA, "")
197+
})
159198
}
160199

161-
func TestReconcilePGAdminMajorVersion(t *testing.T) {
200+
func TestReconcilePGAdminVersion(t *testing.T) {
162201
ctx := context.Background()
163202
pod := corev1.Pod{}
164203
pod.Namespace = "test-namespace"
@@ -180,30 +219,15 @@ func TestReconcilePGAdminMajorVersion(t *testing.T) {
180219
assert.Equal(t, namespace, "test-namespace")
181220
assert.Equal(t, container, naming.ContainerPGAdmin)
182221

183-
// Simulate a v7 version of pgAdmin by setting stdout to "7" for
184-
// podexec call in reconcilePGAdminMajorVersion
185-
_, _ = stdout.Write([]byte("7"))
222+
// Simulate a v9.3 version of pgAdmin by setting stdout to "9.3"
223+
// for podexec call in reconcilePGAdminVersion
224+
_, _ = stdout.Write([]byte("9.3"))
186225
return nil
187226
}
188227

189-
version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
228+
version, err := reconciler.reconcilePGAdminVersion(ctx, podExecutor)
190229
assert.NilError(t, err)
191-
assert.Equal(t, version, 7)
192-
})
193-
194-
t.Run("FailedRetrieval", func(t *testing.T) {
195-
reconciler.PodExec = func(
196-
ctx context.Context, namespace, pod, container string,
197-
stdin io.Reader, stdout, stderr io.Writer, command ...string,
198-
) error {
199-
// Simulate the python call giving bad data (not a version int)
200-
_, _ = stdout.Write([]byte("asdfjkl;"))
201-
return nil
202-
}
203-
204-
version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
205-
assert.Check(t, err != nil)
206-
assert.Equal(t, version, 0)
230+
assert.Equal(t, version, "9.3")
207231
})
208232

209233
t.Run("PodExecError", func(t *testing.T) {
@@ -214,9 +238,9 @@ func TestReconcilePGAdminMajorVersion(t *testing.T) {
214238
return errors.New("PodExecError")
215239
}
216240

217-
version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
241+
version, err := reconciler.reconcilePGAdminVersion(ctx, podExecutor)
218242
assert.Check(t, err != nil)
219-
assert.Equal(t, version, 0)
243+
assert.Equal(t, version, "")
220244
})
221245
}
222246

@@ -244,6 +268,14 @@ func TestWritePGAdminUsers(t *testing.T) {
244268
}`)
245269
assert.NilError(t, cc.Create(ctx, pgadmin))
246270

271+
// fake the status so that the correct commands will be used when creating
272+
// users.
273+
pgadmin.Status = v1beta1.PGAdminStatus{
274+
ImageSHA: "fakesha",
275+
MajorVersion: 9,
276+
MinorVersion: "9.3",
277+
}
278+
247279
userPasswordSecret1 := &corev1.Secret{
248280
ObjectMeta: metav1.ObjectMeta{
249281
Name: "user-password-secret1",

‎pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,10 @@ type PGAdminStatus struct {
231231
// +optional
232232
MajorVersion int `json:"majorVersion,omitempty"`
233233

234+
// MinorVersion represents the minor version of the running pgAdmin.
235+
// +optional
236+
MinorVersion string `json:"minorVersion,omitempty"`
237+
234238
// observedGeneration represents the .metadata.generation on which the status was based.
235239
// +optional
236240
// +kubebuilder:validation:Minimum=0

‎testing/kuttl/e2e/standalone-pgadmin-user-management/01-assert.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ commands:
88
99
users_in_pgadmin=$(kubectl exec -n "${NAMESPACE}" "${pod_name}" -- bash -c "python3 /usr/local/lib/python3.11/site-packages/pgadmin4/setup.py get-users --json")
1010
11-
bob_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="bob@example.com") | .role')
12-
dave_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="dave@example.com") | .role')
11+
bob_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="bob@example.com") | .role')
12+
dave_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="dave@example.com") | .role')
1313
14-
[ $bob_role = 1 ] && [ $dave_role = 2 ] || exit 1
14+
# Prior to pgAdmin 9.3, the role values were integers rather than strings. This supports both variations.
15+
( [ $bob_role = 1 ] && [ $dave_role = 2 ] ) || ( [ $bob_role = "Administrator" ] && [ $dave_role = "User" ] ) || exit 1
1516
1617
users_in_secret=$(kubectl get "${secret_name}" -n "${NAMESPACE}" -o 'go-template={{index .data "users.json" }}' | base64 -d)
1718

‎testing/kuttl/e2e/standalone-pgadmin-user-management/03-assert.yaml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ commands:
88
99
users_in_pgadmin=$(kubectl exec -n "${NAMESPACE}" "${pod_name}" -- bash -c "python3 /usr/local/lib/python3.11/site-packages/pgadmin4/setup.py get-users --json")
1010
11-
bob_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="bob@example.com") | .role')
12-
dave_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="dave@example.com") | .role')
13-
jimi_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="jimi@example.com") | .role')
11+
bob_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="bob@example.com") | .role')
12+
dave_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="dave@example.com") | .role')
13+
jimi_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="jimi@example.com") | .role')
1414
15-
[ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] || exit 1
15+
# Prior to pgAdmin 9.3, the role values were integers rather than strings. This supports both variations.
16+
( [ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] ) || ( [ $bob_role = "Administrator" ] && [ $dave_role = "Administrator" ] && [ $jimi_role = "User" ] ) || exit 1
1617
1718
users_in_secret=$(kubectl get "${secret_name}" -n "${NAMESPACE}" -o 'go-template={{index .data "users.json" }}' | base64 -d)
1819

‎testing/kuttl/e2e/standalone-pgadmin-user-management/05-assert.yaml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ commands:
88
99
users_in_pgadmin=$(kubectl exec -n "${NAMESPACE}" "${pod_name}" -- bash -c "python3 /usr/local/lib/python3.11/site-packages/pgadmin4/setup.py get-users --json")
1010
11-
bob_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="bob@example.com") | .role')
12-
dave_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="dave@example.com") | .role')
13-
jimi_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="jimi@example.com") | .role')
11+
bob_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="bob@example.com") | .role')
12+
dave_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="dave@example.com") | .role')
13+
jimi_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="jimi@example.com") | .role')
1414
15-
[ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] || exit 1
15+
# Prior to pgAdmin 9.3, the role values were integers rather than strings. This supports both variations.
16+
( [ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] ) || ( [ $bob_role = "Administrator" ] && [ $dave_role = "Administrator" ] && [ $jimi_role = "User" ] ) || exit 1
1617
1718
users_in_secret=$(kubectl get "${secret_name}" -n "${NAMESPACE}" -o 'go-template={{index .data "users.json" }}' | base64 -d)
1819

‎testing/kuttl/e2e/standalone-pgadmin-user-management/07-assert.yaml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ commands:
88
99
users_in_pgadmin=$(kubectl exec -n "${NAMESPACE}" "${pod_name}" -- bash -c "python3 /usr/local/lib/python3.11/site-packages/pgadmin4/setup.py get-users --json")
1010
11-
bob_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="bob@example.com") | .role')
12-
dave_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="dave@example.com") | .role')
13-
jimi_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="jimi@example.com") | .role')
11+
bob_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="bob@example.com") | .role')
12+
dave_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="dave@example.com") | .role')
13+
jimi_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="jimi@example.com") | .role')
1414
15-
[ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] || exit 1
15+
# Prior to pgAdmin 9.3, the role values were integers rather than strings. This supports both variations.
16+
( [ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] ) || ( [ $bob_role = "Administrator" ] && [ $dave_role = "Administrator" ] && [ $jimi_role = "User" ] ) || exit 1
1617
1718
users_in_secret=$(kubectl get "${secret_name}" -n "${NAMESPACE}" -o 'go-template={{index .data "users.json" }}' | base64 -d)
1819

0 commit comments

Comments
 (0)