github.com/darmach/terratest@v0.34.8-0.20210517103231-80931f95e3ff/REFACTOR.md (about)

     1  # Terratest refactor
     2  
     3  Terratest started out as a set of Bash scripts we were using at Gruntwork to test some of our Terraform code. As the
     4  amount of Terraform code grew, it was getting trickier and trickier to test it with Bash, so we rewrote those scripts
     5  in Go. Over time, this Go code grew into a library called Terratest, which contains collection of utilities that we
     6  use to test all aspects of our [Infrastructure as Code Library](https://gruntwork.io/infrastructure-as-code-library/).
     7  
     8  We developed patterns to test Terraform configurations, Packer templates, Docker images, SSH access, AWS APIs, shell
     9  commands, and much more. We built this library because we couldn't find any existing tools out there that could do
    10  the type of real-world testing we needed. It turns out many other companies want to do this type of testing too, so
    11  now it's time to open source Terratest.
    12  
    13  This library grew organically, so it needs lots of refactoring, cleanup, and documentation to be useful to people
    14  outside of Gruntwork. This document lays out the refactoring we are planning to (a) get feedback and (b) document what
    15  changed so that when we update our code, we know how to deal with the backwards incompatibilities.
    16  
    17  
    18  
    19  
    20  ## Name change
    21  
    22  "Terratest" made sense as the name for this library when it was all about testing Terraform code, but now this library
    23  also can help you test Packer templates, Docker images, and much more. I propose that we rename it. Some ideas:
    24  
    25  - grunt-test
    26  - test-grunt
    27  - gruntUnit
    28  - iac-test
    29  - infratest
    30  
    31  
    32  
    33  
    34  ## Build updates
    35  
    36  1. Move from Glide to Dep
    37  1. Move from CircleCI 1.0 to 2.0
    38  1. Add support for Google Cloud Platform
    39  1. Move from Dep to Go Modules.
    40  
    41  
    42  ## Folder structure
    43  
    44  Change to the same folder structure we use for just about all other Gruntwork repos:
    45  
    46  - `examples`: This will contain a number of real-world examples of code you might want to test with Terratest, such as
    47    Terraform modules, Packer templates, and Docker images. The `test` folder (described below) shows how to use
    48    Terratest to test these examples.
    49  
    50  - `modules`: The Terratest source code. Move all the `.go` files and packages into this folder so it's easier to browse
    51    the repo. That does mean all Terratest imports will have to be updated to
    52    `github.com/gruntwork-io/terratest/modules/xxx`. Unit tests for the Go source code will be in this folder too (e.g., the
    53    unit test for `foo.go` will be in `foo_test.go`).
    54  
    55  - `test`: This will contain the automated tests for the examples in the `examples` folder. These will act both as an
    56    example of how to use Terratest, as well as integration tests for the library.
    57  
    58  
    59  
    60  
    61  ## Documentation
    62  
    63  Update the root `README.md` with documentation that shows how to use Terratest:
    64  
    65  - Overview of what Terratest is.
    66  - Link to blog post we'll write about Terratest (this blog post is a TODO for after this refactor).
    67  - Discuss some of the challenges of testing infrastructure code: i.e., lack of "localhost," lock of "unit tests,"
    68    slowness, brittleness.
    69  - Discuss the value of doing this testing despite the challenges: i.e., there is no way to maintain lots of
    70    infrastructure code without tests, building reusable, tested, versioned modules changes how you manage
    71    infrastructure.
    72  - Discussion of test strategies: using Docker for local testing, test stages, retries, mocks, small modules, test
    73    pyramid, cleanup, `cloud-nuke`.
    74  - Point to `examples` folder for real-world code you may want to test and `test` folder for examples of how to use
    75    Terratest to test that code.
    76  - Overview of Terratest packages. Explain what each top-level package in Terratest does. We can't do a method-by-method
    77    breakdown, as that would go out of date immediately, so instead, link to the appropriate `examples` subfolder that
    78    shows real-world usage of that package.
    79  - In the future: links to our open source repos (Vault, Consul, Nomad, Couchbase, etc) that show how we use Terratest
    80    with our own code. We can't add this until we update those open source repos to this refactored version of Terratest
    81    so the code matches up.
    82  
    83  
    84  
    85  
    86  ## Package-by-package refactor
    87  
    88  I've gone through each of the packages in Terratest and took down some notes on cleanup we need to do. This is not a
    89  comprehensive list, as things will become clearer once I actually start doing the work.
    90  
    91  In fact, my plan is to first create all the examples in the `examples` folder, then write tests for them in the `test`
    92  folder using "wishful thinking" (in the
    93  [SICP](https://www.amazon.com/Structure-Interpretation-Computer-Programs-Engineering/dp/0262510871) sense), where I
    94  come up with the test API I want to have for doing the testing, and then go and refactor the Terratest code to match.
    95  
    96  
    97  ### Root package
    98  
    99  We have a lot of stuff in the root package and I propose moving all of it out into appropriate sub-packages:
   100  
   101  - `apply.go`, `apply_and_destroy.go`, `destroy.go`, `output.go`, and `output_test.go` will all be moved into
   102    `modules/terraform`, as they are all specific to testing Terraform code.
   103  
   104  - I propose deleting `rand_resources.go` and `rand_resource_test.go` and extracting its logic into other places. The
   105    `RandomResourceCollection` ended up being a, well, random collection of resources, most of which don't apply to most
   106    of our tests, and certainly won't apply to tests written by the open source community. Here's what
   107    `RandomResourceCollection` contains and what I propose to do with it:
   108  
   109      - `UniqueId`: We already have a separate method for generating a unique ID and we can pass it around as a `string`.
   110  
   111      - `AwsRegion`: This is only needed for AWS tests. We want to expand Terratest to support other clouds, so it needs
   112        to be separated anyway. Code that needs an AWS region should call a method in the `modules/aws` package to pick a
   113        random AWS region (passing in a list of forbidden regions, if necessary) and can pass that around as a `string`.
   114  
   115      - `KeyPair`: This is only needed for a small percentage of our AWS tests that deploy EC2 Instances and SSH to them.
   116        Those tests should call a standalone method in the `modules/aws` package to generate this `KeyPair` when they need it,
   117        instead of us assuming every single test needs it.
   118  
   119      - `AmiId`: We used to look up vanilla Ubuntu or Amazon Linux AMI IDs and put them in this field, but now that
   120        Terraform has `data` sources and Packer has `source_ami_filter`, this is no longer necessary. We can keep the
   121        methods around to find Ubuntu or Amazon Linux AMI IDs for tests that need them, but there's no need to assume
   122        every single test needs this.
   123  
   124      - `AccountId`: Our Terraform examples used to require an account ID to be passed in. We now avoid this to make the
   125        examples easier to use, and fetch it automatically using Terraform's `aws_caller_identity` data source if it's
   126        absolutely necessary. Code that needs an account ID should call a method in the `modules/aws` package to fetch it,
   127        but we shouldn't assume every single test needs it.
   128  
   129      - `SnsTopicArn`: A very, very small percentage of our tests needed an SNS topic passed in. Those tests should call
   130        a method in the `modules/aws` package to create this topic instead of us assuming every single test needs it.
   131  
   132  - I propose moving `terratest_options.go` to `modules/terraform/options.go` and renaming the struct within it from
   133    `TerratestOptions` to `Options`, since this is solely used for testing Terraform code. We should also rename
   134    `TemplatePath` to `TerraformDir`, as `.tf` files are technically called "configurations" and not "templates".
   135  
   136  - `url_checker.go` will be deleted. It's too hard-coded for one specific type of check. The reuse value is limited and
   137    it's not obvious the code exists, so it's best for the test cases to reimplement this themselves, with their specific
   138    needs, even if it's a tiny bit less DRY.
   139  
   140  
   141  ### _docker-images package
   142  
   143  - Rename to `test-docker-images` to make it clearer these are only used for testing.
   144  - Use these Docker images in the `examples` folder to show how to do "unit tests" for Packer templates.
   145  - Follow-up PR: build and push a new version of these Docker images on each release?
   146  - Follow-up PR: tag each new Docker image with a unique version number (e.g., sha1 of commit).
   147  
   148  
   149  ### `aws` package
   150  
   151  - Right now, much of this code has no unit tests, since it relies on resources in AWS. By adding an `examples` folder
   152    that deploys real resources in AWS, we will be able to test this code better, _and_ show users how to use this code!
   153  
   154  - `ami.go`: Update these methods to use the AWS APIs to find the latest Ubuntu / Amazon Linux AMI IDs instead of
   155    hard-coding them.
   156  
   157  - `kms.go`: What to do about `GetDedicatedTestKeyArn`? For tests that use KMS, we don't want to create a new CMK each
   158    time the test runs, as AWS charges $1/month for CMKs, even if you delete them immediately after use. This method
   159    currently assumes we have a key called `alias/dedicated-test-key` in every AWS region. Should we leave it as-is and
   160    document it for Terratest users that want to follow a similar pattern? Or perhaps read the key name from an env var?
   161  
   162  - `region.go`: What should we do about `GetGloballyForbiddenRegions`? Right now, it's hard-coded to include `us-west-2`
   163    as a globally forbidden region, as Josh is running his personal blog there. Obviously, we don't want that in the open
   164    source version. Josh, can you finally migrate your blog out of there so we don't have to have this exception?
   165  
   166  
   167  ### `log` package
   168  
   169  - Rename to `logger`. That way, we don't have to alias it as `terralog` all over our test code.
   170  - Change what the package does. Instead of creating a custom `*log.Logger` and passing it around, we are going to have
   171    a `Log` and `Logf` method you can call from anywhere. To use those methods, you have to pass them a `*testing.T`,
   172    which they will use to read out the test name. We already pass `*testing.T` to almost all of our test methods, so
   173    this reduces the number of arguments by one.
   174  
   175  
   176  ### `parallel` package
   177  
   178  - I propose removing this package entirely. Now that go has [subtests](https://blog.golang.org/subtests) that you can
   179    easily run with `t.Run()` and parallelize with `t.Parallel()`, I think that's a cleaner way of handling parallelism
   180    than this custom package.
   181  
   182  
   183  ### `packer` package
   184  
   185  - Rename `PackerOptions` to `Options` (the package name is already `packer`).
   186  
   187  
   188  ### `resources` package
   189  
   190  - `base_resources.go` is no longer necessary if we remove `RandomResourceCollection`.
   191  - `exclusions.go` is not used much and very out of date.
   192  - `terraform_options.go` is hard-coded to how we do things at Gruntwork, but won't apply to many other users.
   193  
   194  
   195  ### `terraform` package
   196  
   197  - `apply.go`: Remove `terraformDebugEnv` and instead make it easy to pass a map of env vars to the `Apply` method.
   198    Refactor `ApplyAndGetOutputWithRetry` to accept a list of errors on which to retry and how many retries to do.
   199  
   200  
   201  ### `test-util` package
   202  
   203  - `dummy_server.go`: Move into the `modules/http` package.
   204  - Remove `test-util` since that would leave it empty!
   205  
   206  
   207  ### `util` package
   208  
   209  - `collections.go`: Move into its own `modules/collections` package.
   210  - `keygen.go`: Move into `modules/ssh` package.
   211  - `network.go`: Move into `modules/aws` package.
   212  - `sleep.go`: Remove. Didn't even know we had this and doubt it gets much use!
   213  - `random.go`: Move into its own `modules/random` package.
   214  - `retry.go`: Move into its own `modules/retry` package.
   215  
   216  
   217  
   218  
   219  ## Error handling
   220  
   221  I am updating most of the methods to support handling errors in one of two ways:
   222  
   223  1. Each method `foo` will take in a `*testing.T` and upon hitting an error, call `t.Fatal`.
   224  1. Each method `fooE` will explicitly return any errors it hits and NOT call `t.Fatal`.
   225  
   226  Example:
   227  
   228  ```go
   229  func GetCurrentBranchName(t *testing.T) string {
   230  	out, err := GetCurrentBranchNameE(t)
   231  	if err != nil {
   232  		t.Fatal(err)
   233  	}
   234  	return out
   235  }
   236  
   237  func GetCurrentBranchNameE(t *testing.T) (string, error) {
   238  	cmd := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD")
   239  	bytes, err := cmd.Output()
   240  	if err != nil {
   241  		return "", err
   242  	}
   243  	return strings.TrimSpace(string(bytes)), nil
   244  }
   245  ```
   246  
   247  In most places in our code, we will use `GetCurrentBranchName`, which will call `t.Fatal` if it hits any errors. This
   248  is typically the behavior we want anyway, and not having to deal with a returned error will keep our code smaller and
   249  easier to read. However, in those cases where we may want to get the original error back and not fail the test
   250  immediately, we can use `GetCurrentBranchNameE`.
   251  
   252  
   253  
   254  
   255  ## Other thoughts on the refactor
   256  
   257  - Updating to the refactored version of Terratest will be a pain that requires lots of search & replace. But in the
   258    long term, it seems like worthwhile cleanup.
   259  
   260  - There are a bunch of patterns we often end up using throughout our tests that would be good to copy into Terratest.
   261    Anyone remember what those are off the top of our head?
   262  
   263  - Having two copies of each method (`foo` and `fooE`) is a bit tedious, but the `foo` variety is essentially the same
   264    boilerplate everywhere, so it only increases the maintenance burden on Terratest library maintainers a little, but
   265    it improves code readability for all Terratest users enormously.