Upgrade conftest version to 0.23 (#1516)

* Upgraded conftest to 0.23.0 and removed --all-namespaces flag (#62)

* Upgraded conftest to 0.23.0 and removed --all-namespaces flag

* fix tests

* fix conftest version name

* make tests pass

* Update e2e tests to use 0.23 conftest

* Fix log function

* Make test client fetch version synchronously.

* Update fixtures.

* Pinning test image

Co-authored-by: Nish Krishnan <[email protected]>

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2
 jobs:
   test:
     docker:
-    - image: runatlantis/testing-env:latest
+    - image: runatlantis/testing-env:20296f9638a4eca04733f60b39e1d3025b9d708a
     steps:
     - checkout
     - run: make check-fmt

Dockerfile

@@ -19,7 +19,7 @@ RUN AVAILABLE_TERRAFORM_VERSIONS="0.8.8 0.9.11 0.10.8 0.11.14 0.12.30 0.13.6 ${D
     done && \
     ln -s /usr/local/bin/tf/versions/${DEFAULT_TERRAFORM_VERSION}/terraform /usr/local/bin/terraform
 
-ENV DEFAULT_CONFTEST_VERSION=0.21.0
+ENV DEFAULT_CONFTEST_VERSION=0.23.0
 
 RUN AVAILABLE_CONFTEST_VERSIONS="${DEFAULT_CONFTEST_VERSION}" && \
     for VERSION in ${AVAILABLE_CONFTEST_VERSIONS}; do \

Dockerfile.dev

@@ -1,6 +1,6 @@
 FROM runatlantis/atlantis:latest
 COPY atlantis /usr/local/bin/atlantis
 # TODO: remove this once we get this in the base image
-ENV DEFAULT_CONFTEST_VERSION=0.21.0
+ENV DEFAULT_CONFTEST_VERSION=0.23.0
 
 WORKDIR /atlantis/src

server/events/runtime/policy/conftest_client.go

@@ -59,7 +59,7 @@ func (c ConftestTestCommandArgs) build() ([]string, error) {
 		commandArgs = append(commandArgs, a.build()...)
 	}
 
-	commandArgs = append(commandArgs, c.InputFile, "--no-color", "--all-namespaces")
+	commandArgs = append(commandArgs, c.InputFile, "--no-color")
 
 	return commandArgs, nil
 }

server/events/runtime/policy/conftest_client_test.go

@@ -22,10 +22,10 @@ import (
 
 func TestConfTestVersionDownloader(t *testing.T) {
 
-	version, _ := version.NewVersion("0.21.0")
+	version, _ := version.NewVersion("0.23.0")
 	destPath := "some/path"
 
-	fullURL := fmt.Sprintf("https://github.com/open-policy-agent/conftest/releases/download/v0.21.0/conftest_0.21.0_%s_x86_64.tar.gz?checksum=file:https://github.com/open-policy-agent/conftest/releases/download/v0.21.0/checksums.txt", strings.Title(runtime.GOOS))
+	fullURL := fmt.Sprintf("https://github.com/open-policy-agent/conftest/releases/download/v0.23.0/conftest_0.23.0_%s_x86_64.tar.gz?checksum=file:https://github.com/open-policy-agent/conftest/releases/download/v0.23.0/checksums.txt", strings.Title(runtime.GOOS))
 
 	RegisterMockTestingT(t)
 
@@ -175,7 +175,7 @@ func TestRun(t *testing.T) {
 
 		expectedOutput := "Success"
 		expectedResult := "Checking plan against the following policies: \n  policy1\n  policy2\nSuccess"
-		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "-p", localPolicySetPath2, "/some_workdir/testproj-default.json", "--no-color", "--all-namespaces"}
+		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "-p", localPolicySetPath2, "/some_workdir/testproj-default.json", "--no-color"}
 
 		When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil)
 		When(mockResolver.Resolve(policySet2)).ThenReturn(localPolicySetPath2, nil)
@@ -196,7 +196,7 @@ func TestRun(t *testing.T) {
 
 		expectedOutput := "Success"
 		expectedResult := "Checking plan against the following policies: \n  policy1\nSuccess"
-		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "/some_workdir/testproj-default.json", "--no-color", "--all-namespaces"}
+		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "/some_workdir/testproj-default.json", "--no-color"}
 
 		When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil)
 		When(mockResolver.Resolve(policySet2)).ThenReturn("", errors.New("err"))
