From 36a9c0601eef055eaa88faa8cfd04fd76fc54b87 Mon Sep 17 00:00:00 2001 From: Ariadna Rouco Date: Mon, 19 Aug 2024 17:08:34 +0200 Subject: [PATCH] updated credential finders logic and added tests Signed-off-by: Ariadna Rouco --- .gitignore | 1 + metricproviders/datadog/datadog.go | 18 ++-- metricproviders/datadog/datadogV1_test.go | 29 +++++- metricproviders/datadog/finders.go | 8 +- metricproviders/datadog/finders_test.go | 111 ++++++++++++++++++++++ 5 files changed, 158 insertions(+), 9 deletions(-) create mode 100644 metricproviders/datadog/finders_test.go diff --git a/.gitignore b/.gitignore index ab4b2ac69e..ce23e23261 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ server/static/* !server/static/.gitkeep coverage-output-e2e/ coverage-output-unit/ +junit-unit-test.xml \ No newline at end of file diff --git a/metricproviders/datadog/datadog.go b/metricproviders/datadog/datadog.go index 0d99445ce8..8778b51827 100644 --- a/metricproviders/datadog/datadog.go +++ b/metricproviders/datadog/datadog.go @@ -435,16 +435,20 @@ func findCredentials(logCtx log.Entry, kubeclientset kubernetes.Interface, names finders := []CredentialsFinder{} secretName := metric.Provider.Datadog.SecretRef.Name namespaced := metric.Provider.Datadog.SecretRef.Namespaced + credentialsNs := defaults.Namespace() - if secretName != "" { - if namespaced { - finders = append(finders, NewSecretFinder(kubeclientset, secretName, namespace)) - } else { - finders = append(finders, NewSecretFinder(kubeclientset, secretName, defaults.Namespace())) + if namespaced { + credentialsNs = namespace + if secretName == "" { + return "", "", "", errors.New("secret name is required for namespaced credentials") } } - finders = append(finders, NewEnvVariablesFinder(), NewSecretFinder(kubeclientset, DatadogTokensSecretName, defaults.Namespace())) + if secretName != "" { + finders = append(finders, NewSecretFinder(kubeclientset, secretName, credentialsNs)) + } else { + finders = append(finders, NewEnvVariablesFinder(), NewSecretFinder(kubeclientset, DatadogTokensSecretName, defaults.Namespace())) + } for _, finder := range finders { address, apiKey, appKey := finder.FindCredentials(logCtx) if address != "" && apiKey != "" && appKey != "" { @@ -452,5 +456,5 @@ func findCredentials(logCtx log.Entry, kubeclientset kubernetes.Interface, names } } - return "", "", "", errors.New("failed to find the credentials for datadog provider") + return "", "", "", errors.New("failed to find the credentials for datadog metrics provider") } diff --git a/metricproviders/datadog/datadogV1_test.go b/metricproviders/datadog/datadogV1_test.go index c45fa14695..e526d4856e 100644 --- a/metricproviders/datadog/datadogV1_test.go +++ b/metricproviders/datadog/datadogV1_test.go @@ -46,9 +46,19 @@ func TestRunSuite(t *testing.T) { }, }, } + ddProviderNamespacedTrue := v1alpha1.MetricProvider{ + Datadog: &v1alpha1.DatadogMetric{ + Query: "avg:kubernetes.cpu.user.total{*}", + Interval: "10m", + SecretRef: v1alpha1.SecretRef{ + Namespaced: true, + }, + }, + } // Test Cases tests := []struct { + name string serverURL string webServerStatus int webServerResponse string @@ -57,6 +67,7 @@ func TestRunSuite(t *testing.T) { expectedValue string expectedPhase v1alpha1.AnalysisPhase expectedErrorMessage string + expectedErrorProvider bool useEnvVarForKeys bool }{ // When last value of time series matches condition then succeed. @@ -250,6 +261,18 @@ func TestRunSuite(t *testing.T) { expectedValue: "0.0003332881882246533", expectedPhase: v1alpha1.AnalysisPhaseSuccessful, useEnvVarForKeys: false, + expectedErrorProvider: false, + }, + // When secretRef is namespaced true but no secret name passed, expect failure + { + metric: v1alpha1.Metric{ + Name: "foo", + SuccessCondition: "result < 0.001", + FailureCondition: "result >= 0.001", + Provider: ddProviderNamespacedTrue, + }, + useEnvVarForKeys: false, + expectedErrorProvider: true, }, } @@ -347,7 +370,11 @@ func TestRunSuite(t *testing.T) { } namespace := "namespace" - provider, _ := NewDatadogProvider(*logCtx, fakeClient, namespace, test.metric) + provider, err := NewDatadogProvider(*logCtx, fakeClient, namespace, test.metric) + assert.Equal(t, err != nil, test.expectedErrorProvider) + if test.expectedErrorProvider { + continue + } metricsMetadata := provider.GetMetadata(test.metric) assert.Nil(t, metricsMetadata) diff --git a/metricproviders/datadog/finders.go b/metricproviders/datadog/finders.go index 293394c7aa..3e66121c09 100644 --- a/metricproviders/datadog/finders.go +++ b/metricproviders/datadog/finders.go @@ -33,11 +33,17 @@ func (sf *secretFinder) FindCredentials(logCtx log.Entry) (string, string, strin address := "" secret, err := sf.kubeclientset.CoreV1().Secrets(sf.namespace).Get(context.TODO(), sf.secretName, metav1.GetOptions{}) if err != nil { - logCtx.Debugf("secret %s in namespace %s", sf.namespace, sf.secretName) + logCtx.Debugf("error searching for secret %s in namespace %s: %s", sf.secretName, sf.namespace, err.Error()) return "", "", "" } + apiKey := string(secret.Data[DatadogApiKey]) appKey := string(secret.Data[DatadogAppKey]) + + if apiKey == "" || appKey == "" { + logCtx.Debugf("credentials missing in secret %s in namespace %s", sf.secretName, sf.namespace) + return "", "", "" + } if _, hasAddress := secret.Data[DatadogAddress]; hasAddress { address = string(secret.Data[DatadogAddress]) } diff --git a/metricproviders/datadog/finders_test.go b/metricproviders/datadog/finders_test.go new file mode 100644 index 0000000000..5659eab929 --- /dev/null +++ b/metricproviders/datadog/finders_test.go @@ -0,0 +1,111 @@ +package datadog + +import ( + "fmt" + "testing" + + log "github.com/sirupsen/logrus" + "github.com/tj/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + k8sfake "k8s.io/client-go/kubernetes/fake" + kubetesting "k8s.io/client-go/testing" +) + +const ( + DatadogUrl = "datadog.com" + ApiKey = "apiKey" + AppKey = "appKey" +) + +func TestRunSuit(t *testing.T) { + + testCases := []struct { + name string + secret *corev1.Secret + expectedApiKey string + expectedAppKey string + expectedAddress string + expectError bool + }{ + { + name: "When secret valid, should be successful", + secret: NewSecretBuilder(). + WithName("DatadogTokensSecretName"). + WithData("address", []byte(DatadogUrl)). + WithData("api-key", []byte(ApiKey)). + WithData("app-key", []byte(AppKey)). + Build(), + expectedApiKey: ApiKey, + expectedAppKey: AppKey, + expectedAddress: DatadogUrl, + }, + { + name: "When secret is found but no data, should return empty values", + secret: NewSecretBuilder(). + WithName("DatadogTokensSecretName"). + Build(), + }, + { + name: "When secret not found, should return empty values", + expectError: true, + }, + } + + for _, tc := range testCases { + fakeClient := k8sfake.NewSimpleClientset() + fakeClient.PrependReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + if tc.expectError { + return true, nil, fmt.Errorf("api error") + } + return true, tc.secret, nil + }) + + secretFinder := NewSecretFinder(fakeClient, "", "") + t.Run(tc.name, func(t *testing.T) { + logCtx := *log.WithField("test", "test") + + address, apiKey, appKey := secretFinder.FindCredentials(logCtx) + assert.Equal(t, tc.expectedAddress, address) + assert.Equal(t, tc.expectedApiKey, apiKey) + assert.Equal(t, tc.expectedAppKey, appKey) + }) + } + +} + +// SecretBuilder helps in constructing a corev1.Secret object +type SecretBuilder struct { + name string + data map[string][]byte +} + +// NewSecretBuilder creates a new SecretBuilder +func NewSecretBuilder() *SecretBuilder { + return &SecretBuilder{ + data: make(map[string][]byte), + } +} + +// WithName sets the name for the Secret +func (b *SecretBuilder) WithName(name string) *SecretBuilder { + b.name = name + return b +} + +// WithData sets the data for the Secret +func (b *SecretBuilder) WithData(key string, value []byte) *SecretBuilder { + b.data[key] = value + return b +} + +// Build constructs the corev1.Secret object +func (b *SecretBuilder) Build() *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: b.name, + }, + Data: b.data, + } +}