Skip to content

Commit bd83e2b

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 af97e79 commit bd83e2b

File tree

8 files changed

+131
-59
lines changed

8 files changed

+131
-59
lines changed

‎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
@@ -79,40 +79,55 @@ func (r *PGAdminReconciler) reconcilePGAdminUsers(ctx context.Context, pgadmin *
7979
return nil
8080
}
8181

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

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

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

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

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

123138
if err != nil {
124-
return 0, err
139+
return "", err
125140
}
126141

127-
return strconv.Atoi(strings.TrimSpace(stdout.String()))
142+
return strings.TrimSpace(stdout.String()), nil
128143
}
129144

130145
// writePGAdminUsers takes the users in the pgAdmin spec and writes (adds or updates) their data
@@ -170,10 +185,23 @@ cd $PGADMIN_DIR
170185
for _, user := range existingUsersArr {
171186
existingUsersMap[user.Username] = user
172187
}
188+
189+
var olderThan9_3 bool
190+
versionFloat, err := strconv.ParseFloat(pgadmin.Status.MinorVersion, 32)
191+
if err != nil {
192+
return err
193+
}
194+
if versionFloat < 9.3 {
195+
olderThan9_3 = true
196+
}
197+
173198
intentUsers := []pgAdminUserForJson{}
174199
for _, user := range pgadmin.Spec.Users {
175200
var stdin, stdout, stderr bytes.Buffer
176-
typeFlag := "--nonadmin"
201+
typeFlag := "--role User"
202+
if olderThan9_3 {
203+
typeFlag = "--nonadmin"
204+
}
177205
isAdmin := false
178206
if user.Role == "Administrator" {
179207
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)