From f06a426f90538dcfd0983a6fd317cd2ba23c9670 Mon Sep 17 00:00:00 2001 From: Elie Date: Thu, 28 Jan 2021 19:57:04 +0100 Subject: [PATCH] Fix acceptance test issues - Set all test AZ to us-east-1 - Use terraform overriden env in PreCheck() to ensure mutations are executed with read write credentials - Fix hidden error in aws_instance test (tag creation failure was not handled - Minor fmt fix --- doc/contributing/tests.md | 4 ++- pkg/resource/aws/aws_instance_test.go | 13 ++++++++-- pkg/resource/aws/aws_route53_record_test.go | 3 +++ pkg/resource/aws/aws_s3_bucket_test.go | 3 +++ .../testdata/acc/aws_instance/providers.tf | 4 +-- .../aws_internet_gateway/.terraform.lock.hcl | 1 + .../acc/aws_route53_record/providers.tf | 4 +-- test/acceptance/result.go | 2 +- test/acceptance/testing.go | 26 +++++++++++++++++++ 9 files changed, 52 insertions(+), 8 deletions(-) diff --git a/doc/contributing/tests.md b/doc/contributing/tests.md index c4cc4ff3..bfaaf72f 100644 --- a/doc/contributing/tests.md +++ b/doc/contributing/tests.md @@ -133,7 +133,9 @@ Acceptance tests need credentials to perform real world action on cloud provider Recommended way to run acc tests is to use two distinct credentials: one for terraform related actions, and one for driftctl scan. -You can override environment variables passed to terraform operations by adding `ACC_` prefix on env variables. +In our acceptance tests, we may need read/write permissions during specific contexts +(e.g. terraform init, apply, destroy)or lifecycle (PreExec and PostExec). +If needed, you can override environment variables in those contexts by adding `ACC_` prefix on env variables. #### AWS diff --git a/pkg/resource/aws/aws_instance_test.go b/pkg/resource/aws/aws_instance_test.go index 0905569c..95e4c8dc 100644 --- a/pkg/resource/aws/aws_instance_test.go +++ b/pkg/resource/aws/aws_instance_test.go @@ -20,6 +20,9 @@ func TestAcc_AwsInstance_WithBlockDevices(t *testing.T) { Args: []string{"scan", "--filter", "Type=='aws_instance'"}, Checks: []acceptance.AccCheck{ { + Env: map[string]string{ + "AWS_REGION": "us-east-1", + }, Check: func(result *acceptance.ScanResult, stdout string, err error) { if err != nil { t.Fatal(err) @@ -28,6 +31,9 @@ func TestAcc_AwsInstance_WithBlockDevices(t *testing.T) { }, }, { + Env: map[string]string{ + "AWS_REGION": "us-east-1", + }, PreExec: func() { client := ec2.New(awsutils.Session()) response, err := client.DescribeInstances(&ec2.DescribeInstancesInput{ @@ -49,11 +55,11 @@ func TestAcc_AwsInstance_WithBlockDevices(t *testing.T) { if err != nil { t.Fatal(err) } - if len(response.Reservations[0].Instances) != 1 { + if len(response.Reservations) != 1 || len(response.Reservations[0].Instances) != 1 { t.Fatal("Error, unexpected number of instances found, manual check required") } mutatedInstanceId = *response.Reservations[0].Instances[0].InstanceId - _, _ = client.CreateTags(&ec2.CreateTagsInput{ + _, err = client.CreateTags(&ec2.CreateTagsInput{ Resources: []*string{&mutatedInstanceId}, Tags: []*ec2.Tag{ { @@ -62,6 +68,9 @@ func TestAcc_AwsInstance_WithBlockDevices(t *testing.T) { }, }, }) + if err != nil { + t.Fatal(err) + } }, Check: func(result *acceptance.ScanResult, stdout string, err error) { if err != nil { diff --git a/pkg/resource/aws/aws_route53_record_test.go b/pkg/resource/aws/aws_route53_record_test.go index 48b3d493..4c4176f8 100644 --- a/pkg/resource/aws/aws_route53_record_test.go +++ b/pkg/resource/aws/aws_route53_record_test.go @@ -12,6 +12,9 @@ func TestAcc_AwsRoute53Record_WithFQDNAsId(t *testing.T) { Args: []string{"scan", "--filter", "Type=='aws_route53_record'"}, Checks: []acceptance.AccCheck{ { + Env: map[string]string{ + "AWS_REGION": "us-east-1", + }, Check: func(result *acceptance.ScanResult, stdout string, err error) { if err != nil { t.Fatal(err) diff --git a/pkg/resource/aws/aws_s3_bucket_test.go b/pkg/resource/aws/aws_s3_bucket_test.go index 75391681..f298fd5a 100644 --- a/pkg/resource/aws/aws_s3_bucket_test.go +++ b/pkg/resource/aws/aws_s3_bucket_test.go @@ -12,6 +12,9 @@ func TestAcc_AwsS3Bucket_BucketInUsEast1(t *testing.T) { Args: []string{"scan", "--filter", "Type=='aws_s3_bucket'"}, Checks: []acceptance.AccCheck{ { + Env: map[string]string{ + "AWS_REGION": "us-east-1", + }, Check: func(result *acceptance.ScanResult, stdout string, err error) { if err != nil { t.Fatal(err) diff --git a/pkg/resource/aws/testdata/acc/aws_instance/providers.tf b/pkg/resource/aws/testdata/acc/aws_instance/providers.tf index 8234795a..b3f4019c 100644 --- a/pkg/resource/aws/testdata/acc/aws_instance/providers.tf +++ b/pkg/resource/aws/testdata/acc/aws_instance/providers.tf @@ -1,5 +1,5 @@ provider "aws" { - region = "eu-west-3" + region = "us-east-1" } terraform { required_providers { @@ -7,4 +7,4 @@ terraform { version = "~> 3.19.0" } } -} \ No newline at end of file +} diff --git a/pkg/resource/aws/testdata/acc/aws_internet_gateway/.terraform.lock.hcl b/pkg/resource/aws/testdata/acc/aws_internet_gateway/.terraform.lock.hcl index 4c3c17a7..8076e21f 100755 --- a/pkg/resource/aws/testdata/acc/aws_internet_gateway/.terraform.lock.hcl +++ b/pkg/resource/aws/testdata/acc/aws_internet_gateway/.terraform.lock.hcl @@ -5,6 +5,7 @@ provider "registry.terraform.io/hashicorp/aws" { version = "3.19.0" constraints = "3.19.0" hashes = [ + "h1:+7Vi7p13+cnrxjXbfJiTimGSFR97xCaQwkkvWcreLns=", "h1:xur9tF49NgsovNnmwmBR8RdpN8Fcg1TD4CKQPJD6n1A=", "zh:185a5259153eb9ee4699d4be43b3d509386b473683392034319beee97d470c3b", "zh:2d9a0a01f93e8d16539d835c02b8b6e1927b7685f4076e96cb07f7dd6944bc6c", diff --git a/pkg/resource/aws/testdata/acc/aws_route53_record/providers.tf b/pkg/resource/aws/testdata/acc/aws_route53_record/providers.tf index 8234795a..b3f4019c 100644 --- a/pkg/resource/aws/testdata/acc/aws_route53_record/providers.tf +++ b/pkg/resource/aws/testdata/acc/aws_route53_record/providers.tf @@ -1,5 +1,5 @@ provider "aws" { - region = "eu-west-3" + region = "us-east-1" } terraform { required_providers { @@ -7,4 +7,4 @@ terraform { version = "~> 3.19.0" } } -} \ No newline at end of file +} diff --git a/test/acceptance/result.go b/test/acceptance/result.go index db887c28..a5e1bdfd 100644 --- a/test/acceptance/result.go +++ b/test/acceptance/result.go @@ -60,7 +60,7 @@ func (r *ScanResult) AssertResourceHasDrift(id, ty string, change analyser.Chang } } if !found { - r.Failf("no differences found", "%s(%s)", id, ty) + r.Failf("no differences found", "%s (%s)", id, ty) } } diff --git a/test/acceptance/testing.go b/test/acceptance/testing.go index 1a4c86f5..198fc7b4 100644 --- a/test/acceptance/testing.go +++ b/test/acceptance/testing.go @@ -40,6 +40,7 @@ type AccTestCase struct { OnEnd func() Checks []AccCheck tmpResultFilePath string + originalEnv []string } func (c *AccTestCase) createResultFile(t *testing.T) error { @@ -188,6 +189,28 @@ func runDriftCtlCmd(driftctlCmd *cmd.DriftctlCmd) (*cobra.Command, string, error return cmd, out, cmdErr } +func (c *AccTestCase) useTerraformEnv() { + c.originalEnv = os.Environ() + c.setEnv(c.resolveTerraformEnv()) +} + +func (c *AccTestCase) restoreEnv() { + if c.originalEnv != nil { + logrus.Debug("Restoring original environment ...") + os.Clearenv() + c.setEnv(c.originalEnv) + c.originalEnv = nil + } +} + +func (c *AccTestCase) setEnv(env []string) { + os.Clearenv() + for _, e := range env { + envKeyValue := strings.SplitN(e, "=", 2) + os.Setenv(envKeyValue[0], envKeyValue[1]) + } +} + func Run(t *testing.T, c AccTestCase) { if os.Getenv("DRIFTCTL_ACC") == "" { @@ -219,6 +242,7 @@ func Run(t *testing.T, c AccTestCase) { } defer func() { + c.restoreEnv() err := c.terraformDestroy() os.Setenv("CHECKPOINT_DISABLE", checkpoint) if err != nil { @@ -248,7 +272,9 @@ func Run(t *testing.T, c AccTestCase) { t.Fatal("Check attribute must be defined") } if check.PreExec != nil { + c.useTerraformEnv() check.PreExec() + c.restoreEnv() } if len(check.Env) > 0 { for key, value := range check.Env {