Update labeling interface to be consistent with others

This moves the labeling interface to match that of similar ones, e.g. "annotations." - `pgo create cluster`, `pgo label`, and `pgo delete label` all now have a single `--label` flag. `--label` can be specified multiple times. - The API call itself takes a mapping of key/value pairs. - The API endpoint parameter for `pgo label` and `pgo delete label` is now called `Labels` Issue: [ch10202]

Author: Jonathan S. Katz

cmd/pgo/cmd/cluster.go

@@ -267,7 +267,7 @@ func createCluster(args []string, ns string, createClusterCmd *cobra.Command) {
 r.PasswordReplication = PasswordReplication
 r.Password = Password
 r.SecretFrom = SecretFrom
-r.UserLabels = UserLabels
+r.UserLabels = getLabels(UserLabels)
 r.Policies = PoliciesFlag
 r.CCPImageTag = CCPImageTag
 r.CCPImage = CCPImage

cmd/pgo/cmd/common.go

@@ -20,7 +20,9 @@ import (
 "fmt"
 "os"
 "reflect"
+"strings"
 
+operatorutil "github.com/crunchydata/postgres-operator/internal/util"
 crv1 "github.com/crunchydata/postgres-operator/pkg/apis/crunchydata.com/v1"
 )
 
@@ -85,6 +87,33 @@ func getHeaderLength(value interface{}, fieldName string) int {
 return len(field.String())
 }
 
+// getLabels determines if the provided labels are in the correct format,
+// and if so, will return them in the appropriate map.
+//
+// If not, we will abort.
+func getLabels(labels []string) map[string]string {
+clusterLabels := map[string]string{}
+
+for _, label := range labels {
+parts := strings.Split(label, "=")
+
+if len(parts) != 2 {
+fmt.Printf("invalid label: found %q, should be \"key=value\"\n", label)
+os.Exit(1)
+}
+
+clusterLabels[parts[0]] = parts[1]
+}
+
+// perform a validation that can save us a round trip to the server
+if err := operatorutil.ValidateLabels(clusterLabels); err != nil {
+fmt.Println(err.Error())
+os.Exit(1)
+}
+
+return clusterLabels
+}
+
 // getMaxLength returns the maxLength of the strings of a particular value in
 // the struct. Increases the max length by 1 to include a buffer
 func getMaxLength(results []interface{}, title, fieldName string) int {

cmd/pgo/cmd/create.go

@@ -39,7 +39,7 @@ var (
 Password string
 SecretFrom string
 PoliciesFlag, PolicyFile string
-UserLabels string
+UserLabels []string
 Tablespaces []string
 ServiceType string
 ServiceTypePgBouncer string
@@ -392,7 +392,8 @@ func init() {
 createClusterCmd.Flags().StringVarP(&CustomConfig, "custom-config", "", "", "The name of a configMap that holds custom PostgreSQL configuration files used to override defaults.")
 createClusterCmd.Flags().StringVarP(&Database, "database", "d", "", "If specified, sets the name of the initial database that is created for the user. Defaults to the value set in the PostgreSQL Operator configuration, or if that is not present, the name of the cluster")
 createClusterCmd.Flags().BoolVarP(&DisableAutofailFlag, "disable-autofail", "", false, "Disables autofail capabitilies in the cluster following cluster initialization.")
-createClusterCmd.Flags().StringVarP(&UserLabels, "labels", "l", "", "The labels to apply to this cluster.")
+createClusterCmd.Flags().StringSliceVar(&UserLabels, "label", []string{}, "Add labels to apply to the PostgreSQL cluster, "+
+"e.g. \"key=value\", \"prefix/key=value\". Can specify flag multiple times.")
 createClusterCmd.Flags().StringVar(&MemoryRequest, "memory", "", "Set the amount of RAM to request, e.g. "+
 "1GiB. Overrides the default server value.")
 createClusterCmd.Flags().StringVar(&MemoryLimit, "memory-limit", "", "Set the amount of RAM to limit, e.g. "+

cmd/pgo/cmd/delete.go

@@ -170,8 +170,9 @@ func init() {
 deleteCmd.AddCommand(deleteLabelCmd)
 // pgo delete label --label
 // the label to be deleted
-deleteLabelCmd.Flags().StringVar(&LabelCmdLabel, "label", "",
-"The label to delete for any selected or specified clusters.")
+deleteLabelCmd.Flags().StringSliceVar(&UserLabels, "label", []string{}, "Delete "+
+"labels to apply to the PostgreSQL cluster, "+"e.g. \"key=value\", \"prefix/key=value\". "+
+"Can specify flag multiple times.")
 // "pgo delete label --selector"
 // "pgo delete label -s"
 // the selector flag that filters which clusters to delete the cluster

cmd/pgo/cmd/label.go

@@ -26,9 +26,7 @@ import (
 )
 
 var (
-LabelCmdLabel string
-LabelMap map[string]string
-DeleteLabel bool
+DeleteLabel bool
 )
 
 var labelCmd = &cobra.Command{
@@ -47,22 +45,25 @@ var labelCmd = &cobra.Command{
 log.Debug("label called")
 if len(args) == 0 && Selector == "" {
 fmt.Println("Error: A selector or list of clusters is required to label a policy.")
-return
+os.Exit(1)
 }
-if LabelCmdLabel == "" {
+
+if len(UserLabels) == 0 {
 fmt.Println("Error: You must specify the label to apply.")
-} else {
-labelClusters(args, Namespace)
+os.Exit(1)
 }
+
+labelClusters(args, Namespace)
 },
 }
 
 func init() {
 RootCmd.AddCommand(labelCmd)
 
+labelCmd.Flags().BoolVar(&DryRun, "dry-run", false, "Shows the clusters that the label would be applied to, without labelling them.")
+labelCmd.Flags().StringSliceVar(&UserLabels, "label", []string{}, "Add labels to apply to the PostgreSQL cluster, "+
+"e.g. \"key=value\", \"prefix/key=value\". Can specify flag multiple times.")
 labelCmd.Flags().StringVarP(&Selector, "selector", "s", "", "The selector to use for cluster filtering.")
-labelCmd.Flags().StringVarP(&LabelCmdLabel, "label", "", "", "The new label to apply for any selected or specified clusters.")
-labelCmd.Flags().BoolVarP(&DryRun, "dry-run", "", false, "Shows the clusters that the label would be applied to, without labelling them.")
 }
 
 func labelClusters(clusters []string, ns string) {
@@ -78,7 +79,7 @@ func labelClusters(clusters []string, ns string) {
 r.Namespace = ns
 r.Selector = Selector
 r.DryRun = DryRun
-r.LabelCmdLabel = LabelCmdLabel
+r.Labels = getLabels(UserLabels)
 r.DeleteLabel = DeleteLabel
 r.ClientVersion = msgs.PGO_VERSION
 
@@ -111,7 +112,7 @@ func deleteLabel(args []string, ns string) {
 req.Selector = Selector
 req.Namespace = ns
 req.Args = args
-req.LabelCmdLabel = LabelCmdLabel
+req.Labels = getLabels(UserLabels)
 req.ClientVersion = msgs.PGO_VERSION
 
 response, err := api.DeleteLabel(httpclient, &SessionCredentials, &req)

docs/content/pgo-client/reference/pgo_create_cluster.md

@@ -45,7 +45,7 @@ pgo create cluster [flags]
  --exporter-memory string Set the amount of memory to request for the Crunchy Postgres Exporter sidecar container. Defaults to server value (24Mi).
  --exporter-memory-limit string Set the amount of memory to limit for the Crunchy Postgres Exporter sidecar container.
  -h, --help help for cluster
- -l, --labels string The labels to apply to this cluster.
+  --label strings Add labels to apply to the PostgreSQL cluster, e.g. "key=value", "prefix/key=value". Can specify flag multiple times.
  --memory string Set the amount of RAM to request, e.g. 1GiB. Overrides the default server value.
  --memory-limit string Set the amount of RAM to limit, e.g. 1GiB.
  --metrics Adds the crunchy-postgres-exporter container to the database pod.
@@ -136,4 +136,4 @@ pgo create cluster [flags]
 
 * [pgo create](/pgo-client/reference/pgo_create/) - Create a Postgres Operator resource
 
-###### Auto generated by spf13/cobra on 14-Jan-2021
+###### Auto generated by spf13/cobra on 18-Jan-2021

docs/content/pgo-client/reference/pgo_delete_label.md

@@ -21,7 +21,7 @@ pgo delete label [flags]
 
 ```
  -h, --help help for label
- --label string  The label to delete for any selected or specified clusters.
+ --label strings Delete labels to apply to the PostgreSQL cluster, e.g. "key=value", "prefix/key=value". Can specify flag multiple times.
  -s, --selector string The selector to use for cluster filtering.
 ```
 
@@ -42,4 +42,4 @@ pgo delete label [flags]
 
 * [pgo delete](/pgo-client/reference/pgo_delete/) - Delete an Operator resource
 
-###### Auto generated by spf13/cobra on 14-Jan-2021
+###### Auto generated by spf13/cobra on 18-Jan-2021

docs/content/pgo-client/reference/pgo_label.md

@@ -23,7 +23,7 @@ pgo label [flags]
 ```
  --dry-run Shows the clusters that the label would be applied to, without labelling them.
  -h, --help help for label
- --label string  The new label to apply for any selected or specified clusters.
+ --label strings Add labels to apply to the PostgreSQL cluster, e.g. "key=value", "prefix/key=value". Can specify flag multiple times.
  -s, --selector string The selector to use for cluster filtering.
 ```
 
@@ -44,4 +44,4 @@ pgo label [flags]
 
 * [pgo](/pgo-client/reference/pgo/) - The pgo command line interface.
 
-###### Auto generated by spf13/cobra on 14-Jan-2021
+###### Auto generated by spf13/cobra on 18-Jan-2021

internal/apiserver/clusterservice/clusterimpl.go

@@ -576,9 +576,7 @@ func CreateCluster(request *msgs.CreateClusterRequest, ns, pgouser string) msgs.
 return resp
 }
 
-userLabelsMap, err := apiserver.ValidateLabel(request.UserLabels)
-
-if err != nil {
+if err := util.ValidateLabels(request.UserLabels); err != nil {
 resp.Status.Code = msgs.Error
 resp.Status.Msg = err.Error()
 return resp
@@ -753,9 +751,6 @@ func CreateCluster(request *msgs.CreateClusterRequest, ns, pgouser string) msgs.
 return resp
 }
 
-log.Debug("userLabelsMap")
-log.Debugf("%v", userLabelsMap)
-
 if request.StorageConfig != "" && !apiserver.IsValidStorageName(request.StorageConfig) {
 resp.Status.Code = msgs.Error
 resp.Status.Msg = fmt.Sprintf("%q storage config was not found", request.StorageConfig)
@@ -857,7 +852,7 @@ func CreateCluster(request *msgs.CreateClusterRequest, ns, pgouser string) msgs.
 }
 
 // Create an instance of our CRD
-newInstance := getClusterParams(request, clusterName, userLabelsMap, ns)
+newInstance := getClusterParams(request, clusterName, ns)
 newInstance.ObjectMeta.Labels[config.LABEL_PGOUSER] = pgouser
 newInstance.Spec.BackrestStorageTypes = backrestStorageTypes
 
@@ -1066,7 +1061,7 @@ func validateConfigPolicies(clusterName, PoliciesFlag, ns string) error {
 return err
 }
 
-func getClusterParams(request *msgs.CreateClusterRequest, name string, userLabelsMap map[string]string, ns string) *crv1.Pgcluster {
+func getClusterParams(request *msgs.CreateClusterRequest, name string, ns string) *crv1.Pgcluster {
 spec := crv1.PgclusterSpec{
 Annotations: crv1.ClusterAnnotations{
 Backrest: map[string]string{},
@@ -1363,7 +1358,7 @@ func getClusterParams(request *msgs.CreateClusterRequest, name string, userLabel
 
 spec.ServiceType = request.ServiceType
 
-spec.UserLabels = userLabelsMap
+spec.UserLabels = request.UserLabels
 spec.UserLabels[config.LABEL_PGO_VERSION] = msgs.PGO_VERSION
 
 // override any values from config file

internal/apiserver/common.go

@@ -27,7 +27,6 @@ import (
 kerrors "k8s.io/apimachinery/pkg/api/errors"
 "k8s.io/apimachinery/pkg/api/resource"
 metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-"k8s.io/apimachinery/pkg/util/validation"
 )
 
 const (
@@ -45,8 +44,6 @@ var (
 // ErrDBContainerNotFound is an error that indicates that a "database" container
 // could not be found in a specific pod
 ErrDBContainerNotFound = errors.New("\"database\" container not found in pod")
-// ErrLabelInvalid indicates that a label is invalid
-ErrLabelInvalid = errors.New("invalid label")
 // ErrStandbyNotAllowed contains the error message returned when an API call is not
 // permitted because it involves a cluster that is in standby mode
 ErrStandbyNotAllowed = errors.New("Action not permitted because standby mode is enabled")
@@ -130,60 +127,6 @@ func ValidateBackrestStorageTypeForCommand(cluster *crv1.Pgcluster, storageTypeS
 return nil
 }
 
-// ValidateLabel is derived from a legacy method and validates if the input is a
-// valid Kubernetes label.
-//
-// A label is composed of a key and value.
-//
-// The key can either be a name or have an optional prefix that i
-// terminated by a "/", e.g. "prefix/name"
-//
-// The name must be a valid DNS 1123 value
-// THe prefix must be a valid DNS 1123 subdomain
-//
-// The value can be validated by machinery provided by Kubenretes
-//
-// Ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/
-func ValidateLabel(labelStr string) (map[string]string, error) {
-labelMap := map[string]string{}
-
-for _, v := range strings.Split(labelStr, ",") {
-pair := strings.Split(v, "=")
-if len(pair) != 2 {
-return labelMap, fmt.Errorf("%w: label format incorrect, requires key=value", ErrLabelInvalid)
-}
-
-// first handle the key
-keyParts := strings.Split(pair[0], "/")
-
-switch len(keyParts) {
-default:
-return labelMap, fmt.Errorf("%w: invalid key for "+v, ErrLabelInvalid)
-case 2:
-if errs := validation.IsDNS1123Subdomain(keyParts[0]); len(errs) > 0 {
-return labelMap, fmt.Errorf("%w: invalid key for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
-}
-
-if errs := validation.IsDNS1123Label(keyParts[1]); len(errs) > 0 {
-return labelMap, fmt.Errorf("%w: invalid key for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
-}
-case 1:
-if errs := validation.IsDNS1123Label(keyParts[0]); len(errs) > 0 {
-return labelMap, fmt.Errorf("%w: invalid key for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
-}
-}
-
-// handle the value
-if errs := validation.IsValidLabelValue(pair[1]); len(errs) > 0 {
-return labelMap, fmt.Errorf("%w: invalid value for %s: %s", ErrLabelInvalid, v, strings.Join(errs, ","))
-}
-
-labelMap[pair[0]] = pair[1]
-}
-
-return labelMap, nil
-}
-
 // ValidateResourceRequestLimit validates that a Kubernetes Requests/Limit pair
 // is valid, both by validating the values are valid quantity values, and then
 // by checking that the limit >= request. This also needs to check against the