remove old code

Author: Sarthak Agrawal

internal/controller/state/graph/backend_tls_policy.go

@@ -37,7 +37,6 @@ func processBackendTLSPolicies(
 	backendTLSPolicies map[types.NamespacedName]*v1alpha3.BackendTLSPolicy,
 	configMapResolver *configMapResolver,
 	secretResolver *secretResolver,
-	ctlrName string,
 	gateways map[types.NamespacedName]*Gateway,
 ) map[types.NamespacedName]*BackendTLSPolicy {
 	if len(backendTLSPolicies) == 0 || len(gateways) == 0 {
@@ -48,7 +47,7 @@ func processBackendTLSPolicies(
 	for nsname, backendTLSPolicy := range backendTLSPolicies {
 		var caCertRef types.NamespacedName
 
-		valid, ignored, conds := validateBackendTLSPolicy(backendTLSPolicy, configMapResolver, secretResolver, ctlrName)
+		valid, ignored, conds := validateBackendTLSPolicy(backendTLSPolicy, configMapResolver, secretResolver)
 
 		if valid && !ignored && backendTLSPolicy.Spec.Validation.CACertificateRefs != nil {
 			caCertRef = types.NamespacedName{
@@ -71,21 +70,13 @@ func validateBackendTLSPolicy(
 	backendTLSPolicy *v1alpha3.BackendTLSPolicy,
 	configMapResolver *configMapResolver,
 	secretResolver *secretResolver,
-	_ string,
 ) (valid, ignored bool, conds []conditions.Condition) {
 	valid = true
 	ignored = false
 
-	// Ancestor limit handling is now done during gateway assignment phase, not validation
-	// if backendTLSPolicyAncestorsFull(backendTLSPolicy.Status.Ancestors, ctlrName) {
-	//	valid = false
-	//	ignored = true
-	//	return
-	// }
-
 	if err := validateBackendTLSHostname(backendTLSPolicy); err != nil {
 		valid = false
-		conds = append(conds, conditions.NewPolicyInvalid(err.Error()))
+		conds = append(conds, conditions.NewPolicyInvalid(fmt.Sprintf("invalid hostname: %s", err.Error())))
 	}
 
 	caCertRefs := backendTLSPolicy.Spec.Validation.CACertificateRefs

internal/controller/state/graph/backend_tls_policy_test.go

@@ -80,7 +80,7 @@ func TestProcessBackendTLSPoliciesEmpty(t *testing.T) {
 			t.Parallel()
 			g := NewWithT(t)
 
-			processed := processBackendTLSPolicies(test.backendTLSPolicies, nil, nil, "test", test.gateways)
+			processed := processBackendTLSPolicies(test.backendTLSPolicies, nil, nil, test.gateways)
 
 			g.Expect(processed).To(Equal(test.expected))
 		})
@@ -477,7 +477,7 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			valid, ignored, conds := validateBackendTLSPolicy(test.tlsPolicy, configMapResolver, secretMapResolver, "test")
+			valid, ignored, conds := validateBackendTLSPolicy(test.tlsPolicy, configMapResolver, secretMapResolver)
 
 			if !test.isValid && !test.ignored {
 				g.Expect(conds).To(HaveLen(1))
@@ -809,105 +809,6 @@ func TestAddGatewaysForBackendTLSPoliciesAncestorLimit(t *testing.T) {
 	g.Expect(logOutput).To(ContainSubstring("ancestor=test/gateway1"))
 }
 
-func TestBackendTLSPolicyAncestorsFullFunc(t *testing.T) {
-	t.Parallel()
-
-	getAncestorRef := func(ctlrName, parentName string) v1alpha2.PolicyAncestorStatus {
-		return v1alpha2.PolicyAncestorStatus{
-			ControllerName: gatewayv1.GatewayController(ctlrName),
-			AncestorRef: gatewayv1.ParentReference{
-				Name:      gatewayv1.ObjectName(parentName),
-				Namespace: helpers.GetPointer(gatewayv1.Namespace("test")),
-				Group:     helpers.GetPointer[gatewayv1.Group](gatewayv1.GroupName),
-				Kind:      helpers.GetPointer[gatewayv1.Kind](kinds.Gateway),
-			},
-		}
-	}
-
-	tests := []struct {
-		name      string
-		ctlrName  string
-		ancestors []v1alpha2.PolicyAncestorStatus
-		expected  bool
-	}{
-		{
-			name:      "empty ancestors list",
-			ancestors: []v1alpha2.PolicyAncestorStatus{},
-			ctlrName:  "nginx-gateway",
-			expected:  false,
-		},
-		{
-			name: "less than 16 ancestors",
-			ancestors: []v1alpha2.PolicyAncestorStatus{
-				getAncestorRef("other-controller", "gateway1"),
-				getAncestorRef("other-controller", "gateway2"),
-			},
-			ctlrName: "nginx-gateway",
-			expected: false,
-		},
-		{
-			name: "exactly 16 ancestors, none from our controller",
-			ancestors: func() []v1alpha2.PolicyAncestorStatus {
-				ancestors := make([]v1alpha2.PolicyAncestorStatus, 16)
-				for i := range 16 {
-					ancestors[i] = getAncestorRef("other-controller", "gateway")
-				}
-				return ancestors
-			}(),
-			ctlrName: "nginx-gateway",
-			expected: true,
-		},
-		{
-			name: "exactly 16 ancestors, one from our controller",
-			ancestors: func() []v1alpha2.PolicyAncestorStatus {
-				ancestors := make([]v1alpha2.PolicyAncestorStatus, 16)
-				for i := range 15 {
-					ancestors[i] = getAncestorRef("other-controller", "gateway")
-				}
-				ancestors[15] = getAncestorRef("nginx-gateway", "our-gateway")
-				return ancestors
-			}(),
-			ctlrName: "nginx-gateway",
-			expected: false, // Not full because we can overwrite our own entry
-		},
-		{
-			name: "more than 16 ancestors, none from our controller",
-			ancestors: func() []v1alpha2.PolicyAncestorStatus {
-				ancestors := make([]v1alpha2.PolicyAncestorStatus, 20)
-				for i := range 20 {
-					ancestors[i] = getAncestorRef("other-controller", "gateway")
-				}
-				return ancestors
-			}(),
-			ctlrName: "nginx-gateway",
-			expected: true,
-		},
-		{
-			name: "more than 16 ancestors, one from our controller",
-			ancestors: func() []v1alpha2.PolicyAncestorStatus {
-				ancestors := make([]v1alpha2.PolicyAncestorStatus, 20)
-				for i := range 19 {
-					ancestors[i] = getAncestorRef("other-controller", "gateway")
-				}
-				ancestors[19] = getAncestorRef("nginx-gateway", "our-gateway")
-				return ancestors
-			}(),
-			ctlrName: "nginx-gateway",
-			expected: false, // Not full because we can overwrite our own entry
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-			g := NewWithT(t)
-
-			result := backendTLSPolicyAncestorsFull(test.ancestors, test.ctlrName)
-			g.Expect(result).To(Equal(test.expected))
-		})
-	}
-}
-
 // testLogSink implements logr.LogSink for testing.
 type testLogSink struct {
 	buffer *bytes.Buffer

internal/controller/state/graph/graph.go

@@ -239,7 +239,6 @@ func BuildGraph(
 		state.BackendTLSPolicies,
 		configMapResolver,
 		secretResolver,
-		controllerName,
 		gws,
 	)
 

internal/controller/state/graph/policy_ancestor.go

@@ -6,7 +6,6 @@ import (
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/types"
 	v1 "sigs.k8s.io/gateway-api/apis/v1"
-	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/policies"
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers"
@@ -19,27 +18,6 @@ func LogAncestorLimitReached(logger logr.Logger, policyName, policyKind, ancesto
 	logger.Info("Policy ancestor limit reached", "policy", policyName, "policyKind", policyKind, "ancestor", ancestorName)
 }
 
-// backendTLSPolicyAncestorsFull returns whether or not an ancestor list is full. A list is not full when:
-// - the number of current ancestors is less than the maximum allowed
-// - an entry for an NGF managed resource already exists in the ancestor list. This means that we are overwriting
-// that status entry with the current status entry, since there is only one ancestor (Gateway) for this policy.
-func backendTLSPolicyAncestorsFull(
-	ancestors []v1alpha2.PolicyAncestorStatus,
-	ctlrName string,
-) bool {
-	if len(ancestors) < maxAncestors {
-		return false
-	}
-
-	for _, ancestor := range ancestors {
-		if string(ancestor.ControllerName) == ctlrName {
-			return false
-		}
-	}
-
-	return true
-}
-
 // ngfPolicyAncestorsFull returns whether or not an ancestor list is full. A list is full when
 // the sum of the following is greater than or equal to the maximum allowed:
 //   - number of non-NGF managed ancestors

internal/controller/state/graph/policy_ancestor_test.go

@@ -12,53 +12,6 @@ import (
 	"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/kinds"
 )
 
-func TestBackendTLSPolicyAncestorsFull(t *testing.T) {
-	t.Parallel()
-	createCurStatus := func(numAncestors int, ctlrName string) []v1alpha2.PolicyAncestorStatus {
-		statuses := make([]v1alpha2.PolicyAncestorStatus, 0, numAncestors)
-
-		for range numAncestors {
-			statuses = append(statuses, v1alpha2.PolicyAncestorStatus{
-				ControllerName: v1.GatewayController(ctlrName),
-			})
-		}
-
-		return statuses
-	}
-
-	tests := []struct {
-		name      string
-		curStatus []v1alpha2.PolicyAncestorStatus
-		expFull   bool
-	}{
-		{
-			name:      "not full",
-			curStatus: createCurStatus(15, "controller"),
-			expFull:   false,
-		},
-		{
-			name:      "full; ancestor does not exist in current status",
-			curStatus: createCurStatus(16, "controller"),
-			expFull:   true,
-		},
-		{
-			name:      "full, but ancestor does exist in current status",
-			curStatus: createCurStatus(16, "nginx-gateway"),
-			expFull:   false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-			g := NewWithT(t)
-
-			full := backendTLSPolicyAncestorsFull(test.curStatus, "nginx-gateway")
-			g.Expect(full).To(Equal(test.expFull))
-		})
-	}
-}
-
 func TestNGFPolicyAncestorsFull(t *testing.T) {
 	t.Parallel()
 	type ancestorConfig struct {