Updates to support changes starting in pgAdmin 9.3

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

@@ -2720,6 +2720,10 @@ spec:
                 description: MajorVersion represents the major version of the running
                   pgAdmin.
                 type: integer
+              minorVersion:
+                description: MinorVersion represents the minor version of the running
+                  pgAdmin.
+                type: string
               observedGeneration:
                 description: observedGeneration represents the .metadata.generation
                   on which the status was based.

internal/controller/standalone_pgadmin/users.go

@@ -80,54 +80,60 @@ func (r *PGAdminReconciler) reconcilePGAdminUsers(ctx context.Context, pgadmin *
 		return nil
 	}
 
-	// If the pgAdmin version is not in the status or the image SHA has changed, get
-	// the pgAdmin version and store it in the status.
-	var pgadminVersion int
-	if pgadmin.Status.MajorVersion == 0 || pgadmin.Status.ImageSHA != pgAdminImageSha {
-		pgadminVersion, err = r.reconcilePGAdminMajorVersion(ctx, podExecutor)
+	// If the pgAdmin major or minor version is not in the status or the image
+	// SHA has changed, get the pgAdmin version and store it in the status.
+	var pgadminMajorVersion int
+	if pgadmin.Status.MajorVersion == 0 || pgadmin.Status.MinorVersion == "" ||
+		pgadmin.Status.ImageSHA != pgAdminImageSha {
+
+		// exec into the pgAdmin pod and retrieve the pgAdmin minor version
+		script := fmt.Sprintf(`
+PGADMIN_DIR=%s
+cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_VERSION)"
+`, pgAdminDir)
+
+		var stdin, stdout, stderr bytes.Buffer
+
+		if err := podExecutor(ctx, &stdin, &stdout, &stderr,
+			[]string{"bash", "-ceu", "--", script}...); err != nil {
+			return err
+		}
+
+		pgadminMinorVersion := strings.TrimSpace(stdout.String())
+
+		// ensure minor version is valid before storing in status
+		parsedMinorVersion, err := strconv.ParseFloat(pgadminMinorVersion, 64)
 		if err != nil {
 			return err
 		}
-		pgadmin.Status.MajorVersion = pgadminVersion
+
+		// Note: "When converting a floating-point number to an integer, the
+		// fraction is discarded (truncation towards zero)."
+		// - https://go.dev/ref/spec#Conversions
+		pgadminMajorVersion = int(parsedMinorVersion)
+
+		pgadmin.Status.MinorVersion = pgadminMinorVersion
+		pgadmin.Status.MajorVersion = pgadminMajorVersion
 		pgadmin.Status.ImageSHA = pgAdminImageSha
 	} else {
-		pgadminVersion = pgadmin.Status.MajorVersion
+		pgadminMajorVersion = pgadmin.Status.MajorVersion
 	}
 
 	// If the pgAdmin version is not v8 or higher, return early as user management is
 	// only supported for pgAdmin v8 and higher.
-	if pgadminVersion < 8 {
+	if pgadminMajorVersion < 8 {
 		// If pgAdmin version is less than v8 and user management is being attempted,
 		// log a message clarifying that it is only supported for pgAdmin v8 and higher.
 		if len(pgadmin.Spec.Users) > 0 {
 			log.Info("User management is only supported for pgAdmin v8 and higher.",
-				"pgadminVersion", pgadminVersion)
+				"pgadminVersion", pgadminMajorVersion)
 		}
 		return err
 	}
 
 	return r.writePGAdminUsers(ctx, pgadmin, podExecutor)
 }
 
