From 7da6982b90d5e41f9d359f7b05b839cda64703f1 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Tue, 6 Dec 2022 14:11:44 -0700 Subject: [PATCH 1/2] Generate default http server if http listener exists --- internal/nginx/config/generator_test.go | 11 ++++- internal/nginx/config/servers.go | 64 ++++++++++++------------- internal/nginx/config/servers_test.go | 34 ++++++++----- internal/state/change_processor_test.go | 51 +++++++++++++++++++- internal/state/configuration.go | 10 ++++ internal/state/configuration_test.go | 28 +++++++++-- 6 files changed, 150 insertions(+), 48 deletions(-) diff --git a/internal/nginx/config/generator_test.go b/internal/nginx/config/generator_test.go index 593d112f69..cfdc380457 100644 --- a/internal/nginx/config/generator_test.go +++ b/internal/nginx/config/generator_test.go @@ -24,13 +24,22 @@ func TestGenerate(t *testing.T) { conf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "example.com", }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "example.com", + SSL: &state.SSL{ + CertificatePath: "/etc/nginx/secrets/default", + }, }, }, Upstreams: []state.Upstream{ @@ -45,7 +54,7 @@ func TestGenerate(t *testing.T) { cfg := string(generator.Generate(conf)) if !strings.Contains(cfg, "listen 80") { - t.Errorf("Generate() did not generate a config with an HTTP server; config: %s", cfg) + t.Errorf("Generate() did not generate a config with a default HTTP server; config: %s", cfg) } if !strings.Contains(cfg, "listen 443") { diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 07cc6fffa7..3075370141 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -21,54 +21,55 @@ func executeServers(conf state.Configuration) []byte { } func createServers(httpServers, sslServers []state.VirtualServer) []http.Server { - confServers := append(httpServers, sslServers...) + servers := make([]http.Server, 0, len(httpServers)+len(sslServers)) - servers := make([]http.Server, 0, len(confServers)+2) - - if len(httpServers) > 0 { - defaultHTTPServer := createDefaultHTTPServer() - - servers = append(servers, defaultHTTPServer) - } - - if len(sslServers) > 0 { - defaultSSLServer := createDefaultSSLServer() - - servers = append(servers, defaultSSLServer) + for _, s := range httpServers { + servers = append(servers, createServer(s)) } - for _, s := range confServers { - servers = append(servers, createServer(s)) + for _, s := range sslServers { + servers = append(servers, createSSLServer(s)) } return servers } -func createServer(virtualServer state.VirtualServer) http.Server { - s := http.Server{ - ServerName: virtualServer.Hostname, +func createSSLServer(virtualServer state.VirtualServer) http.Server { + if virtualServer.IsDefault { + return createDefaultSSLServer() } - listenerPort := 80 - - if virtualServer.SSL != nil { - s.SSL = &http.SSL{ + return http.Server{ + ServerName: virtualServer.Hostname, + SSL: &http.SSL{ Certificate: virtualServer.SSL.CertificatePath, CertificateKey: virtualServer.SSL.CertificatePath, - } + }, + Locations: createLocations(virtualServer.PathRules, 443), + } +} + +func createServer(virtualServer state.VirtualServer) http.Server { + if virtualServer.IsDefault { + return createDefaultHTTPServer() + } - listenerPort = 443 + return http.Server{ + ServerName: virtualServer.Hostname, + Locations: createLocations(virtualServer.PathRules, 80), } +} + +func createLocations(pathRules []state.PathRule, listenerPort int) []http.Location { + lenPathRules := len(pathRules) - if len(virtualServer.PathRules) == 0 { - // generate default "/" 404 location - s.Locations = []http.Location{{Path: "/", Return: &http.Return{Code: http.StatusNotFound}}} - return s + if lenPathRules == 0 { + return []http.Location{{Path: "/", Return: &http.Return{Code: http.StatusNotFound}}} } - locs := make([]http.Location, 0, len(virtualServer.PathRules)) // FIXME(pleshakov): expand with rule.Routes + locs := make([]http.Location, 0, lenPathRules) // FIXME(pleshakov): expand with rule.Routes - for _, rule := range virtualServer.PathRules { + for _, rule := range pathRules { matches := make([]httpMatch, 0, len(rule.MatchRules)) for matchRuleIdx, r := range rule.MatchRules { @@ -129,8 +130,7 @@ func createServer(virtualServer state.VirtualServer) http.Server { } } - s.Locations = locs - return s + return locs } func createDefaultSSLServer() http.Server { diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index 42e3b3d5fa..f95530743c 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -19,6 +19,9 @@ import ( func TestExecuteServers(t *testing.T) { conf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "example.com", }, @@ -27,6 +30,9 @@ func TestExecuteServers(t *testing.T) { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "example.com", SSL: &state.SSL{ @@ -76,48 +82,48 @@ func TestExecuteForDefaultServers(t *testing.T) { conf: state.Configuration{}, httpDefault: false, sslDefault: false, - msg: "no servers", + msg: "no default servers", }, { conf: state.Configuration{ HTTPServers: []state.VirtualServer{ { - Hostname: "example.com", + IsDefault: true, }, }, }, httpDefault: true, sslDefault: false, - msg: "only HTTP servers", + msg: "only HTTP default server", }, { conf: state.Configuration{ SSLServers: []state.VirtualServer{ { - Hostname: "example.com", + IsDefault: true, }, }, }, httpDefault: false, sslDefault: true, - msg: "only HTTPS servers", + msg: "only HTTPS default server", }, { conf: state.Configuration{ HTTPServers: []state.VirtualServer{ { - Hostname: "example.com", + IsDefault: true, }, }, SSLServers: []state.VirtualServer{ { - Hostname: "example.com", + IsDefault: true, }, }, }, httpDefault: true, sslDefault: true, - msg: "both HTTP and HTTPS servers", + msg: "both HTTP and HTTPS default servers", }, } @@ -398,6 +404,9 @@ func TestCreateServers(t *testing.T) { } httpServers := []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "cafe.example.com", PathRules: cafePathRules, @@ -405,6 +414,9 @@ func TestCreateServers(t *testing.T) { } sslServers := []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "cafe.example.com", SSL: &state.SSL{CertificatePath: certPath}, @@ -494,13 +506,13 @@ func TestCreateServers(t *testing.T) { { IsDefaultHTTP: true, }, - { - IsDefaultSSL: true, - }, { ServerName: "cafe.example.com", Locations: getExpectedLocations(false), }, + { + IsDefaultSSL: true, + }, { ServerName: "cafe.example.com", SSL: &http.SSL{Certificate: certPath, CertificateKey: certPath}, diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index e0c4d84641..eb4811fd86 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -329,6 +329,9 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []state.PathRule{ @@ -347,6 +350,9 @@ var _ = Describe("ChangeProcessor", func() { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", SSL: &state.SSL{CertificatePath: certificatePath}, @@ -432,6 +438,9 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []state.PathRule{ @@ -450,6 +459,9 @@ var _ = Describe("ChangeProcessor", func() { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", SSL: &state.SSL{CertificatePath: certificatePath}, @@ -535,6 +547,9 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []state.PathRule{ @@ -553,6 +568,9 @@ var _ = Describe("ChangeProcessor", func() { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", SSL: &state.SSL{CertificatePath: certificatePath}, @@ -637,6 +655,9 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []state.PathRule{ @@ -655,6 +676,9 @@ var _ = Describe("ChangeProcessor", func() { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", SSL: &state.SSL{CertificatePath: certificatePath}, @@ -736,6 +760,9 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []state.PathRule{ @@ -754,6 +781,9 @@ var _ = Describe("ChangeProcessor", func() { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []state.PathRule{ @@ -832,6 +862,9 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []state.PathRule{ @@ -850,6 +883,9 @@ var _ = Describe("ChangeProcessor", func() { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", SSL: &state.SSL{CertificatePath: certificatePath}, @@ -946,6 +982,9 @@ var _ = Describe("ChangeProcessor", func() { expectedConf := state.Configuration{ HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "bar.example.com", PathRules: []state.PathRule{ @@ -964,6 +1003,9 @@ var _ = Describe("ChangeProcessor", func() { }, }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "bar.example.com", SSL: &state.SSL{CertificatePath: certificatePath}, @@ -1038,8 +1080,15 @@ var _ = Describe("ChangeProcessor", func() { ) expectedConf := state.Configuration{ - HTTPServers: []state.VirtualServer{}, + HTTPServers: []state.VirtualServer{ + { + IsDefault: true, + }, + }, SSLServers: []state.VirtualServer{ + { + IsDefault: true, + }, { Hostname: "~^", SSL: &state.SSL{CertificatePath: certificatePath}, diff --git a/internal/state/configuration.go b/internal/state/configuration.go index 9061a09683..36b4184b70 100644 --- a/internal/state/configuration.go +++ b/internal/state/configuration.go @@ -36,6 +36,8 @@ type VirtualServer struct { Hostname string // PathRules is a collection of routing rules. PathRules []PathRule + // IsDefault indicates whether the server is the default server. + IsDefault bool } type Upstream struct { @@ -244,6 +246,7 @@ type hostPathRules struct { rulesPerHost map[string]map[string]PathRule listenersForHost map[string]*listener listeners []*listener + listenersExist bool } func newHostPathRules() *hostPathRules { @@ -255,6 +258,8 @@ func newHostPathRules() *hostPathRules { } func (hpr *hostPathRules) upsertListener(l *listener) { + hpr.listenersExist = true + if l.Source.Protocol == v1beta1.HTTPSProtocolType { hpr.listeners = append(hpr.listeners, l) } @@ -353,6 +358,11 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { } } + // if any listeners exist, we need to generate a default server block. + if hpr.listenersExist { + servers = append(servers, VirtualServer{IsDefault: true}) + } + // We sort the servers so the order is preserved after reconfiguration. sort.Slice(servers, func(i, j int) bool { return servers[i].Hostname < servers[j].Hostname diff --git a/internal/state/configuration_test.go b/internal/state/configuration_test.go index c5bbc84220..949e78b115 100644 --- a/internal/state/configuration_test.go +++ b/internal/state/configuration_test.go @@ -20,7 +20,7 @@ import ( ) func TestBuildConfiguration(t *testing.T) { - createRoute := func(name string, hostname string, listenerName string, paths ...string) *v1beta1.HTTPRoute { + createRoute := func(name, hostname, listenerName string, paths ...string) *v1beta1.HTTPRoute { rules := make([]v1beta1.HTTPRouteRule, 0, len(paths)) for _, p := range paths { rules = append(rules, v1beta1.HTTPRouteRule{ @@ -282,8 +282,12 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*route{}, }, expConf: Configuration{ - HTTPServers: []VirtualServer{}, - SSLServers: []VirtualServer{}, + HTTPServers: []VirtualServer{ + { + IsDefault: true, + }, + }, + SSLServers: []VirtualServer{}, }, msg: "http listener with no routes", }, @@ -317,6 +321,9 @@ func TestBuildConfiguration(t *testing.T) { expConf: Configuration{ HTTPServers: []VirtualServer{}, SSLServers: []VirtualServer{ + { + IsDefault: true, + }, { Hostname: string(hostname), SSL: &SSL{CertificatePath: secretPath}, @@ -398,6 +405,9 @@ func TestBuildConfiguration(t *testing.T) { }, expConf: Configuration{ HTTPServers: []VirtualServer{ + { + IsDefault: true, + }, { Hostname: "bar.example.com", PathRules: []PathRule{ @@ -481,6 +491,9 @@ func TestBuildConfiguration(t *testing.T) { expConf: Configuration{ HTTPServers: []VirtualServer{}, SSLServers: []VirtualServer{ + { + IsDefault: true, + }, { Hostname: "bar.example.com", PathRules: []PathRule{ @@ -591,6 +604,9 @@ func TestBuildConfiguration(t *testing.T) { }, expConf: Configuration{ HTTPServers: []VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []PathRule{ @@ -637,6 +653,9 @@ func TestBuildConfiguration(t *testing.T) { }, }, SSLServers: []VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", SSL: &SSL{ @@ -796,6 +815,9 @@ func TestBuildConfiguration(t *testing.T) { }, expConf: Configuration{ HTTPServers: []VirtualServer{ + { + IsDefault: true, + }, { Hostname: "foo.example.com", PathRules: []PathRule{ From 17251e1f91dc60831aa8f81b6a061c06c1148ea4 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 9 Dec 2022 10:37:42 -0700 Subject: [PATCH 2/2] listeners -> httpsListeners --- internal/state/configuration.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/state/configuration.go b/internal/state/configuration.go index 36b4184b70..41728cb30d 100644 --- a/internal/state/configuration.go +++ b/internal/state/configuration.go @@ -245,7 +245,7 @@ func buildServers(listeners map[string]*listener) (http, ssl []VirtualServer) { type hostPathRules struct { rulesPerHost map[string]map[string]PathRule listenersForHost map[string]*listener - listeners []*listener + httpsListeners []*listener listenersExist bool } @@ -253,7 +253,7 @@ func newHostPathRules() *hostPathRules { return &hostPathRules{ rulesPerHost: make(map[string]map[string]PathRule), listenersForHost: make(map[string]*listener), - listeners: make([]*listener, 0), + httpsListeners: make([]*listener, 0), } } @@ -261,7 +261,7 @@ func (hpr *hostPathRules) upsertListener(l *listener) { hpr.listenersExist = true if l.Source.Protocol == v1beta1.HTTPSProtocolType { - hpr.listeners = append(hpr.listeners, l) + hpr.httpsListeners = append(hpr.httpsListeners, l) } for _, r := range l.Routes { @@ -309,7 +309,7 @@ func (hpr *hostPathRules) upsertListener(l *listener) { } func (hpr *hostPathRules) buildServers() []VirtualServer { - servers := make([]VirtualServer, 0, len(hpr.rulesPerHost)+len(hpr.listeners)) + servers := make([]VirtualServer, 0, len(hpr.rulesPerHost)+len(hpr.httpsListeners)) for h, rules := range hpr.rulesPerHost { s := VirtualServer{ @@ -340,7 +340,7 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { servers = append(servers, s) } - for _, l := range hpr.listeners { + for _, l := range hpr.httpsListeners { hostname := getListenerHostname(l.Source.Hostname) // generate a 404 ssl server block for listeners with no routes or listeners with wildcard (match-all) routes // FIXME(kate-osborn): when we support regex hostnames (e.g. *.example.com)