Skip to content

Commit f87a846

Browse files
liggittliggitt
committed
Validate ingress TLS secretName in v1
1 parent abede62 commit f87a846

File tree

3 files changed

+105
-11
lines changed

3 files changed

+105
-11
lines changed

‎pkg/apis/networking/validation/validation.go‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ var ValidateIngressName = apimachineryvalidation.NameIsDNSSubdomain
199199

200200
// IngressValidationOptions cover beta to GA transitions for HTTP PathType
201201
type IngressValidationOptions struct {
202+
// AllowInvalidSecretName indicates whether spec.tls[*].secretName values that are not valid Secret names should be allowed
203+
AllowInvalidSecretName bool
202204
}
203205

204206
// ValidateIngress validates Ingresses on create and update.
@@ -212,7 +214,9 @@ func validateIngress(ingress *networking.Ingress, opts IngressValidationOptions,
212214
func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVersion) field.ErrorList {
213215
allErrs := field.ErrorList{}
214216
var opts IngressValidationOptions
215-
opts = IngressValidationOptions{}
217+
opts = IngressValidationOptions{
218+
AllowInvalidSecretName: allowInvalidSecretName(requestGV, nil),
219+
}
216220
allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
217221
annotationVal, annotationIsSet := ingress.Annotations[annotationIngressClass]
218222
if annotationIsSet && ingress.Spec.IngressClassName != nil {
@@ -226,26 +230,34 @@ func ValidateIngressCreate(ingress *networking.Ingress, requestGV schema.GroupVe
226230
func ValidateIngressUpdate(ingress, oldIngress *networking.Ingress, requestGV schema.GroupVersion) field.ErrorList {
227231
allErrs := apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta, field.NewPath("metadata"))
228232
var opts IngressValidationOptions
229-
opts = IngressValidationOptions{}
233+
opts = IngressValidationOptions{
234+
AllowInvalidSecretName: allowInvalidSecretName(requestGV, oldIngress),
235+
}
230236

231237
allErrs = append(allErrs, validateIngress(ingress, opts, requestGV)...)
232238
return allErrs
233239
}
234240

235-
func validateIngressTLS(spec *networking.IngressSpec, fldPath *field.Path) field.ErrorList {
241+
func validateIngressTLS(spec *networking.IngressSpec, fldPath *field.Path, opts IngressValidationOptions) field.ErrorList {
236242
allErrs := field.ErrorList{}
237243
// TODO: Perform a more thorough validation of spec.TLS.Hosts that takes
238244
// the wildcard spec from RFC 6125 into account.
239-
for _, itls := range spec.TLS {
245+
for tlsIndex, itls := range spec.TLS {
240246
for i, host := range itls.Hosts {
241247
if strings.Contains(host, "*") {
242248
for _, msg := range validation.IsWildcardDNS1123Subdomain(host) {
243-
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("hosts"), host, msg))
249+
allErrs = append(allErrs, field.Invalid(fldPath.Index(tlsIndex).Child("hosts").Index(i), host, msg))
244250
}
245251
continue
246252
}
247253
for _, msg := range validation.IsDNS1123Subdomain(host) {
248-
allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("hosts"), host, msg))
254+
allErrs = append(allErrs, field.Invalid(fldPath.Index(tlsIndex).Child("hosts").Index(i), host, msg))
255+
}
256+
}
257+
258+
if !opts.AllowInvalidSecretName {
259+
for _, msg := range validateTLSSecretName(itls.SecretName) {
260+
allErrs = append(allErrs, field.Invalid(fldPath.Index(tlsIndex).Child("secretName"), itls.SecretName, msg))
249261
}
250262
}
251263
}
@@ -278,7 +290,7 @@ func ValidateIngressSpec(spec *networking.IngressSpec, fldPath *field.Path, opts
278290
allErrs = append(allErrs, validateIngressRules(spec.Rules, fldPath.Child("rules"), opts, requestGV)...)
279291
}
280292
if len(spec.TLS) > 0 {
281-
allErrs = append(allErrs, validateIngressTLS(spec, fldPath.Child("tls"))...)
293+
allErrs = append(allErrs, validateIngressTLS(spec, fldPath.Child("tls"), opts)...)
282294
}
283295
if spec.IngressClassName != nil {
284296
for _, msg := range ValidateIngressClassName(*spec.IngressClassName, false) {
@@ -523,3 +535,26 @@ func validateIngressTypedLocalObjectReference(params *api.TypedLocalObjectRefere
523535

524536
return allErrs
525537
}
538+
539+
func allowInvalidSecretName(gv schema.GroupVersion, oldIngress *networking.Ingress) bool {
540+
if gv == networkingv1beta1.SchemeGroupVersion || gv == extensionsv1beta1.SchemeGroupVersion {
541+
// backwards compatibility with released API versions that allowed invalid names
542+
return true
543+
}
544+
if oldIngress != nil {
545+
for _, tls := range oldIngress.Spec.TLS {
546+
if len(validateTLSSecretName(tls.SecretName)) > 0 {
547+
// backwards compatibility with existing persisted object
548+
return true
549+
}
550+
}
551+
}
552+
return false
553+
}
554+
555+
func validateTLSSecretName(name string) []string {
556+
if len(name) == 0 {
557+
return nil
558+
}
559+
return apivalidation.ValidateSecretName(name, false)
560+
}

‎pkg/apis/networking/validation/validation_test.go‎

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1379,6 +1379,31 @@ func TestValidateIngressCreate(t *testing.T) {
13791379
},
13801380
expectedErrs: field.ErrorList{},
13811381
},
1382+
"v1: valid secret": {
1383+
groupVersion: &networkingv1.SchemeGroupVersion,
1384+
tweakIngress: func(ingress *networking.Ingress) {
1385+
ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}}
1386+
},
1387+
},
1388+
"v1: invalid secret": {
1389+
groupVersion: &networkingv1.SchemeGroupVersion,
1390+
tweakIngress: func(ingress *networking.Ingress) {
1391+
ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name"}}
1392+
},
1393+
expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("tls").Index(0).Child("secretName"), "invalid name", `a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`)},
1394+
},
1395+
"v1beta1: valid secret": {
1396+
groupVersion: &networkingv1beta1.SchemeGroupVersion,
1397+
tweakIngress: func(ingress *networking.Ingress) {
1398+
ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}}
1399+
},
1400+
},
1401+
"v1beta1: invalid secret": {
1402+
groupVersion: &networkingv1beta1.SchemeGroupVersion,
1403+
tweakIngress: func(ingress *networking.Ingress) {
1404+
ingress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}}
1405+
},
1406+
},
13821407
}
13831408

