Repository: chanzuckerberg/fogg
Pull request #389 (2019-12-30 17:49:35)

simplify `make fmt`
Previously `terraform fmt` would always recurse into nested directories,
including .terraform. This caused two problems–

1. `terraform fmt` would try to reformat modules which had been init'ed.
    Beacause these modules are just git checkouts later updates to those
    modules would fail due to uncommited changes in those working dirs.
2. `terraform fmt -check`, run via `make lint` would fail if modules in-updates
    had format errors.

To work around this, we did a non-recursive list of terraform files and
called `terraform fmt` for each one, which is somewhat slower.

Now `terraform fmt` *does not* recurse by default, so we can remove our
workaround.

### Summary
A description of changes or issues addressed by this PR. Provide links to relevant PRs if any.

### Test Plan
Say unittests, or list out steps to verify changes.

### References
(Optional) Additional links to provide more context.


codecov[bot]: # [Codecov](https://codecov.io/gh/chanzuckerberg/fogg/pull/389?src=pr&el=h1) Report
> Merging [#389](https://codecov.io/gh/chanzuckerberg/fogg/pull/389?src=pr&el=desc) into [master](https://codecov.io/gh/chanzuckerberg/fogg/commit/3ec5fe6cc0d36207944c561fa0eb361acd8e280b?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/chanzuckerberg/fogg/pull/389/graphs/tree.svg?width=650&token=57DhtZ7g2A&height=150&src=pr)](https://codecov.io/gh/chanzuckerberg/fogg/pull/389?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #389   +/-   ##
=======================================
  Coverage   79.04%   79.04%           
=======================================
  Files          25       25           
  Lines        2009     2009           
=======================================
  Hits         1588     1588           
  Misses        278      278           
  Partials      143      143
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/chanzuckerberg/fogg/pull/389?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/chanzuckerberg/fogg/pull/389?src=pr&el=footer). Last update [3ec5fe6...1e8070d](https://codecov.io/gh/chanzuckerberg/fogg/pull/389?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/templates/repo/scripts/common.mk b/templates/repo/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/templates/repo/scripts/common.mk
+++ b/templates/repo/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/templates/repo/scripts/component.mk b/templates/repo/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/templates/repo/scripts/component.mk
+++ b/templates/repo/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component


Test patch:
diff --git a/testdata/bless_provider/scripts/common.mk b/testdata/bless_provider/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/bless_provider/scripts/common.mk
+++ b/testdata/bless_provider/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/bless_provider/scripts/component.mk b/testdata/bless_provider/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/bless_provider/scripts/component.mk
+++ b/testdata/bless_provider/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component
diff --git a/testdata/github_provider/scripts/common.mk b/testdata/github_provider/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/github_provider/scripts/common.mk
+++ b/testdata/github_provider/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/github_provider/scripts/component.mk b/testdata/github_provider/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/github_provider/scripts/component.mk
+++ b/testdata/github_provider/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component
diff --git a/testdata/okta_provider/scripts/common.mk b/testdata/okta_provider/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/okta_provider/scripts/common.mk
+++ b/testdata/okta_provider/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/okta_provider/scripts/component.mk b/testdata/okta_provider/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/okta_provider/scripts/component.mk
+++ b/testdata/okta_provider/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component
diff --git a/testdata/snowflake_provider/scripts/common.mk b/testdata/snowflake_provider/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/snowflake_provider/scripts/common.mk
+++ b/testdata/snowflake_provider/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/snowflake_provider/scripts/component.mk b/testdata/snowflake_provider/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/snowflake_provider/scripts/component.mk
+++ b/testdata/snowflake_provider/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component
diff --git a/testdata/v1_full/scripts/common.mk b/testdata/v1_full/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/v1_full/scripts/common.mk
+++ b/testdata/v1_full/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/v1_full/scripts/component.mk b/testdata/v1_full/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/v1_full/scripts/component.mk
+++ b/testdata/v1_full/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component
diff --git a/testdata/v2_full/scripts/common.mk b/testdata/v2_full/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/v2_full/scripts/common.mk
+++ b/testdata/v2_full/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/v2_full/scripts/component.mk b/testdata/v2_full/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/v2_full/scripts/component.mk
+++ b/testdata/v2_full/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component
diff --git a/testdata/v2_minimal_valid/scripts/common.mk b/testdata/v2_minimal_valid/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/v2_minimal_valid/scripts/common.mk
+++ b/testdata/v2_minimal_valid/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/v2_minimal_valid/scripts/component.mk b/testdata/v2_minimal_valid/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/v2_minimal_valid/scripts/component.mk
+++ b/testdata/v2_minimal_valid/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component
diff --git a/testdata/v2_no_aws_provider/scripts/common.mk b/testdata/v2_no_aws_provider/scripts/common.mk
index 6dbaa0c7..d4d03047 100644
--- a/testdata/v2_no_aws_provider/scripts/common.mk
+++ b/testdata/v2_no_aws_provider/scripts/common.mk
@@ -8,10 +8,6 @@ REPO_RELATIVE_PATH := $(shell git rev-parse --show-prefix)
 AUTO_APPROVE := false
 REFRESH_ENABLED ?= true # Should terraform refresh during plan/apply
 
-# We need to do this because `terraform fmt` recurses into .terraform/modules
-# and wont' accept more than one file at a time.
-TF=$(wildcard *.tf)
-
 TFENV_DIR ?= $(REPO_ROOT)/.fogg/tfenv
 export PATH :=$(TFENV_DIR)/libexec:$(TFENV_DIR)/versions/$(TERRAFORM_VERSION)/:$(REPO_ROOT)/.fogg/bin:$(PATH)
 export TF_PLUGIN_CACHE_DIR=$(REPO_ROOT)/.terraform.d/plugin-cache
diff --git a/testdata/v2_no_aws_provider/scripts/component.mk b/testdata/v2_no_aws_provider/scripts/component.mk
index 08c359e8..22ba1b1b 100644
--- a/testdata/v2_no_aws_provider/scripts/component.mk
+++ b/testdata/v2_no_aws_provider/scripts/component.mk
@@ -17,9 +17,7 @@ check: lint check-plan ## run all checks for this component
 .PHONY: check
 
 fmt: terraform ## format code in this component
-	@printf "fmt: ";
-	@for f in $(TF); do printf .; $(terraform_command) fmt $(TF_ARGS) $$f; done
-	@echo
+	$(terraform_command) fmt $(TF_ARGS)
 .PHONY: fmt
 
 lint: lint-terraform-fmt lint-tflint ## run all linters for this component