diff --git a/app/controlplane/pkg/biz/casbackend.go b/app/controlplane/pkg/biz/casbackend.go index 4e99773c0..e959444a2 100644 --- a/app/controlplane/pkg/biz/casbackend.go +++ b/app/controlplane/pkg/biz/casbackend.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -429,7 +429,11 @@ func (uc *CASBackendUseCase) Update(ctx context.Context, orgID, id string, descr credentialsUpdated := creds != nil // We want to rotate credentials if creds != nil { - secretName, err = uc.credsRW.SaveCredentials(ctx, orgID, creds) + var saveOpts []credentials.SaveOption + if before.SecretName != "" { + saveOpts = append(saveOpts, credentials.WithExistingSecret(before.SecretName)) + } + secretName, err = uc.credsRW.SaveCredentials(ctx, orgID, creds, saveOpts...) if err != nil { return nil, fmt.Errorf("storing the credentials: %w", err) } diff --git a/app/controlplane/pkg/biz/casbackend_integration_test.go b/app/controlplane/pkg/biz/casbackend_integration_test.go index cc2f86a9f..281537e06 100644 --- a/app/controlplane/pkg/biz/casbackend_integration_test.go +++ b/app/controlplane/pkg/biz/casbackend_integration_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -410,7 +410,7 @@ func (s *CASBackendIntegrationTestSuite) TestUpdate() { creds := struct{}{} ctx := context.TODO() s.credsWriter.Mock = mock.Mock{} - s.credsWriter.On("SaveCredentials", ctx, s.orgNoBackend.ID, creds).Return("new-secret", nil) + s.credsWriter.On("SaveCredentials", ctx, s.orgNoBackend.ID, creds, mock.Anything).Return("new-secret", nil) s.credsWriter.On("ReadCredentials", ctx, "new-secret", mock.Anything).Return(nil) s.backendProvider.On("ValidateAndExtractCredentials", location, mock.Anything).Return(nil, nil) defaultB, err = s.CASBackend.Update(ctx, s.orgNoBackend.ID, defaultB.ID.String(), toPtrS(description), creds, nil, nil, nil) diff --git a/app/controlplane/pkg/biz/casbackend_test.go b/app/controlplane/pkg/biz/casbackend_test.go index 54afa8141..ef8f15030 100644 --- a/app/controlplane/pkg/biz/casbackend_test.go +++ b/app/controlplane/pkg/biz/casbackend_test.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -264,6 +264,95 @@ func (s *casBackendTestSuite) TestNewCASBackendUseCase() { } } +// TestUpdateRotatesCredentialsInPlace verifies that Update() passes the existing SecretName +// as a WithExistingSecret option so the credential store upserts in-place instead of +// creating a new entry. +func (s *casBackendTestSuite) TestUpdateRotatesCredentialsInPlace() { + ctx := context.Background() + existingSecretName := "org/existing-secret-path" + backendID := uuid.New() + newCreds := &credentials.OCIKeypair{Repo: "r", Username: "u", Password: "p"} + + tests := []struct { + name string + existingSecret string + wantWithExistingSecret bool // whether the SaveOption should carry the existing secret name + returnedSecretName string // what SaveCredentials mock returns + }{ + { + name: "existing secret name is forwarded as WithExistingSecret", + existingSecret: existingSecretName, + wantWithExistingSecret: true, + returnedSecretName: existingSecretName, + }, + { + name: "empty secret name generates a new path", + existingSecret: "", + wantWithExistingSecret: false, + returnedSecretName: "new-secret-path", + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.resetMock() + + before := &biz.CASBackend{ + ID: backendID, + SecretName: tc.existingSecret, + Provider: backendType, + } + + // Step 1: FindByIDInOrg returns the existing backend (consumed once). + s.repo.On("FindByIDInOrg", ctx, s.validUUID, backendID).Return(before, nil).Once() + + // Step 2: SaveCredentials — capture SaveOption(s) to verify the secret name. + var capturedSecretName string + var optWasPassed bool + saveMatcher := mock.MatchedBy(func(_ interface{}) bool { return true }) + s.credsRW.On("SaveCredentials", ctx, s.validUUID.String(), saveMatcher, mock.Anything). + Run(func(args mock.Arguments) { + if len(args) > 3 { + optWasPassed = true + if opt, ok := args.Get(3).(credentials.SaveOption); ok { + o := credentials.ApplySaveOptions(opt) + capturedSecretName = o.SecretName + } + } + }).Return(tc.returnedSecretName, nil).Maybe() + + // Fallback for no-opts case (empty existing secret → no WithExistingSecret). + s.credsRW.On("SaveCredentials", ctx, s.validUUID.String(), saveMatcher). + Return(tc.returnedSecretName, nil).Maybe() + + // Step 3: repo.Update persists the change. + updatedBackend := &biz.CASBackend{ID: backendID, SecretName: tc.returnedSecretName, Provider: backendType} + s.repo.On("Update", ctx, mock.Anything).Return(updatedBackend, nil) + + // Steps 4–7: PerformValidation is called internally with new creds. + s.repo.On("FindByID", mock.Anything, backendID).Return(updatedBackend, nil) + s.credsRW.On("ReadCredentials", mock.Anything, mock.Anything, mock.Anything).Return(nil) + s.backendProvider.On("ValidateAndExtractCredentials", mock.Anything, mock.Anything).Return(nil, nil) + s.repo.On("UpdateValidationStatus", mock.Anything, backendID, biz.CASBackendValidationOK, mock.Anything).Return(nil) + + // Step 8: FindByIDInOrg reload after validation. + s.repo.On("FindByIDInOrg", ctx, s.validUUID, backendID).Return(updatedBackend, nil) + + got, err := s.useCase.Update(ctx, s.validUUID.String(), backendID.String(), nil, newCreds, nil, nil, nil) + s.Require().NoError(err) + s.Equal(tc.returnedSecretName, got.SecretName) + + if tc.wantWithExistingSecret { + s.True(optWasPassed, "expected SaveOption to be passed to SaveCredentials") + s.Equal(existingSecretName, capturedSecretName, + "expected WithExistingSecret(%q) to be forwarded to SaveCredentials", existingSecretName) + } else { + s.False(optWasPassed, "expected no SaveOption to be passed to SaveCredentials") + } + }) + } +} + // Run all the tests func TestCASBackend(t *testing.T) { suite.Run(t, new(casBackendTestSuite)) diff --git a/pkg/credentials/.mockery.yml b/pkg/credentials/.mockery.yml new file mode 100644 index 000000000..f542504e6 --- /dev/null +++ b/pkg/credentials/.mockery.yml @@ -0,0 +1,26 @@ +all: false +formatter: goimports +include-auto-generated: false +log-level: info +recursive: false +require-template-schema-exists: true +template: testify +template-schema: "{{.Template}}.schema.json" +packages: + github.com/chainloop-dev/chainloop/pkg/credentials: + config: + dir: "{{.InterfaceDir}}/mocks" + filename: "{{.InterfaceName}}.go" + pkgname: mocks + structname: "{{.InterfaceName}}" + interfaces: + ReaderWriter: + Writer: + github.com/chainloop-dev/chainloop/pkg/credentials/aws: + config: + dir: "{{.InterfaceDir}}/mocks" + filename: "{{.InterfaceName}}.go" + pkgname: mocks + structname: "{{.InterfaceName}}" + interfaces: + SecretsManagerIface: diff --git a/pkg/credentials/aws/mocks/SecretsManagerIface.go b/pkg/credentials/aws/mocks/SecretsManagerIface.go index b54cd46e2..aec03d6c9 100644 --- a/pkg/credentials/aws/mocks/SecretsManagerIface.go +++ b/pkg/credentials/aws/mocks/SecretsManagerIface.go @@ -1,5 +1,4 @@ -// -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -13,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Code generated by mockery v2.20.0. DO NOT EDIT. +// Code generated by mockery v3. DO NOT EDIT. package mocks @@ -62,6 +61,39 @@ func (_m *SecretsManagerIface) CreateSecret(ctx context.Context, params *secrets return r0, r1 } +// PutSecretValue provides a mock function with given fields: ctx, params, optFns +func (_m *SecretsManagerIface) PutSecretValue(ctx context.Context, params *secretsmanager.PutSecretValueInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.PutSecretValueOutput, error) { + _va := make([]interface{}, len(optFns)) + for _i := range optFns { + _va[_i] = optFns[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, params) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 *secretsmanager.PutSecretValueOutput + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *secretsmanager.PutSecretValueInput, ...func(*secretsmanager.Options)) (*secretsmanager.PutSecretValueOutput, error)); ok { + return rf(ctx, params, optFns...) + } + if rf, ok := ret.Get(0).(func(context.Context, *secretsmanager.PutSecretValueInput, ...func(*secretsmanager.Options)) *secretsmanager.PutSecretValueOutput); ok { + r0 = rf(ctx, params, optFns...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*secretsmanager.PutSecretValueOutput) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *secretsmanager.PutSecretValueInput, ...func(*secretsmanager.Options)) error); ok { + r1 = rf(ctx, params, optFns...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // DeleteSecret provides a mock function with given fields: ctx, params, optFns func (_m *SecretsManagerIface) DeleteSecret(ctx context.Context, params *secretsmanager.DeleteSecretInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.DeleteSecretOutput, error) { _va := make([]interface{}, len(optFns)) @@ -128,13 +160,11 @@ func (_m *SecretsManagerIface) GetSecretValue(ctx context.Context, params *secre return r0, r1 } -type mockConstructorTestingTNewSecretsManagerIface interface { +// NewSecretsManagerIface creates a new instance of SecretsManagerIface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewSecretsManagerIface(t interface { mock.TestingT Cleanup(func()) -} - -// NewSecretsManagerIface creates a new instance of SecretsManagerIface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewSecretsManagerIface(t mockConstructorTestingTNewSecretsManagerIface) *SecretsManagerIface { +}) *SecretsManagerIface { mock := &SecretsManagerIface{} mock.Mock.Test(t) diff --git a/pkg/credentials/aws/secretmanager.go b/pkg/credentials/aws/secretmanager.go index 4d504a0fd..6e0f6f6a4 100644 --- a/pkg/credentials/aws/secretmanager.go +++ b/pkg/credentials/aws/secretmanager.go @@ -1,5 +1,5 @@ // -// Copyright 2023-2025 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" awscreds "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/service/secretsmanager" + smtypes "github.com/aws/aws-sdk-go-v2/service/secretsmanager/types" "github.com/aws/aws-sdk-go-v2/service/sso/types" "github.com/aws/smithy-go" "github.com/chainloop-dev/chainloop/pkg/credentials" @@ -38,6 +39,7 @@ import ( type SecretsManagerIface interface { GetSecretValue(ctx context.Context, params *secretsmanager.GetSecretValueInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.GetSecretValueOutput, error) CreateSecret(ctx context.Context, params *secretsmanager.CreateSecretInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.CreateSecretOutput, error) + PutSecretValue(ctx context.Context, params *secretsmanager.PutSecretValueInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.PutSecretValueOutput, error) DeleteSecret(ctx context.Context, params *secretsmanager.DeleteSecretInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.DeleteSecretOutput, error) } @@ -79,10 +81,13 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { }, nil } -// Save Credentials, this is a generic function that can be used to save any type of credentials -// as long as they can be passed to json.Marshal -func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) (string, error) { - secretName := strings.Join([]string{m.secretPrefix, orgID, uuid.New().String()}, "/") +// SaveCredentials saves credentials. If opts includes WithExistingSecret, upserts at the given path. +func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, opts ...credentials.SaveOption) (string, error) { + o := credentials.ApplySaveOptions(opts...) + secretName := o.SecretName + if secretName == "" { + secretName = strings.Join([]string{m.secretPrefix, orgID, uuid.New().String()}, "/") + } // Store the credentials as json key pairs c, err := json.Marshal(creds) @@ -90,10 +95,29 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) return "", fmt.Errorf("marshaling credentials to be stored: %w", err) } - if _, err = m.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{ - Name: aws.String(secretName), SecretString: aws.String(string(c)), - }); err != nil { - return "", fmt.Errorf("creating secret in AWS: %w", err) + if o.SecretName != "" { + // Upsert: try to update existing secret first + _, err = m.client.PutSecretValue(ctx, &secretsmanager.PutSecretValueInput{ + SecretId: aws.String(secretName), + SecretString: aws.String(string(c)), + }) + var notFoundErr *smtypes.ResourceNotFoundException + if errors.As(err, ¬FoundErr) { + // Secret does not exist yet, create it + _, err = m.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{ + Name: aws.String(secretName), + SecretString: aws.String(string(c)), + }) + } + } else { + _, err = m.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{ + Name: aws.String(secretName), + SecretString: aws.String(string(c)), + }) + } + + if err != nil { + return "", fmt.Errorf("saving secret in AWS: %w", err) } return secretName, nil diff --git a/pkg/credentials/aws/secretmanager_test.go b/pkg/credentials/aws/secretmanager_test.go index a20b24f95..b5770aef4 100644 --- a/pkg/credentials/aws/secretmanager_test.go +++ b/pkg/credentials/aws/secretmanager_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -24,6 +24,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/secretsmanager" + smtypes "github.com/aws/aws-sdk-go-v2/service/secretsmanager/types" "github.com/aws/aws-sdk-go-v2/service/sso/types" "github.com/aws/smithy-go" "github.com/chainloop-dev/chainloop/pkg/credentials" @@ -202,6 +203,75 @@ func (s *testSuite) TestReadWriteCredentials() { } } +func (s *testSuite) TestSaveCredentialsUpsert() { + assert := assert.New(s.T()) + creds := &credentials.OCIKeypair{Repo: "r", Username: "u", Password: "p"} + existingSecretName := "org/existing-secret" + + testCases := []struct { + name string + secretName string // non-empty = WithExistingSecret upsert + awsNotFound bool // simulate ResourceNotFoundException on PutSecretValue + expectedError bool + }{ + { + name: "new secret — CreateSecret called", + secretName: "", + }, + { + name: "upsert existing — PutSecretValue succeeds", + secretName: existingSecretName, + }, + { + name: "upsert not found — fallback to CreateSecret", + secretName: existingSecretName, + awsNotFound: true, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + initMockedManager(s) + m := s.mockedManager + mc, _ := m.client.(*mclient.SecretsManagerIface) + ctx := context.Background() + + var saveOpts []credentials.SaveOption + if tc.secretName != "" { + saveOpts = append(saveOpts, credentials.WithExistingSecret(tc.secretName)) + } + + switch { + case tc.secretName == "": + // New secret path: only CreateSecret is called. + mc.On("CreateSecret", ctx, mock.Anything).Return(nil, nil) + case tc.awsNotFound: + // Upsert path: PutSecretValue returns not-found, then CreateSecret. + mc.On("PutSecretValue", ctx, mock.Anything).Return(nil, &smtypes.ResourceNotFoundException{}) + mc.On("CreateSecret", ctx, mock.Anything).Return(nil, nil) + default: + // Upsert path: PutSecretValue succeeds directly. + mc.On("PutSecretValue", ctx, mock.Anything).Return(&secretsmanager.PutSecretValueOutput{}, nil) + } + + returned, err := m.SaveCredentials(ctx, orgID, creds, saveOpts...) + if tc.expectedError { + assert.Error(err) + return + } + assert.NoError(err) + + if tc.secretName != "" { + // In-place upsert must return the same path. + assert.Equal(tc.secretName, returned) + } else { + // New path must be auto-generated (non-empty and different from the existing one). + assert.NotEmpty(returned) + } + }) + } +} + // // Create a new secret, delete it and check it does not exist antymore func (s *testSuite) TestDeleteCredentials() { assert := assert.New(s.T()) diff --git a/pkg/credentials/azurekv/keyvault.go b/pkg/credentials/azurekv/keyvault.go index 711c6711b..6d7a1a8c3 100644 --- a/pkg/credentials/azurekv/keyvault.go +++ b/pkg/credentials/azurekv/keyvault.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -120,9 +120,14 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { }, nil } -// SaveCredentials saves credentials -func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) (string, error) { - secretName := strings.Join([]string{m.secretPrefix, orgID, uuid.New().String()}, "-") +// SaveCredentials saves credentials. If opts includes WithExistingSecret, upserts at the given path. +func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, opts ...credentials.SaveOption) (string, error) { + o := credentials.ApplySaveOptions(opts...) + secretName := o.SecretName + if secretName == "" { + secretName = strings.Join([]string{m.secretPrefix, orgID, uuid.New().String()}, "-") + } + // Store the credentials as json key pairs c, err := json.Marshal(creds) if err != nil { diff --git a/pkg/credentials/azurekv/keyvault_test.go b/pkg/credentials/azurekv/keyvault_test.go index 5b8986aa7..27494a16c 100644 --- a/pkg/credentials/azurekv/keyvault_test.go +++ b/pkg/credentials/azurekv/keyvault_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -159,6 +159,56 @@ func (s *testSuite) TestSaveCredentials() { }) } +func (s *testSuite) TestSaveCredentialsUpsert() { + creds := &credentials.APICreds{Host: "myhost", Key: "mykey"} + existingSecretName := "org-existing-secret" + + testCases := []struct { + name string + secretName string // non-empty = WithExistingSecret upsert + }{ + { + name: "new secret — auto-generated name", + secretName: "", + }, + { + name: "upsert existing — SetSecret called with given name", + secretName: existingSecretName, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + ctx := context.Background() + secretsRW := mocks.NewSecretsRW(s.T()) + + var capturedName string + secretsRW.On("SetSecret", ctx, mock.Anything, mock.Anything, mock.Anything). + Return(azsecrets.SetSecretResponse{}, nil). + Run(func(args mock.Arguments) { + capturedName = args.Get(1).(string) + }) + + m := &Manager{client: secretsRW, secretPrefix: "my-prefix"} + + var saveOpts []credentials.SaveOption + if tc.secretName != "" { + saveOpts = append(saveOpts, credentials.WithExistingSecret(tc.secretName)) + } + + returned, err := m.SaveCredentials(ctx, "my-org", creds, saveOpts...) + s.NoError(err) + s.Equal(capturedName, returned) + + if tc.secretName != "" { + s.Equal(tc.secretName, returned, "upsert must return the existing secret name unchanged") + } else { + s.NotEmpty(returned) + } + }) + } +} + func (s *testSuite) TestReadCredentials() { want := &credentials.APICreds{ Host: "myhost", diff --git a/pkg/credentials/credentials.go b/pkg/credentials/credentials.go index 53734337c..26ec8ef1a 100644 --- a/pkg/credentials/credentials.go +++ b/pkg/credentials/credentials.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -34,8 +34,35 @@ type ReaderWriter interface { Writer } +// SaveOpts holds optional parameters for SaveCredentials. +type SaveOpts struct { + // SecretName when non-empty causes SaveCredentials to upsert at the given path + // instead of generating a new UUID-based path. + SecretName string +} + +// SaveOption is a functional option for SaveCredentials. +type SaveOption func(*SaveOpts) + +// WithExistingSecret instructs SaveCredentials to upsert at the provided existing +// secret path. An empty name is a no-op (auto-generate behavior is preserved). +func WithExistingSecret(name string) SaveOption { + return func(o *SaveOpts) { o.SecretName = name } +} + +// ApplySaveOptions applies the given options and returns the resulting SaveOpts. +func ApplySaveOptions(opts ...SaveOption) SaveOpts { + o := SaveOpts{} + for _, opt := range opts { + if opt != nil { + opt(&o) + } + } + return o +} + type Writer interface { - SaveCredentials(ctx context.Context, org string, credentials any) (string, error) + SaveCredentials(ctx context.Context, org string, credentials any, opts ...SaveOption) (string, error) DeleteCredentials(ctx context.Context, credID string) error } diff --git a/pkg/credentials/gcp/secretmanager.go b/pkg/credentials/gcp/secretmanager.go index 601a18f0d..b0c9cc784 100644 --- a/pkg/credentials/gcp/secretmanager.go +++ b/pkg/credentials/gcp/secretmanager.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -26,6 +26,8 @@ import ( secretmanager "cloud.google.com/go/secretmanager/apiv1" "cloud.google.com/go/secretmanager/apiv1/secretmanagerpb" "google.golang.org/api/option" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/chainloop-dev/chainloop/pkg/credentials" "github.com/chainloop-dev/chainloop/pkg/servicelogger" @@ -83,18 +85,21 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { }, nil } -// SaveCredentials saves credentials -func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) (string, error) { +// SaveCredentials saves credentials. If opts includes WithExistingSecret, upserts at the given path. +func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, opts ...credentials.SaveOption) (string, error) { // store creds in key-value pair c, err := json.Marshal(creds) if err != nil { return "", fmt.Errorf("marshaling credentials to be stored: %w", err) } - secretID := strings.Join([]string{m.secretPrefix, orgID, uuid.Generate().String()}, "-") + o := credentials.ApplySaveOptions(opts...) + secretID := o.SecretName + if secretID == "" { + secretID = strings.Join([]string{m.secretPrefix, orgID, uuid.Generate().String()}, "-") + } - // first create the secret itself - createSecretReq := &secretmanagerpb.CreateSecretRequest{ + secretReq := &secretmanagerpb.CreateSecretRequest{ Parent: fmt.Sprintf("projects/%s", m.projectID), SecretId: secretID, Secret: &secretmanagerpb.Secret{ @@ -105,20 +110,42 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) }, }, } - secret, err := m.client.CreateSecret(ctx, createSecretReq) - if err != nil { - return "", fmt.Errorf("creating secret in GCP: %w", err) - } - m.logger.Infow("msg", "created new secret", "secretID", secretID) - // once the secret is created store it as the newest version - addSecretVersionReq := &secretmanagerpb.AddSecretVersionRequest{ - Parent: secret.Name, - Payload: &secretmanagerpb.SecretPayload{ - Data: c, - }, + var parent string + if o.SecretName != "" { + // Upsert: try adding a new version first (only needs secretmanager.versions.add). + parent = fmt.Sprintf("projects/%s/secrets/%s", m.projectID, secretID) + v, addErr := m.client.AddSecretVersion(ctx, &secretmanagerpb.AddSecretVersionRequest{ + Parent: parent, + Payload: &secretmanagerpb.SecretPayload{Data: c}, + }) + if addErr == nil { + m.logger.Infow("msg", "added new secret version", "secretID", secretID, "versionID", v.Name) + return secretID, nil + } + if !isNotFoundErr(addErr) { + return "", fmt.Errorf("adding secret version in GCP: %w", addErr) + } + // Secret container doesn't exist yet — create it and fall through to AddSecretVersion below. + if _, err = m.client.CreateSecret(ctx, secretReq); err != nil { + return "", fmt.Errorf("creating secret in GCP: %w", err) + } + m.logger.Infow("msg", "created new secret", "secretID", secretID) + } else { + // New secret: create the container and use the canonical name from the API response. + secret, createErr := m.client.CreateSecret(ctx, secretReq) + if createErr != nil { + return "", fmt.Errorf("creating secret in GCP: %w", createErr) + } + parent = secret.Name + m.logger.Infow("msg", "created new secret", "secretID", secretID) } - v, err := m.client.AddSecretVersion(ctx, addSecretVersionReq) + + // Add a new version with the credential payload + v, err := m.client.AddSecretVersion(ctx, &secretmanagerpb.AddSecretVersionRequest{ + Parent: parent, + Payload: &secretmanagerpb.SecretPayload{Data: c}, + }) if err != nil { return "", fmt.Errorf("creating secret version in GCP: %w", err) } @@ -127,6 +154,12 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) return secretID, nil } +// isNotFoundErr returns true when the error is a gRPC NotFound status. +func isNotFoundErr(err error) bool { + s, ok := status.FromError(err) + return ok && s.Code() == codes.NotFound +} + // ReadCredentials reads the latest version of the credentials func (m *Manager) ReadCredentials(ctx context.Context, secretID string, creds any) error { getSecretRequest := secretmanagerpb.AccessSecretVersionRequest{ diff --git a/pkg/credentials/gcp/secretmanager_test.go b/pkg/credentials/gcp/secretmanager_test.go index ad238717a..0fe9df4aa 100644 --- a/pkg/credentials/gcp/secretmanager_test.go +++ b/pkg/credentials/gcp/secretmanager_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -28,6 +28,8 @@ import ( "github.com/go-kratos/kratos/v2/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) const defaultOrgID = "test-org" @@ -74,8 +76,8 @@ func TestCreateCredentials(t *testing.T) { clientMock := gcpmocks.NewSecretsManagerInterface(t) m.client = clientMock - clientMock.On("CreateSecret", ctx, mock.Anything).Return(&secretmanagerpb.Secret{}, nil).Once() - clientMock.On("AddSecretVersion", ctx, mock.Anything).Return(&secretmanagerpb.SecretVersion{}, nil) + clientMock.On("CreateSecret", ctx, mock.Anything, mock.Anything).Return(&secretmanagerpb.Secret{}, nil).Once() + clientMock.On("AddSecretVersion", ctx, mock.Anything, mock.Anything).Return(&secretmanagerpb.SecretVersion{}, nil) _, err = m.SaveCredentials(ctx, defaultOrgID, creds) assert.NoError(t, err) @@ -107,6 +109,78 @@ func TestReadCredentials(t *testing.T) { assert.Equal(t, "key", creds.Key) } +func TestSaveCredentialsUpsert(t *testing.T) { + existingSecretName := "my-existing-secret" + + testCases := []struct { + name string + secretName string // non-empty = WithExistingSecret upsert + gcpNotFound bool // simulate secret container absent in GCP + }{ + { + name: "new secret - CreateSecret then AddSecretVersion", + secretName: "", + }, + { + name: "upsert existing - AddSecretVersion succeeds directly", + secretName: existingSecretName, + }, + { + name: "upsert not found - AddSecretVersion returns NotFound then CreateSecret + AddSecretVersion", + secretName: existingSecretName, + gcpNotFound: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + m := &Manager{projectID: defaultProjectID} + m.logger = servicelogger.ScopedHelper(log.NewStdLogger(io.Discard), "credentials/gcp-secrets-manager") + clientMock := gcpmocks.NewSecretsManagerInterface(t) + m.client = clientMock + + ociCreds := &credentials.OCIKeypair{Repo: "repo", Username: "user", Password: "password"} + + var saveOpts []credentials.SaveOption + if tc.secretName != "" { + saveOpts = append(saveOpts, credentials.WithExistingSecret(tc.secretName)) + } + + switch { + case tc.secretName == "": + // New path: CreateSecret returns a secret, then AddSecretVersion. + clientMock.On("CreateSecret", ctx, mock.Anything, mock.Anything). + Return(&secretmanagerpb.Secret{Name: "projects/1234-5678-9012/secrets/generated-id"}, nil).Once() + clientMock.On("AddSecretVersion", ctx, mock.Anything, mock.Anything). + Return(&secretmanagerpb.SecretVersion{Name: "v1"}, nil).Once() + case tc.gcpNotFound: + // Upsert: AddSecretVersion returns NotFound, then CreateSecret, then AddSecretVersion again. + notFoundErr := status.Error(codes.NotFound, "secret not found") + clientMock.On("AddSecretVersion", ctx, mock.Anything, mock.Anything). + Return(nil, notFoundErr).Once() + clientMock.On("CreateSecret", ctx, mock.Anything, mock.Anything). + Return(&secretmanagerpb.Secret{}, nil).Once() + clientMock.On("AddSecretVersion", ctx, mock.Anything, mock.Anything). + Return(&secretmanagerpb.SecretVersion{Name: "v1"}, nil).Once() + default: + // Upsert: AddSecretVersion succeeds directly (no CreateSecret call). + clientMock.On("AddSecretVersion", ctx, mock.Anything, mock.Anything). + Return(&secretmanagerpb.SecretVersion{Name: "v2"}, nil).Once() + } + + returned, err := m.SaveCredentials(ctx, defaultOrgID, ociCreds, saveOpts...) + assert.NoError(t, err) + + if tc.secretName != "" { + assert.Equal(t, tc.secretName, returned, "upsert must return the same secret name") + } else { + assert.NotEmpty(t, returned) + } + }) + } +} + func TestDeleteCredentials(t *testing.T) { m := &Manager{} m.logger = servicelogger.ScopedHelper(log.NewStdLogger(io.Discard), "credentials/gcp-secrets-manager") diff --git a/pkg/credentials/mocks/ReaderWriter.go b/pkg/credentials/mocks/ReaderWriter.go index b5c4d1748..3752512aa 100644 --- a/pkg/credentials/mocks/ReaderWriter.go +++ b/pkg/credentials/mocks/ReaderWriter.go @@ -1,10 +1,25 @@ -// Code generated by mockery v2.20.0. DO NOT EDIT. +// Code generated by mockery v3. DO NOT EDIT. + +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. package mocks import ( context "context" + credentials "github.com/chainloop-dev/chainloop/pkg/credentials" mock "github.com/stretchr/testify/mock" ) @@ -41,23 +56,30 @@ func (_m *ReaderWriter) ReadCredentials(ctx context.Context, secretName string, return r0 } -// SaveCredentials provides a mock function with given fields: ctx, org, _a2 -func (_m *ReaderWriter) SaveCredentials(ctx context.Context, org string, _a2 interface{}) (string, error) { - ret := _m.Called(ctx, org, _a2) +// SaveCredentials provides a mock function with given fields: ctx, org, _a2, opts +func (_m *ReaderWriter) SaveCredentials(ctx context.Context, org string, _a2 interface{}, opts ...credentials.SaveOption) (string, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, org, _a2) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) var r0 string var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) (string, error)); ok { - return rf(ctx, org, _a2) + if rf, ok := ret.Get(0).(func(context.Context, string, interface{}, ...credentials.SaveOption) (string, error)); ok { + return rf(ctx, org, _a2, opts...) } - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) string); ok { - r0 = rf(ctx, org, _a2) + if rf, ok := ret.Get(0).(func(context.Context, string, interface{}, ...credentials.SaveOption) string); ok { + r0 = rf(ctx, org, _a2, opts...) } else { r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string, interface{}) error); ok { - r1 = rf(ctx, org, _a2) + if rf, ok := ret.Get(1).(func(context.Context, string, interface{}, ...credentials.SaveOption) error); ok { + r1 = rf(ctx, org, _a2, opts...) } else { r1 = ret.Error(1) } @@ -65,13 +87,11 @@ func (_m *ReaderWriter) SaveCredentials(ctx context.Context, org string, _a2 int return r0, r1 } -type mockConstructorTestingTNewReaderWriter interface { +// NewReaderWriter creates a new instance of ReaderWriter. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewReaderWriter(t interface { mock.TestingT Cleanup(func()) -} - -// NewReaderWriter creates a new instance of ReaderWriter. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewReaderWriter(t mockConstructorTestingTNewReaderWriter) *ReaderWriter { +}) *ReaderWriter { mock := &ReaderWriter{} mock.Mock.Test(t) diff --git a/pkg/credentials/mocks/Writer.go b/pkg/credentials/mocks/Writer.go index 8ce555c38..f71ac54d2 100644 --- a/pkg/credentials/mocks/Writer.go +++ b/pkg/credentials/mocks/Writer.go @@ -1,10 +1,25 @@ -// Code generated by mockery v2.20.0. DO NOT EDIT. +// Code generated by mockery v3. DO NOT EDIT. + +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. package mocks import ( context "context" + credentials "github.com/chainloop-dev/chainloop/pkg/credentials" mock "github.com/stretchr/testify/mock" ) @@ -27,23 +42,30 @@ func (_m *Writer) DeleteCredentials(ctx context.Context, credID string) error { return r0 } -// SaveCredentials provides a mock function with given fields: ctx, org, _a2 -func (_m *Writer) SaveCredentials(ctx context.Context, org string, _a2 interface{}) (string, error) { - ret := _m.Called(ctx, org, _a2) +// SaveCredentials provides a mock function with given fields: ctx, org, _a2, opts +func (_m *Writer) SaveCredentials(ctx context.Context, org string, _a2 interface{}, opts ...credentials.SaveOption) (string, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, org, _a2) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) var r0 string var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) (string, error)); ok { - return rf(ctx, org, _a2) + if rf, ok := ret.Get(0).(func(context.Context, string, interface{}, ...credentials.SaveOption) (string, error)); ok { + return rf(ctx, org, _a2, opts...) } - if rf, ok := ret.Get(0).(func(context.Context, string, interface{}) string); ok { - r0 = rf(ctx, org, _a2) + if rf, ok := ret.Get(0).(func(context.Context, string, interface{}, ...credentials.SaveOption) string); ok { + r0 = rf(ctx, org, _a2, opts...) } else { r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string, interface{}) error); ok { - r1 = rf(ctx, org, _a2) + if rf, ok := ret.Get(1).(func(context.Context, string, interface{}, ...credentials.SaveOption) error); ok { + r1 = rf(ctx, org, _a2, opts...) } else { r1 = ret.Error(1) } @@ -51,13 +73,11 @@ func (_m *Writer) SaveCredentials(ctx context.Context, org string, _a2 interface return r0, r1 } -type mockConstructorTestingTNewWriter interface { +// NewWriter creates a new instance of Writer. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewWriter(t interface { mock.TestingT Cleanup(func()) -} - -// NewWriter creates a new instance of Writer. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewWriter(t mockConstructorTestingTNewWriter) *Writer { +}) *Writer { mock := &Writer{} mock.Mock.Test(t) diff --git a/pkg/credentials/vault/keyval.go b/pkg/credentials/vault/keyval.go index 64ebf69e2..6321f4940 100644 --- a/pkg/credentials/vault/keyval.go +++ b/pkg/credentials/vault/keyval.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -136,18 +136,22 @@ func validateReaderClient(kv *vault.KVv2, pathPrefix string) error { return nil } -func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) (string, error) { +func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, opts ...credentials.SaveOption) (string, error) { credsM, err := structToMap(creds) if err != nil { return "", fmt.Errorf("converting struct to map: %w", err) } - secretName := strings.Join([]string{m.secretPrefix, orgID, uuid.Generate().String()}, "/") + o := credentials.ApplySaveOptions(opts...) + secretName := o.SecretName + if secretName == "" { + secretName = strings.Join([]string{m.secretPrefix, orgID, uuid.Generate().String()}, "/") + } m.logger.Infow("msg", "storing credentials", "path", secretName) _, err = m.client.Put(ctx, secretName, credsM) if err != nil { - return "", fmt.Errorf("creating secret in Vault: %w", err) + return "", fmt.Errorf("storing secret in Vault: %w", err) } return secretName, nil diff --git a/pkg/credentials/vault/keyval_test.go b/pkg/credentials/vault/keyval_test.go index 68e9df095..6ddb06321 100644 --- a/pkg/credentials/vault/keyval_test.go +++ b/pkg/credentials/vault/keyval_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023-2025 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -129,6 +129,57 @@ func (s *testSuite) TestReadWriteCredentials() { assert.ErrorIs(err, credentials.ErrNotFound) } +// TestSaveCredentialsUpsert verifies that WithExistingSecret causes SaveCredentials to write +// to the provided path (overwriting if it already exists) instead of generating a new UUID. +func (s *testSuite) TestSaveCredentialsUpsert() { + assert := assert.New(s.T()) + require := require.New(s.T()) + + testCases := []struct { + name string + useExisting bool // whether to pass WithExistingSecret + expectedSamePath bool + }{ + {"new secret generates unique path", false, false}, + {"upsert at existing path returns same path", true, true}, + } + + opts := &vault.NewManagerOpts{AuthToken: defaultToken, Address: s.connectionString, SecretPrefix: "test-prefix"} + m, err := vault.NewManager(opts) + require.NoError(err) + + // First write to establish the path. + firstCreds := &credentials.OCIKeypair{Repo: "repo1", Username: "user1", Password: "pass1"} + existingPath, err := m.SaveCredentials(context.Background(), orgID, firstCreds) + require.NoError(err) + require.NotEmpty(existingPath) + + for _, tc := range testCases { + s.Run(tc.name, func() { + updatedCreds := &credentials.OCIKeypair{Repo: "repo2", Username: "user2", Password: "pass2"} + + var saveOpts []credentials.SaveOption + if tc.useExisting { + saveOpts = append(saveOpts, credentials.WithExistingSecret(existingPath)) + } + + returnedPath, err := m.SaveCredentials(context.Background(), orgID, updatedCreds, saveOpts...) + assert.NoError(err) + + if tc.expectedSamePath { + assert.Equal(existingPath, returnedPath, "upsert must return the same path") + + // Verify the credential was updated at the existing path. + got := &credentials.OCIKeypair{} + assert.NoError(m.ReadCredentials(context.Background(), returnedPath, got)) + assert.Equal(updatedCreds, got, "secret value must reflect the updated credentials") + } else { + assert.NotEqual(existingPath, returnedPath, "new write must generate a different path") + } + }) + } +} + // Create a new secret, delete it and check it does not exist antymore func (s *testSuite) TestDeleteCreds() { assert := assert.New(s.T())