From 44391a424036fd3922aa8ed30fbe888213851e04 Mon Sep 17 00:00:00 2001 From: Ville Aikas <11279988+vaikas@users.noreply.github.com> Date: Wed, 23 Sep 2020 00:52:27 -0700 Subject: [PATCH] optionals to pointers + tests (#1174) * optionals to pointers + tests Signed-off-by: Ville Aikas * oops, changelog Signed-off-by: Ville Aikas * move the entry in Changelog to Other section Signed-off-by: Ville Aikas Signed-off-by: silenceper --- CHANGELOG.md | 1 + api/v1alpha1/triggerauthentication_types.go | 20 ++-- api/v1alpha1/zz_generated.deepcopy.go | 18 +++- go.mod | 1 + pkg/scaling/resolver/scale_resolvers.go | 10 +- pkg/scaling/resolver/scale_resolvers_test.go | 108 ++++++++++++++++++- 6 files changed, 138 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c664b4a9bc3..37711f7fe5e 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/api/v1alpha1/triggerauthentication_types.go b/api/v1alpha1/triggerauthentication_types.go index b43b959a617..5d54b95531a 100644 --- a/api/v1alpha1/triggerauthentication_types.go +++ b/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 diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 01b16e7e9b9..89386e1a59d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -163,7 +163,11 @@ func (in *HashiCorpVault) DeepCopyInto(out *HashiCorpVault) { *out = make([]VaultSecret, len(*in)) copy(*out, *in) } - out.Credential = in.Credential + if in.Credential != nil { + in, out := &in.Credential, &out.Credential + *out = new(Credential) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HashiCorpVault. @@ -605,7 +609,11 @@ func (in *TriggerAuthenticationList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TriggerAuthenticationSpec) DeepCopyInto(out *TriggerAuthenticationSpec) { *out = *in - out.PodIdentity = in.PodIdentity + if in.PodIdentity != nil { + in, out := &in.PodIdentity, &out.PodIdentity + *out = new(AuthPodIdentity) + **out = **in + } if in.SecretTargetRef != nil { in, out := &in.SecretTargetRef, &out.SecretTargetRef *out = make([]AuthSecretTargetRef, len(*in)) @@ -616,7 +624,11 @@ func (in *TriggerAuthenticationSpec) DeepCopyInto(out *TriggerAuthenticationSpec *out = make([]AuthEnvironment, len(*in)) copy(*out, *in) } - in.HashiCorpVault.DeepCopyInto(&out.HashiCorpVault) + if in.HashiCorpVault != nil { + in, out := &in.HashiCorpVault, &out.HashiCorpVault + *out = new(HashiCorpVault) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TriggerAuthenticationSpec. diff --git a/go.mod b/go.mod index 7f30270f05a..b856197389c 100644 --- a/go.mod +++ b/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 diff --git a/pkg/scaling/resolver/scale_resolvers.go b/pkg/scaling/resolver/scale_resolvers.go index d14a0105065..a9828c0d763 100644 --- a/pkg/scaling/resolver/scale_resolvers.go +++ b/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) diff --git a/pkg/scaling/resolver/scale_resolvers_test.go b/pkg/scaling/resolver/scale_resolvers_test.go index ed3234b1678..b5eeb4f3088 100644 --- a/pkg/scaling/resolver/scale_resolvers_test.go +++ b/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) + } + }) + } +}