Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ Minions cannot contain the following annotations:
- nginx.org/proxy-hide-headers
- nginx.org/proxy-pass-headers
- nginx.org/redirect-to-https
- ingress.kubernetes.io/ssl-redirect
- ingress.kubernetes.io/ssl-redirect (deprecated, use nginx.org/ssl-redirect instead)
- nginx.org/ssl-redirect
- nginx.org/hsts
- nginx.org/hsts-max-age
- nginx.org/hsts-include-subdomains
Expand Down
12 changes: 11 additions & 1 deletion internal/configs/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ const SSLPreferServerCiphersAnnotation = "nginx.org/ssl-prefer-server-ciphers"
// UseClusterIPAnnotation is the annotation where the use-cluster-ip boolean is specified.
const UseClusterIPAnnotation = "nginx.org/use-cluster-ip"

// SSLRedirectAnnotation is the annotation where the SSL redirect boolean is specified.
const SSLRedirectAnnotation = "nginx.org/ssl-redirect"

// AppProtectPolicyAnnotation is where the NGINX App Protect policy is specified
const AppProtectPolicyAnnotation = "appprotect.f5.com/app-protect-policy"

Expand Down Expand Up @@ -62,6 +65,7 @@ var minionDenylist = map[string]bool{
"nginx.org/proxy-pass-headers": true,
"nginx.org/redirect-to-https": true,
"ingress.kubernetes.io/ssl-redirect": true,
SSLRedirectAnnotation: true,
"nginx.org/hsts": true,
"nginx.org/hsts-max-age": true,
"nginx.org/hsts-include-subdomains": true,
Expand Down Expand Up @@ -263,7 +267,13 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
}
}

if sslRedirect, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "ingress.kubernetes.io/ssl-redirect", ingEx.Ingress); exists {
if sslRedirect, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, SSLRedirectAnnotation, ingEx.Ingress); exists {
if err != nil {
nl.Error(l, err)
} else {
cfgParams.SSLRedirect = sslRedirect
}
} else if sslRedirect, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "ingress.kubernetes.io/ssl-redirect", ingEx.Ingress); exists {
if err != nil {
nl.Error(l, err)
} else {
Expand Down
84 changes: 84 additions & 0 deletions internal/configs/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -927,3 +927,87 @@ func TestClientBodyBufferSizeAnnotationInvalid(t *testing.T) {
})
}
}

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

tests := []struct {
name string
annotations map[string]string
expected bool
}{
{
name: "nginx.org/ssl-redirect enabled",
annotations: map[string]string{
"nginx.org/ssl-redirect": "true",
},
expected: true,
},
{
name: "nginx.org/ssl-redirect disabled",
annotations: map[string]string{
"nginx.org/ssl-redirect": "false",
},
expected: false,
},
{
name: "deprecated ingress.kubernetes.io/ssl-redirect enabled",
annotations: map[string]string{
"ingress.kubernetes.io/ssl-redirect": "true",
},
expected: true,
},
{
name: "deprecated ingress.kubernetes.io/ssl-redirect disabled",
annotations: map[string]string{
"ingress.kubernetes.io/ssl-redirect": "false",
},
expected: false,
},
{
name: "nginx.org/ssl-redirect takes precedence when both present",
annotations: map[string]string{
"nginx.org/ssl-redirect": "false",
"ingress.kubernetes.io/ssl-redirect": "true",
},
expected: false,
},
{
name: "nginx.org/ssl-redirect enabled takes precedence",
annotations: map[string]string{
"nginx.org/ssl-redirect": "true",
"ingress.kubernetes.io/ssl-redirect": "false",
},
expected: true,
},
{
name: "no ssl-redirect annotations",
annotations: map[string]string{},
expected: true, // Default is true
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

ingEx := &IngressEx{
Ingress: &networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ingress",
Namespace: "default",
Annotations: tt.annotations,
},
},
}

