From 3e33a4b0ca0c37de41859e1eef60d0aeaedb9bfa Mon Sep 17 00:00:00 2001 From: Venelin Date: Thu, 2 Nov 2023 15:55:04 +0000 Subject: [PATCH 1/5] run credentials validation only once --- provider/resources.go | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/provider/resources.go b/provider/resources.go index 598ec14bcaf..ba47925abcc 100644 --- a/provider/resources.go +++ b/provider/resources.go @@ -22,6 +22,7 @@ import ( "path/filepath" "strconv" "strings" + "sync/atomic" "unicode" "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" @@ -531,19 +532,7 @@ func arrayValue(vars resource.PropertyMap, prop resource.PropertyKey, envs []str return vals } -// preConfigureCallback validates that AWS credentials can be successfully discovered. This emulates the credentials -// configuration subset of `github.com/terraform-providers/terraform-provider-aws/aws.providerConfigure`. We do this -// before passing control to the TF provider to ensure we can report actionable errors. -func preConfigureCallback(vars resource.PropertyMap, c shim.ResourceConfig) error { - skipCredentialsValidation := boolValue(vars, "skipCredentialsValidation", - []string{"AWS_SKIP_CREDENTIALS_VALIDATION"}) - - // if we skipCredentialsValidation then we don't need to do anything in - // preConfigureCallback as this is an explicit operation - if skipCredentialsValidation { - return nil - } - +func validateCredentials(vars resource.PropertyMap, c shim.ResourceConfig) error { config := &awsbase.Config{ AccessKey: stringValue(vars, "accessKey", []string{"AWS_ACCESS_KEY_ID"}), SecretKey: stringValue(vars, "secretKey", []string{"AWS_SECRET_ACCESS_KEY"}), @@ -626,6 +615,30 @@ func preConfigureCallback(vars resource.PropertyMap, c shim.ResourceConfig) erro return nil } +// We should only run the validation once to avoid duplicating the reported errors. +var credentialsValidationCounter atomic.Int32 + +// preConfigureCallback validates that AWS credentials can be successfully discovered. This emulates the credentials +// configuration subset of `github.com/terraform-providers/terraform-provider-aws/aws.providerConfigure`. We do this +// before passing control to the TF provider to ensure we can report actionable errors. +func preConfigureCallback(vars resource.PropertyMap, c shim.ResourceConfig) error { + skipCredentialsValidation := boolValue(vars, "skipCredentialsValidation", + []string{"AWS_SKIP_CREDENTIALS_VALIDATION"}) + + // if we skipCredentialsValidation then we don't need to do anything in + // preConfigureCallback as this is an explicit operation + if skipCredentialsValidation { + return nil + } + + counterVal := credentialsValidationCounter.Add(1) + if counterVal > 1 { + return nil + } + + return validateCredentials(vars, c) +} + // managedByPulumi is a default used for some managed resources, in the absence of something more meaningful. var managedByPulumi = &tfbridge.DefaultInfo{Value: "Managed by Pulumi"} From dac593a03b00c386d04a4f17bfb05252304e974b Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 3 Nov 2023 09:30:04 +0000 Subject: [PATCH 2/5] replace counter with once --- provider/resources.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/provider/resources.go b/provider/resources.go index ba47925abcc..539c72d084b 100644 --- a/provider/resources.go +++ b/provider/resources.go @@ -22,7 +22,7 @@ import ( "path/filepath" "strconv" "strings" - "sync/atomic" + "sync" "unicode" "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" @@ -616,7 +616,7 @@ func validateCredentials(vars resource.PropertyMap, c shim.ResourceConfig) error } // We should only run the validation once to avoid duplicating the reported errors. -var credentialsValidationCounter atomic.Int32 +var credentialsValidationOnce sync.Once // preConfigureCallback validates that AWS credentials can be successfully discovered. This emulates the credentials // configuration subset of `github.com/terraform-providers/terraform-provider-aws/aws.providerConfigure`. We do this @@ -631,12 +631,11 @@ func preConfigureCallback(vars resource.PropertyMap, c shim.ResourceConfig) erro return nil } - counterVal := credentialsValidationCounter.Add(1) - if counterVal > 1 { - return nil - } - - return validateCredentials(vars, c) + var err error = nil + credentialsValidationOnce.Do(func() { + err = validateCredentials(vars, c) + }) + return err } // managedByPulumi is a default used for some managed resources, in the absence of something more meaningful. From 4c8b6b3026782055e47f5ed72a1072933c4aa652 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 3 Nov 2023 10:50:29 +0000 Subject: [PATCH 3/5] add a test for the error duplication --- examples/bucket-yaml/Pulumi.yaml | 10 +++++++++ examples/diagnostic_test.go | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 examples/bucket-yaml/Pulumi.yaml create mode 100644 examples/diagnostic_test.go diff --git a/examples/bucket-yaml/Pulumi.yaml b/examples/bucket-yaml/Pulumi.yaml new file mode 100644 index 00000000000..70eac466494 --- /dev/null +++ b/examples/bucket-yaml/Pulumi.yaml @@ -0,0 +1,10 @@ +name: bucket-yaml +runtime: yaml +description: A minimal Pulumi YAML program +resources: + bucket: + type: aws:s3:Bucket + properties: + tags: + Environment: Dev + Name: My bucket diff --git a/examples/diagnostic_test.go b/examples/diagnostic_test.go new file mode 100644 index 00000000000..5f38862e82c --- /dev/null +++ b/examples/diagnostic_test.go @@ -0,0 +1,36 @@ +package examples + +import ( + "bytes" + "path/filepath" + "q" + "strings" + "testing" + + "github.com/pulumi/pulumi/pkg/v3/testing/integration" + "github.com/stretchr/testify/assert" +) + +func TestCredentialsErrorNotDuplicated(t *testing.T) { + var outputBuf bytes.Buffer + test := getBaseOptions(). + With(integration.ProgramTestOptions{ + Dir: filepath.Join(getCwd(t), "bucket-yaml"), + Quick: true, + Stderr: &outputBuf, + ExpectFailure: true, + ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) { + assert.Equal(t, 1, strings.Count(outputBuf.String(), "No valid credential sources found")) + }, + LocalProviders: []integration.LocalDependency{ + { + Package: "aws", + Path: filepath.Join(getCwd(t), "..", "bin"), + }, + }, + }) + + integration.ProgramTest(t, &test) + + q.Q(outputBuf.String()) +} From 25041cabf7b717e545c40127c6056f504373377f Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 3 Nov 2023 11:21:50 +0000 Subject: [PATCH 4/5] remove debug stuff --- examples/diagnostic_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/examples/diagnostic_test.go b/examples/diagnostic_test.go index 5f38862e82c..110b359a8ca 100644 --- a/examples/diagnostic_test.go +++ b/examples/diagnostic_test.go @@ -3,7 +3,6 @@ package examples import ( "bytes" "path/filepath" - "q" "strings" "testing" @@ -22,15 +21,7 @@ func TestCredentialsErrorNotDuplicated(t *testing.T) { ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) { assert.Equal(t, 1, strings.Count(outputBuf.String(), "No valid credential sources found")) }, - LocalProviders: []integration.LocalDependency{ - { - Package: "aws", - Path: filepath.Join(getCwd(t), "..", "bin"), - }, - }, }) integration.ProgramTest(t, &test) - - q.Q(outputBuf.String()) } From 9d7fc9c64d74df4df9c15316e396869f258c9ad0 Mon Sep 17 00:00:00 2001 From: Venelin Date: Fri, 3 Nov 2023 18:17:14 +0000 Subject: [PATCH 5/5] use invalid credentials instead of the default ones --- examples/diagnostic_test.go | 23 +++++++++++++---------- provider/resources.go | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/examples/diagnostic_test.go b/examples/diagnostic_test.go index 110b359a8ca..19e64692523 100644 --- a/examples/diagnostic_test.go +++ b/examples/diagnostic_test.go @@ -12,16 +12,19 @@ import ( func TestCredentialsErrorNotDuplicated(t *testing.T) { var outputBuf bytes.Buffer - test := getBaseOptions(). - With(integration.ProgramTestOptions{ - Dir: filepath.Join(getCwd(t), "bucket-yaml"), - Quick: true, - Stderr: &outputBuf, - ExpectFailure: true, - ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) { - assert.Equal(t, 1, strings.Count(outputBuf.String(), "No valid credential sources found")) - }, - }) + test := integration.ProgramTestOptions{ + Dir: filepath.Join(getCwd(t), "bucket-yaml"), + Quick: true, + Stderr: &outputBuf, + ExpectFailure: true, + Env: []string{ + "AWS_ACCESS_KEY_ID=INVALID", + "AWS_SECRET_ACCESS_KEY=INVALID", + }, + ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) { + assert.Equal(t, 1, strings.Count(outputBuf.String(), "The security token included in the request is invalid")) + }, + } integration.ProgramTest(t, &test) } diff --git a/provider/resources.go b/provider/resources.go index 539c72d084b..b26cbf0b0e3 100644 --- a/provider/resources.go +++ b/provider/resources.go @@ -631,7 +631,7 @@ func preConfigureCallback(vars resource.PropertyMap, c shim.ResourceConfig) erro return nil } - var err error = nil + var err error credentialsValidationOnce.Do(func() { err = validateCredentials(vars, c) })