Skip to content

Commit 25095db

Browse files
authored
Merge pull request #1570 from fluxcd/fix-option-slices
Fix decryptor copy of auth.Option slices (avoid overrides)
2 parents b531fe1 + ad1ba12 commit 25095db

File tree

2 files changed

+104
-5
lines changed

2 files changed

+104
-5
lines changed

internal/decryptor/decryptor.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"net/url"
2727
"os"
2828
"path/filepath"
29+
"slices"
2930
"strings"
3031
"sync"
3132
"time"
@@ -342,19 +343,20 @@ func (d *Decryptor) SetAuthOptions(ctx context.Context) {
342343
}
343344

344345
if d.awsCredentialsProvider == nil {
345-
awsOpts := opts
346+
awsOpts := slices.Clone(opts)
346347
if d.tokenCache != nil {
347348
involvedObject.Operation = kustomizev1.MetricDecryptWithAWS
348349
awsOpts = append(awsOpts, auth.WithCache(*d.tokenCache, involvedObject))
349350
}
350351
d.awsCredentialsProvider = func(region string) awssdk.CredentialsProvider {
351-
awsOpts := append(awsOpts, auth.WithSTSRegion(region))
352-
return aws.NewCredentialsProvider(ctx, awsOpts...)
352+
awsOptsWithRegion := slices.Clone(awsOpts)
353+
awsOptsWithRegion = append(awsOptsWithRegion, auth.WithSTSRegion(region))
354+
return aws.NewCredentialsProvider(ctx, awsOptsWithRegion...)
353355
}
354356
}
355357

356358
if d.azureTokenCredential == nil {
357-
azureOpts := opts
359+
azureOpts := slices.Clone(opts)
358360
if d.tokenCache != nil {
359361
involvedObject.Operation = kustomizev1.MetricDecryptWithAzure
360362
azureOpts = append(azureOpts, auth.WithCache(*d.tokenCache, involvedObject))
@@ -363,7 +365,7 @@ func (d *Decryptor) SetAuthOptions(ctx context.Context) {
363365
}
364366

365367
if d.gcpTokenSource == nil {
366-
gcpOpts := opts
368+
gcpOpts := slices.Clone(opts)
367369
if d.tokenCache != nil {
368370
involvedObject.Operation = kustomizev1.MetricDecryptWithGCP
369371
gcpOpts = append(gcpOpts, auth.WithCache(*d.tokenCache, involvedObject))

internal/decryptor/decryptor_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
"sigs.k8s.io/yaml"
4848

4949
"github.com/fluxcd/pkg/apis/meta"
50+
"github.com/fluxcd/pkg/cache"
5051

5152
kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1"
5253
)
@@ -445,6 +446,102 @@ func TestDecryptor_SetAuthOptions(t *testing.T) {
445446
g.Expect(d.azureTokenCredential).NotTo(BeNil())
446447
g.Expect(d.gcpTokenSource).NotTo(BeNil())
447448
})
449+
450+
t.Run("awsCredentialsProvider returns independent providers for different regions", func(t *testing.T) {
451+
g := NewWithT(t)
452+
453+
d := &Decryptor{
454+
kustomization: &kustomizev1.Kustomization{
455+
Spec: kustomizev1.KustomizationSpec{
456+
Decryption: &kustomizev1.Decryption{
457+
Provider: DecryptionProviderSOPS,
458+
},
459+
},
460+
},
461+
}
462+
463+
d.SetAuthOptions(context.Background())
464+
465+
// Call awsCredentialsProvider multiple times with different regions.
466+
// Before the fix, these would share the same underlying slice and
467+
// the region option would be overwritten by subsequent calls.
468+
provider1 := d.awsCredentialsProvider("us-east-1")
469+
provider2 := d.awsCredentialsProvider("us-west-2")
470+
provider3 := d.awsCredentialsProvider("eu-west-1")
471+
472+
// All providers should be non-nil and independent.
473+
g.Expect(provider1).NotTo(BeNil())
474+
g.Expect(provider2).NotTo(BeNil())
475+
g.Expect(provider3).NotTo(BeNil())
476+
477+
// The providers should be different instances (not the same pointer).
478+
// This verifies that each call creates an independent provider.
479+
g.Expect(fmt.Sprintf("%p", provider1)).NotTo(Equal(fmt.Sprintf("%p", provider2)))
480+
g.Expect(fmt.Sprintf("%p", provider2)).NotTo(Equal(fmt.Sprintf("%p", provider3)))
481+
})
482+
483+
t.Run("provider options are independent between AWS, Azure, and GCP", func(t *testing.T) {
484+
g := NewWithT(t)
485+
486+
// Create a decryptor with token cache to ensure options are appended
487+
// (WithCache is added when tokenCache is not nil).
488+
tokenCache, err := cache.NewTokenCache(1)
489+
g.Expect(err).NotTo(HaveOccurred())
490+
491+
d := &Decryptor{
492+
kustomization: &kustomizev1.Kustomization{
493+
ObjectMeta: metav1.ObjectMeta{
494+
Name: "test",
495+
Namespace: "test-ns",
496+
},
497+
Spec: kustomizev1.KustomizationSpec{
498+
Decryption: &kustomizev1.Decryption{
499+
Provider: DecryptionProviderSOPS,
500+
},
501+
},
502+
},
503+
tokenCache: tokenCache,
504+
}
505+
506+
d.SetAuthOptions(context.Background())
507+
508+
// All providers should be set.
509+
g.Expect(d.awsCredentialsProvider).NotTo(BeNil())
510+
g.Expect(d.azureTokenCredential).NotTo(BeNil())
511+
g.Expect(d.gcpTokenSource).NotTo(BeNil())
512+
513+
// The AWS credentials provider should work correctly even after
514+
// Azure and GCP options have been set (options should be isolated).
515+
awsProvider := d.awsCredentialsProvider("us-east-1")
516+
g.Expect(awsProvider).NotTo(BeNil())
517+
})
518+
519+
t.Run("multiple awsCredentialsProvider calls with same region are independent", func(t *testing.T) {
520+
g := NewWithT(t)
521+
522+
d := &Decryptor{
523+
kustomization: &kustomizev1.Kustomization{
524+
Spec: kustomizev1.KustomizationSpec{
525+
Decryption: &kustomizev1.Decryption{
526+
Provider: DecryptionProviderSOPS,
527+
},
528+
},
529+
},
530+
}
531+
532+
d.SetAuthOptions(context.Background())
533+
534+
// Even with the same region, each call should create an independent provider.
535+
// This tests that the slices.Clone is applied correctly.
536+
provider1 := d.awsCredentialsProvider("us-east-1")
537+
provider2 := d.awsCredentialsProvider("us-east-1")
538+
539+
g.Expect(provider1).NotTo(BeNil())
540+
g.Expect(provider2).NotTo(BeNil())
541+
542+
// They should be different instances.
543+
g.Expect(fmt.Sprintf("%p", provider1)).NotTo(Equal(fmt.Sprintf("%p", provider2)))
544+
})
448545
}
449546

450547
func TestDecryptor_SopsDecryptWithFormat(t *testing.T) {

0 commit comments

Comments
 (0)