Skip to content

Commit 1fe821e

Browse files
authored
[fix] [testing] make TDD more productive (#206)
This uses a technique inspired by [terratest's test-structure](https://github.com/gruntwork-io/terratest/tree/master/modules/test-structure) tools to make it easier to iterate on this code. The idea is that each test will be broken into stages 'options', 'apply', 'validate' and 'cleanup' which can be skipped but also persist any state (like randomly generated options). So you can do something like: `make test TEST=./aws-s3-private-bucket SKIP=cleanup` Which will 1. run 'options' to generate and persist a set of random variables 2. run 'apply' which will `terraform apply` to create resources and persist the state file 3. run 'validate' to make assertions about the resources created 4. skip 'cleanup' This time will be dominated by how long it takes to apply the changes (seconds to ~1hr). Then if you run: `make test TEST=./aws-s3-private-bucket SKIP=cleanup,options,apply` It will only run the validate step, which takes < 10s. You can re run this, iterating on your tests until you are satisfied. At some point you might realize you need to update the terraform code. After you make those edits you can run: `make test TEST=./aws-s3-private-bucket SKIP=cleanup,options` which will 1. skip options and use the persisted options 2. run apply, using existing options and state file. This will mean that for some resources you can do an update-in-place, potentially much faster. 3. run validate 4. skip cleanup When all done, you can run cleanup and then re-run with no skips to make sure it works end-to-end.
1 parent 9c9ef09 commit 1fe821e

File tree

8 files changed

+303
-23
lines changed

8 files changed

+303
-23
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,4 @@
33
*.tfstate.backup
44
bin
55
.idea
6+
.test-data

Makefile

+2-2
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ clean:
6868
rm **/*.tfstate*; true
6969
.PHONY: clean
7070

71-
test: fmt
72-
go test -count=1 -parallel 10 -test.timeout 45m $(TEST)
71+
test:
72+
go test -count=1 -v -parallel 10 -test.timeout 45m $(TEST)
7373
.PHONY: test
7474

7575
test-ci:

aws-s3-private-bucket/main.tf

+6-3
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,12 @@ data "aws_iam_policy_document" "bucket_policy" {
123123
source_json = var.bucket_policy
124124

125125
statement {
126-
sid = "EnforceTLS"
127-
actions = ["*"]
128-
resources = ["arn:aws:s3:::${var.bucket_name}/*"]
126+
sid = "EnforceTLS"
127+
actions = ["*"]
128+
resources = [
129+
"arn:aws:s3:::${var.bucket_name}",
130+
"arn:aws:s3:::${var.bucket_name}/*",
131+
]
129132

130133
principals {
131134
type = "*"

aws-s3-private-bucket/module_test.go

+105-17
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,121 @@
11
package test
22

33
import (
4+
"fmt"
45
"testing"
56

7+
"github.com/aws/aws-sdk-go/service/s3"
68
"github.com/chanzuckerberg/cztack/testutil"
9+
"github.com/gruntwork-io/terratest/modules/aws"
710
"github.com/gruntwork-io/terratest/modules/terraform"
11+
"github.com/stretchr/testify/require"
812
)
913

10-
func TestPrivateBucket(t *testing.T) {
14+
func TestPrivateBucketDefaults(t *testing.T) {
1115

12-
project := testutil.UniqueId()
13-
env := testutil.UniqueId()
14-
service := testutil.UniqueId()
15-
owner := testutil.UniqueId()
16+
test := &testutil.Test{
17+
Options: func(t *testing.T) *terraform.Options {
18+
project := testutil.UniqueId()
19+
env := testutil.UniqueId()
20+
service := testutil.UniqueId()
21+
owner := testutil.UniqueId()
1622

17-
bucketName := testutil.UniqueId()
23+
bucketName := testutil.UniqueId()
1824

19-
options := testutil.Options(
20-
testutil.DefaultRegion,
21-
map[string]interface{}{
22-
"project": project,
23-
"env": env,
24-
"service": service,
25-
"owner": owner,
25+
return testutil.Options(
26+
testutil.DefaultRegion,
27+
map[string]interface{}{
28+
"project": project,
29+
"env": env,
30+
"service": service,
31+
"owner": owner,
2632

27-
"bucket_name": bucketName,
33+
"bucket_name": bucketName,
34+
},
35+
)
2836
},
29-
)
3037

31-
defer terraform.Destroy(t, options)
32-
testutil.Run(t, options)
38+
Validate: func(t *testing.T, options *terraform.Options) {
39+
r := require.New(t)
40+
region := options.EnvVars["AWS_DEFAULT_REGION"]
41+
bucket := options.Vars["bucket_name"].(string)
42+
outputs := terraform.OutputAll(t, options)
43+
bucketArn := outputs["arn"].(string)
44+
45+
// some assertions built into terratest
46+
aws.AssertS3BucketExists(t, region, bucket)
47+
aws.AssertS3BucketPolicyExists(t, region, bucket)
48+
aws.AssertS3BucketVersioningExists(t, region, bucket)
49+
50+
bucketPolicy := aws.GetS3BucketPolicy(t, region, bucket)
51+
policy, err := testutil.UnmarshalS3BucketPolicy(bucketPolicy)
52+
r.NoError(err)
53+
r.NotNil(policy)
54+
55+
r.Len(policy.Statement, 1)
56+
r.Equal(policy.Statement[0].Effect, "Deny")
57+
r.Equal(policy.Statement[0].Principal, "*")
58+
r.Equal(policy.Statement[0].Action, "*")
59+
r.Len(policy.Statement[0].Condition, 1)
60+
r.Equal(policy.Statement[0].Condition["Bool"]["aws:SecureTransport"], "false")
61+
62+
// get a client to query for other assertions
63+
s3Client := aws.NewS3Client(t, region)
64+
65+
acl, err := s3Client.GetBucketAcl(&s3.GetBucketAclInput{
66+
Bucket: &bucket,
67+
})
68+
69+
r.NoError(err)
70+
r.Len(acl.Grants, 1)
71+
72+
r.Equal("CanonicalUser", *acl.Grants[0].Grantee.Type)
73+
r.Equal("FULL_CONTROL", *acl.Grants[0].Permission)
74+
75+
enc, err := s3Client.GetBucketEncryption(&s3.GetBucketEncryptionInput{
76+
Bucket: &bucket,
77+
})
78+
r.NoError(err)
79+
80+
r.NotNil(enc.ServerSideEncryptionConfiguration)
81+
r.Len(enc.ServerSideEncryptionConfiguration.Rules, 1)
82+
83+
block, err := s3Client.GetPublicAccessBlock(&s3.GetPublicAccessBlockInput{
84+
Bucket: &bucket,
85+
})
86+
r.NoError(err)
87+
r.NotNil(block)
88+
r.True(*block.PublicAccessBlockConfiguration.BlockPublicAcls)
89+
r.True(*block.PublicAccessBlockConfiguration.BlockPublicPolicy)
90+
r.True(*block.PublicAccessBlockConfiguration.IgnorePublicAcls)
91+
r.True(*block.PublicAccessBlockConfiguration.RestrictPublicBuckets)
92+
93+
// policy simulations
94+
95+
objectArn := fmt.Sprintf("%s/foo", bucketArn)
96+
97+
sims := []struct {
98+
action string
99+
secureTransport bool
100+
arn string
101+
result string
102+
}{
103+
// deny when not using https
104+
{"s3:ListBucket", false, bucketArn, "explicitDeny"},
105+
// allow with https
106+
{"s3:ListBucket", true, bucketArn, "allowed"},
107+
// deny when not using https
108+
{"s3:GetObject", false, objectArn, "explicitDeny"},
109+
// allow with https
110+
{"s3:GetObject", true, objectArn, "allowed"},
111+
}
112+
113+
for _, test := range sims {
114+
resp := testutil.S3SimulateRequest(t, region, test.action, test.arn, bucketPolicy, test.secureTransport)
115+
r.Equal(test.result, *resp.EvalDecision)
116+
}
117+
},
118+
}
119+
120+
test.Run(t)
33121
}

0 commit comments

Comments
 (0)