Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add aws custom configuration support for local testing (i.e: localstack use) #495

Open
wants to merge 98 commits into
base: master
Choose a base branch
from

Conversation

ffernandezcast
Copy link

@ffernandezcast ffernandezcast commented Apr 7, 2020

Issues related:

The goal is be able to use Terratest assert like AssertS3BucketExists without receive credentials issues because is trying to use aws cloud and not localstack terraform provider config

i.e:

include in your test something like:

Global test config file

var LocalEndpoints = map[string]string{
	"apigateway":     "http://localhost:4567",
	"cloudformation": "http://localhost:4581",
	"cloudwatch":     "http://localhost:4582",
	"dynamodb":       "http://localhost:4569",
	"es":             "http://localhost:4578",
	"firehose":       "http://localhost:4573",
	"iam":            "http://localhost:4593",
	"kinesis":        "http://localhost:4568",
	"lambda":         "http://localhost:4574",
	"route53":        "http://localhost:4580",
	"redshift":       "http://localhost:4577",
	"s3":             "http://localhost:4572",
	"secretsmanager": "http://localhost:4584",
	"ses":            "http://localhost:4579",
	"sns":            "http://localhost:4575",
	"sqs":            "http://localhost:4576",
	"ssm":            "http://localhost:4583",
	"stepfunctions":  "http://localhost:4585",
	"sts":            "http://localhost:4592",
}

Inside Global Setup Tests Suite File

func TestMain(m *testing.M) {
     aws.SetAwsEndpointsOverrides(LocalEndpoints)
}

@ffernandezcast ffernandezcast changed the title Add aws custom configuration support for localstack use Add aws custom configuration support for local testing (i.e: localstack use) Apr 7, 2020
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

modules/aws/config.go Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
modules/aws/auth.go Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
@ffernandezcast ffernandezcast force-pushed the add-aws-custom-configuration-support branch from 901bf9e to bd55bbf Compare April 8, 2020 12:44
.circleci/config.yml Outdated Show resolved Hide resolved
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates! I suggested a few more changes. One other important thing: we need to add an automated test of some kind for this behavior.

modules/aws/auth.go Show resolved Hide resolved
modules/aws/auth.go Outdated Show resolved Hide resolved
modules/aws/auth.go Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay; I was on vacation. Left a few more thoughts.

.circleci/config.yml Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
modules/aws/config.go Outdated Show resolved Hide resolved
@ffernandezcast ffernandezcast force-pushed the add-aws-custom-configuration-support branch 2 times, most recently from a347f4e to ae936ee Compare September 10, 2020 18:25
@brikis98
Copy link
Member

brikis98 commented Mar 1, 2021

Sorry folks, we are unusually busy right now, and doing our best to work through the backlog of issues/PRs. We'll get to this as soon as we can.

@synfinatic
Copy link

Been over two months now since last update. I'm hoping to open source some code for terratest & localstack soon and I'd really like to avoid requiring people to pin a custom fork for terratest in their go.mod file.

@synfinatic
Copy link

So one thing that this PR does not support is running multiple localstack containers for testing in parallel. This is because the variables storing the AWS Endpoints are global. Ideally this would not be the case, but TBH this is better than nothing. That said, I'm not sure adding this is supported without massive changes to terratest.

@synfinatic
Copy link

synfinatic commented Oct 12, 2021

Ugh, this PR has been open for 18 months and now has a merge conflict because of that. Right now I have code I'd like to open source which depends on this being merged, but it just doesn't make sense to release it if I have to maintain a fork.

@brikis98 How can I help move this forward? Or is this PR/solution dead in the water?

@whummer
Copy link

whummer commented Oct 12, 2021

This looks awesome - would also love to see this getting merged. 👍 Anything we can do to help push this forward? @ffernandezcast @brikis98 Thanks! 🙏

@ffernandezcast
Copy link
Author

ffernandezcast commented Oct 18, 2021

This looks awesome - would also love to see this getting merged. 👍 Anything we can do to help push this forward? @ffernandezcast @brikis98 Thanks! 🙏

I did a quick update. Maybe could you test it to verify if keep backward compatibility. @whummer

@ffernandezcast
Copy link
Author

Ugh, this PR has been open for 18 months and now has a merge conflict because of that. Right now I have code I'd like to open source which depends on this being merged, but it just doesn't make sense to release it if I have to maintain a fork.

@brikis98 How can I help move this forward? Or is this PR/solution dead in the water?

I did a quick update. Maybe could you test it to verify if keep backward compatibility. @synfinatic

@synfinatic
Copy link

@ffernandezcast : Yep, all my unit tests pass with your latest PR.

@oblogic7
Copy link

Would be nice to get this merged. Any reason it is sitting for so long?

@tata2000
Copy link

tata2000 commented May 5, 2022

Are there any updates on this PR? Will this PR be merged at all?

@jimsheldon
Copy link

jimsheldon commented Jun 2, 2022

It would be great to have this merged!

https://github.com/spulec/moto is another use case for this feature

@segator
Copy link

segator commented Jul 10, 2022

why is this not yet merged? how people test with localstack?

