Fetch Site or DdUrl before initializing MetricsForwarder

Make golint happy by renaming baseUrl baseURL

Fix site reference

Log the base URL that was fetched

Co-authored-by: Ahmed Mezghani <38987709+ahmed-mez@users.noreply.github.com>

Add unit test for getbaseURL

re-fix the potential nil Agent

Add test case for testing precedence of DDUrl over Site

Use defaultBaseURL const in return

Undo change on datadogagent_controller.go

Remove Todo

pkg/apis/datadoghq/v1alpha1/test/new.go

@@ -57,6 +57,7 @@ type NewDatadogAgentOptions struct {
 	ClusterChecksRunnerVolumeMounts []corev1.VolumeMount
 	ClusterChecksRunnerEnvVars      []corev1.EnvVar
 	APIKeyExistingSecret            string
+	Site                            string
 }
 
 // NewDefaultedDatadogAgent returns an initialized and defaulted DatadogAgent for testing purpose
@@ -86,6 +87,7 @@ func NewDefaultedDatadogAgent(ns, name string, options *NewDatadogAgentOptions)
 			Log:                datadoghqv1alpha1.LogSpec{},
 			Process:            datadoghqv1alpha1.ProcessSpec{},
 		},
+		Site: options.Site,
 	}
 	if options != nil {
 		if options.UseEDS {

pkg/controller/utils/datadog/metrics_forwarder.go

@@ -49,6 +49,7 @@ const (
 	reconcileMetricFormat       = "%s.reconcile.success"
 	reconcileErrTagFormat       = "reconcile_err:%s"
 	datadogOperatorSourceType   = "datadog_operator"
+	defaultbaseURL              = "https://api.datadoghq.com"
 )
 
 var (
@@ -81,6 +82,7 @@ type metricsForwarder struct {
 	delegator           delegatedAPI
 	decryptor           secrets.Decryptor
 	creds               sync.Map
+	baseURL             string
 	sync.Mutex
 	status *datadoghqv1alpha1.DatadogAgentCondition
 }
@@ -108,6 +110,7 @@ func newMetricsForwarder(k8sClient client.Client, decryptor secrets.Decryptor, o
 		lastReconcileErr:    errInitValue,
 		decryptor:           decryptor,
 		creds:               sync.Map{},
+		baseURL:             defaultbaseURL,
 		logger:              log.WithValues("CustomResource.Namespace", obj.GetNamespace(), "CustomResource.Name", obj.GetName()),
 	}
 }
@@ -133,7 +136,7 @@ func (mf *metricsForwarder) start(wg *sync.WaitGroup) {
 		return
 	}
 
-	mf.logger.Info("Datadog metrics forwarder initilized successfully")
+	mf.logger.Info("Datadog metrics forwarder initialized successfully")
 
 	// Send CR detection event
 	crEvent := crDetected(mf.id)
@@ -200,6 +203,8 @@ func (mf *metricsForwarder) connectToDatadogAPI() (bool, error) {
 	}
 	mf.logger.Info("Getting Datadog credentials")
 	apiKey, appKey, err := mf.getCredentials(dda)
+	mf.baseURL = getbaseURL(dda)
+	mf.logger.Info("Got Datadog Site", "site", mf.baseURL)
 	defer mf.updateStatusIfNeeded(err)
 	if err != nil {
 		mf.logger.Error(err, "cannot get Datadog credentials,  will retry later...")
@@ -350,12 +355,13 @@ func (mf *metricsForwarder) validateCreds(apiKey, appKey string) (*api.Client, e
 // delegatedValidateCreds is separated from validateCreds to facilitate mocking the Datadog API
 func (mf *metricsForwarder) delegatedValidateCreds(apiKey, appKey string) (*api.Client, error) {
 	datadogClient := api.NewClient(apiKey, appKey)
+	datadogClient.SetBaseUrl(mf.baseURL)
 	valid, err := datadogClient.Validate()
 	if err != nil {
 		return nil, fmt.Errorf("cannot validate datadog credentials: %v", err)
 	}
 	if !valid {
-		return nil, errors.New("invalid datadog credentials")
+		return nil, fmt.Errorf("invalid datadog credentials on %s", mf.baseURL)
 	}
 	return datadogClient, nil
 }
@@ -653,3 +659,11 @@ func (mf *metricsForwarder) isErrChanFull() bool {
 func (mf *metricsForwarder) isEventChanFull() bool {
 	return len(mf.eventChan) == cap(mf.eventChan)
 }
+
+func getbaseURL(dda *datadoghqv1alpha1.DatadogAgent) string {
+	if dda.Spec.Agent != nil && dda.Spec.Agent.Config.DDUrl != nil {
+		return *dda.Spec.Agent.Config.DDUrl
+	} else if dda.Spec.Site != "" {
+		return fmt.Sprintf("https://api.%s", dda.Spec.Site)
+	}
+	return defaultbaseURL

pkg/controller/utils/datadog/metrics_forwarder_test.go

@@ -1029,3 +1029,59 @@ func Test_metricsForwarder_getSecretsFromCache(t *testing.T) {
 		})
 	}
 }
+
+func Test_getbaseURL(t *testing.T) {
+	type args struct {
+		dda *datadoghqv1alpha1.DatadogAgent
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "Get default baseURL",
+			args: args{
+				dda: test.NewDefaultedDatadogAgent("foo", "bar", &test.NewDatadogAgentOptions{}),
+			},
+			want: "https://api.datadoghq.com",
+		},
+		{
+			name: "Compute baseURL from site when passing Site",
+			args: args{
+				dda: test.NewDefaultedDatadogAgent("foo", "bar", &test.NewDatadogAgentOptions{
+					Site: "datadoghq.eu",
+				}),
+			},
+			want: "https://api.datadoghq.eu",
+		},
+		{
+			name: "Compute baseURL from ddUrl when Site is not defined",
+			args: args{
+				dda: test.NewDefaultedDatadogAgent("foo", "bar", &test.NewDatadogAgentOptions{
+					NodeAgentConfig: &datadoghqv1alpha1.NodeAgentConfig{
+						DDUrl: datadoghqv1alpha1.NewStringPointer("https://test.url.com"),
+					}}),
+			},
+			want: "https://test.url.com",
+		},
+		{
+			name: "Test that DDUrl takes precedence over Site",
+			args: args{
+				dda: test.NewDefaultedDatadogAgent("foo", "bar", &test.NewDatadogAgentOptions{
+					Site: "datadoghq.eu",
+					NodeAgentConfig: &datadoghqv1alpha1.NodeAgentConfig{
+						DDUrl: datadoghqv1alpha1.NewStringPointer("https://test.url.com"),
+					}}),
+			},
+			want: "https://test.url.com",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := getbaseURL(tt.args.dda); got != tt.want {
+				t.Errorf("getbaseURL() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}