From 1d7931b0bb63c5802b84b5c800fdb0c31ecb5684 Mon Sep 17 00:00:00 2001 From: Cedric Lamoriniere Date: Wed, 16 Sep 2020 13:55:19 +0200 Subject: [PATCH] Fix RBAC with compliance and admission controller (#151) * Fix DCA resources creation issues * Fix ClusterRole and Role DCA creation when ExternalMetrics is not enabled * Add missing DCA permission for the AdmissionController Signed-off-by: cedric lamoriniere * Add missing DCA RBAC for orchestrator cluster ID Signed-off-by: cedric lamoriniere * Update example with admission controler and security-agent Signed-off-by: cedric lamoriniere * Sync ClusterRole and Role in Helm chart Signed-off-by: cedric lamoriniere * update after codereview comments --- .../templates/clusterrole.yaml | 45 ++++--- chart/datadog-operator/templates/role.yaml | 21 +++- deploy/clusterrole.yaml | 31 +++-- deploy/role.yaml | 11 +- examples/datadog-agent-all.yaml | 13 ++ pkg/apis/datadoghq/v1alpha1/const.go | 62 +++++----- pkg/controller/datadogagent/clusteragent.go | 115 +++++++++++------- .../datadogagent_controller_test.go | 21 ++-- 8 files changed, 192 insertions(+), 127 deletions(-) diff --git a/chart/datadog-operator/templates/clusterrole.yaml b/chart/datadog-operator/templates/clusterrole.yaml index 8d8444d7e..689f92397 100644 --- a/chart/datadog-operator/templates/clusterrole.yaml +++ b/chart/datadog-operator/templates/clusterrole.yaml @@ -6,38 +6,53 @@ metadata: labels: {{ include "datadog-operator.labels" . | indent 4 }} rules: +- apiGroups: + - "security.openshift.io" + resources: + - securitycontextconstraints + verbs: + - use + resourceNames: + - restricted - apiGroups: - rbac.authorization.k8s.io - roles.rbac.authorization.k8s.io - authorization.k8s.io resources: - - 'clusterroles' - - 'clusterrolebindings' + - clusterroles + - clusterrolebindings + verbs: + - "*" +- apiGroups: + - datadoghq.com + resources: + - datadogagents + - datadogagents/status + - datadogagents/finalizers verbs: - '*' - apiGroups: - admissionregistration.k8s.io - - '' resources: - - 'mutatingwebhookconfigurations' - - 'secrets' + - mutatingwebhookconfigurations + - secrets verbs: - - 'get' - - 'list' - - 'watch' - - 'update' + - get + - list + - watch + - update - create - apiGroups: - apps - batch resources: - - 'replicasets' - - 'deployments' - - 'statefulsets' - - 'jobs' - - 'cronjobs' + - replicasets + - deployments + - statefulsets + - jobs + - cronjobs verbs: - - 'get' + - get - apiGroups: - apiregistration.k8s.io resources: diff --git a/chart/datadog-operator/templates/role.yaml b/chart/datadog-operator/templates/role.yaml index f6985d7e5..a9664baef 100644 --- a/chart/datadog-operator/templates/role.yaml +++ b/chart/datadog-operator/templates/role.yaml @@ -25,13 +25,21 @@ rules: - daemonsets verbs: - '*' +- apiGroups: + - apps + resourceNames: + - datadog-operator + resources: + - deployments/finalizers + verbs: + - update - apiGroups: - rbac.authorization.k8s.io - roles.rbac.authorization.k8s.io - authorization.k8s.io resources: - - 'roles' - - 'rolebindings' + - roles + - rolebindings verbs: - '*' - apiGroups: @@ -43,10 +51,11 @@ rules: - apiGroups: - datadoghq.com resources: - - 'datadogagents' - - 'datadogagents/status' - - 'datadogagents/finalizers' - - 'extendeddaemonsets' + - datadogagents + - datadogagents/status + - datadogagents/finalizers + - extendeddaemonsets + - datadogmetrics verbs: - '*' {{- end -}} diff --git a/deploy/clusterrole.yaml b/deploy/clusterrole.yaml index cf825f0cb..1d1fa0a5f 100644 --- a/deploy/clusterrole.yaml +++ b/deploy/clusterrole.yaml @@ -16,10 +16,10 @@ rules: - roles.rbac.authorization.k8s.io - authorization.k8s.io resources: - - 'clusterroles' - - 'clusterrolebindings' + - clusterroles + - clusterrolebindings verbs: - - '*' + - "*" - apiGroups: - datadoghq.com resources: @@ -30,27 +30,26 @@ rules: - '*' - apiGroups: - admissionregistration.k8s.io - - '' resources: - - 'mutatingwebhookconfigurations' - - 'secrets' + - mutatingwebhookconfigurations + - secrets verbs: - - 'get' - - 'list' - - 'watch' - - 'update' + - get + - list + - watch + - update - create - apiGroups: - apps - batch resources: - - 'replicasets' - - 'deployments' - - 'statefulsets' - - 'jobs' - - 'cronjobs' + - replicasets + - deployments + - statefulsets + - jobs + - cronjobs verbs: - - 'get' + - get - apiGroups: - apiregistration.k8s.io resources: diff --git a/deploy/role.yaml b/deploy/role.yaml index 767aa959f..18f70c21a 100644 --- a/deploy/role.yaml +++ b/deploy/role.yaml @@ -1,7 +1,6 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: - creationTimestamp: null name: datadog-operator rules: - apiGroups: @@ -49,10 +48,10 @@ rules: - apiGroups: - datadoghq.com resources: - - 'datadogagents' - - 'datadogagents/status' - - 'datadogagents/finalizers' - - 'extendeddaemonsets' - - 'datadogmetrics' + - datadogagents + - datadogagents/status + - datadogagents/finalizers + - extendeddaemonsets + - datadogmetrics verbs: - '*' diff --git a/examples/datadog-agent-all.yaml b/examples/datadog-agent-all.yaml index 7b8eca8a8..f2fb72fca 100644 --- a/examples/datadog-agent-all.yaml +++ b/examples/datadog-agent-all.yaml @@ -18,3 +18,16 @@ spec: systemProbe: enabled: true bpfDebugEnabled: true + security: + compliance: + enabled: true + runtime: + enabled: false + clusterAgent: + image: + name: "datadog/cluster-agent:latest" + config: + externalMetrics: + enabled: true + admissionController: + enabled: true diff --git a/pkg/apis/datadoghq/v1alpha1/const.go b/pkg/apis/datadoghq/v1alpha1/const.go index 50867a6c0..6d63f2abb 100644 --- a/pkg/apis/datadoghq/v1alpha1/const.go +++ b/pkg/apis/datadoghq/v1alpha1/const.go @@ -215,42 +215,46 @@ const ( // Resources - ServicesResource = "services" - EventsResource = "events" - EndpointsResource = "endpoints" - PodsResource = "pods" - NodesResource = "nodes" - ComponentStatusesResource = "componentstatuses" - ConfigMapsResource = "configmaps" - ClusterResourceQuotasResource = "clusterresourcequotas" - NodeMetricsResource = "nodes/metrics" - NodeSpecResource = "nodes/spec" - NodeProxyResource = "nodes/proxy" - NodeStats = "nodes/stats" - HorizontalPodAutoscalersRecource = "horizontalpodautoscalers" - DatadogMetricsResource = "datadogmetrics" - DatadogMetricsStatusResource = "datadogmetrics/status" - WpaResource = "watermarkpodautoscalers" - MutatingConfigResource = "mutatingwebhookconfigurations" - SecretsResource = "secrets" - ReplicasetsResource = "replicasets" - DeploymentsResource = "deployments" - StatefulsetsResource = "statefulsets" - JobsResource = "jobs" - CronjobsResource = "cronjobs" - ServiceAccountResource = "serviceaccounts" - NamespaceResource = "namespaces" - PodSecurityPolicyResource = "podsecuritypolicies" - ClusterRoleBindingResource = "clusterrolebindings" - RoleBindingResource = "rolebindings" - NetworkPolicyResource = "networkpolicies" + ServicesResource = "services" + EventsResource = "events" + EndpointsResource = "endpoints" + PodsResource = "pods" + NodesResource = "nodes" + ComponentStatusesResource = "componentstatuses" + ConfigMapsResource = "configmaps" + ClusterResourceQuotasResource = "clusterresourcequotas" + NodeMetricsResource = "nodes/metrics" + NodeSpecResource = "nodes/spec" + NodeProxyResource = "nodes/proxy" + NodeStats = "nodes/stats" + HorizontalPodAutoscalersRecource = "horizontalpodautoscalers" + DatadogMetricsResource = "datadogmetrics" + DatadogMetricsStatusResource = "datadogmetrics/status" + WpaResource = "watermarkpodautoscalers" + MutatingConfigResource = "mutatingwebhookconfigurations" + SecretsResource = "secrets" + ReplicasetsResource = "replicasets" + DeploymentsResource = "deployments" + StatefulsetsResource = "statefulsets" + DaemonsetsResource = "daemonsets" + JobsResource = "jobs" + CronjobsResource = "cronjobs" + ExtendedDaemonSetReplicaSetResource = "extendeddaemonsetreplicasets" + ServiceAccountResource = "serviceaccounts" + NamespaceResource = "namespaces" + PodSecurityPolicyResource = "podsecuritypolicies" + ClusterRoleBindingResource = "clusterrolebindings" + RoleBindingResource = "rolebindings" + NetworkPolicyResource = "networkpolicies" // Resource names DatadogTokenResourceName = "datadogtoken" DatadogLeaderElectionResourceName = "datadog-leader-election" DatadogCustomMetricsResourceName = "datadog-custom-metrics" + DatadogClusterIDResourceName = "datadog-cluster-id" ExtensionAPIServerAuthResourceName = "extension-apiserver-authentication" + KubeSystemResourceName = "kube-system" // Non resource URLs diff --git a/pkg/controller/datadogagent/clusteragent.go b/pkg/controller/datadogagent/clusteragent.go index 5c4fd0436..8e571606e 100644 --- a/pkg/controller/datadogagent/clusteragent.go +++ b/pkg/controller/datadogagent/clusteragent.go @@ -622,6 +622,16 @@ func (r *ReconcileDatadogAgent) manageClusterAgentRBACs(logger logr.Logger, dda rbacResourcesName := getClusterAgentRbacResourcesName(dda) clusterAgentVersion := getClusterAgentVersion(dda) + + // Create ServiceAccount + serviceAccount := &corev1.ServiceAccount{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName, Namespace: dda.Namespace}, serviceAccount); err != nil { + if errors.IsNotFound(err) { + return r.createServiceAccount(logger, dda, rbacResourcesName, clusterAgentVersion) + } + return reconcile.Result{}, err + } + // Create or update ClusterRole clusterRole := &rbacv1.ClusterRole{} if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName}, clusterRole); err != nil { @@ -647,6 +657,32 @@ func (r *ReconcileDatadogAgent) manageClusterAgentRBACs(logger logr.Logger, dda return reconcile.Result{}, err } + // Create or update Role + role := &rbacv1.Role{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName, Namespace: dda.Namespace}, role); err != nil { + if errors.IsNotFound(err) { + return r.createClusterAgentRole(logger, dda, rbacResourcesName, clusterAgentVersion) + } + return reconcile.Result{}, err + } + if result, err := r.updateIfNeededClusterAgentRole(logger, dda, rbacResourcesName, clusterAgentVersion, role); err != nil { + return result, err + } + + // Create or update RoleBinding + roleBinding := &rbacv1.RoleBinding{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName, Namespace: dda.Namespace}, roleBinding); err != nil { + if errors.IsNotFound(err) { + info := roleBindingInfo{ + name: rbacResourcesName, + roleName: rbacResourcesName, + serviceAccountName: getClusterAgentServiceAccount(dda), + } + return r.createClusterAgentRoleBinding(logger, dda, info, clusterAgentVersion) + } + return reconcile.Result{}, err + } + metricsProviderEnabled := isMetricsProviderEnabled(dda.Spec.ClusterAgent) // Create or delete HPA ClusterRoleBinding hpaClusterRoleBindingName := getHPAClusterRoleBindingName(dda) @@ -692,39 +728,6 @@ func (r *ReconcileDatadogAgent) manageClusterAgentRBACs(logger logr.Logger, dda return r.cleanupClusterRoleBinding(logger, r.client, dda, metricsReaderClusterRoleName) } - // Create ServiceAccount - serviceAccount := &corev1.ServiceAccount{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName, Namespace: dda.Namespace}, serviceAccount); err != nil { - if errors.IsNotFound(err) { - return r.createServiceAccount(logger, dda, rbacResourcesName, clusterAgentVersion) - } - return reconcile.Result{}, err - } - - // Create or update Role - role := &rbacv1.Role{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName, Namespace: dda.Namespace}, role); err != nil { - if errors.IsNotFound(err) { - return r.createClusterAgentRole(logger, dda, rbacResourcesName, clusterAgentVersion) - } - return reconcile.Result{}, err - } - if result, err := r.updateIfNeededClusterAgentRole(logger, dda, rbacResourcesName, clusterAgentVersion, role); err != nil { - return result, err - } - // Create or update RoleBinding - roleBinding := &rbacv1.RoleBinding{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName, Namespace: dda.Namespace}, roleBinding); err != nil { - if errors.IsNotFound(err) { - info := roleBindingInfo{ - name: rbacResourcesName, - roleName: rbacResourcesName, - serviceAccountName: getClusterAgentServiceAccount(dda), - } - return r.createClusterAgentRoleBinding(logger, dda, info, clusterAgentVersion) - } - return reconcile.Result{}, err - } if result, err := r.updateIfNeededClusterAgentRoleBinding(logger, dda, clusterAgentVersion, roleBinding); err != nil { return result, err } @@ -993,6 +996,15 @@ func buildClusterAgentClusterRole(dda *datadoghqv1alpha1.DatadogAgent, name, age Verbs: []string{datadoghqv1alpha1.ListVerb, datadoghqv1alpha1.WatchVerb}, }) + rbacRules = append(rbacRules, rbacv1.PolicyRule{ + APIGroups: []string{datadoghqv1alpha1.CoreAPIGroup}, + Resources: []string{datadoghqv1alpha1.NamespaceResource}, + ResourceNames: []string{ + datadoghqv1alpha1.KubeSystemResourceName, + }, + Verbs: []string{datadoghqv1alpha1.GetVerb}, + }) + if dda.Spec.Agent != nil { if datadoghqv1alpha1.BoolValue(dda.Spec.Agent.Config.CollectEvents) { rbacRules = append(rbacRules, getEventCollectionPolicyRule()) @@ -1070,25 +1082,25 @@ func buildClusterAgentClusterRole(dda *datadoghqv1alpha1.DatadogAgent, name, age datadoghqv1alpha1.UpdateVerb}, }) - // Replicasets + // ExtendedDaemonsetReplicaSets rbacRules = append(rbacRules, rbacv1.PolicyRule{ - APIGroups: []string{datadoghqv1alpha1.AppsAPIGroup}, - Resources: []string{datadoghqv1alpha1.ReplicasetsResource}, - Verbs: []string{datadoghqv1alpha1.GetVerb}, - }) - - // Deployments - rbacRules = append(rbacRules, rbacv1.PolicyRule{ - APIGroups: []string{datadoghqv1alpha1.AppsAPIGroup}, - Resources: []string{datadoghqv1alpha1.DeploymentsResource}, - Verbs: []string{datadoghqv1alpha1.GetVerb}, + APIGroups: []string{datadoghqv1alpha1.SchemeGroupVersion.Group}, + Resources: []string{ + datadoghqv1alpha1.ExtendedDaemonSetReplicaSetResource, + }, + Verbs: []string{datadoghqv1alpha1.GetVerb}, }) - // Deployments + // Deployments, Replicasets, Statefulsets, Daemonsets, rbacRules = append(rbacRules, rbacv1.PolicyRule{ APIGroups: []string{datadoghqv1alpha1.AppsAPIGroup}, - Resources: []string{datadoghqv1alpha1.StatefulsetsResource}, - Verbs: []string{datadoghqv1alpha1.GetVerb}, + Resources: []string{ + datadoghqv1alpha1.DeploymentsResource, + datadoghqv1alpha1.ReplicasetsResource, + datadoghqv1alpha1.StatefulsetsResource, + datadoghqv1alpha1.DaemonsetsResource, + }, + Verbs: []string{datadoghqv1alpha1.GetVerb}, }) // Jobs @@ -1164,6 +1176,15 @@ func buildClusterAgentRole(dda *datadoghqv1alpha1.DatadogAgent, name, agentVersi rbacRules := getLeaderElectionPolicyRule() + rbacRules = append(rbacRules, rbacv1.PolicyRule{ + APIGroups: []string{datadoghqv1alpha1.CoreAPIGroup}, + Resources: []string{datadoghqv1alpha1.ConfigMapsResource}, + ResourceNames: []string{ + datadoghqv1alpha1.DatadogClusterIDResourceName, + }, + Verbs: []string{datadoghqv1alpha1.GetVerb, datadoghqv1alpha1.UpdateVerb, datadoghqv1alpha1.CreateVerb}, + }) + if dda.Spec.ClusterAgent.Config.ExternalMetrics != nil && dda.Spec.ClusterAgent.Config.ExternalMetrics.Enabled { rbacRules = append(rbacRules, rbacv1.PolicyRule{ APIGroups: []string{datadoghqv1alpha1.CoreAPIGroup}, diff --git a/pkg/controller/datadogagent/datadogagent_controller_test.go b/pkg/controller/datadogagent/datadogagent_controller_test.go index 968cd331c..a9c110a00 100644 --- a/pkg/controller/datadogagent/datadogagent_controller_test.go +++ b/pkg/controller/datadogagent/datadogagent_controller_test.go @@ -8,18 +8,19 @@ package datadogagent import ( "context" "encoding/base64" - "errors" "reflect" + "time" "fmt" "testing" - "time" + + "github.com/pkg/errors" + assert "github.com/stretchr/testify/require" datadoghqv1alpha1 "github.com/DataDog/datadog-operator/pkg/apis/datadoghq/v1alpha1" test "github.com/DataDog/datadog-operator/pkg/apis/datadoghq/v1alpha1/test" "github.com/DataDog/datadog-operator/pkg/controller/utils/comparison" "github.com/DataDog/datadog-operator/pkg/controller/utils/datadog" - assert "github.com/stretchr/testify/require" edsdatadoghqv1alpha1 "github.com/datadog/extendeddaemonset/pkg/apis/datadoghq/v1alpha1" @@ -29,8 +30,8 @@ import ( policyv1 "k8s.io/api/policy/v1beta1" rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" @@ -1121,6 +1122,7 @@ func TestReconcileDatadogAgent_Reconcile(t *testing.T) { _ = c.Create(context.TODO(), test.NewSecret(resourcesNamespace, "foo", &test.NewSecretOptions{Labels: commonDCAlabels, Data: map[string][]byte{ "token": []byte(base64.StdEncoding.EncodeToString([]byte("token-foo"))), }})) + _ = c.Create(context.TODO(), buildServiceAccount(dda, "foo-cluster-agent", getClusterAgentVersion(dda))) dcaService := test.NewService(resourcesNamespace, "foo-cluster-agent", &test.NewServiceOptions{Spec: &corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, Selector: map[string]string{ @@ -1237,6 +1239,7 @@ func TestReconcileDatadogAgent_Reconcile(t *testing.T) { _ = c.Create(context.TODO(), test.NewSecret(resourcesNamespace, "foo", &test.NewSecretOptions{Labels: commonDCAlabels, Data: map[string][]byte{ "token": []byte(base64.StdEncoding.EncodeToString([]byte("token-foo"))), }})) + _ = c.Create(context.TODO(), buildServiceAccount(dda, "foo-cluster-agent", getClusterAgentVersion(dda))) dcaService := test.NewService(resourcesNamespace, "foo-cluster-agent", &test.NewServiceOptions{Spec: &corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, Selector: map[string]string{ @@ -1335,6 +1338,7 @@ func TestReconcileDatadogAgent_Reconcile(t *testing.T) { _, _ = comparison.SetMD5GenerationAnnotation(&dcaService.ObjectMeta, dcaService.Spec) dcaService.Labels = commonDCAlabels _ = c.Create(context.TODO(), dcaService) + _ = c.Create(context.TODO(), buildServiceAccount(dda, "foo-cluster-agent", getClusterAgentVersion(dda))) _ = c.Create(context.TODO(), buildClusterAgentClusterRole(dda, "foo-cluster-agent", getClusterAgentVersion(dda))) _ = c.Create(context.TODO(), buildClusterAgentPDB(dda)) }, @@ -1412,7 +1416,6 @@ func TestReconcileDatadogAgent_Reconcile(t *testing.T) { _ = c.Create(context.TODO(), dcaExternalMetricsAPIService) _ = c.Create(context.TODO(), buildClusterAgentPDB(dda)) - }, }, want: reconcile.Result{Requeue: true}, @@ -2280,11 +2283,13 @@ func createClusterAgentDependencies(c client.Client, dda *datadoghqv1alpha1.Data _ = c.Create(context.TODO(), buildClusterAgentClusterRole(dda, "foo-cluster-agent", version)) _ = c.Create(context.TODO(), buildClusterAgentRole(dda, "foo-cluster-agent", version)) _ = c.Create(context.TODO(), buildServiceAccount(dda, clusterAgentSAName, version)) - _ = c.Create(context.TODO(), buildClusterRoleBinding(dda, roleBindingInfo{ + info := roleBindingInfo{ name: "foo-cluster-agent", roleName: "foo-cluster-agent", - serviceAccountName: clusterAgentSAName, - }, version)) + serviceAccountName: getClusterAgentServiceAccount(dda), + } + _ = c.Create(context.TODO(), buildClusterRoleBinding(dda, info, version)) + _ = c.Create(context.TODO(), buildRoleBinding(dda, info, version)) _ = c.Create(context.TODO(), buildClusterAgentPDB(dda)) dcaService := test.NewService(resourcesNamespace, "foo-cluster-agent", &test.NewServiceOptions{Spec: &corev1.ServiceSpec{