Skip to content

Commit

Permalink
Enforce error checking for Vault component initialization on tests.
Browse files Browse the repository at this point in the history 
Tests in vault_test.go had the following :

```go
    // This call will throw an error on Windows systems because of the of
    // the call x509.SystemCertPool() because system root pool is not
    // available on Windows so ignore the error for when the tests are run
    // on the Windows platform during CI
    _ = target.Init(m)
```

As of Go 1.18 this is not the case for Windows anymore and
we can instead enforce error checking. References:

* golang/go#16736
* golang/go#18609
* rancher/system-agent#84
* jaegertracing/jaeger#2756

Given Dapr depends on Go 1.19, we can enforce tests on `Init` result
and remove this comment.

While enforcing error checking we notice that the code above was
actually hiding errors in the test setup. Component initialization was
ending prematurely due to those errors and the test code was wrongfully
testing for the behavior of a component that has not been successfully
initialized. This is also addressed in this PR.

Closes #2330.

Signed-off-by: Tiago Alves Macambira <tmacam@burocrata.org>
  • Loading branch information
@tmacam
tmacam committed Nov 30, 2022
1 parent 47b0d73 commit b4c65ed
Showing 1 changed file with 58 additions and 59 deletions.
117 changes: 58 additions & 59 deletions secretstores/hashicorp/vault/vault_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,49 +28,53 @@ import (

const (
// base64 encoded certificate.
certificate = "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVakNDQWpvQ0NRRFlZdzdMeXN4VXRUQU5CZ2txaGtpRzl3MEJBUXNGQURCck1Rc3dDUVlEVlFRR0V3SkQKUVRFWk1CY0dBMVVFQ0F3UVFuSnBkR2x6YUNCRGIyeDFiV0pwWVRFU01CQUdBMVVFQnd3SlZtRnVZMjkxZG1WeQpNUk13RVFZRFZRUUtEQXB0YVhOb2NtRmpiM0p3TVJnd0ZnWURWUVFEREE5MllYVnNkSEJ5YjJwbFkzUXVhVzh3CkhoY05NVGt4TVRBeE1UQTBPREV5V2hjTk1qQXhNRE14TVRBME9ERXlXakJyTVFzd0NRWURWUVFHRXdKRFFURVoKTUJjR0ExVUVDQXdRUW5KcGRHbHphQ0JEYjJ4MWJXSnBZVEVTTUJBR0ExVUVCd3dKVm1GdVkyOTFkbVZ5TVJNdwpFUVlEVlFRS0RBcHRhWE5vY21GamIzSndNUmd3RmdZRFZRUUREQTkyWVhWc2RIQnliMnBsWTNRdWFXOHdnZ0VpCk1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRQ3JtaitTTmtGUHEvK2FXUFV1MlpFamtSK3AKTm1PeEVNSnZZcGhHNkJvRFAySE9ZbGRzdk9FWkRkbTBpWFlmeFIwZm5rUmtTMWEzSlZiYmhINWJnTElKb0dxcwo5aWpzN2hyQ0Rrdk9uRWxpUEZuc05pQ2NWNDNxNkZYaFMvNFpoNGpOMnlyUkU2UmZiS1BEeUw0a282NkFhSld1CnVkTldKVWpzSFZBSWowZHlnTXFKYm0rT29iSzk5ckUxcDg5Z3RNUStJdzFkWnUvUFF4SjlYOStMeXdxZUxPckQKOWhpNWkxajNFUUp2RXQxSVUzclEwc2E0NU5zZkt4YzEwZjdhTjJuSDQzSnhnMVRiZXNPOWYrcWlyeDBHYmVSYQpyVmNaazNVaFc2cHZmam9XbDBEc0NwNTJwZDBQN05rUmhmak44b2RMN0h3bFVIc1NqemlSYytsTG5YREJBZ01CCkFBRXdEUVlKS29aSWh2Y05BUUVMQlFBRGdnRUJBSVdKdmRPZ01PUnQxWk53SENkNTNieTlkMlBkcW5tWHFZZ20KNDZHK2Fvb1dSeTJKMEMwS3ZOVGZGbEJFOUlydzNXUTVNMnpqY25qSUp5bzNLRUM5TDdPMnQ1WC9LTGVDck5ZVgpIc1d4cU5BTVBGY2VBa09HT0I1TThGVllkdjJTaVV2UDJjMEZQSzc2WFVzcVNkdnRsWGFkTk5ENzE3T0NTNm0yCnBIVjh1NWJNd1VmR2NCVFpEV2o4bjIzRVdHaXdnYkJkdDc3Z3h3YWc5NTROZkM2Ny9nSUc5ZlRrTTQ4aVJCUzEKc0NGYVBjMkFIT3hiMSs0ajVCMVY2Z29iZDZYWkFvbHdNaTNHUUtkbEM1NXZNeTNwK09WbDNNbEc3RWNTVUpMdApwZ2ZKaWw3L3dTWWhpUnhJU3hrYkk5cWhvNEwzZm5PZVB3clFVd2FzU1ZiL1lxbHZ2WHM9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"
expectedTok = "myRootToken"
expectedTokMountPath = "./vault.txt"
certificate = "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURVakNDQWpvQ0NRRFlZdzdMeXN4VXRUQU5CZ2txaGtpRzl3MEJBUXNGQURCck1Rc3dDUVlEVlFRR0V3SkQKUVRFWk1CY0dBMVVFQ0F3UVFuSnBkR2x6YUNCRGIyeDFiV0pwWVRFU01CQUdBMVVFQnd3SlZtRnVZMjkxZG1WeQpNUk13RVFZRFZRUUtEQXB0YVhOb2NtRmpiM0p3TVJnd0ZnWURWUVFEREE5MllYVnNkSEJ5YjJwbFkzUXVhVzh3CkhoY05NVGt4TVRBeE1UQTBPREV5V2hjTk1qQXhNRE14TVRBME9ERXlXakJyTVFzd0NRWURWUVFHRXdKRFFURVoKTUJjR0ExVUVDQXdRUW5KcGRHbHphQ0JEYjJ4MWJXSnBZVEVTTUJBR0ExVUVCd3dKVm1GdVkyOTFkbVZ5TVJNdwpFUVlEVlFRS0RBcHRhWE5vY21GamIzSndNUmd3RmdZRFZRUUREQTkyWVhWc2RIQnliMnBsWTNRdWFXOHdnZ0VpCk1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFRQ3JtaitTTmtGUHEvK2FXUFV1MlpFamtSK3AKTm1PeEVNSnZZcGhHNkJvRFAySE9ZbGRzdk9FWkRkbTBpWFlmeFIwZm5rUmtTMWEzSlZiYmhINWJnTElKb0dxcwo5aWpzN2hyQ0Rrdk9uRWxpUEZuc05pQ2NWNDNxNkZYaFMvNFpoNGpOMnlyUkU2UmZiS1BEeUw0a282NkFhSld1CnVkTldKVWpzSFZBSWowZHlnTXFKYm0rT29iSzk5ckUxcDg5Z3RNUStJdzFkWnUvUFF4SjlYOStMeXdxZUxPckQKOWhpNWkxajNFUUp2RXQxSVUzclEwc2E0NU5zZkt4YzEwZjdhTjJuSDQzSnhnMVRiZXNPOWYrcWlyeDBHYmVSYQpyVmNaazNVaFc2cHZmam9XbDBEc0NwNTJwZDBQN05rUmhmak44b2RMN0h3bFVIc1NqemlSYytsTG5YREJBZ01CCkFBRXdEUVlKS29aSWh2Y05BUUVMQlFBRGdnRUJBSVdKdmRPZ01PUnQxWk53SENkNTNieTlkMlBkcW5tWHFZZ20KNDZHK2Fvb1dSeTJKMEMwS3ZOVGZGbEJFOUlydzNXUTVNMnpqY25qSUp5bzNLRUM5TDdPMnQ1WC9LTGVDck5ZVgpIc1d4cU5BTVBGY2VBa09HT0I1TThGVllkdjJTaVV2UDJjMEZQSzc2WFVzcVNkdnRsWGFkTk5ENzE3T0NTNm0yCnBIVjh1NWJNd1VmR2NCVFpEV2o4bjIzRVdHaXdnYkJkdDc3Z3h3YWc5NTROZkM2Ny9nSUc5ZlRrTTQ4aVJCUzEKc0NGYVBjMkFIT3hiMSs0ajVCMVY2Z29iZDZYWkFvbHdNaTNHUUtkbEM1NXZNeTNwK09WbDNNbEc3RWNTVUpMdApwZ2ZKaWw3L3dTWWhpUnhJU3hrYkk5cWhvNEwzZm5PZVB3clFVd2FzU1ZiL1lxbHZ2WHM9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"
expectedTok = "myRootToken"
expectedTokenMountFileContents = "Hey! TokenMountFile contents here!"
)

func TestReadVaultToken(t *testing.T) {
t.Run("read correct token", func(t *testing.T) {
dir := os.TempDir()
f, err := os.CreateTemp(dir, "vault-token")
assert.NoError(t, err)
fileName := f.Name()
defer os.Remove(fileName)
func createTempFileWithContent(t *testing.T, contents string) (fileName string, cleanUpFunc func()) {
dir := os.TempDir()
f, err := os.CreateTemp(dir, "vault-token")
assert.NoError(t, err)
fileName = f.Name()
cleanUpFunc = func() {
os.Remove(fileName)
}

_, err = f.WriteString(contents)
assert.NoError(t, err)

return fileName, cleanUpFunc
}

func createTokenMountPathFile(t *testing.T) (fileName string, cleanUpFunc func()) {
return createTempFileWithContent(t, expectedTokenMountFileContents)
}

tokenString := "thisisnottheroottoken"
_, err = f.WriteString(tokenString)
assert.NoError(t, err)
func TestReadVaultToken(t *testing.T) {
tokenString := "This-IS-TheRootToken"
tmpFileName, cleanUpFunc := createTempFileWithContent(t, tokenString)
defer cleanUpFunc()

t.Run("read correct token", func(t *testing.T) {
v := vaultSecretStore{
vaultTokenMountPath: f.Name(),
vaultTokenMountPath: tmpFileName,
}

err = v.initVaultToken()
err := v.initVaultToken()
assert.Nil(t, err)
assert.Equal(t, tokenString, v.vaultToken)
})

t.Run("read incorrect token", func(t *testing.T) {
dir := os.TempDir()
f, err := os.CreateTemp(dir, "vault-token")
assert.NoError(t, err)
fileName := f.Name()
defer os.Remove(fileName)

tokenString := "thisisnottheroottoken"
_, err = f.WriteString(tokenString)
assert.NoError(t, err)

v := vaultSecretStore{
vaultTokenMountPath: f.Name(),
vaultTokenMountPath: tmpFileName,
}
err = v.initVaultToken()

err := v.initVaultToken()
assert.Nil(t, err)
assert.NotEqual(t, "thisistheroottoken", v.vaultToken)
assert.NotEqual(t, "ThisIs-NOT-TheRootToken", v.vaultToken)
})

t.Run("read token from vaultToken", func(t *testing.T) {
Expand Down Expand Up @@ -126,6 +130,9 @@ func TestVaultEnginePath(t *testing.T) {
}

func TestVaultTokenPrefix(t *testing.T) {
expectedTokMountPath, cleanUpFunc := createTokenMountPathFile(t)
defer cleanUpFunc()

t.Run("default value of vaultKVUsePrefix is true to emulate previous behaviour", func(t *testing.T) {
properties := map[string]string{
componentVaultToken: expectedTok,
Expand All @@ -140,11 +147,9 @@ func TestVaultTokenPrefix(t *testing.T) {
logger: nil,
}

// This call will throw an error on Windows systems because of the of
// the call x509.SystemCertPool() because system root pool is not
// available on Windows so ignore the error for when the tests are run
// on the Windows platform during CI
_ = target.Init(m)
if err := target.Init(m); err != nil {
t.Fatal(err)
}

assert.Equal(t, defaultVaultKVPrefix, target.vaultKVPrefix)
})
Expand All @@ -165,11 +170,9 @@ func TestVaultTokenPrefix(t *testing.T) {
logger: nil,
}

// This call will throw an error on Windows systems because of the of
// the call x509.SystemCertPool() because system root pool is not
// available on Windows so ignore the error for when the tests are run
// on the Windows platform during CI
_ = target.Init(m)
if err := target.Init(m); err != nil {
t.Fatal(err)
}

assert.Equal(t, "", target.vaultKVPrefix)
})
Expand All @@ -190,17 +193,16 @@ func TestVaultTokenPrefix(t *testing.T) {
logger: nil,
}

// This call will throw an error on Windows systems because of the of
// the call x509.SystemCertPool() because system root pool is not
// available on Windows so ignore the error for when the tests are run
// on the Windows platform during CI
err := target.Init(m)

assert.NotNil(t, err)
})
}

func TestVaultTokenMountPathOrVaultTokenRequired(t *testing.T) {
expectedTokMountPath, cleanUpFunc := createTokenMountPathFile(t)
defer cleanUpFunc()

t.Run("without vaultTokenMount or vaultToken", func(t *testing.T) {
properties := map[string]string{}

Expand Down Expand Up @@ -235,13 +237,11 @@ func TestVaultTokenMountPathOrVaultTokenRequired(t *testing.T) {
logger: nil,
}

// This call will throw an error on Windows systems because of the of
// the call x509.SystemCertPool() because system root pool is not
// available on Windows so ignore the error for when the tests are run
// on the Windows platform during CI
_ = target.Init(m)
if err := target.Init(m); err != nil {
t.Fatal(err)
}

assert.Equal(t, "", target.vaultToken)
assert.Equal(t, expectedTokenMountFileContents, target.vaultToken)
assert.Equal(t, expectedTokMountPath, target.vaultTokenMountPath)
})

Expand All @@ -259,11 +259,9 @@ func TestVaultTokenMountPathOrVaultTokenRequired(t *testing.T) {
logger: nil,
}

// This call will throw an error on Windows systems because of the of
// the call x509.SystemCertPool() because system root pool is not
// available on Windows so ignore the error for when the tests are run
// on the Windows platform during CI
_ = target.Init(m)
if err := target.Init(m); err != nil {
t.Fatal(err)
}

assert.Equal(t, "", target.vaultTokenMountPath)
assert.Equal(t, expectedTok, target.vaultToken)
Expand Down Expand Up @@ -294,9 +292,12 @@ func TestVaultTokenMountPathOrVaultTokenRequired(t *testing.T) {
}

func TestDefaultVaultAddress(t *testing.T) {
expectedTokMountPath, cleanUpFunc := createTokenMountPathFile(t)
defer cleanUpFunc()

t.Run("with blank vaultAddr", func(t *testing.T) {
properties := map[string]string{
"vaultTokenMountPath": "./vault.txt",
"vaultTokenMountPath": expectedTokMountPath,
}

m := secretstores.Metadata{
Expand All @@ -308,11 +309,9 @@ func TestDefaultVaultAddress(t *testing.T) {
logger: nil,
}

// This call will throw an error on Windows systems because of the of
// the call x509.SystemCertPool() because system root pool is not
// available on Windows so ignore the error for when the tests are run
// on the Windows platform during CI
_ = target.Init(m)
if err := target.Init(m); err != nil {
t.Fatal(err)
}

assert.Equal(t, defaultVaultAddress, target.vaultAddress, "default was not set")
})
Expand Down

0 comments on commit b4c65ed

Please # to comment.