[defaults] Fix cluster agent token autogeneration

apis/datadoghq/v1alpha1/datadogagent_default.go

@@ -112,7 +112,7 @@ func DefaultDatadogAgent(dda *DatadogAgent) *DatadogAgentStatus {
 	}
 
 	// Creds
-	defaultCredentials(&dda.Spec, dso)
+	defaultCredentials(dda, dso)
 
 	// Override spec given featureset
 	FeatureOverride(&dda.Spec, dso.DefaultOverride)
@@ -133,35 +133,68 @@ func DefaultDatadogAgent(dda *DatadogAgent) *DatadogAgentStatus {
 	return dso
 }
 
-func defaultCredentials(ddaSpec *DatadogAgentSpec, dso *DatadogAgentStatus) {
-	if ddaSpec.Credentials == nil {
-		ddaSpec.Credentials = &AgentCredentials{}
+func defaultCredentials(dda *DatadogAgent, dso *DatadogAgentStatus) {
+	if dda.Spec.Credentials == nil {
+		dda.Spec.Credentials = &AgentCredentials{}
 	}
-	if ddaSpec.Credentials.UseSecretBackend == nil {
-		ddaSpec.Credentials.UseSecretBackend = apiutils.NewBoolPointer(false)
+
+	if dda.Spec.Credentials.UseSecretBackend == nil {
+		dda.Spec.Credentials.UseSecretBackend = apiutils.NewBoolPointer(false)
 		if dso.DefaultOverride == nil {
 			dso.DefaultOverride = &DatadogAgentSpec{}
 		}
 		dso.DefaultOverride.Credentials = &AgentCredentials{
-			UseSecretBackend: ddaSpec.Credentials.UseSecretBackend,
+			UseSecretBackend: dda.Spec.Credentials.UseSecretBackend,
 		}
 	}
 
-	// Generate a Token for clusterAgent-Agent communication if not provided
-	if ddaSpec.Credentials.Token == "" {
-		if dso.ClusterAgent == nil {
-			dso.ClusterAgent = &DeploymentStatus{}
-		}
-		if dso.ClusterAgent.GeneratedToken == "" {
-			dso.ClusterAgent.GeneratedToken = apiutils.GenerateRandomString(32)
+	defaultClusterAgentToken(dda, dso)
+}
+
+func defaultClusterAgentToken(dda *DatadogAgent, dso *DatadogAgentStatus) {
+	// Token provided in the spec. No need to generate one.
+	if dda.Spec.Credentials.Token != "" {
+		return
+	}
+
+	if dso.DefaultOverride == nil {
+		dso.DefaultOverride = &DatadogAgentSpec{}
+	}
+
+	if dso.DefaultOverride.Credentials == nil {
+		dso.DefaultOverride.Credentials = &AgentCredentials{}
+	}
+
+	defaultedToken := DefaultedClusterAgentToken(&dda.Status)
+
+	if defaultedToken != "" {
+		dso.DefaultOverride.Credentials.Token = defaultedToken
+	} else {
+		// For backwards-compatibility, if the token is already in the status
+		// use it.
+		if dso.ClusterAgent != nil && dso.ClusterAgent.GeneratedToken != "" {
+			dso.DefaultOverride.Credentials.Token = dso.ClusterAgent.GeneratedToken
+		} else {
+			dso.DefaultOverride.Credentials.Token = apiutils.GenerateRandomString(32)
 		}
-	} else if dso.ClusterAgent != nil {
-		// cleanup the generated token if after an update the user specify one
-		// in the spec.Credentials.Token
-		dso.ClusterAgent.GeneratedToken = ""
 	}
 }
 
+// DefaultedClusterAgentToken returns the autogenerated token used for the
+// communication between the agents and the DCA. If the token has not been
+// autogenerated, this function returns an empty string.
+func DefaultedClusterAgentToken(ddaStatus *DatadogAgentStatus) string {
+	tokenHasBeenDefaulted := ddaStatus.DefaultOverride != nil &&
+		ddaStatus.DefaultOverride.Credentials != nil &&
+		ddaStatus.DefaultOverride.Credentials.Token != ""
+
+	if !tokenHasBeenDefaulted {
+		return ""
+	}
+
+	return ddaStatus.DefaultOverride.Credentials.Token
+}
+
 // FeatureOverride defaults the feature section of the DatadogAgent
 // TODO surface in the status when Overrides are not possible. Security agent requires the System Probe
 func FeatureOverride(dda *DatadogAgentSpec, dso *DatadogAgentSpec) {

apis/datadoghq/v1alpha1/datadogagent_default_test.go

@@ -1001,66 +1001,106 @@ func TestDefaultDatadogAgentSpecAgentApm(t *testing.T) {
 }
 
 func Test_defaultCredentials(t *testing.T) {
-	type args struct {
-		ddaSpec *DatadogAgentSpec
-		dso     *DatadogAgentStatus
-	}
 	tests := []struct {
-		name              string
-		args              args
-		wantGenerateToken bool
+		name                      string
+		tokenInSpec               string
+		defaultedToken            string
+		expectsDefaultedToken     bool
+		expectsSameDefaultedToken bool
 	}{
 		{
-			name: "token_in_spec, should not generate a token",
-			args: args{
-				ddaSpec: &DatadogAgentSpec{
+			name:                  "token in spec",
+			tokenInSpec:           "a_token",
+			defaultedToken:        "",
+			expectsDefaultedToken: false,
+		},
+		{
+			name:                      "no token in spec and not defaulted",
+			tokenInSpec:               "",
+			defaultedToken:            "",
+			expectsDefaultedToken:     true,
+			expectsSameDefaultedToken: false,
+		},
+		{
+			name:                      "no token in spec, but already defaulted",
+			tokenInSpec:               "",
+			defaultedToken:            "a_defaulted_token",
+			expectsDefaultedToken:     true,
+			expectsSameDefaultedToken: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dda := DatadogAgent{
+				Spec: DatadogAgentSpec{
 					Credentials: &AgentCredentials{
-						Token: "foobarfoobar",
+						Token: tt.tokenInSpec,
 					},
 				},
-				dso: &DatadogAgentStatus{
-					ClusterAgent: &DeploymentStatus{},
+				Status: DatadogAgentStatus{
+					DefaultOverride: &DatadogAgentSpec{
+						Credentials: &AgentCredentials{
+							Token: tt.defaultedToken,
+						},
+					},
 				},
+			}
+
+			ddaStatus := DatadogAgentStatus{}
+
+			defaultCredentials(&dda, &ddaStatus)
+
+			if tt.expectsDefaultedToken {
+				assert.NotEmpty(t, ddaStatus.DefaultOverride.Credentials.Token)
+			} else if ddaStatus.DefaultOverride != nil && ddaStatus.DefaultOverride.Credentials != nil {
+				assert.Empty(t, ddaStatus.DefaultOverride.Credentials.Token)
+			}
+
+			if tt.expectsSameDefaultedToken {
+				assert.Equal(t, tt.defaultedToken, ddaStatus.DefaultOverride.Credentials.Token)
+			}
+		})
+	}
+}
+
+func TestDefaultedClusterAgentToken(t *testing.T) {
+	tests := []struct {
+		name          string
+		ddaStatus     *DatadogAgentStatus
+		expectedToken string
+	}{
+		{
+			name: "status without default overrides",
+			ddaStatus: &DatadogAgentStatus{
+				DefaultOverride: nil,
 			},
-			wantGenerateToken: false,
+			expectedToken: "",
 		},
 		{
-			name: "no token in spec, should generate a token",
-			args: args{
-				ddaSpec: &DatadogAgentSpec{
-					Credentials: &AgentCredentials{},
-				},
-				dso: &DatadogAgentStatus{
-					ClusterAgent: &DeploymentStatus{},
+			name: "status with overrides but no overridden credentials",
+			ddaStatus: &DatadogAgentStatus{
+				DefaultOverride: &DatadogAgentSpec{
+					Credentials: nil,
 				},
 			},
-			wantGenerateToken: true,
+			expectedToken: "",
 		},
 		{
-			name: "token in spec + previous token in status: should remove the generated token",
-			args: args{
-				ddaSpec: &DatadogAgentSpec{
+			name: "status with defaulted token",
+			ddaStatus: &DatadogAgentStatus{
+				DefaultOverride: &DatadogAgentSpec{
 					Credentials: &AgentCredentials{
-						Token: "foobarfoobar",
-					},
-				},
-				dso: &DatadogAgentStatus{
-					ClusterAgent: &DeploymentStatus{
-						GeneratedToken: "previous_generated_token",
+						Token: "some_token",
 					},
 				},
 			},
-			wantGenerateToken: false,
+			expectedToken: "some_token",
 		},
 	}
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			defaultCredentials(tt.args.ddaSpec, tt.args.dso)
-			if tt.wantGenerateToken {
-				assert.False(t, tt.args.dso.ClusterAgent.GeneratedToken == "", "the generated token should not be empty, value:%s", tt.args.dso.ClusterAgent.GeneratedToken)
-			} else {
-				assert.False(t, tt.args.dso.ClusterAgent.GeneratedToken != "", "the generated token should be empty, value:%s", tt.args.dso.ClusterAgent.GeneratedToken)
-			}
+			assert.Equal(t, tt.expectedToken, DefaultedClusterAgentToken(tt.ddaStatus))
 		})
 	}
 }

controllers/datadogagent/secret_agent.go

@@ -38,8 +38,11 @@ func newAgentSecret(name string, dda *datadoghqv1alpha1.DatadogAgent) (*corev1.S
 	// Agent credentials has two more fields
 	if creds.Token != "" {
 		data[datadoghqv1alpha1.DefaultTokenKey] = []byte(creds.Token)
-	} else if isClusterAgentEnabled(dda.Spec.ClusterAgent) && dda.Status.ClusterAgent != nil {
-		data[datadoghqv1alpha1.DefaultTokenKey] = []byte(dda.Status.ClusterAgent.GeneratedToken)
+	} else if isClusterAgentEnabled(dda.Spec.ClusterAgent) {
+		defaultedToken := datadoghqv1alpha1.DefaultedClusterAgentToken(&dda.Status)
+		if defaultedToken != "" {
+			data[datadoghqv1alpha1.DefaultTokenKey] = []byte(defaultedToken)
+		}
 	}
 
 	secret := &corev1.Secret{