feat: support multi account Datadog metrics provider (#3787)

* Adding support for multi datadog accounts. Signed-off-by: Ariadna Rouco <[email protected]> * Removing default secret name in the manifest. Signed-off-by: Ariadna Rouco <[email protected]> * Adding secret ref to analysis template, analsys run and cluster analysis template. Signed-off-by: Ariadna Rouco <[email protected]> * reentering removed command Signed-off-by: Ariadna Rouco <[email protected]> * Documentation updated Signed-off-by: Ariadna Rouco <[email protected]> * Update docs/analysis/datadog.md Co-authored-by: Thibault Jamet <[email protected]> Signed-off-by: Ariadna Rouco <[email protected]> * Adding removed code and removing string from log. Signed-off-by: Ariadna Rouco <[email protected]> * Rewriting documentation with the correct flow. Signed-off-by: Ariadna Rouco <[email protected]> * updated credential finders logic and added tests Signed-off-by: Ariadna Rouco <[email protected]> (cherry picked from commit 36a9c06) Signed-off-by: Ariadna Rouco <[email protected]> * Added test case for credentials not found. Signed-off-by: Ariadna Rouco <[email protected]> (cherry picked from commit 7703ff9) Signed-off-by: Ariadna Rouco <[email protected]> * added tests for findCredential method. (cherry picked from commit eab99be) Signed-off-by: Ariadna Rouco <[email protected]> * updating generated.pd.go Signed-off-by: Ariadna Rouco <[email protected]> --------- Signed-off-by: Ariadna Rouco <[email protected]> Signed-off-by: Ariadna Rouco <[email protected]> Co-authored-by: Thibault Jamet <[email protected]>
argoproj · Aug 27, 2024 · bfef7f0 · bfef7f0
1 parent 39764fb
commit bfef7f0
Show file tree

Hide file tree

Showing 26 changed files with 1,280 additions and 495 deletions.
diff --git a/.gitignore b/.gitignore
@@ -19,3 +19,4 @@ server/static/*
 !server/static/.gitkeep
 coverage-output-e2e/
 coverage-output-unit/
+junit-unit-test.xml
diff --git a/analysis/analysis.go b/analysis/analysis.go
@@ -332,7 +332,7 @@ func (c *Controller) runMeasurements(run *v1alpha1.AnalysisRun, tasks []metricTa
 			logger := logutil.WithRedactor(*logutil.WithAnalysisRun(run).WithField("metric", t.metric.Name), secrets)
 
 			var newMeasurement v1alpha1.Measurement
-			provider, providerErr := c.newProvider(*logger, t.metric)
+			provider, providerErr := c.newProvider(*logger, run.Namespace, t.metric)
 			if providerErr != nil {
 				log.Errorf("Error in getting metric provider :%v", providerErr)
 				if t.incompleteMeasurement != nil {
@@ -744,7 +744,7 @@ func (c *Controller) garbageCollectMeasurements(run *v1alpha1.AnalysisRun, measu
 				continue
 			}
 			logger := logutil.WithAnalysisRun(run).WithField("metric", metric.Name)
-			provider, err := c.newProvider(*logger, metric)
+			provider, err := c.newProvider(*logger, run.Namespace, metric)
 			if err != nil {
 				errors = append(errors, err)
 				continue

diff --git a/analysis/analysis_test.go b/analysis/analysis_test.go
@@ -1113,7 +1113,7 @@ func TestGarbageCollectArgResolution(t *testing.T) {
 	defer f.Close()
 	c, _, _ := f.newController(noResyncPeriodFunc)
 
-	c.newProvider = func(logCtx log.Entry, metric v1alpha1.Metric) (metric.Provider, error) {
+	c.newProvider = func(logCtx log.Entry, namespace string, metric v1alpha1.Metric) (metric.Provider, error) {
 		assert.Equal(t, "https://prometheus.kubeaddons:8080", metric.Provider.Prometheus.Address)
 		return f.provider, nil
 	}

diff --git a/analysis/controller.go b/analysis/controller.go
@@ -54,7 +54,7 @@ type Controller struct {
 
 	metricsServer *metrics.MetricsServer
 
-	newProvider func(logCtx log.Entry, metric v1alpha1.Metric) (metric.Provider, error)
+	newProvider func(logCtx log.Entry, namespace string, metric v1alpha1.Metric) (metric.Provider, error)
 
 	// used for unit testing
 	enqueueAnalysis      func(obj any)

diff --git a/analysis/controller_test.go b/analysis/controller_test.go
@@ -143,7 +143,7 @@ func (f *fixture) newController(resync resyncFunc) (*Controller, informers.Share
 		c.enqueueAnalysis(obj)
 	}
 	f.provider = &mocks.Provider{}
-	c.newProvider = func(logCtx log.Entry, metric v1alpha1.Metric) (metric.Provider, error) {
+	c.newProvider = func(logCtx log.Entry, namespace string, metric v1alpha1.Metric) (metric.Provider, error) {
 		return f.provider, nil
 	}
 
@@ -370,7 +370,7 @@ func TestFailedToCreateProviderError(t *testing.T) {
 	f.objects = append(f.objects, ar)
 
 	c, i, k8sI := f.newController(noResyncPeriodFunc)
-	c.newProvider = func(logCtx log.Entry, metric v1alpha1.Metric) (metric.Provider, error) {
+	c.newProvider = func(logCtx log.Entry, namespace string, metric v1alpha1.Metric) (metric.Provider, error) {
 		return nil, fmt.Errorf("failed to create provider")
 	}
 

diff --git a/docs/analysis/datadog.md b/docs/analysis/datadog.md
@@ -41,6 +41,48 @@ stringData:
 
 `apiVersion` here is different from the `apiVersion` from the Datadog configuration above.
 
+!!! important
+    ###### Namespaced secret
+    Datadog integration supports referring to secrets inside the same namespace as argo-rollouts (by default)
+    or referring to a secret in the same namespace as the `AnalysisTemplate`.
+
+    To use a secret from the `AnalysisTemplate` namespace, include a `secretRef` section in the template, specifying the `name` of the secret and setting the `namespaced` property to `true`.
+
+    The process for retrieving Datadog credentials is as follows:
+    1. **If a `secretRef` is defined in the `AnalysisTemplate`:** Argo Rollouts will search for the secret with the specified name in the namespace where the template resides.
+    2. **If the secret is not found in the specified namespace:** Argo Rollouts will then check the environment variables.
+    3. **If the credentials are not found in environment variables:** Argo Rollouts will look for a secret named "Datadog" in the namespace where Argo Rollouts itself is deployed.
+
+--- 
+
+Let me know if there's anything else you'd like to adjust!
+
+    ```yaml
+    apiVersion: argoproj.io/v1alpha1
+    kind: AnalysisTemplate
+    metadata:
+      name: loq-error-rate
+    spec:
+      args:
+      - name: service-name
+      metrics:
+      - name: error-rate
+        interval: 5m
+        successCondition: result <= 0.01
+        failureLimit: 3
+        provider:
+          datadog:
+            apiVersion: v2
+            interval: 5m
+            secretRef:
+              name: "mysecret"
+              namespaced: true
+            query: |
+              sum:requests.error.rate{service:{{args.service-name}}}
+    ```
+
+
+
 ### Working with Datadog API v2
 
 !!! important

diff --git a/docs/features/kustomize/rollout_cr_schema.json b/docs/features/kustomize/rollout_cr_schema.json
@@ -280,6 +280,17 @@
                                                     },
                                                     "query": {
                                                         "type": "string"
+                                                    },
+                                                    "secretRef": {
+                                                        "properties": {
+                                                            "name": {
+                                                                "type": "string"
+                                                            },
+                                                            "namespaced": {
+                                                                "type": "boolean"
+                                                            }
+                                                        },
+                                                        "type": "object"
                                                     }
                                                 },
                                                 "type": "object"
@@ -5135,6 +5146,17 @@
                                                     },
                                                     "query": {
                                                         "type": "string"
+                                                    },
+                                                    "secretRef": {
+                                                        "properties": {
+                                                            "name": {
+                                                                "type": "string"
+                                                            },
+                                                            "namespaced": {
+                                                                "type": "boolean"
+                                                            }
+                                                        },
+                                                        "type": "object"
                                                     }
                                                 },
                                                 "type": "object"
@@ -10003,6 +10025,17 @@
                                                     },
                                                     "query": {
                                                         "type": "string"
+                                                    },
+                                                    "secretRef": {
+                                                        "properties": {
+                                                            "name": {
+                                                                "type": "string"
+                                                            },
+                                                            "namespaced": {
+                                                                "type": "boolean"
+                                                            }
+                                                        },
+                                                        "type": "object"
                                                     }
                                                 },
                                                 "type": "object"

diff --git a/manifests/crds/analysis-run-crd.yaml b/manifests/crds/analysis-run-crd.yaml
@@ -207,6 +207,13 @@ spec:
                               type: object
                             query:
                               type: string
+                            secretRef:
+                              properties:
+                                name:
+                                  type: string
+                                namespaced:
+                                  type: boolean
+                              type: object
                           type: object
                         graphite:
                           properties:

diff --git a/manifests/crds/analysis-template-crd.yaml b/manifests/crds/analysis-template-crd.yaml
@@ -203,6 +203,13 @@ spec:
                               type: object
                             query:
                               type: string
+                            secretRef:
+                              properties:
+                                name:
+                                  type: string
+                                namespaced:
+                                  type: boolean
+                              type: object
                           type: object
                         graphite:
                           properties:

diff --git a/manifests/crds/cluster-analysis-template-crd.yaml b/manifests/crds/cluster-analysis-template-crd.yaml
@@ -203,6 +203,13 @@ spec:
                               type: object
                             query:
                               type: string
+                            secretRef:
+                              properties:
+                                name:
+                                  type: string
+                                namespaced:
+                                  type: boolean
+                              type: object
                           type: object
                         graphite:
                           properties:

diff --git a/manifests/install.yaml b/manifests/install.yaml
@@ -208,6 +208,13 @@ spec:
                               type: object
                             query:
                               type: string
+                            secretRef:
+                              properties:
+                                name:
+                                  type: string
+                                namespaced:
+                                  type: boolean
+                              type: object
                           type: object
                         graphite:
                           properties:
@@ -3509,6 +3516,13 @@ spec:
                               type: object
                             query:
                               type: string
+                            secretRef:
+                              properties:
+                                name:
+                                  type: string
+                                namespaced:
+                                  type: boolean
+                              type: object
                           type: object
                         graphite:
                           properties:
@@ -6688,6 +6702,13 @@ spec:
                               type: object
                             query:
                               type: string
+                            secretRef:
+                              properties:
+                                name:
+                                  type: string
+                                namespaced:
+                                  type: boolean
+                              type: object
                           type: object
                         graphite:
                           properties:

diff --git a/metricproviders/datadog/datadog.go b/metricproviders/datadog/datadog.go
@@ -2,17 +2,14 @@ package datadog
 
 import (
 	"bytes"
-	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
-	"os"
 	"reflect"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
@@ -22,7 +19,6 @@ import (
 	timeutil "github.com/argoproj/argo-rollouts/utils/time"
 
 	log "github.com/sirupsen/logrus"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 )
 
@@ -369,18 +365,6 @@ func (p *Provider) GarbageCollect(run *v1alpha1.AnalysisRun, metric v1alpha1.Met
 	return nil
 }
 
-func lookupKeysInEnv(keys []string) map[string]string {
-	valuesByKey := make(map[string]string)
-	for i := range keys {
-		key := keys[i]
-		formattedKey := strings.ToUpper(strings.ReplaceAll(key, "-", "_"))
-		if value, ok := os.LookupEnv(fmt.Sprintf("DD_%s", formattedKey)); ok {
-			valuesByKey[key] = value
-		}
-	}
-	return valuesByKey
-}
-
 // The current gen tooling we are using can't generate CRD with all the validations we need.
 // This is unfortunate, user has more ways to deliver an invalid Analysis Template vs
 // being rejected on delivery by k8s (and allowing for a validation step if desired in CI/CD).
@@ -422,32 +406,13 @@ func validateIncomingProps(dd *v1alpha1.DatadogMetric) error {
 	return nil
 }
 
-func NewDatadogProvider(logCtx log.Entry, kubeclientset kubernetes.Interface, metric v1alpha1.Metric) (*Provider, error) {
-	ns := defaults.Namespace()
-
-	apiKey := ""
-	appKey := ""
-	address := ""
-	secretKeys := []string{DatadogApiKey, DatadogAppKey, DatadogAddress}
-	envValuesByKey := lookupKeysInEnv(secretKeys)
-	if len(envValuesByKey) == len(secretKeys) {
-		apiKey = envValuesByKey[DatadogApiKey]
-		appKey = envValuesByKey[DatadogAppKey]
-		address = envValuesByKey[DatadogAddress]
-	} else {
-		secret, err := kubeclientset.CoreV1().Secrets(ns).Get(context.TODO(), DatadogTokensSecretName, metav1.GetOptions{})
-		if err != nil {
-			return nil, err
-		}
-		apiKey = string(secret.Data[DatadogApiKey])
-		appKey = string(secret.Data[DatadogAppKey])
-		if _, hasAddress := secret.Data[DatadogAddress]; hasAddress {
-			address = string(secret.Data[DatadogAddress])
-		}
+func NewDatadogProvider(logCtx log.Entry, kubeclientset kubernetes.Interface, namespace string, metric v1alpha1.Metric) (*Provider, error) {
+	address, apiKey, appKey, err := findCredentials(logCtx, kubeclientset, namespace, metric)
+	if err != nil {
+		return nil, err
 	}
 
 	if apiKey != "" && appKey != "" {
-
 		err := validateIncomingProps(metric.Provider.Datadog)
 		if err != nil {
 			return nil, err
@@ -465,3 +430,31 @@ func NewDatadogProvider(logCtx log.Entry, kubeclientset kubernetes.Interface, me
 		return nil, errors.New("API or App token not found")
 	}
 }
+
+func findCredentials(logCtx log.Entry, kubeclientset kubernetes.Interface, namespace string, metric v1alpha1.Metric) (string, string, string, error) {
+	finders := []CredentialsFinder{}
+	secretName := metric.Provider.Datadog.SecretRef.Name
+	namespaced := metric.Provider.Datadog.SecretRef.Namespaced
+	credentialsNs := defaults.Namespace()
+
+	if namespaced {
+		credentialsNs = namespace
+		if secretName == "" {
+			return "", "", "", errors.New("secret name is required for namespaced credentials")
+		}
+	}
+
+	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 != "" {
+			return address, apiKey, appKey, nil
+		}
+	}
+
+	return "", "", "", errors.New("failed to find the credentials for datadog metrics provider")
+}