-// reconcilePGAdminMajorVersion execs into the pgAdmin pod and retrieves the pgAdmin major version
-func (r *PGAdminReconciler) reconcilePGAdminMajorVersion(ctx context.Context, exec Executor) (int, error) {
-	script := fmt.Sprintf(`
-PGADMIN_DIR=%s
-cd $PGADMIN_DIR && python3 -c "import config; print(config.APP_RELEASE)"
-`, pgAdminDir)
-
-	var stdin, stdout, stderr bytes.Buffer
-
-	err := exec(ctx, &stdin, &stdout, &stderr,
-		[]string{"bash", "-ceu", "--", script}...)
-
-	if err != nil {
-		return 0, err
-	}
-
-	return strconv.Atoi(strings.TrimSpace(stdout.String()))
-}
-
 // writePGAdminUsers takes the users in the pgAdmin spec and writes (adds or updates) their data
 // to both pgAdmin and the users.json file that is stored in the pgAdmin secret. If a user is
 // removed from the spec, its data is removed from users.json, but it is not deleted from pgAdmin.
@@ -171,10 +177,25 @@ cd $PGADMIN_DIR
 	for _, user := range existingUsersArr {
 		existingUsersMap[user.Username] = user
 	}
+
+	var olderThan9_3 bool
+	versionFloat, err := strconv.ParseFloat(pgadmin.Status.MinorVersion, 64)
+	if err != nil {
+		return err
+	}
+	if versionFloat < 9.3 {
+		olderThan9_3 = true
+	}
+
 	intentUsers := []pgAdminUserForJson{}
 	for _, user := range pgadmin.Spec.Users {
 		var stdin, stdout, stderr bytes.Buffer
-		typeFlag := "--nonadmin"
+		// starting in pgAdmin 9.3, custom roles are supported and a new flag is used
+		// - https://github.com/pgadmin-org/pgadmin4/pull/8631
+		typeFlag := "--role User"
+		if olderThan9_3 {
+			typeFlag = "--nonadmin"
+		}
 		isAdmin := false
 		if user.Role == "Administrator" {
 			typeFlag = "--admin"
@@ -230,8 +251,13 @@ cd $PGADMIN_DIR
 					log.Error(err, "PodExec failed: ")
 					intentUsers = append(intentUsers, existingUser)
 					continue
+
+				} else if strings.Contains(strings.TrimSpace(stderr.String()), "UserWarning: pkg_resources is deprecated as an API") {
+					// Started seeing this error with pgAdmin 9.7 when using Python 3.11.
+					// Issue appears to resolve with Python 3.13.
+					log.Info(stderr.String())
 				} else if strings.TrimSpace(stderr.String()) != "" {
-					log.Error(errors.New(stderr.String()), fmt.Sprintf("pgAdmin setup.py error for %s: ",
+					log.Error(errors.New(stderr.String()), fmt.Sprintf("pgAdmin setup.py update-user error for %s: ",
 						intentUser.Username))
 					intentUsers = append(intentUsers, existingUser)
 					continue
@@ -264,8 +290,12 @@ cd $PGADMIN_DIR
 				log.Error(err, "PodExec failed: ")
 				continue
 			}
-			if strings.TrimSpace(stderr.String()) != "" {
-				log.Error(errors.New(stderr.String()), fmt.Sprintf("pgAdmin setup.py error for %s: ",
+			if strings.Contains(strings.TrimSpace(stderr.String()), "UserWarning: pkg_resources is deprecated as an API") {
+				// Started seeing this error with pgAdmin 9.7 when using Python 3.11.
+				// Issue appears to resolve with Python 3.13.
+				log.Info(stderr.String())
+			} else if strings.TrimSpace(stderr.String()) != "" {
+				log.Error(errors.New(stderr.String()), fmt.Sprintf("pgAdmin setup.py add-user error for %s: ",
 					intentUser.Username))
 				continue
 			}

internal/controller/standalone_pgadmin/users_test.go

@@ -110,15 +110,16 @@ func TestReconcilePGAdminUsers(t *testing.T) {
 			assert.Equal(t, namespace, pgadmin.Namespace)
 			assert.Equal(t, container, naming.ContainerPGAdmin)
 
-			// Simulate a v7 version of pgAdmin by setting stdout to "7" for
-			// podexec call in reconcilePGAdminMajorVersion
-			_, _ = stdout.Write([]byte("7"))
+			// Simulate a v7.1 version of pgAdmin by setting stdout to "7.1"
+			// for podexec call in reconcilePGAdminVersion
+			_, _ = stdout.Write([]byte("7.1"))
 			return nil
 		}
 
 		assert.NilError(t, r.reconcilePGAdminUsers(ctx, pgadmin))
 		assert.Equal(t, calls, 1, "PodExec should be called once")
 		assert.Equal(t, pgadmin.Status.MajorVersion, 7)
+		assert.Equal(t, pgadmin.Status.MinorVersion, "7.1")
 		assert.Equal(t, pgadmin.Status.ImageSHA, "fakeSHA")
 	})
 
@@ -145,78 +146,89 @@ func TestReconcilePGAdminUsers(t *testing.T) {
 		) error {
 			calls++
 
-			// Simulate a v7 version of pgAdmin by setting stdout to "7" for
-			// podexec call in reconcilePGAdminMajorVersion
-			_, _ = stdout.Write([]byte("7"))
+			// Simulate a v7.1 version of pgAdmin by setting stdout to "7.1"
+			// for podexec call in reconcilePGAdminVersion
+			_, _ = stdout.Write([]byte("7.1"))
 			return nil
 		}
 
 		assert.NilError(t, r.reconcilePGAdminUsers(ctx, pgadmin))
 		assert.Equal(t, calls, 1, "PodExec should be called once")
 		assert.Equal(t, pgadmin.Status.MajorVersion, 7)
+		assert.Equal(t, pgadmin.Status.MinorVersion, "7.1")
 		assert.Equal(t, pgadmin.Status.ImageSHA, "newFakeSHA")
 	})
-}
 
-func TestReconcilePGAdminMajorVersion(t *testing.T) {
-	ctx := context.Background()
-	pod := corev1.Pod{}
-	pod.Namespace = "test-namespace"
-	pod.Name = "pgadmin-123-0"
-	reconciler := &PGAdminReconciler{}
+	t.Run("PodHealthyBadVersion", func(t *testing.T) {
+		pgadmin := pgadmin.DeepCopy()
+		pod := pod.DeepCopy()
 
-	podExecutor := func(
-		ctx context.Context, stdin io.Reader, stdout, stderr io.Writer, command ...string,
-	) error {
-		return reconciler.PodExec(ctx, pod.Namespace, pod.Name, "pgadmin", stdin, stdout, stderr, command...)
-	}
+		pod.DeletionTimestamp = nil
+		pod.Status.ContainerStatuses =
+			[]corev1.ContainerStatus{{Name: naming.ContainerPGAdmin}}
+		pod.Status.ContainerStatuses[0].State.Running =
+			new(corev1.ContainerStateRunning)
+		pod.Status.ContainerStatuses[0].ImageID = "fakeSHA"
 
-	t.Run("SuccessfulRetrieval", func(t *testing.T) {
-		reconciler.PodExec = func(
+		r := new(PGAdminReconciler)
+		r.Reader = fake.NewClientBuilder().WithObjects(pod).Build()
+
+		calls := 0
+		r.PodExec = func(
 			ctx context.Context, namespace, pod, container string,
 			stdin io.Reader, stdout, stderr io.Writer, command ...string,
 		) error {
+			calls++
+
 			assert.Equal(t, pod, "pgadmin-123-0")
-			assert.Equal(t, namespace, "test-namespace")
+			assert.Equal(t, namespace, pgadmin.Namespace)
 			assert.Equal(t, container, naming.ContainerPGAdmin)
 
-			// Simulate a v7 version of pgAdmin by setting stdout to "7" for
-			// podexec call in reconcilePGAdminMajorVersion
-			_, _ = stdout.Write([]byte("7"))
+			// set expected version to something completely wrong
+			_, _ = stdout.Write([]byte("woot"))
 			return nil
 		}
 
-		version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
-		assert.NilError(t, err)
-		assert.Equal(t, version, 7)
+		assert.ErrorContains(t, r.reconcilePGAdminUsers(ctx, pgadmin), "strconv.ParseFloat: parsing \"woot\": invalid syntax")
+		assert.Equal(t, calls, 1, "PodExec should be called once")
+		assert.Equal(t, pgadmin.Status.MajorVersion, 0)
+		assert.Equal(t, pgadmin.Status.MinorVersion, "")
+		assert.Equal(t, pgadmin.Status.ImageSHA, "")
 	})
 
-	t.Run("FailedRetrieval", func(t *testing.T) {
-		reconciler.PodExec = func(
-			ctx context.Context, namespace, pod, container string,
-			stdin io.Reader, stdout, stderr io.Writer, command ...string,
-		) error {
-			// Simulate the python call giving bad data (not a version int)
-			_, _ = stdout.Write([]byte("asdfjkl;"))
-			return nil
-		}
+	t.Run("PodExecError", func(t *testing.T) {
+		pgadmin := pgadmin.DeepCopy()
+		pod := pod.DeepCopy()
 
-		version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
-		assert.Check(t, err != nil)
-		assert.Equal(t, version, 0)
-	})
+		pod.DeletionTimestamp = nil
+		pod.Status.ContainerStatuses =
+			[]corev1.ContainerStatus{{Name: naming.ContainerPGAdmin}}
+		pod.Status.ContainerStatuses[0].State.Running =
+			new(corev1.ContainerStateRunning)
+		pod.Status.ContainerStatuses[0].ImageID = "fakeSHA"
 
-	t.Run("PodExecError", func(t *testing.T) {
-		reconciler.PodExec = func(
+		r := new(PGAdminReconciler)
+		r.Reader = fake.NewClientBuilder().WithObjects(pod).Build()
+
+		calls := 0
+		r.PodExec = func(
 			ctx context.Context, namespace, pod, container string,
 			stdin io.Reader, stdout, stderr io.Writer, command ...string,
 		) error {
+			calls++
+
+			assert.Equal(t, pod, "pgadmin-123-0")
+			assert.Equal(t, namespace, pgadmin.Namespace)
+			assert.Equal(t, container, naming.ContainerPGAdmin)
+
 			return errors.New("PodExecError")
 		}
 
-		version, err := reconciler.reconcilePGAdminMajorVersion(ctx, podExecutor)
-		assert.Check(t, err != nil)
-		assert.Equal(t, version, 0)
+		assert.Error(t, r.reconcilePGAdminUsers(ctx, pgadmin), "PodExecError")
+		assert.Equal(t, calls, 1, "PodExec should be called once")
+		assert.Equal(t, pgadmin.Status.MajorVersion, 0)
+		assert.Equal(t, pgadmin.Status.MinorVersion, "")
+		assert.Equal(t, pgadmin.Status.ImageSHA, "")
 	})
 }
 
@@ -244,6 +256,14 @@ func TestWritePGAdminUsers(t *testing.T) {
 	}`)
 	assert.NilError(t, cc.Create(ctx, pgadmin))
 
+	// fake the status so that the correct commands will be used when creating
+	// users.
+	pgadmin.Status = v1beta1.PGAdminStatus{
+		ImageSHA:     "fakesha",
+		MajorVersion: 9,
+		MinorVersion: "9.3",
+	}
+
 	userPasswordSecret1 := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "user-password-secret1",

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

@@ -231,6 +231,10 @@ type PGAdminStatus struct {
 	// +optional
 	MajorVersion int `json:"majorVersion,omitempty"`
 
+	// MinorVersion represents the minor version of the running pgAdmin.
+	// +optional
+	MinorVersion string `json:"minorVersion,omitempty"`
+
 	// observedGeneration represents the .metadata.generation on which the status was based.
 	// +optional
 	// +kubebuilder:validation:Minimum=0

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

@@ -6,12 +6,14 @@ commands:
     pod_name=$(kubectl get pod -n "${NAMESPACE}" -l postgres-operator.crunchydata.com/pgadmin=pgadmin -o name)
     secret_name=$(kubectl get secret -n "${NAMESPACE}" -l postgres-operator.crunchydata.com/pgadmin=pgadmin -o name)
 
+    # /usr/local/lib/python3.11/site-packages/pgadmin4 allows for various Python versions to be referenced in testing
     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")
 
-    bob_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="bob@example.com") | .role')
-    dave_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="dave@example.com") | .role')
+    bob_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="bob@example.com") | .role')
+    dave_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="dave@example.com") | .role')
 
-    [ $bob_role = 1 ] && [ $dave_role = 2 ] || exit 1
+    # Prior to pgAdmin 9.3, the role values were integers rather than strings. This supports both variations.
+    ( [ $bob_role = 1 ] && [ $dave_role = 2 ] ) || ( [ $bob_role = "Administrator" ] && [ $dave_role = "User" ] ) || exit 1
 
     users_in_secret=$(kubectl get "${secret_name}" -n "${NAMESPACE}" -o 'go-template={{index .data "users.json" }}' | base64 -d)
 

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

@@ -6,13 +6,15 @@ commands:
     pod_name=$(kubectl get pod -n "${NAMESPACE}" -l postgres-operator.crunchydata.com/pgadmin=pgadmin -o name)
     secret_name=$(kubectl get secret -n "${NAMESPACE}" -l postgres-operator.crunchydata.com/pgadmin=pgadmin -o name)
 
+    # /usr/local/lib/python3.11/site-packages/pgadmin4 allows for various Python versions to be referenced in testing
     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")
 
-    bob_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="bob@example.com") | .role')
-    dave_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="dave@example.com") | .role')
-    jimi_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="jimi@example.com") | .role')
+    bob_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="bob@example.com") | .role')
+    dave_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="dave@example.com") | .role')
+    jimi_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="jimi@example.com") | .role')
 
-    [ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] || exit 1
+    # Prior to pgAdmin 9.3, the role values were integers rather than strings. This supports both variations.
+    ( [ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] ) || ( [ $bob_role = "Administrator" ] && [ $dave_role = "Administrator" ] && [ $jimi_role = "User" ] ) || exit 1
 
     users_in_secret=$(kubectl get "${secret_name}" -n "${NAMESPACE}" -o 'go-template={{index .data "users.json" }}' | base64 -d)
 

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

@@ -6,13 +6,15 @@ commands:
     pod_name=$(kubectl get pod -n "${NAMESPACE}" -l postgres-operator.crunchydata.com/pgadmin=pgadmin -o name)
     secret_name=$(kubectl get secret -n "${NAMESPACE}" -l postgres-operator.crunchydata.com/pgadmin=pgadmin -o name)
 
+    # /usr/local/lib/python3.11/site-packages/pgadmin4 allows for various Python versions to be referenced in testing
     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")
 
-    bob_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="bob@example.com") | .role')
-    dave_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="dave@example.com") | .role')
-    jimi_role=$(printf '%s\n' $users_in_pgadmin | jq '.[] | select(.username=="jimi@example.com") | .role')
+    bob_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="bob@example.com") | .role')
+    dave_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="dave@example.com") | .role')
+    jimi_role=$(printf '%s\n' $users_in_pgadmin | jq -r '.[] | select(.username=="jimi@example.com") | .role')
 
-    [ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] || exit 1
+    # Prior to pgAdmin 9.3, the role values were integers rather than strings. This supports both variations.
+    ( [ $bob_role = 1 ] && [ $dave_role = 1 ] && [ $jimi_role = 2 ] ) || ( [ $bob_role = "Administrator" ] && [ $dave_role = "Administrator" ] && [ $jimi_role = "User" ] ) || exit 1
 
     users_in_secret=$(kubectl get "${secret_name}" -n "${NAMESPACE}" -o 'go-template={{index .data "users.json" }}' | base64 -d)