Skip to content

Commit

Permalink
Do not generate nginx plus template code when running on oss
Browse files Browse the repository at this point in the history
  • Loading branch information
@bjee19
bjee19 committed Jan 30, 2025
1 parent 19d3ea0 commit 05ab72d
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 33 deletions.
4 changes: 2 additions & 2 deletions internal/mode/static/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log
return
case state.EndpointsOnlyChange:
h.version++
cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version)
cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version, h.cfg.plus)
depCtx, getErr := h.getDeploymentContext(ctx)
if getErr != nil {
logger.Error(getErr, "error getting deployment context for usage reporting")
Expand All @@ -190,7 +190,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log
}
case state.ClusterStateChange:
h.version++
cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version)
cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version, h.cfg.plus)
depCtx, getErr := h.getDeploymentContext(ctx)
if getErr != nil {
logger.Error(getErr, "error getting deployment context for usage reporting")
Expand Down
1 change: 1 addition & 0 deletions internal/mode/static/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ var _ = Describe("eventHandler", func() {
handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch)

dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 1)
dcfg.NginxPlus = dataplane.NginxPlus{AllowedAddresses: []string{"127.0.0.1"}}
Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty())

Expect(fakeGenerator.GenerateCallCount()).To(Equal(0))
Expand Down
8 changes: 7 additions & 1 deletion internal/mode/static/nginx/config/plus_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@ var plusAPITemplate = gotemplate.Must(gotemplate.New("plusAPI").Parse(plusAPITem
func executePlusAPI(conf dataplane.Configuration) []executeResult {
result := executeResult{
dest: nginxPlusConfigFile,
data: helpers.MustExecuteTemplate(plusAPITemplate, conf.NginxPlus),
}
// if AllowedAddresses is empty, it means that we are not running on nginx plus, and we don't want this generated
if conf.NginxPlus.AllowedAddresses != nil {
result = executeResult{
dest: nginxPlusConfigFile,
data: helpers.MustExecuteTemplate(plusAPITemplate, conf.NginxPlus),
}
}

return []executeResult{result}
Expand Down
26 changes: 26 additions & 0 deletions internal/mode/static/nginx/config/plus_api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,29 @@ func TestExecutePlusAPI(t *testing.T) {
g.Expect(expCount).To(Equal(strings.Count(string(res[0].data), expSubStr)))
}
}

func TestExecutePlusAPI_EmptyNginxPlus(t *testing.T) {
t.Parallel()
conf := dataplane.Configuration{
NginxPlus: dataplane.NginxPlus{},
}

g := NewWithT(t)
expSubStrings := map[string]int{
"listen unix:/var/run/nginx/nginx-plus-api.sock;": 0,
"access_log off;": 0,
"api write=on;": 0,
"listen 8765;": 0,
"root /usr/share/nginx/html;": 0,
"allow 127.0.0.1;": 0,
"deny all;": 0,
"location = /dashboard.html {}": 0,
"api write=off;": 0,
}

for expSubStr, expCount := range expSubStrings {
res := executePlusAPI(conf)
g.Expect(res).To(HaveLen(1))
g.Expect(expCount).To(Equal(strings.Count(string(res[0].data), expSubStr)))
}
}
17 changes: 14 additions & 3 deletions internal/mode/static/state/dataplane/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,15 @@ func BuildConfiguration(
g *graph.Graph,
serviceResolver resolver.ServiceResolver,
configVersion int,
plus bool,
) Configuration {
if g.GatewayClass == nil || !g.GatewayClass.Valid || g.Gateway == nil {
return GetDefaultConfiguration(g, configVersion)
config := GetDefaultConfiguration(g, configVersion)
if plus {
config.NginxPlus = buildNginxPlus(g)
}

return config
}

baseHTTPConfig := buildBaseHTTPConfig(g)
Expand All @@ -50,6 +56,11 @@ func BuildConfiguration(
baseHTTPConfig.IPFamily,
)

var nginxPlus NginxPlus
if plus {
nginxPlus = buildNginxPlus(g)
}

config := Configuration{
HTTPServers: httpServers,
SSLServers: sslServers,
Expand All @@ -63,7 +74,7 @@ func BuildConfiguration(
Telemetry: buildTelemetry(g),
BaseHTTPConfig: baseHTTPConfig,
Logging: buildLogging(g),
NginxPlus: buildNginxPlus(g),
NginxPlus: nginxPlus,
MainSnippets: buildSnippetsForContext(g.SnippetsFilters, ngfAPIv1alpha1.NginxContextMain),
AuxiliarySecrets: buildAuxiliarySecrets(g.PlusSecrets),
}
Expand Down Expand Up @@ -1009,7 +1020,7 @@ func GetDefaultConfiguration(g *graph.Graph, configVersion int) Configuration {
return Configuration{
Version: configVersion,
Logging: buildLogging(g),
NginxPlus: buildNginxPlus(g),
NginxPlus: NginxPlus{},
AuxiliarySecrets: buildAuxiliarySecrets(g.PlusSecrets),
}
}
149 changes: 122 additions & 27 deletions internal/mode/static/state/dataplane/configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ func getExpectedConfiguration() Configuration {
Logging: Logging{
ErrorLevel: defaultErrorLogLevel,
},
NginxPlus: NginxPlus{
AllowedAddresses: []string{"127.0.0.1"},
},
NginxPlus: NginxPlus{},
}
}

Expand Down Expand Up @@ -870,7 +868,7 @@ func TestBuildConfiguration(t *testing.T) {

defaultConfig := Configuration{
Logging: Logging{ErrorLevel: defaultErrorLogLevel},
NginxPlus: NginxPlus{AllowedAddresses: []string{"127.0.0.1"}},
NginxPlus: NginxPlus{},
}

tests := []struct {
Expand Down Expand Up @@ -1559,36 +1557,14 @@ func TestBuildConfiguration(t *testing.T) {
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.GatewayClass.Valid = false
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
},
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
}
return g
}),
expConf: defaultConfig,
msg: "invalid gatewayclass",
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.GatewayClass.Valid = false
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
},
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
}
g.GatewayClass = nil
return g
}),
expConf: defaultConfig,
Expand Down Expand Up @@ -2374,6 +2350,100 @@ func TestBuildConfiguration(t *testing.T) {
}),
msg: "SnippetsFilters with main and http snippet",
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{
Name: "gw",
Namespace: "ns",
}
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
})
g.NginxProxy = &graph.NginxProxy{
Valid: true,
Source: &ngfAPIv1alpha1.NginxProxy{
Spec: ngfAPIv1alpha1.NginxProxySpec{
NginxPlus: &ngfAPIv1alpha1.NginxPlus{
AllowedAddresses: []ngfAPIv1alpha1.NginxPlusAllowAddress{
{Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "127.0.0.3"},
{Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "25.0.0.3"},
},
},
},
},
}
return g
}),
expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration {
conf.SSLServers = []VirtualServer{}
conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{}
return conf
}),
msg: "NginxProxy with NginxPlus allowed addresses configured but running on nginx oss",
},
}