baseCfgParams := NewDefaultConfigParams(context.Background(), false)
result := parseAnnotations(ingEx, baseCfgParams, false, false, false, false, false)

if result.SSLRedirect != tt.expected {
t.Errorf("Test %q: expected SSLRedirect %t, got %t", tt.name, tt.expected, result.SSLRedirect)
}
})
}
}
4 changes: 4 additions & 0 deletions internal/configs/configurator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1651,6 +1651,7 @@ func TestGetIngressAnnotations(t *testing.T) {
Annotations: map[string]string{
"appprotect.f5.com/app-protect-enable": "False",
"nginx.org/proxy-set-header": "X-Forwarded-ABC",
"nginx.org/ssl-redirect": "True",
"ingress.kubernetes.io/ssl-redirect": "True",
},
},
Expand All @@ -1667,6 +1668,7 @@ func TestGetIngressAnnotations(t *testing.T) {
expectedAnnotations := []string{
"appprotect.f5.com/app-protect-enable",
"nginx.org/proxy-set-header",
"nginx.org/ssl-redirect",
"ingress.kubernetes.io/ssl-redirect",
}

Expand Down Expand Up @@ -1748,6 +1750,7 @@ func TestGetMixedIngressAnnotations(t *testing.T) {
"alb.ingress.kubernetes.io/scheme": "internal",
"appprotect.f5.com/app-protect-enable": "False",
"nginx.org/proxy-set-header": "X-Forwarded-ABC",
"nginx.org/ssl-redirect": "True",
"ingress.kubernetes.io/ssl-redirect": "True",
},
},
Expand All @@ -1760,6 +1763,7 @@ func TestGetMixedIngressAnnotations(t *testing.T) {
}

expectedAnnotations := []string{
"nginx.org/ssl-redirect",
"ingress.kubernetes.io/ssl-redirect",
"nginx.org/proxy-set-header",
"appprotect.f5.com/app-protect-enable",
Expand Down
5 changes: 5 additions & 0 deletions internal/configs/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings)
allWarnings := newWarnings()
allWarnings.Add(rewriteTargetWarnings)

// Check for deprecated SSL redirect annotation and add warning
if _, exists := p.ingEx.Ingress.Annotations["ingress.kubernetes.io/ssl-redirect"]; exists {
allWarnings.AddWarningf(p.ingEx.Ingress, "The annotation 'ingress.kubernetes.io/ssl-redirect' is deprecated and will be removed. Please use 'nginx.org/ssl-redirect' instead.")
}

var servers []version1.Server
var limitReqZones []version1.LimitReqZone

Expand Down
65 changes: 65 additions & 0 deletions internal/configs/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2703,3 +2703,68 @@ func TestScaleRatelimit(t *testing.T) {
}
}
}

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

cafeIngressEx := createCafeIngressEx()

tests := []struct {
annotations map[string]string
expectedWarnings Warnings
msg string
}{
{
annotations: map[string]string{
"ingress.kubernetes.io/ssl-redirect": "true",
},
expectedWarnings: Warnings{
cafeIngressEx.Ingress: {"The annotation 'ingress.kubernetes.io/ssl-redirect' is deprecated and will be removed. Please use 'nginx.org/ssl-redirect' instead."},
},
msg: "deprecated annotation generates warning",
},
{
annotations: map[string]string{
"nginx.org/ssl-redirect": "true",
},
expectedWarnings: Warnings{},
msg: "new annotation does not generate warning",
},
{
annotations: map[string]string{
"ingress.kubernetes.io/ssl-redirect": "true",
"nginx.org/ssl-redirect": "false",
},
expectedWarnings: Warnings{
cafeIngressEx.Ingress: {"The annotation 'ingress.kubernetes.io/ssl-redirect' is deprecated and will be removed. Please use 'nginx.org/ssl-redirect' instead."},
},
msg: "both annotations present generates warning",
},
{
annotations: map[string]string{},
expectedWarnings: Warnings{},
msg: "no ssl-redirect annotations",
},
}

