images/kube-webhook-certgen/rootfs: pass failure policy by value

k8s.k8s.patchWebhookConfigurations() always dereferences it and we do not do a nil check, so the code may panic in some conditions, so it's safer to just pass it by value, as it's just a wrapped string. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
kubernetes · Sep 16, 2021 · d573087 · d573087
1 parent 96d20f1
commit d573087
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 16 deletions.
diff --git a/images/kube-webhook-certgen/rootfs/cmd/patch.go b/images/kube-webhook-certgen/rootfs/cmd/patch.go
@@ -67,7 +67,7 @@ func Patch(ctx context.Context, cfg *PatchConfig) error {
 
 	options := k8s.PatchOptions{
 		CABundle:          ca,
-		FailurePolicyType: &failurePolicy,
+		FailurePolicyType: failurePolicy,
 		APIServiceName:    cfg.APIServiceName,
 	}
 

diff --git a/images/kube-webhook-certgen/rootfs/cmd/patch_test.go b/images/kube-webhook-certgen/rootfs/cmd/patch_test.go
@@ -125,8 +125,8 @@ func Test_Patch(t *testing.T) {
 
 		patcher := testPatcher()
 		patcher.patchObjects = func(_ context.Context, options k8s.PatchOptions) error {
-			if options.FailurePolicyType != nil {
-				return fmt.Errorf("expected policy to be nil. got: %q", *options.FailurePolicyType)
+			if options.FailurePolicyType != "" {
+				return fmt.Errorf("expected policy to be nil. got: %q", options.FailurePolicyType)
 			}
 
 			return nil
@@ -146,8 +146,8 @@ func Test_Patch(t *testing.T) {
 
 		patcher := testPatcher()
 		patcher.patchObjects = func(_ context.Context, options k8s.PatchOptions) error {
-			if options.FailurePolicyType == nil || *options.FailurePolicyType != "Fail" {
-				return fmt.Errorf("unexpected policy: %q", *options.FailurePolicyType)
+			if options.FailurePolicyType == "" || options.FailurePolicyType != "Fail" {
+				return fmt.Errorf("unexpected policy: %q", options.FailurePolicyType)
 			}
 
 			return nil

diff --git a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go
@@ -38,15 +38,15 @@ type PatchOptions struct {
 	MutatingWebhookConfigurationName   string
 	APIServiceName                     string
 	CABundle                           []byte
-	FailurePolicyType                  *admissionv1.FailurePolicyType
+	FailurePolicyType                  admissionv1.FailurePolicyType
 }
 
 func (k8s *k8s) PatchObjects(ctx context.Context, options PatchOptions) error {
 	patchMutating := options.MutatingWebhookConfigurationName != ""
 	patchValidating := options.ValidatingWebhookConfigurationName != ""
 	patchAPIService := options.APIServiceName != ""
 
-	if !patchMutating && !patchValidating && options.FailurePolicyType != nil {
+	if !patchMutating && !patchValidating && options.FailurePolicyType != "" {
 		return fmt.Errorf("failurePolicy specified, but no webhook will be patched")
 	}
 
@@ -105,11 +105,11 @@ func (k8s *k8s) patchWebhookConfigurations(
 	ctx context.Context,
 	configurationName string,
 	ca []byte,
-	failurePolicy *admissionv1.FailurePolicyType,
+	failurePolicy admissionv1.FailurePolicyType,
 	patchMutating bool,
 	patchValidating bool,
 ) error {
-	log.Infof("patching webhook configurations '%s' mutating=%t, validating=%t, failurePolicy=%s", configurationName, patchMutating, patchValidating, *failurePolicy)
+	log.Infof("patching webhook configurations '%s' mutating=%t, validating=%t, failurePolicy=%s", configurationName, patchMutating, patchValidating, failurePolicy)
 
 	if patchValidating {
 		if err := k8s.patchValidating(ctx, configurationName, ca, failurePolicy); err != nil {
@@ -147,7 +147,7 @@ func (err wrappedError) Unwrap() error {
 	return err.err
 }
 
-func (k8s *k8s) patchValidating(ctx context.Context, configurationName string, ca []byte, failurePolicy *admissionv1.FailurePolicyType) error {
+func (k8s *k8s) patchValidating(ctx context.Context, configurationName string, ca []byte, failurePolicy admissionv1.FailurePolicyType) error {
 	valHook, err := k8s.clientset.
 		AdmissionregistrationV1().
 		ValidatingWebhookConfigurations().
@@ -162,8 +162,8 @@ func (k8s *k8s) patchValidating(ctx context.Context, configurationName string, c
 	for i := range valHook.Webhooks {
 		h := &valHook.Webhooks[i]
 		h.ClientConfig.CABundle = ca
-		if *failurePolicy != "" {
-			h.FailurePolicy = failurePolicy
+		if failurePolicy != "" {
+			h.FailurePolicy = &failurePolicy
 		}
 	}
 
@@ -180,7 +180,7 @@ func (k8s *k8s) patchValidating(ctx context.Context, configurationName string, c
 	return nil
 }
 
-func (k8s *k8s) patchMutating(ctx context.Context, configurationName string, ca []byte, failurePolicy *admissionv1.FailurePolicyType) error {
+func (k8s *k8s) patchMutating(ctx context.Context, configurationName string, ca []byte, failurePolicy admissionv1.FailurePolicyType) error {
 	mutHook, err := k8s.clientset.
 		AdmissionregistrationV1().
 		MutatingWebhookConfigurations().
@@ -195,8 +195,8 @@ func (k8s *k8s) patchMutating(ctx context.Context, configurationName string, ca
 	for i := range mutHook.Webhooks {
 		h := &mutHook.Webhooks[i]
 		h.ClientConfig.CABundle = ca
-		if *failurePolicy != "" {
-			h.FailurePolicy = failurePolicy
+		if failurePolicy != "" {
+			h.FailurePolicy = &failurePolicy
 		}
 	}
 

diff --git a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go
@@ -115,7 +115,7 @@ func TestPatchWebhookConfigurations(t *testing.T) {
 			Webhooks: []admissionv1.ValidatingWebhook{{Name: "v1"}, {Name: "v2"}},
 		}, metav1.CreateOptions{})
 
-	if err := k.patchWebhookConfigurations(context.TODO(), testWebhookName, ca, &fail, true, true); err != nil {
+	if err := k.patchWebhookConfigurations(context.TODO(), testWebhookName, ca, fail, true, true); err != nil {
 		t.Fatalf("Unexpected error patching webhooks: %s: %v", err.Error(), errors.Unwrap(err))
 	}