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

Gunzip for VS #3790

Merged
merged 16 commits into from
Apr 25, 2023
Merged
Changes from 13 commits
Commits
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
2 changes: 2 additions & 0 deletions deployments/common/crds/k8s.nginx.org_virtualservers.yaml
Original file line number Diff line number Diff line change
@@ -87,6 +87,8 @@ spec:
format: int64
recordType:
type: string
gunzip:
type: string
host:
type: string
http-snippets:
Original file line number Diff line number Diff line change
@@ -87,6 +87,8 @@ spec:
format: int64
recordType:
type: string
gunzip:
type: string
host:
type: string
http-snippets:
Original file line number Diff line number Diff line change
@@ -26,6 +26,7 @@ spec:
host: cafe.example.com
tls:
secret: cafe-secret
gunzip: on
upstreams:
- name: tea
service: tea-svc
@@ -53,6 +54,7 @@ spec:
| ---| ---| ---| --- |
|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. When using a wildcard domain like ``*.example.com`` the domain must be contained in double quotes. The ``host`` value needs to be unique among all Ingress and VirtualServer resources. See also [Handling Host and Listener Collisions](/nginx-ingress-controller/configuration/handling-host-and-listener-collisions). | ``string`` | Yes |
|``tls`` | The TLS termination configuration. | [tls](#virtualservertls) | No |
|``gunzip`` | Enables or disables decompression of gzipped responses for clients. | ``string`` | No |
haywoodsh marked this conversation as resolved.
Show resolved Hide resolved
|``externalDNS`` | The externalDNS configuration for a VirtualServer. | [externalDNS](#virtualserverexternaldns) | No |
|``dos`` | A reference to a DosProtectedResource, setting this enables DOS protection of the VirtualServer. | ``string`` | No |
|``policies`` | A list of policies. | [[]policy](#virtualserverpolicy) | No |
1 change: 1 addition & 0 deletions internal/configs/version2/http.go
Original file line number Diff line number Diff line change
@@ -79,6 +79,7 @@ type Server struct {
VSNamespace string
VSName string
DisableIPV6 bool
Gunzip string
}

// SSL defines SSL configuration for a server.
1 change: 1 addition & 0 deletions internal/configs/version2/nginx-plus.virtualserver.tmpl
Original file line number Diff line number Diff line change
@@ -68,6 +68,7 @@ match {{ $m.Name }} {
{{ end }}

server {
{{ if (eq $s.Gunzip "on") }}gunzip {{ $s.Gunzip }};{{end}}
listen 80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};
{{ if not $s.DisableIPV6 }}listen [::]:80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }}

1 change: 1 addition & 0 deletions internal/configs/version2/nginx.virtualserver.tmpl
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@ limit_req_zone {{ $z.Key }} zone={{ $z.ZoneName }}:{{ $z.ZoneSize }} rate={{ $z.

{{ $s := .Server }}
server {
{{ if (eq $s.Gunzip "on") }}gunzip {{ $s.Gunzip }};{{end}}
listen 80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};
{{ if not $s.DisableIPV6 }}listen [::]:80{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};{{ end }}

2,642 changes: 1,866 additions & 776 deletions internal/configs/version2/templates_test.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/apis/configuration/v1/types.go
Original file line number Diff line number Diff line change
@@ -39,6 +39,7 @@ type VirtualServerSpec struct {
IngressClass string `json:"ingressClassName"`
Host string `json:"host"`
TLS *TLS `json:"tls"`
Gunzip string `json:"gunzip"`
Policies []PolicyReference `json:"policies"`
Upstreams []Upstream `json:"upstreams"`
Routes []Route `json:"routes"`
10 changes: 10 additions & 0 deletions pkg/apis/configuration/validation/virtualserver.go
Original file line number Diff line number Diff line change
@@ -77,6 +77,7 @@ func (vsv *VirtualServerValidator) validateVirtualServerSpec(spec *v1.VirtualSer
allErrs := field.ErrorList{}

allErrs = append(allErrs, validateHost(spec.Host, fieldPath.Child("host"))...)
allErrs = append(allErrs, validateGunzip(spec.Gunzip, fieldPath.Child("gunzip"))...)
allErrs = append(allErrs, vsv.validateTLS(spec.TLS, fieldPath.Child("tls"))...)
allErrs = append(allErrs, validatePolicies(spec.Policies, fieldPath.Child("policies"), namespace)...)

@@ -114,6 +115,15 @@ func validateHost(host string, fieldPath *field.Path) field.ErrorList {
return allErrs
}

func validateGunzip(fieldValue string, fl *field.Path) field.ErrorList {
switch fieldValue {
case "on", "off", "":
return nil
default:
return field.ErrorList{field.NotSupported(fl, fieldValue, []string{"on", "off"})}
}
}

func validatePolicies(policies []v1.PolicyReference, fieldPath *field.Path, namespace string) field.ErrorList {
allErrs := field.ErrorList{}
policyKeys := sets.Set[string]{}
230 changes: 230 additions & 0 deletions pkg/apis/configuration/validation/virtualserver_test.go
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@ import (

func TestValidateVirtualServer(t *testing.T) {
t.Parallel()

virtualServer := v1.VirtualServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "cafe",
@@ -68,6 +69,215 @@ func TestValidateVirtualServer(t *testing.T) {
}
}

func TestValidateVirtualServer_PassesOnValidGunzipOn(t *testing.T) {
t.Parallel()

vsv := &VirtualServerValidator{isPlus: false, isDosEnabled: false}
err := vsv.ValidateVirtualServer(&virtualServerWithValidGunzipOn)
if err != nil {
t.Errorf("ValidateVirtualServer() returned error %v for valid input %+v", err, virtualServerWithValidGunzipOn)
}
}

func TestValidateVirtualServer_PassesOnValidGunzipOff(t *testing.T) {
t.Parallel()

vsv := &VirtualServerValidator{isPlus: false, isDosEnabled: false}
err := vsv.ValidateVirtualServer(&virtualServerWithValidGunzipOff)
if err != nil {
t.Errorf("ValidateVirtualServer() returned error %v for valid input %+v", err, virtualServerWithValidGunzipOff)
}
}

func TestValidateVirtualServer_PassesOnNoGunzip(t *testing.T) {
t.Parallel()

vsv := &VirtualServerValidator{isPlus: false, isDosEnabled: false}
err := vsv.ValidateVirtualServer(&virtualServerWithNoGunzip)
if err != nil {
t.Errorf("ValidateVirtualServer() returned error %v for valid input %+v", err, virtualServerWithNoGunzip)
}
}

func TestValidateVirtualServer_FailsOnBogusGunzipValue(t *testing.T) {
t.Parallel()

vsv := &VirtualServerValidator{isPlus: false, isDosEnabled: false}
err := vsv.ValidateVirtualServer(&virtualServerWithBogusGunzipValue)
if err == nil {
t.Error("ValidateVirtualServer() returned no error on bogus gunzip value")
}
}

var (
virtualServerWithValidGunzipOn = v1.VirtualServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "cafe",
Namespace: "default",
},
Spec: v1.VirtualServerSpec{
Host: "example.com",
Gunzip: "on",
Upstreams: []v1.Upstream{
{
Name: "first",
Service: "service-1",
LBMethod: "random",
Port: 80,
MaxFails: createPointerFromInt(8),
MaxConns: createPointerFromInt(16),
Keepalive: createPointerFromInt(32),
Type: "grpc",
},
{
Name: "second",
Service: "service-2",
Port: 80,
},
},
Routes: []v1.Route{
{
Path: "/first",
Action: &v1.Action{
Pass: "first",
},
},
{
Path: "/second",
Action: &v1.Action{
Pass: "second",
},
},
},
},
}

virtualServerWithValidGunzipOff = v1.VirtualServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "cafe",
Namespace: "default",
},
Spec: v1.VirtualServerSpec{
Host: "example.com",
Gunzip: "off",
Upstreams: []v1.Upstream{
{
Name: "first",
Service: "service-1",
LBMethod: "random",
Port: 80,
MaxFails: createPointerFromInt(8),
MaxConns: createPointerFromInt(16),
Keepalive: createPointerFromInt(32),
Type: "grpc",
},
{
Name: "second",
Service: "service-2",
Port: 80,
},
},
Routes: []v1.Route{
{
Path: "/first",
Action: &v1.Action{
Pass: "first",
},
},
{
Path: "/second",
Action: &v1.Action{
Pass: "second",
},
},
},
},
}

virtualServerWithNoGunzip = v1.VirtualServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "cafe",
Namespace: "default",
},
Spec: v1.VirtualServerSpec{
Host: "example.com",
Upstreams: []v1.Upstream{
{
Name: "first",
Service: "service-1",
LBMethod: "random",
Port: 80,
MaxFails: createPointerFromInt(8),
MaxConns: createPointerFromInt(16),
Keepalive: createPointerFromInt(32),
Type: "grpc",
},
{
Name: "second",
Service: "service-2",
Port: 80,
},
},
Routes: []v1.Route{
{
Path: "/first",
Action: &v1.Action{
Pass: "first",
},
},
{
Path: "/second",
Action: &v1.Action{
Pass: "second",
},
},
},
},
}

virtualServerWithBogusGunzipValue = v1.VirtualServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "cafe",
Namespace: "default",
},
Spec: v1.VirtualServerSpec{
Host: "example.com",
Gunzip: "bogus",
Upstreams: []v1.Upstream{
{
Name: "first",
Service: "service-1",
LBMethod: "random",
Port: 80,
MaxFails: createPointerFromInt(8),
MaxConns: createPointerFromInt(16),
Keepalive: createPointerFromInt(32),
Type: "grpc",
},
{
Name: "second",
Service: "service-2",
Port: 80,
},
},
Routes: []v1.Route{
{
Path: "/first",
Action: &v1.Action{
Pass: "first",
},
},
{
Path: "/second",
Action: &v1.Action{
Pass: "second",
},
},
},
},
}
)

func TestValidateHost(t *testing.T) {
t.Parallel()
validHosts := []string{
@@ -101,6 +311,26 @@ func TestValidateHost(t *testing.T) {
}
}

func TestValidateGunzip_FailsOnBogusGunzipValue(t *testing.T) {
t.Parallel()
bogusGunzipValue := "bogus"
allErr := validateGunzip(bogusGunzipValue, field.NewPath("gunzip"))
if len(allErr) == 0 {
t.Errorf("validateGunzip(%q) did not return error on invalid input.", bogusGunzipValue)
}
}

func TestValidateGunzip_PassesOnValidGunzipValues(t *testing.T) {
t.Parallel()
tt := []string{"on", "off", ""}
for _, v := range tt {
allErr := validateGunzip(v, field.NewPath("gunzip"))
if len(allErr) > 0 {
t.Errorf("validateGunzip(%q) returned errors %v for valid input", v, allErr)
}
}
}

func TestValidateDos(t *testing.T) {
t.Parallel()
validDosResources := []string{