Skip to content

Commit

Permalink
[clusteragent/admission/webhook] Add selectors needed for AKS (#11367)
Browse files Browse the repository at this point in the history 
This fixes a problem that happens when enabling the DCA admission
controller on Azure AKS.

AKS automatically updates our webhooks adding a selector that excludes
the "control-plane" namespace. This creates conflicts when we reconcile
the webhooks in the controller and spams the logs with messages like:

`Couldn't reconcile Webhook datadog-webhook: Operation cannot be
fulfilled on mutatingwebhookconfigurations.admissionregistration.k8s.io
"datadog-webhook": the object has been modified; please apply your
changes to the latest version and try again`.

References:
- https://docs.microsoft.com/en-us/azure/aks/faq#can-i-use-admission-controller-webhooks-on-aks
- Azure/AKS#1771
  • Loading branch information
@davidor
davidor authored Apr 4, 2022
1 parent 681c509 commit 221c333
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 27 deletions.
56 changes: 50 additions & 6 deletions pkg/clusteragent/admission/controllers/webhook/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// buildLabelSelector returns the mutating webhooks object selector based on the configuration
func buildLabelSelector() *metav1.LabelSelector {
// buildLabelSelectors returns the mutating webhooks object selector based on the configuration
func buildLabelSelectors(useNamespaceSelector bool) (namespaceSelector, objectSelector *metav1.LabelSelector) {
var labelSelector metav1.LabelSelector

if config.Datadog.GetBool("admission_controller.mutate_unlabelled") {
// Accept all, ignore pods if they're explicitly filtered-out
return &metav1.LabelSelector{
labelSelector = metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: common.EnabledLabelKey,
Expand All @@ -28,12 +30,54 @@ func buildLabelSelector() *metav1.LabelSelector {
},
},
}
} else {
// Ignore all, accept pods if they're explicitly allowed
labelSelector = metav1.LabelSelector{
MatchLabels: map[string]string{
common.EnabledLabelKey: "true",
},
}
}

if config.Datadog.GetBool("admission_controller.add_aks_selectors") {
return aksSelectors(useNamespaceSelector, labelSelector)
}

if useNamespaceSelector {
return &labelSelector, nil
}

// Ignore all, accept pods if they're explicitly allowed
return nil, &labelSelector
}

// aksSelectors takes a label selector and builds a namespace and object
// selector adapted for AKS. AKS adds automatically some selector requirements
// if we don't, so we need to add them to avoid conflicts when updating the
// webhook.
//
// Ref: https://docs.microsoft.com/en-us/azure/aks/faq#can-i-use-admission-controller-webhooks-on-aks
// Ref: https://github.com/Azure/AKS/issues/1771
func aksSelectors(useNamespaceSelector bool, labelSelector metav1.LabelSelector) (namespaceSelector, objectSelector *metav1.LabelSelector) {
if useNamespaceSelector {
labelSelector.MatchExpressions = append(
labelSelector.MatchExpressions,
azureAKSLabelSelectorRequirement(),
)
return &labelSelector, nil
}

// Azure AKS adds the namespace selector even in Kubernetes versions that
// support object selectors, so we need to add it to avoid conflicts.
return &metav1.LabelSelector{
MatchLabels: map[string]string{
common.EnabledLabelKey: "true",
MatchExpressions: []metav1.LabelSelectorRequirement{
azureAKSLabelSelectorRequirement(),
},
}, &labelSelector
}

func azureAKSLabelSelectorRequirement() metav1.LabelSelectorRequirement {
return metav1.LabelSelectorRequirement{
Key: "control-plane",
Operator: metav1.LabelSelectorOpDoesNotExist,
}
}
8 changes: 1 addition & 7 deletions pkg/clusteragent/admission/controllers/webhook/controller_v1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,7 @@ func (c *ControllerV1) getWebhookSkeleton(nameSuffix, path string) admiv1.Mutati
AdmissionReviewVersions: []string{"v1", "v1beta1"},
}

labelSelector := buildLabelSelector()
if c.config.useNamespaceSelector() {
webhook.NamespaceSelector = labelSelector
return webhook
}

webhook.ObjectSelector = labelSelector
webhook.NamespaceSelector, webhook.ObjectSelector = buildLabelSelectors(c.config.useNamespaceSelector())

return webhook
}
Expand Down
76 changes: 73 additions & 3 deletions pkg/clusteragent/admission/controllers/webhook/controller_v1_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,78 @@ func TestGenerateTemplatesV1(t *testing.T) {
return []admiv1.MutatingWebhook{webhookConfig, webhookTags}
},
},
{
name: "AKS-specific label selector without namespace selector enabled",
setupConfig: func() {
mockConfig.Set("admission_controller.add_aks_selectors", true)
mockConfig.Set("admission_controller.namespace_selector_fallback", false)
mockConfig.Set("admission_controller.inject_config.enabled", true)
mockConfig.Set("admission_controller.mutate_unlabelled", true)
mockConfig.Set("admission_controller.inject_tags.enabled", false)
},
configFunc: func() Config { return NewConfig(false, false) },
want: func() []admiv1.MutatingWebhook {
webhook := webhook(
"datadog.webhook.config",
"/injectconfig",
&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "admission.datadoghq.com/enabled",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"false"},
},
},
},
&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "control-plane",
Operator: metav1.LabelSelectorOpDoesNotExist,
},
},
},
)
return []admiv1.MutatingWebhook{webhook}
},
},
{
name: "AKS-specific label selector with namespace selector enabled",
setupConfig: func() {
mockConfig.Set("admission_controller.add_aks_selectors", true)
mockConfig.Set("admission_controller.namespace_selector_fallback", true)
mockConfig.Set("admission_controller.inject_config.enabled", true)
mockConfig.Set("admission_controller.mutate_unlabelled", true)
mockConfig.Set("admission_controller.inject_tags.enabled", false)
},
configFunc: func() Config { return NewConfig(false, true) },
want: func() []admiv1.MutatingWebhook {
webhook := webhook(
"datadog.webhook.config",
"/injectconfig",
nil,
&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "admission.datadoghq.com/enabled",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"false"},
},
{
Key: "control-plane",
Operator: metav1.LabelSelectorOpDoesNotExist,
},
},
},
)
return []admiv1.MutatingWebhook{webhook}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.setupConfig()
defer mockConfig.Set("admission_controller.add_aks_selectors", false) // Reset to default

