Skip to content

Add support for multiple gateways #3275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Apr 22, 2025
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 7 additions & 36 deletions cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"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"
Expand Down Expand Up @@ -59,11 +58,9 @@
func createControllerCommand() *cobra.Command {
// flag names
const (
gatewayFlag = "gateway"
configFlag = "config"
serviceFlag = "service"
agentTLSSecretFlag = "agent-tls-secret"
updateGCStatusFlag = "update-gatewayclass-status"
metricsDisableFlag = "metrics-disable"
metricsSecureFlag = "metrics-secure-serving"
metricsPortFlag = "metrics-port"
Expand Down Expand Up @@ -94,9 +91,7 @@
validator: validateResourceName,
}

updateGCStatus bool
gateway = namespacedNameValue{}
configName = stringValidatingValue{
configName = stringValidatingValue{
validator: validateResourceName,
}
serviceName = stringValidatingValue{
Expand Down Expand Up @@ -200,11 +195,6 @@
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")
Expand All @@ -229,14 +219,12 @@
}

conf := config.Config{
GatewayCtlrName: gatewayCtlrName.value,
ConfigName: configName.String(),
Logger: logger,
AtomicLevel: atom,
GatewayClassName: gatewayClassName.value,
GatewayNsName: gwNsName,
UpdateGatewayClassStatus: updateGCStatus,
GatewayPodConfig: podConfig,
GatewayCtlrName: gatewayCtlrName.value,
ConfigName: configName.String(),
Logger: logger,
AtomicLevel: atom,
GatewayClassName: gatewayClassName.value,
GatewayPodConfig: podConfig,

Check warning on line 227 in cmd/gateway/commands.go

View check run for this annotation

Codecov / codecov/patch

cmd/gateway/commands.go#L222-L227

Added lines #L222 - L227 were not covered by tests
HealthConfig: config.HealthConfig{
Enabled: !disableHealth,
Port: healthListenPort.value,
Expand Down Expand Up @@ -293,16 +281,6 @@
)
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,
Expand All @@ -326,13 +304,6 @@
`NGINX Gateway Fabric control plane is running in (default namespace: nginx-gateway).`,
)

cmd.Flags().BoolVar(
&updateGCStatus,
updateGCStatusFlag,
true,
"Update the status of the GatewayClass resource.",
)

cmd.Flags().BoolVar(
&disableMetrics,
metricsDisableFlag,
Expand Down
66 changes: 0 additions & 66 deletions cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -137,11 +136,9 @@ 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",
"--update-gatewayclass-status=true",
"--metrics-port=9114",
"--metrics-disable",
"--metrics-secure-serving",
Expand Down Expand Up @@ -170,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{
Expand Down Expand Up @@ -235,22 +215,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
wantErr: true,
expectedErrPrefix: `invalid argument "!@#$" for "--agent-tls-secret" flag: invalid format`,
},
{
name: "update-gatewayclass-status is set to empty string",
args: []string{
"--update-gatewayclass-status=",
},
wantErr: true,
expectedErrPrefix: `invalid argument "" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
},
{
name: "update-gatewayclass-status is invalid",
args: []string{
"--update-gatewayclass-status=invalid", // not a boolean
},
wantErr: true,
expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
},
{
name: "metrics-port is invalid type",
args: []string{
Expand Down Expand Up @@ -727,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",
Expand All @@ -760,9 +700,6 @@ func TestParseFlags(t *testing.T) {

"customStringFlagDefault",
"customStringFlagUserDefined",

"customStringFlagNoDefaultValueUnset",
"customStringFlagNoDefaultValueUserDefined",
}
expectedValues := []string{
"true",
Expand All @@ -773,9 +710,6 @@ func TestParseFlags(t *testing.T) {

"default",
"user-defined",

"default",
"user-defined",
}

flagKeys, flagValues := parseFlags(flagSet)
Expand Down
30 changes: 0 additions & 30 deletions cmd/gateway/validating_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"fmt"
"strconv"
"strings"

"k8s.io/apimachinery/pkg/types"
)

// stringValidatingValue is a string flag value with custom validation logic.
Expand Down Expand Up @@ -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"
}
35 changes: 0 additions & 35 deletions cmd/gateway/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"strconv"
"strings"

"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
)

Expand Down Expand Up @@ -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")
Expand Down
Loading
Loading