From 33e497b858dc3053f45cc1facfd382cb80f703f7 Mon Sep 17 00:00:00 2001 From: Benjamin Blattberg Date: Thu, 12 Oct 2023 10:29:52 -0500 Subject: [PATCH] PR feedback and adding comments (#3741) --- .../bases/postgres-operator.crunchydata.com_pgadmins.yaml | 3 ++- docs/content/references/crd.md | 2 +- internal/controller/standalone_pgadmin/configmap.go | 8 ++------ internal/controller/standalone_pgadmin/postgrescluster.go | 4 ++-- .../v1beta1/standalone_pgadmin_types.go | 5 +++-- .../v1beta1/zz_generated.deepcopy.go | 6 +----- 6 files changed, 11 insertions(+), 17 deletions(-) diff --git a/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml b/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml index fba577fdb0..4c1de77005 100644 --- a/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml +++ b/config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml @@ -39,7 +39,8 @@ spec: properties: adminUsername: description: AdminUsername is the username to set during pgAdmin startup. - pgAdmin requires this to have a valid email form. + pgAdmin requires this to have a valid email form in order to log + in. Once set, this cannot be changed. type: string affinity: description: 'Scheduling constraints of the PGAdmin pod. More info: diff --git a/docs/content/references/crd.md b/docs/content/references/crd.md index b702bcc7d8..ef0f1fedd1 100644 --- a/docs/content/references/crd.md +++ b/docs/content/references/crd.md @@ -96,7 +96,7 @@ PGAdminSpec defines the desired state of PGAdmin adminUsername string - AdminUsername is the username to set during pgAdmin startup. pgAdmin requires this to have a valid email form. + AdminUsername is the username to set during pgAdmin startup. pgAdmin requires this to have a valid email form in order to log in. Once set, this cannot be changed. false affinity diff --git a/internal/controller/standalone_pgadmin/configmap.go b/internal/controller/standalone_pgadmin/configmap.go index e7127a882a..c4960d54b5 100644 --- a/internal/controller/standalone_pgadmin/configmap.go +++ b/internal/controller/standalone_pgadmin/configmap.go @@ -147,15 +147,12 @@ func generateClusterConfig( sort.Strings(keys) clusterServers := map[int]any{} - // Because we allow multiple ServerGroups to be defined, we use `currentOffset` to keep - // track of the last number added to the `Servers` group - var currentOffset = 0 for _, serverGroupName := range keys { sort.Slice(clusters[serverGroupName].Items, func(i, j int) bool { return clusters[serverGroupName].Items[i].Name < clusters[serverGroupName].Items[j].Name }) - for i, cluster := range clusters[serverGroupName].Items { + for _, cluster := range clusters[serverGroupName].Items { object := map[string]any{ "Name": cluster.Name, "Group": serverGroupName, @@ -167,9 +164,8 @@ func generateClusterConfig( "SSLMode": "prefer", "Shared": true, } - clusterServers[i+1+currentOffset] = object + clusterServers[len(clusterServers)+1] = object } - currentOffset = len(clusters[serverGroupName].Items) + currentOffset } servers := map[string]any{ "Servers": clusterServers, diff --git a/internal/controller/standalone_pgadmin/postgrescluster.go b/internal/controller/standalone_pgadmin/postgrescluster.go index 26a6e8dae7..84b47e1125 100644 --- a/internal/controller/standalone_pgadmin/postgrescluster.go +++ b/internal/controller/standalone_pgadmin/postgrescluster.go @@ -44,7 +44,7 @@ func (r *PGAdminReconciler) findPGAdminsForPostgresCluster( }) == nil { for i := range pgadmins.Items { for _, serverGroup := range pgadmins.Items[i].Spec.ServerGroups { - if selector, err := naming.AsSelector(*serverGroup.PostgresClusterSelector); err == nil { + if selector, err := naming.AsSelector(serverGroup.PostgresClusterSelector); err == nil { if selector.Matches(labels.Set(cluster.GetLabels())) { matching = append(matching, &pgadmins.Items[i]) } @@ -67,7 +67,7 @@ func (r *PGAdminReconciler) getClustersForPGAdmin( var selector labels.Selector for _, serverGroup := range pgAdmin.Spec.ServerGroups { - if selector, err = naming.AsSelector(*serverGroup.PostgresClusterSelector); err == nil { + if selector, err = naming.AsSelector(serverGroup.PostgresClusterSelector); err == nil { var filteredList v1beta1.PostgresClusterList err = r.List(ctx, &filteredList, client.InNamespace(pgAdmin.Namespace), diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go index 3f625df958..30991955bd 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/standalone_pgadmin_types.go @@ -99,7 +99,8 @@ type PGAdminSpec struct { Tolerations []corev1.Toleration `json:"tolerations,omitempty"` // AdminUsername is the username to set during pgAdmin startup. - // pgAdmin requires this to have a valid email form. + // pgAdmin requires this to have a valid email form in order to log in. + // Once set, this cannot be changed. // +optional AdminUsername string `json:"adminUsername"` @@ -126,7 +127,7 @@ type ServerGroup struct { // PostgresClusterSelector selects clusters to dynamically add to pgAdmin by matching labels. // An empty selector like `{}` will select ALL clusters in the namespace. - PostgresClusterSelector *metav1.LabelSelector `json:"postgresClusterSelector"` + PostgresClusterSelector metav1.LabelSelector `json:"postgresClusterSelector"` } // PGAdminStatus defines the observed state of PGAdmin diff --git a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go index b1446f1724..049a1a857c 100644 --- a/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/postgres-operator.crunchydata.com/v1beta1/zz_generated.deepcopy.go @@ -1910,11 +1910,7 @@ func (in SchemalessObject) DeepCopyInto(out *SchemalessObject) { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServerGroup) DeepCopyInto(out *ServerGroup) { *out = *in - if in.PostgresClusterSelector != nil { - in, out := &in.PostgresClusterSelector, &out.PostgresClusterSelector - *out = new(metav1.LabelSelector) - (*in).DeepCopyInto(*out) - } + in.PostgresClusterSelector.DeepCopyInto(&out.PostgresClusterSelector) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServerGroup.