Skip to content

Commit

Permalink
[fix] [testing] make TDD more productive (#206)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ryanking authored Jun 29, 2020
1 parent 9c9ef09 commit 1fe821e
Show file tree
Hide file tree
Showing 8 changed files with 303 additions and 23 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
*.tfstate.backup
bin
.idea
.test-data
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ clean:
rm **/*.tfstate*; true
.PHONY: clean

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

test-ci:
Expand Down
9 changes: 6 additions & 3 deletions aws-s3-private-bucket/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,12 @@ data "aws_iam_policy_document" "bucket_policy" {
source_json = var.bucket_policy

statement {
sid = "EnforceTLS"
actions = ["*"]
resources = ["arn:aws:s3:::${var.bucket_name}/*"]
sid = "EnforceTLS"
actions = ["*"]
resources = [
"arn:aws:s3:::${var.bucket_name}",
"arn:aws:s3:::${var.bucket_name}/*",
]

principals {
type = "*"
Expand Down
122 changes: 105 additions & 17 deletions aws-s3-private-bucket/module_test.go
Original file line number Diff line number Diff line change
@@ -1,33 +1,121 @@
package test

import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/service/s3"
"github.com/chanzuckerberg/cztack/testutil"
"github.com/gruntwork-io/terratest/modules/aws"
"github.com/gruntwork-io/terratest/modules/terraform"
"github.com/stretchr/testify/require"
)

func TestPrivateBucket(t *testing.T) {
func TestPrivateBucketDefaults(t *testing.T) {

project := testutil.UniqueId()
env := testutil.UniqueId()
service := testutil.UniqueId()
owner := testutil.UniqueId()
test := &testutil.Test{
Options: func(t *testing.T) *terraform.Options {
project := testutil.UniqueId()
env := testutil.UniqueId()
service := testutil.UniqueId()
owner := testutil.UniqueId()

bucketName := testutil.UniqueId()
bucketName := testutil.UniqueId()

options := testutil.Options(
testutil.DefaultRegion,
map[string]interface{}{
"project": project,
"env": env,
"service": service,
"owner": owner,
return testutil.Options(
testutil.DefaultRegion,
map[string]interface{}{
"project": project,
"env": env,
"service": service,
"owner": owner,

"bucket_name": bucketName,
"bucket_name": bucketName,
},
)
},
)

defer terraform.Destroy(t, options)
testutil.Run(t, options)
Validate: func(t *testing.T, options *terraform.Options) {
r := require.New(t)
region := options.EnvVars["AWS_DEFAULT_REGION"]
bucket := options.Vars["bucket_name"].(string)
outputs := terraform.OutputAll(t, options)
bucketArn := outputs["arn"].(string)

// some assertions built into terratest
aws.AssertS3BucketExists(t, region, bucket)
aws.AssertS3BucketPolicyExists(t, region, bucket)
aws.AssertS3BucketVersioningExists(t, region, bucket)

bucketPolicy := aws.GetS3BucketPolicy(t, region, bucket)
policy, err := testutil.UnmarshalS3BucketPolicy(bucketPolicy)
r.NoError(err)
r.NotNil(policy)

r.Len(policy.Statement, 1)
r.Equal(policy.Statement[0].Effect, "Deny")
r.Equal(policy.Statement[0].Principal, "*")
r.Equal(policy.Statement[0].Action, "*")
r.Len(policy.Statement[0].Condition, 1)
r.Equal(policy.Statement[0].Condition["Bool"]["aws:SecureTransport"], "false")

// get a client to query for other assertions
s3Client := aws.NewS3Client(t, region)

acl, err := s3Client.GetBucketAcl(&s3.GetBucketAclInput{
Bucket: &bucket,
})

r.NoError(err)
r.Len(acl.Grants, 1)

r.Equal("CanonicalUser", *acl.Grants[0].Grantee.Type)
r.Equal("FULL_CONTROL", *acl.Grants[0].Permission)

enc, err := s3Client.GetBucketEncryption(&s3.GetBucketEncryptionInput{
Bucket: &bucket,
})
r.NoError(err)

r.NotNil(enc.ServerSideEncryptionConfiguration)
r.Len(enc.ServerSideEncryptionConfiguration.Rules, 1)

block, err := s3Client.GetPublicAccessBlock(&s3.GetPublicAccessBlockInput{
Bucket: &bucket,
})
r.NoError(err)
r.NotNil(block)
r.True(*block.PublicAccessBlockConfiguration.BlockPublicAcls)
r.True(*block.PublicAccessBlockConfiguration.BlockPublicPolicy)
r.True(*block.PublicAccessBlockConfiguration.IgnorePublicAcls)
r.True(*block.PublicAccessBlockConfiguration.RestrictPublicBuckets)

// policy simulations

objectArn := fmt.Sprintf("%s/foo", bucketArn)

sims := []struct {
action string
secureTransport bool
arn string
result string
}{
// deny when not using https
{"s3:ListBucket", false, bucketArn, "explicitDeny"},
// allow with https
{"s3:ListBucket", true, bucketArn, "allowed"},
// deny when not using https
{"s3:GetObject", false, objectArn, "explicitDeny"},
// allow with https
{"s3:GetObject", true, objectArn, "allowed"},
}

for _, test := range sims {
resp := testutil.S3SimulateRequest(t, region, test.action, test.arn, bucketPolicy, test.secureTransport)
r.Equal(test.result, *resp.EvalDecision)
}
},
}

test.Run(t)
}
Loading

0 comments on commit 1fe821e

Please sign in to comment.