Fix logic relating to backends/policies/nginxproxy/etc

Author: sjberman

cmd/gateway/commands.go

@@ -12,7 +12,6 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"go.uber.org/zap"
-	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/klog/v2"
 	ctlr "sigs.k8s.io/controller-runtime"
@@ -59,7 +58,6 @@ func createRootCommand() *cobra.Command {
 func createControllerCommand() *cobra.Command {
 	// flag names
 	const (
-		gatewayFlag                    = "gateway"
 		configFlag                     = "config"
 		serviceFlag                    = "service"
 		agentTLSSecretFlag             = "agent-tls-secret"
@@ -93,7 +91,6 @@ func createControllerCommand() *cobra.Command {
 			validator: validateResourceName,
 		}
 
-		gateway    = namespacedNameValue{}
 		configName = stringValidatingValue{
 			validator: validateResourceName,
 		}
@@ -198,11 +195,6 @@ func createControllerCommand() *cobra.Command {
 				return fmt.Errorf("error parsing telemetry endpoint insecure: %w", err)
 			}
 
-			var gwNsName *types.NamespacedName
-			if cmd.Flags().Changed(gatewayFlag) {
-				gwNsName = &gateway.value
-			}
-
 			var usageReportConfig config.UsageReportConfig
 			if plus && usageReportSecretName.value == "" {
 				return errors.New("usage-report-secret is required when using NGINX Plus")
@@ -232,7 +224,6 @@ func createControllerCommand() *cobra.Command {
 				Logger:           logger,
 				AtomicLevel:      atom,
 				GatewayClassName: gatewayClassName.value,
-				GatewayNsName:    gwNsName,
 				GatewayPodConfig: podConfig,
 				HealthConfig: config.HealthConfig{
 					Enabled: !disableHealth,
@@ -290,16 +281,6 @@ func createControllerCommand() *cobra.Command {
 	)
 	utilruntime.Must(cmd.MarkFlagRequired(gatewayClassFlag))
 
-	cmd.Flags().Var(
-		&gateway,
-		gatewayFlag,
-		"The namespaced name of the Gateway resource to use. "+
-			"Must be of the form: NAMESPACE/NAME. "+
-			"If not specified, the control plane will process all Gateways for the configured GatewayClass. "+
-			"However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are "+
-			"equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.",
-	)
-
 	cmd.Flags().VarP(
 		&configName,
 		configFlag,

cmd/gateway/commands_test.go

@@ -9,7 +9,6 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/nginx/nginx-gateway-fabric/internal/mode/static/config"
 )
@@ -63,8 +62,6 @@ func TestCommonFlagsValidation(t *testing.T) {
 			args: []string{
 				"--gateway-ctlr-name=gateway.nginx.org/nginx-gateway",
 				"--gatewayclass=nginx",
-				"--gateway=nginx-gateway/nginx",
-				"--config=nginx-gateway-config",
 			},
 			wantErr: false,
 		},
@@ -139,7 +136,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
 			args: []string{
 				"--gateway-ctlr-name=gateway.nginx.org/nginx-gateway", // common and required flag
 				"--gatewayclass=nginx",                                // common and required flag
-				"--gateway=nginx-gateway/nginx",
 				"--config=nginx-gateway-config",
 				"--service=nginx-gateway",
 				"--agent-tls-secret=agent-tls",
@@ -171,23 +167,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
 			},
 			wantErr: false,
 		},
-		{
-			name: "gateway is set to empty string",
-			args: []string{
-				"--gateway=",
-			},
-			wantErr:           true,
-			expectedErrPrefix: `invalid argument "" for "--gateway" flag: must be set`,
-		},
-		{
-			name: "gateway is invalid",
-			args: []string{
-				"--gateway=nginx-gateway", // no namespace
-			},
-			wantErr: true,
-			expectedErrPrefix: `invalid argument "nginx-gateway" for "--gateway" flag: invalid format; ` +
-				"must be NAMESPACE/NAME",
-		},
 		{
 			name: "config is set to empty string",
 			args: []string{
@@ -712,30 +691,6 @@ func TestParseFlags(t *testing.T) {
 	err = flagSet.Set("customStringFlagUserDefined", "changed-test-flag-value")
 	g.Expect(err).To(Not(HaveOccurred()))
 
-	customStringFlagNoDefaultValueUnset := namespacedNameValue{
-		value: types.NamespacedName{},
-	}
-	flagSet.Var(
-		&customStringFlagNoDefaultValueUnset,
-		"customStringFlagNoDefaultValueUnset",
-		"no default value custom string test flag",
-	)
-
-	customStringFlagNoDefaultValueUserDefined := namespacedNameValue{
-		value: types.NamespacedName{},
-	}
-	flagSet.Var(
-		&customStringFlagNoDefaultValueUserDefined,
-		"customStringFlagNoDefaultValueUserDefined",
-		"no default value but with user defined namespacedName test flag",
-	)
-	userDefinedNamespacedName := types.NamespacedName{
-		Namespace: "changed-namespace",
-		Name:      "changed-name",
-	}
-	err = flagSet.Set("customStringFlagNoDefaultValueUserDefined", userDefinedNamespacedName.String())
-	g.Expect(err).To(Not(HaveOccurred()))
-
 	expectedKeys := []string{
 		"boolFlagTrue",
 		"boolFlagFalse",
@@ -745,9 +700,6 @@ func TestParseFlags(t *testing.T) {
 
 		"customStringFlagDefault",
 		"customStringFlagUserDefined",
-
-		"customStringFlagNoDefaultValueUnset",
-		"customStringFlagNoDefaultValueUserDefined",
 	}
 	expectedValues := []string{
 		"true",
@@ -758,9 +710,6 @@ func TestParseFlags(t *testing.T) {
 
 		"default",
 		"user-defined",
-
-		"default",
-		"user-defined",
 	}
 
 	flagKeys, flagValues := parseFlags(flagSet)

cmd/gateway/validating_types.go

@@ -6,8 +6,6 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
-
-	"k8s.io/apimachinery/pkg/types"
 )
 
 // stringValidatingValue is a string flag value with custom validation logic.
@@ -106,31 +104,3 @@ func (v *intValidatingValue) Set(param string) error {
 func (v *intValidatingValue) Type() string {
 	return "int"
 }
-
-// namespacedNameValue is a string flag value that represents a namespaced name.
-// it implements the pflag.Value interface.
-type namespacedNameValue struct {
-	value types.NamespacedName
-}
-
-func (v *namespacedNameValue) String() string {
-	if (v.value == types.NamespacedName{}) {
-		// if we don't do that, the default value in the help message will be printed as "/"
-		return ""
-	}
-	return v.value.String()
-}
-
-func (v *namespacedNameValue) Set(param string) error {
-	nsname, err := parseNamespacedResourceName(param)
-	if err != nil {
-		return err
-	}
-
-	v.value = nsname
-	return nil
-}
-
-func (v *namespacedNameValue) Type() string {
-	return "string"
-}

cmd/gateway/validation.go

@@ -8,7 +8,6 @@ import (
 	"strconv"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/validation"
 )
 
@@ -55,40 +54,6 @@ func validateResourceName(value string) error {
 	return nil
 }
 
-func validateNamespaceName(value string) error {
-	// used by Kubernetes to validate resource namespace names
-	messages := validation.IsDNS1123Label(value)
-	if len(messages) > 0 {
-		msg := strings.Join(messages, "; ")
-		return fmt.Errorf("invalid format: %s", msg)
-	}
-
-	return nil
-}
-
-func parseNamespacedResourceName(value string) (types.NamespacedName, error) {
-	if value == "" {
-		return types.NamespacedName{}, errors.New("must be set")
-	}
-
-	parts := strings.Split(value, "/")
-	if len(parts) != 2 {
-		return types.NamespacedName{}, errors.New("invalid format; must be NAMESPACE/NAME")
-	}
-
-	if err := validateNamespaceName(parts[0]); err != nil {
-		return types.NamespacedName{}, fmt.Errorf("invalid namespace name: %w", err)
-	}
-	if err := validateResourceName(parts[1]); err != nil {
-		return types.NamespacedName{}, fmt.Errorf("invalid resource name: %w", err)
-	}
-
-	return types.NamespacedName{
-		Namespace: parts[0],
-		Name:      parts[1],
-	}, nil
-}
-
 func validateQualifiedName(name string) error {
 	if len(name) == 0 {
 		return errors.New("must be set")

cmd/gateway/validation_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
-	"k8s.io/apimachinery/pkg/types"
 )
 
 func TestValidateGatewayControllerName(t *testing.T) {
@@ -132,137 +131,6 @@ func TestValidateResourceName(t *testing.T) {
 	}
 }
 
-func TestValidateNamespaceName(t *testing.T) {
-	t.Parallel()
-	tests := []struct {
-		name   string
-		value  string
-		expErr bool
-	}{
-		{
-			name:   "valid",
-			value:  "mynamespace",
-			expErr: false,
-		},
-		{
-			name:   "valid - with dash",
-			value:  "my-namespace",
-			expErr: false,
-		},
-		{
-			name:   "valid - with numbers",
-			value:  "mynamespace123",
-			expErr: false,
-		},
-		{
-			name:   "invalid - empty",
-			value:  "",
-			expErr: true,
-		},
-		{
-			name:   "invalid - invalid character '.'",
-			value:  "my.namespace",
-			expErr: true,
-		},
-		{
-			name:   "invalid - invalid character '/'",
-			value:  "my/namespace",
-			expErr: true,
-		},
-		{
-			name:   "invalid - invalid character '_'",
-			value:  "my_namespace",
-			expErr: true,
-		},
-		{
-			name:   "invalid - invalid character '@'",
-			value:  "my@namespace",
-			expErr: true,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-			g := NewWithT(t)
-
-			err := validateNamespaceName(test.value)
-
-			if test.expErr {
-				g.Expect(err).To(HaveOccurred())
-			} else {
-				g.Expect(err).ToNot(HaveOccurred())
-			}
-		})
-	}
-}
-
-func TestParseNamespacedResourceName(t *testing.T) {
-	t.Parallel()
-	tests := []struct {
-		name              string
-		value             string
-		expectedErrPrefix string
-		expectedNsName    types.NamespacedName
-		expectErr         bool
-	}{
-		{
-			name:  "valid",
-			value: "test/my-gateway",
-			expectedNsName: types.NamespacedName{
-				Namespace: "test",
-				Name:      "my-gateway",
-			},
-			expectErr: false,
-		},
-		{
-			name:              "empty",
-			value:             "",
-			expectedNsName:    types.NamespacedName{},
-			expectErr:         true,
-			expectedErrPrefix: "must be set",
-		},
-		{
-			name:              "wrong number of parts",
-			value:             "test",
-			expectedNsName:    types.NamespacedName{},
-			expectErr:         true,
-			expectedErrPrefix: "invalid format; must be NAMESPACE/NAME",
-		},
-		{
-			name:              "invalid namespace",
-			value:             "t@st/my-gateway",
-			expectedNsName:    types.NamespacedName{},
-			expectErr:         true,
-			expectedErrPrefix: "invalid namespace name",
-		},
-		{
-			name:              "invalid name",
-			value:             "test/my-g@teway",
-			expectedNsName:    types.NamespacedName{},
-			expectErr:         true,
-			expectedErrPrefix: "invalid resource name",
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-			g := NewWithT(t)
-
-			nsName, err := parseNamespacedResourceName(test.value)
-
-			if test.expectErr {
-				g.Expect(err).To(HaveOccurred())
-				g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix))
-			} else {
-				g.Expect(err).ToNot(HaveOccurred())
-				g.Expect(nsName).To(Equal(test.expectedNsName))
-			}
-		})
-	}
-}
-
 func TestValidateQualifiedName(t *testing.T) {
 	t.Parallel()
 	tests := []struct {