diff --git a/api/v1alpha1/dnspolicy_types.go b/api/v1alpha1/dnspolicy_types.go index 0a2fecbb7..5aa5ad64f 100644 --- a/api/v1alpha1/dnspolicy_types.go +++ b/api/v1alpha1/dnspolicy_types.go @@ -54,7 +54,6 @@ const ( ) // DNSPolicySpec defines the desired state of DNSPolicy -// +kubebuilder:validation:XValidation:rule="(!has(oldSelf.loadBalancing) || has(self.loadBalancing)) && (has(oldSelf.loadBalancing) || !has(self.loadBalancing))", message="loadBalancing is immutable" 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'" diff --git a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml index afbe5a2f3..ee0177562 100644 --- a/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml +++ b/bundle/manifests/kuadrant-operator.clusterserviceversion.yaml @@ -106,7 +106,7 @@ metadata: capabilities: Basic Install categories: Integration & Delivery containerImage: quay.io/kuadrant/kuadrant-operator:latest - createdAt: "2024-10-25T15:27:18Z" + createdAt: "2024-10-31T12:05:38Z" description: A Kubernetes Operator to manage the lifecycle of the Kuadrant system operators.operatorframework.io/builder: operator-sdk-v1.32.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 diff --git a/bundle/manifests/kuadrant.io_dnspolicies.yaml b/bundle/manifests/kuadrant.io_dnspolicies.yaml index 98b4226b0..d4ecbc78a 100644 --- a/bundle/manifests/kuadrant.io_dnspolicies.yaml +++ b/bundle/manifests/kuadrant.io_dnspolicies.yaml @@ -227,10 +227,6 @@ spec: - providerRefs - targetRef type: object - x-kubernetes-validations: - - message: loadBalancing is immutable - rule: (!has(oldSelf.loadBalancing) || has(self.loadBalancing)) && (has(oldSelf.loadBalancing) - || !has(self.loadBalancing)) status: description: DNSPolicyStatus defines the observed state of DNSPolicy properties: diff --git a/charts/kuadrant-operator/templates/manifests.yaml b/charts/kuadrant-operator/templates/manifests.yaml index b34da88b7..74d2f6b15 100644 --- a/charts/kuadrant-operator/templates/manifests.yaml +++ b/charts/kuadrant-operator/templates/manifests.yaml @@ -6995,10 +6995,6 @@ spec: - providerRefs - targetRef type: object - x-kubernetes-validations: - - message: loadBalancing is immutable - rule: (!has(oldSelf.loadBalancing) || has(self.loadBalancing)) && (has(oldSelf.loadBalancing) - || !has(self.loadBalancing)) status: description: DNSPolicyStatus defines the observed state of DNSPolicy properties: diff --git a/config/crd/bases/kuadrant.io_dnspolicies.yaml b/config/crd/bases/kuadrant.io_dnspolicies.yaml index d08a38f69..20811e043 100644 --- a/config/crd/bases/kuadrant.io_dnspolicies.yaml +++ b/config/crd/bases/kuadrant.io_dnspolicies.yaml @@ -226,10 +226,6 @@ spec: - providerRefs - targetRef type: object - x-kubernetes-validations: - - message: loadBalancing is immutable - rule: (!has(oldSelf.loadBalancing) || has(self.loadBalancing)) && (has(oldSelf.loadBalancing) - || !has(self.loadBalancing)) status: description: DNSPolicyStatus defines the observed state of DNSPolicy properties: diff --git a/tests/common/dnspolicy/dnspolicy_controller_test.go b/tests/common/dnspolicy/dnspolicy_controller_test.go index 4771f597e..8662dd1bf 100644 --- a/tests/common/dnspolicy/dnspolicy_controller_test.go +++ b/tests/common/dnspolicy/dnspolicy_controller_test.go @@ -100,27 +100,19 @@ var _ = Describe("DNSPolicy controller", func() { WithTargetGateway("test-gateway") Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed()) - // should not allow adding loadBalancing field value after creation + // should allow adding loadBalancing field value after creation Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsPolicy.Spec.LoadBalancing).To(BeNil()) dnsPolicy.Spec.LoadBalancing = &v1alpha1.LoadBalancingSpec{ Weight: 100, Geo: "foo", DefaultGeo: false, } err = k8sClient.Update(ctx, dnsPolicy) - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(ContainSubstring("loadBalancing is immutable"))) + g.Expect(err).To(Succeed()) }, tests.TimeoutMedium, time.Second).Should(Succeed()) - Expect(k8sClient.Delete(ctx, dnsPolicy)).ToNot(HaveOccurred()) - - // loadbalanced should succeed - dnsPolicy = tests.NewDNSPolicy("test-dns-policy", testNamespace). - WithProviderSecret(*dnsProviderSecret). - WithTargetGateway("test-gateway"). - WithLoadBalancingFor(100, "foo", false) - Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed()) // should allow loadBalancing struct fields to be updated Eventually(func(g Gomega) { @@ -137,14 +129,13 @@ var _ = Describe("DNSPolicy controller", func() { g.Expect(err).To(Succeed()) }, tests.TimeoutMedium, time.Second).Should(Succeed()) - // should not allow removing loadBalancing field value after creation + // should allow removing loadBalancing field value after creation Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) g.Expect(err).NotTo(HaveOccurred()) dnsPolicy.Spec.LoadBalancing = nil err = k8sClient.Update(ctx, dnsPolicy) - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(ContainSubstring("loadBalancing is immutable"))) + g.Expect(err).To(Succeed()) }, tests.TimeoutMedium, time.Second).Should(Succeed()) Expect(k8sClient.Delete(ctx, dnsPolicy)).ToNot(HaveOccurred()) }, testTimeOut) @@ -948,6 +939,62 @@ var _ = Describe("DNSPolicy controller", func() { }, tests.TimeoutLong, time.Second).Should(BeNil()) }, testTimeOut) + It("should re-create dns record when loadbalanced section added/removed", func(ctx SpecContext) { + //listener 1 & 2 - Default gateway policy has a loadbalancing section so will create loadbalanced records with CNAME Records for the rootHost + currentRecRootEndpoint := endpointMatcher(tests.HostOne(domain), "CNAME", "", 300, "klb.test."+domain) + currentWildcardRecRootEndpoint := endpointMatcher(tests.HostWildcard(domain), "CNAME", "", 300, "klb."+domain) + + //get the current dnsrecord and wildcard dnsrecord + currentRec := &kuadrantdnsv1alpha1.DNSRecord{} + currentWildcardRec := &kuadrantdnsv1alpha1.DNSRecord{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: recordName, Namespace: testNamespace}, currentRec)).To(Succeed()) + g.Expect(currentRec.Status.Conditions).To(containReadyCondition) + g.Expect(currentRec.Spec.Endpoints).To(ContainElement(currentRecRootEndpoint)) + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: wildcardRecordName, Namespace: testNamespace}, currentWildcardRec)).To(Succeed()) + g.Expect(currentWildcardRec.Status.Conditions).To(containReadyCondition) + g.Expect(currentWildcardRec.Spec.Endpoints).To(ContainElement(currentWildcardRecRootEndpoint)) + }, tests.TimeoutLong, time.Second).Should(BeNil()) + + // should allow removing loadBalancing field value after creation + By("removing the loadBalancing field from the dnspolicy") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) + g.Expect(err).NotTo(HaveOccurred()) + dnsPolicy.Spec.LoadBalancing = nil + err = k8sClient.Update(ctx, dnsPolicy) + g.Expect(err).To(Succeed()) + }, tests.TimeoutMedium, time.Second).Should(Succeed()) + + //listener 1 & 2 - Default gateway policy has no loadbalancing section so will create simple records with A Records for the rootHost + newRecRootEndpoint := endpointMatcher(tests.HostOne(domain), "A", "", 60, tests.IPAddressOne, tests.IPAddressTwo) + newWildcardRecRootEndpoint := endpointMatcher(tests.HostWildcard(domain), "A", "", 60, tests.IPAddressOne, tests.IPAddressTwo) + + //get the dnsrecord again and verify it's no longer the same DNSRecord resource and the record type for the root host has changed + //get the wildcard dnsrecord again and verify it's no longer the same DNSRecord resource and the record type for the root host has changed + Eventually(func(g Gomega) { + newRec := &kuadrantdnsv1alpha1.DNSRecord{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: recordName, Namespace: testNamespace}, newRec)).To(Succeed()) + g.Expect(newRec.Spec.RootHost).To(Equal(currentRec.Spec.RootHost)) + g.Expect(newRec.UID).ToNot(Equal(currentRec.UID)) // if/when we remove the need for record re-creation on policy changes, these assertions can be removed + g.Expect(newRec.Status.Conditions).To(containReadyCondition) + g.Expect(newRec.Spec.Endpoints).To(ContainElement(newRecRootEndpoint)) + + newWildcardRec := &kuadrantdnsv1alpha1.DNSRecord{} + g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: wildcardRecordName, Namespace: testNamespace}, newWildcardRec)).To(Succeed()) + g.Expect(newWildcardRec.Spec.RootHost).To(Equal(currentWildcardRec.Spec.RootHost)) + g.Expect(newWildcardRec.UID).ToNot(Equal(currentWildcardRec.UID)) + g.Expect(newWildcardRec.Status.Conditions).To(containReadyCondition) + g.Expect(newWildcardRec.Spec.Endpoints).To(ContainElement(newWildcardRecRootEndpoint)) + + //ToDo Add checks for policy affected by on gateway when possible, will require discoverability changes + + currentRec = newRec + currentWildcardRec = newWildcardRec + }, tests.TimeoutLong, time.Second).Should(BeNil()) + + }, testTimeOut) + Context("update events", func() { It("should update dns records when policy is updated", func(ctx SpecContext) { endpointMatcher := func(geo string) types.GomegaMatcher { @@ -1073,25 +1120,9 @@ var _ = Describe("DNSPolicy controller", func() { Context("section name", func() { It("should handle policy with section name", func(ctx SpecContext) { - containReadyCondition := ContainElements( - MatchFields(IgnoreExtras, Fields{ - "Type": Equal(string(kuadrantdnsv1alpha1.ConditionTypeReady)), - "Status": Equal(metav1.ConditionTrue), - })) - - rootEndpointMatcher := func(dnsName, recordType, setID string, ttl int64, targets ...string) types.GomegaMatcher { - return PointTo(MatchFields(IgnoreExtras, Fields{ - "DNSName": Equal(dnsName), - "Targets": ConsistOf(targets), - "RecordType": Equal(recordType), - "SetIdentifier": Equal(setID), - "RecordTTL": Equal(externaldns.TTL(ttl)), - })) - } - //listener 1 & 2 - Default gateway policy has a loadbalancing section so will create loadbalanced records with CNAME Records for the rootHost - currentRecRootEndpoint := rootEndpointMatcher(tests.HostOne(domain), "CNAME", "", 300, "klb.test."+domain) - currentWildcardRecRootEndpoint := rootEndpointMatcher(tests.HostWildcard(domain), "CNAME", "", 300, "klb."+domain) + currentRecRootEndpoint := endpointMatcher(tests.HostOne(domain), "CNAME", "", 300, "klb.test."+domain) + currentWildcardRecRootEndpoint := endpointMatcher(tests.HostWildcard(domain), "CNAME", "", 300, "klb."+domain) //get the current dnsrecord and wildcard dnsrecord currentRec := &kuadrantdnsv1alpha1.DNSRecord{} @@ -1113,7 +1144,7 @@ var _ = Describe("DNSPolicy controller", func() { Expect(k8sClient.Create(ctx, dnsPolicyWithSection)).To(Succeed()) //listener 1 - Listener policy has no loadbalancing section so will create simple records with A Records for the rootHost - newRecRootEndpoint := rootEndpointMatcher(tests.HostOne(domain), "A", "", 60, tests.IPAddressOne, tests.IPAddressTwo) + newRecRootEndpoint := endpointMatcher(tests.HostOne(domain), "A", "", 60, tests.IPAddressOne, tests.IPAddressTwo) //listener 2 - Default gateway policy has a loadbalancing section so will create loadbalanced records with CNAME Records for the rootHost newWildcardRecRootEndpoint := currentWildcardRecRootEndpoint @@ -1152,9 +1183,9 @@ var _ = Describe("DNSPolicy controller", func() { }, tests.TimeoutMedium, time.Second).Should(Succeed()) //listener 1 - Default gateway policy has a loadbalancing section so will create loadbalanced records with CNAME Records for the rootHost - newRecRootEndpoint = rootEndpointMatcher(tests.HostOne(domain), "CNAME", "", 300, "klb.test."+domain) + newRecRootEndpoint = endpointMatcher(tests.HostOne(domain), "CNAME", "", 300, "klb.test."+domain) //listener 2 - Listener policy has no loadbalancing section so will create simple records with A Records for the rootHost - newWildcardRecRootEndpoint = rootEndpointMatcher(tests.HostWildcard(domain), "A", "", 60, tests.IPAddressOne, tests.IPAddressTwo) + newWildcardRecRootEndpoint = endpointMatcher(tests.HostWildcard(domain), "A", "", 60, tests.IPAddressOne, tests.IPAddressTwo) //get the dnsrecord again and verify it's no longer the same DNSRecord resource and the record type for the root host has changed //get the wildcard dnsrecord and verify it's no longer the same DNSRecord resource and the record type for the root host has changed @@ -1183,8 +1214,8 @@ var _ = Describe("DNSPolicy controller", func() { Expect(k8sClient.Delete(ctx, dnsPolicyWithSection)).To(Succeed()) //listener 1 & 2 - Default gateway policy has a loadbalancing section so will create loadbalanced records with CNAME Records for the rootHost - newRecRootEndpoint = rootEndpointMatcher(tests.HostOne(domain), "CNAME", "", 300, "klb.test."+domain) - newWildcardRecRootEndpoint = rootEndpointMatcher(tests.HostWildcard(domain), "CNAME", "", 300, "klb."+domain) + newRecRootEndpoint = endpointMatcher(tests.HostOne(domain), "CNAME", "", 300, "klb.test."+domain) + newWildcardRecRootEndpoint = endpointMatcher(tests.HostWildcard(domain), "CNAME", "", 300, "klb."+domain) //get the dnsrecord again and verify the DNSRecord resource is unchanged //get the wildcard dnsrecord and verify it's no longer the same DNSRecord resource and the record type for the root host has changed diff --git a/tests/common/dnspolicy/suite_test.go b/tests/common/dnspolicy/suite_test.go index 19b012a72..4bcb84b20 100644 --- a/tests/common/dnspolicy/suite_test.go +++ b/tests/common/dnspolicy/suite_test.go @@ -25,11 +25,18 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" + "github.com/onsi/gomega/types" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + externaldns "sigs.k8s.io/external-dns/endpoint" + + kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" "github.com/kuadrant/kuadrant-operator/controllers" "github.com/kuadrant/kuadrant-operator/pkg/log" @@ -109,3 +116,21 @@ func TestMain(m *testing.M) { log.SetLogger(logger) os.Exit(m.Run()) } + +var ( + containReadyCondition = ContainElements( + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(kuadrantdnsv1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + })) + + endpointMatcher = func(dnsName, recordType, setID string, ttl int64, targets ...string) types.GomegaMatcher { + return PointTo(MatchFields(IgnoreExtras, Fields{ + "DNSName": Equal(dnsName), + "Targets": ConsistOf(targets), + "RecordType": Equal(recordType), + "SetIdentifier": Equal(setID), + "RecordTTL": Equal(externaldns.TTL(ttl)), + })) + } +)