Skip to content

Commit

Permalink
[controllers/datadogagent] Fix watch of ClusterRoles and ClusterRoleB…
Browse files Browse the repository at this point in the history
…indings (#369)

* controllers/datadogagent: add PartOfLabelValue to handle part-of labels

* controllers/datadogagent: watch ClusterRoles and ClusterRoleBindings
  • Loading branch information
davidor authored and celenechang committed Sep 28, 2021
1 parent e06da64 commit 434db7c
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 9 deletions.
2 changes: 1 addition & 1 deletion controllers/datadogagent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func buildAgentNetworkPolicy(dda *datadoghqv1alpha1.DatadogAgent, name string) *
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
kubernetes.AppKubernetesInstanceLabelKey: datadoghqv1alpha1.DefaultAgentResourceSuffix,
kubernetes.AppKubernetesPartOfLabelKey: dda.Namespace + "-" + dda.Name,
kubernetes.AppKubernetesPartOfLabelKey: NewPartOfLabelValue(dda).String(),
},
},
Ingress: ingressRules,
Expand Down
6 changes: 3 additions & 3 deletions controllers/datadogagent/clusteragent.go
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ func buildClusterAgentNetworkPolicy(dda *datadoghqv1alpha1.DatadogAgent, name st
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
kubernetes.AppKubernetesInstanceLabelKey: datadoghqv1alpha1.DefaultAgentResourceSuffix,
kubernetes.AppKubernetesPartOfLabelKey: dda.Namespace + "-" + dda.Name,
kubernetes.AppKubernetesPartOfLabelKey: NewPartOfLabelValue(dda).String(),
},
},
},
Expand All @@ -1549,7 +1549,7 @@ func buildClusterAgentNetworkPolicy(dda *datadoghqv1alpha1.DatadogAgent, name st
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
kubernetes.AppKubernetesInstanceLabelKey: datadoghqv1alpha1.DefaultClusterChecksRunnerResourceSuffix,
kubernetes.AppKubernetesPartOfLabelKey: dda.Namespace + "-" + dda.Name,
kubernetes.AppKubernetesPartOfLabelKey: NewPartOfLabelValue(dda).String(),
},
},
},
Expand Down Expand Up @@ -1580,7 +1580,7 @@ func buildClusterAgentNetworkPolicy(dda *datadoghqv1alpha1.DatadogAgent, name st
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
kubernetes.AppKubernetesInstanceLabelKey: datadoghqv1alpha1.DefaultClusterAgentResourceSuffix,
kubernetes.AppKubernetesPartOfLabelKey: dda.Namespace + "-" + dda.Name,
kubernetes.AppKubernetesPartOfLabelKey: NewPartOfLabelValue(dda).String(),
},
},
Ingress: ingressRules,
Expand Down
2 changes: 1 addition & 1 deletion controllers/datadogagent/clusterchecksrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ func buildClusterChecksRunnerNetworkPolicy(dda *datadoghqv1alpha1.DatadogAgent,
PodSelector: metav1.LabelSelector{
MatchLabels: map[string]string{
kubernetes.AppKubernetesInstanceLabelKey: datadoghqv1alpha1.DefaultClusterChecksRunnerResourceSuffix,
kubernetes.AppKubernetesPartOfLabelKey: dda.Namespace + "-" + dda.Name,
kubernetes.AppKubernetesPartOfLabelKey: NewPartOfLabelValue(dda).String(),
},
},
Egress: egressRules,
Expand Down
2 changes: 1 addition & 1 deletion controllers/datadogagent/common_rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (r *Reconciler) updateIfNeededClusterRoleBinding(logger logr.Logger, dda *d
// labels to know whether a DatadogAgent object owns them.
func isOwnerBasedOnLabels(dda *datadoghqv1alpha1.DatadogAgent, labels map[string]string) bool {
isManagedByOperator := labels[kubernetes.AppKubernetesManageByLabelKey] == "datadog-operator"
isPartOfDDA := labels[kubernetes.AppKubernetesPartOfLabelKey] == dda.Namespace+"-"+dda.Name
isPartOfDDA := labels[kubernetes.AppKubernetesPartOfLabelKey] == NewPartOfLabelValue(dda).String()
return isManagedByOperator && isPartOfDDA
}

Expand Down
60 changes: 60 additions & 0 deletions controllers/datadogagent/labels.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package datadogagent

import (
"strings"

datadoghqv1alpha1 "github.com/DataDog/datadog-operator/api/v1alpha1"
"k8s.io/apimachinery/pkg/types"
)

const (
partOfSplitChar string = "-"
partOfEscapedSplitChar string = "--"
)

// PartOfLabelValue is helpful to work with the "app.kubernetes.io/part-of"
// label. We use that label to track the of an object when we can't use owner
// references (cannot be used across namespaces).
// In order to identify an owner, we encode its namespace and name in the value
// of the label separated with a "-". Because names and namespaces can contain
// "-" too, we escape them. There are not any characters that are allowed in
// labels but not in names and namespaces so escaping is needed.
type PartOfLabelValue struct {
Value string
}

// NewPartOfLabelValue creates an instance of PartOfLabelValue from a
// DatadogAgent.
func NewPartOfLabelValue(dda *datadoghqv1alpha1.DatadogAgent) *PartOfLabelValue {
value := strings.ReplaceAll(dda.Namespace, partOfSplitChar, partOfEscapedSplitChar) +
partOfSplitChar +
strings.ReplaceAll(dda.Name, partOfSplitChar, partOfEscapedSplitChar)

return &PartOfLabelValue{Value: value}
}

// NamespacedName returns the NamespaceName that corresponds to the value of a
// part-of label.
func (partOfLabelValue *PartOfLabelValue) NamespacedName() types.NamespacedName {
l := len(partOfLabelValue.Value)
for i, c := range partOfLabelValue.Value {
if string(c) == partOfSplitChar {
// Tt's split index if it's just one "-"
isSplitIndex := (i != 0 && string(partOfLabelValue.Value[i-1]) != partOfSplitChar) &&
(i != l-1 && string(partOfLabelValue.Value[i+1]) != partOfSplitChar)

if isSplitIndex {
return types.NamespacedName{
// The ReplaceAll calls undoes the escaping done in the constructor
Namespace: strings.ReplaceAll(partOfLabelValue.Value[:i], partOfEscapedSplitChar, partOfSplitChar),
Name: strings.ReplaceAll(partOfLabelValue.Value[i+1:], partOfEscapedSplitChar, partOfSplitChar),
}
}
}
}
return types.NamespacedName{}
}

func (partOfLabelValue *PartOfLabelValue) String() string {
return partOfLabelValue.Value
}
46 changes: 46 additions & 0 deletions controllers/datadogagent/labels_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package datadogagent

import (
datadoghqv1alpha1 "github.com/DataDog/datadog-operator/api/v1alpha1"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"testing"
)

func Test_NamespacedName(t *testing.T) {
tests := []struct {
name string
agentName string
agentNamespace string
expectedNamespacedName types.NamespacedName
}{
{
name: "without the split char",
agentNamespace: "foo",
agentName: "bar",
expectedNamespacedName: types.NamespacedName{Namespace: "foo", Name: "bar"},
},
{
name: "with the split char",
agentNamespace: "f-o-o",
agentName: "bar",
expectedNamespacedName: types.NamespacedName{Namespace: "f-o-o", Name: "bar"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dda := datadoghqv1alpha1.DatadogAgent{
ObjectMeta: metav1.ObjectMeta{
Name: tt.agentName,
Namespace: tt.agentNamespace,
},
}

value := NewPartOfLabelValue(&dda)

assert.Equal(t, tt.expectedNamespacedName, value.NamespacedName())
})
}
}
2 changes: 1 addition & 1 deletion controllers/datadogagent/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1912,7 +1912,7 @@ func getDefaultLabels(dda *datadoghqv1alpha1.DatadogAgent, instanceName, version
labels := make(map[string]string)
labels[kubernetes.AppKubernetesNameLabelKey] = "datadog-agent-deployment"
labels[kubernetes.AppKubernetesInstanceLabelKey] = instanceName
labels[kubernetes.AppKubernetesPartOfLabelKey] = dda.Namespace + "-" + dda.Name
labels[kubernetes.AppKubernetesPartOfLabelKey] = NewPartOfLabelValue(dda).String()
labels[kubernetes.AppKubernetesVersionLabelKey] = version
labels[kubernetes.AppKubernetesManageByLabelKey] = "datadog-operator"

Expand Down
26 changes: 24 additions & 2 deletions controllers/datadogagent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@ package controllers
import (
"context"

"github.com/DataDog/datadog-operator/pkg/kubernetes"
"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
ctrlbuilder "sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -161,11 +165,16 @@ func (r *DatadogAgentReconciler) SetupWithManager(mgr ctrl.Manager) error {
Owns(&rbacv1.Role{}).
Owns(&rbacv1.RoleBinding{}).
Owns(&corev1.ServiceAccount{}).
Owns(&rbacv1.ClusterRole{}).
Owns(&rbacv1.ClusterRoleBinding{}).
Owns(&policyv1.PodDisruptionBudget{}).
Owns(&networkingv1.NetworkPolicy{})

// DatadogAgent is namespaced whereas ClusterRole and ClusterRoleBinding are
// cluster-scoped. That means that DatadogAgent cannot be their owner, and
// we cannot use .Owns().
handlerEnqueue := handler.EnqueueRequestsFromMapFunc(enqueueIfOwnedByDatadogAgent)
builder.Watches(&source.Kind{Type: &rbacv1.ClusterRole{}}, handlerEnqueue)
builder.Watches(&source.Kind{Type: &rbacv1.ClusterRoleBinding{}}, handlerEnqueue)

if r.Options.SupportExtendedDaemonset {
builder = builder.Owns(&edsdatadoghqv1alpha1.ExtendedDaemonSet{})
}
Expand Down Expand Up @@ -198,3 +207,16 @@ func (r *DatadogAgentReconciler) SetupWithManager(mgr ctrl.Manager) error {

return nil
}

func enqueueIfOwnedByDatadogAgent(obj client.Object) []reconcile.Request {
labels := obj.GetLabels()

if labels[kubernetes.AppKubernetesManageByLabelKey] != "datadog-operator" {
return nil
}

partOfLabelVal := datadogagent.PartOfLabelValue{Value: labels[kubernetes.AppKubernetesPartOfLabelKey]}
owner := partOfLabelVal.NamespacedName()

return []reconcile.Request{{NamespacedName: owner}}
}

0 comments on commit 434db7c

Please sign in to comment.