13841409
for name, testCase := range testCases {
@@ -1431,6 +1456,7 @@ func TestValidateIngressUpdate(t *testing.T) {
14311456
}
14321457

14331458
testCases := map[string]struct {
1459+
gv schema.GroupVersion
14341460
tweakIngresses func(newIngress, oldIngress *networking.Ingress)
14351461
expectedErrs field.ErrorList
14361462
}{
@@ -1714,6 +1740,35 @@ func TestValidateIngressUpdate(t *testing.T) {
17141740
},
17151741
expectedErrs: field.ErrorList{},
17161742
},
1743+
"v1: change valid secret -> invalid secret": {
1744+
gv: networkingv1.SchemeGroupVersion,
1745+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1746+
oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}}
1747+
newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name"}}
1748+
},
1749+
expectedErrs: field.ErrorList{field.Invalid(field.NewPath("spec").Child("tls").Index(0).Child("secretName"), "invalid name", `a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`)},
1750+
},
1751+
"v1: change invalid secret -> invalid secret": {
1752+
gv: networkingv1.SchemeGroupVersion,
1753+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1754+
oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}}
1755+
newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 2"}}
1756+
},
1757+
},
1758+
"v1beta1: change valid secret -> invalid secret": {
1759+
gv: networkingv1beta1.SchemeGroupVersion,
1760+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1761+
oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "valid"}}
1762+
newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name"}}
1763+
},
1764+
},
1765+
"v1beta1: change invalid secret -> invalid secret": {
1766+
gv: networkingv1beta1.SchemeGroupVersion,
1767+
tweakIngresses: func(newIngress, oldIngress *networking.Ingress) {
1768+
oldIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 1"}}
1769+
newIngress.Spec.TLS = []networking.IngressTLS{{SecretName: "invalid name 2"}}
1770+
},
1771+
},
17171772
}
17181773

17191774
for name, testCase := range testCases {
@@ -1722,7 +1777,11 @@ func TestValidateIngressUpdate(t *testing.T) {
17221777
oldIngress := baseIngress.DeepCopy()
17231778
testCase.tweakIngresses(newIngress, oldIngress)
17241779

1725-
errs := ValidateIngressUpdate(newIngress, oldIngress, networkingv1beta1.SchemeGroupVersion)
1780+
gv := testCase.gv
1781+
if gv.Empty() {
1782+
gv = networkingv1beta1.SchemeGroupVersion
1783+
}
1784+
errs := ValidateIngressUpdate(newIngress, oldIngress, gv)
17261785

17271786
if len(errs) != len(testCase.expectedErrs) {
17281787
t.Fatalf("Expected %d errors, got %d (%+v)", len(testCase.expectedErrs), len(errs), errs)
@@ -1978,7 +2037,7 @@ func TestValidateIngressTLS(t *testing.T) {
19782037
Hosts: []string{wildcardHost},
19792038
},
19802039
}
1981-
badWildcardTLSErr := fmt.Sprintf("spec.tls[0].hosts: Invalid value: '%v'", wildcardHost)
2040+
badWildcardTLSErr := fmt.Sprintf("spec.tls[0].hosts[0]: Invalid value: '%v'", wildcardHost)
19822041
errorCases[badWildcardTLSErr] = badWildcardTLS
19832042

19842043
for k, v := range errorCases {

‎pkg/registry/networking/ingress/storage/storage_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ var (
6060
defaultPathType = networking.PathTypeImplementationSpecific
6161
defaultPathMap = map[string]string{defaultPath: defaultBackendName}
6262
defaultTLS = []networking.IngressTLS{
63-
{Hosts: []string{"foo.bar.com", "*.bar.com"}, SecretName: "fooSecret"},
63+
{Hosts: []string{"foo.bar.com", "*.bar.com"}, SecretName: "foosecret"},
6464
}
6565
serviceBackend = &networking.IngressServiceBackend{
6666
Name: "defaultbackend",
@@ -173,7 +173,7 @@ func TestUpdate(t *testing.T) {
173173
})
174174
object.Spec.TLS = append(object.Spec.TLS, networking.IngressTLS{
175175
Hosts: []string{"*.google.com"},
176-
SecretName: "googleSecret",
176+
SecretName: "googlesecret",
177177
})
178178
return object
179179
},

0 commit comments

Comments
 (0)