From 61ec7aa5c9190c576a90eab2b8ca357b1cfed17e Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@hashicorp.com>
Date: Tue, 24 Mar 2020 15:16:13 -0400
Subject: [PATCH] ci: Run all connect/ca tests from the integration suite

To reduce the chance of some tests not being run because it does not
match the regex passed to '-run'.

Also document why some tests are allowed to be skipped on CI.
---
 .circleci/config.yml                    | 12 ++++++------
 GNUmakefile                             |  8 ++++----
 agent/connect/ca/provider_aws_test.go   |  7 +++++++
 agent/connect/ca/provider_vault_test.go | 13 ++++++-------
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 411363598b..8cf6b23544 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -569,8 +569,8 @@ jobs:
       ENVOY_VERSIONS: "1.13.0"
     steps: *ENVOY_INTEGRATION_TEST_STEPS
 
-  # run tests on vault ca provider integration tests
-  vault-ca-provider:
+  # run integration tests for the connect ca providers
+  test-connect-ca-providers:
     docker:
       - image: *GOLANG_IMAGE
     environment:
@@ -586,7 +586,7 @@ jobs:
       # Gather deps to run go tests
       - checkout
       # Run go tests
-      - run: make test-vault-ca-provider
+      - run: make test-connect-ca-providers
       - store_test_results:
           path: *TEST_RESULTS_DIR
 
@@ -635,6 +635,8 @@ workflows:
             - go-test
             - go-test-api
             - go-test-sdk
+      - test-connect-ca-providers: *go-test
+
   build-distros:
     jobs:
       - check-vendor:
@@ -686,9 +688,7 @@ workflows:
       - envoy-integration-test-1.13.0:
           requires:
             - dev-build
-      - vault-ca-provider:
-          requires:
-            - dev-build
+
   website:
     jobs:
       - build-website
diff --git a/GNUmakefile b/GNUmakefile
index def8b73e5d..e611b7f531 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -381,14 +381,14 @@ ui-docker: ui-build-image
 test-envoy-integ: $(ENVOY_INTEG_DEPS)
 	@$(SHELL) $(CURDIR)/test/integration/connect/envoy/run-tests.sh
 
-test-vault-ca-provider:
+test-connect-ca-providers:
 ifeq ("$(CIRCLECI)","true")
 # Run in CI
-	gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report.xml" -- $(CURDIR)/agent/connect/ca/* -run 'TestVault(CA)?Provider'
+	gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report.xml" -- ./agent/connect/ca
 else
 # Run locally
-	@echo "Running /agent/connect/ca TestVault(CA)?Provider tests in verbose mode"
-	@go test $(CURDIR)/agent/connect/ca/* -run 'TestVault(CA)?Provider' -v
+	@echo "Running /agent/connect/ca tests in verbose mode"
+	@go test -v ./agent/connect/ca
 endif
 
 proto-delete:
diff --git a/agent/connect/ca/provider_aws_test.go b/agent/connect/ca/provider_aws_test.go
index beab9402a1..12cf1e3ab8 100644
--- a/agent/connect/ca/provider_aws_test.go
+++ b/agent/connect/ca/provider_aws_test.go
@@ -10,6 +10,13 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// skipIfAWSNotConfigured skips the test unless ENABLE_AWS_PCA_TESTS=true.
+//
+// These tests are not run in CI.  If you are making changes to the AWS provider
+// you probably want to run these tests locally. The tests will run using any
+// credentials available to the AWS SDK. See
+// https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html#specifying-credentials
+// for a list of options.
 func skipIfAWSNotConfigured(t *testing.T) {
 	enabled := os.Getenv("ENABLE_AWS_PCA_TESTS")
 	ok, err := strconv.ParseBool(enabled)
diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go
index 098439aee1..b1935365f7 100644
--- a/agent/connect/ca/provider_vault_test.go
+++ b/agent/connect/ca/provider_vault_test.go
@@ -385,10 +385,10 @@ func testVaultProviderWithConfig(t *testing.T, isPrimary bool, rawConf map[strin
 	return provider, testVault
 }
 
-var printedVaultVersion sync.Once
-
-var mustAlwaysRun = os.Getenv("CI") == "true"
-
+// skipIfVaultNotPresent skips the test if the vault binary is not in PATH.
+//
+// These tests may be skipped in CI. They are run as part of a separate
+// integration test suite.
 func skipIfVaultNotPresent(t *testing.T) {
 	vaultBinaryName := os.Getenv("VAULT_BINARY_NAME")
 	if vaultBinaryName == "" {
@@ -397,9 +397,6 @@ func skipIfVaultNotPresent(t *testing.T) {
 
 	path, err := exec.LookPath(vaultBinaryName)
 	if err != nil || path == "" {
-		if mustAlwaysRun {
-			t.Fatalf("%q not found on $PATH", vaultBinaryName)
-		}
 		t.Skipf("%q not found on $PATH - download and install to run this test", vaultBinaryName)
 	}
 }
@@ -474,6 +471,8 @@ type testVaultServer struct {
 	returnPortsFn func()
 }
 
+var printedVaultVersion sync.Once
+
 func (v *testVaultServer) WaitUntilReady(t *testing.T) {
 	var version string
 	retry.Run(t, func(r *retry.R) {