Update loadbalanced routingStrategy CRD validation

Update the CRD validations to make the "loadBalancing" field required if
the routingStrategy is "loadbalanced" and make the "defaultWeight" and
"defaultGeo" fields mandatory within the "loadBalancing" section.

Minimum valid specs are now:

Simple:
```
spec:
  targetRef:
    name: prod-web
    group: gateway.networking.k8s.io
    kind: Gateway
  routingStrategy: simple
```

LoadBalanced:
```
spec:
  targetRef:
    name: prod-web
    group: gateway.networking.k8s.io
    kind: Gateway
  routingStrategy: loadbalanced
  loadBalancing:
    geo:
      defaultGeo: foo
    weighted:
      defaultWeight: 100
```

api/v1alpha1/dnspolicy_types.go

@@ -44,6 +44,7 @@ const (
 )
 
 // DNSPolicySpec defines the desired state of DNSPolicy
+// +kubebuilder:validation:XValidation:rule="!(self.routingStrategy == 'loadbalanced' && !has(self.loadBalancing))",message="spec.loadBalancing is a required field when spec.routingStrategy == 'loadbalanced'"
 type DNSPolicySpec struct {
 	// TargetRef identifies an API object to apply policy to.
 	// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'",message="Invalid targetRef.group. The only supported value is 'gateway.networking.k8s.io'"
@@ -63,11 +64,9 @@ type DNSPolicySpec struct {
 }
 
 type LoadBalancingSpec struct {
-	// +optional
-	Weighted *LoadBalancingWeighted `json:"weighted,omitempty"`
+	Weighted LoadBalancingWeighted `json:"weighted"`
 
-	// +optional
-	Geo *LoadBalancingGeo `json:"geo,omitempty"`
+	Geo LoadBalancingGeo `json:"geo"`
 }
 
 // +kubebuilder:validation:Minimum=0
@@ -87,7 +86,6 @@ type LoadBalancingWeighted struct {
 	// The maximum value accepted is determined by the target dns provider, please refer to the appropriate docs below.
 	//
 	// Route53: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/routing-policy-weighted.html
-	// +kubebuilder:default=120
 	DefaultWeight Weight `json:"defaultWeight"`
 
 	// custom list of custom weight selectors.
@@ -111,6 +109,7 @@ type LoadBalancingGeo struct {
 	// The values accepted are determined by the target dns provider, please refer to the appropriate docs below.
 	//
 	// Route53: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-geo.html
+	// Google: https://cloud.google.com/compute/docs/regions-zones
 	DefaultGeo string `json:"defaultGeo"`
 }
 
@@ -346,15 +345,15 @@ func (p *DNSPolicy) WithLoadBalancingWeighted(lbWeighted LoadBalancingWeighted)
 	if p.Spec.LoadBalancing == nil {
 		p.WithLoadBalancing(LoadBalancingSpec{})
 	}
-	p.Spec.LoadBalancing.Weighted = &lbWeighted
+	p.Spec.LoadBalancing.Weighted = lbWeighted
 	return p
 }
 
 func (p *DNSPolicy) WithLoadBalancingGeo(lbGeo LoadBalancingGeo) *DNSPolicy {
 	if p.Spec.LoadBalancing == nil {
 		p.Spec.LoadBalancing = &LoadBalancingSpec{}
 	}
-	p.Spec.LoadBalancing.Geo = &lbGeo
+	p.Spec.LoadBalancing.Geo = lbGeo
 	return p
 }
 

config/crd/bases/kuadrant.io_dnspolicies.yaml

@@ -98,6 +98,7 @@ spec:
 
 
                           Route53: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-geo.html
+                          Google: https://cloud.google.com/compute/docs/regions-zones
                         type: string
                     required:
                     - defaultGeo
@@ -165,7 +166,6 @@ spec:
                           type: object
                         type: array
                       defaultWeight:
-                        default: 120
                         description: |-
                           defaultWeight is the record weight to use when no other can be determined for a dns target cluster.
 
@@ -179,6 +179,9 @@ spec:
                     required:
                     - defaultWeight
                     type: object
+                required:
+                - geo
+                - weighted
                 type: object
               routingStrategy:
                 default: loadbalanced
@@ -232,6 +235,10 @@ spec:
             - routingStrategy
             - targetRef
             type: object
+            x-kubernetes-validations:
+            - message: spec.loadBalancing is a required field when spec.routingStrategy
+                == 'loadbalanced'
+              rule: '!(self.routingStrategy == ''loadbalanced'' && !has(self.loadBalancing))'
           status:
             description: DNSPolicyStatus defines the observed state of DNSPolicy
             properties:

controllers/dnspolicy_controller_test.go

@@ -66,6 +66,36 @@ var _ = Describe("DNSPolicy controller", func() {
 		DeleteNamespaceCallback(&testNamespace)()
 	})
 
+	It("should validate routing strategy field correctly", func() {
+		gateway = NewGatewayBuilder("test-gateway", gatewayClass.Name, testNamespace).
+			WithHTTPListener(TestListenerNameOne, TestHostTwo).Gateway
+
+		// simple should succeed
+		dnsPolicy = v1alpha1.NewDNSPolicy("test-dns-policy", testNamespace).
+			WithTargetGateway("test-gateway").
+			WithRoutingStrategy(v1alpha1.SimpleRoutingStrategy)
+		Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed())
+
+		// should not allow changing routing strategy
+		dnsPolicy.Spec.RoutingStrategy = v1alpha1.LoadBalancedRoutingStrategy
+		Expect(k8sClient.Update(ctx, dnsPolicy)).To(MatchError(ContainSubstring("RoutingStrategy is immutable")))
+		Expect(k8sClient.Delete(ctx, dnsPolicy)).ToNot(HaveOccurred())
+
+		// loadbalanced missing loadbalancing field
+		dnsPolicy = v1alpha1.NewDNSPolicy("test-dns-policy", testNamespace).
+			WithTargetGateway("test-gateway").
+			WithRoutingStrategy(v1alpha1.LoadBalancedRoutingStrategy)
+		Expect(k8sClient.Create(ctx, dnsPolicy)).To(MatchError(ContainSubstring("spec.loadBalancing is a required field when spec.routingStrategy == 'loadbalanced'")))
+
+		// loadbalanced should succeed
+		dnsPolicy = v1alpha1.NewDNSPolicy("test-dns-policy", testNamespace).
+			WithTargetGateway("test-gateway").
+			WithRoutingStrategy(v1alpha1.LoadBalancedRoutingStrategy).
+			WithLoadBalancingWeightedFor(100, nil).
+			WithLoadBalancingGeoFor("foo")
+		Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed())
+	})
+
 	Context("invalid target", func() {
 
 		BeforeEach(func() {
@@ -111,17 +141,6 @@ var _ = Describe("DNSPolicy controller", func() {
 			}, TestTimeoutMedium, time.Second).Should(Succeed())
 		})
 
-		It("should not allow changing routing strategy", func() {
-			Eventually(func(g Gomega) {
-				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy)
-				g.Expect(err).NotTo(HaveOccurred())
-				dnsPolicy.Spec.RoutingStrategy = v1alpha1.LoadBalancedRoutingStrategy
-				err = k8sClient.Update(ctx, dnsPolicy)
-				g.Expect(err).To(HaveOccurred())
-				g.Expect(err).To(MatchError(ContainSubstring("RoutingStrategy is immutable")))
-			}, TestTimeoutMedium, time.Second).Should(Succeed())
-		})
-
 		It("should not process gateway with inconsistent addresses", func() {
 			// build invalid gateway
 			gateway = NewGatewayBuilder("test-gateway", gatewayClass.Name, testNamespace).

pkg/multicluster/target.go

@@ -47,14 +47,14 @@ func (t *GatewayTarget) GroupTargetsByGeo() map[v1alpha1.GeoCode][]ClusterGatewa
 }
 
 func (t *GatewayTarget) GetDefaultGeo() v1alpha1.GeoCode {
-	if t.LoadBalancing != nil && t.LoadBalancing.Geo != nil {
+	if t.LoadBalancing != nil {
 		return v1alpha1.GeoCode(t.LoadBalancing.Geo.DefaultGeo)
 	}
 	return v1alpha1.DefaultGeo
 }
 
 func (t *GatewayTarget) GetDefaultWeight() int {
-	if t.LoadBalancing != nil && t.LoadBalancing.Weighted != nil {
+	if t.LoadBalancing != nil {
 		return int(t.LoadBalancing.Weighted.DefaultWeight)
 	}
 	return int(v1alpha1.DefaultWeight)
@@ -64,7 +64,7 @@ func (t *GatewayTarget) setClusterGatewayTargets(clusterGateways []ClusterGatewa
 	cgTargets := []ClusterGatewayTarget{}
 	for _, cg := range clusterGateways {
 		var customWeights []*v1alpha1.CustomWeight
-		if t.LoadBalancing != nil && t.LoadBalancing.Weighted != nil {
+		if t.LoadBalancing != nil {
 			customWeights = t.LoadBalancing.Weighted.Custom
 		}
 		cgt, err := NewClusterGatewayTarget(cg, t.GetDefaultGeo(), t.GetDefaultWeight(), customWeights)