From 42a57bd75207acb0355f5c5340ac713bf3fd8b6a Mon Sep 17 00:00:00 2001 From: Maskym Vavilov Date: Thu, 13 Jun 2024 15:56:34 +0100 Subject: [PATCH] GH-110 use CRD validation on DNSRecord --- api/v1alpha1/dnsrecord_types.go | 26 +++++++++++----- .../dns-operator.clusterserviceversion.yaml | 2 +- bundle/manifests/kuadrant.io_dnsrecords.yaml | 22 ++++++++++++++ config/crd/bases/kuadrant.io_dnsrecords.yaml | 22 ++++++++++++++ .../controller/dnsrecord_controller_test.go | 30 +++++++++++++++++++ .../geo-dnsrecord-healthchecks.yaml | 2 +- 6 files changed, 95 insertions(+), 9 deletions(-) diff --git a/api/v1alpha1/dnsrecord_types.go b/api/v1alpha1/dnsrecord_types.go index 3c147837..b41d3795 100644 --- a/api/v1alpha1/dnsrecord_types.go +++ b/api/v1alpha1/dnsrecord_types.go @@ -35,10 +35,22 @@ const HttpsProtocol HealthProtocol = "HTTPS" // By default this health check will be applied to each unique DNS A Record for // the listeners assigned to the target gateway type HealthCheckSpec struct { - Endpoint string `json:"endpoint,omitempty"` - Port *int `json:"port,omitempty"` - Protocol *HealthProtocol `json:"protocol,omitempty"` - FailureThreshold *int `json:"failureThreshold,omitempty"` + // Endpoint 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}$` + Endpoint string `json:"endpoint,omitempty"` + + // 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"` + + // 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 *HealthProtocol `json:"protocol,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"` } type HealthCheckStatus struct { @@ -68,7 +80,10 @@ type DNSRecordSpec struct { // rootHost is the single root for all endpoints in a DNSRecord. // it is expected all defined endpoints are children of or equal to this rootHost + // Must contain at least two groups of valid URL characters separated by a "." // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=255 + // +kubebuilder:validation:Pattern=`^(?:[\w\-.~:\/?#[\]@!$&'()*+,;=]+)\.(?:[\w\-.~:\/?#[\]@!$&'()*+,;=]+)$` RootHost string `json:"rootHost"` // managedZone is a reference to a ManagedZone instance to which this record will publish its endpoints. @@ -172,9 +187,6 @@ const WildcardPrefix = "*." func (s *DNSRecord) Validate() error { root := s.Spec.RootHost - if len(strings.Split(root, ".")) <= 1 { - return fmt.Errorf("invalid domain format no tld discovered") - } if len(s.Spec.Endpoints) == 0 { return fmt.Errorf("no endpoints defined for DNSRecord. Nothing to do") } diff --git a/bundle/manifests/dns-operator.clusterserviceversion.yaml b/bundle/manifests/dns-operator.clusterserviceversion.yaml index 27eb9593..bc87ab97 100644 --- a/bundle/manifests/dns-operator.clusterserviceversion.yaml +++ b/bundle/manifests/dns-operator.clusterserviceversion.yaml @@ -56,7 +56,7 @@ metadata: capabilities: Basic Install categories: Integration & Delivery containerImage: quay.io/kuadrant/dns-operator:latest - createdAt: "2024-06-27T16:25:50Z" + createdAt: "2024-06-28T14:24:36Z" 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_dnsrecords.yaml b/bundle/manifests/kuadrant.io_dnsrecords.yaml index 27bc7c21..4b10579c 100644 --- a/bundle/manifests/kuadrant.io_dnsrecords.yaml +++ b/bundle/manifests/kuadrant.io_dnsrecords.yaml @@ -99,13 +99,32 @@ spec: the listeners assigned to the target gateway properties: endpoint: + description: |- + Endpoint 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 + pattern: ^(?:\?|\/)[\w\-.~:\/?#\[\]@!$&'()*+,;=]+(?:[a-zA-Z0-9]|\/){1}$ type: string failureThreshold: + description: FailureThreshold is a limit of consecutive failures + that must occur for a host to be considered unhealthy type: integer + x-kubernetes-validations: + - message: Failure threshold must be greater than 0 + rule: self > 0 port: + description: Port to connect to the host on. Must be either 80, + 443 or 1024-49151 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" type: string + x-kubernetes-validations: + - message: Only HTTP or HTTPS protocols are allowed + rule: self in ['HTTP','HTTPS'] type: object managedZone: description: managedZone is a reference to a ManagedZone instance @@ -133,7 +152,10 @@ spec: description: |- rootHost is the single root for all endpoints in a DNSRecord. it is expected all defined endpoints are children of or equal to this rootHost + Must contain at least two groups of valid URL characters separated by a "." + maxLength: 255 minLength: 1 + pattern: ^(?:[\w\-.~:\/?#[\]@!$&'()*+,;=]+)\.(?:[\w\-.~:\/?#[\]@!$&'()*+,;=]+)$ type: string required: - managedZone diff --git a/config/crd/bases/kuadrant.io_dnsrecords.yaml b/config/crd/bases/kuadrant.io_dnsrecords.yaml index fb8c4524..1c2fd38f 100644 --- a/config/crd/bases/kuadrant.io_dnsrecords.yaml +++ b/config/crd/bases/kuadrant.io_dnsrecords.yaml @@ -99,13 +99,32 @@ spec: the listeners assigned to the target gateway properties: endpoint: + description: |- + Endpoint 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 + pattern: ^(?:\?|\/)[\w\-.~:\/?#\[\]@!$&'()*+,;=]+(?:[a-zA-Z0-9]|\/){1}$ type: string failureThreshold: + description: FailureThreshold is a limit of consecutive failures + that must occur for a host to be considered unhealthy type: integer + x-kubernetes-validations: + - message: Failure threshold must be greater than 0 + rule: self > 0 port: + description: Port to connect to the host on. Must be either 80, + 443 or 1024-49151 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" type: string + x-kubernetes-validations: + - message: Only HTTP or HTTPS protocols are allowed + rule: self in ['HTTP','HTTPS'] type: object managedZone: description: managedZone is a reference to a ManagedZone instance @@ -133,7 +152,10 @@ spec: description: |- rootHost is the single root for all endpoints in a DNSRecord. it is expected all defined endpoints are children of or equal to this rootHost + Must contain at least two groups of valid URL characters separated by a "." + maxLength: 255 minLength: 1 + pattern: ^(?:[\w\-.~:\/?#[\]@!$&'()*+,;=]+)\.(?:[\w\-.~:\/?#[\]@!$&'()*+,;=]+)$ type: string required: - managedZone diff --git a/internal/controller/dnsrecord_controller_test.go b/internal/controller/dnsrecord_controller_test.go index 7c476e6b..f7169bdc 100644 --- a/internal/controller/dnsrecord_controller_test.go +++ b/internal/controller/dnsrecord_controller_test.go @@ -28,6 +28,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" externaldnsendpoint "sigs.k8s.io/external-dns/endpoint" @@ -111,6 +112,35 @@ var _ = Describe("DNSRecordReconciler", func() { } }) + It("prevents creation of invalid records", func(ctx SpecContext) { + dnsRecord = &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar.example.com", + Namespace: testNamespace, + }, + Spec: v1alpha1.DNSRecordSpec{ + RootHost: "bar.example .com", + ManagedZoneRef: &v1alpha1.ManagedZoneReference{ + Name: managedZone.Name, + }, + Endpoints: getTestEndpoints("bar.example.com", "127.0.0.1"), + HealthCheck: &v1alpha1.HealthCheckSpec{ + Endpoint: "health", + Port: ptr.To(5), + Protocol: ptr.To(v1alpha1.HealthProtocol("cat")), + FailureThreshold: ptr.To(-1), + }, + }, + } + err := k8sClient.Create(ctx, dnsRecord) + Expect(err).To(MatchError(ContainSubstring("spec.rootHost: Invalid value"))) + Expect(err).To(MatchError(ContainSubstring("spec.healthCheck.endpoint: Invalid value"))) + Expect(err).To(MatchError(ContainSubstring("Only ports 80, 443, 1024-49151 are allowed"))) + Expect(err).To(MatchError(ContainSubstring("Only HTTP or HTTPS protocols are allowed"))) + Expect(err).To(MatchError(ContainSubstring("Failure threshold must be greater than 0"))) + + }) + It("handles records with similar root hosts", func(ctx SpecContext) { dnsRecord = &v1alpha1.DNSRecord{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/e2e/fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml b/test/e2e/fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml index ffd3ab85..a2df6420 100644 --- a/test/e2e/fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml +++ b/test/e2e/fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml @@ -5,7 +5,7 @@ metadata: namespace: ${testNamespace} spec: healthCheck: - endpoint: "/" + endpoint: "/health" port: 80 protocol: "HTTPS" failureThreshold: 3