diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 1c4e11d5d..91ab7ae8b 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -472,63 +472,37 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( // generate upstreams for VirtualServer for _, u := range vsEx.VirtualServer.Spec.Upstreams { - - if (sslConfig == nil || !vsc.cfgParams.HTTP2) && isGRPC(u.Type) { - vsc.addWarningf(vsEx.VirtualServer, "gRPC cannot be configured for upstream %s. gRPC requires enabled HTTP/2 and TLS termination.", u.Name) - } - - upstreamName := virtualServerUpstreamNamer.GetNameForUpstream(u.Name) - upstreamNamespace := vsEx.VirtualServer.Namespace - endpoints := vsc.generateEndpointsForUpstream(vsEx.VirtualServer, upstreamNamespace, u, vsEx) - backupEndpoints := vsc.generateBackupEndpointsForUpstream(vsEx.VirtualServer, upstreamNamespace, u, vsEx) - - // isExternalNameSvc is always false for OSS - _, isExternalNameSvc := vsEx.ExternalNameSvcs[GenerateExternalNameSvcKey(upstreamNamespace, u.Service)] - ups := vsc.generateUpstream(vsEx.VirtualServer, upstreamName, u, isExternalNameSvc, endpoints, backupEndpoints) - upstreams = append(upstreams, ups) - - u.TLS.Enable = isTLSEnabled(u, vsc.spiffeCerts, vsEx.VirtualServer.Spec.InternalRoute) - crUpstreams[upstreamName] = u - - if hc := generateHealthCheck(u, upstreamName, vsc.cfgParams); hc != nil { - healthChecks = append(healthChecks, *hc) - if u.HealthCheck.StatusMatch != "" { - statusMatches = append( - statusMatches, - generateUpstreamStatusMatch(upstreamName, u.HealthCheck.StatusMatch), - ) - } - } + upstreams, healthChecks, statusMatches = generateUpstreams( + sslConfig, + vsc, + u, + vsEx.VirtualServer, + vsEx.VirtualServer.Namespace, + virtualServerUpstreamNamer, + vsEx, + upstreams, + crUpstreams, + healthChecks, + statusMatches, + ) } // generate upstreams for each VirtualServerRoute for _, vsr := range vsEx.VirtualServerRoutes { upstreamNamer := NewUpstreamNamerForVirtualServerRoute(vsEx.VirtualServer, vsr) for _, u := range vsr.Spec.Upstreams { - if (sslConfig == nil || !vsc.cfgParams.HTTP2) && isGRPC(u.Type) { - vsc.addWarningf(vsr, "gRPC cannot be configured for upstream %s. gRPC requires enabled HTTP/2 and TLS termination", u.Name) - } - - upstreamName := upstreamNamer.GetNameForUpstream(u.Name) - upstreamNamespace := vsr.Namespace - endpoints := vsc.generateEndpointsForUpstream(vsr, upstreamNamespace, u, vsEx) - backup := vsc.generateBackupEndpointsForUpstream(vsEx.VirtualServer, upstreamNamespace, u, vsEx) - - // isExternalNameSvc is always false for OSS - _, isExternalNameSvc := vsEx.ExternalNameSvcs[GenerateExternalNameSvcKey(upstreamNamespace, u.Service)] - ups := vsc.generateUpstream(vsr, upstreamName, u, isExternalNameSvc, endpoints, backup) - upstreams = append(upstreams, ups) - u.TLS.Enable = isTLSEnabled(u, vsc.spiffeCerts, vsEx.VirtualServer.Spec.InternalRoute) - crUpstreams[upstreamName] = u - - if hc := generateHealthCheck(u, upstreamName, vsc.cfgParams); hc != nil { - healthChecks = append(healthChecks, *hc) - if u.HealthCheck.StatusMatch != "" { - statusMatches = append( - statusMatches, - generateUpstreamStatusMatch(upstreamName, u.HealthCheck.StatusMatch), - ) - } - } + upstreams, healthChecks, statusMatches = generateUpstreams( + sslConfig, + vsc, + u, + vsr, + vsr.Namespace, + upstreamNamer, + vsEx, + upstreams, + crUpstreams, + healthChecks, + statusMatches, + ) } } @@ -551,11 +525,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( // generates config for VirtualServer routes for _, r := range vsEx.VirtualServer.Spec.Routes { - errorPages := errorPageDetails{ - pages: r.ErrorPages, - index: len(errorPageLocations), - owner: vsEx.VirtualServer, - } + errorPages := generateErrorPageDetails(r.ErrorPages, errorPageLocations, vsEx.VirtualServer) errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPages.index, errorPages.pages)...) // ignore routes that reference VirtualServerRoute @@ -699,11 +669,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( isVSR := true upstreamNamer := NewUpstreamNamerForVirtualServerRoute(vsEx.VirtualServer, vsr) for _, r := range vsr.Spec.Subroutes { - errorPages := errorPageDetails{ - pages: r.ErrorPages, - index: len(errorPageLocations), - owner: vsr, - } + errorPages := generateErrorPageDetails(r.ErrorPages, errorPageLocations, vsr) errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPages.index, errorPages.pages)...) vsrNamespaceName := fmt.Sprintf("%v/%v", vsr.Namespace, vsr.Name) // use the VirtualServer error pages if the route does not define any @@ -929,6 +895,46 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( return vsCfg, vsc.warnings } +func generateUpstreams( + sslConfig *version2.SSL, + vsc *virtualServerConfigurator, + u conf_v1.Upstream, + owner runtime.Object, + ownerNamespace string, + upstreamNamer *upstreamNamer, + vsEx *VirtualServerEx, + upstreams []version2.Upstream, + crUpstreams map[string]conf_v1.Upstream, + healthChecks []version2.HealthCheck, + statusMatches []version2.StatusMatch, +) ([]version2.Upstream, []version2.HealthCheck, []version2.StatusMatch) { + if (sslConfig == nil || !vsc.cfgParams.HTTP2) && isGRPC(u.Type) { + vsc.addWarningf(owner, "gRPC cannot be configured for upstream %s. gRPC requires enabled HTTP/2 and TLS termination", u.Name) + } + + upstreamName := upstreamNamer.GetNameForUpstream(u.Name) + endpoints := vsc.generateEndpointsForUpstream(owner, ownerNamespace, u, vsEx) + backup := vsc.generateBackupEndpointsForUpstream(owner, ownerNamespace, u, vsEx) + + // isExternalNameSvc is always false for OSS + _, isExternalNameSvc := vsEx.ExternalNameSvcs[GenerateExternalNameSvcKey(ownerNamespace, u.Service)] + ups := vsc.generateUpstream(owner, upstreamName, u, isExternalNameSvc, endpoints, backup) + upstreams = append(upstreams, ups) + u.TLS.Enable = isTLSEnabled(u, vsc.spiffeCerts, vsEx.VirtualServer.Spec.InternalRoute) + crUpstreams[upstreamName] = u + + if hc := generateHealthCheck(u, upstreamName, vsc.cfgParams); hc != nil { + healthChecks = append(healthChecks, *hc) + if u.HealthCheck.StatusMatch != "" { + statusMatches = append( + statusMatches, + generateUpstreamStatusMatch(upstreamName, u.HealthCheck.StatusMatch), + ) + } + } + return upstreams, healthChecks, statusMatches +} + // rateLimit hold the configuration for the ratelimiting Policy type rateLimit struct { Reqs []version2.LimitReq @@ -3217,6 +3223,14 @@ func generateErrorPages(errPageIndex int, errorPages []conf_v1.ErrorPage) []vers return ePages } +func generateErrorPageDetails(errorPages []conf_v1.ErrorPage, errorPageLocations []version2.ErrorPageLocation, owner runtime.Object) errorPageDetails { + return errorPageDetails{ + pages: errorPages, + index: len(errorPageLocations), + owner: owner, + } +} + func generateErrorPageLocations(errPageIndex int, errorPages []conf_v1.ErrorPage) []version2.ErrorPageLocation { var errorPageLocations []version2.ErrorPageLocation for i, e := range errorPages { diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index cd5170afd..94c972147 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -17032,6 +17032,75 @@ func TestGenerateErrorPageLocations(t *testing.T) { } } +func TestGenerateErrorPageDetails(t *testing.T) { + t.Parallel() + tests := []struct { + errorPages []conf_v1.ErrorPage + errorLocations []version2.ErrorPageLocation + owner runtime.Object + expected errorPageDetails + }{ + {}, // empty + { + errorPages: []conf_v1.ErrorPage{ + { + Codes: []int{404, 405, 500, 502}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Headers: nil, + }, + }, + Redirect: nil, + }, + }, + errorLocations: []version2.ErrorPageLocation{ + { + Name: "@error_page_0_0", + DefaultType: "text/plain", + Return: &version2.Return{ + Text: "All Good", + }, + }, + }, + owner: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "namespace", + Name: "name", + }, + }, + expected: errorPageDetails{ + pages: []conf_v1.ErrorPage{ + { + Codes: []int{404, 405, 500, 502}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Headers: nil, + }, + }, + Redirect: nil, + }, + }, + index: 1, + owner: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Namespace: "namespace", + Name: "name", + }, + }, + }, + }, + } + + for _, test := range tests { + result := generateErrorPageDetails(test.errorPages, test.errorLocations, test.owner) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("generateErrorPageDetails() returned %v but expected %v", result, test.expected) + } + } +} + func TestGenerateProxySSLName(t *testing.T) { t.Parallel() result := generateProxySSLName("coffee-v1", "default")