From 78b00dacee2421b4b1109b298d4a87d19ce3711e Mon Sep 17 00:00:00 2001 From: Maskym Vavilov Date: Wed, 13 Nov 2024 10:43:08 +0000 Subject: [PATCH] refactor CEL validation on healthcheck specs Signed-off-by: Maskym Vavilov --- api/v1alpha1/dnshealthcheckprobe_types.go | 16 +++-- api/v1alpha1/dnsrecord_types.go | 17 ++++- api/v1alpha1/zz_generated.deepcopy.go | 12 +++- .../dns-operator.clusterserviceversion.yaml | 2 +- .../kuadrant.io_dnshealthcheckprobes.yaml | 9 ++- bundle/manifests/kuadrant.io_dnsrecords.yaml | 24 ++++--- charts/dns-operator/templates/manifests.yaml | 33 ++++++---- .../kuadrant.io_dnshealthcheckprobes.yaml | 9 ++- config/crd/bases/kuadrant.io_dnsrecords.yaml | 24 ++++--- .../controller/dnsrecord_healthchecks_test.go | 62 ++++++++++++++++++- internal/controller/helper_test.go | 2 +- internal/probes/worker_test.go | 2 +- 12 files changed, 160 insertions(+), 52 deletions(-) diff --git a/api/v1alpha1/dnshealthcheckprobe_types.go b/api/v1alpha1/dnshealthcheckprobe_types.go index e1ce752..8f52c47 100644 --- a/api/v1alpha1/dnshealthcheckprobe_types.go +++ b/api/v1alpha1/dnshealthcheckprobe_types.go @@ -28,29 +28,37 @@ type DNSHealthCheckProbeSpec struct { // Port to connect to the host on. Must be either 80, 443 or 1024-49151 // +kubebuilder:validation:XValidation:rule="self in [80, 443] || (self >= 1024 && self <= 49151)",message="Only ports 80, 443, 1024-49151 are allowed" Port int `json:"port,omitempty"` + // Hostname is the value sent in the host header, to route the request to the correct service - // +kubebuilder:validation:Pattern=`^[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+$` + // Represents a root host of the parent DNS Record. Hostname string `json:"hostname,omitempty"` + // Address to connect to the host on (IP Address (A Record) or hostname (CNAME)). - // +kubebuilder:validation:Pattern=`^([1-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?|[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+)?$` Address string `json:"address,omitempty"` + // Path is the path to append to the host to reach the expected health check. // Must start with "?" or "/", contain only valid URL characters and end with alphanumeric char or "/". For example "/" or "/healthz" are common // +kubebuilder:validation:Pattern=`^(?:\?|\/)[\w\-.~:\/?#\[\]@!$&'()*+,;=]+(?:[a-zA-Z0-9]|\/){1}$` Path string `json:"path,omitempty"` + // Protocol to use when connecting to the host, valid values are "HTTP" or "HTTPS" // +kubebuilder:validation:XValidation:rule="self in ['HTTP','HTTPS']",message="Only HTTP or HTTPS protocols are allowed" Protocol Protocol `json:"protocol,omitempty"` + // Interval defines how frequently this probe should execute - Interval metav1.Duration `json:"interval,omitempty"` + Interval *metav1.Duration `json:"interval,omitempty"` + // AdditionalHeadersRef refers to a secret that contains extra headers to send in the probe request, this is primarily useful if an authentication // token is required by the endpoint. + // +optional AdditionalHeadersRef *AdditionalHeadersRef `json:"additionalHeadersRef,omitempty"` + // FailureThreshold is a limit of consecutive failures that must occur for a host to be considered unhealthy // +kubebuilder:validation:XValidation:rule="self > 0",message="Failure threshold must be greater than 0" FailureThreshold int `json:"failureThreshold,omitempty"` + // AllowInsecureCertificate will instruct the health check probe to not fail on a self-signed or otherwise invalid SSL certificate - // this is primarily used in development or testing environments + // this is primarily used in development or testing environments and is set by the --insecure-health-checks flag AllowInsecureCertificate bool `json:"allowInsecureCertificate,omitempty"` } diff --git a/api/v1alpha1/dnsrecord_types.go b/api/v1alpha1/dnsrecord_types.go index 1dc7924..0545489 100644 --- a/api/v1alpha1/dnsrecord_types.go +++ b/api/v1alpha1/dnsrecord_types.go @@ -36,23 +36,36 @@ const HttpsProtocol Protocol = "HTTPS" // the listeners assigned to the target gateway type HealthCheckSpec struct { // Port to connect to the host on. Must be either 80, 443 or 1024-49151 + // Defaults to port 443 // +kubebuilder:validation:XValidation:rule="self in [80, 443] || (self >= 1024 && self <= 49151)",message="Only ports 80, 443, 1024-49151 are allowed" + // +kubebuilder:default=443 Port int `json:"port,omitempty"` + // Path is the path to append to the host to reach the expected health check. // Must start with "?" or "/", contain only valid URL characters and end with alphanumeric char or "/". For example "/" or "/healthz" are common // +kubebuilder:validation:Pattern=`^(?:\?|\/)[\w\-.~:\/?#\[\]@!$&'()*+,;=]+(?:[a-zA-Z0-9]|\/){1}$` Path string `json:"path,omitempty"` + // Protocol to use when connecting to the host, valid values are "HTTP" or "HTTPS" + // Defaults to HTTPS // +kubebuilder:validation:XValidation:rule="self in ['HTTP','HTTPS']",message="Only HTTP or HTTPS protocols are allowed" + // +kubebuilder:default=HTTPS Protocol Protocol `json:"protocol,omitempty"` + // Interval defines how frequently this probe should execute - // +default:5m - Interval metav1.Duration `json:"interval,omitempty"` + // Defaults to 5 minutes + // +kubebuilder:default="5m" + Interval *metav1.Duration `json:"interval,omitempty"` + // AdditionalHeadersRef refers to a secret that contains extra headers to send in the probe request, this is primarily useful if an authentication // token is required by the endpoint. + // +optional AdditionalHeadersRef *AdditionalHeadersRef `json:"additionalHeadersRef,omitempty"` + // FailureThreshold is a limit of consecutive failures that must occur for a host to be considered unhealthy + // Defaults to 5 // +kubebuilder:validation:XValidation:rule="self > 0",message="Failure threshold must be greater than 0" + // +kubebuilder:default=5 FailureThreshold int `json:"failureThreshold,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 12ae890..0544a5b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -137,7 +137,11 @@ func (in *DNSHealthCheckProbeList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DNSHealthCheckProbeSpec) DeepCopyInto(out *DNSHealthCheckProbeSpec) { *out = *in - out.Interval = in.Interval + if in.Interval != nil { + in, out := &in.Interval, &out.Interval + *out = new(v1.Duration) + **out = **in + } if in.AdditionalHeadersRef != nil { in, out := &in.AdditionalHeadersRef, &out.AdditionalHeadersRef *out = new(AdditionalHeadersRef) @@ -325,7 +329,11 @@ func (in *DNSRecordStatus) DeepCopy() *DNSRecordStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HealthCheckSpec) DeepCopyInto(out *HealthCheckSpec) { *out = *in - out.Interval = in.Interval + if in.Interval != nil { + in, out := &in.Interval, &out.Interval + *out = new(v1.Duration) + **out = **in + } if in.AdditionalHeadersRef != nil { in, out := &in.AdditionalHeadersRef, &out.AdditionalHeadersRef *out = new(AdditionalHeadersRef) diff --git a/bundle/manifests/dns-operator.clusterserviceversion.yaml b/bundle/manifests/dns-operator.clusterserviceversion.yaml index cb36b11..42ff14c 100644 --- a/bundle/manifests/dns-operator.clusterserviceversion.yaml +++ b/bundle/manifests/dns-operator.clusterserviceversion.yaml @@ -58,7 +58,7 @@ metadata: capabilities: Basic Install categories: Integration & Delivery containerImage: quay.io/kuadrant/dns-operator:latest - createdAt: "2024-11-07T15:15:46Z" + createdAt: "2024-11-14T13:59:23Z" description: A Kubernetes Operator to manage the lifecycle of DNS resources operators.operatorframework.io/builder: operator-sdk-v1.33.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 diff --git a/bundle/manifests/kuadrant.io_dnshealthcheckprobes.yaml b/bundle/manifests/kuadrant.io_dnshealthcheckprobes.yaml index 4f77c9c..8ba7058 100644 --- a/bundle/manifests/kuadrant.io_dnshealthcheckprobes.yaml +++ b/bundle/manifests/kuadrant.io_dnshealthcheckprobes.yaml @@ -62,12 +62,11 @@ spec: address: description: Address to connect to the host on (IP Address (A Record) or hostname (CNAME)). - pattern: ^([1-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?|[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+)?$ type: string allowInsecureCertificate: description: |- AllowInsecureCertificate will instruct the health check probe to not fail on a self-signed or otherwise invalid SSL certificate - this is primarily used in development or testing environments + this is primarily used in development or testing environments and is set by the --insecure-health-checks flag type: boolean failureThreshold: description: FailureThreshold is a limit of consecutive failures that @@ -77,9 +76,9 @@ spec: - message: Failure threshold must be greater than 0 rule: self > 0 hostname: - description: Hostname is the value sent in the host header, to route - the request to the correct service - pattern: ^[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+$ + description: |- + Hostname is the value sent in the host header, to route the request to the correct service + Represents a root host of the parent DNS Record. type: string interval: description: Interval defines how frequently this probe should execute diff --git a/bundle/manifests/kuadrant.io_dnsrecords.yaml b/bundle/manifests/kuadrant.io_dnsrecords.yaml index 8dae514..eeec44e 100644 --- a/bundle/manifests/kuadrant.io_dnsrecords.yaml +++ b/bundle/manifests/kuadrant.io_dnsrecords.yaml @@ -109,15 +109,19 @@ spec: - name type: object failureThreshold: - description: FailureThreshold is a limit of consecutive failures - that must occur for a host to be considered unhealthy + default: 5 + description: |- + FailureThreshold is a limit of consecutive failures that must occur for a host to be considered unhealthy + Defaults to 5 type: integer x-kubernetes-validations: - message: Failure threshold must be greater than 0 rule: self > 0 interval: - description: Interval defines how frequently this probe should - execute + default: 5m + description: |- + Interval defines how frequently this probe should execute + Defaults to 5 minutes type: string path: description: |- @@ -126,15 +130,19 @@ spec: pattern: ^(?:\?|\/)[\w\-.~:\/?#\[\]@!$&'()*+,;=]+(?:[a-zA-Z0-9]|\/){1}$ type: string port: - description: Port to connect to the host on. Must be either 80, - 443 or 1024-49151 + default: 443 + description: |- + Port to connect to the host on. Must be either 80, 443 or 1024-49151 + Defaults to port 443 type: integer x-kubernetes-validations: - message: Only ports 80, 443, 1024-49151 are allowed rule: self in [80, 443] || (self >= 1024 && self <= 49151) protocol: - description: Protocol to use when connecting to the host, valid - values are "HTTP" or "HTTPS" + default: HTTPS + description: |- + Protocol to use when connecting to the host, valid values are "HTTP" or "HTTPS" + Defaults to HTTPS type: string x-kubernetes-validations: - message: Only HTTP or HTTPS protocols are allowed diff --git a/charts/dns-operator/templates/manifests.yaml b/charts/dns-operator/templates/manifests.yaml index 49d6771..77d1e97 100644 --- a/charts/dns-operator/templates/manifests.yaml +++ b/charts/dns-operator/templates/manifests.yaml @@ -63,12 +63,11 @@ spec: address: description: Address to connect to the host on (IP Address (A Record) or hostname (CNAME)). - pattern: ^([1-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?|[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+)?$ type: string allowInsecureCertificate: description: |- AllowInsecureCertificate will instruct the health check probe to not fail on a self-signed or otherwise invalid SSL certificate - this is primarily used in development or testing environments + this is primarily used in development or testing environments and is set by the --insecure-health-checks flag type: boolean failureThreshold: description: FailureThreshold is a limit of consecutive failures that @@ -78,9 +77,9 @@ spec: - message: Failure threshold must be greater than 0 rule: self > 0 hostname: - description: Hostname is the value sent in the host header, to route - the request to the correct service - pattern: ^[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+$ + description: |- + Hostname is the value sent in the host header, to route the request to the correct service + Represents a root host of the parent DNS Record. type: string interval: description: Interval defines how frequently this probe should execute @@ -241,15 +240,19 @@ spec: - name type: object failureThreshold: - description: FailureThreshold is a limit of consecutive failures - that must occur for a host to be considered unhealthy + default: 5 + description: |- + FailureThreshold is a limit of consecutive failures that must occur for a host to be considered unhealthy + Defaults to 5 type: integer x-kubernetes-validations: - message: Failure threshold must be greater than 0 rule: self > 0 interval: - description: Interval defines how frequently this probe should - execute + default: 5m + description: |- + Interval defines how frequently this probe should execute + Defaults to 5 minutes type: string path: description: |- @@ -258,15 +261,19 @@ spec: pattern: ^(?:\?|\/)[\w\-.~:\/?#\[\]@!$&'()*+,;=]+(?:[a-zA-Z0-9]|\/){1}$ type: string port: - description: Port to connect to the host on. Must be either 80, - 443 or 1024-49151 + default: 443 + description: |- + Port to connect to the host on. Must be either 80, 443 or 1024-49151 + Defaults to port 443 type: integer x-kubernetes-validations: - message: Only ports 80, 443, 1024-49151 are allowed rule: self in [80, 443] || (self >= 1024 && self <= 49151) protocol: - description: Protocol to use when connecting to the host, valid - values are "HTTP" or "HTTPS" + default: HTTPS + description: |- + Protocol to use when connecting to the host, valid values are "HTTP" or "HTTPS" + Defaults to HTTPS type: string x-kubernetes-validations: - message: Only HTTP or HTTPS protocols are allowed diff --git a/config/crd/bases/kuadrant.io_dnshealthcheckprobes.yaml b/config/crd/bases/kuadrant.io_dnshealthcheckprobes.yaml index 16dca83..c02a788 100644 --- a/config/crd/bases/kuadrant.io_dnshealthcheckprobes.yaml +++ b/config/crd/bases/kuadrant.io_dnshealthcheckprobes.yaml @@ -62,12 +62,11 @@ spec: address: description: Address to connect to the host on (IP Address (A Record) or hostname (CNAME)). - pattern: ^([1-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?\.[0-9][0-9]?[0-9]?|[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+)?$ type: string allowInsecureCertificate: description: |- AllowInsecureCertificate will instruct the health check probe to not fail on a self-signed or otherwise invalid SSL certificate - this is primarily used in development or testing environments + this is primarily used in development or testing environments and is set by the --insecure-health-checks flag type: boolean failureThreshold: description: FailureThreshold is a limit of consecutive failures that @@ -77,9 +76,9 @@ spec: - message: Failure threshold must be greater than 0 rule: self > 0 hostname: - description: Hostname is the value sent in the host header, to route - the request to the correct service - pattern: ^[a-z][a-z0-9\-]+\.([a-z][a-z0-9\-]+\.)*[a-z][a-z0-9\-]+$ + description: |- + Hostname is the value sent in the host header, to route the request to the correct service + Represents a root host of the parent DNS Record. type: string interval: description: Interval defines how frequently this probe should execute diff --git a/config/crd/bases/kuadrant.io_dnsrecords.yaml b/config/crd/bases/kuadrant.io_dnsrecords.yaml index d993a3c..59a5c6e 100644 --- a/config/crd/bases/kuadrant.io_dnsrecords.yaml +++ b/config/crd/bases/kuadrant.io_dnsrecords.yaml @@ -109,15 +109,19 @@ spec: - name type: object failureThreshold: - description: FailureThreshold is a limit of consecutive failures - that must occur for a host to be considered unhealthy + default: 5 + description: |- + FailureThreshold is a limit of consecutive failures that must occur for a host to be considered unhealthy + Defaults to 5 type: integer x-kubernetes-validations: - message: Failure threshold must be greater than 0 rule: self > 0 interval: - description: Interval defines how frequently this probe should - execute + default: 5m + description: |- + Interval defines how frequently this probe should execute + Defaults to 5 minutes type: string path: description: |- @@ -126,15 +130,19 @@ spec: pattern: ^(?:\?|\/)[\w\-.~:\/?#\[\]@!$&'()*+,;=]+(?:[a-zA-Z0-9]|\/){1}$ type: string port: - description: Port to connect to the host on. Must be either 80, - 443 or 1024-49151 + default: 443 + description: |- + Port to connect to the host on. Must be either 80, 443 or 1024-49151 + Defaults to port 443 type: integer x-kubernetes-validations: - message: Only ports 80, 443, 1024-49151 are allowed rule: self in [80, 443] || (self >= 1024 && self <= 49151) protocol: - description: Protocol to use when connecting to the host, valid - values are "HTTP" or "HTTPS" + default: HTTPS + description: |- + Protocol to use when connecting to the host, valid values are "HTTP" or "HTTPS" + Defaults to HTTPS type: string x-kubernetes-validations: - message: Only HTTP or HTTPS protocols are allowed diff --git a/internal/controller/dnsrecord_healthchecks_test.go b/internal/controller/dnsrecord_healthchecks_test.go index 72c8b40..255244b 100644 --- a/internal/controller/dnsrecord_healthchecks_test.go +++ b/internal/controller/dnsrecord_healthchecks_test.go @@ -83,7 +83,7 @@ var _ = Describe("DNSRecordReconciler_HealthChecks", func() { "Address": Equal("172.32.200.1"), "Path": Equal("/healthz"), "Protocol": Equal(v1alpha1.Protocol("HTTPS")), - "Interval": Equal(metav1.Duration{Duration: time.Minute}), + "Interval": PointTo(Equal(metav1.Duration{Duration: time.Minute})), "AdditionalHeadersRef": PointTo(MatchFields(IgnoreExtras, Fields{ "Name": Equal("headers"), })), @@ -102,7 +102,7 @@ var _ = Describe("DNSRecordReconciler_HealthChecks", func() { "Address": Equal("172.32.200.2"), "Path": Equal("/healthz"), "Protocol": Equal(v1alpha1.Protocol("HTTPS")), - "Interval": Equal(metav1.Duration{Duration: time.Minute}), + "Interval": PointTo(Equal(metav1.Duration{Duration: time.Minute})), "AdditionalHeadersRef": PointTo(MatchFields(IgnoreExtras, Fields{ "Name": Equal("headers"), })), @@ -167,6 +167,64 @@ var _ = Describe("DNSRecordReconciler_HealthChecks", func() { }, TestTimeoutMedium, time.Second).Should(Succeed()) }) + It("Should create valid probe CRs with default values", func() { + //Create test dnsrecord with nils for optional fields + dnsRecord.Spec.HealthCheck = &v1alpha1.HealthCheckSpec{ + Path: "/health", + } + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + + By("Validating created probes") + Eventually(func(g Gomega) { + probes := &v1alpha1.DNSHealthCheckProbeList{} + + g.Expect(k8sClient.List(ctx, probes, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord), + }), + Namespace: dnsRecord.Namespace, + })).To(Succeed()) + + g.Expect(probes.Items).To(HaveLen(2)) + g.Expect(probes.Items).To(ConsistOf( + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal(fmt.Sprintf("%s-%s", dnsRecord.Name, "172.32.200.1")), + "Namespace": Equal(testNamespace), + }), + "Spec": MatchFields(IgnoreExtras, Fields{ + "Port": Equal(443), + "Hostname": Equal(testHostname), + "Address": Equal("172.32.200.1"), + "Path": Equal("/health"), + "Protocol": Equal(v1alpha1.Protocol("HTTPS")), + "Interval": PointTo(Equal(metav1.Duration{Duration: 5 * time.Minute})), + "AdditionalHeadersRef": BeNil(), + "FailureThreshold": Equal(5), + "AllowInsecureCertificate": Equal(true), + }), + }), + MatchFields(IgnoreExtras, Fields{ + "ObjectMeta": MatchFields(IgnoreExtras, Fields{ + "Name": Equal(fmt.Sprintf("%s-%s", dnsRecord.Name, "172.32.200.2")), + "Namespace": Equal(testNamespace), + }), + "Spec": MatchFields(IgnoreExtras, Fields{ + "Port": Equal(443), + "Hostname": Equal(testHostname), + "Address": Equal("172.32.200.2"), + "Path": Equal("/health"), + "Protocol": Equal(v1alpha1.Protocol("HTTPS")), + "Interval": PointTo(Equal(metav1.Duration{Duration: 5 * time.Minute})), + "AdditionalHeadersRef": BeNil(), + "FailureThreshold": Equal(5), + "AllowInsecureCertificate": Equal(true), + }), + }), + )) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + }) + It("Should not publish unhealthy enpoints", func() { //Create default test dnsrecord Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) diff --git a/internal/controller/helper_test.go b/internal/controller/helper_test.go index f89542f..7df3d40 100644 --- a/internal/controller/helper_test.go +++ b/internal/controller/helper_test.go @@ -50,7 +50,7 @@ func getTestHealthCheckSpec() *v1alpha1.HealthCheckSpec { Port: 443, Protocol: "HTTPS", FailureThreshold: 5, - Interval: metav1.Duration{Duration: time.Minute}, + Interval: &metav1.Duration{Duration: time.Minute}, AdditionalHeadersRef: &v1alpha1.AdditionalHeadersRef{ Name: "headers", }, diff --git a/internal/probes/worker_test.go b/internal/probes/worker_test.go index 0bf33d0..aeedff2 100644 --- a/internal/probes/worker_test.go +++ b/internal/probes/worker_test.go @@ -34,7 +34,7 @@ func TestWorker_ProbeSuccess(t *testing.T) { Hostname: "example.com", Address: "1.2.1.1", Path: "/test", - Interval: v1.Duration{Duration: time.Second}, + Interval: &v1.Duration{Duration: time.Second}, Port: 80, Protocol: v1alpha1.HttpProtocol, FailureThreshold: 3,