Set default failure policy (config) (#572)

Set Default FailurePolicy for admission webhooks
flant · Dec 21, 2023 · f8d5376 · f8d5376
1 parent d62bdee
commit f8d5376
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 77 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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)
+	}
 }