Skip to content

Commit 717da40

Browse files
pleshakovpleshakov
authored
Make static mode compatible with provisioner (#657)
This commit adds two features for the static mode: - Support configuring a single Gateway resource to watch. - Support not reporting the GatewayClass status, so that it doesn't conflict with the status updates done by the provisioner. Needed by #634
1 parent 3d6b009 commit 717da40

18 files changed

+615
-147
lines changed

‎cmd/gateway/commands.go

+67-6
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os"
77

88
"github.com/spf13/cobra"
9+
"k8s.io/apimachinery/pkg/types"
910
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1011
"sigs.k8s.io/controller-runtime/pkg/log/zap"
1112

@@ -36,7 +37,7 @@ var (
3637
)
3738

3839
// stringValidatingValue is a string flag value with custom validation logic.
39-
// stringValidatingValue implements the pflag.Value interface.
40+
// it implements the pflag.Value interface.
4041
type stringValidatingValue struct {
4142
validator func(v string) error
4243
value string
@@ -58,6 +59,34 @@ func (v *stringValidatingValue) Type() string {
5859
return "string"
5960
}
6061

62+
// namespacedNameValue is a string flag value that represents a namespaced name.
63+
// it implements the pflag.Value interface.
64+
type namespacedNameValue struct {
65+
value types.NamespacedName
66+
}
67+
68+
func (v *namespacedNameValue) String() string {
69+
if (v.value == types.NamespacedName{}) {
70+
// if we don't do that, the default value in the help message will be printed as "/"
71+
return ""
72+
}
73+
return v.value.String()
74+
}
75+
76+
func (v *namespacedNameValue) Set(param string) error {
77+
nsname, err := parseNamespacedResourceName(param)
78+
if err != nil {
79+
return err
80+
}
81+
82+
v.value = nsname
83+
return nil
84+
}
85+
86+
func (v *namespacedNameValue) Type() string {
87+
return "string"
88+
}
89+
6190
func createRootCommand() *cobra.Command {
6291
rootCmd := &cobra.Command{
6392
Use: "gateway",
@@ -84,7 +113,13 @@ func createRootCommand() *cobra.Command {
84113
}
85114

86115
func createStaticModeCommand() *cobra.Command {
87-
return &cobra.Command{
116+
const gatewayFlag = "gateway"
117+
118+
// flag values
119+
gateway := namespacedNameValue{}
120+
var updateGCStatus bool
121+
122+
cmd := &cobra.Command{
88123
Use: "static-mode",
89124
Short: "Configure NGINX in the scope of a single Gateway resource",
90125
RunE: func(cmd *cobra.Command, args []string) error {
@@ -100,11 +135,18 @@ func createStaticModeCommand() *cobra.Command {
100135
return fmt.Errorf("error validating POD_IP environment variable: %w", err)
101136
}
102137

138+
var gwNsName *types.NamespacedName
139+
if cmd.Flags().Changed(gatewayFlag) {
140+
gwNsName = &gateway.value
141+
}
142+
103143
conf := config.Config{
104-
GatewayCtlrName: gatewayCtlrName.value,
105-
Logger: logger,
106-
GatewayClassName: gatewayClassName.value,
107-
PodIP: podIP,
144+
GatewayCtlrName: gatewayCtlrName.value,
145+
Logger: logger,
146+
GatewayClassName: gatewayClassName.value,
147+
PodIP: podIP,
148+
GatewayNsName: gwNsName,
149+
UpdateGatewayClassStatus: updateGCStatus,
108150
}
109151

110152
if err := manager.Start(conf); err != nil {
@@ -114,6 +156,25 @@ func createStaticModeCommand() *cobra.Command {
114156
return nil
115157
},
116158
}
159+
160+
cmd.Flags().Var(
161+
&gateway,
162+
gatewayFlag,
163+
"The namespaced name of the Gateway resource to use. "+
164+
"Must be of the form: NAMESPACE/NAME. "+
165+
"If not specified, the control plane will process all Gateways for the configured GatewayClass. "+
166+
"However, among them, it will choose the oldest resource by creation timestamp. If the timestamps are "+
167+
"equal, it will choose the resource that appears first in alphabetical order by {namespace}/{name}.",
168+
)
169+
170+
cmd.Flags().BoolVar(
171+
&updateGCStatus,
172+
"update-gatewayclass-status",
173+
true,
174+
"Update the status of the GatewayClass resource.",
175+
)
176+
177+
return cmd
117178
}
118179

119180
func createProvisionerModeCommand() *cobra.Command {

‎cmd/gateway/commands_test.go

+88-19
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,40 @@ import (
55
"testing"
66

77
. "github.com/onsi/gomega"
8+
"github.com/spf13/cobra"
89
)
910

11+
type flagTestCase struct {
12+
name string
13+
expectedErrPrefix string
14+
args []string
15+
wantErr bool
16+
}
17+
18+
func testFlag(t *testing.T, cmd *cobra.Command, test flagTestCase) {
19+
g := NewGomegaWithT(t)
20+
// discard any output generated by cobra
21+
cmd.SetOut(io.Discard)
22+
cmd.SetErr(io.Discard)
23+
24+
// override RunE to avoid executing the command
25+
cmd.RunE = func(cmd *cobra.Command, args []string) error {
26+
return nil
27+
}
28+
29+
cmd.SetArgs(test.args)
30+
err := cmd.Execute()
31+
32+
if test.wantErr {
33+
g.Expect(err).To(HaveOccurred())
34+
g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix))
35+
} else {
36+
g.Expect(err).NotTo(HaveOccurred())
37+
}
38+
}
39+
1040
func TestRootCmdFlagValidation(t *testing.T) {
11-
tests := []struct {
12-
name string
13-
expectedErrPrefix string
14-
args []string
15-
wantErr bool
16-
}{
41+
tests := []flagTestCase{
1742
{
1843
name: "valid flags",
1944
args: []string{
@@ -79,22 +104,66 @@ func TestRootCmdFlagValidation(t *testing.T) {
79104

80105
for _, test := range tests {
81106
t.Run(test.name, func(t *testing.T) {
82-
g := NewGomegaWithT(t)
83-
84107
rootCmd := createRootCommand()
85-
// discard any output generated by cobra
86-
rootCmd.SetOut(io.Discard)
87-
rootCmd.SetErr(io.Discard)
108+
testFlag(t, rootCmd, test)
109+
})
110+
}
111+
}
88112

89-
rootCmd.SetArgs(test.args)
90-
err := rootCmd.Execute()
113+
func TestStaticModeCmdFlagValidation(t *testing.T) {
114+
tests := []flagTestCase{
115+
{
116+
name: "valid flags",
117+
args: []string{
118+
"--gateway=nginx-gateway/nginx",
119+
"--update-gatewayclass-status=true",
120+
},
121+
wantErr: false,
122+
},
123+
{
124+
name: "valid flags, not set",
125+
args: nil,
126+
wantErr: false,
127+
},
128+
{
129+
name: "gateway is set to empty string",
130+
args: []string{
131+
"--gateway=",
132+
},
133+
wantErr: true,
134+
expectedErrPrefix: `invalid argument "" for "--gateway" flag: must be set`,
135+
},
136+
{
137+
name: "gateway is invalid",
138+
args: []string{
139+
"--gateway=nginx-gateway", // no namespace
140+
},
141+
wantErr: true,
142+
expectedErrPrefix: `invalid argument "nginx-gateway" for "--gateway" flag: invalid format; ` +
143+
"must be NAMESPACE/NAME",
144+
},
145+
{
146+
name: "update-gatewayclass-status is set to empty string",
147+
args: []string{
148+
"--update-gatewayclass-status=",
149+
},
150+
wantErr: true,
151+
expectedErrPrefix: `invalid argument "" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
152+
},
153+
{
154+
name: "update-gatewayclass-status is invalid",
155+
args: []string{
156+
"--update-gatewayclass-status=invalid", // not a boolean
157+
},
158+
wantErr: true,
159+
expectedErrPrefix: `invalid argument "invalid" for "--update-gatewayclass-status" flag: strconv.ParseBool`,
160+
},
161+
}
91162

92-
if test.wantErr {
93-
g.Expect(err).To(HaveOccurred())
94-
g.Expect(err.Error()).To(HavePrefix(test.expectedErrPrefix))
95-
} else {
96-
g.Expect(err).ToNot(HaveOccurred())
97-
}
163+
for _, test := range tests {
164+
t.Run(test.name, func(t *testing.T) {
165+
cmd := createStaticModeCommand()
166+
testFlag(t, cmd, test)
98167
})
99168
}
100169
}

‎cmd/gateway/validation.go

+35
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"regexp"
88
"strings"
99

10+
"k8s.io/apimachinery/pkg/types"
1011
"k8s.io/apimachinery/pkg/util/validation"
1112
)
1213

@@ -54,6 +55,40 @@ func validateResourceName(value string) error {
5455
return nil
5556
}
5657

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+
5792
func validateIP(ip string) error {
5893
if ip == "" {
5994
return errors.New("IP address must be set")

0 commit comments

Comments
 (0)