optionals to pointers + tests (#1174)

* optionals to pointers + tests

Signed-off-by: Ville Aikas <vaikas@vmware.com>

* oops, changelog

Signed-off-by: Ville Aikas <vaikas@vmware.com>

* move the entry in Changelog to Other section

Signed-off-by: Ville Aikas <vaikas@vmware.com>

CHANGELOG.md

@@ -56,6 +56,7 @@
 - Use `host` instead of `apiHost` in `rabbitmq` scaler. Add `protocol` in trigger spec to specify which protocol should be used ([#1115](https://github.com/kedacore/keda/pull/1115))
 
 ### Other
+- Change API optional structs to pointers to conform with k8s guide ([#1170](https://github.com/kedacore/keda/issues/1170))
 - Update Operator SDK and k8s deps ([#1007](https://github.com/kedacore/keda/pull/1007),[#870](https://github.com/kedacore/keda/issues/870),[#1180](https://github.com/kedacore/keda/pull/1180))
 - Change Metrics Server image name from `keda-metrics-adapter` to `keda-metrics-apiserver` ([#1105](https://github.com/kedacore/keda/issues/1105))
 

api/v1alpha1/triggerauthentication_types.go

@@ -22,16 +22,16 @@ type TriggerAuthentication struct {
 // TriggerAuthenticationSpec defines the various ways to authenticate
 type TriggerAuthenticationSpec struct {
 	// +optional
-	PodIdentity AuthPodIdentity `json:"podIdentity"`
+	PodIdentity *AuthPodIdentity `json:"podIdentity,omitempty"`
 
 	// +optional
-	SecretTargetRef []AuthSecretTargetRef `json:"secretTargetRef"`
+	SecretTargetRef []AuthSecretTargetRef `json:"secretTargetRef,omitempty"`
 
 	// +optional
-	Env []AuthEnvironment `json:"env"`
+	Env []AuthEnvironment `json:"env,omitempty"`
 
 	// +optional
-	HashiCorpVault HashiCorpVault `json:"hashiCorpVault"`
+	HashiCorpVault *HashiCorpVault `json:"hashiCorpVault,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -84,7 +84,7 @@ type AuthEnvironment struct {
 	Name      string `json:"name"`
 
 	// +optional
-	ContainerName string `json:"containerName"`
+	ContainerName string `json:"containerName,omitempty"`
 }
 
 // HashiCorpVault is used to authenticate using Hashicorp Vault
@@ -94,22 +94,22 @@ type HashiCorpVault struct {
 	Secrets        []VaultSecret       `json:"secrets"`
 
 	// +optional
-	Credential Credential `json:"credential"`
+	Credential *Credential `json:"credential,omitempty"`
 
 	// +optional
-	Role string `json:"role"`
+	Role string `json:"role,omitempty"`
 
 	// +optional
-	Mount string `json:"mount"`
+	Mount string `json:"mount,omitempty"`
 }
 
 // Credential defines the Hashicorp Vault credentials depending on the authentication method
 type Credential struct {
 	// +optional
-	Token string `json:"token"`
+	Token string `json:"token,omitempty"`
 
 	// +optional
-	ServiceAccount string `json:"serviceAccount"`
+	ServiceAccount string `json:"serviceAccount,omitempty"`
 }
 
 // VaultAuthentication contains the list of Hashicorp Vault authentication methods

go.mod

@@ -19,6 +19,7 @@ require (
 	github.com/go-sql-driver/mysql v1.5.0
 	github.com/golang/mock v1.4.4
 	github.com/golang/protobuf v1.4.2
+	github.com/google/go-cmp v0.5.1
 	github.com/hashicorp/vault/api v1.0.4
 	github.com/imdario/mergo v0.3.11
 	github.com/kubernetes-incubator/custom-metrics-apiserver v0.0.0-20200618121405-54026617ec44

pkg/scaling/resolver/scale_resolvers.go

@@ -47,13 +47,15 @@ func ResolveAuthRef(client client.Client, logger logr.Logger, triggerAuthRef *ke
 	result := make(map[string]string)
 	podIdentity := ""
 
-	if triggerAuthRef != nil && triggerAuthRef.Name != "" {
+	if namespace != "" && triggerAuthRef != nil && triggerAuthRef.Name != "" {
 		triggerAuth := &kedav1alpha1.TriggerAuthentication{}
 		err := client.Get(context.TODO(), types.NamespacedName{Name: triggerAuthRef.Name, Namespace: namespace}, triggerAuth)
 		if err != nil {
 			logger.Error(err, "Error getting triggerAuth", "triggerAuthRef.Name", triggerAuthRef.Name)
 		} else {
-			podIdentity = string(triggerAuth.Spec.PodIdentity.Provider)
+			if triggerAuth.Spec.PodIdentity != nil {
+				podIdentity = string(triggerAuth.Spec.PodIdentity.Provider)
+			}
 			if triggerAuth.Spec.Env != nil {
 				for _, e := range triggerAuth.Spec.Env {
 					env, err := ResolveContainerEnv(client, logger, podSpec, e.ContainerName, namespace)
@@ -69,8 +71,8 @@ func ResolveAuthRef(client client.Client, logger logr.Logger, triggerAuthRef *ke
 					result[e.Parameter] = resolveAuthSecret(client, logger, e.Name, namespace, e.Key)
 				}
 			}
-			if triggerAuth.Spec.HashiCorpVault.Secrets != nil {
-				vault := NewHashicorpVaultHandler(&triggerAuth.Spec.HashiCorpVault)
+			if triggerAuth.Spec.HashiCorpVault != nil && len(triggerAuth.Spec.HashiCorpVault.Secrets) > 0 {
+				vault := NewHashicorpVaultHandler(triggerAuth.Spec.HashiCorpVault)
 				err := vault.Initialize(logger)
 				if err != nil {
 					logger.Error(err, "Error authenticate to Vault", "triggerAuthRef.Name", triggerAuthRef.Name)

pkg/scaling/resolver/scale_resolvers_test.go

@@ -3,15 +3,24 @@ package resolver
 import (
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	kedav1alpha1 "github.com/kedacore/keda/api/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 var (
-	namespace  = "test-namespace"
-	trueValue  = true
-	falseValue = false
+	namespace                 = "test-namespace"
+	triggerAuthenticationName = "triggerauth"
+	secretName                = "supersecret"
+	secretKey                 = "mysecretkey"
+	secretData                = "secretDataHere"
+	trueValue                 = true
+	falseValue                = false
 )
 
 type testMetadata struct {
@@ -119,3 +128,96 @@ func TestResolveNonExistingConfigMapsOrSecretsEnv(t *testing.T) {
 		}
 	}
 }
+
+func TestResolveAuthRef(t *testing.T) {
+	corev1.AddToScheme(scheme.Scheme)
+	kedav1alpha1.AddToScheme(scheme.Scheme)
+	tests := []struct {
+		name                string
+		existing            []runtime.Object
+		soar                *kedav1alpha1.ScaledObjectAuthRef
+		podSpec             *corev1.PodSpec
+		expected            map[string]string
+		expectedPodIdentity string
+	}{
+		{
+			name:     "foo",
+			expected: make(map[string]string),
+		},
+		{
+			name:     "no triggerauth exists",
+			soar:     &kedav1alpha1.ScaledObjectAuthRef{Name: "notthere"},
+			expected: make(map[string]string),
+		},
+		{
+			name:     "no triggerauth exists",
+			soar:     &kedav1alpha1.ScaledObjectAuthRef{Name: "notthere"},
+			expected: make(map[string]string),
+		},
+		{
+			name: "triggerauth exists, podidentity nil",
+			existing: []runtime.Object{
+				&kedav1alpha1.TriggerAuthentication{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: namespace,
+						Name:      triggerAuthenticationName,
+					},
+					Spec: kedav1alpha1.TriggerAuthenticationSpec{
+						SecretTargetRef: []kedav1alpha1.AuthSecretTargetRef{
+							{
+								Parameter: "host",
+								Name:      secretName,
+								Key:       secretKey,
+							},
+						},
+					},
+				},
+			},
+			soar:     &kedav1alpha1.ScaledObjectAuthRef{Name: triggerAuthenticationName},
+			expected: map[string]string{"host": ""},
+		},
+		{
+			name: "triggerauth exists and secret",
+			existing: []runtime.Object{
+				&kedav1alpha1.TriggerAuthentication{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: namespace,
+						Name:      triggerAuthenticationName,
+					},
+					Spec: kedav1alpha1.TriggerAuthenticationSpec{
+						PodIdentity: &kedav1alpha1.AuthPodIdentity{
+							Provider: kedav1alpha1.PodIdentityProviderNone,
+						},
+						SecretTargetRef: []kedav1alpha1.AuthSecretTargetRef{
+							{
+								Parameter: "host",
+								Name:      secretName,
+								Key:       secretKey,
+							},
+						},
+					},
+				},
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: namespace,
+						Name:      secretName,
+					},
+					Data: map[string][]byte{secretKey: []byte(secretData)}},
+			},
+			soar:                &kedav1alpha1.ScaledObjectAuthRef{Name: triggerAuthenticationName},
+			expected:            map[string]string{"host": secretData},
+			expectedPodIdentity: "none",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			gotMap, gotPodIdentity := ResolveAuthRef(fake.NewFakeClientWithScheme(scheme.Scheme, test.existing...), logf.Log.WithName("test"), test.soar, test.podSpec, namespace)
+			if diff := cmp.Diff(gotMap, test.expected); diff != "" {
+				t.Errorf("Returned authParams are different: %s", diff)
+			}
+			if gotPodIdentity != test.expectedPodIdentity {
+				t.Errorf("Unexpected podidentity, wanted: %q got: %q", test.expectedPodIdentity, gotPodIdentity)
+			}
+		})
+	}
+}