fix: safely parse renewable field from vault response (kedacore#5193)

* fix: safely parse renewable field from vault response Signed-off-by: Andika Ahmad Ramadhan <andikahmadr@gmail.com> * fix: typos Signed-off-by: Andika Ahmad Ramadhan <andikahmadr@gmail.com> --------- Signed-off-by: Andika Ahmad Ramadhan <andikahmadr@gmail.com> Signed-off-by: Yoon Park <yoongon.park@gmail.com>
yoongon · Nov 24, 2023 · 99e8b44 · 99e8b44
1 parent cec201b
commit 99e8b44
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -65,6 +65,7 @@ Here is an overview of all new **experimental** features:
 - **General**: Support TriggerAuthentication properties from ConfigMap ([#4830](https://github.com/kedacore/keda/issues/4830))
 - **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))
 - **Kafka Scaler**: Ability to set upper bound to the number of partitions with lag ([#3997](https://github.com/kedacore/keda/issues/3997))
 - **Kafka Scaler**: Add more logging to check Sarama DescribeTopics method ([#5102](https://github.com/kedacore/keda/issues/5102))
 - **Kafka Scaler**: Add support for Kerberos authentication (SASL / GSSAPI) ([#4836](https://github.com/kedacore/keda/issues/4836))

diff --git a/pkg/scaling/resolver/hashicorpvault_handler.go b/pkg/scaling/resolver/hashicorpvault_handler.go
@@ -75,8 +75,7 @@ func (vh *HashicorpVaultHandler) Initialize(logger logr.Logger) error {
 		return err
 	}
 
-	renew := lookup.Data["renewable"].(bool)
-	if renew {
+	if renew, ok := lookup.Data["renewable"].(bool); ok && renew {
 		vh.stopCh = make(chan struct{})
 		go vh.renewToken(logger)
 	}

diff --git a/pkg/scaling/resolver/hashicorpvault_handler_test.go b/pkg/scaling/resolver/hashicorpvault_handler_test.go
@@ -117,13 +117,16 @@ func TestGetPkiRequest(t *testing.T) {
 	}
 }
 
-func mockVault(t *testing.T) *httptest.Server {
+func mockVault(t *testing.T, useRootToken bool) *httptest.Server {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		var data map[string]interface{}
 		switch r.URL.Path {
 		case "/v1/auth/token/lookup-self":
-
 			data = vaultTokenSelf
+			if useRootToken {
+				// remove renewable field
+				delete(data, "renewable")
+			}
 		case "/v1/kv_v2/data/keda": //todo: more generic
 			data = kvV2SecretDataKeda
 		case "/v1/kv/keda": //todo: more generic
@@ -170,7 +173,7 @@ func mockVault(t *testing.T) *httptest.Server {
 }
 
 func TestHashicorpVaultHandler_getSecretValue_specify_secret_type(t *testing.T) {
-	server := mockVault(t)
+	server := mockVault(t, false)
 	defer server.Close()
 
 	vault := kedav1alpha1.HashiCorpVault{
@@ -191,7 +194,7 @@ func TestHashicorpVaultHandler_getSecretValue_specify_secret_type(t *testing.T)
 	}}
 	assert.Equalf(t, kedav1alpha1.VaultSecretTypeGeneric, secrets[0].Type, "Expected secret to not have a vlue")
 	secrets, _ = vaultHandler.ResolveSecrets(secrets)
-	assert.Len(t, secrets, 1, "Supposed to got back one secret")
+	assert.Len(t, secrets, 1, "Supposed to get back one secret")
 	secret := secrets[0]
 	assert.Equalf(t, kedav1alpha1.VaultSecretTypeSecretV2, secret.Type, "Expexted secret type be %s", kedav1alpha1.VaultSecretTypeSecretV2)
 	assert.Equalf(t, kedaSecretValue, secret.Value, "Expexted secret to be %s", kedaSecretValue)
@@ -202,7 +205,7 @@ func TestHashicorpVaultHandler_getSecretValue_specify_secret_type(t *testing.T)
 	}}
 	assert.Equalf(t, kedav1alpha1.VaultSecretTypeGeneric, secrets[0].Type, "Expected secret to not have a vlue")
 	secrets, _ = vaultHandler.ResolveSecrets(secrets)
-	assert.Len(t, secrets, 1, "Supposed to got back one secret")
+	assert.Len(t, secrets, 1, "Supposed to get back one secret")
 	secret = secrets[0]
 	assert.Equalf(t, kedav1alpha1.VaultSecretTypeSecret, secret.Type, "Expexted secret type be %s", kedav1alpha1.VaultSecretTypeSecret)
 	assert.Equalf(t, kedaSecretValue, secret.Value, "Expexted secret to be %s", kedaSecretValue)
@@ -310,7 +313,43 @@ var resolveRequestTestDataSet = []resolveRequestTestData{
 }
 
 func TestHashicorpVaultHandler_ResolveSecret(t *testing.T) {
-	server := mockVault(t)
+	server := mockVault(t, false)
+	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 resolveRequestTestDataSet {
+		secrets := []kedav1alpha1.VaultSecret{{
+			Parameter: "test",
+			Path:      testData.path,
+			Key:       testData.key,
+			Type:      testData.secretType,
+			PkiData:   testData.pkiData,
+		}}
+		secrets, err := vaultHandler.ResolveSecrets(secrets)
+		assert.Len(t, secrets, 1, "Supposed to get back one secret")
+		secret := secrets[0]
+		if testData.isError {
+			assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData)
+			continue
+		}
+		assert.Nilf(t, err, "test %s: expected success but got error - %s", testData.name, err)
+		assert.Equalf(t, testData.expectedValue, secret.Value, "test %s: expected data does not match given secret", testData.name)
+	}
+}
+
+func TestHashicorpVaultHandler_ResolveSecret_UsingRootToken(t *testing.T) {
+	server := mockVault(t, true)
 	defer server.Close()
 
 	vault := kedav1alpha1.HashiCorpVault{
@@ -334,7 +373,7 @@ func TestHashicorpVaultHandler_ResolveSecret(t *testing.T) {
 			PkiData:   testData.pkiData,
 		}}
 		secrets, err := vaultHandler.ResolveSecrets(secrets)
-		assert.Len(t, secrets, 1, "Supposed to got back one secret")
+		assert.Len(t, secrets, 1, "Supposed to get back one secret")
 		secret := secrets[0]
 		if testData.isError {
 			assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData)
@@ -347,7 +386,7 @@ func TestHashicorpVaultHandler_ResolveSecret(t *testing.T) {
 
 func TestHashicorpVaultHandler_DefaultKubernetesVaultRole(t *testing.T) {
 	defaultServiceAccountPath := "/var/run/secrets/kubernetes.io/serviceaccount/token"
-	server := mockVault(t)
+	server := mockVault(t, false)
 	defer server.Close()
 
 	vault := kedav1alpha1.HashiCorpVault{
@@ -365,7 +404,7 @@ func TestHashicorpVaultHandler_DefaultKubernetesVaultRole(t *testing.T) {
 }
 
 func TestHashicorpVaultHandler_ResolveSecrets_SameCertAndKey(t *testing.T) {
-	server := mockVault(t)
+	server := mockVault(t, false)
 	defer server.Close()
 
 	vault := kedav1alpha1.HashiCorpVault{
@@ -393,6 +432,6 @@ func TestHashicorpVaultHandler_ResolveSecrets_SameCertAndKey(t *testing.T) {
 		PkiData:   kedav1alpha1.VaultPkiData{CommonName: "test"},
 	}}
 	secrets, _ = vaultHandler.ResolveSecrets(secrets)
-	assert.Len(t, secrets, 2, "Supposed to got back two secret")
+	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")
 }