for _, test := range tests {
t.Run(test.msg, func(t *testing.T) {
t.Parallel()
g := NewWithT(t)

result := BuildConfiguration(
context.TODO(),
test.graph,
fakeResolver,
1,
false,
)

g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups))
g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams))
g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers))
g.Expect(result.SSLServers).To(ConsistOf(test.expConf.SSLServers))
g.Expect(result.TLSPassthroughServers).To(ConsistOf(test.expConf.TLSPassthroughServers))
g.Expect(result.SSLKeyPairs).To(Equal(test.expConf.SSLKeyPairs))
g.Expect(result.Version).To(Equal(1))
g.Expect(result.CertBundles).To(Equal(test.expConf.CertBundles))
g.Expect(result.Telemetry).To(Equal(test.expConf.Telemetry))
g.Expect(result.BaseHTTPConfig).To(Equal(test.expConf.BaseHTTPConfig))
g.Expect(result.Logging).To(Equal(test.expConf.Logging))
g.Expect(result.NginxPlus).To(Equal(test.expConf.NginxPlus))
})
}
}

func TestBuildConfiguration_Plus(t *testing.T) {
t.Parallel()
fooEndpoints := []resolver.Endpoint{
{
Address: "10.0.0.0",
Port: 8080,
},
}

fakeResolver := &resolverfakes.FakeServiceResolver{}
fakeResolver.ResolveReturns(fooEndpoints, nil)

listener80 := v1.Listener{
Name: "listener-80-1",
Hostname: nil,
Port: 80,
Protocol: v1.HTTPProtocolType,
}

defaultPlusConfig := Configuration{
Logging: Logging{ErrorLevel: defaultErrorLogLevel},
NginxPlus: NginxPlus{AllowedAddresses: []string{"127.0.0.1"}},
}

tests := []struct {
graph *graph.Graph
msg string
expConf Configuration
}{
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{
Expand Down Expand Up @@ -2409,6 +2479,30 @@ func TestBuildConfiguration(t *testing.T) {
}),
msg: "NginxProxy with NginxPlus allowed addresses configured",
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.GatewayClass.Valid = false
return g
}),
expConf: defaultPlusConfig,
msg: "invalid gatewayclass",
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.GatewayClass = nil
return g
}),
expConf: defaultPlusConfig,
msg: "missing gatewayclass",
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway = nil
return g
}),
expConf: defaultPlusConfig,
msg: "missing gateway",
},
}

for _, test := range tests {
Expand All @@ -2421,6 +2515,7 @@ func TestBuildConfiguration(t *testing.T) {
test.graph,
fakeResolver,
1,
true,
)

g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups))
Expand Down

0 comments on commit 05ab72d

Please sign in to comment.