Merge pull request #107 from mikenairn/require_ownerid_and_root_host

Make ownerID and rootHost required in record spec

api/v1alpha1/dnsrecord_types.go

@@ -61,13 +61,14 @@ type HealthCheckStatusProbe struct {
 type DNSRecordSpec struct {
 	// ownerID is a unique string used to identify the owner of this record.
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="OwnerID is immutable"
-	// +optional
-	OwnerID *string `json:"ownerID,omitempty"`
+	// +kubebuilder:validation:MinLength=6
+	// +kubebuilder:validation:MaxLength=12
+	OwnerID string `json:"ownerID"`
 
 	// rootHost is the single root for all endpoints in a DNSRecord.
-	// If rootHost is set, it is expected all defined endpoints are children of or equal to this rootHost
-	// +optional
-	RootHost *string `json:"rootHost,omitempty"`
+	// it is expected all defined endpoints are children of or equal to this rootHost
+	// +kubebuilder:validation:MinLength=1
+	RootHost string `json:"rootHost"`
 
 	// managedZone is a reference to a ManagedZone instance to which this record will publish its endpoints.
 	ManagedZoneRef *ManagedZoneReference `json:"managedZone"`
@@ -173,43 +174,37 @@ const (
 const WildcardPrefix = "*."
 
 func (s *DNSRecord) Validate() error {
-	if s.Spec.RootHost != nil {
-		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")
-		}
+	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")
+	}
 
-		root, _ = strings.CutPrefix(root, WildcardPrefix)
+	root, _ = strings.CutPrefix(root, WildcardPrefix)
 
-		rootEndpointFound := false
-		for _, ep := range s.Spec.Endpoints {
-			if !strings.HasSuffix(ep.DNSName, root) {
-				return fmt.Errorf("invalid endpoint discovered %s all endpoints should be equal to or end with the rootHost %s", ep.DNSName, root)
-			}
-			if !rootEndpointFound {
-				//check original root
-				if ep.DNSName == *s.Spec.RootHost {
-					rootEndpointFound = true
-				}
-			}
+	rootEndpointFound := false
+	for _, ep := range s.Spec.Endpoints {
+		if !strings.HasSuffix(ep.DNSName, root) {
+			return fmt.Errorf("invalid endpoint discovered %s all endpoints should be equal to or end with the rootHost %s", ep.DNSName, root)
 		}
 		if !rootEndpointFound {
-			return fmt.Errorf("invalid endpoint set. rootHost is set but found no endpoint defining a record for the rootHost %s", root)
+			//check original root
+			if ep.DNSName == s.Spec.RootHost {
+				rootEndpointFound = true
+			}
 		}
 	}
+	if !rootEndpointFound {
+		return fmt.Errorf("invalid endpoint set. rootHost is set but found no endpoint defining a record for the rootHost %s", root)
+	}
 	return nil
 }
 
 func (s *DNSRecord) GetRegistry(provider externaldnsprovider.Provider, managedDNSRecordTypes, excludeDNSRecordTypes []string) (externaldnsregistry.Registry, error) {
-	if s.Spec.OwnerID != nil {
-		return registry.NewTXTRegistry(provider, txtRegistryPrefix, txtRegistrySuffix, *s.Spec.OwnerID, txtRegistryCacheInterval,
-			txtRegistryWildcardReplacement, managedDNSRecordTypes, excludeDNSRecordTypes, txtRegistryEncryptEnabled, []byte(txtRegistryEncryptAESKey))
-	} else {
-		return externaldnsregistry.NewNoopRegistry(provider)
-	}
+	return registry.NewTXTRegistry(provider, txtRegistryPrefix, txtRegistrySuffix, s.Spec.OwnerID, txtRegistryCacheInterval,
+		txtRegistryWildcardReplacement, managedDNSRecordTypes, excludeDNSRecordTypes, txtRegistryEncryptEnabled, []byte(txtRegistryEncryptAESKey))
 }
 
 func init() {

api/v1alpha1/dnsrecord_types_test.go

@@ -73,7 +73,7 @@ func TestValidate(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			record := &DNSRecord{
 				Spec: DNSRecordSpec{
-					RootHost: &tt.rootHost,
+					RootHost: tt.rootHost,
 				},
 			}
 			for idx := range tt.dnsNames {

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-04-24T14:40:11Z"
+    createdAt: "2024-05-08T11:03:19Z"
     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

bundle/manifests/kuadrant.io_dnsrecords.yaml

@@ -114,17 +114,22 @@ spec:
               ownerID:
                 description: ownerID is a unique string used to identify the owner
                   of this record.
+                maxLength: 12
+                minLength: 6
                 type: string
                 x-kubernetes-validations:
                 - message: OwnerID is immutable
                   rule: self == oldSelf
               rootHost:
                 description: rootHost is the single root for all endpoints in a DNSRecord.
-                  If rootHost is set, it is expected all defined endpoints are children
-                  of or equal to this rootHost
+                  it is expected all defined endpoints are children of or equal to
+                  this rootHost
+                minLength: 1
                 type: string
             required:
             - managedZone
+            - ownerID
+            - rootHost
             type: object
           status:
             description: DNSRecordStatus defines the observed state of DNSRecord

config/crd/bases/kuadrant.io_dnsrecords.yaml

@@ -114,17 +114,22 @@ spec:
               ownerID:
                 description: ownerID is a unique string used to identify the owner
                   of this record.
+                maxLength: 12
+                minLength: 6
                 type: string
                 x-kubernetes-validations:
                 - message: OwnerID is immutable
                   rule: self == oldSelf
               rootHost:
                 description: rootHost is the single root for all endpoints in a DNSRecord.
-                  If rootHost is set, it is expected all defined endpoints are children
-                  of or equal to this rootHost
+                  it is expected all defined endpoints are children of or equal to
+                  this rootHost
+                minLength: 1
                 type: string
             required:
             - managedZone
+            - ownerID
+            - rootHost
             type: object
           status:
             description: DNSRecordStatus defines the observed state of DNSRecord

docs/reference/dnsrecord.md

@@ -15,8 +15,8 @@
 
 | **Field**        | **Type**                                                                                 | **Required** | **Description**                                                                     |
 |------------------|------------------------------------------------------------------------------------------|:------------:|-------------------------------------------------------------------------------------|
-| `ownerID`        | String                                                                                   |      No      | Unique string used to identify the owner of this record                             | 
-| `rootHost`       | String                                                                                   |      No      | Single root host of all endpoints in a DNSRecord                                    |
+| `ownerID`        | String                                                                                   |     Yes      | Unique string used to identify the owner of this record                             | 
+| `rootHost`       | String                                                                                   |     Yes      | Single root host of all endpoints in a DNSRecord                                    |
 | `managedZone`    | [ManagedZoneReference](#managedzonereference)                                            |     Yes      | Reference to a ManagedZone instance to which this record will publish its endpoints |
 | `endpoints`      | [][ExternalDNS Endpoint](https://pkg.go.dev/sigs.k8s.io/external-dns/endpoint#Endpoint)  |      No      | Endpoints to manage in the dns provider                                             |
 | `healthCheck`    | [HealthCheckSpec](#healthcheckspec)                                                      |      No      | Health check configuration                                                          |

internal/controller/dnsrecord_controller.go

@@ -254,7 +254,7 @@ func (r *DNSRecordReconciler) publishRecord(ctx context.Context, dnsRecord *v1al
 	}
 	expiryTime := metav1.NewTime(dnsRecord.Status.QueuedAt.Add(requeueIn))
 	if !generationChanged(dnsRecord) && reconcileStart.Before(&expiryTime) {
-		logger.V(3).Info("Skipping managed zone to which the DNS dnsRecord is already published and is still valid", "dnsRecord", dnsRecord.Name, "managedZone", managedZone.Name)
+		logger.V(1).Info("Skipping managed zone to which the DNS dnsRecord is already published and is still valid", "dnsRecord", dnsRecord.Name, "managedZone", managedZone.Name)
 		return requeueIn, nil
 	}
 	if generationChanged(dnsRecord) {
@@ -311,20 +311,16 @@ func (r *DNSRecordReconciler) getDNSProvider(ctx context.Context, dnsRecord *v1a
 func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alpha1.DNSRecord, managedZone *v1alpha1.ManagedZone, isDelete bool) (time.Duration, error) {
 	logger := log.FromContext(ctx)
 	zoneDomainName, _ := strings.CutPrefix(managedZone.Spec.DomainName, v1alpha1.WildcardPrefix)
-	var rootDomainName string
-	if dnsRecord.Spec.RootHost != nil {
-		rootDomainName, _ = strings.CutPrefix(*dnsRecord.Spec.RootHost, v1alpha1.WildcardPrefix)
-	}
+	rootDomainName, _ := strings.CutPrefix(dnsRecord.Spec.RootHost, v1alpha1.WildcardPrefix)
 	zoneDomainFilter := externaldnsendpoint.NewDomainFilter([]string{zoneDomainName})
+	managedDNSRecordTypes := []string{externaldnsendpoint.RecordTypeA, externaldnsendpoint.RecordTypeAAAA, externaldnsendpoint.RecordTypeCNAME}
+	excludeDNSRecordTypes := []string{}
 
 	dnsProvider, err := r.getDNSProvider(ctx, dnsRecord)
 	if err != nil {
 		return noRequeueDuration, err
 	}
 
-	managedDNSRecordTypes := []string{externaldnsendpoint.RecordTypeA, externaldnsendpoint.RecordTypeAAAA, externaldnsendpoint.RecordTypeCNAME}
-	excludeDNSRecordTypes := []string{}
-
 	registry, err := dnsRecord.GetRegistry(dnsProvider, managedDNSRecordTypes, excludeDNSRecordTypes)
 	if err != nil {
 		return noRequeueDuration, err
@@ -360,9 +356,9 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp
 	}
 
 	//Note: All endpoint lists should be in the same provider specific format at this point
-	logger.V(3).Info("applyChanges", "zoneEndpoints", zoneEndpoints)
-	logger.V(3).Info("applyChanges", "specEndpoints", specEndpoints)
-	logger.V(3).Info("applyChanges", "statusEndpoints", statusEndpoints)
+	logger.V(1).Info("applyChanges", "zoneEndpoints", zoneEndpoints)
+	logger.V(1).Info("applyChanges", "specEndpoints", specEndpoints)
+	logger.V(1).Info("applyChanges", "statusEndpoints", statusEndpoints)
 
 	plan := &externaldnsplan.Plan{
 		Policies:       []externaldnsplan.Policy{policy},
@@ -391,7 +387,7 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp
 			if dnsRecord.Status.WriteCounter < WriteCounterLimit {
 				dnsRecord.Status.WriteCounter++
 				wrtiteCounter.WithLabelValues(dnsRecord.Name, dnsRecord.Namespace).Inc()
-				logger.V(3).Info("Changes needed on the same generation of record")
+				logger.V(1).Info("Changes needed on the same generation of record")
 			} else {
 				err = errors.New("reached write limit to the DNS provider for the same generation of record")
 				logger.Error(err, "Giving up on trying to maintain desired state of the DNS record - changes are being overridden")