@brikis98
Copy link
Member

Hey folks, my apologies for the continued delay here. There are two things at play here:

  1. To properly support LocalStack, there are two areas to consider, and there are open questions with both:
    1. Updating Terratest's own helpers to be able to talk to LocalStack. This is what this PR does, which is handy. However, it's only part of the story. Moreover, there are some open questions here to think through: e.g., the list of localEndpoints is already out of date, so how do we do this in a maintainable way?
    2. Updating the code you're deploying with Terratest to talk to LocalStack. For example, if you're using Terratest's terraform.Apply(t testing.T, opts *terraform.Options) helper to deploy a Terraform module, it would make sense to update the terraform.Options struct with a UseLocalStack parameter, that if set, overrides the Terraform module's provider configuration to use LocalStack (similar to the tflocal script). I don't think this PR does this yet. Moreover, there are open questions here to think through as well: e.g., do we just call tflocal directly or re-implement its logic in Go (and how maintainable will that be)?
  2. We are very overloaded right now:
    1. We're a small, bootstrapped company, so we have to be very picky in where we spend our time & resources. We do our best to maintain all aspects of Terratest that we depend on (e.g., the Terraform helpers, Docker helpers, K8S helpers, etc), but we don't currently use LocalStack ourselves. So it has been hard to justify carving out our time to think through those open questions. That said, we should've done a better job of updating you all on this!
    2. Note that we are currently working on our roadmap for the next few months and using LocalStack is one of the items we're considering. I don't know if it'll make the cut in the near term, but when it does, then we'll come back and give more thought to this PR. If not, others are welcome to think through the open questions above, and we'll try to respond if we can, but please understand we're doing our best to maintain this (fairly popular) open source project with limited resources, so we have to pick our battles.

@synfinatic
Copy link

@brikis98 Well, it's been 18mo since I started following this PR and honestly I no longer care at this point. I've killed my project which was dependent on this PR and moved on to other projects.

@whummer
Copy link

whummer commented Aug 1, 2022

Hi @brikis98 , thanks for the detailed response!

the list of localEndpoints is already out of date, so how do we do this in a maintainable way?

That's a great point - in fact, the list of services would need to be adjusted to cover the latest additions to LocalStack. The endpoint URL is the same for all services (port 4566 by default), so we can parameterize the list of services, then generate the localEndpoints map dynamically. 👍

it would make sense to update the terraform.Options struct with a UseLocalStack parameter, that if set, overrides the Terraform module's provider configuration to use LocalStack

Thanks for the pointer - that would be quite useful indeed!

LocalStack is one of the items we're considering [...] so we have to pick our battles

That's great, glad to hear that! And yes, very much understandable that you need to pick your battles. I'll see if I or someone from the LocalStack team can help address the points you've raised, so we can jointly work on getting this over the line. Thanks for your help!


@ffernandezcast Would you be ok if we work on this PR collaboratively, could you please give me push access to your fork/branch? (Alternatively, I could branch off your PR, and make sure that your original authorship is retained..) Thanks!

@blairham
Copy link

blairham commented Dec 2, 2022

@ ffernandezcast Thank you for this, I've had to fork this repo and apply your patch. Works perfectly, too bad GruntWork hasn't addressed this in the 2+ years now

@ffernandezcast
Copy link
Author

ffernandezcast commented Dec 2, 2022

👏 thanks @blairham . It's a pity after we the community members invest our time for free and also promote and evangelise about framework use. Also I should answer to @brikis98 comments because I feel there is some misunderstanding in his comments about the proposal

@coding-yogi
Copy link

@brikis98 , there are two different things we are talking about

  1. To support terratest terraform APIs to talk to localstack
  2. To support terratest aws APIs to talk to localstack

First point is not even an issue currently as this can be easily handled from the .tf files with absolutely no changes to terratest APIs. I am not sure why this has to be even considered for this PR

PR is targeting point 2 where we are unable to use the aws module just because we cannot set an endpoint to do so.

Regarding your comment on time crunch, I can fully understand and that is why there is a community to help. When @ffernandezcast has spent considerable time on PR and even you have spent time reviewing it, I don't understand where its stuck? Reviewer's team has to spend time reviewing it, which is already being done.

This PR is fulfilling the purpose it was open for. It is not meant to do a full-fledge support for LocalStack as such. Cos this implementation can support moto as well which has nothing to do with localstack. I request you to re-look into this PR as its almost ready. This can help many of us 🙏

@blairham
Copy link

blairham commented Apr 28, 2023

👏 thanks @blairham . It's a pity after we the community members invest our time for free and also promote and evangelise about framework use. Also I should answer to @brikis98 comments because I feel there is some misunderstanding in his comments about the proposal

I have it working flawlessly in my own fork, it is a pity as I opened an MR but they wanted way too much from me to merge it. I'll continue to use my fork and maybe open it up to everyone else so the community can provide input

If you're interested, it's an easy fix with:
#1211

MatMoore added a commit to MatMoore/terraform-testing-examples that referenced this pull request Aug 30, 2023
It seems like there is still active development to make terratest work well with localstack.
See gruntwork-io/terratest#495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet