Skip to content

Commit 61c9184

Browse files
sjbermansjberman
committed
Fix logic relating to backends/policies/nginxproxy/etc
1 parent 46705fe commit 61c9184

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+2968
-2745
lines changed

Diff for: ‎cmd/gateway/commands.go

-19
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/spf13/cobra"
1313
"github.com/spf13/pflag"
1414
"go.uber.org/zap"
15-
"k8s.io/apimachinery/pkg/types"
1615
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1716
"k8s.io/klog/v2"
1817
ctlr "sigs.k8s.io/controller-runtime"
@@ -59,7 +58,6 @@ func createRootCommand() *cobra.Command {
5958
func createControllerCommand() *cobra.Command {
6059
// flag names
6160
const (
62-
gatewayFlag = "gateway"
6361
configFlag = "config"
6462
serviceFlag = "service"
6563
agentTLSSecretFlag = "agent-tls-secret"
@@ -93,7 +91,6 @@ func createControllerCommand() *cobra.Command {
9391
validator: validateResourceName,
9492
}
9593

96-
gateway = namespacedNameValue{}
9794
configName = stringValidatingValue{
9895
validator: validateResourceName,
9996
}
@@ -198,11 +195,6 @@ func createControllerCommand() *cobra.Command {
198195
return fmt.Errorf("error parsing telemetry endpoint insecure: %w", err)
199196
}
200197

201-
var gwNsName *types.NamespacedName
202-
if cmd.Flags().Changed(gatewayFlag) {
203-
gwNsName = &gateway.value
204-
}
205-
206198
var usageReportConfig config.UsageReportConfig
207199
if plus && usageReportSecretName.value == "" {
208200
return errors.New("usage-report-secret is required when using NGINX Plus")
@@ -232,7 +224,6 @@ func createControllerCommand() *cobra.Command {
232224
Logger: logger,
233225
AtomicLevel: atom,
234226
GatewayClassName: gatewayClassName.value,
235-
GatewayNsName: gwNsName,
236227
GatewayPodConfig: podConfig,
237228
HealthConfig: config.HealthConfig{
238229
Enabled: !disableHealth,
@@ -290,16 +281,6 @@ func createControllerCommand() *cobra.Command {
290281
)
291282
utilruntime.Must(cmd.MarkFlagRequired(gatewayClassFlag))
292283

293-
cmd.Flags().Var(
294-
&gateway,
295-
gatewayFlag,
296-
"The namespaced name of the Gateway resource to use. "+
297-
"Must be of the form: NAMESPACE/NAME. "+
298-
"If not specified, the control plane will process all Gateways for the configured GatewayClass. "+
299-
"However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are "+
300-
"equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.",
301-
)
302-
303284
cmd.Flags().VarP(
304285
&configName,
305286
configFlag,

Diff for: ‎cmd/gateway/commands_test.go

-51
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
. "github.com/onsi/gomega"
1010
"github.com/spf13/cobra"
1111
"github.com/spf13/pflag"
12-
"k8s.io/apimachinery/pkg/types"
1312

1413
"github.com/nginx/nginx-gateway-fabric/internal/mode/static/config"
1514
)
@@ -63,8 +62,6 @@ func TestCommonFlagsValidation(t *testing.T) {
6362
args: []string{
6463
"--gateway-ctlr-name=gateway.nginx.org/nginx-gateway",
6564
"--gatewayclass=nginx",
66-
"--gateway=nginx-gateway/nginx",
67-
"--config=nginx-gateway-config",
6865
},
6966
wantErr: false,
7067
},
@@ -139,7 +136,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
139136
args: []string{
140137
"--gateway-ctlr-name=gateway.nginx.org/nginx-gateway", // common and required flag
141138
"--gatewayclass=nginx", // common and required flag
142-
"--gateway=nginx-gateway/nginx",
143139
"--config=nginx-gateway-config",
144140
"--service=nginx-gateway",
145141
"--agent-tls-secret=agent-tls",
@@ -171,23 +167,6 @@ func TestControllerCmdFlagValidation(t *testing.T) {
171167
},
172168
wantErr: false,
173169
},
174-
{
175-
name: "gateway is set to empty string",
176-
args: []string{
177-
"--gateway=",
178-
},
179-
wantErr: true,
180-
expectedErrPrefix: `invalid argument "" for "--gateway" flag: must be set`,
181-
},
182-
{
183-
name: "gateway is invalid",
184-
args: []string{
185-
"--gateway=nginx-gateway", // no namespace
186-
},
187-
wantErr: true,
188-
expectedErrPrefix: `invalid argument "nginx-gateway" for "--gateway" flag: invalid format; ` +
189-
"must be NAMESPACE/NAME",
190-
},
191170
{
192171
name: "config is set to empty string",
193172
args: []string{
@@ -712,30 +691,6 @@ func TestParseFlags(t *testing.T) {
712691
err = flagSet.Set("customStringFlagUserDefined", "changed-test-flag-value")
713692
g.Expect(err).To(Not(HaveOccurred()))
714693

715-
customStringFlagNoDefaultValueUnset := namespacedNameValue{
716-
value: types.NamespacedName{},
717-
}
718-
flagSet.Var(
719-
&customStringFlagNoDefaultValueUnset,
720-
"customStringFlagNoDefaultValueUnset",
721-
"no default value custom string test flag",
722-
)
723-
724-
customStringFlagNoDefaultValueUserDefined := namespacedNameValue{
725-
value: types.NamespacedName{},
726-
}
727-
flagSet.Var(
728-
&customStringFlagNoDefaultValueUserDefined,
729-
"customStringFlagNoDefaultValueUserDefined",
730-
"no default value but with user defined namespacedName test flag",
731-
)
732-
userDefinedNamespacedName := types.NamespacedName{
733-
Namespace: "changed-namespace",
734-
Name: "changed-name",
735-
}
736-
err = flagSet.Set("customStringFlagNoDefaultValueUserDefined", userDefinedNamespacedName.String())
737-
g.Expect(err).To(Not(HaveOccurred()))
738-
739694
expectedKeys := []string{
740695
"boolFlagTrue",
741696
"boolFlagFalse",
@@ -745,9 +700,6 @@ func TestParseFlags(t *testing.T) {
745700

746701
"customStringFlagDefault",
747702
"customStringFlagUserDefined",
748-
749-
"customStringFlagNoDefaultValueUnset",
750-
"customStringFlagNoDefaultValueUserDefined",
751703
}
752704
expectedValues := []string{
753705
"true",
@@ -758,9 +710,6 @@ func TestParseFlags(t *testing.T) {
758710

759711
"default",
760712
"user-defined",
761-
762-
"default",
763-
"user-defined",
764713
}
765714

766715
flagKeys, flagValues := parseFlags(flagSet)

Diff for: ‎cmd/gateway/validating_types.go

-30
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"fmt"
77
"strconv"
88
"strings"
9-
10-
"k8s.io/apimachinery/pkg/types"
119
)
1210

1311
// stringValidatingValue is a string flag value with custom validation logic.
@@ -106,31 +104,3 @@ func (v *intValidatingValue) Set(param string) error {
106104
func (v *intValidatingValue) Type() string {
107105
return "int"
108106
}
109-
110-
// namespacedNameValue is a string flag value that represents a namespaced name.
111-
// it implements the pflag.Value interface.
112-
type namespacedNameValue struct {
113-
value types.NamespacedName
114-
}
115-
116-
func (v *namespacedNameValue) String() string {
117-
if (v.value == types.NamespacedName{}) {
118-
// if we don't do that, the default value in the help message will be printed as "/"
119-
return ""
120-
}
121-
return v.value.String()
122-
}
123-
124-
func (v *namespacedNameValue) Set(param string) error {
125-
nsname, err := parseNamespacedResourceName(param)
126-
if err != nil {
127-
return err
128-
}
129-
130-
v.value = nsname
131-
return nil
132-
}
133-
134-
func (v *namespacedNameValue) Type() string {
135-
return "string"
136-
}

Diff for: ‎cmd/gateway/validation.go

-35
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"strconv"
99
"strings"
1010

11-
"k8s.io/apimachinery/pkg/types"
1211
"k8s.io/apimachinery/pkg/util/validation"
1312
)
1413

@@ -55,40 +54,6 @@ func validateResourceName(value string) error {
5554
return nil
5655
}
5756

58-
func validateNamespaceName(value string) error {
59-
// used by Kubernetes to validate resource namespace names
60-
messages := validation.IsDNS1123Label(value)
61-
if len(messages) > 0 {
62-
msg := strings.Join(messages, "; ")
63-
return fmt.Errorf("invalid format: %s", msg)
64-
}
65-
66-
return nil
67-
}
68-
69-
func parseNamespacedResourceName(value string) (types.NamespacedName, error) {
70-
if value == "" {
71-
return types.NamespacedName{}, errors.New("must be set")
72-
}
73-
74-
parts := strings.Split(value, "/")
75-
if len(parts) != 2 {
76-
return types.NamespacedName{}, errors.New("invalid format; must be NAMESPACE/NAME")
77-
}
78-
79-
if err := validateNamespaceName(parts[0]); err != nil {
80-
return types.NamespacedName{}, fmt.Errorf("invalid namespace name: %w", err)
81-
}
82-
if err := validateResourceName(parts[1]); err != nil {
83-
return types.NamespacedName{}, fmt.Errorf("invalid resource name: %w", err)
84-
}
85-
86-
return types.NamespacedName{
87-
Namespace: parts[0],
88-
Name: parts[1],
89-
}, nil
90-
}
91-
9257
func validateQualifiedName(name string) error {
9358
if len(name) == 0 {
9459
return errors.New("must be set")

Diff for: ‎cmd/gateway/validation_test.go

-132
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"testing"
55

66
. "github.com/onsi/gomega"
7-
"k8s.io/apimachinery/pkg/types"
87
)
98

109
func TestValidateGatewayControllerName(t *testing.T) {
@@ -132,137 +131,6 @@ func TestValidateResourceName(t *testing.T) {
132131
}
133132
}
134133

135-
func TestValidateNamespaceName(t *testing.T) {
136-
t.Parallel()
137-
tests := []struct {
138-
name string
139-
value string
140-
expErr bool
141-
}{
142-
{
143-
name: "valid",
144-
value: "mynamespace",
145-
expErr: false,
146-
},
147-
{
148-
name: "valid - with dash",
149-
value: "my-namespace",
150-
expErr: false,
151-
},
152-
{
153-
name: "valid - with numbers",
154-
value: "mynamespace123",
155-
expErr: false,
156-
},
157-
{
158-
name: "invalid - empty",
159-
value: "",
160-
expErr: true,
161-
},
162-
{
163-
name: "invalid - invalid character '.'",
164-
value: "my.namespace",
165-
expErr: true,
166-
},
167-
{
168-
name: "invalid - invalid character '/'",
169-
value: "my/namespace",
170-
expErr: true,
171-
},
172-
{
173-
name: "invalid - invalid character '_'",
174-
value: "my_namespace",
175-
expErr: true,
176-
},
177-
{
178-
name: "invalid - invalid character '@'",
179-
value: "my@namespace",
180-
expErr: true,
181-
},
182-
}
183-
184-
for _, test := range tests {
185-
t.Run(test.name, func(t *testing.T) {
186-
t.Parallel()
187-
g := NewWithT(t)
188-
189-
err := validateNamespaceName(test.value)
190-
191-
if test.expErr {
192-
g.Expect(err).To(HaveOccurred())
193-
} else {
194-
g.Expect(err).ToNot(HaveOccurred())
195-
}
196-
})
197-
}
198-
}
199-
200-
func TestParseNamespacedResourceName(t *testing.T) {
201-
t.Parallel()
202-
tests := []struct {
203-
name string
204-
value string
205-
expectedErrPrefix string
206-
expectedNsName types.NamespacedName
207-
expectErr bool
208-
}{
209-
{
210-
name: "valid",
211-
value: "test/my-gateway",
212-
expectedNsName: types.NamespacedName{
213-
Namespace: "test",
214-
Name: "my-gateway",
215-
},
216-
expectErr: false,
217-
},
218-
{
219-
name: "empty",
220-
value: "",
221-
expectedNsName: types.NamespacedName{},
222-
expectErr: true,
223-
expectedErrPrefix: "must be set",
224-
},
225-
{
226-
name: "wrong number of parts",
227-
value: "test",
228-
expectedNsName: types.NamespacedName{},
229-
expectErr: true,
230-
expectedErrPrefix: "invalid format; must be NAMESPACE/NAME",
231-
},
232-
{
233-
name: "invalid namespace",
234-
value: "t@st/my-gateway",
235-
expectedNsName: types.NamespacedName{},
236-
expectErr: true,
237-
expectedErrPrefix: "invalid namespace name",
238-
},
239-
{
240-
name: "invalid name",
241-
value: "test/my-g@teway",
242-
expectedNsName: types.NamespacedName{},
243-
expectErr: true,
244-
expectedErrPrefix: "invalid resource name",
245-
},
246-
}
247-
248-
for _, test := range tests {
249-
t.Run(test.name, func(t *testing.T) {
250-
t.Parallel()
251-
g := NewWithT(t)
252-
253-
nsName, err := parseNamespacedResourceName(test.value)
254-
255-
if test.expectErr {
256-
g.Expect(err).To(HaveOccurred())
257-
g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix))
258-
} else {
259-
g.Expect(err).ToNot(HaveOccurred())
260-
g.Expect(nsName).To(Equal(test.expectedNsName))
261-
}
262-
})
263-
}
264-
}
265-
266134
func TestValidateQualifiedName(t *testing.T) {
267135
t.Parallel()
268136
tests := []struct {

0 commit comments

Comments
 (0)