From f8d5376251e5de56dc17d2b58aeca24f719332d7 Mon Sep 17 00:00:00 2001 From: Yuriy Losev Date: Thu, 21 Dec 2023 22:09:03 +0400 Subject: [PATCH] Set default failure policy (config) (#572) Set Default FailurePolicy for admission webhooks --- pkg/app/webhook.go | 4 +- pkg/hook/config/config_v1.go | 5 +- pkg/shell-operator/bootstrap.go | 2 - pkg/webhook/admission/manager.go | 6 +- pkg/webhook/admission/manager_test.go | 94 ++++++++------------------- 5 files changed, 34 insertions(+), 77 deletions(-) diff --git a/pkg/app/webhook.go b/pkg/app/webhook.go index 406be470..f961213b 100644 --- a/pkg/app/webhook.go +++ b/pkg/app/webhook.go @@ -59,9 +59,9 @@ func DefineValidatingWebhookFlags(cmd *kingpin.CmdClause) { cmd.Flag("validating-webhook-client-ca", "A path to a server certificate for ValidatingWebhookConfiguration. Can be set with $VALIDATING_WEBHOOK_CLIENT_CA."). Envar("VALIDATING_WEBHOOK_CLIENT_CA"). StringsVar(&ValidatingWebhookSettings.ClientCAPaths) - cmd.Flag("validating-failure-policy", "Defines default FailurePolicy for ValidatingWebhookConfiguration."). + cmd.Flag("validating-webhook-failure-policy", "Defines default FailurePolicy for ValidatingWebhookConfiguration."). Default("Fail"). - Envar("VALIDATING_FAILURE_POLICY"). + Envar("VALIDATING_WEBHOOK_FAILURE_POLICY"). EnumVar(&ValidatingWebhookSettings.DefaultFailurePolicy, "Fail", "Ignore") } diff --git a/pkg/hook/config/config_v1.go b/pkg/hook/config/config_v1.go index b180e614..16307950 100644 --- a/pkg/hook/config/config_v1.go +++ b/pkg/hook/config/config_v1.go @@ -11,6 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/flant/shell-operator/pkg/app" . "github.com/flant/shell-operator/pkg/hook/types" "github.com/flant/shell-operator/pkg/kube_events_manager" . "github.com/flant/shell-operator/pkg/kube_events_manager/types" @@ -429,7 +430,6 @@ func convertValidating(cfgV1 KubernetesAdmissionConfigV1) (ValidatingConfig, err cfg.IncludeSnapshotsFrom = cfgV1.IncludeSnapshotsFrom cfg.BindingName = cfgV1.Name - DefaultFailurePolicy := v1.Fail DefaultSideEffects := v1.SideEffectClassNone DefaultTimeoutSeconds := int32(10) @@ -446,7 +446,8 @@ func convertValidating(cfgV1 KubernetesAdmissionConfigV1) (ValidatingConfig, err if cfgV1.FailurePolicy != nil { webhook.FailurePolicy = cfgV1.FailurePolicy } else { - webhook.FailurePolicy = &DefaultFailurePolicy + defaultFailurePolicy := v1.FailurePolicyType(app.ValidatingWebhookSettings.DefaultFailurePolicy) + webhook.FailurePolicy = &defaultFailurePolicy } if cfgV1.SideEffects != nil { webhook.SideEffects = cfgV1.SideEffects diff --git a/pkg/shell-operator/bootstrap.go b/pkg/shell-operator/bootstrap.go index 24ba382b..c6d456ec 100644 --- a/pkg/shell-operator/bootstrap.go +++ b/pkg/shell-operator/bootstrap.go @@ -5,7 +5,6 @@ import ( "fmt" log "github.com/sirupsen/logrus" - v1 "k8s.io/api/admissionregistration/v1" "github.com/flant/shell-operator/pkg/app" "github.com/flant/shell-operator/pkg/config" @@ -174,7 +173,6 @@ func (op *ShellOperator) setupHookManagers(hooksDir string, tempDir string) { op.AdmissionWebhookManager = admission.NewWebhookManager(op.KubeClient) op.AdmissionWebhookManager.Settings = app.ValidatingWebhookSettings op.AdmissionWebhookManager.Namespace = app.Namespace - op.AdmissionWebhookManager.DefaultFailurePolicy = v1.FailurePolicyType(app.ValidatingWebhookSettings.DefaultFailurePolicy) // Initialize conversion webhooks manager. op.ConversionWebhookManager = conversion.NewWebhookManager() diff --git a/pkg/webhook/admission/manager.go b/pkg/webhook/admission/manager.go index f9d02eb5..9f85725c 100644 --- a/pkg/webhook/admission/manager.go +++ b/pkg/webhook/admission/manager.go @@ -4,7 +4,6 @@ import ( "os" log "github.com/sirupsen/logrus" - v1 "k8s.io/api/admissionregistration/v1" klient "github.com/flant/kube-client/client" "github.com/flant/shell-operator/pkg/webhook/server" @@ -27,7 +26,6 @@ type WebhookManager struct { Namespace string DefaultConfigurationId string - DefaultFailurePolicy v1.FailurePolicyType Server *server.WebhookServer ValidatingResources map[string]*ValidatingWebhookResource @@ -96,9 +94,7 @@ func (m *WebhookManager) AddValidatingWebhook(config *ValidatingWebhookConfig) { ) m.ValidatingResources[confId] = r } - if config.FailurePolicy == nil { - config.FailurePolicy = &m.DefaultFailurePolicy - } + r.Set(config) } diff --git a/pkg/webhook/admission/manager_test.go b/pkg/webhook/admission/manager_test.go index 4bf5496b..782910bc 100644 --- a/pkg/webhook/admission/manager_test.go +++ b/pkg/webhook/admission/manager_test.go @@ -17,82 +17,44 @@ func Test_Manager_AddWebhook(t *testing.T) { vs.ServerCertPath = "testdata/demo-certs/server.crt" vs.CAPath = "testdata/demo-certs/ca.pem" m.Settings = vs - m.DefaultFailurePolicy = v1.Ignore err := m.Init() if err != nil { t.Fatalf("WebhookManager should init: %v", err) } - t.Run("Webhook with set FailurePolicy", func(t *testing.T) { - fail := v1.Fail - none := v1.SideEffectClassNone - timeoutSeconds := int32(10) - - cfg := &ValidatingWebhookConfig{ - ValidatingWebhook: &v1.ValidatingWebhook{ - Name: "test-validating", - Rules: []v1.RuleWithOperations{ - { - Operations: []v1.OperationType{v1.OperationAll}, - Rule: v1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }, - }, - FailurePolicy: &fail, - SideEffects: &none, - TimeoutSeconds: &timeoutSeconds, - }, - } - m.AddValidatingWebhook(cfg) - - if len(m.ValidatingResources) != 1 { - t.Fatalf("WebhookManager should have resources: got length %d", len(m.ValidatingResources)) - } - - for k, v := range m.ValidatingResources { - if len(v.hooks) != 1 { - t.Fatalf("Resource '%s' should have Webhooks: got length %d", k, len(m.ValidatingResources)) - } - assert.Equal(t, v1.Fail, *v.hooks[""].FailurePolicy) - } - }) - - t.Run("Webhook with default FailurePolicy", func(t *testing.T) { - none := v1.SideEffectClassNone - timeoutSeconds := int32(10) - - cfg := &ValidatingWebhookConfig{ - ValidatingWebhook: &v1.ValidatingWebhook{ - Name: "test-validating", - Rules: []v1.RuleWithOperations{ - { - Operations: []v1.OperationType{v1.OperationAll}, - Rule: v1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, + fail := v1.Fail + none := v1.SideEffectClassNone + timeoutSeconds := int32(10) + + cfg := &ValidatingWebhookConfig{ + ValidatingWebhook: &v1.ValidatingWebhook{ + Name: "test-validating", + Rules: []v1.RuleWithOperations{ + { + Operations: []v1.OperationType{v1.OperationAll}, + Rule: v1.Rule{ + APIGroups: []string{"apps"}, + APIVersions: []string{"v1"}, + Resources: []string{"deployments"}, }, }, - SideEffects: &none, - TimeoutSeconds: &timeoutSeconds, }, - } - m.AddValidatingWebhook(cfg) + FailurePolicy: &fail, + SideEffects: &none, + TimeoutSeconds: &timeoutSeconds, + }, + } + m.AddValidatingWebhook(cfg) - if len(m.ValidatingResources) != 1 { - t.Fatalf("WebhookManager should have resources: got length %d", len(m.ValidatingResources)) - } + if len(m.ValidatingResources) != 1 { + t.Fatalf("WebhookManager should have resources: got length %d", len(m.ValidatingResources)) + } - for k, v := range m.ValidatingResources { - if len(v.hooks) != 1 { - t.Fatalf("Resource '%s' should have Webhooks: got length %d", k, len(m.ValidatingResources)) - } - assert.Equal(t, v1.Ignore, *v.hooks[""].FailurePolicy) + for k, v := range m.ValidatingResources { + if len(v.hooks) != 1 { + t.Fatalf("Resource '%s' should have Webhooks: got length %d", k, len(m.ValidatingResources)) } - }) + assert.Equal(t, v1.Fail, *v.hooks[""].FailurePolicy) + } }