From 57d2a6415777a6ac347e30c45cf526ba55b3048a Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Mon, 17 Oct 2022 13:57:50 +0000 Subject: [PATCH 1/6] Webhook: validate certificateRefs when tls is set and mode is Terminate. Signed-off-by: Huang Xin --- apis/v1beta1/validation/gateway.go | 16 ++++++++++++++++ apis/v1beta1/validation/gateway_test.go | 10 ++++++++++ 2 files changed, 26 insertions(+) diff --git a/apis/v1beta1/validation/gateway.go b/apis/v1beta1/validation/gateway.go index 2ec62a23a6..32b93fe4a6 100644 --- a/apis/v1beta1/validation/gateway.go +++ b/apis/v1beta1/validation/gateway.go @@ -62,6 +62,7 @@ func validateGatewayListeners(listeners []gatewayv1b1.Listener, path *field.Path var errs field.ErrorList errs = append(errs, validateListenerTLSConfig(listeners, path)...) errs = append(errs, validateListenerHostname(listeners, path)...) + errs = append(errs, validateTLSCertificateRefs(listeners, path)...) return errs } @@ -91,3 +92,18 @@ func validateListenerHostname(listeners []gatewayv1b1.Listener, path *field.Path } return errs } + +// validateTLSCertificateRefs validates the certificateRefs +// must be set when tls config is set and TLSModeType is +// terminate +func validateTLSCertificateRefs(listeners []gatewayv1b1.Listener, path *field.Path) field.ErrorList { + var errs field.ErrorList + for i, h := range listeners { + if h.TLS != nil { + if h.TLS.Mode != nil && *h.TLS.Mode == "Terminate" && h.TLS.CertificateRefs == nil { + errs = append(errs, field.Forbidden(path.Index(i).Child("tls").Child("certificateRefs"), fmt.Sprintln("should be set when TLSModeType is Terminate"))) + } + } + } + return errs +} diff --git a/apis/v1beta1/validation/gateway_test.go b/apis/v1beta1/validation/gateway_test.go index 807bdf1c0f..3b262541aa 100644 --- a/apis/v1beta1/validation/gateway_test.go +++ b/apis/v1beta1/validation/gateway_test.go @@ -82,6 +82,16 @@ func TestValidateGateway(t *testing.T) { }, expectErrsOnFields: []string{"spec.listeners[0].hostname"}, }, + "certificatedRefs not set with TLS terminate mode": { + mutate: func(gw *gatewayv1b1.Gateway) { + hostname := gatewayv1b1.Hostname("foo.bar.com") + tlsMode := gatewayv1b1.TLSModeType("Terminate") + gw.Spec.Listeners[0].Hostname = &hostname + gw.Spec.Listeners[0].TLS = &tlsConfig + gw.Spec.Listeners[0].TLS.Mode = &tlsMode + }, + expectErrsOnFields: []string{"spec.listeners[0].tls.certificateRefs"}, + }, } for name, tc := range testCases { From 505ce6ca4662beb319ad331d7cecae3818f955a5 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 18 Oct 2022 11:36:52 +0000 Subject: [PATCH 2/6] Remove check for whether TLS mode is nil and add certificateRefs validation to v1alpha2 Signed-off-by: Huang Xin --- apis/v1alpha2/validation/gateway.go | 16 ++++++++++++++++ apis/v1alpha2/validation/gateway_test.go | 11 +++++++++++ apis/v1beta1/validation/gateway.go | 8 ++++---- apis/v1beta1/validation/gateway_test.go | 1 + 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/apis/v1alpha2/validation/gateway.go b/apis/v1alpha2/validation/gateway.go index b6dd4c9407..7dc9bf297e 100644 --- a/apis/v1alpha2/validation/gateway.go +++ b/apis/v1alpha2/validation/gateway.go @@ -62,6 +62,7 @@ func validateGatewayListeners(listeners []gatewayv1a2.Listener, path *field.Path var errs field.ErrorList errs = append(errs, validateListenerTLSConfig(listeners, path)...) errs = append(errs, validateListenerHostname(listeners, path)...) + errs = append(errs, validateTLSCertificateRefs(listeners, path)...) return errs } @@ -91,3 +92,18 @@ func validateListenerHostname(listeners []gatewayv1a2.Listener, path *field.Path } return errs } + +// validateTLSCertificateRefs validates the certificateRefs +// must be set when tls config is set and TLSModeType is +// terminate +func validateTLSCertificateRefs(listeners []gatewayv1a2.Listener, path *field.Path) field.ErrorList { + var errs field.ErrorList + for i, c := range listeners { + if c.Protocol == gatewayv1a2.HTTPSProtocolType && c.TLS != nil { + if *c.TLS.Mode == gatewayv1a2.TLSModeTerminate && c.TLS.CertificateRefs == nil { + errs = append(errs, field.Forbidden(path.Index(i).Child("tls").Child("certificateRefs"), fmt.Sprintln("should be set and not empty when TLSModeType is Terminate"))) + } + } + } + return errs +} diff --git a/apis/v1alpha2/validation/gateway_test.go b/apis/v1alpha2/validation/gateway_test.go index b4d9200d03..70ed17de13 100644 --- a/apis/v1alpha2/validation/gateway_test.go +++ b/apis/v1alpha2/validation/gateway_test.go @@ -82,6 +82,17 @@ func TestValidateGateway(t *testing.T) { }, expectErrsOnFields: []string{"spec.listeners[0].hostname"}, }, + "certificatedRefs not set with TLS terminate mode": { + mutate: func(gw *gatewayv1a2.Gateway) { + hostname := gatewayv1a2.Hostname("foo.bar.com") + tlsMode := gatewayv1a2.TLSModeType("Terminate") + gw.Spec.Listeners[0].Protocol = gatewayv1a2.HTTPSProtocolType + gw.Spec.Listeners[0].Hostname = &hostname + gw.Spec.Listeners[0].TLS = &tlsConfig + gw.Spec.Listeners[0].TLS.Mode = &tlsMode + }, + expectErrsOnFields: []string{"spec.listeners[0].tls.certificateRefs"}, + }, } for name, tc := range testCases { diff --git a/apis/v1beta1/validation/gateway.go b/apis/v1beta1/validation/gateway.go index 32b93fe4a6..a4e52aae96 100644 --- a/apis/v1beta1/validation/gateway.go +++ b/apis/v1beta1/validation/gateway.go @@ -98,10 +98,10 @@ func validateListenerHostname(listeners []gatewayv1b1.Listener, path *field.Path // terminate func validateTLSCertificateRefs(listeners []gatewayv1b1.Listener, path *field.Path) field.ErrorList { var errs field.ErrorList - for i, h := range listeners { - if h.TLS != nil { - if h.TLS.Mode != nil && *h.TLS.Mode == "Terminate" && h.TLS.CertificateRefs == nil { - errs = append(errs, field.Forbidden(path.Index(i).Child("tls").Child("certificateRefs"), fmt.Sprintln("should be set when TLSModeType is Terminate"))) + for i, c := range listeners { + if c.Protocol == gatewayv1b1.HTTPSProtocolType && c.TLS != nil { + if *c.TLS.Mode == gatewayv1b1.TLSModeTerminate && c.TLS.CertificateRefs == nil { + errs = append(errs, field.Forbidden(path.Index(i).Child("tls").Child("certificateRefs"), fmt.Sprintln("should be set and not empty when TLSModeType is Terminate"))) } } } diff --git a/apis/v1beta1/validation/gateway_test.go b/apis/v1beta1/validation/gateway_test.go index 3b262541aa..38071b1778 100644 --- a/apis/v1beta1/validation/gateway_test.go +++ b/apis/v1beta1/validation/gateway_test.go @@ -86,6 +86,7 @@ func TestValidateGateway(t *testing.T) { mutate: func(gw *gatewayv1b1.Gateway) { hostname := gatewayv1b1.Hostname("foo.bar.com") tlsMode := gatewayv1b1.TLSModeType("Terminate") + gw.Spec.Listeners[0].Protocol = gatewayv1b1.HTTPSProtocolType gw.Spec.Listeners[0].Hostname = &hostname gw.Spec.Listeners[0].TLS = &tlsConfig gw.Spec.Listeners[0].TLS.Mode = &tlsMode From 8b54657d4d4954c2ab46b2bfe8457a4252a6df05 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 18 Oct 2022 21:48:37 +0000 Subject: [PATCH 3/6] Use length to check CertificateRefs Signed-off-by: Huang Xin --- apis/v1alpha2/validation/gateway.go | 2 +- apis/v1beta1/validation/gateway.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apis/v1alpha2/validation/gateway.go b/apis/v1alpha2/validation/gateway.go index 7dc9bf297e..59f1e1d389 100644 --- a/apis/v1alpha2/validation/gateway.go +++ b/apis/v1alpha2/validation/gateway.go @@ -100,7 +100,7 @@ func validateTLSCertificateRefs(listeners []gatewayv1a2.Listener, path *field.Pa var errs field.ErrorList for i, c := range listeners { if c.Protocol == gatewayv1a2.HTTPSProtocolType && c.TLS != nil { - if *c.TLS.Mode == gatewayv1a2.TLSModeTerminate && c.TLS.CertificateRefs == nil { + if *c.TLS.Mode == gatewayv1a2.TLSModeTerminate && len(c.TLS.CertificateRefs) == 0 { errs = append(errs, field.Forbidden(path.Index(i).Child("tls").Child("certificateRefs"), fmt.Sprintln("should be set and not empty when TLSModeType is Terminate"))) } } diff --git a/apis/v1beta1/validation/gateway.go b/apis/v1beta1/validation/gateway.go index a4e52aae96..0ea92578e8 100644 --- a/apis/v1beta1/validation/gateway.go +++ b/apis/v1beta1/validation/gateway.go @@ -100,7 +100,7 @@ func validateTLSCertificateRefs(listeners []gatewayv1b1.Listener, path *field.Pa var errs field.ErrorList for i, c := range listeners { if c.Protocol == gatewayv1b1.HTTPSProtocolType && c.TLS != nil { - if *c.TLS.Mode == gatewayv1b1.TLSModeTerminate && c.TLS.CertificateRefs == nil { + if *c.TLS.Mode == gatewayv1b1.TLSModeTerminate && len(c.TLS.CertificateRefs) == 0 { errs = append(errs, field.Forbidden(path.Index(i).Child("tls").Child("certificateRefs"), fmt.Sprintln("should be set and not empty when TLSModeType is Terminate"))) } } From cf6b856ead8ce9240d8c79cca9ba49d4b289d897 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Tue, 18 Oct 2022 22:49:37 +0000 Subject: [PATCH 4/6] Update comments of validateTLSCertificateRefs Signed-off-by: Huang Xin --- apis/v1alpha2/validation/gateway.go | 4 ++-- apis/v1beta1/validation/gateway.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apis/v1alpha2/validation/gateway.go b/apis/v1alpha2/validation/gateway.go index 59f1e1d389..a571de56c6 100644 --- a/apis/v1alpha2/validation/gateway.go +++ b/apis/v1alpha2/validation/gateway.go @@ -94,8 +94,8 @@ func validateListenerHostname(listeners []gatewayv1a2.Listener, path *field.Path } // validateTLSCertificateRefs validates the certificateRefs -// must be set when tls config is set and TLSModeType is -// terminate +// must be set and not empty when tls config is set and +// TLSModeType is terminate func validateTLSCertificateRefs(listeners []gatewayv1a2.Listener, path *field.Path) field.ErrorList { var errs field.ErrorList for i, c := range listeners { diff --git a/apis/v1beta1/validation/gateway.go b/apis/v1beta1/validation/gateway.go index 0ea92578e8..5f195c70cf 100644 --- a/apis/v1beta1/validation/gateway.go +++ b/apis/v1beta1/validation/gateway.go @@ -94,8 +94,8 @@ func validateListenerHostname(listeners []gatewayv1b1.Listener, path *field.Path } // validateTLSCertificateRefs validates the certificateRefs -// must be set when tls config is set and TLSModeType is -// terminate +// must be set and not empty when tls config is set and +// TLSModeType is terminate func validateTLSCertificateRefs(listeners []gatewayv1b1.Listener, path *field.Path) field.ErrorList { var errs field.ErrorList for i, c := range listeners { From 7b2d1bb592f710a4c45c51b359e9aac74b5b03f4 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Wed, 19 Oct 2022 10:38:05 +0000 Subject: [PATCH 5/6] Keep the v1beta1 validation code and tests, and include that in the v1alpha2 validation Signed-off-by: Huang Xin --- apis/v1alpha2/validation/gateway.go | 21 ++++++--------------- apis/v1alpha2/validation/gateway_test.go | 11 ----------- apis/v1beta1/validation/gateway.go | 6 +++--- 3 files changed, 9 insertions(+), 29 deletions(-) diff --git a/apis/v1alpha2/validation/gateway.go b/apis/v1alpha2/validation/gateway.go index a571de56c6..42dad58a8c 100644 --- a/apis/v1alpha2/validation/gateway.go +++ b/apis/v1alpha2/validation/gateway.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" gatewayv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gatewayvalidationv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1/validation" ) var ( @@ -36,6 +37,11 @@ var ( gatewayv1a2.UDPProtocolType: {}, gatewayv1a2.TCPProtocolType: {}, } + + // ValidateTLSCertificateRefs validates the certificateRefs + // must be set and not empty when tls config is set and + // TLSModeType is terminate + validateTLSCertificateRefs = gatewayvalidationv1b1.ValidateTLSCertificateRefs ) // ValidateGateway validates gw according to the Gateway API specification. @@ -92,18 +98,3 @@ func validateListenerHostname(listeners []gatewayv1a2.Listener, path *field.Path } return errs } - -// validateTLSCertificateRefs validates the certificateRefs -// must be set and not empty when tls config is set and -// TLSModeType is terminate -func validateTLSCertificateRefs(listeners []gatewayv1a2.Listener, path *field.Path) field.ErrorList { - var errs field.ErrorList - for i, c := range listeners { - if c.Protocol == gatewayv1a2.HTTPSProtocolType && c.TLS != nil { - if *c.TLS.Mode == gatewayv1a2.TLSModeTerminate && len(c.TLS.CertificateRefs) == 0 { - errs = append(errs, field.Forbidden(path.Index(i).Child("tls").Child("certificateRefs"), fmt.Sprintln("should be set and not empty when TLSModeType is Terminate"))) - } - } - } - return errs -} diff --git a/apis/v1alpha2/validation/gateway_test.go b/apis/v1alpha2/validation/gateway_test.go index 70ed17de13..b4d9200d03 100644 --- a/apis/v1alpha2/validation/gateway_test.go +++ b/apis/v1alpha2/validation/gateway_test.go @@ -82,17 +82,6 @@ func TestValidateGateway(t *testing.T) { }, expectErrsOnFields: []string{"spec.listeners[0].hostname"}, }, - "certificatedRefs not set with TLS terminate mode": { - mutate: func(gw *gatewayv1a2.Gateway) { - hostname := gatewayv1a2.Hostname("foo.bar.com") - tlsMode := gatewayv1a2.TLSModeType("Terminate") - gw.Spec.Listeners[0].Protocol = gatewayv1a2.HTTPSProtocolType - gw.Spec.Listeners[0].Hostname = &hostname - gw.Spec.Listeners[0].TLS = &tlsConfig - gw.Spec.Listeners[0].TLS.Mode = &tlsMode - }, - expectErrsOnFields: []string{"spec.listeners[0].tls.certificateRefs"}, - }, } for name, tc := range testCases { diff --git a/apis/v1beta1/validation/gateway.go b/apis/v1beta1/validation/gateway.go index 5f195c70cf..060bdaca46 100644 --- a/apis/v1beta1/validation/gateway.go +++ b/apis/v1beta1/validation/gateway.go @@ -62,7 +62,7 @@ func validateGatewayListeners(listeners []gatewayv1b1.Listener, path *field.Path var errs field.ErrorList errs = append(errs, validateListenerTLSConfig(listeners, path)...) errs = append(errs, validateListenerHostname(listeners, path)...) - errs = append(errs, validateTLSCertificateRefs(listeners, path)...) + errs = append(errs, ValidateTLSCertificateRefs(listeners, path)...) return errs } @@ -93,10 +93,10 @@ func validateListenerHostname(listeners []gatewayv1b1.Listener, path *field.Path return errs } -// validateTLSCertificateRefs validates the certificateRefs +// ValidateTLSCertificateRefs validates the certificateRefs // must be set and not empty when tls config is set and // TLSModeType is terminate -func validateTLSCertificateRefs(listeners []gatewayv1b1.Listener, path *field.Path) field.ErrorList { +func ValidateTLSCertificateRefs(listeners []gatewayv1b1.Listener, path *field.Path) field.ErrorList { var errs field.ErrorList for i, c := range listeners { if c.Protocol == gatewayv1b1.HTTPSProtocolType && c.TLS != nil { From 3e80016986c15c12fc6ceafa77675df1ab9f2fca Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Wed, 19 Oct 2022 10:41:51 +0000 Subject: [PATCH 6/6] Fix comments format Signed-off-by: Huang Xin --- apis/v1alpha2/validation/gateway.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apis/v1alpha2/validation/gateway.go b/apis/v1alpha2/validation/gateway.go index 42dad58a8c..5079293604 100644 --- a/apis/v1alpha2/validation/gateway.go +++ b/apis/v1alpha2/validation/gateway.go @@ -39,8 +39,8 @@ var ( } // ValidateTLSCertificateRefs validates the certificateRefs - // must be set and not empty when tls config is set and - // TLSModeType is terminate + // must be set and not empty when tls config is set and + // TLSModeType is terminate validateTLSCertificateRefs = gatewayvalidationv1b1.ValidateTLSCertificateRefs )