Skip to content

Commit 9dc4ff6

Browse files
authored
feat(credentials): add in-place credential rotation via upsert (#2919)
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent 21430e3 commit 9dc4ff6

16 files changed

Lines changed: 613 additions & 86 deletions

app/controlplane/pkg/biz/casbackend.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// 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
429429
credentialsUpdated := creds != nil
430430
// We want to rotate credentials
431431
if creds != nil {
432-
secretName, err = uc.credsRW.SaveCredentials(ctx, orgID, creds)
432+
var saveOpts []credentials.SaveOption
433+
if before.SecretName != "" {
434+
saveOpts = append(saveOpts, credentials.WithExistingSecret(before.SecretName))
435+
}
436+
secretName, err = uc.credsRW.SaveCredentials(ctx, orgID, creds, saveOpts...)
433437
if err != nil {
434438
return nil, fmt.Errorf("storing the credentials: %w", err)
435439
}

app/controlplane/pkg/biz/casbackend_integration_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -410,7 +410,7 @@ func (s *CASBackendIntegrationTestSuite) TestUpdate() {
410410
creds := struct{}{}
411411
ctx := context.TODO()
412412
s.credsWriter.Mock = mock.Mock{}
413-
s.credsWriter.On("SaveCredentials", ctx, s.orgNoBackend.ID, creds).Return("new-secret", nil)
413+
s.credsWriter.On("SaveCredentials", ctx, s.orgNoBackend.ID, creds, mock.Anything).Return("new-secret", nil)
414414
s.credsWriter.On("ReadCredentials", ctx, "new-secret", mock.Anything).Return(nil)
415415
s.backendProvider.On("ValidateAndExtractCredentials", location, mock.Anything).Return(nil, nil)
416416
defaultB, err = s.CASBackend.Update(ctx, s.orgNoBackend.ID, defaultB.ID.String(), toPtrS(description), creds, nil, nil, nil)

app/controlplane/pkg/biz/casbackend_test.go

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -264,6 +264,95 @@ func (s *casBackendTestSuite) TestNewCASBackendUseCase() {
264264
}
265265
}
266266

267+
// TestUpdateRotatesCredentialsInPlace verifies that Update() passes the existing SecretName
268+
// as a WithExistingSecret option so the credential store upserts in-place instead of
269+
// creating a new entry.
270+
func (s *casBackendTestSuite) TestUpdateRotatesCredentialsInPlace() {
271+
ctx := context.Background()
272+
existingSecretName := "org/existing-secret-path"
273+
backendID := uuid.New()
274+
newCreds := &credentials.OCIKeypair{Repo: "r", Username: "u", Password: "p"}
275+
276+
tests := []struct {
277+
name string
278+
existingSecret string
279+
wantWithExistingSecret bool // whether the SaveOption should carry the existing secret name
280+
returnedSecretName string // what SaveCredentials mock returns
281+
}{
282+
{
283+
name: "existing secret name is forwarded as WithExistingSecret",
284+
existingSecret: existingSecretName,
285+
wantWithExistingSecret: true,
286+
returnedSecretName: existingSecretName,
287+
},
288+
{
289+
name: "empty secret name generates a new path",
290+
existingSecret: "",
291+
wantWithExistingSecret: false,
292+
returnedSecretName: "new-secret-path",
293+
},
294+
}
295+
296+
for _, tc := range tests {
297+
s.Run(tc.name, func() {
298+
s.resetMock()
299+
300+
before := &biz.CASBackend{
301+
ID: backendID,
302+
SecretName: tc.existingSecret,
303+
Provider: backendType,
304+
}
305+
306+
// Step 1: FindByIDInOrg returns the existing backend (consumed once).
307+
s.repo.On("FindByIDInOrg", ctx, s.validUUID, backendID).Return(before, nil).Once()
308+
309+
// Step 2: SaveCredentials — capture SaveOption(s) to verify the secret name.
310+
var capturedSecretName string
311+
var optWasPassed bool
312+
saveMatcher := mock.MatchedBy(func(_ interface{}) bool { return true })
313+
s.credsRW.On("SaveCredentials", ctx, s.validUUID.String(), saveMatcher, mock.Anything).
314+
Run(func(args mock.Arguments) {
315+
if len(args) > 3 {
316+
optWasPassed = true
317+
if opt, ok := args.Get(3).(credentials.SaveOption); ok {
318+
o := credentials.ApplySaveOptions(opt)
319+
capturedSecretName = o.SecretName
320+
}
321+
}
322+
}).Return(tc.returnedSecretName, nil).Maybe()
323+
324+
// Fallback for no-opts case (empty existing secret → no WithExistingSecret).
325+
s.credsRW.On("SaveCredentials", ctx, s.validUUID.String(), saveMatcher).
326+
Return(tc.returnedSecretName, nil).Maybe()
327+
328+
// Step 3: repo.Update persists the change.
329+
updatedBackend := &biz.CASBackend{ID: backendID, SecretName: tc.returnedSecretName, Provider: backendType}
330+
s.repo.On("Update", ctx, mock.Anything).Return(updatedBackend, nil)
331+
332+
// Steps 4–7: PerformValidation is called internally with new creds.
333+
s.repo.On("FindByID", mock.Anything, backendID).Return(updatedBackend, nil)
334+
s.credsRW.On("ReadCredentials", mock.Anything, mock.Anything, mock.Anything).Return(nil)
335+
s.backendProvider.On("ValidateAndExtractCredentials", mock.Anything, mock.Anything).Return(nil, nil)
336+
s.repo.On("UpdateValidationStatus", mock.Anything, backendID, biz.CASBackendValidationOK, mock.Anything).Return(nil)
337+
338+
// Step 8: FindByIDInOrg reload after validation.
339+
s.repo.On("FindByIDInOrg", ctx, s.validUUID, backendID).Return(updatedBackend, nil)
340+
341+
got, err := s.useCase.Update(ctx, s.validUUID.String(), backendID.String(), nil, newCreds, nil, nil, nil)
342+
s.Require().NoError(err)
343+
s.Equal(tc.returnedSecretName, got.SecretName)
344+
345+
if tc.wantWithExistingSecret {
346+
s.True(optWasPassed, "expected SaveOption to be passed to SaveCredentials")
347+
s.Equal(existingSecretName, capturedSecretName,
348+
"expected WithExistingSecret(%q) to be forwarded to SaveCredentials", existingSecretName)
349+
} else {
350+
s.False(optWasPassed, "expected no SaveOption to be passed to SaveCredentials")
351+
}
352+
})
353+
}
354+
}
355+
267356
// Run all the tests
268357
func TestCASBackend(t *testing.T) {
269358
suite.Run(t, new(casBackendTestSuite))

pkg/credentials/.mockery.yml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
all: false
2+
formatter: goimports
3+
include-auto-generated: false
4+
log-level: info
5+
recursive: false
6+
require-template-schema-exists: true
7+
template: testify
8+
template-schema: "{{.Template}}.schema.json"
9+
packages:
10+
github.com/chainloop-dev/chainloop/pkg/credentials:
11+
config:
12+
dir: "{{.InterfaceDir}}/mocks"
13+
filename: "{{.InterfaceName}}.go"
14+
pkgname: mocks
15+
structname: "{{.InterfaceName}}"
16+
interfaces:
17+
ReaderWriter:
18+
Writer:
19+
github.com/chainloop-dev/chainloop/pkg/credentials/aws:
20+
config:
21+
dir: "{{.InterfaceDir}}/mocks"
22+
filename: "{{.InterfaceName}}.go"
23+
pkgname: mocks
24+
structname: "{{.InterfaceName}}"
25+
interfaces:
26+
SecretsManagerIface:

pkg/credentials/aws/mocks/SecretsManagerIface.go

Lines changed: 38 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/credentials/aws/secretmanager.go

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2023-2025 The Chainloop Authors.
2+
// Copyright 2023-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@ import (
2626
"github.com/aws/aws-sdk-go-v2/aws"
2727
awscreds "github.com/aws/aws-sdk-go-v2/credentials"
2828
"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
29+
smtypes "github.com/aws/aws-sdk-go-v2/service/secretsmanager/types"
2930
"github.com/aws/aws-sdk-go-v2/service/sso/types"
3031
"github.com/aws/smithy-go"
3132
"github.com/chainloop-dev/chainloop/pkg/credentials"
@@ -38,6 +39,7 @@ import (
3839
type SecretsManagerIface interface {
3940
GetSecretValue(ctx context.Context, params *secretsmanager.GetSecretValueInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.GetSecretValueOutput, error)
4041
CreateSecret(ctx context.Context, params *secretsmanager.CreateSecretInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.CreateSecretOutput, error)
42+
PutSecretValue(ctx context.Context, params *secretsmanager.PutSecretValueInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.PutSecretValueOutput, error)
4143
DeleteSecret(ctx context.Context, params *secretsmanager.DeleteSecretInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.DeleteSecretOutput, error)
4244
}
4345

@@ -79,21 +81,43 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) {
7981
}, nil
8082
}
8183

82-
// Save Credentials, this is a generic function that can be used to save any type of credentials
83-
// as long as they can be passed to json.Marshal
84-
func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any) (string, error) {
85-
secretName := strings.Join([]string{m.secretPrefix, orgID, uuid.New().String()}, "/")
84+
// SaveCredentials saves credentials. If opts includes WithExistingSecret, upserts at the given path.
85+
func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, opts ...credentials.SaveOption) (string, error) {
86+
o := credentials.ApplySaveOptions(opts...)
87+
secretName := o.SecretName
88+
if secretName == "" {
89+
secretName = strings.Join([]string{m.secretPrefix, orgID, uuid.New().String()}, "/")
90+
}
8691

8792
// Store the credentials as json key pairs
8893
c, err := json.Marshal(creds)
8994
if err != nil {
9095
return "", fmt.Errorf("marshaling credentials to be stored: %w", err)
9196
}
9297

93-
if _, err = m.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{
94-
Name: aws.String(secretName), SecretString: aws.String(string(c)),
95-
}); err != nil {
96-
return "", fmt.Errorf("creating secret in AWS: %w", err)
98+
if o.SecretName != "" {
99+
// Upsert: try to update existing secret first
100+
_, err = m.client.PutSecretValue(ctx, &secretsmanager.PutSecretValueInput{
101+
SecretId: aws.String(secretName),
102+
SecretString: aws.String(string(c)),
103+
})
104+
var notFoundErr *smtypes.ResourceNotFoundException
105+
if errors.As(err, &notFoundErr) {
106+
// Secret does not exist yet, create it
107+
_, err = m.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{
108+
Name: aws.String(secretName),
109+
SecretString: aws.String(string(c)),
110+
})
111+
}
112+
} else {
113+
_, err = m.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{
114+
Name: aws.String(secretName),
115+
SecretString: aws.String(string(c)),
116+
})
117+
}
118+
119+
if err != nil {
120+
return "", fmt.Errorf("saving secret in AWS: %w", err)
97121
}
98122

99123
return secretName, nil

0 commit comments

Comments
 (0)