Skip to content

Commit 3153836

Browse files
author
Mike Stephen
committed
hsts annotation validation has been relaxed to comform with existing behaviour
1 parent aa531ba commit 3153836

File tree

2 files changed

+34
-35
lines changed

2 files changed

+34
-35
lines changed

internal/k8s/validation.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -137,17 +137,17 @@ var (
137137
validateBoolAnnotation,
138138
},
139139
hstsMaxAgeAnnotation: {
140-
validateRelatedAnnotation(hstsAnnotation, validateIsTrue),
140+
validateRelatedAnnotation(hstsAnnotation, validateIsBool),
141141
validateRequiredAnnotation,
142142
validateInt64Annotation,
143143
},
144144
hstsIncludeSubdomainsAnnotation: {
145-
validateRelatedAnnotation(hstsAnnotation, validateIsTrue),
145+
validateRelatedAnnotation(hstsAnnotation, validateIsBool),
146146
validateRequiredAnnotation,
147147
validateBoolAnnotation,
148148
},
149149
hstsBehindProxyAnnotation: {
150-
validateRelatedAnnotation(hstsAnnotation, validateIsTrue),
150+
validateRelatedAnnotation(hstsAnnotation, validateIsBool),
151151
validateRequiredAnnotation,
152152
validateBoolAnnotation,
153153
},
@@ -471,6 +471,11 @@ func validateRewriteListAnnotation(context *annotationValidationContext) field.E
471471
return allErrs
472472
}
473473

474+
func validateIsBool(v string) error {
475+
_, err := configs.ParseBool(v)
476+
return err
477+
}
478+
474479
func validateIsTrue(v string) error {
475480
b, err := configs.ParseBool(v)
476481
if err != nil {

internal/k8s/validation_test.go

+26-32
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,18 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
751751
expectedErrors: nil,
752752
msg: "valid nginx.org/hsts-max-age annotation",
753753
},
754+
{
755+
annotations: map[string]string{
756+
"nginx.org/hsts": "false",
757+
"nginx.org/hsts-max-age": "120",
758+
},
759+
specServices: map[string]bool{},
760+
isPlus: false,
761+
appProtectEnabled: false,
762+
internalRoutesEnabled: false,
763+
expectedErrors: nil,
764+
msg: "valid nginx.org/hsts-max-age nginx.org/hsts can be false",
765+
},
754766
{
755767
annotations: map[string]string{
756768
"nginx.org/hsts": "true",
@@ -778,32 +790,30 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
778790
},
779791
msg: "invalid nginx.org/hsts-max-age, related annotation nginx.org/hsts not set",
780792
},
793+
781794
{
782795
annotations: map[string]string{
783-
"nginx.org/hsts": "false",
784-
"nginx.org/hsts-max-age": "120",
796+
"nginx.org/hsts": "true",
797+
"nginx.org/hsts-include-subdomains": "true",
785798
},
786799
specServices: map[string]bool{},
787800
isPlus: false,
788801
appProtectEnabled: false,
789802
internalRoutesEnabled: false,
790-
expectedErrors: []string{
791-
"annotations.nginx.org/hsts-max-age: Forbidden: related annotation nginx.org/hsts: must be true",
792-
},
793-
msg: "invalid nginx.org/hsts-max-age nginx.org/hsts is not true",
803+
expectedErrors: nil,
804+
msg: "valid nginx.org/hsts-include-subdomains annotation",
794805
},
795-
796806
{
797807
annotations: map[string]string{
798-
"nginx.org/hsts": "true",
808+
"nginx.org/hsts": "false",
799809
"nginx.org/hsts-include-subdomains": "true",
800810
},
801811
specServices: map[string]bool{},
802812
isPlus: false,
803813
appProtectEnabled: false,
804814
internalRoutesEnabled: false,
805815
expectedErrors: nil,
806-
msg: "valid nginx.org/hsts-include-subdomains annotation",
816+
msg: "valid nginx.org/hsts-include-subdomains, nginx.org/hsts can be false",
807817
},
808818
{
809819
annotations: map[string]string{
@@ -832,32 +842,30 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
832842
},
833843
msg: "invalid nginx.org/hsts-include-subdomains, related annotation nginx.org/hsts not set",
834844
},
845+
835846
{
836847
annotations: map[string]string{
837-
"nginx.org/hsts": "false",
838-
"nginx.org/hsts-include-subdomains": "true",
848+
"nginx.org/hsts": "true",
849+
"nginx.org/hsts-behind-proxy": "true",
839850
},
840851
specServices: map[string]bool{},
841852
isPlus: false,
842853
appProtectEnabled: false,
843854
internalRoutesEnabled: false,
844-
expectedErrors: []string{
845-
"annotations.nginx.org/hsts-include-subdomains: Forbidden: related annotation nginx.org/hsts: must be true",
846-
},
847-
msg: "invalid nginx.org/hsts-include-subdomains nginx.org/hsts is not true",
855+
expectedErrors: nil,
856+
msg: "valid nginx.org/hsts-behind-proxy annotation",
848857
},
849-
850858
{
851859
annotations: map[string]string{
852-
"nginx.org/hsts": "true",
860+
"nginx.org/hsts": "false",
853861
"nginx.org/hsts-behind-proxy": "true",
854862
},
855863
specServices: map[string]bool{},
856864
isPlus: false,
857865
appProtectEnabled: false,
858866
internalRoutesEnabled: false,
859867
expectedErrors: nil,
860-
msg: "valid nginx.org/hsts-behind-proxy annotation",
868+
msg: "valid nginx.org/hsts-behind-proxy, nginx.org/hsts can be false",
861869
},
862870
{
863871
annotations: map[string]string{
@@ -886,20 +894,6 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
886894
},
887895
msg: "invalid nginx.org/hsts-behind-proxy, related annotation nginx.org/hsts not set",
888896
},
889-
{
890-
annotations: map[string]string{
891-
"nginx.org/hsts": "false",
892-
"nginx.org/hsts-behind-proxy": "true",
893-
},
894-
specServices: map[string]bool{},
895-
isPlus: false,
896-
appProtectEnabled: false,
897-
internalRoutesEnabled: false,
898-
expectedErrors: []string{
899-
"annotations.nginx.org/hsts-behind-proxy: Forbidden: related annotation nginx.org/hsts: must be true",
900-
},
901-
msg: "invalid nginx.org/hsts-behind-proxy nginx.org/hsts is not true",
902-
},
903897

904898
{
905899
annotations: map[string]string{

0 commit comments

Comments
 (0)