Skip to content

Commit 01d4670

Browse files
salonichf5salonichf5
committed
updates based on reviews
1 parent 7da79b9 commit 01d4670

File tree

13 files changed

+130
-130
lines changed

13 files changed

+130
-130
lines changed

‎apis/v1alpha1/nginxproxy_types.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@ import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1010

1111
// NginxProxy is a configuration object that is attached to a GatewayClass parametersRef. It provides a way
1212
// to configure global settings for all Gateways defined from the GatewayClass.
13-
type NginxProxy struct {
14-
metav1.TypeMeta `json:",inline"`
13+
type NginxProxy struct { //nolint:govet // standard field alignment, don't change it
1514
metav1.ObjectMeta `json:"metadata,omitempty"`
1615

1716
// Spec defines the desired state of the NginxProxy.
1817
Spec NginxProxySpec `json:"spec"`
18+
19+
metav1.TypeMeta `json:",inline"`
1920
}
2021

2122
// +kubebuilder:object:root=true
@@ -43,15 +44,15 @@ const (
4344

4445
// NginxProxySpec defines the desired state of the NginxProxy.
4546
type NginxProxySpec struct {
46-
// Telemetry specifies the OpenTelemetry configuration.
47-
//
48-
// +optional
49-
Telemetry *Telemetry `json:"telemetry,omitempty"`
5047
// IPFamily specifies the IP family to be used by the server.
5148
// Default is "dual", meaning the server will use both IPv4 and IPv6.
5249
//
5350
// +optional
54-
IPFamily IPFamilyType `json:"ipFamily,omitempty"`
51+
IPFamily *IPFamilyType `json:"ipFamily,omitempty"`
52+
// Telemetry specifies the OpenTelemetry configuration.
53+
//
54+
// +optional
55+
Telemetry *Telemetry `json:"telemetry,omitempty"`
5556
// DisableHTTP2 defines if http2 should be disabled for all servers.
5657
// Default is false, meaning http2 will be enabled for all servers.
5758
//

‎apis/v1alpha1/zz_generated.deepcopy.go

+6-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/mode/static/nginx/config/http/config.go

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ type Server struct {
1010
IsDefaultHTTP bool
1111
IsDefaultSSL bool
1212
GRPC bool
13-
IPFamily IPFamily
1413
}
1514

1615
// IPFamily holds the IP family configuration for all servers.

‎internal/mode/static/nginx/config/servers.go

+19-26
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"strings"
99
gotemplate "text/template"
1010

11-
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
1211
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
1312
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
1413
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
@@ -58,13 +57,23 @@ var grpcBaseHeaders = []http.Header{
5857
},
5958
}
6059

60+
type serverConfig struct {
61+
Servers []http.Server
62+
IPFamily http.IPFamily
63+
}
64+
6165
func executeServers(conf dataplane.Configuration) []executeResult {
6266
ipFamily := getIPFamily(conf.BaseHTTPConfig)
63-
servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers, ipFamily)
67+
servers, httpMatchPairs := createServers(conf.HTTPServers, conf.SSLServers)
68+
69+
serverConfig := serverConfig{
70+
Servers: servers,
71+
IPFamily: ipFamily,
72+
}
6473

6574
serverResult := executeResult{
6675
dest: httpConfigFile,
67-
data: helpers.MustExecuteTemplate(serversTemplate, servers),
76+
data: helpers.MustExecuteTemplate(serversTemplate, serverConfig),
6877
}
6978

7079
// create httpMatchPair conf
@@ -91,9 +100,9 @@ func executeServers(conf dataplane.Configuration) []executeResult {
91100
// getIPFamily returns whether the server should be configured for IPv4, IPv6, or both.
92101
func getIPFamily(baseHTTPConfig dataplane.BaseHTTPConfig) http.IPFamily {
93102
switch ip := baseHTTPConfig.IPFamily; ip {
94-
case ngfAPI.IPv4:
103+
case dataplane.IPv4:
95104
return http.IPFamily{IPv4: true}
96-
case ngfAPI.IPv6:
105+
case dataplane.IPv6:
97106
return http.IPFamily{IPv6: true}
98107
}
99108

@@ -155,39 +164,30 @@ func createIncludes(additions []dataplane.Addition) []string {
155164
return includes
156165
}
157166

158-
func createServers(
159-
httpServers,
160-
sslServers []dataplane.VirtualServer,
161-
ipFamily http.IPFamily,
162-
) ([]http.Server, httpMatchPairs) {
167+
func createServers(httpServers, sslServers []dataplane.VirtualServer) ([]http.Server, httpMatchPairs) {
163168
servers := make([]http.Server, 0, len(httpServers)+len(sslServers))
164169
finalMatchPairs := make(httpMatchPairs)
165170

166171
for serverID, s := range httpServers {
167-
httpServer, matchPairs := createServer(s, serverID, ipFamily)
172+
httpServer, matchPairs := createServer(s, serverID)
168173
servers = append(servers, httpServer)
169174
maps.Copy(finalMatchPairs, matchPairs)
170175
}
171176

172177
for serverID, s := range sslServers {
173-
sslServer, matchPair := createSSLServer(s, serverID, ipFamily)
178+
sslServer, matchPair := createSSLServer(s, serverID)
174179
servers = append(servers, sslServer)
175180
maps.Copy(finalMatchPairs, matchPair)
176181
}
177182

178183
return servers, finalMatchPairs
179184
}
180185

181-
func createSSLServer(
182-
virtualServer dataplane.VirtualServer,
183-
serverID int,
184-
ipFamily http.IPFamily,
185-
) (http.Server, httpMatchPairs) {
186+
func createSSLServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) {
186187
if virtualServer.IsDefault {
187188
return http.Server{
188189
IsDefaultSSL: true,
189190
Port: virtualServer.Port,
190-
IPFamily: ipFamily,
191191
}, nil
192192
}
193193

@@ -203,20 +203,14 @@ func createSSLServer(
203203
Port: virtualServer.Port,
204204
GRPC: grpc,
205205
Includes: createIncludes(virtualServer.Additions),
206-
IPFamily: ipFamily,
207206
}, matchPairs
208207
}
209208

210-
func createServer(
211-
virtualServer dataplane.VirtualServer,
212-
serverID int,
213-
ipFamily http.IPFamily,
214-
) (http.Server, httpMatchPairs) {
209+
func createServer(virtualServer dataplane.VirtualServer, serverID int) (http.Server, httpMatchPairs) {
215210
if virtualServer.IsDefault {
216211
return http.Server{
217212
IsDefaultHTTP: true,
218213
Port: virtualServer.Port,
219-
IPFamily: ipFamily,
220214
}, nil
221215
}
222216

@@ -228,7 +222,6 @@ func createServer(
228222
Port: virtualServer.Port,
229223
GRPC: grpc,
230224
Includes: createIncludes(virtualServer.Additions),
231-
IPFamily: ipFamily,
232225
}, matchPairs
233226
}
234227

‎internal/mode/static/nginx/config/servers_template.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,24 @@ package config
22

33
const serversTemplateText = `
44
js_preload_object matches from /etc/nginx/conf.d/matches.json;
5-
{{- range $s := . -}}
5+
{{- range $s := .Servers -}}
66
{{ if $s.IsDefaultSSL -}}
77
server {
8-
{{- if $s.IPFamily.IPv4 }}
8+
{{- if $.IPFamily.IPv4 }}
99
listen {{ $s.Port }} ssl default_server;
1010
{{- end }}
11-
{{- if $s.IPFamily.IPv6 }}
11+
{{- if $.IPFamily.IPv6 }}
1212
listen [::]:{{ $s.Port }} ssl default_server;
1313
{{- end }}
1414
1515
ssl_reject_handshake on;
1616
}
1717
{{- else if $s.IsDefaultHTTP }}
1818
server {
19-
{{- if $s.IPFamily.IPv4 }}
19+
{{- if $.IPFamily.IPv4 }}
2020
listen {{ $s.Port }} default_server;
2121
{{- end }}
22-
{{- if $s.IPFamily.IPv6 }}
22+
{{- if $.IPFamily.IPv6 }}
2323
listen [::]:{{ $s.Port }} default_server;
2424
{{- end }}
2525
@@ -29,10 +29,10 @@ server {
2929
{{- else }}
3030
server {
3131
{{- if $s.SSL }}
32-
{{- if $s.IPFamily.IPv4 }}
32+
{{- if $.IPFamily.IPv4 }}
3333
listen {{ $s.Port }} ssl;
3434
{{- end }}
35-
{{- if $s.IPFamily.IPv6 }}
35+
{{- if $.IPFamily.IPv6 }}
3636
listen [::]:{{ $s.Port }} ssl;
3737
{{- end }}
3838
ssl_certificate {{ $s.SSL.Certificate }};
@@ -42,10 +42,10 @@ server {
4242
return 421;
4343
}
4444
{{- else }}
45-
{{- if $s.IPFamily.IPv4 }}
45+
{{- if $.IPFamily.IPv4 }}
4646
listen {{ $s.Port }};
4747
{{- end }}
48-
{{- if $s.IPFamily.IPv6 }}
48+
{{- if $.IPFamily.IPv6 }}
4949
listen [::]:{{ $s.Port }};
5050
{{- end }}
5151
{{- end }}

‎internal/mode/static/nginx/config/servers_test.go

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

12-
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
1312
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
1413
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
1514
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
@@ -50,7 +49,7 @@ func TestExecuteServers(t *testing.T) {
5049
},
5150
},
5251
BaseHTTPConfig: dataplane.BaseHTTPConfig{
53-
IPFamily: ngfAPI.IPv4,
52+
IPFamily: dataplane.IPv4,
5453
},
5554
},
5655
expectedHTTPConfig: map[string]int{
@@ -145,7 +144,7 @@ func TestExecuteServers(t *testing.T) {
145144
},
146145
},
147146
BaseHTTPConfig: dataplane.BaseHTTPConfig{
148-
IPFamily: ngfAPI.Dual,
147+
IPFamily: dataplane.Dual,
149148
},
150149
},
151150
expectedHTTPConfig: map[string]int{
@@ -1092,7 +1091,6 @@ func TestCreateServers(t *testing.T) {
10921091
{
10931092
IsDefaultHTTP: true,
10941093
Port: 8080,
1095-
IPFamily: http.IPFamily{},
10961094
},
10971095
{
10981096
ServerName: "cafe.example.com",
@@ -1103,12 +1101,10 @@ func TestCreateServers(t *testing.T) {
11031101
includesFolder + "/server-addition-1.conf",
11041102
includesFolder + "/server-addition-2.conf",
11051103
},
1106-
IPFamily: http.IPFamily{},
11071104
},
11081105
{
11091106
IsDefaultSSL: true,
11101107
Port: 8443,
1111-
IPFamily: http.IPFamily{},
11121108
},
11131109
{
11141110
ServerName: "cafe.example.com",
@@ -1123,13 +1119,12 @@ func TestCreateServers(t *testing.T) {
11231119
includesFolder + "/server-addition-1.conf",
11241120
includesFolder + "/server-addition-3.conf",
11251121
},
1126-
IPFamily: http.IPFamily{},
11271122
},
11281123
}
11291124

11301125
g := NewWithT(t)
11311126

1132-
result, httpMatchPair := createServers(httpServers, sslServers, http.IPFamily{})
1127+
result, httpMatchPair := createServers(httpServers, sslServers)
11331128

11341129
g.Expect(httpMatchPair).To(Equal(allExpMatchPair))
11351130
g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty())
@@ -1338,7 +1333,7 @@ func TestCreateServersConflicts(t *testing.T) {
13381333

13391334
g := NewWithT(t)
13401335

1341-
result, _ := createServers(httpServers, []dataplane.VirtualServer{}, http.IPFamily{})
1336+
result, _ := createServers(httpServers, []dataplane.VirtualServer{})
13421337
g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty())
13431338
})
13441339
}

‎internal/mode/static/nginx/config/upstreams.go

+5-6
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,12 @@ func (g GeneratorImpl) createUpstream(up dataplane.Upstream) http.Upstream {
6868

6969
upstreamServers := make([]http.UpstreamServer, len(up.Endpoints))
7070
for idx, ep := range up.Endpoints {
71-
upstreamServers[idx] = http.UpstreamServer{
72-
Address: fmt.Sprintf("%s:%d", ep.Address, ep.Port),
73-
}
71+
format := "%s:%d"
7472
if ep.IPv6 {
75-
upstreamServers[idx] = http.UpstreamServer{
76-
Address: fmt.Sprintf("[%s]:%d", ep.Address, ep.Port),
77-
}
73+
format = "[%s]:%d"
74+
}
75+
upstreamServers[idx] = http.UpstreamServer{
76+
Address: fmt.Sprintf(format, ep.Address, ep.Port),
7877
}
7978
}
8079

‎internal/mode/static/state/dataplane/configuration.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig {
659659
baseConfig := BaseHTTPConfig{
660660
// HTTP2 should be enabled by default
661661
HTTP2: true,
662-
IPFamily: ngfAPI.Dual,
662+
IPFamily: Dual,
663663
}
664664
if g.NginxProxy == nil || !g.NginxProxy.Valid {
665665
return baseConfig
@@ -669,11 +669,11 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig {
669669
baseConfig.HTTP2 = false
670670
}
671671

672-
switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; ipFamily {
672+
switch ipFamily := g.NginxProxy.Source.Spec.IPFamily; *ipFamily {
673673
case ngfAPI.IPv4:
674-
baseConfig.IPFamily = ngfAPI.IPv4
674+
baseConfig.IPFamily = IPv4
675675
case ngfAPI.IPv6:
676-
baseConfig.IPFamily = ngfAPI.IPv6
676+
baseConfig.IPFamily = IPv6
677677
}
678678

679679
return baseConfig

0 commit comments

Comments
 (0)