ManagedZone updates

* Make description optional
* fix managedZoneListToZoneList - use range index
* Make ID and description pointers
* Remove hostedzone prefix from AWS hosted zones
* Use status id in managedZoneListToZoneList

config/policy-controller/crd/bases/kuadrant.io_managedzones.yaml

@@ -91,7 +91,6 @@ spec:
                 - name
                 type: object
             required:
-            - description
             - domainName
             - providerRef
             type: object

pkg/apis/v1alpha2/managedzone_types.go

@@ -31,12 +31,14 @@ type ManagedZoneReference struct {
 type ManagedZoneSpec struct {
 	// ID is the provider assigned id of this  zone (i.e. route53.HostedZone.ID).
 	// +optional
-	ID string `json:"id,omitempty"`
+	ID *string `json:"id,omitempty"`
 	//Domain name of this ManagedZone
+	// +required
 	// +kubebuilder:validation:Pattern=`^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`
 	DomainName string `json:"domainName"`
 	//Description for this ManagedZone
-	Description string `json:"description"`
+	// +optional
+	Description *string `json:"description"`
 	// Reference to another managed zone that this managed zone belongs to.
 	// +optional
 	ParentManagedZone *ManagedZoneReference `json:"parentManagedZone,omitempty"`
@@ -97,13 +99,6 @@ type ManagedZoneList struct {
 	Items           []ManagedZone `json:"items"`
 }
 
-type ManagedHost struct {
-	Subdomain   string
-	Host        string
-	ManagedZone *ManagedZone
-	DnsRecord   *DNSRecord
-}
-
 func init() {
 	SchemeBuilder.Register(&ManagedZone{}, &ManagedZoneList{})
 }

pkg/controllers/managedzone/managedzone_controller.go

@@ -180,7 +180,7 @@ func (r *ManagedZoneReconciler) publishManagedZone(ctx context.Context, managedZ
 }
 
 func (r *ManagedZoneReconciler) deleteManagedZone(ctx context.Context, managedZone *v1alpha2.ManagedZone) error {
-	if managedZone.Spec.ID != "" {
+	if managedZone.Spec.ID != nil {
 		log.Log.Info("Skipping deletion of managed zone with provider ID specified in spec", "managedZone", managedZone.Name)
 		return nil
 	}

pkg/dns/aws/dns.go

@@ -114,7 +114,7 @@ func (p *Route53DNSProvider) ListZones() (dns.ZoneList, error) {
 	}
 	for _, zone := range zones {
 		dnsName := removeTrailingDot(*zone.Name)
-		zoneID := strings.TrimPrefix(*zone.Id, "/hostedzone/")
+		zoneID := removeHostedZoneIDPrefix(*zone.Id)
 		zoneList.Items = append(zoneList.Items, &dns.Zone{
 			ID:      &zoneID,
 			DNSName: &dnsName,
@@ -125,8 +125,8 @@ func (p *Route53DNSProvider) ListZones() (dns.ZoneList, error) {
 
 func (p *Route53DNSProvider) EnsureManagedZone(zone *v1alpha2.ManagedZone) (dns.ManagedZoneOutput, error) {
 	var zoneID string
-	if zone.Spec.ID != "" {
-		zoneID = zone.Spec.ID
+	if zone.Spec.ID != nil {
+		zoneID = *zone.Spec.ID
 	} else {
 		zoneID = zone.Status.ID
 	}
@@ -142,15 +142,18 @@ func (p *Route53DNSProvider) EnsureManagedZone(zone *v1alpha2.ManagedZone) (dns.
 			return managedZoneOutput, err
 		}
 
-		_, err = p.client.UpdateHostedZoneComment(&route53.UpdateHostedZoneCommentInput{
-			Comment: &zone.Spec.Description,
-			Id:      &zoneID,
-		})
-		if err != nil {
-			log.Log.Error(err, "failed to update hosted zone comment")
+		//Only update if we created the managed zone and description is set
+		if zone.Spec.ID != nil && zone.Spec.Description != nil {
+			_, err = p.client.UpdateHostedZoneComment(&route53.UpdateHostedZoneCommentInput{
+				Comment: zone.Spec.Description,
+				Id:      &zoneID,
+			})
+			if err != nil {
+				log.Log.Error(err, "failed to update hosted zone comment")
+			}
 		}
 
-		managedZoneOutput.ID = *getResp.HostedZone.Id
+		managedZoneOutput.ID = removeHostedZoneIDPrefix(*getResp.HostedZone.Id)
 		managedZoneOutput.RecordCount = *getResp.HostedZone.ResourceRecordSetCount
 		managedZoneOutput.NameServers = getResp.DelegationSet.NameServers
 
@@ -166,7 +169,7 @@ func (p *Route53DNSProvider) EnsureManagedZone(zone *v1alpha2.ManagedZone) (dns.
 		CallerReference: &callerRef,
 		Name:            &zone.Spec.DomainName,
 		HostedZoneConfig: &route53.HostedZoneConfig{
-			Comment:     &zone.Spec.Description,
+			Comment:     zone.Spec.Description,
 			PrivateZone: aws.Bool(false),
 		},
 	})
@@ -397,3 +400,7 @@ func removeTrailingDot(hostname string) string {
 
 	return strings.TrimSuffix(hostname, ".")
 }
+
+func removeHostedZoneIDPrefix(id string) string {
+	return strings.TrimPrefix(id, "/hostedzone/")
+}

pkg/dns/dns.go

@@ -90,8 +90,12 @@ func (*FakeProvider) Delete(_ *v1alpha2.DNSRecord) error {
 func (*FakeProvider) ListZones() (ZoneList, error) {
 	return ZoneList{}, nil
 }
-func (*FakeProvider) EnsureManagedZone(_ *v1alpha2.ManagedZone) (ManagedZoneOutput, error) {
-	return ManagedZoneOutput{}, nil
+func (*FakeProvider) EnsureManagedZone(mz *v1alpha2.ManagedZone) (ManagedZoneOutput, error) {
+	return ManagedZoneOutput{
+		ID:          *mz.Spec.ID,
+		NameServers: nil,
+		RecordCount: 0,
+	}, nil
 }
 func (*FakeProvider) DeleteManagedZone(_ *v1alpha2.ManagedZone) error { return nil }
 

pkg/dns/google/google.go

@@ -232,8 +232,8 @@ func (g *GoogleDNSProvider) DeleteManagedZone(managedZone *v1alpha2.ManagedZone)
 func (g *GoogleDNSProvider) EnsureManagedZone(managedZone *v1alpha2.ManagedZone) (dns.ManagedZoneOutput, error) {
 	var zoneID string
 
-	if managedZone.Spec.ID != "" {
-		zoneID = managedZone.Spec.ID
+	if managedZone.Spec.ID != nil {
+		zoneID = *managedZone.Spec.ID
 	} else {
 		zoneID = managedZone.Status.ID
 	}
@@ -251,7 +251,7 @@ func (g *GoogleDNSProvider) createManagedZone(managedZone *v1alpha2.ManagedZone)
 	zone := dnsv1.ManagedZone{
 		Name:        zoneID,
 		DnsName:     ensureTrailingDot(managedZone.Spec.DomainName),
-		Description: managedZone.Spec.Description,
+		Description: *managedZone.Spec.Description,
 	}
 	mz, err := g.managedZonesClient.Create(g.project, &zone).Do()
 	if err != nil {

test/policy_integration/dnspolicy_controller_health_checks_test.go

@@ -40,7 +40,13 @@ var _ = Describe("DNSPolicy Health Checks", func() {
 		gatewayClass = testutil.NewTestGatewayClass("foo", "default", "kuadrant.io/bar")
 		Expect(k8sClient.Create(ctx, gatewayClass)).To(Succeed())
 
-		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace, "example.com").ManagedZone
+		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace).
+			WithID("1234").
+			WithDomainName("example.com").
+			WithDescription("example.com").
+			WithProviderSecret("secretname").
+			ManagedZone
+
 		Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
 
 		gateway = testutil.NewGatewayBuilder(TestGatewayName, gatewayClass.Name, testNamespace).
@@ -166,15 +172,15 @@ var _ = Describe("DNSPolicy Health Checks", func() {
 								MatchFields(IgnoreExtras, Fields{
 									"ObjectMeta": HaveField("Name", recordName),
 									"Spec": MatchFields(IgnoreExtras, Fields{
-										"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+										"ZoneID":      Equal(managedZone.Spec.ID),
 										"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 										"Endpoints":   HaveLen(6),
 									}),
 								}),
 								MatchFields(IgnoreExtras, Fields{
 									"ObjectMeta": HaveField("Name", wildcardRecordName),
 									"Spec": MatchFields(IgnoreExtras, Fields{
-										"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+										"ZoneID":      Equal(managedZone.Spec.ID),
 										"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 										"Endpoints":   HaveLen(6),
 									}),

test/policy_integration/dnspolicy_controller_multi_cluster_test.go

@@ -38,7 +38,12 @@ var _ = Describe("DNSPolicy Multi Cluster", func() {
 		gatewayClass = testutil.NewTestGatewayClass("foo", "default", "kuadrant.io/bar")
 		Expect(k8sClient.Create(ctx, gatewayClass)).To(Succeed())
 
-		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace, "example.com").ManagedZone
+		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace).
+			WithID("1234").
+			WithDomainName("example.com").
+			WithDescription("example.com").
+			WithProviderSecret("secretname").
+			ManagedZone
 		Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
 
 		gateway = testutil.NewGatewayBuilder(TestGatewayName, gatewayClass.Name, testNamespace).
@@ -150,7 +155,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() {
 						MatchFields(IgnoreExtras, Fields{
 							"ObjectMeta": HaveField("Name", recordName),
 							"Spec": MatchFields(IgnoreExtras, Fields{
-								"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+								"ZoneID":      Equal(managedZone.Spec.ID),
 								"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 								"Endpoints": ConsistOf(
 									PointTo(MatchFields(IgnoreExtras, Fields{
@@ -166,7 +171,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() {
 						MatchFields(IgnoreExtras, Fields{
 							"ObjectMeta": HaveField("Name", wildcardRecordName),
 							"Spec": MatchFields(IgnoreExtras, Fields{
-								"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+								"ZoneID":      Equal(managedZone.Spec.ID),
 								"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 								"Endpoints": ConsistOf(
 									PointTo(MatchFields(IgnoreExtras, Fields{
@@ -210,7 +215,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() {
 							MatchFields(IgnoreExtras, Fields{
 								"ObjectMeta": HaveField("Name", recordName),
 								"Spec": MatchFields(IgnoreExtras, Fields{
-									"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+									"ZoneID":      Equal(managedZone.Spec.ID),
 									"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 									"Endpoints": ConsistOf(
 										PointTo(MatchFields(IgnoreExtras, Fields{
@@ -264,7 +269,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() {
 							MatchFields(IgnoreExtras, Fields{
 								"ObjectMeta": HaveField("Name", wildcardRecordName),
 								"Spec": MatchFields(IgnoreExtras, Fields{
-									"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+									"ZoneID":      Equal(managedZone.Spec.ID),
 									"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 									"Endpoints": ConsistOf(
 										PointTo(MatchFields(IgnoreExtras, Fields{
@@ -342,7 +347,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() {
 							MatchFields(IgnoreExtras, Fields{
 								"ObjectMeta": HaveField("Name", recordName),
 								"Spec": MatchFields(IgnoreExtras, Fields{
-									"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+									"ZoneID":      Equal(managedZone.Spec.ID),
 									"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 									"Endpoints": ConsistOf(
 										PointTo(MatchFields(IgnoreExtras, Fields{
@@ -404,7 +409,7 @@ var _ = Describe("DNSPolicy Multi Cluster", func() {
 							MatchFields(IgnoreExtras, Fields{
 								"ObjectMeta": HaveField("Name", wildcardRecordName),
 								"Spec": MatchFields(IgnoreExtras, Fields{
-									"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+									"ZoneID":      Equal(managedZone.Spec.ID),
 									"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 									"Endpoints": ConsistOf(
 										PointTo(MatchFields(IgnoreExtras, Fields{

test/policy_integration/dnspolicy_controller_single_cluster_test.go

@@ -35,7 +35,12 @@ var _ = Describe("DNSPolicy Single Cluster", func() {
 		gatewayClass = testutil.NewTestGatewayClass("foo", "default", "kuadrant.io/bar")
 		Expect(k8sClient.Create(ctx, gatewayClass)).To(Succeed())
 
-		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace, "example.com").ManagedZone
+		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace).
+			WithID("1234").
+			WithDomainName("example.com").
+			WithDescription("example.com").
+			WithProviderSecret("secretname").
+			ManagedZone
 		Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
 
 		gateway = testutil.NewGatewayBuilder(TestGatewayName, gatewayClass.Name, testNamespace).
@@ -121,7 +126,7 @@ var _ = Describe("DNSPolicy Single Cluster", func() {
 						MatchFields(IgnoreExtras, Fields{
 							"ObjectMeta": HaveField("Name", recordName),
 							"Spec": MatchFields(IgnoreExtras, Fields{
-								"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+								"ZoneID":      Equal(managedZone.Spec.ID),
 								"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 								"Endpoints": ConsistOf(
 									PointTo(MatchFields(IgnoreExtras, Fields{
@@ -137,7 +142,7 @@ var _ = Describe("DNSPolicy Single Cluster", func() {
 						MatchFields(IgnoreExtras, Fields{
 							"ObjectMeta": HaveField("Name", wildcardRecordName),
 							"Spec": MatchFields(IgnoreExtras, Fields{
-								"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+								"ZoneID":      Equal(managedZone.Spec.ID),
 								"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 								"Endpoints": ConsistOf(
 									PointTo(MatchFields(IgnoreExtras, Fields{
@@ -175,7 +180,7 @@ var _ = Describe("DNSPolicy Single Cluster", func() {
 						MatchFields(IgnoreExtras, Fields{
 							"ObjectMeta": HaveField("Name", recordName),
 							"Spec": MatchFields(IgnoreExtras, Fields{
-								"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+								"ZoneID":      Equal(managedZone.Spec.ID),
 								"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 								"Endpoints": ConsistOf(
 									PointTo(MatchFields(IgnoreExtras, Fields{
@@ -214,7 +219,7 @@ var _ = Describe("DNSPolicy Single Cluster", func() {
 						MatchFields(IgnoreExtras, Fields{
 							"ObjectMeta": HaveField("Name", wildcardRecordName),
 							"Spec": MatchFields(IgnoreExtras, Fields{
-								"ZoneID":      PointTo(Equal(managedZone.Spec.ID)),
+								"ZoneID":      Equal(managedZone.Spec.ID),
 								"ProviderRef": Equal(dnsPolicy.Spec.ProviderRef),
 								"Endpoints": ContainElements(
 									PointTo(MatchFields(IgnoreExtras, Fields{

test/policy_integration/dnspolicy_controller_test.go

@@ -41,7 +41,12 @@ var _ = Describe("DNSPolicy", func() {
 		gatewayClass = testutil.NewTestGatewayClass("foo", "default", "kuadrant.io/bar")
 		Expect(k8sClient.Create(ctx, gatewayClass)).To(Succeed())
 
-		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace, "example.com").ManagedZone
+		managedZone = testutil.NewManagedZoneBuilder("mz-example-com", testNamespace).
+			WithID("1234").
+			WithDomainName("example.com").
+			WithDescription("example.com").
+			WithProviderSecret("secretname").
+			ManagedZone
 		Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
 
 		dnsPolicyBuilder = testutil.NewDNSPolicyBuilder("test-dns-policy", testNamespace).

test/policy_integration/managedzone_controller_test.go

@@ -39,7 +39,7 @@ var _ = Describe("ManagedZoneReconciler", func() {
 					Namespace: defaultNS,
 				},
 				Spec: v1alpha2.ManagedZoneSpec{
-					ID:         testutil.Domain,
+					ID:         testutil.Pointer(testutil.Domain),
 					DomainName: testutil.Domain,
 					ProviderRef: v1alpha2.ProviderRef{
 						Name: providerCredential,
@@ -87,7 +87,7 @@ var _ = Describe("ManagedZoneReconciler", func() {
 					Namespace: defaultNS,
 				},
 				Spec: v1alpha2.ManagedZoneSpec{
-					ID:         "invalid_domain",
+					ID:         testutil.Pointer("invalid_domain"),
 					DomainName: "invalid_domain",
 				},
 			}