navapbc / template-infra

A template to set up foundational infrastructure for your application in AWS
Apache License 2.0
9 stars 2 forks source link

Pin terraform version #599

Closed rocketnova closed 2 months ago

rocketnova commented 2 months ago

Ticket

Resolves #587

Changes

What was added, updated, or removed in this PR.

Context for reviewers

Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers.

We are currently pinning our terraform version only to the major version:

required_version = ">= 1.2.0, < 2.0.0"

This causes mysterious and sometimes hard to debug issues like #586. We should instead pin to the terraform minor version (e.g. 1.8.x) and do controlled upgrades to new minor versions (e.g. 1.8.x -> 1.9.x).

This PR uses the pessimistic constraint operator to ensure that only updates to the rightmost version component are applied.

Testing

Provide evidence that the code works as expected. Explain what was done for testing and the results of the test plan. Include screenshots, GIF demos, shell commands or output to help show the changes working as expected. ProTip: you can drag and drop or paste images into this textbox.

For CI/CD changes, see https://github.com/navapbc/template-infra/actions/runs/9119524960

To test terraform CLI changes:

  1. Pull these updates into a working repo, such as https://github.com/navapbc/platform-test/
  2. Change to the latest terraform version: tfenv install 1.8.3 && tfenv use 1.8.3
  3. Re-initialize terraform modules:
    ./bin/terraform-init.sh infra/accounts <ACCOUNT_NAME>.<ACCOUNT_ID>
    ./bin/terraform-init.sh infra/networks dev
    ./bin/terraform-init.sh infra/app/build-repository shared
    ./bin/terraform-init.sh infra/app/database dev
    ./bin/terraform-init.sh infra/app/service dev
Example screenshot CleanShot 2024-05-16 at 16 01 31@2x
lorenyu commented 2 months ago

@rocketnova FYI for the future, for testing this change I probably would have been ok with just template CI coverage rather than requiring manual testing. If I'm not mistaken template CI would hit most if not all of the code paths you tested which would be enough to sanity check.

rocketnova commented 2 months ago

@rocketnova FYI for the future, for testing this change I probably would have been ok with just template CI coverage rather than requiring manual testing. If I'm not mistaken template CI would hit most if not all of the code paths you tested which would be enough to sanity check.

@lorenyu Good to know. I'm still trying to tune in to what is the right level of testing you're looking for since you previously asked for more testing evidence.

For this particular testing, so I can improve my understanding of the CI, I know that the CI covers all of the changes to the /.github files. Does it also cover all of the changes to the /infra dir?

lorenyu commented 1 month ago

@rocketnova template CI will set up the account, network, build repository, and service layers, skipping the database layer. So it will cover quite a bit of surface area. It won't cover the infra/app/database/main.tf change, but since that change is the same as the other ones I think it's low risk. You can see what it does here: https://github.com/navapbc/template-infra/blob/5f92599fa1831535b91f94205a80ccbd883c76d6/template-only-test/template_infra_test.go

rocketnova commented 1 month ago

@rocketnova template CI will set up the account, network, build repository, and service layers, skipping the database layer. So it will cover quite a bit of surface area. It won't cover the infra/app/database/main.tf change, but since that change is the same as the other ones I think it's low risk. You can see what it does here: https://github.com/navapbc/template-infra/blob/5f92599fa1831535b91f94205a80ccbd883c76d6/template-only-test/template_infra_test.go

Oh right. Thanks for the reminder. I get the template CI and CD functionality mixed up sometimes.