Add ServiceAccountAnnotations override (#1425)

* feat(rbac): support adding SA annotations * chore(rbac): update TODO comment for Rules * feat(dda-spec): support v2 custom SA annotations * feat(features): add SA annotations as feature * docs: applying SA annotations with overrides * fix vars * use lowercase and add test case * fix test --------- Co-authored-by: Fanny Jiang <[email protected]>
DataDog · Oct 10, 2024 · 79d2319 · 79d2319
1 parent 5b810a7
commit 79d2319
Show file tree

Hide file tree

Showing 10 changed files with 364 additions and 7 deletions.
diff --git a/api/datadoghq/v2alpha1/datadogagent_types.go b/api/datadoghq/v2alpha1/datadogagent_types.go
@@ -1418,6 +1418,10 @@ type DatadogAgentComponentOverride struct {
 	// +optional
 	ServiceAccountName *string `json:"serviceAccountName,omitempty"`
 
+	// Sets the ServiceAccountAnnotations used by this component.
+	// +optional
+	ServiceAccountAnnotations map[string]string `json:"serviceAccountAnnotations,omitempty"`
+
 	// The container image of the different components (Datadog Agent, Cluster Agent, Cluster Check Runner).
 	// +optional
 	Image *AgentImageConfig `json:"image,omitempty"`

diff --git a/api/datadoghq/v2alpha1/utils.go b/api/datadoghq/v2alpha1/utils.go
@@ -53,6 +53,33 @@ func GetClusterChecksRunnerServiceAccount(dda *DatadogAgent) string {
 	return saDefault
 }
 
+// GetClusterAgentServiceAccountAnnotations returns the annotations for the cluster-agent service account.
+func GetClusterAgentServiceAccountAnnotations(dda *DatadogAgent) map[string]string {
+	defaultAnnotations := map[string]string{}
+	if dda.Spec.Override[ClusterAgentComponentName] != nil && dda.Spec.Override[ClusterAgentComponentName].ServiceAccountAnnotations != nil {
+		return dda.Spec.Override[ClusterAgentComponentName].ServiceAccountAnnotations
+	}
+	return defaultAnnotations
+}
+
+// GetAgentServiceAccountAnnotations returns the annotations for the agent service account.
+func GetAgentServiceAccountAnnotations(dda *DatadogAgent) map[string]string {
+	defaultAnnotations := map[string]string{}
+	if dda.Spec.Override[NodeAgentComponentName] != nil && dda.Spec.Override[NodeAgentComponentName].ServiceAccountAnnotations != nil {
+		return dda.Spec.Override[NodeAgentComponentName].ServiceAccountAnnotations
+	}
+	return defaultAnnotations
+}
+
+// GetClusterChecksRunnerServiceAccountAnnotations returns the annotations for the cluster-checks-runner service account.
+func GetClusterChecksRunnerServiceAccountAnnotations(dda *DatadogAgent) map[string]string {
+	defaultAnnotations := map[string]string{}
+	if dda.Spec.Override[ClusterChecksRunnerComponentName] != nil && dda.Spec.Override[ClusterChecksRunnerComponentName].ServiceAccountAnnotations != nil {
+		return dda.Spec.Override[ClusterChecksRunnerComponentName].ServiceAccountAnnotations
+	}
+	return defaultAnnotations
+}
+
 // IsHostNetworkEnabled returns whether the pod should use the host's network namespace
 func IsHostNetworkEnabled(dda *DatadogAgent, component ComponentName) bool {
 	if dda.Spec.Override != nil {

diff --git a/api/datadoghq/v2alpha1/utils_test.go b/api/datadoghq/v2alpha1/utils_test.go
@@ -108,7 +108,7 @@ func Test_GetImage(t *testing.T) {
 	}
 }
 
-func TestServiceAccountOverride(t *testing.T) {
+func TestServiceAccountNameOverride(t *testing.T) {
 	customServiceAccount := "fake"
 	ddaName := "test-dda"
 	tests := []struct {
@@ -154,3 +154,122 @@ func TestServiceAccountOverride(t *testing.T) {
 		})
 	}
 }
+
+func TestServiceAccountAnnotationOverride(t *testing.T) {
+	customServiceAccount := "fake"
+	customServiceAccountAnnotations := map[string]string{
+		"eks.amazonaws.com/role-arn": "arn:aws:iam::123456789012:role/datadog-role",
+		"really.important":           "annotation",
+	}
+	ddaName := "test-dda"
+	tests := []struct {
+		name string
+		dda  *DatadogAgent
+		want map[ComponentName]map[string]interface{}
+	}{
+		{
+			name: "custom serviceaccount annotations for dda, dca and clc",
+			dda: &DatadogAgent{
+				ObjectMeta: v1.ObjectMeta{
+					Name: ddaName,
+				},
+				Spec: DatadogAgentSpec{
+					Override: map[ComponentName]*DatadogAgentComponentOverride{
+						ClusterAgentComponentName: {
+							ServiceAccountName:        &customServiceAccount,
+							ServiceAccountAnnotations: customServiceAccountAnnotations,
+						},
+						ClusterChecksRunnerComponentName: {
+							ServiceAccountAnnotations: customServiceAccountAnnotations,
+						},
+						NodeAgentComponentName: {
+							ServiceAccountAnnotations: customServiceAccountAnnotations,
+						},
+					},
+				},
+			},
+			want: map[ComponentName]map[string]interface{}{
+				ClusterAgentComponentName: {
+					"name":        customServiceAccount,
+					"annotations": customServiceAccountAnnotations,
+				},
+				NodeAgentComponentName: {
+					"name":        fmt.Sprintf("%s-%s", ddaName, DefaultAgentResourceSuffix),
+					"annotations": customServiceAccountAnnotations,
+				},
+				ClusterChecksRunnerComponentName: {
+					"name":        fmt.Sprintf("%s-%s", ddaName, DefaultClusterChecksRunnerResourceSuffix),
+					"annotations": customServiceAccountAnnotations,
+				},
+			},
+		},
+		{
+			name: "custom serviceaccount annotations for dca",
+			dda: &DatadogAgent{
+				ObjectMeta: v1.ObjectMeta{
+					Name: ddaName,
+				},
+				Spec: DatadogAgentSpec{
+					Override: map[ComponentName]*DatadogAgentComponentOverride{
+						ClusterAgentComponentName: {
+							ServiceAccountName:        &customServiceAccount,
+							ServiceAccountAnnotations: customServiceAccountAnnotations,
+						},
+					},
+				},
+			},
+			want: map[ComponentName]map[string]interface{}{
+				NodeAgentComponentName: {
+					"name":        fmt.Sprintf("%s-%s", ddaName, DefaultAgentResourceSuffix),
+					"annotations": map[string]string{},
+				},
+				ClusterAgentComponentName: {
+					"name":        customServiceAccount,
+					"annotations": customServiceAccountAnnotations,
+				},
+				ClusterChecksRunnerComponentName: {
+					"name":        fmt.Sprintf("%s-%s", ddaName, DefaultClusterChecksRunnerResourceSuffix),
+					"annotations": map[string]string{},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			res := map[ComponentName]map[string]interface{}{
+				NodeAgentComponentName: {
+					"name":        GetAgentServiceAccount(tt.dda),
+					"annotations": GetAgentServiceAccountAnnotations(tt.dda),
+				},
+				ClusterChecksRunnerComponentName: {
+					"name":        GetClusterChecksRunnerServiceAccount(tt.dda),
+					"annotations": GetClusterChecksRunnerServiceAccountAnnotations(tt.dda),
+				},
+				ClusterAgentComponentName: {
+					"name":        GetClusterAgentServiceAccount(tt.dda),
+					"annotations": GetClusterAgentServiceAccountAnnotations(tt.dda),
+				},
+			}
+			for componentName, sa := range tt.want {
+				if res[componentName]["name"] != sa["name"] {
+					t.Errorf("Service Account Override Name error = %v, want %v", res[componentName], tt.want[componentName])
+				}
+				if !mapsEqual(res[componentName]["annotations"].(map[string]string), sa["annotations"].(map[string]string)) {
+					t.Errorf("Service Account Override Annotation error = %v, want %v", res[componentName], tt.want[componentName])
+				}
+			}
+		})
+	}
+}
+
+func mapsEqual(a, b map[string]string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for key, value := range a {
+		if bValue, ok := b[key]; !ok || value != bValue {
+			return false
+		}
+	}
+	return true
+}
diff --git a/api/datadoghq/v2alpha1/zz_generated.deepcopy.go b/api/datadoghq/v2alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/v1/datadoghq.com_datadogagents.yaml b/config/crd/bases/v1/datadoghq.com_datadogagents.yaml
@@ -4259,6 +4259,11 @@ spec:
                                 type: string
                             type: object
                         type: object
+                      serviceAccountAnnotations:
+                        additionalProperties:
+                          type: string
+                        description: Sets the ServiceAccountAnnotations used by this component.
+                        type: object
                       serviceAccountName:
                         description: |-
                           Sets the ServiceAccount used by this component.

diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml
@@ -2,7 +2,7 @@ resources:
 - manager.yaml
 images:
 - name: controller
-  newName: gcr.io/datadoghq/operator
-  newTag: 1.7.0
+  newName: fannyatdd/operator
+  newTag: saa
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
diff --git a/docs/configuration.v2alpha1.md b/docs/configuration.v2alpha1.md
@@ -387,6 +387,7 @@ In the table, `spec.override.nodeAgent.image.name` and `spec.override.nodeAgent.
 | [key].securityContext.windowsOptions.gmsaCredentialSpecName | GMSACredentialSpecName is the name of the GMSA credential spec to use. |
 | [key].securityContext.windowsOptions.hostProcess | HostProcess determines if a container should be run as a 'Host Process' container. All of a Pod's containers must have the same effective HostProcess value (it is not allowed to have a mix of HostProcess containers and non-HostProcess containers). In addition, if HostProcess is true then HostNetwork must also be set to true. |
 | [key].securityContext.windowsOptions.runAsUserName | The UserName in Windows to run the entrypoint of the container process. Defaults to the user specified in image metadata if unspecified. May also be set in PodSecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence. |
+| [key].serviceAccountAnnotations `map[string]string` | Sets the ServiceAccountAnnotations used by this component. |
 | [key].serviceAccountName | Sets the ServiceAccount used by this component. Ignored if the field CreateRbac is true. |
 | [key].tolerations `[]object` | Configure the component tolerations. |
 | [key].updateStrategy.rollingUpdate.maxSurge | MaxSurge behaves differently based on the Kubernetes resource. Refer to the Kubernetes API documentation for additional details. |

diff --git a/internal/controller/datadogagent/feature/enabledefault/feature.go b/internal/controller/datadogagent/feature/enabledefault/feature.go
@@ -95,15 +95,18 @@ type secretInfo struct {
 }
 
 type clusterAgentConfig struct {
-	serviceAccountName string
+	serviceAccountName        string
+	serviceAccountAnnotations map[string]string
 }
 
 type agentConfig struct {
-	serviceAccountName string
+	serviceAccountName        string
+	serviceAccountAnnotations map[string]string
 }
 
 type clusterChecksRunnerConfig struct {
-	serviceAccountName string
+	serviceAccountName        string
+	serviceAccountAnnotations map[string]string
 }
 
 // ID returns the ID of the Feature
@@ -119,6 +122,10 @@ func (f *defaultFeature) Configure(dda *v2alpha1.DatadogAgent) feature.RequiredC
 	f.agent.serviceAccountName = v2alpha1.GetAgentServiceAccount(dda)
 	f.clusterChecksRunner.serviceAccountName = v2alpha1.GetClusterChecksRunnerServiceAccount(dda)
 
+	f.clusterAgent.serviceAccountAnnotations = v2alpha1.GetClusterAgentServiceAccountAnnotations(dda)
+	f.agent.serviceAccountAnnotations = v2alpha1.GetAgentServiceAccountAnnotations(dda)
+	f.clusterChecksRunner.serviceAccountAnnotations = v2alpha1.GetClusterChecksRunnerServiceAccountAnnotations(dda)
+
 	if dda.Spec.Global != nil {
 		if dda.Spec.Global.DisableNonResourceRules != nil && *dda.Spec.Global.DisableNonResourceRules {
 			f.disableNonResourceRules = true
@@ -273,6 +280,13 @@ func (f *defaultFeature) agentDependencies(managers feature.ResourceManagers, re
 		}
 	}
 
+	// serviceAccountAnnotations
+	if f.agent.serviceAccountAnnotations != nil {
+		if err := managers.RBACManager().AddServiceAccountAnnotationsByComponent(f.owner.GetNamespace(), f.agent.serviceAccountName, f.agent.serviceAccountAnnotations, string(v2alpha1.NodeAgentComponentName)); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
 	// ClusterRole creation
 	if err := managers.RBACManager().AddClusterPolicyRules(f.owner.GetNamespace(), componentagent.GetAgentRoleName(f.owner), f.agent.serviceAccountName, getDefaultAgentClusterRolePolicyRules(f.disableNonResourceRules)); err != nil {
 		errs = append(errs, err)
@@ -313,6 +327,13 @@ func (f *defaultFeature) clusterAgentDependencies(managers feature.ResourceManag
 		}
 	}
 
+	// serviceAccountAnnotations
+	if f.agent.serviceAccountAnnotations != nil {
+		if err := managers.RBACManager().AddServiceAccountAnnotationsByComponent(f.owner.GetNamespace(), f.clusterAgent.serviceAccountName, f.clusterAgent.serviceAccountAnnotations, string(v2alpha1.ClusterAgentComponentName)); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
 	dcaService := componentdca.GetClusterAgentService(f.owner)
 	if err := managers.Store().AddOrUpdate(kubernetes.ServicesKind, dcaService); err != nil {
 		return err
@@ -336,6 +357,13 @@ func (f *defaultFeature) clusterChecksRunnerDependencies(managers feature.Resour
 		}
 	}
 
+	// serviceAccountAnnotations
+	if f.agent.serviceAccountAnnotations != nil {
+		if err := managers.RBACManager().AddServiceAccountAnnotationsByComponent(f.owner.GetNamespace(), f.clusterChecksRunner.serviceAccountName, f.clusterChecksRunner.serviceAccountAnnotations, string(v2alpha1.ClusterChecksRunnerComponentName)); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
 	return errors.NewAggregate(errs)
 }
 

diff --git a/internal/controller/datadogagent/merger/rbac.go b/internal/controller/datadogagent/merger/rbac.go
@@ -21,6 +21,8 @@ import (
 type RBACManager interface {
 	AddServiceAccount(namespace string, name string) error
 	AddServiceAccountByComponent(namespace, name, component string) error
+	AddServiceAccountAnnotations(namespace string, name string, annotations map[string]string) error
+	AddServiceAccountAnnotationsByComponent(namespace string, name string, annotations map[string]string, component string) error
 	AddPolicyRules(namespace string, roleName string, saName string, policies []rbacv1.PolicyRule, saNamespace ...string) error
 	AddPolicyRulesByComponent(namespace string, roleName string, saName string, policies []rbacv1.PolicyRule, component string) error
 	AddRoleBinding(roleNamespace, roleName, saNamespace, saName string, roleRef rbacv1.RoleRef) error
@@ -86,6 +88,28 @@ func (m *rbacManagerImpl) DeleteServiceAccountByComponent(component, namespace s
 	return errors.NewAggregate(errs)
 }
 
+// AddServiceAccountAnnotations updates the annotations for an existing ServiceAccount.
+func (m *rbacManagerImpl) AddServiceAccountAnnotations(namespace, saName string, annotations map[string]string) error {
+	obj, _ := m.store.Get(kubernetes.ServiceAccountsKind, namespace, saName)
+	sa, ok := obj.(*corev1.ServiceAccount)
+	if !ok {
+		return fmt.Errorf("unable to get from the store the ServiceAccount %s/%s", namespace, saName)
+	}
+	if sa.Annotations == nil {
+		sa.Annotations = make(map[string]string)
+	}
+	for key, value := range annotations {
+		sa.Annotations[key] = value
+	}
+	return m.store.AddOrUpdate(kubernetes.ServiceAccountsKind, sa)
+}
+
+// AddServiceAccountAnnotationsByComponent updates the annotations for a ServiceAccount and associates it with a component.
+func (m *rbacManagerImpl) AddServiceAccountAnnotationsByComponent(namespace, saName string, annotations map[string]string, component string) error {
+	m.serviceAccountByComponent[component] = append(m.serviceAccountByComponent[component], saName)
+	return m.AddServiceAccountAnnotations(namespace, saName, annotations)
+}
+
 // AddPolicyRules is used to add PolicyRules to a Role. It also creates the RoleBinding.
 func (m *rbacManagerImpl) AddPolicyRules(namespace string, roleName string, saName string, policies []rbacv1.PolicyRule, saNamespace ...string) error {
 	obj, _ := m.store.GetOrCreate(kubernetes.RolesKind, namespace, roleName)
@@ -94,7 +118,7 @@ func (m *rbacManagerImpl) AddPolicyRules(namespace string, roleName string, saNa
 		return fmt.Errorf("unable to get from the store the ClusterRole %s", roleName)
 	}
 
-	// TODO: can be improve by checking if the policies don't already existe.
+	// TODO: can be improve by checking if the policies don't already exist.
 	role.Rules = append(role.Rules, policies...)
 	if err := m.store.AddOrUpdate(kubernetes.RolesKind, role); err != nil {
 		return err