Skip to content

Added labels support for ci-secret-bootstrap #5189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hector-vido wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added labels support for ci-secret-bootstrap #5189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions cmd/ci-secret-bootstrap/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Comment thread
hector-vido marked this conversation as resolved.
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]))
}
Comment thread
hector-vido marked this conversation as resolved.
Comment thread
hector-vido marked this conversation as resolved.
}
}
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 {
Copy link
Copy Markdown
Member

@droslean droslean May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you changed that. Can you elaborate?

Copy link
Copy Markdown
Contributor Author

@hector-vido hector-vido May 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this key is not used to create an entry inside the data secret stanza, we need to skip it the same way we skip the target cluster vault data.

continue
}
if _, alreadyExists := entry.Data[vaultKey]; alreadyExists {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
64 changes: 60 additions & 4 deletions cmd/ci-secret-bootstrap/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1017,22 +1017,23 @@ Code: 404. Errors:
Data: map[string]string{
"secretsync/target-namespace": "some-namespace",
"secretsync/target-name": "some-name",
"secretsync/target-labels": "label:value",
Comment thread
hector-vido marked this conversation as resolved.
"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"}},
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"),
"secretsync-vault-source-path": []byte("prefix/my/vault/secret"),
},
}},
"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"),
Expand All @@ -1049,14 +1050,15 @@ 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",
},
},
},
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"),
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions cmd/vault-subpath-proxy/kv_update_transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Copy Markdown
Member

@droslean droslean May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are mutating the real value here.

Copy link
Copy Markdown
Contributor Author

@hector-vido hector-vido May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens during the put/patch, I am mutating, indeed, but this is the only way to check if this value is not empty or some blank string.
Why is this method a problem?

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
}
Expand Down Expand Up @@ -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}) {
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment thread
hector-vido marked this conversation as resolved.
knative.dev/pkg v0.0.0-20250415155312-ed3e2158b883 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect
)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
1 change: 1 addition & 0 deletions pkg/api/vault/vault.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down