From 55780444031714fc931d90af298a4b193888977a Mon Sep 17 00:00:00 2001
From: Miciah Masters <miciah.masters@gmail.com>
Date: Thu, 20 Jul 2023 17:44:54 -0400
Subject: [PATCH] Check public DNS zone when reporting status

When reporting the "DNSReady" status condition on an IngressController,
check the status conditions for both the public zone and the private zone
of the associated DNSRecord.

Before this commit, only the status condition for the private zone was used
to compute the "DNSReady" status condition.  If the operator failed to
publish a DNS record in the public zone, this failure was not reported in
the IngressController's status conditions or the "ingress" clusteroperator
status conditions.

Follow-up to commit 9f329233f31db93d7e7fa6061e1f514dc02af5dc.

This commit fixes OCPBUGS-15978.

https://issues.redhat.com/browse/OCPBUGS-15978

* pkg/operator/controller/ingress/status.go (checkZoneInConfig): Use the
new zonesMatch helper function to check both the public zone as well as the
private zone.
(zonesMatch): New function.  Return a Boolean value indicating whether two
DNS zones match, based on their respective ID or "Name" tags.
* pkg/operator/controller/ingress/status_test.go (Test_checkZoneInConfig):
Verify that checkZoneInConfig checks the public zone as well as the private
zone.  Delete "[PrivateZone]" from test-case descriptions as each test case
is now tested as both a public zone and a private zone.
---
 pkg/operator/controller/ingress/status.go     | 26 +++++++++++--------
 .../controller/ingress/status_test.go         | 22 ++++++++++------
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/pkg/operator/controller/ingress/status.go b/pkg/operator/controller/ingress/status.go
index 9bb0a6950..ec6073ce5 100644
--- a/pkg/operator/controller/ingress/status.go
+++ b/pkg/operator/controller/ingress/status.go
@@ -1059,19 +1059,23 @@ func computeDNSStatus(ic *operatorv1.IngressController, wildcardRecord *iov1.DNS
 
 // checkZoneInConfig - private utility to check for a zone in the current config
 func checkZoneInConfig(dnsConfig *configv1.DNS, zone configv1.DNSZone) bool {
-	// check PrivateZone settings only
-	// check for private zone ID
-	if dnsConfig.Spec.PrivateZone != nil && dnsConfig.Spec.PrivateZone.ID != "" && zone.ID != "" {
-		if dnsConfig.Spec.PrivateZone.ID == zone.ID {
-			return true
-		}
+	return zonesMatch(&zone, dnsConfig.Spec.PublicZone) || zonesMatch(&zone, dnsConfig.Spec.PrivateZone)
+}
+
+// zonesMatch returns a Boolean value indicating whether two DNS zones have the
+// matching ID or "Name" tag.  If either or both zones are nil, this function
+// returns false.
+func zonesMatch(a, b *configv1.DNSZone) bool {
+	if a == nil || b == nil {
+		return false
 	}
 
-	// check for private zone Tags
-	if dnsConfig.Spec.PrivateZone != nil && dnsConfig.Spec.PrivateZone.Tags["Name"] != "" && zone.Tags["Name"] != "" {
-		if dnsConfig.Spec.PrivateZone.Tags["Name"] == zone.Tags["Name"] {
-			return true
-		}
+	if a.ID != "" && b.ID != "" && a.ID == b.ID {
+		return true
+	}
+
+	if a.Tags["Name"] != "" && b.Tags["Name"] != "" && a.Tags["Name"] == b.Tags["Name"] {
+		return true
 	}
 
 	return false
diff --git a/pkg/operator/controller/ingress/status_test.go b/pkg/operator/controller/ingress/status_test.go
index 234529cd7..9b3dbd4e4 100644
--- a/pkg/operator/controller/ingress/status_test.go
+++ b/pkg/operator/controller/ingress/status_test.go
@@ -2128,56 +2128,56 @@ func Test_checkZoneInConfig(t *testing.T) {
 		in, zone, zoneType string
 	}{
 		{
-			description: "[PrivateZone] empty strings (should fail)",
+			description: "empty strings (should fail)",
 			expected:    false,
 			in:          "",
 			zone:        "",
 			zoneType:    "ID",
 		},
 		{
-			description: "[PrivateZone] zone.ID empty string (should fail)",
+			description: "zone.ID empty string (should fail)",
 			expected:    false,
 			in:          "test",
 			zone:        "",
 			zoneType:    "ID",
 		},
 		{
-			description: "[PrivateZone] zone.ID with value (not equal should fail)",
+			description: "zone.ID with value (not equal should fail)",
 			expected:    false,
 			in:          "test",
 			zone:        "notest",
 			zoneType:    "ID",
 		},
 		{
-			description: "[PrivateZone] zone.ID with value (equal should pass)",
+			description: "zone.ID with value (equal should pass)",
 			expected:    true,
 			in:          "test",
 			zone:        "test",
 			zoneType:    "ID",
 		},
 		{
-			description: "[PrivateZone] empty strings (should fail)",
+			description: "empty strings (should fail)",
 			expected:    false,
 			in:          "",
 			zone:        "",
 			zoneType:    "TAG",
 		},
 		{
-			description: "[PrivateZone] zone.Tags['Name'] empty string (should fail)",
+			description: "zone.Tags['Name'] empty string (should fail)",
 			expected:    false,
 			in:          "test",
 			zone:        "",
 			zoneType:    "TAG",
 		},
 		{
-			description: "[PrivateZone] zone.Tags['Name'] with value (not equal should fail)",
+			description: "zone.Tags['Name'] with value (not equal should fail)",
 			expected:    false,
 			in:          "test",
 			zone:        "notest",
 			zoneType:    "TAG",
 		},
 		{
-			description: "[PrivateZone] zone.tags['Name'] with value (equal should pass)",
+			description: "zone.tags['Name'] with value (equal should pass)",
 			expected:    true,
 			in:          "test",
 			zone:        "test",
@@ -2202,6 +2202,12 @@ func Test_checkZoneInConfig(t *testing.T) {
 			if actual != test.expected {
 				t.Errorf("expected:%v actual:%v\n", test.expected, actual)
 			}
+			dnsSpec = configv1.DNSSpec{PublicZone: z}
+			dnsConfig = &configv1.DNS{Spec: dnsSpec}
+			actual = checkZoneInConfig(dnsConfig, dnsZone)
+			if actual != test.expected {
+				t.Errorf("expected:%v actual:%v\n", test.expected, actual)
+			}
 		})
 	}
 }