diff --git a/pkg/controller/deployment/cert_manager_networkpolicy.go b/pkg/controller/deployment/cert_manager_networkpolicy.go index e3d17ea18..925877be4 100644 --- a/pkg/controller/deployment/cert_manager_networkpolicy.go +++ b/pkg/controller/deployment/cert_manager_networkpolicy.go @@ -5,6 +5,7 @@ import ( "fmt" networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -133,12 +134,15 @@ func (c *CertManagerNetworkPolicyUserDefinedController) sync(ctx context.Context } // Apply user-defined network policies - if err := c.reconcileUserNetworkPolicies(ctx, certManager); err != nil { + changed, err := c.reconcileUserNetworkPolicies(ctx, certManager) + if err != nil { c.eventRecorder.Warningf("UserNetworkPolicyReconcileFailed", "Failed to reconcile user network policies: %v", err) return fmt.Errorf("failed to reconcile user network policies: %w", err) } - c.eventRecorder.Event("UserNetworkPolicyReconcileSuccess", "Successfully reconciled user-defined network policies") + if changed { + c.eventRecorder.Event("UserNetworkPolicyReconcileSuccess", "Successfully reconciled user-defined network policies") + } return nil } @@ -165,16 +169,21 @@ func (c *CertManagerNetworkPolicyUserDefinedController) validateComponentName(co } } -func (c *CertManagerNetworkPolicyUserDefinedController) reconcileUserNetworkPolicies(ctx context.Context, certManager *v1alpha1.CertManager) error { +func (c *CertManagerNetworkPolicyUserDefinedController) reconcileUserNetworkPolicies(ctx context.Context, certManager *v1alpha1.CertManager) (bool, error) { + changed := false // Apply each user-defined network policy for _, userPolicy := range certManager.Spec.NetworkPolicies { policy := c.createUserNetworkPolicy(userPolicy) - if err := c.createOrUpdateNetworkPolicy(ctx, policy); err != nil { - return fmt.Errorf("failed to create/update user network policy %s: %w", policy.Name, err) + policyChanged, err := c.createOrUpdateNetworkPolicy(ctx, policy) + if err != nil { + return changed, fmt.Errorf("failed to create/update user network policy %s: %w", policy.Name, err) + } + if policyChanged { + changed = true } } - return nil + return changed, nil } func (c *CertManagerNetworkPolicyUserDefinedController) createUserNetworkPolicy(userPolicy v1alpha1.NetworkPolicy) *networkingv1.NetworkPolicy { @@ -227,30 +236,32 @@ func (c *CertManagerNetworkPolicyUserDefinedController) getPodSelectorForCompone } } -func (c *CertManagerNetworkPolicyUserDefinedController) createOrUpdateNetworkPolicy(ctx context.Context, policy *networkingv1.NetworkPolicy) error { +func (c *CertManagerNetworkPolicyUserDefinedController) createOrUpdateNetworkPolicy(ctx context.Context, policy *networkingv1.NetworkPolicy) (bool, error) { existing, err := c.kubeClient.NetworkingV1().NetworkPolicies(policy.Namespace).Get(ctx, policy.Name, metav1.GetOptions{}) if err != nil { if errors.IsNotFound(err) { - // Create new policy _, err := c.kubeClient.NetworkingV1().NetworkPolicies(policy.Namespace).Create(ctx, policy, metav1.CreateOptions{}) if err != nil { - return fmt.Errorf("failed to create network policy: %w", err) + return false, fmt.Errorf("failed to create network policy: %w", err) } c.eventRecorder.Eventf("NetworkPolicyCreated", "Created user-defined network policy %s", policy.Name) - return nil + return true, nil } - return fmt.Errorf("failed to get existing network policy: %w", err) + return false, fmt.Errorf("failed to get existing network policy: %w", err) + } + + if equality.Semantic.DeepEqual(existing.Spec, policy.Spec) && + equality.Semantic.DeepEqual(existing.Labels, policy.Labels) { + return false, nil } - // Update existing policy existing.Spec = policy.Spec existing.Labels = policy.Labels _, err = c.kubeClient.NetworkingV1().NetworkPolicies(policy.Namespace).Update(ctx, existing, metav1.UpdateOptions{}) if err != nil { - return fmt.Errorf("failed to update network policy: %w", err) + return false, fmt.Errorf("failed to update network policy: %w", err) } c.eventRecorder.Eventf("NetworkPolicyUpdated", "Updated user-defined network policy %s", policy.Name) - - return nil + return true, nil } diff --git a/pkg/controller/deployment/cert_manager_networkpolicy_test.go b/pkg/controller/deployment/cert_manager_networkpolicy_test.go new file mode 100644 index 000000000..33ecf7092 --- /dev/null +++ b/pkg/controller/deployment/cert_manager_networkpolicy_test.go @@ -0,0 +1,130 @@ +package deployment + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/utils/clock" + + "github.com/openshift/library-go/pkg/operator/events" +) + +func newTestController() (*CertManagerNetworkPolicyUserDefinedController, events.InMemoryRecorder) { + recorder := events.NewInMemoryRecorder("test", clock.RealClock{}) + c := &CertManagerNetworkPolicyUserDefinedController{ + kubeClient: fake.NewSimpleClientset(), + eventRecorder: recorder.WithComponentSuffix("cert-manager-networkpolicy-user-defined"), + } + return c, recorder +} + +func newTestNetworkPolicy(name string, port int) *networkingv1.NetworkPolicy { + proto := corev1.ProtocolTCP + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: certManagerNamespace, + Labels: map[string]string{ + networkPolicyOwnerLabel: "cert-manager", + }, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "cert-manager"}, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeEgress, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(port)}, + Protocol: &proto, + }, + }, + }, + }, + }, + } +} + +func TestCreateOrUpdateNetworkPolicy_Create(t *testing.T) { + c, recorder := newTestController() + ctx := context.Background() + + policy := newTestNetworkPolicy("cert-manager-user-test", 443) + changed, err := c.createOrUpdateNetworkPolicy(ctx, policy) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !changed { + t.Fatal("expected changed=true for new policy creation") + } + + evts := recorder.Events() + if len(evts) != 1 { + t.Fatalf("expected 1 event, got %d", len(evts)) + } + if evts[0].Reason != "NetworkPolicyCreated" { + t.Errorf("expected reason NetworkPolicyCreated, got %s", evts[0].Reason) + } +} + +func TestCreateOrUpdateNetworkPolicy_NoOpUpdate(t *testing.T) { + c, recorder := newTestController() + ctx := context.Background() + + policy := newTestNetworkPolicy("cert-manager-user-test", 443) + + _, err := c.kubeClient.NetworkingV1().NetworkPolicies(certManagerNamespace).Create(ctx, policy, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("setup: unexpected error: %v", err) + } + + changed, err := c.createOrUpdateNetworkPolicy(ctx, policy) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if changed { + t.Fatal("expected changed=false when spec and labels are identical") + } + + evts := recorder.Events() + if len(evts) != 0 { + t.Fatalf("expected 0 events for no-op update, got %d", len(evts)) + } +} + +func TestCreateOrUpdateNetworkPolicy_UpdateOnSpecChange(t *testing.T) { + c, recorder := newTestController() + ctx := context.Background() + + original := newTestNetworkPolicy("cert-manager-user-test", 443) + _, err := c.kubeClient.NetworkingV1().NetworkPolicies(certManagerNamespace).Create(ctx, original, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("setup: unexpected error: %v", err) + } + + updated := newTestNetworkPolicy("cert-manager-user-test", 8443) + changed, err := c.createOrUpdateNetworkPolicy(ctx, updated) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !changed { + t.Fatal("expected changed=true when spec differs") + } + + evts := recorder.Events() + if len(evts) != 1 { + t.Fatalf("expected 1 event, got %d", len(evts)) + } + if evts[0].Reason != "NetworkPolicyUpdated" { + t.Errorf("expected reason NetworkPolicyUpdated, got %s", evts[0].Reason) + } +}