@@ -214,7 +214,7 @@ func TestRun(t *testing.T) {
 	t.Run("error resolving both policy sources", func(t *testing.T) {
 
 		expectedResult := "Success"
-		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "/some_workdir/testproj-default.json", "--no-color", "--all-namespaces"}
+		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "/some_workdir/testproj-default.json", "--no-color"}
 
 		When(mockResolver.Resolve(policySet1)).ThenReturn("", errors.New("err"))
 		When(mockResolver.Resolve(policySet2)).ThenReturn("", errors.New("err"))
@@ -232,7 +232,7 @@ func TestRun(t *testing.T) {
 	t.Run("error running cmd", func(t *testing.T) {
 		expectedOutput := "FAIL - /some_workdir/testproj-default.json - failure"
 		expectedResult := "Checking plan against the following policies: \n  policy1\n  policy2\nFAIL - <redacted plan file> - failure"
-		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "-p", localPolicySetPath2, "/some_workdir/testproj-default.json", "--no-color", "--all-namespaces"}
+		expectedArgs := []string{executablePath, "test", "-p", localPolicySetPath1, "-p", localPolicySetPath2, "/some_workdir/testproj-default.json", "--no-color"}
 
 		When(mockResolver.Resolve(policySet1)).ThenReturn(localPolicySetPath1, nil)
 		When(mockResolver.Resolve(policySet2)).ThenReturn(localPolicySetPath2, nil)

server/events/terraform/terraform_client.go

@@ -88,15 +88,8 @@ type Downloader interface {
 //	   => 0.11.10
 var versionRegex = regexp.MustCompile("Terraform v(.*?)(\\s.*)?\n")
 
-// NewClient constructs a terraform client.
-// tfeToken is an optional terraform enterprise token.
-// defaultVersionStr is an optional default terraform version to use unless
-// a specific version is set.
-// defaultVersionFlagName is the name of the flag that sets the default terraform
-// version.
-// tfDownloader is used to download terraform versions.
-// Will asynchronously download the required version if it doesn't exist already.
-func NewClient(
+// NewClientWithDefaultVersion creates a new terraform client and pre-fetches the default version
+func NewClientWithDefaultVersion(
 	log logging.SimpleLogging,
 	binDir string,
 	cacheDir string,
@@ -106,7 +99,9 @@ func NewClient(
 	defaultVersionFlagName string,
 	tfDownloadURL string,
 	tfDownloader Downloader,
-	usePluginCache bool) (*DefaultClient, error) {
+	usePluginCache bool,
+	fetchAsync bool,
+) (*DefaultClient, error) {
 	var finalDefaultVersion *version.Version
 	var localVersion *version.Version
 	versions := make(map[string]string)
@@ -135,7 +130,7 @@ func NewClient(
 			return nil, err
 		}
 		finalDefaultVersion = defaultVersion
-		go func() {
+		ensureVersionFunc := func() {
 			// Since ensureVersion might end up downloading terraform,
 			// we call it asynchronously so as to not delay server startup.
 			versionsLock.Lock()
@@ -144,7 +139,13 @@ func NewClient(
 			if err != nil {
 				log.Err("could not download terraform %s: %s", defaultVersion.String(), err)
 			}
-		}()
+		}
+
+		if fetchAsync {
+			go ensureVersionFunc()
+		} else {
+			ensureVersionFunc()
+		}
 	}
 
 	// If tfeToken is set, we try to create a ~/.terraformrc file.
@@ -168,6 +169,67 @@ func NewClient(
 		versions:                versions,
 		usePluginCache:          usePluginCache,
 	}, nil
+
+}
+
+func NewTestClient(
+	log logging.SimpleLogging,
+	binDir string,
+	cacheDir string,
+	tfeToken string,
+	tfeHostname string,
+	defaultVersionStr string,
+	defaultVersionFlagName string,
+	tfDownloadURL string,
+	tfDownloader Downloader,
+	usePluginCache bool) (*DefaultClient, error) {
+	return NewClientWithDefaultVersion(
+		log,
+		binDir,
+		cacheDir,
+		tfeToken,
+		tfeHostname,
+		defaultVersionStr,
+		defaultVersionFlagName,
+		tfDownloadURL,
+		tfDownloader,
+		usePluginCache,
+		false,
+	)
+}
+
+// NewClient constructs a terraform client.
+// tfeToken is an optional terraform enterprise token.
+// defaultVersionStr is an optional default terraform version to use unless
+// a specific version is set.
+// defaultVersionFlagName is the name of the flag that sets the default terraform
+// version.
+// tfDownloader is used to download terraform versions.
+// Will asynchronously download the required version if it doesn't exist already.
+func NewClient(
+	log logging.SimpleLogging,
+	binDir string,
+	cacheDir string,
+	tfeToken string,
+	tfeHostname string,
+	defaultVersionStr string,
+	defaultVersionFlagName string,
+	tfDownloadURL string,
+	tfDownloader Downloader,
+	usePluginCache bool) (*DefaultClient, error) {
+	return NewClientWithDefaultVersion(
+		log,
+		binDir,
+		cacheDir,
+		tfeToken,
+		tfeHostname,
+		defaultVersionStr,
+		defaultVersionFlagName,
+		tfDownloadURL,
+		tfDownloader,
+		usePluginCache,
+		true,
+	)
 }
 
 // Version returns the default version of Terraform we use if no other version

server/events/terraform/terraform_client_test.go

@@ -258,7 +258,7 @@ func TestEnsureVersion_downloaded(t *testing.T) {
 
 	mockDownloader := mocks.NewMockDownloader()
 
-	c, err := terraform.NewClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, true)
+	c, err := terraform.NewTestClient(logger, binDir, cacheDir, "", "", "0.11.10", cmd.DefaultTFVersionFlag, cmd.DefaultTFDownloadURL, mockDownloader, true)
 	Ok(t, err)
 
 	Equals(t, "0.11.10", c.DefaultVersion().String())

server/events_controller_e2e_test.go

@@ -37,6 +37,8 @@ import (
 	. "github.com/runatlantis/atlantis/testing"
 )
 
+const ConftestVersion = "0.23.0"
+
 var applyLocker locking.ApplyLocker
 var userConfig server.UserConfig
 
@@ -56,7 +58,7 @@ type LocalConftestCache struct {
 }
 
 func (m *LocalConftestCache) Get(key *version.Version) (string, error) {
-	return exec.LookPath("conftest0.21.0")
+	return exec.LookPath(fmt.Sprintf("conftest%s", ConftestVersion))
 }
 
 func TestGitHubWorkflow(t *testing.T) {
@@ -634,7 +636,7 @@ func setupE2E(t *testing.T, repoDir string) (server.EventsController, *vcsmocks.
 
 	if userConfig.EnablePolicyChecksFlag {
 		// need this to be set or we'll fail the policy check step
-		os.Setenv(policy.DefaultConftestVersionEnvKey, "0.21.0")
+		os.Setenv(policy.DefaultConftestVersionEnvKey, "0.23.0")
 	}
 
 	// Mocks.
@@ -725,7 +727,7 @@ func setupE2E(t *testing.T, repoDir string) (server.EventsController, *vcsmocks.
 
 	Ok(t, err)
 
-	conftestVersion, _ := version.NewVersion("0.21.0")
+	conftestVersion, _ := version.NewVersion(ConftestVersion)
 
 	conftextExec := policy.NewConfTestExecutorWorkflow(logger, binDir, &NoopTFDownloader{})
 
@@ -1077,11 +1079,11 @@ func mkSubDirs(t *testing.T) (string, string, string, func()) {
 	return tmp, binDir, cachedir, cleanup
 }
 
-// Will fail test if conftest isn't in path and isn't version >= 0.21.0
+// Will fail test if conftest isn't in path and isn't version >= 0.23.0
 func ensureRunningConftest(t *testing.T) {
-	localPath, err := exec.LookPath("conftest0.21.0")
+	localPath, err := exec.LookPath(fmt.Sprintf("conftest%s", ConftestVersion))
 	if err != nil {
-		t.Log("conftest >= 0.21 must be installed to run this test")
+		t.Logf("conftest >= %s must be installed to run this test", ConftestVersion)
 		t.FailNow()
 	}
 	versionOutBytes, err := exec.Command(localPath, "--version").Output() // #nosec
@@ -1097,7 +1099,7 @@ func ensureRunningConftest(t *testing.T) {
 	}
 	localVersion, err := version.NewVersion(match[1])
 	Ok(t, err)
-	minVersion, err := version.NewVersion("0.21.0")
+	minVersion, err := version.NewVersion(ConftestVersion)
 	Ok(t, err)
 	if localVersion.LessThan(minVersion) {
 		t.Logf("must have contest version >= %s, you have %s", minVersion, localVersion)

server/testfixtures/test-repos/policy-checks-apply-reqs/exp-output-auto-policy-check.txt

@@ -5,7 +5,7 @@ Ran Policy Check for dir: `.` workspace: `default`
 exit status 1
 Checking plan against the following policies: 
   test_policy
-FAIL - <redacted plan file> - WARNING: Null Resource creation is prohibited.
+FAIL - <redacted plan file> - main - WARNING: Null Resource creation is prohibited.
 
 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions
 

server/testfixtures/test-repos/policy-checks-diff-owner/exp-output-auto-policy-check.txt

@@ -5,7 +5,7 @@ Ran Policy Check for dir: `.` workspace: `default`
 exit status 1
 Checking plan against the following policies: 
   test_policy
-FAIL - <redacted plan file> - WARNING: Null Resource creation is prohibited.
+FAIL - <redacted plan file> - main - WARNING: Null Resource creation is prohibited.
 
 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions
 

server/testfixtures/test-repos/policy-checks-multi-projects/exp-output-auto-policy-check.txt

@@ -25,7 +25,7 @@ Checking plan against the following policies:
 exit status 1
 Checking plan against the following policies: 
   test_policy
-FAIL - <redacted plan file> - WARNING: Forbidden Resource creation is prohibited.
+FAIL - <redacted plan file> - main - WARNING: Forbidden Resource creation is prohibited.
 
 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions
 

server/testfixtures/test-repos/policy-checks/exp-output-auto-policy-check.txt

@@ -5,7 +5,7 @@ Ran Policy Check for dir: `.` workspace: `default`
 exit status 1
 Checking plan against the following policies: 
   test_policy
-FAIL - <redacted plan file> - WARNING: Null Resource creation is prohibited.
+FAIL - <redacted plan file> - main - WARNING: Null Resource creation is prohibited.
 
 1 test, 0 passed, 0 warnings, 1 failure, 0 exceptions
 

testing/Dockerfile

@@ -13,7 +13,7 @@ RUN curl -LOks https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/ter
     rm terraform_${TERRAFORM_VERSION}_linux_amd64.zip
 
 # Install conftest
-ENV DEFAULT_CONFTEST_VERSION=0.21.0
+ENV DEFAULT_CONFTEST_VERSION=0.23.0
 
 RUN AVAILABLE_CONFTEST_VERSIONS="${DEFAULT_CONFTEST_VERSION}" && \
     for VERSION in ${AVAILABLE_CONFTEST_VERSIONS}; do \