Tests and semaphoreci integration #2

soerenmartius · 2020-03-10T13:30:59Z

Implement an integration test:

Create ECR repository
Build a docker image
Authenticate with ECR
Push the docker image to ECR
Use Docker-out-of-Docker ( DooD ) approach to build docker image inside the build-tools container

mariux

lgtm

soerenmartius · 2020-03-11T11:28:17Z

Hi Marius,

The code is still a bit messy but it should be clear what we're doing here:

Create ECR repository
Build a docker image
Authenticate with ECR
Push the docker image to EC

Executing the test with AWS_ACCESS_KEY_ID=XXXX AWS_SECRET_ACCESS_KEY=XXXX go test -v -parallel 128 ./... works well now.

However, we currently can't run the test with our (https://hub.docker.com/r/mineiros/build-tools)[build-tools} docker image. If we'd like to be able to run the test inside a docker container, we need to add docker as a dependency to the build-tools image and start a privileged container. I don't think that this is a good practice though.

I would suggest, that we use a packer image to create and push the test docker image. However, that would come with a few downsides:

We can't cache docker builds with packer
The test would take longer
The test would likely create costs because packer spans up an EC2 instance.

wdyt @mariux ?

soerenmartius · 2020-03-11T11:28:42Z

lgtm

We shouldn't approve PR's with failing tests though...

…de the build-tools container

soerenmartius · 2020-03-15T20:09:34Z

Hi Marius,

The code is still a bit messy but it should be clear what we're doing here:

Create ECR repository

Build a docker image

Authenticate with ECR

Push the docker image to EC

Executing the test with AWS_ACCESS_KEY_ID=XXXX AWS_SECRET_ACCESS_KEY=XXXX go test -v -parallel 128 ./... works well now.

However, we currently can't run the test with our ([https://hub.docker.com/r/mineiros/build-tools)build-tools}%5Bbuild-tools%7D) docker image. If we'd like to be able to run the test inside a docker container, we need to add docker as a dependency to the build-tools image and start a privileged container. I don't think that this is a good practice though.

I would suggest, that we use a packer image to create and push the test docker image. However, that would come with a few downsides:

We can't cache docker builds with packer

The test would take longer

The test would likely create costs because packer spans up an EC2 instance.

wdyt @mariux ?

For now, I chose the easiest way possible: Docker out of Docker

…he created repository.

soerenmartius · 2020-03-15T23:08:42Z

Ready for review @mariux

soerenmartius · 2020-03-16T06:52:59Z

I just saw that whenever we use terraform output, terratest writes the full output to stdout. It's not critical, since the created IAM User comes without any implicit permissions and will be destroyed after the test finished.
Still, it's not acceptable and a no go. I am gonna open a PR for terratest to suppress output logs for sensitive outputs.

soerenmartius · 2020-03-16T07:09:35Z

I just saw that whenever we use terraform output, terratest writes the full output to stdout. It's not critical, since the created IAM User comes without any implicit permissions and will be destroyed after the test finished.
Still, it's not acceptable and a no go. I am gonna open a PR for terratest to suppress output logs for sensitive outputs.

gruntwork-io/terratest#476

mariux

we should discuss how we want to deliver outputs... as this duplicates exported data and creates documentation overhead.

mariux · 2020-03-16T11:30:05Z

outputs.tf

+output "repository_arn" {
+  description = "The ARN of the repository."
+  value       = try(aws_ecr_repository.repository[0].arn, null)
+}
+
+output "repository_id" {
+  description = "The Id of the repository."
+  value       = try(aws_ecr_repository.repository[0].id, null)
+}
+
+output "repository_url" {
+  description = "The URL of the repository."
+  value       = try(aws_ecr_repository.repository[0].repository_url, null)
+}
+


why? you can get them from the repository itself

module.name.repository.arn module.name.repository_arn

fixed in aaa3cc7

My take on outputs:

I don't have a strong opinion yet but here are some bullet points that lead me in the direction of explicit outputs:

