Skip to content

Commit

Permalink
Fix: remove Ownerref on Cluster level resources (#377)
Browse files Browse the repository at this point in the history 
The PR#359 has already remove several ownerref on cluster level resources.

But some other cluster resources still have them. This PR remove every
ownerref on these resources, and refactor how we update ClusterRole, Role,
ClusterRoleBinding and RoleBinding.
  • Loading branch information
@clamoriniere @celenechang
clamoriniere authored and celenechang committed Sep 28, 2021
1 parent 6f3ff28 commit 6aec7a1
Show file tree
Hide file tree
Showing 12 changed files with 234 additions and 194 deletions.
52 changes: 31 additions & 21 deletions controllers/datadogagent/agent_rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,7 @@ func (r *Reconciler) manageAgentRBACs(logger logr.Logger, dda *datadoghqv1alpha1
agentVersion := getAgentVersion(dda)

// Create or update ClusterRole
clusterRole := &rbacv1.ClusterRole{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName}, clusterRole); err != nil {
if errors.IsNotFound(err) {
return r.createAgentClusterRole(logger, dda, rbacResourcesName, agentVersion)
}
return reconcile.Result{}, err
}
if result, err := r.updateIfNeededAgentClusterRole(logger, dda, rbacResourcesName, agentVersion, clusterRole); err != nil {
if result, err := r.manageClusterRole(logger, dda, rbacResourcesName, agentVersion, r.createAgentClusterRole, r.updateIfNeededAgentClusterRole, false); err != nil {
return result, err
}

Expand All @@ -53,32 +46,38 @@ func (r *Reconciler) manageAgentRBACs(logger logr.Logger, dda *datadoghqv1alpha1
}

// Create ClusterRoleBinding
clusterRoleBinding := &rbacv1.ClusterRoleBinding{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName}, clusterRoleBinding); err != nil {
if errors.IsNotFound(err) {
return r.createClusterRoleBinding(logger, dda, roleBindingInfo{
name: rbacResourcesName,
roleName: rbacResourcesName,
serviceAccountName: serviceAccountName,
}, agentVersion)
}
return reconcile.Result{}, err
return r.manageClusterRoleBinding(logger, dda, rbacResourcesName, agentVersion, r.createAgentClusterRoleBinding, r.updateIfNeedAgentClusterRoleBinding, false)
}

func (r *Reconciler) createAgentClusterRoleBinding(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string) (reconcile.Result, error) {
newClusterRoleBinding := buildAgentClusterRoleBinding(dda, name, agentVersion)

return r.createClusterRoleBinding(logger, dda, newClusterRoleBinding)
}

func (r *Reconciler) updateIfNeedAgentClusterRoleBinding(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, clusterRoleBinding *rbacv1.ClusterRoleBinding) (reconcile.Result, error) {
serviceAccountName := getAgentServiceAccount(dda)
bindingInfo := roleBindingInfo{
name: name,
roleName: name,
serviceAccountName: serviceAccountName,
}
newClusterRoleBinding := buildClusterRoleBinding(dda, bindingInfo, agentVersion)

return r.updateIfNeededClusterRoleBinding(logger, dda, rbacResourcesName, rbacResourcesName, serviceAccountName, agentVersion, clusterRoleBinding)
return r.updateIfNeededClusterRoleBindingRaw(logger, dda, clusterRoleBinding, newClusterRoleBinding)
}

// cleanupAgentRbacResources deletes ClusterRole, ClusterRoleBindings, and ServiceAccount of the Agent
func (r *Reconciler) cleanupAgentRbacResources(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent) (reconcile.Result, error) {
rbacResourcesName := getAgentRbacResourcesName(dda)

// Delete ClusterRole
if result, err := r.cleanupClusterRole(logger, r.client, dda, rbacResourcesName); err != nil {
if result, err := r.cleanupClusterRole(logger, dda, rbacResourcesName); err != nil {
return result, err
}

// Delete Cluster Role Binding
if result, err := r.cleanupClusterRoleBinding(logger, r.client, dda, rbacResourcesName); err != nil {
if result, err := r.cleanupClusterRoleBinding(logger, dda, rbacResourcesName); err != nil {
return result, err
}

Expand All @@ -89,3 +88,14 @@ func (r *Reconciler) cleanupAgentRbacResources(logger logr.Logger, dda *datadogh

return reconcile.Result{}, nil
}

func buildAgentClusterRoleBinding(dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string) *rbacv1.ClusterRoleBinding {
serviceAccountName := getAgentServiceAccount(dda)
bindingInfo := roleBindingInfo{
name: name,
roleName: name,
serviceAccountName: serviceAccountName,
}

return buildClusterRoleBinding(dda, bindingInfo, agentVersion)
}
116 changes: 21 additions & 95 deletions controllers/datadogagent/clusteragent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
rbacv1 "k8s.io/api/rbac/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -812,7 +811,7 @@ func (r *Reconciler) manageClusterAgentRBACs(logger logr.Logger, dda *datadoghqv
clusterRoleBinding := &rbacv1.ClusterRoleBinding{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: rbacResourcesName}, clusterRoleBinding); err != nil {
if errors.IsNotFound(err) {
return r.createClusterRoleBinding(logger, dda, roleBindingInfo{
return r.createClusterRoleBindingFromInfo(logger, dda, roleBindingInfo{
name: rbacResourcesName,
roleName: rbacResourcesName,
serviceAccountName: serviceAccountName,
Expand Down Expand Up @@ -850,7 +849,6 @@ func (r *Reconciler) manageClusterAgentRBACs(logger logr.Logger, dda *datadoghqv
return reconcile.Result{}, err
}

clusterAgentSuffix := "cluster-agent"
if isKSMCoreEnabled(dda) && !isKSMCoreClusterCheck(dda) {
if result, err := r.createOrUpdateKubeStateMetricsCoreRBAC(logger, dda, serviceAccountName, clusterAgentVersion, clusterAgentSuffix); err != nil {
return result, err
Expand All @@ -865,56 +863,24 @@ func (r *Reconciler) manageClusterAgentRBACs(logger logr.Logger, dda *datadoghqv
if result, err := r.createOrUpdateOrchestratorCoreRBAC(logger, dda, serviceAccountName, clusterAgentVersion, clusterAgentSuffix); err != nil {
return result, err
}
} else {
if result, err := r.cleanupOrchestratorCoreRBAC(logger, dda, clusterAgentSuffix); err != nil {
return result, err
}
} else if result, err := r.cleanupOrchestratorCoreRBAC(logger, dda, clusterAgentSuffix); err != nil {
return result, err
}

metricsProviderEnabled := isMetricsProviderEnabled(dda.Spec.ClusterAgent)
// Create or delete HPA ClusterRoleBinding
hpaClusterRoleBindingName := getHPAClusterRoleBindingName(dda)
hpaClusterRoleBinding := &rbacv1.ClusterRoleBinding{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: hpaClusterRoleBindingName}, hpaClusterRoleBinding); err != nil {
if errors.IsNotFound(err) {
if metricsProviderEnabled {
return r.createHPAClusterRoleBinding(logger, dda, hpaClusterRoleBindingName, clusterAgentVersion)
}
} else {
return reconcile.Result{}, err
}
} else if !metricsProviderEnabled {
return r.cleanupClusterRoleBinding(logger, r.client, dda, hpaClusterRoleBindingName)
if result, err := r.manageClusterRoleBinding(logger, dda, hpaClusterRoleBindingName, clusterAgentVersion, r.createHPAClusterRoleBinding, r.updateIfNeededHPAClusterRole, metricsProviderEnabled); err != nil {
return result, err
}

// Create or delete external metrics reader ClusterRole and ClusterRoleBinding
metricsReaderClusterRoleName := getExternalMetricsReaderClusterRoleName(dda, r.versionInfo)

metricsReaderClusterRole := &rbacv1.ClusterRole{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: metricsReaderClusterRoleName}, metricsReaderClusterRole); err != nil {
if errors.IsNotFound(err) {
if metricsProviderEnabled {
return r.createExternalMetricsReaderClusterRole(logger, dda, metricsReaderClusterRoleName, clusterAgentVersion)
}
} else {
return reconcile.Result{}, err
}
} else if !metricsProviderEnabled {
return r.cleanupClusterRole(logger, r.client, dda, metricsReaderClusterRoleName)
if result, err := r.manageClusterRole(logger, dda, metricsReaderClusterRoleName, clusterAgentVersion, r.createExternalMetricsReaderClusterRole, r.updateIfNeededExternalMetricsReaderClusterRole, metricsProviderEnabled); err != nil {
return result, err
}

metricsReaderClusterRoleBinding := &rbacv1.ClusterRoleBinding{}
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: metricsReaderClusterRoleName}, metricsReaderClusterRoleBinding); err != nil {
if errors.IsNotFound(err) {
if metricsProviderEnabled {
return r.createExternalMetricsReaderClusterRoleBinding(logger, dda, metricsReaderClusterRoleName, clusterAgentVersion)
}
} else {
return reconcile.Result{}, err
}
} else if !metricsProviderEnabled {
return r.cleanupClusterRoleBinding(logger, r.client, dda, metricsReaderClusterRoleName)
} else if result, err := r.updateIfNeededClusterAgentRoleBinding(logger, dda, clusterAgentVersion, roleBinding); err != nil {
if result, err := r.manageClusterRoleBinding(logger, dda, metricsReaderClusterRoleName, clusterAgentVersion, r.createExternalMetricsReaderClusterRoleBinding, r.updateIfNeededClusterAgentClusterRoleBinding, metricsProviderEnabled); err != nil {
return result, err
}

Expand Down Expand Up @@ -960,79 +926,47 @@ func (r *Reconciler) createClusterCheckRunnerClusterRole(logger logr.Logger, dda

func (r *Reconciler) updateIfNeededClusterAgentClusterRole(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, clusterRole *rbacv1.ClusterRole) (reconcile.Result, error) {
newClusterRole := buildClusterAgentClusterRole(dda, name, agentVersion)
if !apiequality.Semantic.DeepEqual(newClusterRole.Rules, clusterRole.Rules) {
logger.V(1).Info("updateClusterAgentClusterRole", "clusterRole.name", clusterRole.Name)
if err := r.client.Update(context.TODO(), newClusterRole); err != nil {
return reconcile.Result{}, err
}
event := buildEventInfo(newClusterRole.Name, newClusterRole.Namespace, clusterRoleKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
}
return reconcile.Result{}, nil
return r.updateIfNeededClusterRole(logger, dda, clusterRole, newClusterRole)
}

func (r *Reconciler) updateIfNeededClusterAgentRole(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, role *rbacv1.Role) (reconcile.Result, error) {
newRole := buildClusterAgentRole(dda, name, agentVersion)
if !apiequality.Semantic.DeepEqual(newRole.Rules, role.Rules) {
logger.V(1).Info("updateClusterAgentRole", "role.name", newRole.Name)
if err := r.client.Update(context.TODO(), newRole); err != nil {
return reconcile.Result{}, err
}
event := buildEventInfo(newRole.Name, newRole.Namespace, roleKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
}
return reconcile.Result{}, nil
return r.updateIfNeededRole(logger, dda, role, newRole)
}

func (r *Reconciler) updateIfNeededAgentClusterRole(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, clusterRole *rbacv1.ClusterRole) (reconcile.Result, error) {
newClusterRole := buildAgentClusterRole(dda, name, agentVersion)
if !apiequality.Semantic.DeepEqual(newClusterRole.Rules, clusterRole.Rules) {
logger.V(1).Info("updateAgentClusterRole", "clusterRole.name", clusterRole.Name)
if err := r.client.Update(context.TODO(), newClusterRole); err != nil {
return reconcile.Result{}, err
}
event := buildEventInfo(newClusterRole.Name, newClusterRole.Namespace, clusterRoleKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
}
return reconcile.Result{}, nil
return r.updateIfNeededClusterRole(logger, dda, clusterRole, newClusterRole)
}

func (r *Reconciler) updateIfNeededClusterCheckRunnerClusterRole(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, clusterRole *rbacv1.ClusterRole) (reconcile.Result, error) {
newClusterRole := buildClusterCheckRunnerClusterRole(dda, name, agentVersion)
if !apiequality.Semantic.DeepEqual(newClusterRole.Rules, clusterRole.Rules) {
logger.V(1).Info("updateAgentClusterRole", "clusterRole.name", clusterRole.Name)
if err := r.client.Update(context.TODO(), newClusterRole); err != nil {
return reconcile.Result{}, err
}
event := buildEventInfo(newClusterRole.Name, newClusterRole.Namespace, clusterRoleKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
}
return reconcile.Result{}, nil
return r.updateIfNeededClusterRole(logger, dda, clusterRole, newClusterRole)
}

// cleanupClusterAgentRbacResources deletes ClusterRole, ClusterRoleBindings, and ServiceAccount of the Cluster Agent
func (r *Reconciler) cleanupClusterAgentRbacResources(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent) (reconcile.Result, error) {
rbacResourcesName := getClusterAgentRbacResourcesName(dda)
// Delete ClusterRole
if result, err := r.cleanupClusterRole(logger, r.client, dda, rbacResourcesName); err != nil {
if result, err := r.cleanupClusterRole(logger, dda, rbacResourcesName); err != nil {
return result, err
}
// Delete Cluster Role Binding
if result, err := r.cleanupClusterRoleBinding(logger, r.client, dda, rbacResourcesName); err != nil {
if result, err := r.cleanupClusterRoleBinding(logger, dda, rbacResourcesName); err != nil {
return result, err
}
// Delete HPA Cluster Role Binding
hpaClusterRoleBindingName := getHPAClusterRoleBindingName(dda)
if result, err := r.cleanupClusterRoleBinding(logger, r.client, dda, hpaClusterRoleBindingName); err != nil {
if result, err := r.cleanupClusterRoleBinding(logger, dda, hpaClusterRoleBindingName); err != nil {
return result, err
}

externalMetricsReaderName := getExternalMetricsReaderClusterRoleName(dda, r.versionInfo)
if result, err := r.cleanupClusterRoleBinding(logger, r.client, dda, externalMetricsReaderName); err != nil {
if result, err := r.cleanupClusterRoleBinding(logger, dda, externalMetricsReaderName); err != nil {
return result, err
}

if result, err := r.cleanupClusterRole(logger, r.client, dda, externalMetricsReaderName); err != nil {
if result, err := r.cleanupClusterRole(logger, dda, externalMetricsReaderName); err != nil {
return result, err
}

Expand All @@ -1054,22 +988,14 @@ func (r *Reconciler) createClusterAgentRoleBinding(logger logr.Logger, dda *data
return reconcile.Result{}, r.client.Create(context.TODO(), roleBinding)
}

func (r *Reconciler) updateIfNeededClusterAgentRoleBinding(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, agentVersion string, roleBinding *rbacv1.RoleBinding) (reconcile.Result, error) {
func (r *Reconciler) updateIfNeededClusterAgentClusterRoleBinding(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, clusterRoleBinding *rbacv1.ClusterRoleBinding) (reconcile.Result, error) {
info := roleBindingInfo{
name: getClusterAgentRbacResourcesName(dda),
name: name,
roleName: getClusterAgentRbacResourcesName(dda),
serviceAccountName: getClusterAgentServiceAccount(dda),
}
newRoleBinding := buildRoleBinding(dda, info, agentVersion)
if !apiequality.Semantic.DeepEqual(newRoleBinding.RoleRef, roleBinding.RoleRef) || !apiequality.Semantic.DeepEqual(newRoleBinding.Subjects, roleBinding.Subjects) {
logger.V(1).Info("updateClusterAgentClusterRoleBinding", "roleBinding.name", newRoleBinding.Name, "roleBinding.namespace", newRoleBinding.Namespace, "serviceAccount", info.serviceAccountName)
event := buildEventInfo(newRoleBinding.Name, newRoleBinding.Namespace, roleBindingKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
if err := r.client.Update(context.TODO(), newRoleBinding); err != nil {
return reconcile.Result{}, err
}
}
return reconcile.Result{}, nil
newRoleBinding := buildClusterRoleBinding(dda, info, agentVersion)
return r.updateIfNeededClusterRoleBindingRaw(logger, dda, clusterRoleBinding, newRoleBinding)
}

// buildAgentClusterRole creates a ClusterRole object for the Agent based on its config
Expand Down
60 changes: 29 additions & 31 deletions controllers/datadogagent/clusteragent_metricsserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,39 @@ import (

// buildMetricsServerClusterRoleBinding creates a ClusterRoleBinding for the Cluster Agent HPA metrics server
func buildMetricsServerClusterRoleBinding(dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string) *rbacv1.ClusterRoleBinding {
if isMetricsProviderEnabled(dda.Spec.ClusterAgent) {
return &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Labels: getDefaultLabels(dda, name, agentVersion),
Name: name,
},
RoleRef: rbacv1.RoleRef{
APIGroup: datadoghqv1alpha1.RbacAPIGroup,
Kind: datadoghqv1alpha1.ClusterRoleKind,
Name: "system:auth-delegator",
},
Subjects: []rbacv1.Subject{
{
Kind: datadoghqv1alpha1.ServiceAccountKind,
Name: getClusterAgentServiceAccount(dda),
Namespace: dda.Namespace,
},
return &rbacv1.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Labels: getDefaultLabels(dda, name, agentVersion),
Name: name,
},
RoleRef: rbacv1.RoleRef{
APIGroup: datadoghqv1alpha1.RbacAPIGroup,
Kind: datadoghqv1alpha1.ClusterRoleKind,
Name: "system:auth-delegator",
},
Subjects: []rbacv1.Subject{
{
Kind: datadoghqv1alpha1.ServiceAccountKind,
Name: getClusterAgentServiceAccount(dda),
Namespace: dda.Namespace,
},
}
},
}
return nil
}

func (r *Reconciler) createHPAClusterRoleBinding(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string) (reconcile.Result, error) {
clusterRoleBinding := buildMetricsServerClusterRoleBinding(dda, name, agentVersion)
if clusterRoleBinding == nil {
return reconcile.Result{}, nil
}
if err := SetOwnerReference(dda, clusterRoleBinding, r.scheme); err != nil {
return reconcile.Result{}, err
}
logger.V(1).Info("createClusterAgentHPARoleBinding", "clusterRoleBinding.name", clusterRoleBinding.Name)
event := buildEventInfo(clusterRoleBinding.Name, clusterRoleBinding.Namespace, clusterRoleBindingKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
return reconcile.Result{Requeue: true}, r.client.Create(context.TODO(), clusterRoleBinding)
}

func (r *Reconciler) updateIfNeededHPAClusterRole(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, clusterRoleBinding *rbacv1.ClusterRoleBinding) (reconcile.Result, error) {
newClusterRoleBinding := buildMetricsServerClusterRoleBinding(dda, name, agentVersion)
return r.updateIfNeededClusterRoleBindingRaw(logger, dda, clusterRoleBinding, newClusterRoleBinding)
}

// buildExternalMetricsReaderClusterRoleBinding creates a ClusterRoleBinding for the HPA controller to be able to read external metrics
func buildExternalMetricsReaderClusterRoleBinding(dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string) *rbacv1.ClusterRoleBinding {
if isMetricsProviderEnabled(dda.Spec.ClusterAgent) {
Expand Down Expand Up @@ -85,9 +81,6 @@ func (r *Reconciler) createExternalMetricsReaderClusterRoleBinding(logger logr.L
if clusterRoleBinding == nil {
return reconcile.Result{}, nil
}
if err := SetOwnerReference(dda, clusterRoleBinding, r.scheme); err != nil {
return reconcile.Result{}, err
}
logger.V(1).Info("createExternalMetricsClusterRoleBinding", "clusterRoleBinding.name", clusterRoleBinding.Name)
event := buildEventInfo(clusterRoleBinding.Name, clusterRoleBinding.Namespace, clusterRoleBindingKind, datadog.UpdateEvent)
r.recordEvent(dda, event)
Expand All @@ -99,15 +92,20 @@ func (r *Reconciler) createExternalMetricsReaderClusterRole(logger logr.Logger,
if clusterRole == nil {
return reconcile.Result{}, nil
}
if err := SetOwnerReference(dda, clusterRole, r.scheme); err != nil {
return reconcile.Result{}, err
}
logger.V(1).Info("createExternalMetricsClusterRole", "clusterRole.name", clusterRole.Name)
event := buildEventInfo(clusterRole.Name, clusterRole.Namespace, clusterRoleKind, datadog.CreationEvent)
r.recordEvent(dda, event)
return reconcile.Result{Requeue: true}, r.client.Create(context.TODO(), clusterRole)
}

func (r *Reconciler) updateIfNeededExternalMetricsReaderClusterRole(logger logr.Logger, dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string, clusterRole *rbacv1.ClusterRole) (reconcile.Result, error) {
newClusterRole := buildExternalMetricsReaderClusterRole(dda, name, agentVersion)
if newClusterRole == nil {
return reconcile.Result{}, nil
}
return r.updateIfNeededClusterRole(logger, dda, clusterRole, newClusterRole)
}

// buildExternalMetricsReaderClusterRole creates a ClusterRole object for access to external metrics resources
func buildExternalMetricsReaderClusterRole(dda *datadoghqv1alpha1.DatadogAgent, name, agentVersion string) *rbacv1.ClusterRole {
if isMetricsProviderEnabled(dda.Spec.ClusterAgent) {
Expand Down

0 comments on commit 6aec7a1

Please sign in to comment.