c := &ControllerV1{}
c.config = tt.configFunc()
Expand All @@ -392,6 +460,8 @@ func TestGetWebhookSkeletonV1(t *testing.T) {
path := "/bar"
defaultTimeout := config.Datadog.GetInt32("admission_controller.timeout_seconds")
customTimeout := int32(2)
namespaceSelector, _ := buildLabelSelectors(true)
_, objectSelector := buildLabelSelectors(false)
webhook := func(to *int32, objSelector, nsSelector *metav1.LabelSelector) admiv1.MutatingWebhook {
return admiv1.MutatingWebhook{
Name: "datadog.webhook.foo",
Expand Down Expand Up @@ -443,7 +513,7 @@ func TestGetWebhookSkeletonV1(t *testing.T) {
path: "/bar",
},
namespaceSelector: false,
want: webhook(&defaultTimeout, buildLabelSelector(), nil),
want: webhook(&defaultTimeout, objectSelector, nil),
},
{
name: "namespace selector",
Expand All @@ -452,7 +522,7 @@ func TestGetWebhookSkeletonV1(t *testing.T) {
path: "/bar",
},
namespaceSelector: true,
want: webhook(&defaultTimeout, nil, buildLabelSelector()),
want: webhook(&defaultTimeout, nil, namespaceSelector),
},
{
name: "custom timeout",
Expand All @@ -462,7 +532,7 @@ func TestGetWebhookSkeletonV1(t *testing.T) {
},
timeout: &customTimeout,
namespaceSelector: false,
want: webhook(&customTimeout, buildLabelSelector(), nil),
want: webhook(&customTimeout, objectSelector, nil),
},
}
for _, tt := range tests {
Expand Down
8 changes: 1 addition & 7 deletions pkg/clusteragent/admission/controllers/webhook/controller_v1beta1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,7 @@ func (c *ControllerV1beta1) getWebhookSkeleton(nameSuffix, path string) admiv1be
AdmissionReviewVersions: []string{"v1beta1"},
}

labelSelector := buildLabelSelector()
if c.config.useNamespaceSelector() {
webhook.NamespaceSelector = labelSelector
return webhook
}

webhook.ObjectSelector = labelSelector
webhook.NamespaceSelector, webhook.ObjectSelector = buildLabelSelectors(c.config.useNamespaceSelector())

return webhook
}
Expand Down
76 changes: 73 additions & 3 deletions pkg/clusteragent/admission/controllers/webhook/controller_v1beta1_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,78 @@ func TestGenerateTemplatesV1beta1(t *testing.T) {
return []admiv1beta1.MutatingWebhook{webhookConfig, webhookTags}
},
},
{
name: "AKS-specific label selector without namespace selector enabled",
setupConfig: func() {
mockConfig.Set("admission_controller.add_aks_selectors", true)
mockConfig.Set("admission_controller.namespace_selector_fallback", false)
mockConfig.Set("admission_controller.inject_config.enabled", true)
mockConfig.Set("admission_controller.mutate_unlabelled", true)
mockConfig.Set("admission_controller.inject_tags.enabled", false)
},
configFunc: func() Config { return NewConfig(false, false) },
want: func() []admiv1beta1.MutatingWebhook {
webhook := webhook(
"datadog.webhook.config",
"/injectconfig",
&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "admission.datadoghq.com/enabled",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"false"},
},
},
},
&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "control-plane",
Operator: metav1.LabelSelectorOpDoesNotExist,
},
},
},
)
return []admiv1beta1.MutatingWebhook{webhook}
},
},
{
name: "AKS-specific label selector with namespace selector enabled",
setupConfig: func() {
mockConfig.Set("admission_controller.add_aks_selectors", true)
mockConfig.Set("admission_controller.namespace_selector_fallback", true)
mockConfig.Set("admission_controller.inject_config.enabled", true)
mockConfig.Set("admission_controller.mutate_unlabelled", true)
mockConfig.Set("admission_controller.inject_tags.enabled", false)
},
configFunc: func() Config { return NewConfig(false, true) },
want: func() []admiv1beta1.MutatingWebhook {
webhook := webhook(
"datadog.webhook.config",
"/injectconfig",
nil,
&metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "admission.datadoghq.com/enabled",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"false"},
},
{
Key: "control-plane",
Operator: metav1.LabelSelectorOpDoesNotExist,
},
},
},
)
return []admiv1beta1.MutatingWebhook{webhook}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.setupConfig()
defer mockConfig.Set("admission_controller.add_aks_selectors", false) // Reset to default

