diff --git a/cmd/ci-secret-bootstrap/main.go b/cmd/ci-secret-bootstrap/main.go index 8b6406e0ec..bd7ad5fe53 100644 --- a/cmd/ci-secret-bootstrap/main.go +++ b/cmd/ci-secret-bootstrap/main.go @@ -717,13 +717,12 @@ func generateUserSecretLabels(secretKeys map[string]string) (map[string]string, return labels, nil } for _, label := range strings.Split(raw, ",") { - label = strings.TrimSpace(label) - if label == "" { + if strings.TrimSpace(label) == "" { continue } - key, value, ok := strings.Cut(label, ":") - if !ok { - return nil, fmt.Errorf("invalid label %q: expected key:value", label) + key, value, err := api.ParseLabel(label) + if err != nil { + return nil, err } labels[key] = value } @@ -1550,11 +1549,18 @@ func constructSecretsFromGSM( if target.Type == "" { target.Type = coreapi.SecretTypeOpaque } + labels := map[string]string{} + for _, l := range bundle.Labels { + if k, v, err := api.ParseLabel(l); err == nil { + labels[k] = v + } + } + labels[api.DPTPRequesterLabel] = api.CISecretBootstrapName secret := &coreapi.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: bundle.Name, Namespace: target.Namespace, - Labels: map[string]string{api.DPTPRequesterLabel: "ci-secret-bootstrap"}, + Labels: labels, }, Type: target.Type, Data: make(map[string][]byte, len(k8sSecretData)), diff --git a/cmd/ci-secret-bootstrap/main_test.go b/cmd/ci-secret-bootstrap/main_test.go index c1e038d2ea..f000567b88 100644 --- a/cmd/ci-secret-bootstrap/main_test.go +++ b/cmd/ci-secret-bootstrap/main_test.go @@ -1137,7 +1137,7 @@ Code: 404. Errors: }, }, config: secretbootstrap.Config{UserSecretsTargetClusters: []string{"a"}}, - expectedError: `invalid label "key@value": expected key:value`, + expectedError: `invalid label "key@value": expected key:value format`, expected: map[string][]*coreapi.Secret{}, }, { @@ -4205,6 +4205,50 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, }, }, + { + name: "bundle with labels", + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ + { + Name: "gitops-cluster-build04", + Targets: []api.TargetSpec{ + {Namespace: "openshift-gitops", Cluster: "build04"}, + }, + SyncToCluster: true, + Labels: []string{"argocd.argoproj.io/secret-type:cluster", "env:ci"}, + GSMSecrets: []api.GSMSecretRef{ + { + Collection: "test-platform-infra", + Group: "gitops", + Fields: []api.FieldEntry{{Name: "token"}}, + }, + }, + }, + }, + }, + gsmSecretsPayloads: map[string][]byte{ + "projects/123456/secrets/test-platform-infra__gitops__token/versions/latest": []byte("my-token"), + }, + expected: map[string][]*coreapi.Secret{ + "build04": { + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gitops-cluster-build04", + Namespace: "openshift-gitops", + Labels: map[string]string{ + "dptp.openshift.io/requester": "ci-secret-bootstrap", + "argocd.argoproj.io/secret-type": "cluster", + "env": "ci", + }, + }, + Type: coreapi.SecretTypeOpaque, + Data: map[string][]byte{ + "token": []byte("my-token"), + }, + }, + }, + }, + }, } for _, tc := range testCases { @@ -4637,7 +4681,7 @@ func TestGenerateUserSecretLabels(t *testing.T) { vaultapi.SecretSyncTargetLabelsKey: "labelKey@labelValue", }, expected: map[string]string{}, - expectedError: `invalid label "labelKey@labelValue": expected key:value`, + expectedError: `invalid label "labelKey@labelValue": expected key:value format`, }, { name: "multiple labels", diff --git a/pkg/api/constant.go b/pkg/api/constant.go index b35a65a2ba..7c662696f6 100644 --- a/pkg/api/constant.go +++ b/pkg/api/constant.go @@ -2,8 +2,10 @@ package api import ( "fmt" + "strings" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" ) const ( @@ -36,7 +38,29 @@ const ( // DPTPRequesterLabel is the label on a Kubernates CR whose value indicates the automated tool that requests the CR DPTPRequesterLabel = "dptp.openshift.io/requester" CISecretBootstrapName = "ci-secret-bootstrap" +) + +// ParseLabel parses a "key:value" label string, returning the key and value. +// Leading/trailing whitespace on the input is trimmed. The key is split on the +// first ":" — the key is validated with validation.IsQualifiedName and the value +// with validation.IsValidLabelValue. Returns an error if the format is invalid +// or either component fails K8s label validation. +func ParseLabel(label string) (string, string, error) { + label = strings.TrimSpace(label) + key, value, ok := strings.Cut(label, ":") + if !ok { + return "", "", fmt.Errorf("invalid label %q: expected key:value format", label) + } + if errs := validation.IsQualifiedName(key); len(errs) > 0 { + return "", "", fmt.Errorf("invalid label key %q: %s", key, errs[0]) + } + if errs := validation.IsValidLabelValue(value); len(errs) > 0 { + return "", "", fmt.Errorf("invalid label value %q: %s", value, errs[0]) + } + return key, value, nil +} +const ( KVMDeviceLabel = "devices.kubevirt.io/kvm" ClusterLabel = "ci-operator.openshift.io/cluster" CloudLabel = "ci-operator.openshift.io/cloud" diff --git a/pkg/api/gsm.go b/pkg/api/gsm.go index 786180ca5a..2f0ce95432 100644 --- a/pkg/api/gsm.go +++ b/pkg/api/gsm.go @@ -33,6 +33,7 @@ type GSMBundle struct { Components []string `json:"components,omitempty"` DockerConfig *DockerConfigSpec `json:"dockerconfig,omitempty"` GSMSecrets []GSMSecretRef `json:"gsm_secrets,omitempty"` + Labels []string `json:"labels,omitempty"` SyncToCluster bool `json:"sync_to_cluster,omitempty"` Targets []TargetSpec `json:"targets,omitempty"` } @@ -240,6 +241,7 @@ func (c *GSMConfig) resolve() error { Name: bundle.Name, Components: nil, // Already resolved in phase 2 DockerConfig: bundle.DockerConfig, + Labels: bundle.Labels, SyncToCluster: bundle.SyncToCluster, Targets: targets, } @@ -448,6 +450,15 @@ func validateBundle(bundle *GSMBundle, idx int) error { } } + for i, label := range bundle.Labels { + key, _, err := ParseLabel(label) + if err != nil { + errs = append(errs, fmt.Errorf("bundle %s labels[%d]: %w", bundle.Name, i, err)) + } else if key == DPTPRequesterLabel { + errs = append(errs, fmt.Errorf("bundle %s labels[%d]: %q is a reserved label key", bundle.Name, i, DPTPRequesterLabel)) + } + } + if len(bundle.GSMSecrets) == 0 && bundle.DockerConfig == nil && len(bundle.Components) == 0 { errs = append(errs, fmt.Errorf("bundle %s has neither gsm_secrets, dockerconfig, nor components", bundle.Name)) } diff --git a/pkg/api/gsm_test.go b/pkg/api/gsm_test.go index dec7e8c52a..4bbbe5d7ea 100644 --- a/pkg/api/gsm_test.go +++ b/pkg/api/gsm_test.go @@ -737,6 +737,94 @@ func TestGSMConfigResolve(t *testing.T) { }, expectedError: `bundle "test-bundle-no-targets" uses ${CLUSTER} variable substitution but has no resolvable targets (check that cluster_groups or cluster references are valid)`, }, + { + name: "resolved bundle doesn't drop labels", + config: GSMConfig{ + ClusterGroups: map[string][]string{ + "managed-clusters": {"build01", "build02", "build03"}, + }, + Bundles: []GSMBundle{ + { + Name: "bundle-with-labels", + GSMSecrets: []GSMSecretRef{ + { + Collection: "test-platform-infra", + Group: "build-farm", + }, + }, + Targets: []TargetSpec{ + {ClusterGroups: []string{"managed-clusters"}, Namespace: "ci"}, + }, + SyncToCluster: true, + Labels: []string{"argocd.argoproj.io/secret-type:cluster"}, + }, + }, + }, + expectedConfig: GSMConfig{ + ClusterGroups: map[string][]string{ + "managed-clusters": {"build01", "build02", "build03"}, + }, + Bundles: []GSMBundle{ + { + Name: "bundle-with-labels", + GSMSecrets: []GSMSecretRef{ + { + Collection: "test-platform-infra", + Group: "build-farm", + }, + }, + SyncToCluster: true, + Targets: []TargetSpec{ + {Cluster: "build01", Namespace: "ci", Type: corev1.SecretTypeOpaque}, + {Cluster: "build02", Namespace: "ci", Type: corev1.SecretTypeOpaque}, + {Cluster: "build03", Namespace: "ci", Type: corev1.SecretTypeOpaque}, + }, + Labels: []string{"argocd.argoproj.io/secret-type:cluster"}, + }, + }, + }, + }, + { + name: "${CLUSTER} substitution preserves labels", + config: GSMConfig{ + Bundles: []GSMBundle{ + { + Name: "gitops-cluster", + GSMSecrets: []GSMSecretRef{ + { + Collection: "test-platform-infra", + Group: "gitops", + Fields: []FieldEntry{{Name: "token-${CLUSTER}"}}, + }, + }, + Targets: []TargetSpec{ + {Cluster: "build01", Namespace: "openshift-gitops"}, + }, + SyncToCluster: true, + Labels: []string{"argocd.argoproj.io/secret-type:cluster", "env:ci"}, + }, + }, + }, + expectedConfig: GSMConfig{ + Bundles: []GSMBundle{ + { + Name: "gitops-cluster", + GSMSecrets: []GSMSecretRef{ + { + Collection: "test-platform-infra", + Group: "gitops", + Fields: []FieldEntry{{Name: "token-build01"}}, + }, + }, + Targets: []TargetSpec{ + {Cluster: "build01", Namespace: "openshift-gitops", Type: corev1.SecretTypeOpaque}, + }, + SyncToCluster: true, + Labels: []string{"argocd.argoproj.io/secret-type:cluster", "env:ci"}, + }, + }, + }, + }, } for _, tc := range testCases { @@ -1607,6 +1695,77 @@ func TestGSMConfigValidate(t *testing.T) { }, expectError: false, }, + { + name: "error: invalid label format", + config: GSMConfig{ + Bundles: []GSMBundle{ + { + Name: "bundle-bad-label", + GSMSecrets: []GSMSecretRef{ + {Collection: "test-secrets", Group: "group1", Fields: []FieldEntry{{Name: "token"}}}, + }, + SyncToCluster: true, + Targets: []TargetSpec{{Cluster: "build01", Namespace: "ci"}}, + Labels: []string{"missing-colon"}, + }, + }, + }, + expectError: true, + errorContains: `invalid label "missing-colon": expected key:value format`, + }, + { + name: "error: invalid label key", + config: GSMConfig{ + Bundles: []GSMBundle{ + { + Name: "bundle-bad-key", + GSMSecrets: []GSMSecretRef{ + {Collection: "test-secrets", Group: "group1", Fields: []FieldEntry{{Name: "token"}}}, + }, + SyncToCluster: true, + Targets: []TargetSpec{{Cluster: "build01", Namespace: "ci"}}, + Labels: []string{"@invalid:value"}, + }, + }, + }, + expectError: true, + errorContains: `invalid label key`, + }, + { + name: "error: reserved label key", + config: GSMConfig{ + Bundles: []GSMBundle{ + { + Name: "bundle-reserved-label", + GSMSecrets: []GSMSecretRef{ + {Collection: "test-secrets", Group: "group1", Fields: []FieldEntry{{Name: "token"}}}, + }, + SyncToCluster: true, + Targets: []TargetSpec{{Cluster: "build01", Namespace: "ci"}}, + Labels: []string{"dptp.openshift.io/requester:evil"}, + }, + }, + }, + expectError: true, + errorContains: `is a reserved label key`, + }, + { + name: "valid: bundle with labels", + config: GSMConfig{ + Bundles: []GSMBundle{ + { + Name: "bundle-with-labels", + GSMSecrets: []GSMSecretRef{ + {Collection: "test-secrets", Group: "group1", Fields: []FieldEntry{{Name: "token"}}}, + }, + SyncToCluster: true, + Targets: []TargetSpec{{Cluster: "build01", Namespace: "ci"}}, + Labels: []string{"argocd.argoproj.io/secret-type:cluster", "env:ci"}, + }, + }, + }, + expectError: false, + }, } for _, tc := range testCases { diff --git a/pkg/api/zz_generated.deepcopy.go b/pkg/api/zz_generated.deepcopy.go index 40de70a18c..bc771c7dc9 100644 --- a/pkg/api/zz_generated.deepcopy.go +++ b/pkg/api/zz_generated.deepcopy.go @@ -502,6 +502,11 @@ func (in *GSMBundle) DeepCopyInto(out *GSMBundle) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Targets != nil { in, out := &in.Targets, &out.Targets *out = make([]TargetSpec, len(*in))