From dc983db333b02aca7812cd9556279047837ea8a9 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 23 Mar 2020 17:02:13 -0400 Subject: [PATCH] ci: Do not skip tests because of missing binaries on CI If the CI environment is not correct for running tests the tests should fail, so that we don't accidentally stop running some tests because of a change to our CI environment. Also removed a duplicate delcaration from init. I believe one was overriding the other as they are both in the same package. --- agent/connect/ca/provider_aws_test.go | 29 ++++--------------- agent/connect/ca/provider_vault_test.go | 30 +++++++------------ agent/connect/generate_test.go | 38 +++++++++++-------------- agent/connect/testing_ca_test.go | 32 +++++++-------------- 4 files changed, 44 insertions(+), 85 deletions(-) diff --git a/agent/connect/ca/provider_aws_test.go b/agent/connect/ca/provider_aws_test.go index 21bedc972..beab9402a 100644 --- a/agent/connect/ca/provider_aws_test.go +++ b/agent/connect/ca/provider_aws_test.go @@ -10,22 +10,18 @@ import ( "github.com/stretchr/testify/require" ) -func skipIfAWSNotConfigured(t *testing.T) bool { +func skipIfAWSNotConfigured(t *testing.T) { enabled := os.Getenv("ENABLE_AWS_PCA_TESTS") ok, err := strconv.ParseBool(enabled) if err != nil || !ok { t.Skip("Skipping because AWS tests are not enabled") - return true } - return false } func TestAWSBootstrapAndSignPrimary(t *testing.T) { // Note not parallel since we could easily hit AWS limits of too many CAs if // all of these tests run at once. - if skipIfAWSNotConfigured(t) { - return - } + skipIfAWSNotConfigured(t) for _, tc := range KeyTestCases { tc := tc @@ -83,9 +79,7 @@ func testSignAndValidate(t *testing.T, p Provider, rootPEM string, intermediateP func TestAWSBootstrapAndSignSecondary(t *testing.T) { // Note not parallel since we could easily hit AWS limits of too many CAs if // all of these tests run at once. - if skipIfAWSNotConfigured(t) { - return - } + skipIfAWSNotConfigured(t) p1 := testAWSProvider(t, testProviderConfigPrimary(t, nil)) defer p1.Cleanup() @@ -179,9 +173,7 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) { func TestAWSBootstrapAndSignSecondaryConsul(t *testing.T) { // Note not parallel since we could easily hit AWS limits of too many CAs if // all of these tests run at once. - if skipIfAWSNotConfigured(t) { - return - } + skipIfAWSNotConfigured(t) t.Run("pri=consul,sec=aws", func(t *testing.T) { conf := testConsulCAConfig() @@ -215,9 +207,7 @@ func TestAWSBootstrapAndSignSecondaryConsul(t *testing.T) { } func TestAWSNoCrossSigning(t *testing.T) { - if skipIfAWSNotConfigured(t) { - return - } + skipIfAWSNotConfigured(t) p1 := testAWSProvider(t, testProviderConfigPrimary(t, nil)) defer p1.Cleanup() @@ -246,15 +236,6 @@ func testAWSProvider(t *testing.T, cfg ProviderConfig) *AWSProvider { return p } -type testLogger struct { - t *testing.T -} - -func (l *testLogger) Write(b []byte) (int, error) { - l.t.Log(string(b)) - return len(b), nil -} - func testProviderConfigPrimary(t *testing.T, cfg map[string]interface{}) ProviderConfig { rawCfg := make(map[string]interface{}) for k, v := range cfg { diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index c5707f1e3..098439aee 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -40,9 +40,7 @@ func TestVaultCAProvider_VaultTLSConfig(t *testing.T) { func TestVaultCAProvider_Bootstrap(t *testing.T) { t.Parallel() - if skipIfVaultNotPresent(t) { - return - } + skipIfVaultNotPresent(t) provider, testVault := testVaultProvider(t) defer testVault.Stop() @@ -103,9 +101,7 @@ func assertCorrectKeyType(t *testing.T, want, certPEM string) { func TestVaultCAProvider_SignLeaf(t *testing.T) { t.Parallel() - if skipIfVaultNotPresent(t) { - return - } + skipIfVaultNotPresent(t) for _, tc := range KeyTestCases { tc := tc @@ -189,9 +185,7 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { func TestVaultCAProvider_CrossSignCA(t *testing.T) { t.Parallel() - if skipIfVaultNotPresent(t) { - return - } + skipIfVaultNotPresent(t) tests := CASigningKeyTypeCases() @@ -246,9 +240,7 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) { func TestVaultProvider_SignIntermediate(t *testing.T) { t.Parallel() - if skipIfVaultNotPresent(t) { - return - } + skipIfVaultNotPresent(t) tests := CASigningKeyTypeCases() @@ -277,9 +269,7 @@ func TestVaultProvider_SignIntermediate(t *testing.T) { func TestVaultProvider_SignIntermediateConsul(t *testing.T) { t.Parallel() - if skipIfVaultNotPresent(t) { - return - } + skipIfVaultNotPresent(t) // primary = Vault, secondary = Consul t.Run("pri=vault,sec=consul", func(t *testing.T) { @@ -397,8 +387,9 @@ func testVaultProviderWithConfig(t *testing.T, isPrimary bool, rawConf map[strin var printedVaultVersion sync.Once -// skipIfVaultNotPresent skips the test and returns true if vault is not found -func skipIfVaultNotPresent(t *testing.T) bool { +var mustAlwaysRun = os.Getenv("CI") == "true" + +func skipIfVaultNotPresent(t *testing.T) { vaultBinaryName := os.Getenv("VAULT_BINARY_NAME") if vaultBinaryName == "" { vaultBinaryName = "vault" @@ -406,10 +397,11 @@ func skipIfVaultNotPresent(t *testing.T) bool { 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) - return true } - return false } func runTestVault() (*testVaultServer, error) { diff --git a/agent/connect/generate_test.go b/agent/connect/generate_test.go index 226ffd853..6d79cd4af 100644 --- a/agent/connect/generate_test.go +++ b/agent/connect/generate_test.go @@ -17,27 +17,23 @@ type KeyConfig struct { keyBits int } -var goodParams, badParams []KeyConfig - -func init() { - goodParams = []KeyConfig{ - {keyType: "rsa", keyBits: 2048}, - {keyType: "rsa", keyBits: 4096}, - {keyType: "ec", keyBits: 224}, - {keyType: "ec", keyBits: 256}, - {keyType: "ec", keyBits: 384}, - {keyType: "ec", keyBits: 521}, - } - badParams = []KeyConfig{ - {keyType: "rsa", keyBits: 0}, - {keyType: "rsa", keyBits: 1024}, - {keyType: "rsa", keyBits: 24601}, - {keyType: "ec", keyBits: 0}, - {keyType: "ec", keyBits: 512}, - {keyType: "ec", keyBits: 321}, - {keyType: "ecdsa", keyBits: 256}, // test for "ecdsa" instead of "ec" - {keyType: "aes", keyBits: 128}, - } +var goodParams = []KeyConfig{ + {keyType: "rsa", keyBits: 2048}, + {keyType: "rsa", keyBits: 4096}, + {keyType: "ec", keyBits: 224}, + {keyType: "ec", keyBits: 256}, + {keyType: "ec", keyBits: 384}, + {keyType: "ec", keyBits: 521}, +} +var badParams = []KeyConfig{ + {keyType: "rsa", keyBits: 0}, + {keyType: "rsa", keyBits: 1024}, + {keyType: "rsa", keyBits: 24601}, + {keyType: "ec", keyBits: 0}, + {keyType: "ec", keyBits: 512}, + {keyType: "ec", keyBits: 321}, + {keyType: "ecdsa", keyBits: 256}, // test for "ecdsa" instead of "ec" + {keyType: "aes", keyBits: 128}, } func makeConfig(kc KeyConfig) structs.CommonCAProviderConfig { diff --git a/agent/connect/testing_ca_test.go b/agent/connect/testing_ca_test.go index 4fdf7ccc1..29c041331 100644 --- a/agent/connect/testing_ca_test.go +++ b/agent/connect/testing_ca_test.go @@ -12,29 +12,22 @@ import ( "github.com/stretchr/testify/require" ) -// hasOpenSSL is used to determine if the openssl CLI exists for unit tests. -var hasOpenSSL bool +var mustAlwaysRun = os.Getenv("CI") == "true" -func init() { - goodParams = []KeyConfig{ - {keyType: "rsa", keyBits: 2048}, - {keyType: "rsa", keyBits: 4096}, - {keyType: "ec", keyBits: 224}, - {keyType: "ec", keyBits: 256}, - {keyType: "ec", keyBits: 384}, - {keyType: "ec", keyBits: 521}, +func skipIfMissingOpenSSL(t *testing.T) { + openSSLBinaryName := "openssl" + _, err := exec.LookPath(openSSLBinaryName) + if err != nil { + if mustAlwaysRun { + t.Fatalf("%q not found on $PATH", openSSLBinaryName) + } + t.Skipf("%q not found on $PATH", openSSLBinaryName) } - - _, err := exec.LookPath("openssl") - hasOpenSSL = err == nil } // Test that the TestCA and TestLeaf functions generate valid certificates. func testCAAndLeaf(t *testing.T, keyType string, keyBits int) { - if !hasOpenSSL { - t.Skip("openssl not found") - return - } + skipIfMissingOpenSSL(t) require := require.New(t) @@ -66,10 +59,7 @@ func testCAAndLeaf(t *testing.T, keyType string, keyBits int) { // Test cross-signing. func testCAAndLeaf_xc(t *testing.T, keyType string, keyBits int) { - if !hasOpenSSL { - t.Skip("openssl not found") - return - } + skipIfMissingOpenSSL(t) assert := assert.New(t)