returning the whole object doesn't really create a contract / valid API. What if the internal representation changes due to Provider upgrade? The module won't be able to guarantee backward compatibility anymore. If we return explicit outputs we can guarantee BC.

What does the user really want? Returning the whole resource will most likely end in a scenario in which the user only uses 20% of the outputs anyways ... do we really need all information?

We might return sensitive outputs per default if we always return the whole object?

No convenient API - the user would need to check the terraform providers documentation to get an overview of the outputs.

I will post this in the related trello task also.
wdyt?

Also: I think we might be ready to get this merged now ? :)

mariux

Lgtm

soerenmartius added 2 commits March 10, 2020 08:50

add semaphoreci integration and basic test case

cdc2f90

add more useful comments to all files

780aa08

soerenmartius requested a review from mariux as a code owner March 10, 2020 13:30

soerenmartius added 2 commits March 10, 2020 15:14

add lifecycle policy rules to example

9fed011

add docker build to example

6ef6dd0

mariux approved these changes Mar 10, 2020

View reviewed changes

mariux previously approved these changes Mar 10, 2020

View reviewed changes

soerenmartius dismissed mariux’s stale review via 7bea34b March 11, 2020 11:23

soerenmartius force-pushed the tests-and-semaphoreci-integration branch from 7bea34b to 537c203 Compare March 11, 2020 13:41

build docker image, authenticate with ecr and push the image

a7f2067

soerenmartius force-pushed the tests-and-semaphoreci-integration branch from 537c203 to a7f2067 Compare March 11, 2020 23:53

soerenmartius added 4 commits March 13, 2020 22:12

upgrade terratest to v.0.26.0

061884d

disable go cache for tests

d0e6c10

format code properly

bd22658

Use Docker-out-of-Docker ( DooD ) approach to build docker image insi…

8f8a49f

…de the build-tools container

soerenmartius requested a review from mariux March 15, 2020 20:13

create a new iam user that only has permissions to pull and push to t…

ee259d4

…he created repository.

don't write secrets to file

f4bd5e6

mariux requested changes Mar 16, 2020

View reviewed changes

soerenmartius added 2 commits March 16, 2020 18:57

remove duplicate outputs

aaa3cc7

fix typo in documnetation

53b8cac

mariux approved these changes Mar 17, 2020

View reviewed changes

soerenmartius merged commit c19bcc1 into master Mar 17, 2020

soerenmartius deleted the tests-and-semaphoreci-integration branch March 17, 2020 10:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests and semaphoreci integration #2

Tests and semaphoreci integration #2

soerenmartius commented Mar 10, 2020 •

edited

Loading

mariux left a comment

soerenmartius commented Mar 11, 2020

soerenmartius commented Mar 11, 2020

soerenmartius commented Mar 15, 2020 •

edited

Loading

soerenmartius commented Mar 15, 2020

soerenmartius commented Mar 16, 2020

soerenmartius commented Mar 16, 2020

mariux left a comment

mariux Mar 16, 2020

soerenmartius Mar 16, 2020

soerenmartius Mar 16, 2020

mariux left a comment

Tests and semaphoreci integration #2

Tests and semaphoreci integration #2

Conversation

soerenmartius commented Mar 10, 2020 • edited Loading

mariux left a comment

Choose a reason for hiding this comment

soerenmartius commented Mar 11, 2020

soerenmartius commented Mar 11, 2020

soerenmartius commented Mar 15, 2020 • edited Loading

soerenmartius commented Mar 15, 2020

soerenmartius commented Mar 16, 2020

soerenmartius commented Mar 16, 2020

mariux left a comment

Choose a reason for hiding this comment

mariux Mar 16, 2020

Choose a reason for hiding this comment

soerenmartius Mar 16, 2020

Choose a reason for hiding this comment

soerenmartius Mar 16, 2020

Choose a reason for hiding this comment

mariux left a comment

Choose a reason for hiding this comment

soerenmartius commented Mar 10, 2020 •

edited

Loading

soerenmartius commented Mar 15, 2020 •

edited

Loading