From ec82e89d964f3a7a10dfe3a68c3f0a6848dab6d5 Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Thu, 2 Apr 2026 11:20:42 -0700 Subject: [PATCH] Installation: add NetworkPolicyManagement toggle - Add NetworkPolicyManagement field to InstallationSpec - Implement transparent filtering in ComponentHandler to skip/delete policies when disabled - Add verification tests in component_test.go and merge_test.go Fixes https://github.com/projectcalico/calico/issues/11935 Fixes https://github.com/tigera/operator/issues/4331 --- api/v1/common_types.go | 10 +++ api/v1/installation_types.go | 6 ++ pkg/controller/utils/component.go | 39 ++++++++++++ pkg/controller/utils/component_test.go | 87 +++++++++++++++++++++++++- pkg/controller/utils/merge.go | 5 ++ pkg/controller/utils/merge_test.go | 17 +++++ 6 files changed, 162 insertions(+), 2 deletions(-) diff --git a/api/v1/common_types.go b/api/v1/common_types.go index d1c7f841de..deb10d7ff2 100644 --- a/api/v1/common_types.go +++ b/api/v1/common_types.go @@ -66,3 +66,13 @@ type NamespacedName struct { Namespace string `json:"namespace"` Name string `json:"name"` } + +// NetworkPolicyManagement specifies whether or not the operator should manage +// NetworkPolicies within the cluster. +// +kubebuilder:validation:Enum=Enabled;Disabled +type NetworkPolicyManagement string + +const ( + NetworkPolicyManagementEnabled NetworkPolicyManagement = "Enabled" + NetworkPolicyManagementDisabled NetworkPolicyManagement = "Disabled" +) diff --git a/api/v1/installation_types.go b/api/v1/installation_types.go index b64227400d..ed606af6f8 100644 --- a/api/v1/installation_types.go +++ b/api/v1/installation_types.go @@ -236,6 +236,12 @@ type InstallationSpec struct { // the cluster (including the API server) are exempt from proxying. // +optional Proxy *Proxy `json:"proxy,omitempty"` + + // NetworkPolicyManagement enables or disables the automatic creation of NetworkPolicies + // by the operator. + // +kubebuilder:default=Enabled + // +optional + NetworkPolicyManagement *NetworkPolicyManagement `json:"networkPolicyManagement,omitempty"` } // BPFNetworkBootstrapType defines how the initial networking configuration is executed. diff --git a/pkg/controller/utils/component.go b/pkg/controller/utils/component.go index 10ffc062c8..b719050c3e 100644 --- a/pkg/controller/utils/component.go +++ b/pkg/controller/utils/component.go @@ -24,6 +24,7 @@ import ( "strings" "sync" + netv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" esv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/elasticsearch/v1" @@ -42,6 +43,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" v3 "github.com/tigera/api/pkg/apis/projectcalico/v3" + operatorv1 "github.com/tigera/operator/api/v1" "github.com/tigera/operator/pkg/apigroup" "github.com/tigera/operator/pkg/common" "github.com/tigera/operator/pkg/controller/status" @@ -445,6 +447,35 @@ func (c *componentHandler) CreateOrUpdateOrDelete(ctx context.Context, component var cronJobs []types.NamespacedName objsToCreate, objsToDelete := component.Objects() + + // If NetworkPolicyManagement is disabled, then we should not create any NetworkPolicies, + // and we should actively delete any that already exist. + hasNetworkPolicies := false + for _, obj := range objsToCreate { + if isNetworkPolicy(obj) { + hasNetworkPolicies = true + break + } + } + + if hasNetworkPolicies { + _, installation, err := GetInstallationSpec(ctx, c.client) + if err != nil && !errors.IsNotFound(err) { + return err + } + if installation != nil && installation.NetworkPolicyManagement != nil && *installation.NetworkPolicyManagement == operatorv1.NetworkPolicyManagementDisabled { + newToCreate := []client.Object{} + for _, obj := range objsToCreate { + if isNetworkPolicy(obj) { + objsToDelete = append(objsToDelete, obj) + } else { + newToCreate = append(newToCreate, obj) + } + } + objsToCreate = newToCreate + } + } + osType := component.SupportedOSType() if len(c.apiGroupEnvs) > 0 { @@ -1185,3 +1216,11 @@ func mergeEnvVars(existing []v1.EnvVar, toMerge []v1.EnvVar) []v1.EnvVar { } return existing } + +func isNetworkPolicy(obj client.Object) bool { + switch obj.(type) { + case *v3.NetworkPolicy, *v3.GlobalNetworkPolicy, *netv1.NetworkPolicy: + return true + } + return false +} diff --git a/pkg/controller/utils/component_test.go b/pkg/controller/utils/component_test.go index 6716fa9221..6510e97ab4 100644 --- a/pkg/controller/utils/component_test.go +++ b/pkg/controller/utils/component_test.go @@ -89,6 +89,63 @@ var _ = Describe("Component handler tests", func() { handler = NewComponentHandler(logf.Log, c, scheme, instance) }) + It("respects NetworkPolicyManagement setting in Installation", func() { + // Create an Installation resource with NetworkPolicyManagement: Disabled + disabled := operatorv1.NetworkPolicyManagementDisabled + install := &operatorv1.Installation{ + ObjectMeta: metav1.ObjectMeta{Name: "default"}, + Spec: operatorv1.InstallationSpec{ + NetworkPolicyManagement: &disabled, + }, + } + Expect(c.Create(ctx, install)).To(BeNil()) + + // Create a component that returns a NetworkPolicy and a Deployment. + np := &v3.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-policy", Namespace: "default"}, + } + dep := &apps.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "test-dep", Namespace: "default"}, + } + fc := &fakeComponent{ + supportedOSType: rmeta.OSTypeLinux, + objs: []client.Object{np, dep}, + } + + // Reconcile the component. + err := handler.CreateOrUpdateOrDelete(ctx, fc, sm) + Expect(err).To(BeNil()) + + // Verify that the Deployment was created, but the NetworkPolicy was not. + err = c.Get(ctx, client.ObjectKey{Name: "test-dep", Namespace: "default"}, &apps.Deployment{}) + Expect(err).To(BeNil()) + + err = c.Get(ctx, client.ObjectKey{Name: "test-policy", Namespace: "default"}, &v3.NetworkPolicy{}) + Expect(errors.IsNotFound(err)).To(BeTrue()) + + // Now enable management and reconcile again. + enabled := operatorv1.NetworkPolicyManagementEnabled + install.Spec.NetworkPolicyManagement = &enabled + Expect(c.Update(ctx, install)).To(BeNil()) + + err = handler.CreateOrUpdateOrDelete(ctx, fc, sm) + Expect(err).To(BeNil()) + + // Verify that the NetworkPolicy was created. + err = c.Get(ctx, client.ObjectKey{Name: "test-policy", Namespace: "default"}, &v3.NetworkPolicy{}) + Expect(err).To(BeNil()) + + // Now disable management again and verify that the policy is deleted. + install.Spec.NetworkPolicyManagement = &disabled + Expect(c.Update(ctx, install)).To(BeNil()) + + err = handler.CreateOrUpdateOrDelete(ctx, fc, sm) + Expect(err).To(BeNil()) + + err = c.Get(ctx, client.ObjectKey{Name: "test-policy", Namespace: "default"}, &v3.NetworkPolicy{}) + Expect(errors.IsNotFound(err)).To(BeTrue()) + }) + It("adds Owner references when Custom Resource is provided", func() { fc := &fakeComponent{ supportedOSType: rmeta.OSTypeLinux, @@ -2191,6 +2248,19 @@ var _ = Describe("Mocked client Component handler tests", func() { } It("NetworkPolicy updates are omitted if there is no change", func() { + mc.Info = append(mc.Info, mockReturn{ + Method: "Get", + Return: nil, + InputMutator: func(object client.Object) { + if inst, ok := object.(*operatorv1.Installation); ok { + inst.Name = "default" + } + }, + }) + mc.Info = append(mc.Info, mockReturn{ + Method: "Get", + Return: errors.NewNotFound(schema.GroupResource{Resource: "Installation"}, "overlay"), + }) mc.Info = append(mc.Info, mockReturn{ Method: "Get", Return: nil, @@ -2199,7 +2269,7 @@ var _ = Describe("Mocked client Component handler tests", func() { err := handler.CreateOrUpdateOrDelete(ctx, fc, nil) Expect(err).To(BeNil()) - Expect(mc.Index).To(Equal(1)) + Expect(mc.Index).To(Equal(3)) }) It("NetworkPolicy updates are applied if there is a change", func() { @@ -2211,6 +2281,19 @@ var _ = Describe("Mocked client Component handler tests", func() { } } + mc.Info = append(mc.Info, mockReturn{ + Method: "Get", + Return: nil, + InputMutator: func(object client.Object) { + if inst, ok := object.(*operatorv1.Installation); ok { + inst.Name = "default" + } + }, + }) + mc.Info = append(mc.Info, mockReturn{ + Method: "Get", + Return: errors.NewNotFound(schema.GroupResource{Resource: "Installation"}, "overlay"), + }) mc.Info = append(mc.Info, mockReturn{ Method: "Get", Return: nil, @@ -2225,7 +2308,7 @@ var _ = Describe("Mocked client Component handler tests", func() { err := handler.CreateOrUpdateOrDelete(ctx, fc, nil) Expect(err).To(BeNil()) - Expect(mc.Index).To(Equal(2)) + Expect(mc.Index).To(Equal(4)) }) }) diff --git a/pkg/controller/utils/merge.go b/pkg/controller/utils/merge.go index 03b78a6856..c5a22b0185 100644 --- a/pkg/controller/utils/merge.go +++ b/pkg/controller/utils/merge.go @@ -224,6 +224,11 @@ func OverrideInstallationSpec(cfg, override operatorv1.InstallationSpec) operato inst.Proxy = override.Proxy } + switch compareFields(inst.NetworkPolicyManagement, override.NetworkPolicyManagement) { + case BOnlySet, Different: + inst.NetworkPolicyManagement = override.NetworkPolicyManagement + } + return inst } diff --git a/pkg/controller/utils/merge_test.go b/pkg/controller/utils/merge_test.go index 8aefe13fb6..59f9b5ebf0 100644 --- a/pkg/controller/utils/merge_test.go +++ b/pkg/controller/utils/merge_test.go @@ -33,6 +33,11 @@ import ( func intPtr(i int32) *int32 { return &i } +var ( + enabled = opv1.NetworkPolicyManagementEnabled + disabled = opv1.NetworkPolicyManagementDisabled +) + var _ = Describe("Installation merge tests", func() { DescribeTable("merge Variant", func(main, second, expectVariant *opv1.ProductVariant) { m := opv1.InstallationSpec{} @@ -1574,5 +1579,17 @@ var _ = Describe("Installation merge tests", func() { }, &defaulted), ) + + DescribeTable("merge NetworkPolicyManagement", func(main, second, expect *opv1.NetworkPolicyManagement) { + m := opv1.InstallationSpec{NetworkPolicyManagement: main} + s := opv1.InstallationSpec{NetworkPolicyManagement: second} + inst := OverrideInstallationSpec(m, s) + Expect(inst.NetworkPolicyManagement).To(Equal(expect)) + }, + Entry("Both unset", nil, nil, nil), + Entry("Main set, second unset", &enabled, nil, &enabled), + Entry("Main unset, second set", nil, &enabled, &enabled), + Entry("Both set, different", &enabled, &disabled, &disabled), + ) }) })