c := &ControllerV1beta1{}
c.config = tt.configFunc()
Expand All @@ -392,6 +460,8 @@ func TestGetWebhookSkeletonV1beta1(t *testing.T) {
path := "/bar"
defaultTimeout := config.Datadog.GetInt32("admission_controller.timeout_seconds")
customTimeout := int32(2)
namespaceSelector, _ := buildLabelSelectors(true)
_, objectSelector := buildLabelSelectors(false)
webhook := func(to *int32, objSelector, nsSelector *metav1.LabelSelector) admiv1beta1.MutatingWebhook {
return admiv1beta1.MutatingWebhook{
Name: "datadog.webhook.foo",
Expand Down Expand Up @@ -443,7 +513,7 @@ func TestGetWebhookSkeletonV1beta1(t *testing.T) {
path: "/bar",
},
namespaceSelector: false,
want: webhook(&defaultTimeout, buildLabelSelector(), nil),
want: webhook(&defaultTimeout, objectSelector, nil),
},
{
name: "namespace selector",
Expand All @@ -452,7 +522,7 @@ func TestGetWebhookSkeletonV1beta1(t *testing.T) {
path: "/bar",
},
namespaceSelector: true,
want: webhook(&defaultTimeout, nil, buildLabelSelector()),
want: webhook(&defaultTimeout, nil, namespaceSelector),
},
{
name: "custom timeout",
Expand All @@ -462,7 +532,7 @@ func TestGetWebhookSkeletonV1beta1(t *testing.T) {
},
timeout: &customTimeout,
namespaceSelector: false,
want: webhook(&customTimeout, buildLabelSelector(), nil),
want: webhook(&customTimeout, objectSelector, nil),
},
}
for _, tt := range tests {
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,7 @@ func InitConfig(config Config) {
config.BindEnvAndSetDefault("admission_controller.namespace_selector_fallback", false)
config.BindEnvAndSetDefault("admission_controller.failure_policy", "Ignore")
config.BindEnvAndSetDefault("admission_controller.reinvocation_policy", "IfNeeded")
config.BindEnvAndSetDefault("admission_controller.add_aks_selectors", false) // adds in the webhook some selectors that are required in AKS

// Telemetry
// Enable telemetry metrics on the internals of the Agent.
Expand Down
9 changes: 8 additions & 1 deletion pkg/config/config_template.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2448,6 +2448,13 @@ api_key:
## See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy
#
# reinvocation_policy: IfNeeded

## @param add_aks_selectors - boolean - optional - default: false
## @env DD_ADMISSION_CONTROLLER_ADD_AKS_SELECTORS - boolean - optional - default: false
## Adds in the admission controller webhook the selectors that are required in AKS.
## See https://docs.microsoft.com/en-us/azure/aks/faq#can-i-use-admission-controller-webhooks-on-aks
#
# add_aks_selectors: false
{{ end -}}
{{- if .DockerTagging }}

Expand Down Expand Up @@ -3216,7 +3223,7 @@ api_key:
## - `raw_value` to report the raw value as a Datadog gauge.
#
# cumulative_monotonic_mode: to_delta

## @param summaries - custom object - optional
## Configuration for OTLP Summaries.
## See https://docs.datadoghq.com/metrics/otlp/?tab=summary for more details.
Expand Down
11 changes: 11 additions & 0 deletions releasenotes-dca/notes/fix-dca-admission-webhook-aks-add7d15b0e793bd4.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
fixes:
- |
Fixed an issue that created lots of log messages when the DCA admission controller was enabled on AKS.

0 comments on commit 221c333

Please sign in to comment.