Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NGINX configuration for UpstreamSettingsPolicy #2877

Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
78d1f01
Add NGINX configuration for UpstreamSettingsPolicy
bjee19 Dec 6, 2024
d39f4c0
Fix naming conventions, test messages, and add more comments
bjee19 Dec 9, 2024
1053ca2
Remove typo
bjee19 Dec 9, 2024
5f7eee7
Change funtionality so connection header value is empty when keep ali…
bjee19 Dec 9, 2024
a420810
Update generated files
bjee19 Dec 9, 2024
b188ef6
Add feedback for upstreams template
bjee19 Dec 10, 2024
939e782
Adjust comment in upstreams template
bjee19 Dec 10, 2024
032a6bc
Add feedback for naming of variables and fields
bjee19 Dec 10, 2024
7c403dd
Add upstreams test case for empty policy
bjee19 Dec 10, 2024
3618fbd
Add processor test case for multiple policies
bjee19 Dec 10, 2024
042cce5
Refactor generating proxy set headers
bjee19 Dec 10, 2024
95d3f8d
Add processor test case for processing non-UpstreamSettingsPolicies
bjee19 Dec 10, 2024
03f3c7e
Add another upstream test case
bjee19 Dec 10, 2024
c54b874
Fix upstream test case
bjee19 Dec 10, 2024
c53c24b
Extract keepalive settings outside of UpstreamSettings
bjee19 Dec 10, 2024
02705e0
Turn executeUpstreams into function
bjee19 Dec 10, 2024
513351e
Add UpstreamKeepAlive struct for http Upstream
bjee19 Dec 11, 2024
a2ace83
Add keepAliveChecker function
bjee19 Dec 11, 2024
b8a28a6
Add review feedback
bjee19 Dec 11, 2024
c489591
Adjust spacing in upstreams template
bjee19 Dec 11, 2024
65a5db6
Add end to end test for unset connection header
bjee19 Dec 11, 2024
db05606
Remove UpstreamSettingsProcessor interface and move UpstreamSettings …
bjee19 Dec 11, 2024
dd3f170
Refactor small code fix
bjee19 Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions apis/v1alpha1/policy_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,15 @@ func (p *ObservabilityPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
func (p *ObservabilityPolicy) SetPolicyStatus(status v1alpha2.PolicyStatus) {
p.Status = status
}

func (p *UpstreamSettingsPolicy) GetTargetRefs() []v1alpha2.LocalPolicyTargetReference {
return p.Spec.TargetRefs
}

func (p *UpstreamSettingsPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
return p.Status
}

func (p *UpstreamSettingsPolicy) SetPolicyStatus(status v1alpha2.PolicyStatus) {
p.Status = status
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.16.2
controller-gen.kubebuilder.io/version: v0.16.5
labels:
gateway.networking.k8s.io/policy: direct
name: upstreamsettingspolicies.gateway.nginx.org
Expand Down Expand Up @@ -76,13 +76,13 @@ spec:
Time defines the maximum time during which requests can be processed through one keep-alive connection.
After this time is reached, the connection is closed following the subsequent request processing.
Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_time
pattern: ^\d{1,4}(ms|s)?$
pattern: ^[0-9]{1,4}(ms|s|m|h)?$
type: string
timeout:
description: |-
Timeout defines the keep-alive timeout for upstreams.
Directive: https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_timeout
pattern: ^\d{1,4}(ms|s)?$
pattern: ^[0-9]{1,4}(ms|s|m|h)?$
type: string
type: object
targetRefs:
Expand Down
17 changes: 13 additions & 4 deletions internal/mode/static/nginx/config/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"github.com/go-logr/logr"

ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/clientsettings"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/observability"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/upstreamsettings"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
)
Expand Down Expand Up @@ -131,7 +133,10 @@ func (g GeneratorImpl) executeConfigTemplates(
) []file.File {
fileBytes := make(map[string][]byte)

for _, execute := range g.getExecuteFuncs(generator) {
httpUpstreams := g.createUpstreams(conf.Upstreams, upstreamsettings.NewProcessor())
keepAliveCheck := newKeepAliveChecker(httpUpstreams)

for _, execute := range g.getExecuteFuncs(generator, httpUpstreams, keepAliveCheck) {
results := execute(conf)
for _, res := range results {
fileBytes[res.dest] = append(fileBytes[res.dest], res.data...)
Expand All @@ -156,12 +161,16 @@ func (g GeneratorImpl) executeConfigTemplates(
return files
}

func (g GeneratorImpl) getExecuteFuncs(generator policies.Generator) []executeFunc {
func (g GeneratorImpl) getExecuteFuncs(
generator policies.Generator,
upstreams []http.Upstream,
keepAliveCheck keepAliveChecker,
) []executeFunc {
return []executeFunc{
executeMainConfig,
executeBaseHTTPConfig,
g.newExecuteServersFunc(generator),
g.executeUpstreams,
g.newExecuteServersFunc(generator, keepAliveCheck),
newExecuteUpstreamsFunc(upstreams),
executeSplitClients,
executeMaps,
executeTelemetry,
Expand Down
19 changes: 15 additions & 4 deletions internal/mode/static/nginx/config/http/config.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package http

import "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared"
import (
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/shared"
)

const (
InternalRoutePathPrefix = "/_ngf-internal"
Expand Down Expand Up @@ -82,9 +84,18 @@ const (

// Upstream holds all configuration for an HTTP upstream.
type Upstream struct {
Name string
ZoneSize string // format: 512k, 1m
Servers []UpstreamServer
Name string
ZoneSize string // format: 512k, 1m
KeepAlive UpstreamKeepAlive
Servers []UpstreamServer
}

// UpstreamKeepAlive holds the keepalive configuration for an HTTP upstream.
type UpstreamKeepAlive struct {
Time string
Timeout string
Connections int32
Requests int32
}

// UpstreamServer holds all configuration for an HTTP upstream server.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package upstreamsettings

import (
ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/http"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies"
)

// Processor processes UpstreamSettingsPolicies.
bjee19 marked this conversation as resolved.
Show resolved Hide resolved
type Processor struct{}

// UpstreamSettings contains settings from UpstreamSettingsPolicy.
type UpstreamSettings struct {
// ZoneSize is the zone size setting.
ZoneSize string
// KeepAlive contains the keepalive settings.
KeepAlive http.UpstreamKeepAlive
}

// NewProcessor returns a new Processor.
func NewProcessor() Processor {
return Processor{}
}

// Process processes policies into an UpstreamSettings object. The policies are already validated and are guaranteed
// to not contain overlapping settings. This method merges all fields in the policies into a single UpstreamSettings
// object.
func (g Processor) Process(pols []policies.Policy) UpstreamSettings {
return processPolicies(pols)
}

func processPolicies(pols []policies.Policy) UpstreamSettings {
upstreamSettings := UpstreamSettings{}

for _, pol := range pols {
usp, ok := pol.(*ngfAPI.UpstreamSettingsPolicy)
if !ok {
continue
bjee19 marked this conversation as resolved.
Show resolved Hide resolved
}

// we can assume that there will be no instance of two or more policies setting the same
// field for the same service
if usp.Spec.ZoneSize != nil {
upstreamSettings.ZoneSize = string(*usp.Spec.ZoneSize)
}

if usp.Spec.KeepAlive != nil {
if usp.Spec.KeepAlive.Connections != nil {
upstreamSettings.KeepAlive.Connections = *usp.Spec.KeepAlive.Connections
}

if usp.Spec.KeepAlive.Requests != nil {
upstreamSettings.KeepAlive.Requests = *usp.Spec.KeepAlive.Requests
}

if usp.Spec.KeepAlive.Time != nil {
upstreamSettings.KeepAlive.Time = string(*usp.Spec.KeepAlive.Time)
}

if usp.Spec.KeepAlive.Timeout != nil {
upstreamSettings.KeepAlive.Timeout = string(*usp.Spec.KeepAlive.Timeout)
}
}
}

return upstreamSettings
}
Loading
Loading