for _, test := range tests {
cafeIngressEx.Ingress.Annotations = test.annotations
configParams := NewDefaultConfigParams(context.Background(), false)

_, warnings := generateNginxCfg(NginxCfgParams{
staticParams: &StaticConfigParams{},
ingEx: &cafeIngressEx,
apResources: nil,
dosResource: nil,
isMinion: false,
isPlus: false,
BaseCfgParams: configParams,
isResolverConfigured: false,
isWildcardEnabled: false,
})

if !reflect.DeepEqual(test.expectedWarnings, warnings) {
t.Errorf("generateNginxCfg() returned %v but expected %v for the case of %s", warnings, test.expectedWarnings, test.msg)
}
}
}
7 changes: 6 additions & 1 deletion internal/k8s/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ const (
clientMaxBodySizeAnnotation = "nginx.org/client-max-body-size"
clientBodyBufferSizeAnnotation = "nginx.org/client-body-buffer-size"
redirectToHTTPSAnnotation = "nginx.org/redirect-to-https"
sslRedirectAnnotation = "ingress.kubernetes.io/ssl-redirect"
sslRedirectAnnotationDeprecated = "ingress.kubernetes.io/ssl-redirect"
sslRedirectAnnotation = "nginx.org/ssl-redirect"
proxyBufferingAnnotation = "nginx.org/proxy-buffering"
hstsAnnotation = "nginx.org/hsts"
hstsMaxAgeAnnotation = "nginx.org/hsts-max-age"
Expand Down Expand Up @@ -188,6 +189,10 @@ var (
validateRequiredAnnotation,
validateBoolAnnotation,
},
sslRedirectAnnotationDeprecated: {
validateRequiredAnnotation,
validateBoolAnnotation,
},
sslRedirectAnnotation: {
validateRequiredAnnotation,
validateBoolAnnotation,
Expand Down
29 changes: 29 additions & 0 deletions internal/k8s/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,35 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
msg: "invalid nginx.org/redirect-to-https annotation",
},

{
annotations: map[string]string{
"nginx.org/ssl-redirect": "true",
},
specServices: map[string]bool{},
isPlus: false,
appProtectEnabled: false,
appProtectDosEnabled: false,
internalRoutesEnabled: false,
directiveAutoAdjust: false,
expectedErrors: nil,
msg: "valid nginx.org/ssl-redirect annotation",
},
{
annotations: map[string]string{
"nginx.org/ssl-redirect": "not_a_boolean",
},
specServices: map[string]bool{},
isPlus: false,
appProtectEnabled: false,
appProtectDosEnabled: false,
internalRoutesEnabled: false,
directiveAutoAdjust: false,
expectedErrors: []string{
`annotations.nginx.org/ssl-redirect: Invalid value: "not_a_boolean": must be a boolean`,
},
msg: "invalid nginx.org/ssl-redirect annotation",
},

{
annotations: map[string]string{
"ingress.kubernetes.io/ssl-redirect": "true",
Expand Down
2 changes: 2 additions & 0 deletions internal/telemetry/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,7 @@ func TestStandardIngressAnnotations(t *testing.T) {
annotations := map[string]string{
"appprotect.f5.com/app-protect-enable": "False",
"nginx.org/proxy-set-header": "X-Forwarded-ABC",
"nginx.org/ssl-redirect": "True",
"ingress.kubernetes.io/ssl-redirect": "True",
"nginx.com/slow-start": "0s",
}
Expand All @@ -851,6 +852,7 @@ func TestStandardIngressAnnotations(t *testing.T) {
expectedAnnotations := []string{
"appprotect.f5.com/app-protect-enable",
"nginx.org/proxy-set-header",
"nginx.org/ssl-redirect",
"nginx.com/slow-start",
"ingress.kubernetes.io/ssl-redirect",
}
Expand Down