From 01a533050435322dba0f0e4d7c3b85f0743ba60d Mon Sep 17 00:00:00 2001 From: Ahmed Mezghani Date: Thu, 24 Mar 2022 15:22:13 +0100 Subject: [PATCH] [DCA][Admission Controller] Use ReinvocationPolicy + make it configurable --- .../admission/controllers/webhook/config.go | 23 +++++----- .../controllers/webhook/controller_v1.go | 15 +++++++ .../controllers/webhook/controller_v1_test.go | 43 +++++++++++++++++++ .../controllers/webhook/controller_v1beta1.go | 15 +++++++ .../webhook/controller_v1beta1_test.go | 43 +++++++++++++++++++ pkg/config/config.go | 1 + pkg/config/config_template.yaml | 7 +++ .../adm-reinvocation-e1435102b435f1d1.yaml | 5 +++ 8 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 releasenotes-dca/notes/adm-reinvocation-e1435102b435f1d1.yaml diff --git a/pkg/clusteragent/admission/controllers/webhook/config.go b/pkg/clusteragent/admission/controllers/webhook/config.go index 8ea435625817d..221645b5d0e26 100644 --- a/pkg/clusteragent/admission/controllers/webhook/config.go +++ b/pkg/clusteragent/admission/controllers/webhook/config.go @@ -28,6 +28,7 @@ type Config struct { svcPort int32 timeout int32 failurePolicy string + reinvocationPolicy string } // NewConfig creates a webhook controller configuration @@ -42,19 +43,21 @@ func NewConfig(admissionV1Enabled, namespaceSelectorEnabled bool) Config { svcPort: int32(443), timeout: config.Datadog.GetInt32("admission_controller.timeout_seconds"), failurePolicy: config.Datadog.GetString("admission_controller.failure_policy"), + reinvocationPolicy: config.Datadog.GetString("admission_controller.reinvocation_policy"), } } -func (w *Config) getWebhookName() string { return w.webhookName } -func (w *Config) getSecretName() string { return w.secretName } -func (w *Config) getSecretNs() string { return w.namespace } -func (w *Config) useAdmissionV1() bool { return w.admissionV1Enabled } -func (w *Config) useNamespaceSelector() bool { return w.namespaceSelectorEnabled } -func (w *Config) getServiceNs() string { return w.namespace } -func (w *Config) getServiceName() string { return w.svcName } -func (w *Config) getServicePort() int32 { return w.svcPort } -func (w *Config) getTimeout() int32 { return w.timeout } -func (w *Config) getFailurePolicy() string { return w.failurePolicy } +func (w *Config) getWebhookName() string { return w.webhookName } +func (w *Config) getSecretName() string { return w.secretName } +func (w *Config) getSecretNs() string { return w.namespace } +func (w *Config) useAdmissionV1() bool { return w.admissionV1Enabled } +func (w *Config) useNamespaceSelector() bool { return w.namespaceSelectorEnabled } +func (w *Config) getServiceNs() string { return w.namespace } +func (w *Config) getServiceName() string { return w.svcName } +func (w *Config) getServicePort() int32 { return w.svcPort } +func (w *Config) getTimeout() int32 { return w.timeout } +func (w *Config) getFailurePolicy() string { return w.failurePolicy } +func (w *Config) getReinvocationPolicy() string { return w.reinvocationPolicy } func (w *Config) configName(suffix string) string { return strings.ReplaceAll(fmt.Sprintf("%s.%s", w.webhookName, suffix), "-", ".") } diff --git a/pkg/clusteragent/admission/controllers/webhook/controller_v1.go b/pkg/clusteragent/admission/controllers/webhook/controller_v1.go index 3a8f389c92cc8..7dfecb1a04193 100644 --- a/pkg/clusteragent/admission/controllers/webhook/controller_v1.go +++ b/pkg/clusteragent/admission/controllers/webhook/controller_v1.go @@ -204,6 +204,7 @@ func (c *ControllerV1) getWebhookSkeleton(nameSuffix, path string) admiv1.Mutati port := c.config.getServicePort() timeout := c.config.getTimeout() failurePolicy := c.getAdmiV1FailurePolicy() + reinvocationPolicy := c.getReinvocationPolicy() webhook := admiv1.MutatingWebhook{ Name: c.config.configName(nameSuffix), ClientConfig: admiv1.WebhookClientConfig{ @@ -226,6 +227,7 @@ func (c *ControllerV1) getWebhookSkeleton(nameSuffix, path string) admiv1.Mutati }, }, }, + ReinvocationPolicy: &reinvocationPolicy, FailurePolicy: &failurePolicy, MatchPolicy: &matchPolicy, SideEffects: &sideEffects, @@ -256,3 +258,16 @@ func (c *ControllerV1) getAdmiV1FailurePolicy() admiv1.FailurePolicyType { return admiv1.Ignore } } + +func (c *ControllerV1) getReinvocationPolicy() admiv1.ReinvocationPolicyType { + policy := strings.ToLower(c.config.getReinvocationPolicy()) + switch policy { + case "ifneeded": + return admiv1.IfNeededReinvocationPolicy + case "never": + return admiv1.NeverReinvocationPolicy + default: + log.Warnf("Unknown reinvocation policy %q - defaulting to %q", policy, admiv1.IfNeededReinvocationPolicy) + return admiv1.IfNeededReinvocationPolicy + } +} diff --git a/pkg/clusteragent/admission/controllers/webhook/controller_v1_test.go b/pkg/clusteragent/admission/controllers/webhook/controller_v1_test.go index 0fd55ec0bacd6..a22382cd50a48 100644 --- a/pkg/clusteragent/admission/controllers/webhook/controller_v1_test.go +++ b/pkg/clusteragent/admission/controllers/webhook/controller_v1_test.go @@ -174,6 +174,7 @@ func TestAdmissionControllerFailureModeFail(t *testing.T) { func TestGenerateTemplatesV1(t *testing.T) { mockConfig := config.Mock() + defaultReinvocationPolicy := admiv1.IfNeededReinvocationPolicy failurePolicy := admiv1.Ignore matchPolicy := admiv1.Exact sideEffects := admiv1.SideEffectClassNone @@ -202,6 +203,7 @@ func TestGenerateTemplatesV1(t *testing.T) { }, }, }, + ReinvocationPolicy: &defaultReinvocationPolicy, FailurePolicy: &failurePolicy, MatchPolicy: &matchPolicy, SideEffects: &sideEffects, @@ -382,6 +384,7 @@ func TestGenerateTemplatesV1(t *testing.T) { } func TestGetWebhookSkeletonV1(t *testing.T) { + defaultReinvocationPolicy := admiv1.IfNeededReinvocationPolicy failurePolicy := admiv1.Ignore matchPolicy := admiv1.Exact sideEffects := admiv1.SideEffectClassNone @@ -412,6 +415,7 @@ func TestGetWebhookSkeletonV1(t *testing.T) { }, }, }, + ReinvocationPolicy: &defaultReinvocationPolicy, FailurePolicy: &failurePolicy, MatchPolicy: &matchPolicy, SideEffects: &sideEffects, @@ -528,3 +532,42 @@ func validateV1(w *admiv1.MutatingWebhookConfiguration, s *corev1.Secret) error return nil } + +func TestAdmissionControllerReinvocationPolicyV1(t *testing.T) { + f := newFixtureV1(t) + c := f.run(t) + c.config = NewConfig(true, false) + + defaultValue := config.Datadog.Get("admission_controller.reinvocation_policy") + defer config.Datadog.Set("admission_controller.reinvocation_policy", defaultValue) + + config.Datadog.Set("admission_controller.reinvocation_policy", "IfNeeded") + c.config = NewConfig(true, false) + webhook := c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "ifneeded") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "Never") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1.NeverReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "never") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1.NeverReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "wrong") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) +} diff --git a/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1.go b/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1.go index ac5701b5fb68b..2f40d507985aa 100644 --- a/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1.go +++ b/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1.go @@ -204,6 +204,7 @@ func (c *ControllerV1beta1) getWebhookSkeleton(nameSuffix, path string) admiv1be port := c.config.getServicePort() timeout := c.config.getTimeout() failurePolicy := c.getAdmiV1Beta1FailurePolicy() + reinvocationPolicy := c.getReinvocationPolicy() webhook := admiv1beta1.MutatingWebhook{ Name: c.config.configName(nameSuffix), ClientConfig: admiv1beta1.WebhookClientConfig{ @@ -226,6 +227,7 @@ func (c *ControllerV1beta1) getWebhookSkeleton(nameSuffix, path string) admiv1be }, }, }, + ReinvocationPolicy: &reinvocationPolicy, FailurePolicy: &failurePolicy, MatchPolicy: &matchPolicy, SideEffects: &sideEffects, @@ -256,3 +258,16 @@ func (c *ControllerV1beta1) getAdmiV1Beta1FailurePolicy() admiv1beta1.FailurePol return admiv1beta1.Ignore } } + +func (c *ControllerV1beta1) getReinvocationPolicy() admiv1beta1.ReinvocationPolicyType { + policy := strings.ToLower(c.config.getReinvocationPolicy()) + switch policy { + case "ifneeded": + return admiv1beta1.IfNeededReinvocationPolicy + case "never": + return admiv1beta1.NeverReinvocationPolicy + default: + log.Warnf("Unknown reinvocation policy %q - defaulting to %q", policy, admiv1beta1.IfNeededReinvocationPolicy) + return admiv1beta1.IfNeededReinvocationPolicy + } +} diff --git a/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1_test.go b/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1_test.go index 510f9accf01f8..46ae45fd96597 100644 --- a/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1_test.go +++ b/pkg/clusteragent/admission/controllers/webhook/controller_v1beta1_test.go @@ -174,6 +174,7 @@ func TestAdmissionControllerFailureModeFailV1beta1(t *testing.T) { func TestGenerateTemplatesV1beta1(t *testing.T) { mockConfig := config.Mock() + defaultReinvocationPolicy := admiv1beta1.IfNeededReinvocationPolicy failurePolicy := admiv1beta1.Ignore matchPolicy := admiv1beta1.Exact sideEffects := admiv1beta1.SideEffectClassNone @@ -202,6 +203,7 @@ func TestGenerateTemplatesV1beta1(t *testing.T) { }, }, }, + ReinvocationPolicy: &defaultReinvocationPolicy, FailurePolicy: &failurePolicy, MatchPolicy: &matchPolicy, SideEffects: &sideEffects, @@ -385,6 +387,7 @@ func TestGetWebhookSkeletonV1beta1(t *testing.T) { failurePolicy := admiv1beta1.Ignore matchPolicy := admiv1beta1.Exact sideEffects := admiv1beta1.SideEffectClassNone + defaultReinvocationPolicy := admiv1beta1.IfNeededReinvocationPolicy port := int32(443) path := "/bar" defaultTimeout := config.Datadog.GetInt32("admission_controller.timeout_seconds") @@ -412,6 +415,7 @@ func TestGetWebhookSkeletonV1beta1(t *testing.T) { }, }, }, + ReinvocationPolicy: &defaultReinvocationPolicy, FailurePolicy: &failurePolicy, MatchPolicy: &matchPolicy, SideEffects: &sideEffects, @@ -526,3 +530,42 @@ func validateV1beta1(w *admiv1beta1.MutatingWebhookConfiguration, s *corev1.Secr } return nil } + +func TestAdmissionControllerReinvocationPolicyV1beta1(t *testing.T) { + f := newFixtureV1beta1(t) + c := f.run(t) + c.config = NewConfig(true, false) + + defaultValue := config.Datadog.Get("admission_controller.reinvocation_policy") + defer config.Datadog.Set("admission_controller.reinvocation_policy", defaultValue) + + config.Datadog.Set("admission_controller.reinvocation_policy", "IfNeeded") + c.config = NewConfig(true, false) + webhook := c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1beta1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "ifneeded") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1beta1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "Never") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1beta1.NeverReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "never") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1beta1.NeverReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "wrong") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1beta1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) + + config.Datadog.Set("admission_controller.reinvocation_policy", "") + c.config = NewConfig(true, false) + webhook = c.getWebhookSkeleton("foo", "/bar") + assert.Equal(t, admiv1beta1.IfNeededReinvocationPolicy, *webhook.ReinvocationPolicy) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 1cba71c6cea15..120184121f95d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -909,6 +909,7 @@ func InitConfig(config Config) { config.BindEnvAndSetDefault("admission_controller.pod_owners_cache_validity", 10) // in minutes config.BindEnvAndSetDefault("admission_controller.namespace_selector_fallback", false) config.BindEnvAndSetDefault("admission_controller.failure_policy", "Ignore") + config.BindEnvAndSetDefault("admission_controller.reinvocation_policy", "IfNeeded") // Telemetry // Enable telemetry metrics on the internals of the Agent. diff --git a/pkg/config/config_template.yaml b/pkg/config/config_template.yaml index cca54ae8c3af7..3febacf02d38f 100644 --- a/pkg/config/config_template.yaml +++ b/pkg/config/config_template.yaml @@ -2441,6 +2441,13 @@ api_key: ## Setting to Fail will require the admission controller to be present and pods to be injected before they are allowed to run. # # failure_policy: Ignore + + ## @param reinvocation_policy - string - optional - default: IfNeeded + ## @env DD_ADMISSION_CONTROLLER_REINVOCATION_POLICY - string - optional - default: IfNeeded + ## Set the reinvocation policy for dynamic admission control. + ## See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy + # + # reinvocation_policy: IfNeeded {{ end -}} {{- if .DockerTagging }} diff --git a/releasenotes-dca/notes/adm-reinvocation-e1435102b435f1d1.yaml b/releasenotes-dca/notes/adm-reinvocation-e1435102b435f1d1.yaml new file mode 100644 index 0000000000000..56c954eb3415f --- /dev/null +++ b/releasenotes-dca/notes/adm-reinvocation-e1435102b435f1d1.yaml @@ -0,0 +1,5 @@ +--- +enhancements: + - | + The admission controller's reinvocation policy is now set to ``IfNeeded`` by default. + It can be changed using the ``admission_controller.reinvocation_policy`` parameter.