Skip to content

Commit 716b088

Browse files
author
Kate Osborn
committed
Validate in updateControlPlane
1 parent fec0736 commit 716b088

File tree

2 files changed

+155
-5
lines changed

2 files changed

+155
-5
lines changed

internal/mode/static/config_updater.go

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,18 +58,35 @@ func updateControlPlane(
5858
)
5959
}
6060

61-
// set the log level
62-
level := string(*controlConfig.Logging.Level)
63-
if err := logLevelSetter.SetLevel(level); err != nil {
64-
fieldErr := field.NotSupported(
61+
level := *controlConfig.Logging.Level
62+
63+
if err := validateLogLevel(level); err != nil {
64+
return err
65+
}
66+
67+
if err := logLevelSetter.SetLevel(string(level)); err != nil {
68+
return field.Invalid(
69+
field.NewPath("logging.level"),
70+
level,
71+
err.Error(),
72+
)
73+
}
74+
75+
return nil
76+
}
77+
78+
func validateLogLevel(level ngfAPI.ControllerLogLevel) error {
79+
switch level {
80+
case ngfAPI.ControllerLogLevelInfo, ngfAPI.ControllerLogLevelDebug, ngfAPI.ControllerLogLevelError:
81+
default:
82+
return field.NotSupported(
6583
field.NewPath("logging.level"),
6684
level,
6785
[]string{
6886
string(ngfAPI.ControllerLogLevelInfo),
6987
string(ngfAPI.ControllerLogLevelDebug),
7088
string(ngfAPI.ControllerLogLevelError),
7189
})
72-
return fieldErr
7390
}
7491

7592
return nil
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
package static
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"testing"
7+
8+
. "github.com/onsi/gomega"
9+
"k8s.io/apimachinery/pkg/types"
10+
"k8s.io/client-go/tools/record"
11+
"sigs.k8s.io/controller-runtime/pkg/log/zap"
12+
13+
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
14+
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
15+
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/staticfakes"
16+
)
17+
18+
func TestUpdateControlPlane(t *testing.T) {
19+
debugLogCfg := &ngfAPI.NginxGateway{
20+
Spec: ngfAPI.NginxGatewaySpec{
21+
Logging: &ngfAPI.Logging{
22+
Level: helpers.GetPointer(ngfAPI.ControllerLogLevelDebug),
23+
},
24+
},
25+
}
26+
27+
invalidLevelConfig := &ngfAPI.NginxGateway{
28+
Spec: ngfAPI.NginxGatewaySpec{
29+
Logging: &ngfAPI.Logging{
30+
Level: helpers.GetPointer[ngfAPI.ControllerLogLevel]("invalid"),
31+
},
32+
},
33+
}
34+
35+
logger := zap.New()
36+
fakeEventRecorder := record.NewFakeRecorder(1)
37+
nsname := types.NamespacedName{Namespace: "test", Name: "test"}
38+
39+
tests := []struct {
40+
setLevelErr error
41+
nginxGateway *ngfAPI.NginxGateway
42+
name string
43+
expErrString string
44+
expSetLevelCallCount int
45+
expEvent bool
46+
}{
47+
{
48+
name: "change log level",
49+
nginxGateway: debugLogCfg,
50+
expSetLevelCallCount: 1,
51+
},
52+
{
53+
name: "invalid log level",
54+
nginxGateway: invalidLevelConfig,
55+
expErrString: `Unsupported value: "invalid"`,
56+
expSetLevelCallCount: 0,
57+
},
58+
{
59+
name: "nil NginxGateway",
60+
nginxGateway: nil,
61+
expEvent: true,
62+
expSetLevelCallCount: 1,
63+
},
64+
{
65+
name: "set log level fails",
66+
nginxGateway: debugLogCfg,
67+
setLevelErr: errors.New("set level failed"),
68+
expErrString: "set level failed",
69+
expSetLevelCallCount: 1,
70+
},
71+
}
72+
73+
for _, test := range tests {
74+
t.Run(test.name, func(t *testing.T) {
75+
g := NewWithT(t)
76+
77+
fakeLogSetter := &staticfakes.FakeLogLevelSetter{
78+
SetLevelStub: func(s string) error {
79+
return test.setLevelErr
80+
},
81+
}
82+
83+
err := updateControlPlane(test.nginxGateway, logger, fakeEventRecorder, nsname, fakeLogSetter)
84+
85+
if test.expErrString != "" {
86+
g.Expect(err).To(HaveOccurred())
87+
g.Expect(err.Error()).To(ContainSubstring(test.expErrString))
88+
} else {
89+
g.Expect(err).ToNot(HaveOccurred())
90+
}
91+
92+
if test.expEvent {
93+
g.Expect(fakeEventRecorder.Events).To(HaveLen(1))
94+
event := <-fakeEventRecorder.Events
95+
g.Expect(event).To(ContainSubstring("ResourceDeleted"))
96+
} else {
97+
g.Expect(fakeEventRecorder.Events).To(BeEmpty())
98+
}
99+
100+
g.Expect(fakeLogSetter.SetLevelCallCount()).To(Equal(test.expSetLevelCallCount))
101+
})
102+
}
103+
}
104+
105+
func TestValidateLogLevel(t *testing.T) {
106+
validLevels := []ngfAPI.ControllerLogLevel{
107+
ngfAPI.ControllerLogLevelError,
108+
ngfAPI.ControllerLogLevelInfo,
109+
ngfAPI.ControllerLogLevelDebug,
110+
}
111+
112+
invalidLevels := []ngfAPI.ControllerLogLevel{
113+
ngfAPI.ControllerLogLevel("invalid"),
114+
ngfAPI.ControllerLogLevel("high"),
115+
ngfAPI.ControllerLogLevel("warn"),
116+
}
117+
118+
for _, level := range validLevels {
119+
t.Run(fmt.Sprintf("valid level %q", level), func(t *testing.T) {
120+
g := NewWithT(t)
121+
122+
g.Expect(validateLogLevel(level)).To(Succeed())
123+
})
124+
}
125+
126+
for _, level := range invalidLevels {
127+
t.Run(fmt.Sprintf("invalid level %q", level), func(t *testing.T) {
128+
g := NewWithT(t)
129+
130+
g.Expect(validateLogLevel(level)).ToNot(Succeed())
131+
})
132+
}
133+
}

0 commit comments

Comments
 (0)