From fbed9a1edf6dcecf8226a34c6ef943ed96accc76 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Thu, 16 Nov 2023 22:59:00 +0000 Subject: [PATCH 1/7] Add tests to hashicorpvault handler Signed-off-by: dttung2905 --- .../resolver/hashicorpvault_handler.go | 2 +- .../resolver/hashicorpvault_handler_test.go | 66 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/pkg/scaling/resolver/hashicorpvault_handler.go b/pkg/scaling/resolver/hashicorpvault_handler.go index 119ec9e62bd..1781a204413 100644 --- a/pkg/scaling/resolver/hashicorpvault_handler.go +++ b/pkg/scaling/resolver/hashicorpvault_handler.go @@ -310,7 +310,7 @@ func (vh *HashicorpVaultHandler) fetchSecret(secretType kedav1alpha1.VaultSecret // ResolveSecrets allows to resolve a slice of secrets by vault. The function returns the list of secrets with the value updated. // If multiple secret refers to the same SecretGroup, the secret will be fetched only once. func (vh *HashicorpVaultHandler) ResolveSecrets(secrets []kedav1alpha1.VaultSecret) ([]kedav1alpha1.VaultSecret, error) { - // Group secret by path and type, this allows to fetch a path only one. This is useful for dynamic credentials + // Group secret by path and type, this allows to fetch a path only once. This is useful for dynamic credentials grouped := make(map[SecretGroup][]kedav1alpha1.VaultSecret) vaultSecrets := make(map[SecretGroup]*vaultapi.Secret) for _, e := range secrets { diff --git a/pkg/scaling/resolver/hashicorpvault_handler_test.go b/pkg/scaling/resolver/hashicorpvault_handler_test.go index 41cab9c4017..47f2448107e 100644 --- a/pkg/scaling/resolver/hashicorpvault_handler_test.go +++ b/pkg/scaling/resolver/hashicorpvault_handler_test.go @@ -435,3 +435,69 @@ func TestHashicorpVaultHandler_ResolveSecrets_SameCertAndKey(t *testing.T) { assert.Len(t, secrets, 2, "Supposed to get back two secrets") assert.Equalf(t, secrets[0].Value, secrets[1].Value, "Refetching same path should yield same value") } + +var fetchSecretTestDataSet = []resolveRequestTestData{ + { + name: "existing_secret_v2", + path: "kv_v2/data/keda", + key: "test", + isError: false, + expectedValue: kedaSecretValue, + }, + { + name: "existing_secret_v1", + path: "kv/keda", + key: "test", + isError: false, + expectedValue: kedaSecretValue, + }, + { + name: "existing_pki", + path: "pki/issue/default", + key: "private_key_type", + isError: false, + secretType: kedav1alpha1.VaultSecretTypePki, + pkiData: kedav1alpha1.VaultPkiData{CommonName: "test"}, + expectedValue: "rsa", + }, + { + name: "existing_pki_ca_chain", + path: "pki/issue/default", + key: "ca_chain", + isError: false, + secretType: kedav1alpha1.VaultSecretTypePki, + pkiData: kedav1alpha1.VaultPkiData{CommonName: "test"}, + expectedValue: pkiCaChain, + }, +} + +func TestHashicorpVaultHandler_fetchSecret(t *testing.T) { + server := mockVault(t) + defer server.Close() + + vault := kedav1alpha1.HashiCorpVault{ + Address: server.URL, + Authentication: kedav1alpha1.VaultAuthenticationToken, + Credential: &kedav1alpha1.Credential{ + Token: vaultTestToken, + }, + } + vaultHandler := NewHashicorpVaultHandler(&vault) + err := vaultHandler.Initialize(logf.Log.WithName("test")) + defer vaultHandler.Stop() + assert.Nil(t, err) + + for _, testData := range fetchSecretTestDataSet { + secretResponse, err := vaultHandler.fetchSecret(testData.secretType, testData.path, &testData.pkiData) + assert.Nil(t, err) + + if testData.isError { + assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData) + } + secretStruct := kedav1alpha1.VaultSecret{Parameter: "test", Path: testData.path, Key: testData.key, Type: testData.secretType, PkiData: testData.pkiData} + secret, err := vaultHandler.getSecretValue(&secretStruct, secretResponse) + + assert.Nil(t, err) + assert.Equalf(t, testData.expectedValue, secret, "test %s: expected data does not match given secret", testData.name) + } +} From 7bf14d9829c90d7a582c3f3b18977ac4434089a9 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Sat, 18 Nov 2023 21:14:28 +0000 Subject: [PATCH 2/7] Add more tests Signed-off-by: dttung2905 --- .../resolver/hashicorpvault_handler_test.go | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/pkg/scaling/resolver/hashicorpvault_handler_test.go b/pkg/scaling/resolver/hashicorpvault_handler_test.go index 47f2448107e..05198c7c852 100644 --- a/pkg/scaling/resolver/hashicorpvault_handler_test.go +++ b/pkg/scaling/resolver/hashicorpvault_handler_test.go @@ -501,3 +501,117 @@ func TestHashicorpVaultHandler_fetchSecret(t *testing.T) { assert.Equalf(t, testData.expectedValue, secret, "test %s: expected data does not match given secret", testData.name) } } + +type initializeTestData struct { + name string + namespace string + token string + isError bool +} + +var initialiseTestDataSet = []initializeTestData{ + { + name: "Namespace and Token", + namespace: "testNamespace", + token: "testToken", + isError: false, + }, + { + name: "No Namespace", + namespace: "", + token: "testToken", + isError: false, + }, +} + +func TestHashicorpVaultHandler_Initialize(t *testing.T) { + server := mockVault(t) + defer server.Close() + + for _, testData := range initialiseTestDataSet { + func() { + vault := kedav1alpha1.HashiCorpVault{ + Address: server.URL, + Authentication: kedav1alpha1.VaultAuthenticationToken, + Credential: &kedav1alpha1.Credential{ + Token: testData.token, + }, + Namespace: testData.namespace, + } + vaultHandler := NewHashicorpVaultHandler(&vault) + err := vaultHandler.Initialize(logf.Log.WithName("test")) + defer vaultHandler.Stop() + assert.Nil(t, err) + + if testData.isError { + assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData) + } else { + assert.Equalf(t, vaultHandler.client.Address(), server.URL, "test case %s", testData.name) + assert.Equalf(t, vaultHandler.client.Token(), testData.token, "test case %s", testData.name) + assert.Equalf(t, vaultHandler.client.Namespace(), testData.namespace, "test case %s", testData.name) + } + }() + } +} + +type tokenTestData struct { + name string + isError bool + authentication kedav1alpha1.VaultAuthentication + credential kedav1alpha1.Credential + mount string + role string +} + +var tokenTestDataSet = []tokenTestData{ + { + name: "Vault Authentication", + isError: false, + authentication: kedav1alpha1.VaultAuthenticationToken, + credential: kedav1alpha1.Credential{ + Token: vaultTestToken, + }, + role: "my-role", + mount: "my-mount", + }, + { + name: "Kubernetes Authentication", + isError: true, // Because the service account path is non-existent + authentication: kedav1alpha1.VaultAuthenticationKubernetes, + credential: kedav1alpha1.Credential{ + ServiceAccount: "random/path", + }, + role: "my-role", + mount: "my-mount", + }, + // TODO: test wrong authentication +} + +func TestHashicorpVaultHandler_Token_VaultTokenAuth(t *testing.T) { + server := mockVault(t) + defer server.Close() + + for _, testData := range tokenTestDataSet { + func() { + vault := kedav1alpha1.HashiCorpVault{ + Address: server.URL, + Authentication: testData.authentication, + Credential: &testData.credential, + Role: testData.role, + Mount: testData.mount, + } + vaultHandler := NewHashicorpVaultHandler(&vault) + defer vaultHandler.Stop() + + config := vaultapi.DefaultConfig() + client, err := vaultapi.NewClient(config) + token, err := vaultHandler.token(client) + if testData.isError { + assert.Equalf(t, vaultHandler.vault.Credential.ServiceAccount, "random/path", "test %s: expected %s but found %s", testData.name, "random/path", vaultHandler.vault.Credential.ServiceAccount) + assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData) + } else { + assert.Equalf(t, token, vaultTestToken, "expected %s but got %s", vaultTestToken, token) + } + }() + } +} From b2424ff5046c9e46b59690d2f9ed7e6771bbfbc8 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Mon, 20 Nov 2023 22:15:22 +0000 Subject: [PATCH 3/7] Add more tests Signed-off-by: dttung2905 --- .../resolver/hashicorpvault_handler_test.go | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/pkg/scaling/resolver/hashicorpvault_handler_test.go b/pkg/scaling/resolver/hashicorpvault_handler_test.go index 05198c7c852..0f4d80523ab 100644 --- a/pkg/scaling/resolver/hashicorpvault_handler_test.go +++ b/pkg/scaling/resolver/hashicorpvault_handler_test.go @@ -290,7 +290,7 @@ var resolveRequestTestDataSet = []resolveRequestTestData{ key: "array", isError: false, expectedValue: "", - secretType: "inexisting_type", + secretType: "non_existing_type", }, { name: "existing_pki", @@ -472,7 +472,7 @@ var fetchSecretTestDataSet = []resolveRequestTestData{ } func TestHashicorpVaultHandler_fetchSecret(t *testing.T) { - server := mockVault(t) + server := mockVault(t, false) defer server.Close() vault := kedav1alpha1.HashiCorpVault{ @@ -525,7 +525,7 @@ var initialiseTestDataSet = []initializeTestData{ } func TestHashicorpVaultHandler_Initialize(t *testing.T) { - server := mockVault(t) + server := mockVault(t, false) defer server.Close() for _, testData := range initialiseTestDataSet { @@ -557,6 +557,7 @@ func TestHashicorpVaultHandler_Initialize(t *testing.T) { type tokenTestData struct { name string isError bool + errorMessage string authentication kedav1alpha1.VaultAuthentication credential kedav1alpha1.Credential mount string @@ -581,14 +582,25 @@ var tokenTestDataSet = []tokenTestData{ credential: kedav1alpha1.Credential{ ServiceAccount: "random/path", }, - role: "my-role", - mount: "my-mount", + role: "my-role", + mount: "my-mount", + errorMessage: "open random/path: no such file or directory", + }, + { + name: "Wrong Authentication Method", + isError: true, + authentication: "random", + credential: kedav1alpha1.Credential{ + ServiceAccount: "random/path", + }, + role: "my-role", + mount: "my-mount", + errorMessage: "vault auth method random is not supported", }, - // TODO: test wrong authentication } func TestHashicorpVaultHandler_Token_VaultTokenAuth(t *testing.T) { - server := mockVault(t) + server := mockVault(t, false) defer server.Close() for _, testData := range tokenTestDataSet { @@ -605,10 +617,12 @@ func TestHashicorpVaultHandler_Token_VaultTokenAuth(t *testing.T) { config := vaultapi.DefaultConfig() client, err := vaultapi.NewClient(config) + assert.Nil(t, err) token, err := vaultHandler.token(client) if testData.isError { - assert.Equalf(t, vaultHandler.vault.Credential.ServiceAccount, "random/path", "test %s: expected %s but found %s", testData.name, "random/path", vaultHandler.vault.Credential.ServiceAccount) + assert.Equalf(t, vaultHandler.vault.Credential.ServiceAccount, testData.credential.ServiceAccount, "test %s: expected %s but found %s", testData.name, "random/path", vaultHandler.vault.Credential.ServiceAccount) assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData) + assert.Contains(t, err.Error(), testData.errorMessage) } else { assert.Equalf(t, token, vaultTestToken, "expected %s but got %s", vaultTestToken, token) } From 44b5230d74788bc4e25b723473d61b6e2c62c139 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Mon, 20 Nov 2023 22:32:39 +0000 Subject: [PATCH 4/7] Add CHANGELOG.md Signed-off-by: dttung2905 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0fd5533843..02a4e6da5ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ Here is an overview of all new **experimental** features: - **General**: Add parameter queryParameters to prometheus-scaler ([#4962](https://github.com/kedacore/keda/issues/4962)) - **General**: Support TriggerAuthentication properties from ConfigMap ([#4830](https://github.com/kedacore/keda/issues/4830)) +- **Hashicorp Vault**: Improve test coverage in `pkg/scaling/resolver/hashicorpvault_handler` ([#5195](https://github.com/kedacore/keda/issues/5195)) - **Hashicorp Vault**: Add support to get secret that needs write operation (e.g. pki) ([#5067](https://github.com/kedacore/keda/issues/5067)) - **Hashicorp Vault**: Fix operator panic when spec.hashiCorpVault.credential.serviceAccount is not set ([#4964](https://github.com/kedacore/keda/issues/4964)) - **Hashicorp Vault**: Fix operator panic when using root token to authenticate to vault server ([#5192](https://github.com/kedacore/keda/issues/5192)) From 3dc87da6739a8c84482d544627d6f19d7834cd9a Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Mon, 20 Nov 2023 23:01:40 +0000 Subject: [PATCH 5/7] dummy commit to re-trigger CI build Signed-off-by: dttung2905 From acf24db3d292b7bc71addae0cab25a8a66e5e4ee Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Tue, 21 Nov 2023 08:38:55 +0000 Subject: [PATCH 6/7] Fix CHANGELOG Signed-off-by: dttung2905 --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02a4e6da5ba..e0fd5533843 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,7 +62,6 @@ Here is an overview of all new **experimental** features: - **General**: Add parameter queryParameters to prometheus-scaler ([#4962](https://github.com/kedacore/keda/issues/4962)) - **General**: Support TriggerAuthentication properties from ConfigMap ([#4830](https://github.com/kedacore/keda/issues/4830)) -- **Hashicorp Vault**: Improve test coverage in `pkg/scaling/resolver/hashicorpvault_handler` ([#5195](https://github.com/kedacore/keda/issues/5195)) - **Hashicorp Vault**: Add support to get secret that needs write operation (e.g. pki) ([#5067](https://github.com/kedacore/keda/issues/5067)) - **Hashicorp Vault**: Fix operator panic when spec.hashiCorpVault.credential.serviceAccount is not set ([#4964](https://github.com/kedacore/keda/issues/4964)) - **Hashicorp Vault**: Fix operator panic when using root token to authenticate to vault server ([#5192](https://github.com/kedacore/keda/issues/5192)) From b13d716be0efe324bcfc917a74f9b15001600f5b Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Tue, 21 Nov 2023 15:16:41 +0000 Subject: [PATCH 7/7] Fix CHANGELOG to other section Signed-off-by: dttung2905 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0fd5533843..e9e1687a23c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,7 @@ New deprecation(s): - **General**: Fix logger in Opentelemetry collector ([#5094](https://github.com/kedacore/keda/issues/5094)) - **General**: Reduce amount of gauge creations for OpenTelemetry metrics ([#5101](https://github.com/kedacore/keda/issues/5101)) - **General**: Support profiling for KEDA components ([#4789](https://github.com/kedacore/keda/issues/4789)) +- **Hashicorp Vault**: Improve test coverage in `pkg/scaling/resolver/hashicorpvault_handler` ([#5195](https://github.com/kedacore/keda/issues/5195)) ## v2.12.0