diff --git a/cmd/ci-operator/main.go b/cmd/ci-operator/main.go index d3e5129730f..8d83f19d64a 100644 --- a/cmd/ci-operator/main.go +++ b/cmd/ci-operator/main.go @@ -450,6 +450,9 @@ type options struct { restrictNetworkAccess bool enableSecretsStoreCSIDriver bool + gsmConfigPath string + gsmConfig api.GSMConfig + gsmCredentialsFile string metricsAgent *metrics.MetricsAgent @@ -505,6 +508,8 @@ func bindOptions(flag *flag.FlagSet) *options { flag.StringVar(&opt.impersonateUser, "as", "", "Username to impersonate") flag.BoolVar(&opt.restrictNetworkAccess, "restrict-network-access", false, "Restrict network access to 10.0.0.0/8 (RedHat intranet).") flag.BoolVar(&opt.enableSecretsStoreCSIDriver, "enable-secrets-store-csi-driver", false, "Use Secrets Store CSI driver for accessing multi-stage credentials.") + flag.StringVar(&opt.gsmConfigPath, "gsm-config", "", "Path to the gsm config file.") + flag.StringVar(&opt.gsmCredentialsFile, "gsm-credentials-file", "", "Path to GCP service account credentials.") // flags needed for the configresolver flag.StringVar(&opt.resolverAddress, "resolver-address", configResolverAddress, "Address of configresolver") @@ -753,6 +758,13 @@ func (o *options) Complete() error { handleTargetAdditionalSuffix(o) + if o.enableSecretsStoreCSIDriver { + err := api.LoadGSMConfigFromFile(o.gsmConfigPath, &o.gsmConfig) + if err != nil { + return err + } + } + return overrideTestStepDependencyParams(o) } @@ -960,7 +972,7 @@ func (o *options) Run() []error { // load the graph from the configuration buildSteps, promotionSteps, err := defaults.FromConfig(ctx, o.configSpec, &o.graphConfig, o.jobSpec, o.templates, o.writeParams, o.promote, o.clusterConfig, o.podPendingTimeout, leaseClient, o.targets.values, o.cloneAuthConfig, o.pullSecret, o.pushSecret, o.censor, o.hiveKubeconfig, - o.nodeName, nodeArchitectures, o.targetAdditionalSuffix, o.manifestToolDockerCfg, o.localRegistryDNS, streams, injectedTest, o.enableSecretsStoreCSIDriver, o.metricsAgent, o.skippedImages) + o.nodeName, nodeArchitectures, o.targetAdditionalSuffix, o.manifestToolDockerCfg, o.localRegistryDNS, streams, injectedTest, o.enableSecretsStoreCSIDriver, &o.gsmConfig, o.gsmCredentialsFile, o.metricsAgent, o.skippedImages) if err != nil { return []error{results.ForReason("defaulting_config").WithError(err).Errorf("failed to generate steps from config: %v", err)} } diff --git a/cmd/ci-secret-bootstrap/main.go b/cmd/ci-secret-bootstrap/main.go index 53039650679..59bea5aeb97 100644 --- a/cmd/ci-secret-bootstrap/main.go +++ b/cmd/ci-secret-bootstrap/main.go @@ -80,7 +80,7 @@ type options struct { vaultConfig secretbootstrap.Config generatorConfig secretgenerator.Config - gsmConfig secretbootstrap.GSMConfig + gsmConfig api.GSMConfig gsmProjectConfig gsm.Config allowUnused flagutil.Strings @@ -151,7 +151,7 @@ func (o *options) completeOptions(censor *secrets.DynamicCensor, kubeConfigs map } if o.enableGsm { - if err := secretbootstrap.LoadGSMConfigFromFile(o.gsmConfigPath, &o.gsmConfig); err != nil { + if err := api.LoadGSMConfigFromFile(o.gsmConfigPath, &o.gsmConfig); err != nil { return err } gsmProjectConfig, err := gsm.GetConfigFromEnv() @@ -227,7 +227,7 @@ func (o *options) completeOptions(censor *secrets.DynamicCensor, kubeConfigs map // This mirrors the Vault filtering above and ensures we only process bundles // for clusters that are available and match any user-specified cluster filter. if o.enableGsm && len(o.gsmConfig.Bundles) > 0 { - var filteredBundles []secretbootstrap.Bundle + var filteredBundles []api.GSMBundle for i := range o.gsmConfig.Bundles { bundle := &o.gsmConfig.Bundles[i] // Preserve bundles with SyncToCluster=false regardless of targets @@ -235,7 +235,7 @@ func (o *options) completeOptions(censor *secrets.DynamicCensor, kubeConfigs map filteredBundles = append(filteredBundles, *bundle) continue } - var filteredTargets []secretbootstrap.TargetSpec + var filteredTargets []api.TargetSpec for _, target := range bundle.Targets { if disabledClusters.Has(target.Cluster) { logrus.WithFields(logrus.Fields{ @@ -377,7 +377,7 @@ func (o *options) validateVaultGSMConflicts() error { // clear ownership of secrets during the migration from Vault to GSM. // // Returns an aggregate error containing all detected conflicts, or nil if no conflicts exist. -func validateGSMVaultConflicts(gsmConfig *secretbootstrap.GSMConfig, vaultConfig *secretbootstrap.Config) error { +func validateGSMVaultConflicts(gsmConfig *api.GSMConfig, vaultConfig *secretbootstrap.Config) error { var errs []error // Build index of Vault secrets @@ -446,7 +446,7 @@ func constructDockerConfigJSONFromVault(client secrets.ReadOnlyClient, dockerCon } // constructDockerConfigJSONFromGSM constructs a .dockerconfigjson from GSM secrets cache -func constructDockerConfigJSONFromGSM(secretsCache map[gsmSecretRef]fetchedSecret, registries []secretbootstrap.RegistryAuthData) ([]byte, error) { +func constructDockerConfigJSONFromGSM(secretsCache map[gsmSecretRef]fetchedSecret, registries []api.RegistryAuthData) ([]byte, error) { auths := make(map[string]secretbootstrap.DockerAuth) for _, reg := range registries { @@ -1174,8 +1174,8 @@ func reconcileSecrets(o options, vaultClient secrets.ReadOnlyClient, gsmClient * logrus.Infof("the config file %s has been validated", o.vaultConfigPath) if o.enableGsm { - var gsmConfig secretbootstrap.GSMConfig - if err := secretbootstrap.LoadGSMConfigFromFile(o.gsmConfigPath, &gsmConfig); err != nil { + var gsmConfig api.GSMConfig + if err := api.LoadGSMConfigFromFile(o.gsmConfigPath, &gsmConfig); err != nil { return append(errs, fmt.Errorf("failed to load GSM config from file: %s", o.gsmConfigPath)) } if err := gsmConfig.Validate(); err != nil { @@ -1295,7 +1295,7 @@ type collectionGroupKey struct { // Returns a map of cluster name to list of Kubernetes Secret objects, and any fetch/build errors. func constructSecretsFromGSM( ctx context.Context, - gsmConfig secretbootstrap.GSMConfig, + gsmConfig api.GSMConfig, gsmClient gsm.SecretManagerClient, gsmProjectConfig gsm.Config, prowDisabledClusters sets.Set[string]) (map[string][]*coreapi.Secret, error) { @@ -1410,7 +1410,7 @@ func constructSecretsFromGSM( bundleHasError := false for _, gsmSecretEntry := range bundle.GSMSecrets { - var fieldsToProcess []secretbootstrap.FieldEntry + var fieldsToProcess []api.FieldEntry if len(gsmSecretEntry.Fields) == 0 { key := collectionGroupKey{ collection: gsmSecretEntry.Collection, @@ -1423,7 +1423,7 @@ func constructSecretsFromGSM( break } for _, fieldName := range fieldNames { - fieldsToProcess = append(fieldsToProcess, secretbootstrap.FieldEntry{ + fieldsToProcess = append(fieldsToProcess, api.FieldEntry{ Name: fieldName, As: "", }) diff --git a/cmd/ci-secret-bootstrap/main_test.go b/cmd/ci-secret-bootstrap/main_test.go index 53d11dbb7d7..b10bc0bf798 100644 --- a/cmd/ci-secret-bootstrap/main_test.go +++ b/cmd/ci-secret-bootstrap/main_test.go @@ -18,7 +18,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/googleapis/gax-go/v2" - "github.com/hashicorp/vault/api" + vaultApi "github.com/hashicorp/vault/api" coreapi "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,6 +28,7 @@ import ( "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" + "github.com/openshift/ci-tools/pkg/api" "github.com/openshift/ci-tools/pkg/api/secretbootstrap" "github.com/openshift/ci-tools/pkg/api/secretgenerator" gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" @@ -2263,7 +2264,7 @@ func (f *fakeVaultClient) GetKV(path string) (*vaultclient.KVData, error) { return item, nil } - return nil, &api.ResponseError{ + return nil, &vaultApi.ResponseError{ HTTPMethod: "GET", StatusCode: 404, URL: "fakeVaultClient.GetKV", @@ -3690,7 +3691,7 @@ func TestConstructSecretsFromGSM(t *testing.T) { testCases := []struct { name string - config secretbootstrap.GSMConfig + config api.GSMConfig gsmSecretsPayloads map[string][]byte disabledClusters sets.Set[string] expected map[string][]*coreapi.Secret @@ -3698,22 +3699,22 @@ func TestConstructSecretsFromGSM(t *testing.T) { }{ { name: "basic case: explicit fields", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "test-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ci", Cluster: "build01", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "test-infra", Group: "build-farm", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "username"}, {Name: "password"}, }, @@ -3745,22 +3746,22 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "multi-level groups", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "multilevel-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "test", Cluster: "app.ci", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "vsphere", Group: "ibmcloud/ci", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "token"}, }, }, @@ -3789,22 +3790,22 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "field denormalization", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "denorm-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ci", Cluster: "build01", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "test", Group: "group1", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "aws--u--creds"}, {Name: "--dot--dockerconfigjson"}, {Name: "renamed--u--field", As: "custom-name"}, @@ -3839,11 +3840,11 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "docker config from GSM", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "docker-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ci", Cluster: "build01", @@ -3851,9 +3852,9 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, }, SyncToCluster: true, - DockerConfig: &secretbootstrap.DockerConfigSpec{ + DockerConfig: &api.DockerConfigSpec{ As: "pull-secret", - Registries: []secretbootstrap.RegistryAuthData{ + Registries: []api.RegistryAuthData{ { Collection: "test-infra", Group: "build-farm", @@ -3895,11 +3896,11 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "secret is not created for disabled cluster", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "test-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ci", Cluster: "build01", @@ -3910,11 +3911,11 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "test", Group: "group1", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "key"}, }, }, @@ -3944,22 +3945,22 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "error: missing GSM secret", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "error-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ci", Cluster: "build01", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "test", Group: "group1", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "missing-field"}, }, }, @@ -3972,22 +3973,22 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "error: bundle with partial failure", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "partial-error-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ci", Cluster: "build01", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "test", Group: "group1", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "field1"}, {Name: "field2"}, }, @@ -4003,22 +4004,22 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "multiple bundles on different clusters", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "secret-1", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ns1", Cluster: "build01", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "col1", Group: "grp1", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "key1"}, }, }, @@ -4026,18 +4027,18 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { Name: "secret-2", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ns2", Cluster: "build02", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "col2", Group: "grp2", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "key2"}, }, }, @@ -4080,22 +4081,22 @@ func TestConstructSecretsFromGSM(t *testing.T) { }, { name: "bundle with empty group", - config: secretbootstrap.GSMConfig{ - Bundles: []secretbootstrap.Bundle{ + config: api.GSMConfig{ + Bundles: []api.GSMBundle{ { Name: "no-group-secret", - Targets: []secretbootstrap.TargetSpec{ + Targets: []api.TargetSpec{ { Namespace: "ci", Cluster: "build01", }, }, SyncToCluster: true, - GSMSecrets: []secretbootstrap.GSMSecretRef{ + GSMSecrets: []api.GSMSecretRef{ { Collection: "simple", Group: "", - Fields: []secretbootstrap.FieldEntry{ + Fields: []api.FieldEntry{ {Name: "token"}, }, }, @@ -4161,7 +4162,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { testCases := []struct { name string secretsCache map[gsmSecretRef]fetchedSecret - registries []secretbootstrap.RegistryAuthData + registries []api.RegistryAuthData expected string expectedError string }{ @@ -4170,7 +4171,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { secretsCache: map[gsmSecretRef]fetchedSecret{ {collection: "test", group: "grp", field: "auth"}: {payload: []byte("dXNlcjpwYXNz")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4186,7 +4187,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { {collection: "test", group: "grp", field: "auth"}: {payload: []byte("dXNlcjpwYXNz")}, {collection: "test", group: "grp", field: "email"}: {payload: []byte("user@example.com")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4204,7 +4205,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { {collection: "test", group: "grp", field: "auth2"}: {payload: []byte("dXNlcjI6cGFzczI=")}, {collection: "test", group: "grp", field: "email2"}: {payload: []byte("user@example.com")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4226,7 +4227,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { secretsCache: map[gsmSecretRef]fetchedSecret{ {collection: "test", group: "grp", field: "auth"}: {payload: []byte(" dXNlcjpwYXNz \n")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4239,7 +4240,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { { name: "error: auth field not found", secretsCache: map[gsmSecretRef]fetchedSecret{}, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4254,7 +4255,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { secretsCache: map[gsmSecretRef]fetchedSecret{ {collection: "test", group: "grp", field: "auth"}: {err: fmt.Errorf("fetch failed")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4269,7 +4270,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { secretsCache: map[gsmSecretRef]fetchedSecret{ {collection: "test", group: "grp", field: "auth"}: {payload: []byte("dXNlcjpwYXNz")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4286,7 +4287,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { {collection: "test", group: "grp", field: "auth"}: {payload: []byte("dXNlcjpwYXNz")}, {collection: "test", group: "grp", field: "email"}: {err: fmt.Errorf("email fetch failed")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", @@ -4302,7 +4303,7 @@ func TestConstructDockerConfigJSONFromGSM(t *testing.T) { secretsCache: map[gsmSecretRef]fetchedSecret{ {collection: "test", group: "grp", field: "auth"}: {payload: []byte("not-base64-encoded")}, }, - registries: []secretbootstrap.RegistryAuthData{ + registries: []api.RegistryAuthData{ { Collection: "test", Group: "grp", diff --git a/cmd/ci-secret-generator/main.go b/cmd/ci-secret-generator/main.go index 6428b92355e..5c32d9a1c4c 100644 --- a/cmd/ci-secret-generator/main.go +++ b/cmd/ci-secret-generator/main.go @@ -19,6 +19,7 @@ import ( "github.com/openshift/ci-tools/pkg/api/secretbootstrap" "github.com/openshift/ci-tools/pkg/api/secretgenerator" gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" + gsmvalidation "github.com/openshift/ci-tools/pkg/gsm-validation" "github.com/openshift/ci-tools/pkg/prowconfigutils" "github.com/openshift/ci-tools/pkg/secrets" ) @@ -211,8 +212,9 @@ func fmtExecCmdErr(action, cmd string, wrappedErr error, stdout, stderr []byte, stdout, stderrPreamble, stderr) } -func updateSecrets(config secretgenerator.Config, client secrets.Client, disabledClusters sets.Set[string]) error { +func updateSecrets(config secretgenerator.Config, client secrets.Client, disabledClusters sets.Set[string], GSMsyncEnabled bool) error { var errs []error + var allSecretsList []string for _, item := range config { logger := logrus.WithField("item", item.ItemName) for _, field := range item.Fields { @@ -239,6 +241,11 @@ func updateSecrets(config secretgenerator.Config, client secrets.Client, disable errs = append(errs, errors.New(msg)) continue } + if GSMsyncEnabled { + normalizedGroup := gsmvalidation.NormalizeName(item.ItemName) + normalizedField := gsmvalidation.NormalizeName(field.Name) + allSecretsList = append(allSecretsList, fmt.Sprintf("%s__%s", normalizedGroup, normalizedField)) + } } // Adding the notes not empty check here since we dont want to overwrite any notes that might already be present @@ -256,6 +263,13 @@ func updateSecrets(config secretgenerator.Config, client secrets.Client, disable } } } + if GSMsyncEnabled { + err := client.UpdateIndexSecret(secrets.TestPlatformCollection, gsm.ConstructIndexSecretContent(allSecretsList)) + if err != nil { + errs = append(errs, err) + } + } + return utilerrors.NewAggregate(errs) } @@ -326,7 +340,7 @@ func generateSecrets(o options, censor *secrets.DynamicCensor) (errs []error) { } } - if err := updateSecrets(o.config, client, o.disabledClusters); err != nil { + if err := updateSecrets(o.config, client, o.disabledClusters, o.enableGsmSync); err != nil { errs = append(errs, fmt.Errorf("failed to update secrets: %w", err)) } diff --git a/cmd/ci-secret-generator/main_test.go b/cmd/ci-secret-generator/main_test.go index 84624109924..76836cffb5b 100644 --- a/cmd/ci-secret-generator/main_test.go +++ b/cmd/ci-secret-generator/main_test.go @@ -7,11 +7,13 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "go.uber.org/mock/gomock" "k8s.io/apimachinery/pkg/util/sets" "github.com/openshift/ci-tools/pkg/api/secretbootstrap" "github.com/openshift/ci-tools/pkg/api/secretgenerator" + gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" "github.com/openshift/ci-tools/pkg/secrets" "github.com/openshift/ci-tools/pkg/testhelper" "github.com/openshift/ci-tools/pkg/vaultclient" @@ -193,7 +195,7 @@ func TestVault(t *testing.T) { } } }() - if err := updateSecrets(tc.config, client, tc.disabledClusters); err != nil { + if err := updateSecrets(tc.config, client, tc.disabledClusters, false); err != nil { t.Errorf("failed to update secrets: %v", err) } list, err := vault.ListKV("secret") @@ -435,3 +437,312 @@ func TestValidateConfig(t *testing.T) { }) } } + +func TestUpdateSecretsWithGSMIndex(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + config secretgenerator.Config + GSMsyncEnabled bool + expectedIndexCalls int + verifyIndexPayload func(t *testing.T, itemName string, payload []byte) + }{ + { + name: "GSM sync enabled - single field creates index secret", + config: secretgenerator.Config{{ + ItemName: "test-item", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + }, + }}, + GSMsyncEnabled: true, + expectedIndexCalls: 1, + verifyIndexPayload: func(t *testing.T, itemName string, payload []byte) { + if itemName != secrets.TestPlatformCollection { + t.Errorf("expected item name %q, got %q", secrets.TestPlatformCollection, itemName) + } + expectedPayload := string(gsm.ConstructIndexSecretContent([]string{"test-item__field1"})) + if string(payload) != expectedPayload { + t.Errorf("expected payload %q, got %q", expectedPayload, string(payload)) + } + }, + }, + { + name: "GSM sync enabled - multiple fields create index with all fields", + config: secretgenerator.Config{{ + ItemName: "multi-field-item", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + {Name: "field2", Cmd: "printf 'value2'"}, + {Name: "field3", Cmd: "printf 'value3'"}, + }, + }}, + GSMsyncEnabled: true, + expectedIndexCalls: 1, + verifyIndexPayload: func(t *testing.T, itemName string, payload []byte) { + if itemName != secrets.TestPlatformCollection { + t.Errorf("expected item name %q, got %q", secrets.TestPlatformCollection, itemName) + } + expectedPayload := string(gsm.ConstructIndexSecretContent([]string{"multi-field-item__field1", "multi-field-item__field2", "multi-field-item__field3"})) + if string(payload) != expectedPayload { + t.Errorf("expected payload %q, got %q", expectedPayload, string(payload)) + } + }, + }, + { + name: "GSM sync enabled - multiple items create single collection-level index", + config: secretgenerator.Config{ + { + ItemName: "item1", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + }, + }, + { + ItemName: "item2", + Fields: []secretgenerator.FieldGenerator{ + {Name: "fieldA", Cmd: "printf 'valueA'"}, + {Name: "fieldB", Cmd: "printf 'valueB'"}, + }, + }, + }, + GSMsyncEnabled: true, + expectedIndexCalls: 1, // Only ONE index for entire collection + verifyIndexPayload: func(t *testing.T, itemName string, payload []byte) { + if itemName != secrets.TestPlatformCollection { + t.Errorf("expected item name %q, got %q", secrets.TestPlatformCollection, itemName) + } + // Index contains ALL fields from ALL groups + expectedPayload := string(gsm.ConstructIndexSecretContent([]string{"item1__field1", "item2__fieldA", "item2__fieldB"})) + if string(payload) != expectedPayload { + t.Errorf("expected payload %q, got %q", expectedPayload, string(payload)) + } + }, + }, + { + name: "GSM sync disabled - no index secret created", + config: secretgenerator.Config{{ + ItemName: "test-item", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + {Name: "field2", Cmd: "printf 'value2'"}, + }, + }}, + GSMsyncEnabled: false, + expectedIndexCalls: 0, + }, + { + name: "GSM sync enabled - disabled cluster field excluded from index", + config: secretgenerator.Config{{ + ItemName: "cluster-test-item", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'", Cluster: "enabled-cluster"}, + {Name: "field2", Cmd: "printf 'value2'", Cluster: "disabled-cluster"}, + {Name: "field3", Cmd: "printf 'value3'", Cluster: "enabled-cluster"}, + }, + }}, + GSMsyncEnabled: true, + expectedIndexCalls: 1, + verifyIndexPayload: func(t *testing.T, itemName string, payload []byte) { + // Only field1 and field3 should be in the index (field2 is from disabled cluster) + expectedPayload := string(gsm.ConstructIndexSecretContent([]string{"cluster-test-item__field1", "cluster-test-item__field3"})) + if string(payload) != expectedPayload { + t.Errorf("expected payload %q, got %q", expectedPayload, string(payload)) + } + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockClient := secrets.NewMockClient(mockCtrl) + mockClient.EXPECT(). + SetFieldOnItem(gomock.Any(), gomock.Any(), gomock.Any()). + AnyTimes(). + Return(nil) + + // set expectations for UpdateIndexSecret based on test case + if tc.expectedIndexCalls > 0 { + mockClient.EXPECT(). + UpdateIndexSecret(gomock.Any(), gomock.Any()). + Times(tc.expectedIndexCalls). + DoAndReturn(func(itemName string, payload []byte) error { + if tc.verifyIndexPayload != nil { + tc.verifyIndexPayload(t, itemName, payload) + } + return nil + }) + } + + // for the disabled cluster test case, pass the disabled clusters set + var disabledClusters sets.Set[string] + if tc.name == "GSM sync enabled - disabled cluster field excluded from index" { + disabledClusters = sets.New[string]("disabled-cluster") + } + + err := updateSecrets(tc.config, mockClient, disabledClusters, tc.GSMsyncEnabled) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +func TestUpdateSecretsWithGSMIndexErrors(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + config secretgenerator.Config + setFieldError error + updateIndexError error + expectedErrorsLen int + partialFailure bool + partialFailureConfig map[string]error // field name -> error (nil = success) + expectedIndexFields []string // fields that should appear in index for partial failure + }{ + { + name: "UpdateIndexSecret error is aggregated", + config: secretgenerator.Config{{ + ItemName: "test-item", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + }, + }}, + updateIndexError: errors.New("GSM update failed"), + expectedErrorsLen: 1, + }, + { + name: "SetFieldOnItem error - all fields fail, index contains only updater-service-account", + config: secretgenerator.Config{{ + ItemName: "test-item", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + {Name: "field2", Cmd: "printf 'value2'"}, + }, + }}, + setFieldError: errors.New("field upload failed"), + updateIndexError: nil, // Index update should still be called + expectedErrorsLen: 2, // Both field errors + }, + { + name: "SetFieldOnItem partial failure - only successful fields in index", + config: secretgenerator.Config{{ + ItemName: "test-item", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + {Name: "field2", Cmd: "printf 'value2'"}, + {Name: "field3", Cmd: "printf 'value3'"}, + }, + }}, + partialFailure: true, + partialFailureConfig: map[string]error{ + "field1": nil, // succeeds + "field2": errors.New("field2 upload failed"), // fails + "field3": nil, // succeeds + }, + expectedIndexFields: []string{"test-item__field1", "test-item__field3"}, + expectedErrorsLen: 1, + }, + { + name: "Multiple items - index errors are aggregated", + config: secretgenerator.Config{ + { + ItemName: "item1", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field1", Cmd: "printf 'value1'"}, + }, + }, + { + ItemName: "item2", + Fields: []secretgenerator.FieldGenerator{ + {Name: "field2", Cmd: "printf 'value2'"}, + }, + }, + }, + updateIndexError: errors.New("GSM update failed"), + expectedErrorsLen: 1, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockClient := secrets.NewMockClient(mockCtrl) + + if tc.partialFailure { + // For partial failure, return different errors based on field name + mockClient.EXPECT(). + SetFieldOnItem(gomock.Any(), gomock.Any(), gomock.Any()). + AnyTimes(). + DoAndReturn(func(itemName, fieldName string, fieldValue []byte) error { + if err, exists := tc.partialFailureConfig[fieldName]; exists { + return err + } + return nil + }) + } else if tc.setFieldError != nil { + mockClient.EXPECT(). + SetFieldOnItem(gomock.Any(), gomock.Any(), gomock.Any()). + AnyTimes(). + Return(tc.setFieldError) + } else { + mockClient.EXPECT(). + SetFieldOnItem(gomock.Any(), gomock.Any(), gomock.Any()). + AnyTimes(). + Return(nil) + } + + // Set expectations for UpdateIndexSecret - it should always be called in these tests + if tc.updateIndexError != nil { + mockClient.EXPECT(). + UpdateIndexSecret(gomock.Any(), gomock.Any()). + AnyTimes(). + Return(tc.updateIndexError) + } else { + // Verify that when SetFieldOnItem fails, the index only contains successful fields + if tc.setFieldError != nil || tc.partialFailure { + mockClient.EXPECT(). + UpdateIndexSecret(gomock.Any(), gomock.Any()). + AnyTimes(). + DoAndReturn(func(itemName string, payload []byte) error { + var expectedPayload string + if tc.partialFailure { + // For partial failure, index should contain only successful fields + expectedPayload = string(gsm.ConstructIndexSecretContent(tc.expectedIndexFields)) + } else { + // When all fields fail, index should only contain updater-service-account + expectedPayload = string(gsm.ConstructIndexSecretContent([]string{})) + } + if string(payload) != expectedPayload { + t.Errorf("expected index payload %q, got %q", expectedPayload, string(payload)) + } + return nil + }) + } else { + mockClient.EXPECT(). + UpdateIndexSecret(gomock.Any(), gomock.Any()). + AnyTimes(). + Return(nil) + } + } + + err := updateSecrets(tc.config, mockClient, nil, true) + if err == nil { + t.Errorf("expected error but got nil") + return + } + errStr := err.Error() + if errStr == "" { + t.Errorf("expected non-empty error message") + } + }) + } +} diff --git a/pkg/api/secretbootstrap/gsm.go b/pkg/api/gsm.go similarity index 97% rename from pkg/api/secretbootstrap/gsm.go rename to pkg/api/gsm.go index 526495b8f23..564b593b0b8 100644 --- a/pkg/api/secretbootstrap/gsm.go +++ b/pkg/api/gsm.go @@ -1,4 +1,4 @@ -package secretbootstrap +package api import ( "encoding/json" @@ -17,11 +17,11 @@ import ( type GSMConfig struct { ClusterGroups map[string][]string `json:"cluster_groups,omitempty"` Components map[string][]GSMSecretRef `json:"components,omitempty"` - Bundles []Bundle `json:"bundles"` + Bundles []GSMBundle `json:"bundles"` } -// Bundle defines a logical group of GSM secrets -type Bundle struct { +// GSMBundle defines a logical group of GSM secrets +type GSMBundle struct { Name string `json:"name"` Components []string `json:"components,omitempty"` DockerConfig *DockerConfigSpec `json:"dockerconfig,omitempty"` @@ -86,7 +86,7 @@ type TargetSpec struct { func LoadGSMConfigFromFile(file string, config *GSMConfig) error { bytes, err := gzip.ReadFileMaybeGZIP(file) if err != nil { - return err + return fmt.Errorf("couldn't read GSM config file: %w", err) } return yaml.UnmarshalStrict(bytes, config) } @@ -169,7 +169,7 @@ func (c *GSMConfig) resolve() error { } // Expand ${CLUSTER} variable substitution - var expandedBundles []Bundle + var expandedBundles []GSMBundle for _, bundle := range c.Bundles { // Check if bundle contains ${CLUSTER} in any secret names hasClusterVar := false @@ -205,7 +205,7 @@ func (c *GSMConfig) resolve() error { // Create one bundle per unique cluster for cluster, targets := range clusterTargets { - expandedBundle := Bundle{ + expandedBundle := GSMBundle{ Name: bundle.Name, Components: nil, // Already resolved in phase 2 DockerConfig: bundle.DockerConfig, @@ -319,7 +319,7 @@ func (c *GSMConfig) Validate() error { return utilerrors.NewAggregate(errs) } -func validateBundle(bundle *Bundle, idx int) error { +func validateBundle(bundle *GSMBundle, idx int) error { var errs []error if bundle.SyncToCluster && len(bundle.Targets) == 0 { diff --git a/pkg/api/secretbootstrap/gsm_test.go b/pkg/api/gsm_test.go similarity index 95% rename from pkg/api/secretbootstrap/gsm_test.go rename to pkg/api/gsm_test.go index c5512a36295..e5290ec6fae 100644 --- a/pkg/api/secretbootstrap/gsm_test.go +++ b/pkg/api/gsm_test.go @@ -1,4 +1,4 @@ -package secretbootstrap +package api import ( "os" @@ -15,7 +15,7 @@ import ( // sortBundlesByCluster sorts bundles by their first target's cluster name for deterministic comparison. // Uses multiple sort keys to ensure stable ordering: cluster, bundle name, then namespace. -func sortBundlesByCluster(bundles []Bundle) { +func sortBundlesByCluster(bundles []GSMBundle) { sort.Slice(bundles, func(i, j int) bool { clusterI := "" if len(bundles[i].Targets) > 0 { @@ -55,7 +55,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01", "build02"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -71,7 +71,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01", "build02"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -92,7 +92,7 @@ func TestGSMConfigResolve(t *testing.T) { "build-clusters": {"build01"}, "app-clusters": {"app01", "app02"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -109,7 +109,7 @@ func TestGSMConfigResolve(t *testing.T) { "build-clusters": {"build01"}, "app-clusters": {"app01", "app02"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -130,7 +130,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -146,7 +146,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -162,7 +162,7 @@ func TestGSMConfigResolve(t *testing.T) { { name: "direct cluster - not expanded", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -175,7 +175,7 @@ func TestGSMConfigResolve(t *testing.T) { }, }, expectedConfig: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -196,7 +196,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "component-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: []string{"my-component"}, @@ -212,7 +212,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "component-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: nil, @@ -237,7 +237,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "secrets-b", Group: "group2"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: []string{"component-a", "component-b"}, @@ -256,7 +256,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "secrets-b", Group: "group2"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: nil, @@ -279,7 +279,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "component-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: []string{"my-component"}, @@ -298,7 +298,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "component-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: nil, @@ -316,7 +316,7 @@ func TestGSMConfigResolve(t *testing.T) { { name: "${CLUSTER} substitution - single cluster", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -335,7 +335,7 @@ func TestGSMConfigResolve(t *testing.T) { }, }, expectedConfig: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -357,7 +357,7 @@ func TestGSMConfigResolve(t *testing.T) { { name: "${CLUSTER} substitution - multiple clusters creates separate bundles", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -377,7 +377,7 @@ func TestGSMConfigResolve(t *testing.T) { }, }, expectedConfig: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -414,7 +414,7 @@ func TestGSMConfigResolve(t *testing.T) { { name: "${CLUSTER} substitution - preserves 'as' field", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -433,7 +433,7 @@ func TestGSMConfigResolve(t *testing.T) { }, }, expectedConfig: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -463,7 +463,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "base-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: []string{"base-component"}, @@ -491,7 +491,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "base-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: nil, @@ -535,7 +535,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -555,7 +555,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -577,7 +577,7 @@ func TestGSMConfigResolve(t *testing.T) { {Collection: "secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", Components: []string{"non-existent-component"}, @@ -595,7 +595,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01", "build02"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -612,7 +612,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01", "build02"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -630,7 +630,7 @@ func TestGSMConfigResolve(t *testing.T) { { name: "multiple ${CLUSTER} variables in same GSMSecretRef", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -652,7 +652,7 @@ func TestGSMConfigResolve(t *testing.T) { }, }, expectedConfig: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -696,7 +696,7 @@ func TestGSMConfigResolve(t *testing.T) { ClusterGroups: map[string][]string{ "build-clusters": {"build01"}, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle-with-cluster-var", GSMSecrets: []GSMSecretRef{ @@ -719,7 +719,7 @@ func TestGSMConfigResolve(t *testing.T) { { name: "error: ${CLUSTER} variable with empty targets", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle-no-targets", GSMSecrets: []GSMSecretRef{ @@ -924,7 +924,7 @@ func TestGSMConfigValidate(t *testing.T) { {Collection: "test-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -942,7 +942,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "valid config - auto discovery (no fields)", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -965,7 +965,7 @@ func TestGSMConfigValidate(t *testing.T) { {Collection: "test-secrets", Group: "group1"}, }, }, - Bundles: []Bundle{}, + Bundles: []GSMBundle{}, }, expectError: true, errorContains: "component has empty name", @@ -976,7 +976,7 @@ func TestGSMConfigValidate(t *testing.T) { Components: map[string][]GSMSecretRef{ "my-component": {}, }, - Bundles: []Bundle{}, + Bundles: []GSMBundle{}, }, expectError: true, errorContains: "component my-component has no GSM secret references", @@ -989,7 +989,7 @@ func TestGSMConfigValidate(t *testing.T) { {Collection: "", Group: "group1"}, }, }, - Bundles: []Bundle{}, + Bundles: []GSMBundle{}, }, expectError: true, errorContains: "component my-component[0] has empty collection", @@ -1002,7 +1002,7 @@ func TestGSMConfigValidate(t *testing.T) { {Collection: "some-malformed$(-collection name", Group: "group1"}, }, }, - Bundles: []Bundle{}, + Bundles: []GSMBundle{}, }, expectError: true, errorContains: "component my-component[0] has invalid collection string", @@ -1015,7 +1015,7 @@ func TestGSMConfigValidate(t *testing.T) { {Collection: "valid-collection", Group: "invalid$group!name"}, }, }, - Bundles: []Bundle{}, + Bundles: []GSMBundle{}, }, expectError: true, errorContains: "component my-component[0] has invalid group string", @@ -1028,7 +1028,7 @@ func TestGSMConfigValidate(t *testing.T) { {Collection: "valid-collection", Group: "group1", Fields: []FieldEntry{{Name: "bad$field!name"}}}, }, }, - Bundles: []Bundle{}, + Bundles: []GSMBundle{}, }, expectError: true, errorContains: "component my-component[0].secrets[0] has invalid name", @@ -1041,7 +1041,7 @@ func TestGSMConfigValidate(t *testing.T) { {Collection: "test-secrets", Group: "", Fields: []FieldEntry{}}, }, }, - Bundles: []Bundle{}, + Bundles: []GSMBundle{}, }, expectError: true, errorContains: "component my-component[0] has neither group nor any fields defined", @@ -1049,7 +1049,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: bundle with empty name", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "", GSMSecrets: []GSMSecretRef{ @@ -1067,7 +1067,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: duplicate bundle by name+cluster+namespace", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1096,7 +1096,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: sync_to_cluster true but no targets", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1113,7 +1113,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: sync_to_cluster false but has targets", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1132,7 +1132,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: target with empty namespace", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1151,7 +1151,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: target with empty cluster", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1170,7 +1170,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: target with invalid type", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1189,7 +1189,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: GSMSecretRef with empty collection", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1208,7 +1208,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: GSMSecretRef with no secrets and no group", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1227,7 +1227,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: bundle GSMSecretRef with invalid group", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1246,7 +1246,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: field with empty name", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1265,7 +1265,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: bundle GSMSecretRef with invalid field name", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", GSMSecrets: []GSMSecretRef{ @@ -1284,7 +1284,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "valid config - dockerconfig with empty 'as' field defaults to .dockerconfigjson", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", DockerConfig: &DockerConfigSpec{ @@ -1305,7 +1305,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: dockerconfig with no registries", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", DockerConfig: &DockerConfigSpec{ @@ -1325,7 +1325,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: dockerconfig registry with empty collection", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", DockerConfig: &DockerConfigSpec{ @@ -1346,7 +1346,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: dockerconfig registry with empty registry_url", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", DockerConfig: &DockerConfigSpec{ @@ -1368,7 +1368,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: dockerconfig registry with empty auth_field", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", DockerConfig: &DockerConfigSpec{ @@ -1390,7 +1390,7 @@ func TestGSMConfigValidate(t *testing.T) { { name: "error: bundle with neither gsm_secrets, dockerconfig, nor components", config: GSMConfig{ - Bundles: []Bundle{ + Bundles: []GSMBundle{ { Name: "test-bundle", SyncToCluster: true, diff --git a/pkg/api/secretbootstrap/testdata/gsm-resolve/complex-expected.yaml b/pkg/api/testdata/gsm-resolve/complex-expected.yaml similarity index 100% rename from pkg/api/secretbootstrap/testdata/gsm-resolve/complex-expected.yaml rename to pkg/api/testdata/gsm-resolve/complex-expected.yaml diff --git a/pkg/api/secretbootstrap/testdata/gsm-resolve/complex-input.yaml b/pkg/api/testdata/gsm-resolve/complex-input.yaml similarity index 100% rename from pkg/api/secretbootstrap/testdata/gsm-resolve/complex-input.yaml rename to pkg/api/testdata/gsm-resolve/complex-input.yaml diff --git a/pkg/api/secretbootstrap/testdata/gsm-resolve/dockerconfig-cluster-var-expected.yaml b/pkg/api/testdata/gsm-resolve/dockerconfig-cluster-var-expected.yaml similarity index 100% rename from pkg/api/secretbootstrap/testdata/gsm-resolve/dockerconfig-cluster-var-expected.yaml rename to pkg/api/testdata/gsm-resolve/dockerconfig-cluster-var-expected.yaml diff --git a/pkg/api/secretbootstrap/testdata/gsm-resolve/dockerconfig-cluster-var-input.yaml b/pkg/api/testdata/gsm-resolve/dockerconfig-cluster-var-input.yaml similarity index 100% rename from pkg/api/secretbootstrap/testdata/gsm-resolve/dockerconfig-cluster-var-input.yaml rename to pkg/api/testdata/gsm-resolve/dockerconfig-cluster-var-input.yaml diff --git a/pkg/api/types.go b/pkg/api/types.go index 364726fe621..d75462988a5 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1133,16 +1133,35 @@ type StepParameter struct { type CredentialReference struct { // Namespace is where the source secret exists. Namespace string `json:"namespace"` - // Collection is the name of the collection the secret belongs to. - // In GCP, the secret is named __ -- this represents - // the part. - Collection string `json:"collection"` + // Bundle is a named bundle reference from the GSM config mapping file. + // Mutually exclusive with Collection/Group/Field. + Bundle string `json:"bundle,omitempty"` + // Collection is the name of the collection the secret belongs to (first level of 3-level ____ hierarchy). + Collection string `json:"collection,omitempty"` + // Group is the group name within the collection (second level of 3-level ____ hierarchy). + // Required when Collection is set, unless Bundle is used. + Group string `json:"group,omitempty"` + // Field is the specific field name (third level of 3-level ____ hierarchy). + // If omitted, all fields in the collection/group are auto-discovered. + Field string `json:"field,omitempty"` // Name is the name of the secret, without the collection prefix. Name string `json:"name"` // MountPath is where the secret should be mounted. MountPath string `json:"mount_path"` } +func (c *CredentialReference) IsAutoDiscovery() bool { + return c.Field == "" && c.Group != "" && c.Collection != "" +} + +func (c *CredentialReference) IsExplicitField() bool { + return c.Field != "" && c.Group != "" && c.Collection != "" +} + +func (c *CredentialReference) IsBundleReference() bool { + return c.Bundle != "" +} + // StepDependency defines a dependency on an image and the environment variable // used to expose the image's pull spec to the step. type StepDependency struct { diff --git a/pkg/api/zz_generated.deepcopy.go b/pkg/api/zz_generated.deepcopy.go index 7d7d4c9e95b..15825cefca9 100644 --- a/pkg/api/zz_generated.deepcopy.go +++ b/pkg/api/zz_generated.deepcopy.go @@ -387,6 +387,26 @@ func (in DependencyOverrides) DeepCopy() DependencyOverrides { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DockerConfigSpec) DeepCopyInto(out *DockerConfigSpec) { + *out = *in + if in.Registries != nil { + in, out := &in.Registries, &out.Registries + *out = make([]RegistryAuthData, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DockerConfigSpec. +func (in *DockerConfigSpec) DeepCopy() *DockerConfigSpec { + if in == nil { + return nil + } + out := new(DockerConfigSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExternalImage) DeepCopyInto(out *ExternalImage) { *out = *in @@ -403,6 +423,136 @@ func (in *ExternalImage) DeepCopy() *ExternalImage { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FieldEntry) DeepCopyInto(out *FieldEntry) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FieldEntry. +func (in *FieldEntry) DeepCopy() *FieldEntry { + if in == nil { + return nil + } + out := new(FieldEntry) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GSMBundle) DeepCopyInto(out *GSMBundle) { + *out = *in + if in.Components != nil { + in, out := &in.Components, &out.Components + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.DockerConfig != nil { + in, out := &in.DockerConfig, &out.DockerConfig + *out = new(DockerConfigSpec) + (*in).DeepCopyInto(*out) + } + if in.GSMSecrets != nil { + in, out := &in.GSMSecrets, &out.GSMSecrets + *out = make([]GSMSecretRef, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Targets != nil { + in, out := &in.Targets, &out.Targets + *out = make([]TargetSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GSMBundle. +func (in *GSMBundle) DeepCopy() *GSMBundle { + if in == nil { + return nil + } + out := new(GSMBundle) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GSMConfig) DeepCopyInto(out *GSMConfig) { + *out = *in + if in.ClusterGroups != nil { + in, out := &in.ClusterGroups, &out.ClusterGroups + *out = make(map[string][]string, len(*in)) + for key, val := range *in { + var outVal []string + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = make([]string, len(*in)) + copy(*out, *in) + } + (*out)[key] = outVal + } + } + if in.Components != nil { + in, out := &in.Components, &out.Components + *out = make(map[string][]GSMSecretRef, len(*in)) + for key, val := range *in { + var outVal []GSMSecretRef + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = make([]GSMSecretRef, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + (*out)[key] = outVal + } + } + if in.Bundles != nil { + in, out := &in.Bundles, &out.Bundles + *out = make([]GSMBundle, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GSMConfig. +func (in *GSMConfig) DeepCopy() *GSMConfig { + if in == nil { + return nil + } + out := new(GSMConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GSMSecretRef) DeepCopyInto(out *GSMSecretRef) { + *out = *in + if in.Fields != nil { + in, out := &in.Fields, &out.Fields + *out = make([]FieldEntry, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GSMSecretRef. +func (in *GSMSecretRef) DeepCopy() *GSMSecretRef { + if in == nil { + return nil + } + out := new(GSMSecretRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GraphConfiguration) DeepCopyInto(out *GraphConfiguration) { *out = *in @@ -1532,6 +1682,21 @@ func (in *RefRepository) DeepCopy() *RefRepository { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RegistryAuthData) DeepCopyInto(out *RegistryAuthData) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegistryAuthData. +func (in *RegistryAuthData) DeepCopy() *RegistryAuthData { + if in == nil { + return nil + } + out := new(RegistryAuthData) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RegistryChain) DeepCopyInto(out *RegistryChain) { *out = *in @@ -2100,6 +2265,26 @@ func (in *StepParameter) DeepCopy() *StepParameter { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TargetSpec) DeepCopyInto(out *TargetSpec) { + *out = *in + if in.ClusterGroups != nil { + in, out := &in.ClusterGroups, &out.ClusterGroups + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TargetSpec. +func (in *TargetSpec) DeepCopy() *TargetSpec { + if in == nil { + return nil + } + out := new(TargetSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in TestDependencies) DeepCopyInto(out *TestDependencies) { { diff --git a/pkg/defaults/defaults.go b/pkg/defaults/defaults.go index c51a639bf7b..e02d61e6337 100644 --- a/pkg/defaults/defaults.go +++ b/pkg/defaults/defaults.go @@ -10,8 +10,10 @@ import ( "strings" "time" + secretmanager "cloud.google.com/go/secretmanager/apiv1" "github.com/hashicorp/go-retryablehttp" "github.com/sirupsen/logrus" + "google.golang.org/api/option" coreapi "k8s.io/api/core/v1" kapierrors "k8s.io/apimachinery/pkg/api/errors" @@ -32,6 +34,7 @@ import ( "github.com/openshift/ci-tools/pkg/api" "github.com/openshift/ci-tools/pkg/api/configresolver" + gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/labeledclient" "github.com/openshift/ci-tools/pkg/lease" @@ -80,6 +83,8 @@ func FromConfig( integratedStreams map[string]*configresolver.IntegratedStream, injectedTest bool, enableSecretsStoreCSIDriver bool, + gsmConfig *api.GSMConfig, + gsmCredentialsFile string, metricsAgent *metrics.MetricsAgent, skippedImages sets.Set[string], ) ([]api.Step, []api.Step, error) { @@ -119,7 +124,7 @@ func FromConfig( httpClient := retryablehttp.NewClient() httpClient.Logger = nil - return fromConfig(ctx, config, graphConf, jobSpec, templates, paramFile, promote, client, buildClient, templateClient, podClient, leaseClient, hiveClient, httpClient.StandardClient(), requiredTargets, cloneAuthConfig, pullSecret, pushSecret, api.NewDeferredParameters(nil), censor, nodeName, targetAdditionalSuffix, nodeArchitectures, integratedStreams, injectedTest, enableSecretsStoreCSIDriver, metricsAgent, skippedImages) + return fromConfig(ctx, config, graphConf, jobSpec, templates, paramFile, promote, client, buildClient, templateClient, podClient, leaseClient, hiveClient, httpClient.StandardClient(), requiredTargets, cloneAuthConfig, pullSecret, pushSecret, api.NewDeferredParameters(nil), censor, nodeName, targetAdditionalSuffix, nodeArchitectures, integratedStreams, injectedTest, enableSecretsStoreCSIDriver, gsmConfig, gsmCredentialsFile, metricsAgent, skippedImages) } func fromConfig( @@ -148,6 +153,8 @@ func fromConfig( integratedStreams map[string]*configresolver.IntegratedStream, injectedTest bool, enableSecretsStoreCSIDriver bool, + gsmConfig *api.GSMConfig, + gsmCredentialsFile string, metricsAgent *metrics.MetricsAgent, skippedImages sets.Set[string], ) ([]api.Step, []api.Step, error) { @@ -160,6 +167,36 @@ func fromConfig( params.Add("JOB_NAME_SAFE", func() (string, error) { return strings.Replace(jobSpec.Job, "_", "-", -1), nil }) params.Add("UNIQUE_HASH", func() (string, error) { return jobSpec.UniqueHash(), nil }) params.Add("NAMESPACE", func() (string, error) { return jobSpec.Namespace(), nil }) + + // Initialize GSM configuration if CSI driver is enabled + var gsmConfiguration *multi_stage.GSMConfiguration + if enableSecretsStoreCSIDriver { + gsmConfiguration = &multi_stage.GSMConfiguration{ + Config: gsmConfig, + CredentialsFile: gsmCredentialsFile, + } + + // Get GSM project config from environment + gsmProjectConfig, err := gsm.GetConfigFromEnv() + if err != nil { + logrus.WithError(err).Error("Failed to get GSM project config from environment") + } else { + gsmConfiguration.ProjectConfig = gsmProjectConfig + } + + // Initialize GSM client with credentials + var opts []option.ClientOption + if gsmCredentialsFile != "" { + opts = append(opts, option.WithCredentialsFile(gsmCredentialsFile)) + } + gsmClient, err := secretmanager.NewClient(ctx, opts...) + if err != nil { + logrus.WithError(err).Error("Failed to initialize GSM client") + } else { + gsmConfiguration.Client = gsmClient + } + } + inputImages := make(inputImageSet) var overridableSteps []api.Step var buildSteps []api.Step @@ -176,7 +213,7 @@ func fromConfig( for _, rawStep := range rawSteps { if testStep := rawStep.TestStepConfiguration; testStep != nil { - steps, err := stepForTest(config, params, podClient, leaseClient, templateClient, client, hiveClient, jobSpec, inputImages, testStep, &imageConfigs, pullSecret, censor, nodeName, targetAdditionalSuffix, enableSecretsStoreCSIDriver, metricsAgent) + steps, err := stepForTest(config, params, podClient, leaseClient, templateClient, client, hiveClient, jobSpec, inputImages, testStep, &imageConfigs, pullSecret, censor, nodeName, targetAdditionalSuffix, enableSecretsStoreCSIDriver, gsmConfiguration, metricsAgent) if err != nil { return nil, nil, err } @@ -457,6 +494,7 @@ func stepForTest( nodeName string, targetAdditionalSuffix string, enableSecretsStoreCSIDriver bool, + gsmConfiguration *multi_stage.GSMConfiguration, metricsAgent *metrics.MetricsAgent, ) ([]api.Step, error) { if test := c.MultiStageTestConfigurationLiteral; test != nil { @@ -466,7 +504,7 @@ func stepForTest( params = api.NewDeferredParameters(params) } var ret []api.Step - step := multi_stage.MultiStageTestStep(*c, config, params, podClient, jobSpec, leases, nodeName, targetAdditionalSuffix, nil, enableSecretsStoreCSIDriver) + step := multi_stage.MultiStageTestStep(*c, config, params, podClient, jobSpec, leases, nodeName, targetAdditionalSuffix, nil, enableSecretsStoreCSIDriver, gsmConfiguration) if ipPoolLease.ResourceType != "" { step = steps.IPPoolStep(leaseClient, podClient, ipPoolLease, step, params, jobSpec.Namespace, metricsAgent) } diff --git a/pkg/defaults/defaults_test.go b/pkg/defaults/defaults_test.go index d1860a9abd7..651df817753 100644 --- a/pkg/defaults/defaults_test.go +++ b/pkg/defaults/defaults_test.go @@ -1874,7 +1874,7 @@ func TestFromConfig(t *testing.T) { params.Add(k, func() (string, error) { return v, nil }) } graphConf := FromConfigStatic(&tc.config) - configSteps, post, err := fromConfig(context.Background(), &tc.config, &graphConf, &jobSpec, tc.templates, tc.paramFiles, tc.promote, client, buildClient, templateClient, podClient, leaseClient, hiveClient, httpClient, tc.requiredTargets, cloneAuthConfig, pullSecret, pushSecret, params, &secrets.DynamicCensor{}, api.ServiceDomainAPPCI, "", nil, map[string]*configresolver.IntegratedStream{}, tc.injectedTest, false, nil, tc.skippedImages) + configSteps, post, err := fromConfig(context.Background(), &tc.config, &graphConf, &jobSpec, tc.templates, tc.paramFiles, tc.promote, client, buildClient, templateClient, podClient, leaseClient, hiveClient, httpClient, tc.requiredTargets, cloneAuthConfig, pullSecret, pushSecret, params, &secrets.DynamicCensor{}, api.ServiceDomainAPPCI, "", nil, map[string]*configresolver.IntegratedStream{}, tc.injectedTest, false, nil, "", nil, tc.skippedImages) if diff := cmp.Diff(tc.expectedErr, err); diff != "" { t.Errorf("unexpected error: %v", diff) } diff --git a/pkg/gsm-secrets/execution.go b/pkg/gsm-secrets/execution.go index ac9f9343b09..a6e5956e56c 100644 --- a/pkg/gsm-secrets/execution.go +++ b/pkg/gsm-secrets/execution.go @@ -255,7 +255,7 @@ func (a *Actions) CreateSecrets(ctx context.Context, secretsClient SecretManager } if s.Type == SecretTypeIndex { - s.Payload = fmt.Appendf(nil, "- updater-service-account") + s.Payload = ConstructIndexSecretContent([]string{}) a.SecretsToCreate[name] = s } diff --git a/pkg/gsm-secrets/secrets.go b/pkg/gsm-secrets/secrets.go index a183d25a08a..c2b334a230f 100644 --- a/pkg/gsm-secrets/secrets.go +++ b/pkg/gsm-secrets/secrets.go @@ -2,6 +2,7 @@ package gsmsecrets import ( "fmt" + "sort" "strings" validation "github.com/openshift/ci-tools/pkg/gsm-validation" @@ -80,3 +81,17 @@ func VerifyIndexSecretContent(payload []byte) error { return nil } + +// ConstructIndexSecretContent constructs the index secret content from the secretsList, +// with UpdaterSASecretName included. +func ConstructIndexSecretContent(secretsList []string) []byte { + secretsList = append(secretsList, UpdaterSASecretName) + sort.Strings(secretsList) + + var formattedSecrets []string + for _, secret := range secretsList { + formattedSecrets = append(formattedSecrets, fmt.Sprintf("- %s", secret)) + } + + return []byte(strings.Join(formattedSecrets, "\n")) +} diff --git a/pkg/gsm-secrets/types.go b/pkg/gsm-secrets/types.go index 6e2f3e379d2..134e056aa30 100644 --- a/pkg/gsm-secrets/types.go +++ b/pkg/gsm-secrets/types.go @@ -17,6 +17,7 @@ const ( GCPMaxServiceAccountIDLength = 30 + UpdaterSASecretName = "updater-service-account" UpdaterSASecretSuffix = "__updater-service-account" IndexSecretSuffix = "____index" diff --git a/pkg/secrets/client.go b/pkg/secrets/client.go index 482cfaf7c5b..70202b447ee 100644 --- a/pkg/secrets/client.go +++ b/pkg/secrets/client.go @@ -18,6 +18,7 @@ type Client interface { ReadOnlyClient SetFieldOnItem(itemName, fieldName string, fieldValue []byte) error UpdateNotesOnItem(itemName string, notes string) error + UpdateIndexSecret(itemName string, payload []byte) error } type SecretUsageComparer interface { diff --git a/pkg/secrets/client_mock.go b/pkg/secrets/client_mock.go new file mode 100644 index 00000000000..9c38fbcf98b --- /dev/null +++ b/pkg/secrets/client_mock.go @@ -0,0 +1,296 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: pkg/secrets/client.go +// +// Generated by this command: +// +// mockgen -source=pkg/secrets/client.go -destination=pkg/secrets/client_mock.go -package=secrets +// + +// Package secrets is a generated GoMock package. +package secrets + +import ( + reflect "reflect" + time "time" + + gomock "go.uber.org/mock/gomock" + + types "k8s.io/apimachinery/pkg/types" + sets "k8s.io/apimachinery/pkg/util/sets" +) + +// MockReadOnlyClient is a mock of ReadOnlyClient interface. +type MockReadOnlyClient struct { + ctrl *gomock.Controller + recorder *MockReadOnlyClientMockRecorder + isgomock struct{} +} + +// MockReadOnlyClientMockRecorder is the mock recorder for MockReadOnlyClient. +type MockReadOnlyClientMockRecorder struct { + mock *MockReadOnlyClient +} + +// NewMockReadOnlyClient creates a new mock instance. +func NewMockReadOnlyClient(ctrl *gomock.Controller) *MockReadOnlyClient { + mock := &MockReadOnlyClient{ctrl: ctrl} + mock.recorder = &MockReadOnlyClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockReadOnlyClient) EXPECT() *MockReadOnlyClientMockRecorder { + return m.recorder +} + +// GetFieldOnItem mocks base method. +func (m *MockReadOnlyClient) GetFieldOnItem(itemName, fieldName string) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetFieldOnItem", itemName, fieldName) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetFieldOnItem indicates an expected call of GetFieldOnItem. +func (mr *MockReadOnlyClientMockRecorder) GetFieldOnItem(itemName, fieldName any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFieldOnItem", reflect.TypeOf((*MockReadOnlyClient)(nil).GetFieldOnItem), itemName, fieldName) +} + +// GetInUseInformationForAllItems mocks base method. +func (m *MockReadOnlyClient) GetInUseInformationForAllItems(optionalPrefix string) (map[string]SecretUsageComparer, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetInUseInformationForAllItems", optionalPrefix) + ret0, _ := ret[0].(map[string]SecretUsageComparer) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetInUseInformationForAllItems indicates an expected call of GetInUseInformationForAllItems. +func (mr *MockReadOnlyClientMockRecorder) GetInUseInformationForAllItems(optionalPrefix any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetInUseInformationForAllItems", reflect.TypeOf((*MockReadOnlyClient)(nil).GetInUseInformationForAllItems), optionalPrefix) +} + +// GetUserSecrets mocks base method. +func (m *MockReadOnlyClient) GetUserSecrets() (map[types.NamespacedName]map[string]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserSecrets") + ret0, _ := ret[0].(map[types.NamespacedName]map[string]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserSecrets indicates an expected call of GetUserSecrets. +func (mr *MockReadOnlyClientMockRecorder) GetUserSecrets() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecrets", reflect.TypeOf((*MockReadOnlyClient)(nil).GetUserSecrets)) +} + +// HasItem mocks base method. +func (m *MockReadOnlyClient) HasItem(itemname string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HasItem", itemname) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HasItem indicates an expected call of HasItem. +func (mr *MockReadOnlyClientMockRecorder) HasItem(itemname any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasItem", reflect.TypeOf((*MockReadOnlyClient)(nil).HasItem), itemname) +} + +// MockClient is a mock of Client interface. +type MockClient struct { + ctrl *gomock.Controller + recorder *MockClientMockRecorder + isgomock struct{} +} + +// MockClientMockRecorder is the mock recorder for MockClient. +type MockClientMockRecorder struct { + mock *MockClient +} + +// NewMockClient creates a new mock instance. +func NewMockClient(ctrl *gomock.Controller) *MockClient { + mock := &MockClient{ctrl: ctrl} + mock.recorder = &MockClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClient) EXPECT() *MockClientMockRecorder { + return m.recorder +} + +// GetFieldOnItem mocks base method. +func (m *MockClient) GetFieldOnItem(itemName, fieldName string) ([]byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetFieldOnItem", itemName, fieldName) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetFieldOnItem indicates an expected call of GetFieldOnItem. +func (mr *MockClientMockRecorder) GetFieldOnItem(itemName, fieldName any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFieldOnItem", reflect.TypeOf((*MockClient)(nil).GetFieldOnItem), itemName, fieldName) +} + +// GetInUseInformationForAllItems mocks base method. +func (m *MockClient) GetInUseInformationForAllItems(optionalPrefix string) (map[string]SecretUsageComparer, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetInUseInformationForAllItems", optionalPrefix) + ret0, _ := ret[0].(map[string]SecretUsageComparer) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetInUseInformationForAllItems indicates an expected call of GetInUseInformationForAllItems. +func (mr *MockClientMockRecorder) GetInUseInformationForAllItems(optionalPrefix any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetInUseInformationForAllItems", reflect.TypeOf((*MockClient)(nil).GetInUseInformationForAllItems), optionalPrefix) +} + +// GetUserSecrets mocks base method. +func (m *MockClient) GetUserSecrets() (map[types.NamespacedName]map[string]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserSecrets") + ret0, _ := ret[0].(map[types.NamespacedName]map[string]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserSecrets indicates an expected call of GetUserSecrets. +func (mr *MockClientMockRecorder) GetUserSecrets() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserSecrets", reflect.TypeOf((*MockClient)(nil).GetUserSecrets)) +} + +// HasItem mocks base method. +func (m *MockClient) HasItem(itemname string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HasItem", itemname) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HasItem indicates an expected call of HasItem. +func (mr *MockClientMockRecorder) HasItem(itemname any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasItem", reflect.TypeOf((*MockClient)(nil).HasItem), itemname) +} + +// SetFieldOnItem mocks base method. +func (m *MockClient) SetFieldOnItem(itemName, fieldName string, fieldValue []byte) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetFieldOnItem", itemName, fieldName, fieldValue) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetFieldOnItem indicates an expected call of SetFieldOnItem. +func (mr *MockClientMockRecorder) SetFieldOnItem(itemName, fieldName, fieldValue any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetFieldOnItem", reflect.TypeOf((*MockClient)(nil).SetFieldOnItem), itemName, fieldName, fieldValue) +} + +// UpdateIndexSecret mocks base method. +func (m *MockClient) UpdateIndexSecret(itemName string, payload []byte) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateIndexSecret", itemName, payload) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateIndexSecret indicates an expected call of UpdateIndexSecret. +func (mr *MockClientMockRecorder) UpdateIndexSecret(itemName, payload any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateIndexSecret", reflect.TypeOf((*MockClient)(nil).UpdateIndexSecret), itemName, payload) +} + +// UpdateNotesOnItem mocks base method. +func (m *MockClient) UpdateNotesOnItem(itemName, notes string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateNotesOnItem", itemName, notes) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateNotesOnItem indicates an expected call of UpdateNotesOnItem. +func (mr *MockClientMockRecorder) UpdateNotesOnItem(itemName, notes any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNotesOnItem", reflect.TypeOf((*MockClient)(nil).UpdateNotesOnItem), itemName, notes) +} + +// MockSecretUsageComparer is a mock of SecretUsageComparer interface. +type MockSecretUsageComparer struct { + ctrl *gomock.Controller + recorder *MockSecretUsageComparerMockRecorder + isgomock struct{} +} + +// MockSecretUsageComparerMockRecorder is the mock recorder for MockSecretUsageComparer. +type MockSecretUsageComparerMockRecorder struct { + mock *MockSecretUsageComparer +} + +// NewMockSecretUsageComparer creates a new mock instance. +func NewMockSecretUsageComparer(ctrl *gomock.Controller) *MockSecretUsageComparer { + mock := &MockSecretUsageComparer{ctrl: ctrl} + mock.recorder = &MockSecretUsageComparerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSecretUsageComparer) EXPECT() *MockSecretUsageComparerMockRecorder { + return m.recorder +} + +// LastChanged mocks base method. +func (m *MockSecretUsageComparer) LastChanged() time.Time { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LastChanged") + ret0, _ := ret[0].(time.Time) + return ret0 +} + +// LastChanged indicates an expected call of LastChanged. +func (mr *MockSecretUsageComparerMockRecorder) LastChanged() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LastChanged", reflect.TypeOf((*MockSecretUsageComparer)(nil).LastChanged)) +} + +// SuperfluousFields mocks base method. +func (m *MockSecretUsageComparer) SuperfluousFields() sets.Set[string] { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SuperfluousFields") + ret0, _ := ret[0].(sets.Set[string]) + return ret0 +} + +// SuperfluousFields indicates an expected call of SuperfluousFields. +func (mr *MockSecretUsageComparerMockRecorder) SuperfluousFields() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SuperfluousFields", reflect.TypeOf((*MockSecretUsageComparer)(nil).SuperfluousFields)) +} + +// UnusedFields mocks base method. +func (m *MockSecretUsageComparer) UnusedFields(inUse sets.Set[string]) sets.Set[string] { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UnusedFields", inUse) + ret0, _ := ret[0].(sets.Set[string]) + return ret0 +} + +// UnusedFields indicates an expected call of UnusedFields. +func (mr *MockSecretUsageComparerMockRecorder) UnusedFields(inUse any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnusedFields", reflect.TypeOf((*MockSecretUsageComparer)(nil).UnusedFields), inUse) +} diff --git a/pkg/secrets/gsm.go b/pkg/secrets/gsm.go index fe6e46ff499..cbd7f412330 100644 --- a/pkg/secrets/gsm.go +++ b/pkg/secrets/gsm.go @@ -3,7 +3,6 @@ package secrets import ( "context" "fmt" - "regexp" secretmanager "cloud.google.com/go/secretmanager/apiv1" "github.com/sirupsen/logrus" @@ -13,12 +12,15 @@ import ( gsmvalidation "github.com/openshift/ci-tools/pkg/gsm-validation" ) +const ( + TestPlatformCollection = "test-platform-infra" +) + type gsmSyncDecorator struct { Client gsmClient *secretmanager.Client config gsm.Config ctx context.Context - pattern *regexp.Regexp } func NewGSMSyncDecorator(wrappedVaultClient Client, gcpProjectConfig gsm.Config, credentialsFile string) (Client, error) { @@ -34,33 +36,32 @@ func NewGSMSyncDecorator(wrappedVaultClient Client, gcpProjectConfig gsm.Config, return nil, fmt.Errorf("failed to create GSM client: %w", err) } - // Hardcoded pattern to match only the fields created for cluster-init secret - pattern := regexp.MustCompile(`^sa\.cluster-init\..*`) - return &gsmSyncDecorator{ Client: wrappedVaultClient, gsmClient: gsmClient, config: gcpProjectConfig, - pattern: pattern, ctx: ctx, }, nil } +// SetFieldOnItem syncs a secret field to both Vault and GSM. +// In the 3-level GSM hierarchy (collection__group__field): +// - collection: TestPlatformCollection constant ("test-platform-infra") +// - group: itemName parameter (e.g., "cluster-init", "build-farm") +// - field: fieldName parameter (e.g., "sa.ci-operator.app.ci.config") +// +// Example: SetFieldOnItem("build-farm", "token.txt", data) +// +// -> GSM secret: test-platform-infra__build-farm__token--dot--txt func (g *gsmSyncDecorator) SetFieldOnItem(itemName, fieldName string, fieldValue []byte) error { // Call the original client (Vault) if err := g.Client.SetFieldOnItem(itemName, fieldName, fieldValue); err != nil { return err } - // Check if this field should sync to GSM (only cluster-init secrets) - if !g.pattern.MatchString(fieldName) { - return nil - } - - // replace forbidden characters: - // e.g., "sa.cluster-init.build01.config" -> "sa--dot--cluster-init--dot--build01--dot--config" - fieldNameNormalized := gsmvalidation.NormalizeName(fieldName) - secretName := fmt.Sprintf("%s__%s", "cluster-init", fieldNameNormalized) + group := gsmvalidation.NormalizeName(itemName) + field := gsmvalidation.NormalizeName(fieldName) + secretName := gsm.GetGSMSecretName(TestPlatformCollection, group, field) labels := make(map[string]string) labels["jira-project"] = "dptp" @@ -72,8 +73,25 @@ func (g *gsmSyncDecorator) SetFieldOnItem(itemName, fieldName string, fieldValue logrus.WithError(err).Errorf("Failed to sync to GSM: %s", secretName) // Don't fail the Vault write } else { - logrus.Debugf("Successfully synced cluster-init secret to GSM: %s", secretName) + logrus.Debugf("Successfully synced secret '%s' to GSM", secretName) } return nil } + +// UpdateIndexSecret creates or updates the index secret for a GSM collection. +// The index secret tracks all field names within a collection. +// +// Parameters: +// - itemName: The GSM collection name (e.g., "test-platform-infra") +// - payload: a list of all field names in the collection +// +// Creates GSM secret named: {itemName}____index +func (g *gsmSyncDecorator) UpdateIndexSecret(itemName string, payload []byte) error { + annotations := make(map[string]string) + annotations["request-information"] = "Created by periodic-ci-secret-generator." + if err := gsm.CreateOrUpdateSecret(g.ctx, g.gsmClient, g.config.ProjectIdNumber, gsm.GetIndexSecretName(itemName), payload, nil, annotations); err != nil { + return err + } + return nil +} diff --git a/pkg/secrets/vault.go b/pkg/secrets/vault.go index c82074ec28b..721acbbec49 100644 --- a/pkg/secrets/vault.go +++ b/pkg/secrets/vault.go @@ -44,6 +44,10 @@ func (d dryRunClient) GetInUseInformationForAllItems(_ string) (map[string]Secre return nil, nil } +func (d dryRunClient) UpdateIndexSecret(itemName string, payload []byte) error { + return nil +} + func (d dryRunClient) GetUserSecrets() (map[types.NamespacedName]map[string]string, error) { return nil, nil } @@ -130,6 +134,11 @@ func (c *vaultClient) GetFieldOnItem(itemName, fieldName string) ([]byte, error) return c.getSecretAtPath(itemName, fieldName) } +func (c *vaultClient) UpdateIndexSecret(itemName string, payload []byte) error { + // No-op: index secrets are only relevant for GSM syncing + return nil +} + func (c *vaultClient) GetInUseInformationForAllItems(optionalSubPath string) (map[string]SecretUsageComparer, error) { prefix := c.prefix if optionalSubPath != "" { diff --git a/pkg/steps/multi_stage/csi_utils.go b/pkg/steps/multi_stage/csi_utils.go index 25529c0c5e6..8c37c91f214 100644 --- a/pkg/steps/multi_stage/csi_utils.go +++ b/pkg/steps/multi_stage/csi_utils.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/yaml" "github.com/openshift/ci-tools/pkg/api" + gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" gsmvalidation "github.com/openshift/ci-tools/pkg/gsm-validation" ) @@ -34,13 +35,14 @@ func groupCredentialsByCollectionAndMountPath(credentials []api.CredentialRefere func buildGCPSecretsParameter(credentials []api.CredentialReference) (string, error) { var secrets []config.Secret for _, credential := range credentials { - fileName, err := replaceForbiddenSymbolsInCredentialName(credential.Name) + fileName, err := restoreForbiddenSymbolsInSecretName(credential.Field) if err != nil { - return "", fmt.Errorf("invalid credential name '%s': %w", credential.Name, err) + return "", fmt.Errorf("invalid field '%s': %w", credential.Field, err) } + gsmSecretName := gsm.GetGSMSecretName(credential.Collection, credential.Group, credential.Field) secrets = append(secrets, config.Secret{ - ResourceName: fmt.Sprintf("projects/%s/secrets/%s__%s/versions/latest", GSMproject, credential.Collection, credential.Name), - FileName: fileName, // we want to mount the secret as a file named without the collection prefix + ResourceName: fmt.Sprintf("projects/%s/secrets/%s/versions/latest", GSMproject, gsmSecretName), + FileName: fileName, // we want to mount the secret as a file named without the collection__group prefix }) } secretsYaml, err := yaml.Marshal(secrets) @@ -50,22 +52,23 @@ func buildGCPSecretsParameter(credentials []api.CredentialReference) (string, er return string(secretsYaml), nil } -// replaceForbiddenSymbolsInCredentialName replaces all DotReplacementString substrings in credentialName with dot (.) symbol. -func replaceForbiddenSymbolsInCredentialName(credentialName string) (string, error) { +// restoreForbiddenSymbolsInSecretName replaces all replacement substrings with the original symbols, +// e.g. '--dot--awscreds' to '.awscreds' +func restoreForbiddenSymbolsInSecretName(s string) (string, error) { // This is an unfortunate workaround needed for the initial migration. // Google Secret Manager doesn't support dots in Secret names. Due to migration from Vault, - // where we had to shard each (multi key-value) secret into multiple ones in GSM, - // some secret names or their keys contained forbidden symbols, '.' (dot) and '/' (slash) in their names. - // All credentials with these forbidden symbols in their names or keys have been renamed, + // where we had to shard each (multi key-value) Vault secret into multiple ones in GSM, + // some of secret names, or their keys, contained forbidden symbols, usually '.' (dot) and '/' (slash) in their names. + // Because all credentials with these forbidden symbols in their names or keys have been renamed, // e.g. '.awscreds' to '--dot--awscreds', to preserve backwards compatibility, - // we need to mount the secret as the original '.awscreds' file to the Pod created by ci-operator. + // we now need to mount the secret as the original '.awscreds' file to the Pod that will be created by ci-operator. - replacedName := gsmvalidation.DenormalizeName(credentialName) + replacedName := gsmvalidation.DenormalizeName(s) re := regexp.MustCompile(`[^a-zA-Z0-9\-._/]`) invalidCharacters := re.FindAllString(replacedName, -1) if invalidCharacters != nil { - return "", fmt.Errorf("credential name '%s' decodes to '%s' which contains forbidden characters (%s); decoded names must only contain letters, numbers, dashes (-), dots (.), underscores (_), and slashes (/)", credentialName, replacedName, strings.Join(invalidCharacters, ", ")) + return "", fmt.Errorf("secret name '%s' decodes to '%s' which contains forbidden characters (%s); decoded names must only contain letters, numbers, dashes (-), dots (.), underscores (_), and slashes (/)", s, replacedName, strings.Join(invalidCharacters, ", ")) } else { return replacedName, nil } @@ -76,13 +79,13 @@ func getSPCName(namespace, collection, mountPath string, credentials []api.Crede var parts []string parts = append(parts, collection, mountPath) - // Sort credential names for deterministic hashing - var credNames []string + // Sort credential field names for deterministic hashing + var credFields []string for _, cred := range credentials { - credNames = append(credNames, cred.Name) + credFields = append(credFields, cred.Field) } - sort.Strings(credNames) - parts = append(parts, credNames...) + sort.Strings(credFields) + parts = append(parts, credFields...) hash := sha256.Sum256([]byte(strings.Join(parts, "-"))) hashStr := fmt.Sprintf("%x", hash[:12]) diff --git a/pkg/steps/multi_stage/csi_utils_test.go b/pkg/steps/multi_stage/csi_utils_test.go index e20d230edd7..fa64babf602 100644 --- a/pkg/steps/multi_stage/csi_utils_test.go +++ b/pkg/steps/multi_stage/csi_utils_test.go @@ -28,60 +28,60 @@ func TestGroupCredentialsByCollectionAndMountPath(t *testing.T) { { name: "single credential", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/cred1"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/cred1"}, }, expected: map[string][]api.CredentialReference{ "collection1:/tmp/cred1": { - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/cred1"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/cred1"}, }, }, }, { name: "multiple credentials different collections and paths", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/cred1"}, - {Name: "cred2", Collection: "collection2", MountPath: "/tmp/cred2"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/cred1"}, + {Collection: "collection2", Group: "group2", Field: "cred2", MountPath: "/tmp/cred2"}, }, expected: map[string][]api.CredentialReference{ "collection1:/tmp/cred1": { - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/cred1"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/cred1"}, }, "collection2:/tmp/cred2": { - {Name: "cred2", Collection: "collection2", MountPath: "/tmp/cred2"}, + {Collection: "collection2", Group: "group2", Field: "cred2", MountPath: "/tmp/cred2"}, }, }, }, { name: "multiple credentials same collection and path", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/shared"}, - {Name: "cred2", Collection: "collection1", MountPath: "/tmp/shared"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/shared"}, + {Collection: "collection1", Group: "group1", Field: "cred2", MountPath: "/tmp/shared"}, }, expected: map[string][]api.CredentialReference{ "collection1:/tmp/shared": { - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/shared"}, - {Name: "cred2", Collection: "collection1", MountPath: "/tmp/shared"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/shared"}, + {Collection: "collection1", Group: "group1", Field: "cred2", MountPath: "/tmp/shared"}, }, }, }, { name: "mixed grouping - some grouped together, some separate", credentials: []api.CredentialReference{ - {Name: "red", Collection: "colours", MountPath: "/tmp/path"}, - {Name: "blue", Collection: "colours", MountPath: "/tmp/path"}, - {Name: "circle", Collection: "shapes", MountPath: "/tmp/path"}, - {Name: "square", Collection: "shapes", MountPath: "/tmp/other"}, + {Collection: "colours", Group: "primary", Field: "red", MountPath: "/tmp/path"}, + {Collection: "colours", Group: "primary", Field: "blue", MountPath: "/tmp/path"}, + {Collection: "shapes", Group: "round", Field: "circle", MountPath: "/tmp/path"}, + {Collection: "shapes", Group: "angular", Field: "square", MountPath: "/tmp/other"}, }, expected: map[string][]api.CredentialReference{ "colours:/tmp/path": { - {Name: "red", Collection: "colours", MountPath: "/tmp/path"}, - {Name: "blue", Collection: "colours", MountPath: "/tmp/path"}, + {Collection: "colours", Group: "primary", Field: "red", MountPath: "/tmp/path"}, + {Collection: "colours", Group: "primary", Field: "blue", MountPath: "/tmp/path"}, }, "shapes:/tmp/path": { - {Name: "circle", Collection: "shapes", MountPath: "/tmp/path"}, + {Collection: "shapes", Group: "round", Field: "circle", MountPath: "/tmp/path"}, }, "shapes:/tmp/other": { - {Name: "square", Collection: "shapes", MountPath: "/tmp/other"}, + {Collection: "shapes", Group: "angular", Field: "square", MountPath: "/tmp/other"}, }, }, }, @@ -111,11 +111,11 @@ func TestBuildGCPSecretsParameter(t *testing.T) { { name: "single credential", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1"}, + {Collection: "collection1", Group: "group1", Field: "cred1"}, }, expected: []config.Secret{ { - ResourceName: fmt.Sprintf("projects/%s/secrets/collection1__cred1/versions/latest", GSMproject), + ResourceName: fmt.Sprintf("projects/%s/secrets/collection1__group1__cred1/versions/latest", GSMproject), FileName: "cred1", }, }, @@ -123,16 +123,16 @@ func TestBuildGCPSecretsParameter(t *testing.T) { { name: "multiple credentials", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1"}, - {Name: "cred2", Collection: "collection2"}, + {Collection: "collection1", Group: "group1", Field: "cred1"}, + {Collection: "collection2", Group: "group2", Field: "cred2"}, }, expected: []config.Secret{ { - ResourceName: fmt.Sprintf("projects/%s/secrets/collection1__cred1/versions/latest", GSMproject), + ResourceName: fmt.Sprintf("projects/%s/secrets/collection1__group1__cred1/versions/latest", GSMproject), FileName: "cred1", }, { - ResourceName: fmt.Sprintf("projects/%s/secrets/collection2__cred2/versions/latest", GSMproject), + ResourceName: fmt.Sprintf("projects/%s/secrets/collection2__group2__cred2/versions/latest", GSMproject), FileName: "cred2", }, }, @@ -173,7 +173,7 @@ func TestGetSPCName(t *testing.T) { collection: "collection1", mountPath: "/tmp/cred1", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/cred1"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/cred1"}, }, }, { @@ -182,7 +182,7 @@ func TestGetSPCName(t *testing.T) { collection: "collection1", mountPath: "/tmp/cred1", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/cred1"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/cred1"}, }, }, { @@ -191,8 +191,8 @@ func TestGetSPCName(t *testing.T) { collection: "collection1", mountPath: "/tmp/shared", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/shared"}, - {Name: "cred2", Collection: "collection1", MountPath: "/tmp/shared"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/shared"}, + {Collection: "collection1", Group: "group1", Field: "cred2", MountPath: "/tmp/shared"}, }, }, { @@ -201,7 +201,7 @@ func TestGetSPCName(t *testing.T) { collection: "collection1", mountPath: "/tmp/shared", credentials: []api.CredentialReference{ - {Name: "cred1", Collection: "collection1", MountPath: "/tmp/shared"}, + {Collection: "collection1", Group: "group1", Field: "cred1", MountPath: "/tmp/shared"}, }, }, } @@ -238,16 +238,16 @@ func TestGetSPCNameUniqueness(t *testing.T) { credentials []api.CredentialReference }{ {"collection1", "/tmp/cred1", []api.CredentialReference{ - {Name: "secret1", Collection: "collection1", MountPath: "/tmp/cred1"}, + {Collection: "collection1", Group: "group1", Field: "secret1", MountPath: "/tmp/cred1"}, }}, {"collection1", "/tmp/cred2", []api.CredentialReference{ - {Name: "secret2", Collection: "collection1", MountPath: "/tmp/cred2"}, + {Collection: "collection1", Group: "group1", Field: "secret2", MountPath: "/tmp/cred2"}, }}, {"collection2", "/tmp/cred1", []api.CredentialReference{ - {Name: "secret1", Collection: "collection2", MountPath: "/tmp/cred1"}, + {Collection: "collection2", Group: "group2", Field: "secret1", MountPath: "/tmp/cred1"}, }}, {"collection2", "/tmp/cred2", []api.CredentialReference{ - {Name: "secret2", Collection: "collection2", MountPath: "/tmp/cred2"}, + {Collection: "collection2", Group: "group2", Field: "secret2", MountPath: "/tmp/cred2"}, }}, } @@ -280,12 +280,12 @@ func TestGetSPCNameCollisionPrevention(t *testing.T) { // Two different sets of credentials with same collection and mountPath credentials1 := []api.CredentialReference{ - {Name: "red", Collection: collection, MountPath: mountPath}, - {Name: "blue", Collection: collection, MountPath: mountPath}, + {Collection: collection, Group: "primary", Field: "red", MountPath: mountPath}, + {Collection: collection, Group: "primary", Field: "blue", MountPath: mountPath}, } credentials2 := []api.CredentialReference{ - {Name: "red", Collection: collection, MountPath: mountPath}, + {Collection: collection, Group: "primary", Field: "red", MountPath: mountPath}, } // They should get different SPC names @@ -525,7 +525,7 @@ func TestReplaceForbiddenSymbolsInCredentialName(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result, err := replaceForbiddenSymbolsInCredentialName(tc.secretName) + result, err := restoreForbiddenSymbolsInSecretName(tc.secretName) if tc.expectError { if err == nil { diff --git a/pkg/steps/multi_stage/gen_test.go b/pkg/steps/multi_stage/gen_test.go index c3b523f7e84..39269082999 100644 --- a/pkg/steps/multi_stage/gen_test.go +++ b/pkg/steps/multi_stage/gen_test.go @@ -137,7 +137,7 @@ func TestGeneratePods(t *testing.T) { t.Parallel() js := jobSpec() - step := newMultiStageTestStep(tc.config.Tests[0], tc.config, nil, nil, &js, nil, "node-name", "", nil, false) + step := newMultiStageTestStep(tc.config.Tests[0], tc.config, nil, nil, &js, nil, "node-name", "", nil, false, nil) step.test[0].Resources = resourceRequirements ret, _, err := step.generatePods(tc.config.Tests[0].MultiStageTestConfigurationLiteral.Test, tc.env, tc.secretVolumes, tc.secretVolumeMounts, nil) @@ -204,7 +204,7 @@ func TestGenerateObservers(t *testing.T) { }, } jobSpec.SetNamespace("namespace") - step := newMultiStageTestStep(config.Tests[0], &config, nil, nil, &jobSpec, nil, "node-name", "", nil, false) + step := newMultiStageTestStep(config.Tests[0], &config, nil, nil, &jobSpec, nil, "node-name", "", nil, false, nil) ret, err := step.generateObservers(observers, nil, nil, nil) if err != nil { t.Fatal(err) @@ -278,7 +278,7 @@ func TestGeneratePodsEnvironment(t *testing.T) { Test: test, Environment: tc.env, }, - }, &api.ReleaseBuildConfiguration{}, nil, nil, &jobSpec, nil, "node-name", "", nil, false) + }, &api.ReleaseBuildConfiguration{}, nil, nil, &jobSpec, nil, "node-name", "", nil, false, nil) pods, _, err := step.(*multiStageTestStep).generatePods(test, nil, nil, nil, nil) if err != nil { t.Fatal(err) @@ -346,7 +346,7 @@ func TestGeneratePodBestEffort(t *testing.T) { }, } jobSpec.SetNamespace("namespace") - step := newMultiStageTestStep(config.Tests[0], &config, nil, nil, &jobSpec, nil, "node-name", "", nil, false) + step := newMultiStageTestStep(config.Tests[0], &config, nil, nil, &jobSpec, nil, "node-name", "", nil, false, nil) _, bestEffortSteps, err := step.generatePods(config.Tests[0].MultiStageTestConfigurationLiteral.Post, nil, nil, nil, nil) if err != nil { t.Fatal(err) diff --git a/pkg/steps/multi_stage/gsm_bundle_resolver.go b/pkg/steps/multi_stage/gsm_bundle_resolver.go new file mode 100644 index 00000000000..cc7ff80a225 --- /dev/null +++ b/pkg/steps/multi_stage/gsm_bundle_resolver.go @@ -0,0 +1,217 @@ +package multi_stage + +import ( + "context" + "fmt" + + "github.com/sirupsen/logrus" + + utilerrors "k8s.io/apimachinery/pkg/util/errors" + + "github.com/openshift/ci-tools/pkg/api" + gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" +) + +type collectionGroupKey struct { + collection string + group string +} + +// ValidateNoGroupCollisionsOnMountPath ensures that different groups within the same collection +// don't share a mount path, which could cause file name collisions. +// +// Example of invalid configuration: +// - collection: my-creds, group: aws, field: access-key, mount_path: /tmp/secrets +// - collection: my-creds, group: gcp, field: access-key, mount_path: /tmp/secrets +// +// Both would try to create /tmp/secrets/access-key +func ValidateNoGroupCollisionsOnMountPath(credentials []api.CredentialReference) error { + type collectionMountKey struct { + collection string + mountPath string + } + mountPathGroups := make(map[collectionMountKey]map[string]bool) + + for _, cred := range credentials { + key := collectionMountKey{ + collection: cred.Collection, + mountPath: cred.MountPath, + } + if mountPathGroups[key] == nil { + mountPathGroups[key] = make(map[string]bool) + } + mountPathGroups[key][cred.Group] = true + } + + for key, groups := range mountPathGroups { + if len(groups) > 1 { + var groupList []string + for group := range groups { + groupList = append(groupList, group) + } + return fmt.Errorf("multiple groups (%v) found for collection=%s, mount_path=%s - different groups in the same collection must use different mount paths to avoid file name collisions", + groupList, key.collection, key.mountPath) + } + } + return nil +} + +func getBundle(config *api.GSMConfig, name string) *api.GSMBundle { + for i := range config.Bundles { + if config.Bundles[i].Name == name { + return &config.Bundles[i] + } + } + return nil +} + +// ResolveCredentialReferences resolves bundle references and auto-discovery +// to concrete (collection, group, field) tuples. +// +// Handles three cases of possible credential entries: +// 1. GSMBundle reference -> expands to all secrets in bundle +// 2. Auto-discovery (collection+group, no field) -> discovers relevant fields from GSM +// 3. Explicit field -> passes through unchanged +func ResolveCredentialReferences( + ctx context.Context, + credentials []api.CredentialReference, + gsmConfig *api.GSMConfig, + gsmClient gsm.SecretManagerClient, + gsmProjectConfig gsm.Config, +) ([]api.CredentialReference, error) { + var resolvedCredentials []api.CredentialReference + var errs []error + + // Track discovered fields to avoid redundant GSM calls + discoveredFields := make(map[collectionGroupKey][]string) + + for _, cred := range credentials { + if cred.IsBundleReference() { + if gsmConfig == nil { + errs = append(errs, fmt.Errorf("bundle reference %q requires gsm-config file, but config is not loaded", cred.Bundle)) + break + } + bundle := getBundle(gsmConfig, cred.Bundle) + if bundle == nil { + errs = append(errs, fmt.Errorf("bundle %q not found in config file", cred.Bundle)) + break + } + + expanded, err := expandBundle(ctx, bundle, gsmClient, gsmProjectConfig, discoveredFields) + if err != nil { + errs = append(errs, fmt.Errorf("failed to expand bundle %q: %w", cred.Bundle, err)) + break + } + + for _, r := range expanded { + resolvedCredentials = append(resolvedCredentials, api.CredentialReference{ + Collection: r.Collection, + Group: r.Group, + Field: r.Field, + MountPath: cred.MountPath, + }) + } + + } else if cred.IsAutoDiscovery() { + if gsmClient == nil { + errs = append(errs, fmt.Errorf("auto-discovery for %s__%s requires GSM client, but client is not initialized", cred.Collection, cred.Group)) + break + } + key := collectionGroupKey{ + cred.Collection, + cred.Group, + } + if _, found := discoveredFields[key]; !found { + fieldNames, err := gsm.ListSecretFieldsByCollectionAndGroup( + ctx, gsmClient, gsmProjectConfig, cred.Collection, cred.Group) + if err != nil { + errs = append(errs, fmt.Errorf("auto-discovery failed for %s__%s: %w", + cred.Collection, cred.Group, err)) + break // if any credential is not correctly set up or available in GSM, we want to fail the whole test + } + discoveredFields[key] = fieldNames + } + + for _, field := range discoveredFields[key] { + resolvedCredentials = append(resolvedCredentials, api.CredentialReference{ + Collection: cred.Collection, + Group: cred.Group, + Field: field, + MountPath: cred.MountPath, + }) + } + + } else if cred.IsExplicitField() { + resolvedCredentials = append(resolvedCredentials, api.CredentialReference{ + Collection: cred.Collection, + Group: cred.Group, + Field: cred.Field, + MountPath: cred.MountPath, + }) + } else { + errs = append(errs, fmt.Errorf("invalid credential reference: must provide bundle, collection+group, or collection+group+field")) + break + } + } + + if len(errs) > 0 { + return nil, utilerrors.NewAggregate(errs) + } + + return resolvedCredentials, nil +} + +// expandBundle expands a bundle definition into individual credential references. +// If fields are not specified for a secret entry, it uses auto-discovery to list all fields. +func expandBundle( + ctx context.Context, + bundle *api.GSMBundle, + gsmClient gsm.SecretManagerClient, + gsmProjectConfig gsm.Config, + discoveredFields map[collectionGroupKey][]string, +) ([]api.CredentialReference, error) { + var resolvedCredentials []api.CredentialReference + + for _, secretEntry := range bundle.GSMSecrets { + if len(secretEntry.Fields) == 0 { // if fields are not specified, discover them using GSM listing + if gsmClient == nil { + return nil, fmt.Errorf("bundle auto-discovery for %s__%s requires GSM client, but client is not initialized", secretEntry.Collection, secretEntry.Group) + } + key := collectionGroupKey{ + secretEntry.Collection, + secretEntry.Group, + } + + // check if we've already discovered fields for this collection+group + if _, alreadyDiscovered := discoveredFields[key]; !alreadyDiscovered { + fieldNames, err := gsm.ListSecretFieldsByCollectionAndGroup( + ctx, gsmClient, gsmProjectConfig, secretEntry.Collection, secretEntry.Group) + if err != nil { + return nil, fmt.Errorf("failed to list fields for collection=%s, group=%s: %w", + secretEntry.Collection, secretEntry.Group, err) + } + discoveredFields[key] = fieldNames + logrus.Debugf("discovered %d fields for collection=%s, group=%s", len(fieldNames), secretEntry.Collection, secretEntry.Group) + } + + for _, fieldName := range discoveredFields[key] { + resolvedCredentials = append(resolvedCredentials, api.CredentialReference{ + Collection: secretEntry.Collection, + Group: secretEntry.Group, + Field: fieldName, + }) + } + } else { + // use explicit fields + for _, fieldEntry := range secretEntry.Fields { + resolvedCredentials = append(resolvedCredentials, api.CredentialReference{ + Collection: secretEntry.Collection, + Group: secretEntry.Group, + Field: fieldEntry.Name, + }) + } + } + } + + return resolvedCredentials, nil +} diff --git a/pkg/steps/multi_stage/init.go b/pkg/steps/multi_stage/init.go index c956e4b34ab..26f5f73df79 100644 --- a/pkg/steps/multi_stage/init.go +++ b/pkg/steps/multi_stage/init.go @@ -18,6 +18,7 @@ import ( csiapi "sigs.k8s.io/secrets-store-csi-driver/apis/v1" "github.com/openshift/ci-tools/pkg/api" + gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/util" ) @@ -87,9 +88,30 @@ func (s *multiStageTestStep) createCredentials(ctx context.Context) error { func (s *multiStageTestStep) createSPCs(ctx context.Context) error { spcsToCreate := map[string]*csiapi.SecretProviderClass{} + // (Crucial) Resolve credential references (bundles, auto-discovery, explicit fields) BEFORE creating SPCs + // This expands bundle references and auto-discovers fields for each step + allSteps := append(append(s.pre, s.test...), s.post...) + for i := range allSteps { + step := &allSteps[i] + resolvedCredentials, err := ResolveCredentialReferences( + ctx, + step.Credentials, + s.gsm.Config, + s.gsm.Client, + s.gsm.ProjectConfig, + ) + if err != nil { + return fmt.Errorf("failed to resolve credentials for step %s: %w", step.As, err) + } + if err := ValidateNoGroupCollisionsOnMountPath(resolvedCredentials); err != nil { + return fmt.Errorf("invalid credentials for step %s: %w", step.As, err) + } + step.Credentials = resolvedCredentials + } + logrus.Infof("Creating SPCs for actual credential usage...") // Create grouped SPCs for actual credential usage - for _, step := range append(s.pre, append(s.test, s.post...)...) { + for _, step := range allSteps { collectionMountGroups := groupCredentialsByCollectionAndMountPath(step.Credentials) for _, credentials := range collectionMountGroups { @@ -105,20 +127,21 @@ func (s *multiStageTestStep) createSPCs(ctx context.Context) error { // Create SPCs for sidecar censoring; each credential gets its own SPC. logrus.Infof("Creating SPCs for sidecar censoring...") seenCredentials := make(map[string]bool) - for _, step := range append(s.pre, append(s.test, s.post...)...) { + for _, step := range allSteps { for _, credential := range step.Credentials { - if seenCredentials[credential.Name] { + fullSecretName := gsm.GetGSMSecretName(credential.Collection, credential.Group, credential.Field) + if seenCredentials[fullSecretName] { continue } - seenCredentials[credential.Name] = true + seenCredentials[fullSecretName] = true - censorMountPath := getCensorMountPath(credential.Name) //arbitrary mount path for uniqueness + censorMountPath := getCensorMountPath(fullSecretName) //arbitrary mount path for uniqueness individualCredentials := []api.CredentialReference{credential} spcName := getSPCName(s.jobSpec.Namespace(), credential.Collection, censorMountPath, individualCredentials) secrets, err := buildGCPSecretsParameter(individualCredentials) if err != nil { - return fmt.Errorf("could not marshal secrets for censored credential %s: %w", credential.Name, err) + return fmt.Errorf("could not marshal secrets for censored credential %s: %w", fullSecretName, err) } spcsToCreate[spcName] = buildSecretProviderClass(spcName, s.jobSpec.Namespace(), secrets) } diff --git a/pkg/steps/multi_stage/init_test.go b/pkg/steps/multi_stage/init_test.go index a472c48a739..076c2a9faec 100644 --- a/pkg/steps/multi_stage/init_test.go +++ b/pkg/steps/multi_stage/init_test.go @@ -19,6 +19,7 @@ import ( csiapi "sigs.k8s.io/secrets-store-csi-driver/apis/v1" "github.com/openshift/ci-tools/pkg/api" + gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" "github.com/openshift/ci-tools/pkg/steps/loggingclient" "github.com/openshift/ci-tools/pkg/testhelper" testhelper_kube "github.com/openshift/ci-tools/pkg/testhelper/kubernetes" @@ -60,8 +61,8 @@ func TestCreateSPCs(t *testing.T) { scheme := runtime.NewScheme() _ = csiapi.AddToScheme(scheme) - credential1 := api.CredentialReference{Name: "credential1", Collection: "test", MountPath: "/tmp/path1"} - credential2 := api.CredentialReference{Name: "credential2", Collection: "test-2", MountPath: "/tmp/path2"} + credential1 := api.CredentialReference{Collection: "test", Group: "group1", Field: "credential1", MountPath: "/tmp/path1"} + credential2 := api.CredentialReference{Collection: "test-2", Group: "group2", Field: "credential2", MountPath: "/tmp/path2"} newGroupedSPC := func(collection, mountPath, ns string, credentials []api.CredentialReference) csiapi.SecretProviderClass { secret, _ := buildGCPSecretsParameter(credentials) @@ -71,11 +72,12 @@ func TestCreateSPCs(t *testing.T) { return *spc } - newCensoringSPC := func(collection, credName, ns string) csiapi.SecretProviderClass { - credential := api.CredentialReference{Name: credName, Collection: collection} + newCensoringSPC := func(collection, group, field, ns string) csiapi.SecretProviderClass { + credential := api.CredentialReference{Collection: collection, Group: group, Field: field} credentials := []api.CredentialReference{credential} secret, _ := buildGCPSecretsParameter(credentials) - censorMountPath := fmt.Sprintf("/censor/%s", credName) + fullSecretName := gsm.GetGSMSecretName(collection, group, field) + censorMountPath := fmt.Sprintf("/censor/%s", fullSecretName) spc := buildSecretProviderClass(getSPCName(ns, collection, censorMountPath, credentials), ns, secret) // Set ResourceVersion for fake client compatibility @@ -101,7 +103,7 @@ func TestCreateSPCs(t *testing.T) { // Grouped SPC for the credential at its mount path newGroupedSPC(credential1.Collection, credential1.MountPath, "test-ns", []api.CredentialReference{credential1}), // Individual SPC for censoring - newCensoringSPC(credential1.Collection, credential1.Name, "test-ns"), + newCensoringSPC(credential1.Collection, credential1.Group, credential1.Field, "test-ns"), }, }, }, @@ -115,24 +117,28 @@ func TestCreateSPCs(t *testing.T) { newGroupedSPC(credential1.Collection, credential1.MountPath, "test-ns", []api.CredentialReference{credential1}), newGroupedSPC(credential2.Collection, credential2.MountPath, "test-ns", []api.CredentialReference{credential2}), // Individual censoring SPCs - newCensoringSPC(credential1.Collection, credential1.Name, "test-ns"), - newCensoringSPC(credential2.Collection, credential2.Name, "test-ns"), + newCensoringSPC(credential1.Collection, credential1.Group, credential1.Field, "test-ns"), + newCensoringSPC(credential2.Collection, credential2.Group, credential2.Field, "test-ns"), }, }, }, { name: "credentials with same collection and path grouped", - pre: []api.LiteralTestStep{{Credentials: []api.CredentialReference{credential1, {Name: "credential3", Collection: "test", MountPath: "/tmp/path1"}}}}, + pre: []api.LiteralTestStep{{ + Credentials: []api.CredentialReference{ + credential1, + {Collection: "test", Group: "group1", Field: "credential3", MountPath: "/tmp/path1"}, + }}}, expectedSPCs: csiapi.SecretProviderClassList{ Items: []csiapi.SecretProviderClass{ // Grouped SPC with both credentials at same path newGroupedSPC("test", "/tmp/path1", "test-ns", []api.CredentialReference{ credential1, - {Name: "credential3", Collection: "test", MountPath: "/tmp/path1"}, + {Collection: "test", Group: "group1", Field: "credential3", MountPath: "/tmp/path1"}, }), // Individual censoring SPCs - newCensoringSPC(credential1.Collection, credential1.Name, "test-ns"), - newCensoringSPC("test", "credential3", "test-ns"), + newCensoringSPC(credential1.Collection, credential1.Group, credential1.Field, "test-ns"), + newCensoringSPC("test", "group1", "credential3", "test-ns"), }, }, }, @@ -148,11 +154,19 @@ func TestCreateSPCs(t *testing.T) { FakePodExecutor: crclient, } step := &multiStageTestStep{ - pre: tc.pre, - test: tc.test, - post: tc.post, - jobSpec: &api.JobSpec{}, - client: fakeClient, + pre: tc.pre, + test: tc.test, + post: tc.post, + jobSpec: &api.JobSpec{}, + client: fakeClient, + enableSecretsStoreCSIDriver: true, + gsm: &GSMConfiguration{ + Config: &api.GSMConfig{}, + ProjectConfig: gsm.Config{ + ProjectIdString: "test-project", + ProjectIdNumber: "123456", + }, + }, } step.jobSpec.SetNamespace("test-ns") err := step.createSPCs(context.TODO()) diff --git a/pkg/steps/multi_stage/multi_stage.go b/pkg/steps/multi_stage/multi_stage.go index b34c8402f92..de9a7263fae 100644 --- a/pkg/steps/multi_stage/multi_stage.go +++ b/pkg/steps/multi_stage/multi_stage.go @@ -8,6 +8,7 @@ import ( "sync" "time" + secretmanager "cloud.google.com/go/secretmanager/apiv1" "github.com/sirupsen/logrus" coreapi "k8s.io/api/core/v1" @@ -17,6 +18,7 @@ import ( "sigs.k8s.io/yaml" "github.com/openshift/ci-tools/pkg/api" + gsm "github.com/openshift/ci-tools/pkg/gsm-secrets" "github.com/openshift/ci-tools/pkg/junit" "github.com/openshift/ci-tools/pkg/kubernetes" "github.com/openshift/ci-tools/pkg/results" @@ -43,6 +45,19 @@ type vpnConf struct { namespaceUID int64 } +// GSMConfiguration contains all Google Secret Manager (GSM) related configuration +// needed for multi-stage test execution. +type GSMConfiguration struct { + // Config contains bundle and secret definitions from gsm-config.yaml file + Config *api.GSMConfig + // CredentialsFile is the path to the GSM service account credentials + CredentialsFile string + // Client is the initialized Google Secret Manager client + Client *secretmanager.Client + // ProjectConfig contains GSM project metadata (ID and number) + ProjectConfig gsm.Config +} + const ( // A test failure should terminate the current phase. // Set for `pre` and `test`, unset for `post`. @@ -106,6 +121,7 @@ type multiStageTestStep struct { cancelObservers func(context.CancelFunc) nodeArchitecture api.NodeArchitecture enableSecretsStoreCSIDriver bool + gsm *GSMConfiguration requireNestedPodman bool } @@ -120,8 +136,9 @@ func MultiStageTestStep( targetAdditionalSuffix string, cancelObservers func(context.CancelFunc), enableSecretsStoreCSIDriver bool, + gsm *GSMConfiguration, ) api.Step { - return newMultiStageTestStep(testConfig, config, params, client, jobSpec, leases, nodeName, targetAdditionalSuffix, cancelObservers, enableSecretsStoreCSIDriver) + return newMultiStageTestStep(testConfig, config, params, client, jobSpec, leases, nodeName, targetAdditionalSuffix, cancelObservers, enableSecretsStoreCSIDriver, gsm) } func newMultiStageTestStep( @@ -135,6 +152,7 @@ func newMultiStageTestStep( targetAdditionalSuffix string, cancelObservers func(context.CancelFunc), enableSecretsStoreCSIDriver bool, + gsm *GSMConfiguration, ) *multiStageTestStep { ms := testConfig.MultiStageTestConfigurationLiteral var flags stepFlag @@ -165,6 +183,7 @@ func newMultiStageTestStep( cancelObservers: cancelObservers, nodeArchitecture: testConfig.NodeArchitecture, enableSecretsStoreCSIDriver: enableSecretsStoreCSIDriver, + gsm: gsm, } s.requireNestedPodman = stepRequiresNestedPodman(s) @@ -204,6 +223,11 @@ func (s *multiStageTestStep) run(ctx context.Context) error { return fmt.Errorf("failed to create secret: %w", err) } if s.enableSecretsStoreCSIDriver { + if s.gsm == nil || s.gsm.Client == nil { + return fmt.Errorf("GSM client was not initialized - credentials file may be missing") + } + defer s.gsm.Client.Close() + logrus.Info("Using initialized GSM client") if err := s.createSPCs(ctx); err != nil { return fmt.Errorf("failed to create SecretProviderClass objects: %w", err) } diff --git a/pkg/steps/multi_stage/multi_stage_test.go b/pkg/steps/multi_stage/multi_stage_test.go index 91d77319257..00b399c9fe5 100644 --- a/pkg/steps/multi_stage/multi_stage_test.go +++ b/pkg/steps/multi_stage/multi_stage_test.go @@ -81,7 +81,7 @@ func TestRequires(t *testing.T) { As: "some-e2e", ClusterClaim: tc.clusterClaim, MultiStageTestConfigurationLiteral: &tc.steps, - }, &tc.config, api.NewDeferredParameters(nil), nil, nil, nil, "node-name", "", nil, false) + }, &tc.config, api.NewDeferredParameters(nil), nil, nil, nil, "node-name", "", nil, false, nil) ret := step.Requires() if len(ret) == len(tc.req) { matches := true diff --git a/pkg/steps/multi_stage/run_test.go b/pkg/steps/multi_stage/run_test.go index 1564b542501..66cc6ed208c 100644 --- a/pkg/steps/multi_stage/run_test.go +++ b/pkg/steps/multi_stage/run_test.go @@ -125,7 +125,7 @@ func TestRun(t *testing.T) { Observers: tc.observers, AllowSkipOnSuccess: &yes, }, - }, &api.ReleaseBuildConfiguration{}, nil, client, &jobSpec, nil, "node-name", "", func(cf context.CancelFunc) {}, false) + }, &api.ReleaseBuildConfiguration{}, nil, client, &jobSpec, nil, "node-name", "", func(cf context.CancelFunc) {}, false, nil) // An Observer pod failure doesn't make the test fail failures := tc.failures.Delete(observerPodNames.UnsortedList()...) @@ -261,7 +261,7 @@ func TestJUnit(t *testing.T) { Test: []api.LiteralTestStep{{As: "test0"}, {As: "test1"}}, Post: []api.LiteralTestStep{{As: "post0"}, {As: "post1"}}, }, - }, &api.ReleaseBuildConfiguration{}, nil, client, &jobSpec, nil, "node-name", "", nil, false) + }, &api.ReleaseBuildConfiguration{}, nil, client, &jobSpec, nil, "node-name", "", nil, false, nil) if err := step.Run(context.Background()); tc.failures == nil && err != nil { t.Error(err) return diff --git a/pkg/webreg/zz_generated.ci_operator_reference.go b/pkg/webreg/zz_generated.ci_operator_reference.go index 72134ea0a69..2c04cbfae61 100644 --- a/pkg/webreg/zz_generated.ci_operator_reference.go +++ b/pkg/webreg/zz_generated.ci_operator_reference.go @@ -708,10 +708,17 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " # Credentials defines the credentials we'll mount into this step.\n" + " credentials:\n" + - " - # Collection is the name of the collection the secret belongs to.\n" + - " # In GCP, the secret is named __ -- this represents\n" + - " # the part.\n" + + " - # Bundle is a named bundle reference from the GSM config mapping file.\n" + + " # Mutually exclusive with Collection/Group/Field.\n" + + " bundle: ' '\n" + + " # Collection is the name of the collection the secret belongs to (first level of 3-level ____ hierarchy).\n" + " collection: ' '\n" + + " # Field is the specific field name (third level of 3-level ____ hierarchy).\n" + + " # If omitted, all fields in the collection/group are auto-discovered.\n" + + " field: ' '\n" + + " # Group is the group name within the collection (second level of 3-level ____ hierarchy).\n" + + " # Required when Collection is set, unless Bundle is used.\n" + + " group: ' '\n" + " # MountPath is where the secret should be mounted.\n" + " mount_path: ' '\n" + " # Name is the name of the secret, without the collection prefix.\n" + @@ -807,10 +814,17 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " # Credentials defines the credentials we'll mount into this step.\n" + " credentials:\n" + - " - # Collection is the name of the collection the secret belongs to.\n" + - " # In GCP, the secret is named __ -- this represents\n" + - " # the part.\n" + + " - # Bundle is a named bundle reference from the GSM config mapping file.\n" + + " # Mutually exclusive with Collection/Group/Field.\n" + + " bundle: ' '\n" + + " # Collection is the name of the collection the secret belongs to (first level of 3-level ____ hierarchy).\n" + " collection: ' '\n" + + " # Field is the specific field name (third level of 3-level ____ hierarchy).\n" + + " # If omitted, all fields in the collection/group are auto-discovered.\n" + + " field: ' '\n" + + " # Group is the group name within the collection (second level of 3-level ____ hierarchy).\n" + + " # Required when Collection is set, unless Bundle is used.\n" + + " group: ' '\n" + " # MountPath is where the secret should be mounted.\n" + " mount_path: ' '\n" + " # Name is the name of the secret, without the collection prefix.\n" + @@ -906,10 +920,17 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " # Credentials defines the credentials we'll mount into this step.\n" + " credentials:\n" + - " - # Collection is the name of the collection the secret belongs to.\n" + - " # In GCP, the secret is named __ -- this represents\n" + - " # the part.\n" + + " - # Bundle is a named bundle reference from the GSM config mapping file.\n" + + " # Mutually exclusive with Collection/Group/Field.\n" + + " bundle: ' '\n" + + " # Collection is the name of the collection the secret belongs to (first level of 3-level ____ hierarchy).\n" + " collection: ' '\n" + + " # Field is the specific field name (third level of 3-level ____ hierarchy).\n" + + " # If omitted, all fields in the collection/group are auto-discovered.\n" + + " field: ' '\n" + + " # Group is the group name within the collection (second level of 3-level ____ hierarchy).\n" + + " # Required when Collection is set, unless Bundle is used.\n" + + " group: ' '\n" + " # MountPath is where the secret should be mounted.\n" + " mount_path: ' '\n" + " # Name is the name of the secret, without the collection prefix.\n" + @@ -1142,7 +1163,10 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " credentials:\n" + " # LiteralTestStep is a full test step definition.\n" + - " - collection: ' '\n" + + " - bundle: ' '\n" + + " collection: ' '\n" + + " field: ' '\n" + + " group: ' '\n" + " mount_path: ' '\n" + " name: ' '\n" + " namespace: ' '\n" + @@ -1209,7 +1233,10 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " credentials:\n" + " # LiteralTestStep is a full test step definition.\n" + - " - collection: ' '\n" + + " - bundle: ' '\n" + + " collection: ' '\n" + + " field: ' '\n" + + " group: ' '\n" + " mount_path: ' '\n" + " name: ' '\n" + " namespace: ' '\n" + @@ -1276,7 +1303,10 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " credentials:\n" + " # LiteralTestStep is a full test step definition.\n" + - " - collection: ' '\n" + + " - bundle: ' '\n" + + " collection: ' '\n" + + " field: ' '\n" + + " group: ' '\n" + " mount_path: ' '\n" + " name: ' '\n" + " namespace: ' '\n" + @@ -1609,10 +1639,17 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " # Credentials defines the credentials we'll mount into this step.\n" + " credentials:\n" + - " - # Collection is the name of the collection the secret belongs to.\n" + - " # In GCP, the secret is named __ -- this represents\n" + - " # the part.\n" + + " - # Bundle is a named bundle reference from the GSM config mapping file.\n" + + " # Mutually exclusive with Collection/Group/Field.\n" + + " bundle: ' '\n" + + " # Collection is the name of the collection the secret belongs to (first level of 3-level ____ hierarchy).\n" + " collection: ' '\n" + + " # Field is the specific field name (third level of 3-level ____ hierarchy).\n" + + " # If omitted, all fields in the collection/group are auto-discovered.\n" + + " field: ' '\n" + + " # Group is the group name within the collection (second level of 3-level ____ hierarchy).\n" + + " # Required when Collection is set, unless Bundle is used.\n" + + " group: ' '\n" + " # MountPath is where the secret should be mounted.\n" + " mount_path: ' '\n" + " # Name is the name of the secret, without the collection prefix.\n" + @@ -1708,10 +1745,17 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " # Credentials defines the credentials we'll mount into this step.\n" + " credentials:\n" + - " - # Collection is the name of the collection the secret belongs to.\n" + - " # In GCP, the secret is named __ -- this represents\n" + - " # the part.\n" + + " - # Bundle is a named bundle reference from the GSM config mapping file.\n" + + " # Mutually exclusive with Collection/Group/Field.\n" + + " bundle: ' '\n" + + " # Collection is the name of the collection the secret belongs to (first level of 3-level ____ hierarchy).\n" + " collection: ' '\n" + + " # Field is the specific field name (third level of 3-level ____ hierarchy).\n" + + " # If omitted, all fields in the collection/group are auto-discovered.\n" + + " field: ' '\n" + + " # Group is the group name within the collection (second level of 3-level ____ hierarchy).\n" + + " # Required when Collection is set, unless Bundle is used.\n" + + " group: ' '\n" + " # MountPath is where the secret should be mounted.\n" + " mount_path: ' '\n" + " # Name is the name of the secret, without the collection prefix.\n" + @@ -1807,10 +1851,17 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " # Credentials defines the credentials we'll mount into this step.\n" + " credentials:\n" + - " - # Collection is the name of the collection the secret belongs to.\n" + - " # In GCP, the secret is named __ -- this represents\n" + - " # the part.\n" + + " - # Bundle is a named bundle reference from the GSM config mapping file.\n" + + " # Mutually exclusive with Collection/Group/Field.\n" + + " bundle: ' '\n" + + " # Collection is the name of the collection the secret belongs to (first level of 3-level ____ hierarchy).\n" + " collection: ' '\n" + + " # Field is the specific field name (third level of 3-level ____ hierarchy).\n" + + " # If omitted, all fields in the collection/group are auto-discovered.\n" + + " field: ' '\n" + + " # Group is the group name within the collection (second level of 3-level ____ hierarchy).\n" + + " # Required when Collection is set, unless Bundle is used.\n" + + " group: ' '\n" + " # MountPath is where the secret should be mounted.\n" + " mount_path: ' '\n" + " # Name is the name of the secret, without the collection prefix.\n" + @@ -2043,7 +2094,10 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " credentials:\n" + " # LiteralTestStep is a full test step definition.\n" + - " - collection: ' '\n" + + " - bundle: ' '\n" + + " collection: ' '\n" + + " field: ' '\n" + + " group: ' '\n" + " mount_path: ' '\n" + " name: ' '\n" + " namespace: ' '\n" + @@ -2110,7 +2164,10 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " credentials:\n" + " # LiteralTestStep is a full test step definition.\n" + - " - collection: ' '\n" + + " - bundle: ' '\n" + + " collection: ' '\n" + + " field: ' '\n" + + " group: ' '\n" + " mount_path: ' '\n" + " name: ' '\n" + " namespace: ' '\n" + @@ -2177,7 +2234,10 @@ const ciOperatorReferenceYaml = "# The list of base images describe\n" + " commands: ' '\n" + " credentials:\n" + " # LiteralTestStep is a full test step definition.\n" + - " - collection: ' '\n" + + " - bundle: ' '\n" + + " collection: ' '\n" + + " field: ' '\n" + + " group: ' '\n" + " mount_path: ' '\n" + " name: ' '\n" + " namespace: ' '\n" +