From fd4030483cce716880714b6227aaa1f2a81b2968 Mon Sep 17 00:00:00 2001 From: Hector Vido Date: Mon, 18 May 2026 21:28:00 -0300 Subject: [PATCH] Added labels support for ci-secret-bootstrap --- cmd/ci-secret-bootstrap/main.go | 27 ++++++-- cmd/ci-secret-bootstrap/main_test.go | 64 +++++++++++++++++-- .../kv_update_transport.go | 12 +++- go.mod | 7 +- go.sum | 2 - pkg/api/vault/vault.go | 1 + 6 files changed, 96 insertions(+), 17 deletions(-) diff --git a/cmd/ci-secret-bootstrap/main.go b/cmd/ci-secret-bootstrap/main.go index 1b952b328b7..2273cdd85df 100644 --- a/cmd/ci-secret-bootstrap/main.go +++ b/cmd/ci-secret-bootstrap/main.go @@ -28,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/kubernetes/scheme" coreclientset "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" @@ -671,19 +672,34 @@ func fetchUserSecrets(secretsMap map[string]map[types.NamespacedName]coreapi.Sec secretsMap[cluster] = map[types.NamespacedName]coreapi.Secret{} } entry, alreadyExists := secretsMap[cluster][secretName] + labels := map[string]string{api.DPTPRequesterLabel: "ci-secret-bootstrap"} + if vaultValue, ok := secretKeys[vaultapi.SecretSyncTargetLabelsKey]; ok { + for _, label := range strings.Split(vaultValue, ",") { + label := strings.Split(label, ":") + if validation.IsQualifiedName(label[0]) == nil && validation.IsValidLabelValue(label[1]) == nil { + labels[strings.Trim(label[0], " ")] = strings.Trim(label[1], " ") + } else { + errs = append(errs, fmt.Errorf("label for secret %s, it is invalid: key '%s' value '%s'", secretName, label[0], label[1])) + } + } + } if !alreadyExists { entry = coreapi.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: secretName.Namespace, Name: secretName.Name, Labels: map[string]string{api.DPTPRequesterLabel: "ci-secret-bootstrap"}}, - Data: map[string][]byte{}, - Type: coreapi.SecretTypeOpaque, + ObjectMeta: metav1.ObjectMeta{ + Namespace: secretName.Namespace, + Name: secretName.Name, + }, + Data: map[string][]byte{}, + Type: coreapi.SecretTypeOpaque, } } + entry.ObjectMeta.Labels = labels if entry.Type != coreapi.SecretTypeOpaque { errs = append(errs, fmt.Errorf("secret %s in cluster %s has ci-secret-bootstrap config as non-opaque type and is targeted by user sync from key %s", secretName.String(), cluster, secretKeys[vaultapi.VaultSourceKey])) continue } for vaultKey, vaultValue := range secretKeys { - if vaultKey == vaultapi.SecretSyncTargetClusterKey { + if vaultKey == vaultapi.SecretSyncTargetClusterKey || vaultKey == vaultapi.SecretSyncTargetLabelsKey { continue } if _, alreadyExists := entry.Data[vaultKey]; alreadyExists { @@ -802,6 +818,7 @@ func updateSecrets(getters map[string]Getter, secretsMap map[string][]*coreapi.S if !shouldCreate { differentData := !equality.Semantic.DeepEqual(secret.Data, existingSecret.Data) + differentLabels := !equality.Semantic.DeepEqual(secret.ObjectMeta.Labels, existingSecret.ObjectMeta.Labels) var addedKeys, changedKeys, removedKeys []string for k, value := range secret.Data { if existingValue, ok := existingSecret.Data[k]; !ok { @@ -821,7 +838,7 @@ func updateSecrets(getters map[string]Getter, secretsMap map[string][]*coreapi.S errs = append(errs, fmt.Errorf("secret %s:%s/%s needs updating in place (%s), use --force to do so", cluster, secret.Namespace, secret.Name, change)) continue } - if existingSecret.Labels == nil || existingSecret.Labels[api.DPTPRequesterLabel] != "ci-secret-bootstrap" || differentData { + if existingSecret.Labels == nil || existingSecret.Labels[api.DPTPRequesterLabel] != "ci-secret-bootstrap" || differentData || differentLabels { if _, err := secretClient.Update(context.TODO(), secret, metav1.UpdateOptions{DryRun: dryRunOptions}); err != nil { errs = append(errs, fmt.Errorf("error updating secret %s:%s/%s: %w", cluster, secret.Namespace, secret.Name, err)) continue diff --git a/cmd/ci-secret-bootstrap/main_test.go b/cmd/ci-secret-bootstrap/main_test.go index af8e94a972e..b2ad398b216 100644 --- a/cmd/ci-secret-bootstrap/main_test.go +++ b/cmd/ci-secret-bootstrap/main_test.go @@ -1017,6 +1017,7 @@ Code: 404. Errors: Data: map[string]string{ "secretsync/target-namespace": "some-namespace", "secretsync/target-name": "some-name", + "secretsync/target-labels": "label:value", "some-data-key": "a-secret", }, }, @@ -1024,7 +1025,7 @@ Code: 404. Errors: config: secretbootstrap.Config{UserSecretsTargetClusters: []string{"a", "b"}}, expected: map[string][]*coreapi.Secret{ "a": {{ - ObjectMeta: metav1.ObjectMeta{Namespace: "some-namespace", Name: "some-name", Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap"}}, + ObjectMeta: metav1.ObjectMeta{Namespace: "some-namespace", Name: "some-name", Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap", "label": "value"}}, Type: coreapi.SecretTypeOpaque, Data: map[string][]byte{ "some-data-key": []byte("a-secret"), @@ -1032,7 +1033,7 @@ Code: 404. Errors: }, }}, "b": {{ - ObjectMeta: metav1.ObjectMeta{Namespace: "some-namespace", Name: "some-name", Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap"}}, + ObjectMeta: metav1.ObjectMeta{Namespace: "some-namespace", Name: "some-name", Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap", "label": "value"}}, Type: coreapi.SecretTypeOpaque, Data: map[string][]byte{ "some-data-key": []byte("a-secret"), @@ -1049,6 +1050,7 @@ Code: 404. Errors: "secretsync/target-namespace": "some-namespace", "secretsync/target-name": "some-name", "secretsync/target-clusters": "a", + "secretsync/target-labels": "some.dns.name/path:value", "some-data-key": "a-secret", }, }, @@ -1056,7 +1058,7 @@ Code: 404. Errors: config: secretbootstrap.Config{UserSecretsTargetClusters: []string{"a", "b"}}, expected: map[string][]*coreapi.Secret{ "a": {{ - ObjectMeta: metav1.ObjectMeta{Namespace: "some-namespace", Name: "some-name", Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap"}}, + ObjectMeta: metav1.ObjectMeta{Namespace: "some-namespace", Name: "some-name", Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap", "some.dns.name/path": "value"}}, Type: coreapi.SecretTypeOpaque, Data: map[string][]byte{ "some-data-key": []byte("a-secret"), @@ -1065,6 +1067,34 @@ Code: 404. Errors: }}, }, }, + { + name: "Usersecret with multiple labels", + items: map[string]vaultclient.KVData{ + "my/vault/secret": { + Data: map[string]string{ + "secretsync/target-namespace": "some-namespace", + "secretsync/target-name": "some-name", + "secretsync/target-clusters": "a", + "secretsync/target-labels": "some.dns.name/path:value,label:value", + "some-data-key": "a-secret", + }, + }, + }, + config: secretbootstrap.Config{UserSecretsTargetClusters: []string{"a", "b"}}, + expected: map[string][]*coreapi.Secret{ + "a": {{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "some-namespace", + Name: "some-name", + Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap", "some.dns.name/path": "value", "label": "value"}}, + Type: coreapi.SecretTypeOpaque, + Data: map[string][]byte{ + "some-data-key": []byte("a-secret"), + "secretsync-vault-source-path": []byte("prefix/my/vault/secret"), + }, + }}, + }, + }, { name: "Usersecret for multiple but not all clusters", items: map[string]vaultclient.KVData{ @@ -1131,7 +1161,7 @@ Code: 404. Errors: }, }, { - name: "Secret gets combined from user- and dptp secret ", + name: "Secret gets combined from user and dptp secret ", items: map[string]vaultclient.KVData{ "my/vault/secret": { Data: map[string]string{ @@ -1336,6 +1366,32 @@ Code: 404. Errors: * no data at path prefix/fake-item`, expected: map[string][]*coreapi.Secret{}, }, + { + name: "Usersecret, label with invalid key name", + items: map[string]vaultclient.KVData{ + "my/vault/secret": { + Data: map[string]string{ + "secretsync/target-namespace": "some-namespace", + "secretsync/target-name": "some-name", + "secretsync/target-clusters": "a", + "secretsync/target-labels": "some:dns.name/path:value", + "some-data-key": "a-secret", + }, + }, + }, + config: secretbootstrap.Config{UserSecretsTargetClusters: []string{"a"}}, + expected: map[string][]*coreapi.Secret{ + "a": {{ + ObjectMeta: metav1.ObjectMeta{Namespace: "some-namespace", Name: "some-name", Labels: map[string]string{"dptp.openshift.io/requester": "ci-secret-bootstrap"}}, + Type: coreapi.SecretTypeOpaque, + Data: map[string][]byte{ + "some-data-key": []byte("a-secret"), + "secretsync-vault-source-path": []byte("prefix/my/vault/secret"), + }, + }}, + }, + expectedError: "label for secret some-namespace/some-name, it is invalid: key 'some' value 'dns.name/path'", + }, } for _, tc := range testCases { diff --git a/cmd/vault-subpath-proxy/kv_update_transport.go b/cmd/vault-subpath-proxy/kv_update_transport.go index c3673f913d0..a964ed35640 100644 --- a/cmd/vault-subpath-proxy/kv_update_transport.go +++ b/cmd/vault-subpath-proxy/kv_update_transport.go @@ -122,6 +122,14 @@ func (k *kvUpdateTransport) RoundTrip(r *http.Request) (*http.Response, error) { } continue } + if key == vault.SecretSyncTargetLabelsKey { + if len(strings.TrimSpace(value)) == 0 { + errs = append(errs, fmt.Sprintf("value of key %s cannot be empty", key)) + } else if valueErrs := validation.IsValidLabelValue(value); len(valueErrs) > 0 { + errs = append(errs, fmt.Sprintf("value of key %s is invalid: %v", key, valueErrs)) + } + continue + } if key == vault.SecretSyncTargetClusterKey { continue } @@ -231,7 +239,7 @@ func (k *kvUpdateTransport) validateKeysDontConflict(ctx context.Context, path s name := types.NamespacedName{Namespace: namespace, Name: data[vault.SecretSyncTargetNameKey]} for key := range data { - if key == vault.SecretSyncTargetNamepaceKey || key == vault.SecretSyncTargetNameKey || key == vault.SecretSyncTargetClusterKey { + if key == vault.SecretSyncTargetNamepaceKey || key == vault.SecretSyncTargetNameKey || key == vault.SecretSyncTargetClusterKey || key == vault.SecretSyncTargetLabelsKey { continue } if k.existingSecretKeysByNamespaceName[name].Has(key) && !namespacedNameKeySliceContains(k.existingSecretKeysByVaultSecretName[path], namespacedNameKey{name: name, key: key}) { @@ -357,7 +365,7 @@ func (k *kvUpdateTransport) syncSecret(data map[string]string) { secret.Data = map[string][]byte{} } for k, v := range data { - if k == vault.SecretSyncTargetNamepaceKey || k == vault.SecretSyncTargetNameKey || k == vault.SecretSyncTargetClusterKey { + if k == vault.SecretSyncTargetNamepaceKey || k == vault.SecretSyncTargetNameKey || k == vault.SecretSyncTargetClusterKey || k == vault.SecretSyncTargetLabelsKey { continue } secret.Data[k] = []byte(v) diff --git a/go.mod b/go.mod index 52b146e6c27..549f00d1b88 100644 --- a/go.mod +++ b/go.mod @@ -126,7 +126,7 @@ require ( github.com/peterbourgon/diskv v2.0.1+incompatible // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/ryanuber/go-glob v1.0.0 - github.com/shurcooL/githubv4 v0.0.0-20210725200734-83ba7b4c9228 + github.com/shurcooL/githubv4 v0.0.0-20210725200734-83ba7b4c9228 // indirect github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f github.com/spf13/cast v1.5.1 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect @@ -154,7 +154,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 k8s.io/apiextensions-apiserver v0.32.8 // indirect k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 // indirect - k8s.io/kubernetes v1.29.2 + k8s.io/kubernetes v1.29.2 // indirect knative.dev/pkg v0.0.0-20250415155312-ed3e2158b883 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect ) @@ -169,7 +169,6 @@ require ( github.com/aws/aws-sdk-go-v2/credentials v1.17.67 github.com/aws/aws-sdk-go-v2/service/cloudformation v1.56.0 github.com/aws/aws-sdk-go-v2/service/ec2 v1.194.0 - github.com/aws/aws-sdk-go-v2/service/s3 v1.69.0 github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 github.com/aws/smithy-go v1.22.2 github.com/coreos/stream-metadata-go v0.1.8 @@ -200,6 +199,7 @@ require ( github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 // indirect github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.50.0 // indirect github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.50.0 // indirect + github.com/aws/aws-sdk-go-v2/service/s3 v1.69.0 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/envoyproxy/go-control-plane/envoy v1.36.0 // indirect github.com/pelletier/go-toml/v2 v2.2.3 // indirect @@ -229,7 +229,6 @@ require ( github.com/PaesslerAG/gval v1.0.0 // indirect github.com/PaesslerAG/jsonpath v0.1.1 // indirect github.com/ProtonMail/go-crypto v1.1.6 // indirect - github.com/andygrunwald/go-gerrit v0.0.0-20230211083816-04e01d7217b2 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/apache/arrow/go/v15 v15.0.2 // indirect github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.7 // indirect diff --git a/go.sum b/go.sum index 2381fbcf106..1e59e277992 100644 --- a/go.sum +++ b/go.sum @@ -153,8 +153,6 @@ github.com/anaskhan96/soup v1.2.4 h1:or+sKs9QbzJGZVTYFmTs2VBateEywoq00a6K14z331E github.com/anaskhan96/soup v1.2.4/go.mod h1:6YnEp9A2yywlYdM4EgDz9NEHclocMepEtku7wg6Cq3s= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/andybalholm/brotli v1.0.4/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig= -github.com/andygrunwald/go-gerrit v0.0.0-20230211083816-04e01d7217b2 h1:ohDU8MHAAE/FMlqs+KVlzLpVx3/gJ86rNO+8TZ9nea8= -github.com/andygrunwald/go-gerrit v0.0.0-20230211083816-04e01d7217b2/go.mod h1:SeP12EkHZxEVjuJ2HZET304NBtHGG2X6w2Gzd0QXAZw= github.com/andygrunwald/go-jira v1.17.0 h1:bbu5H676l6MaNcV6A7VDIAjIOQVgzNGEhNAwNI/Cjgo= github.com/andygrunwald/go-jira v1.17.0/go.mod h1:tiZsPUu9824bwcI2BUXatE4hJbs9rUOif0nv1lkq1hQ= github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFIImctFaOjnTIavg87rW78vTPkQqLI8= diff --git a/pkg/api/vault/vault.go b/pkg/api/vault/vault.go index 71e6d63d512..a331a29c571 100644 --- a/pkg/api/vault/vault.go +++ b/pkg/api/vault/vault.go @@ -10,6 +10,7 @@ const ( SecretSyncTargetNamepaceKey = "secretsync/target-namespace" SecretSyncTargetNameKey = "secretsync/target-name" SecretSyncTargetClusterKey = "secretsync/target-clusters" + SecretSyncTargetLabelsKey = "secretsync/target-labels" // VaultSourceKey is the key in the resulting kubernetes secret // that holds the vault path from which the user secret sync