Provide the correct Hub DNS name in the Ansible extra_vars

Makefile

@@ -262,6 +262,7 @@ install-crds: manifests
 	kubectl apply -f https://raw.githubusercontent.com/open-cluster-management-io/api/main/cluster/v1beta1/0000_02_clusters.open-cluster-management.io_placements.crd.yaml --validate=false
 	kubectl apply -f https://raw.githubusercontent.com/open-cluster-management-io/api/main/cluster/v1beta1/0000_03_clusters.open-cluster-management.io_placementdecisions.crd.yaml --validate=false
 	kubectl apply -f deploy/crds/external/tower.ansible.com_joblaunch_crd.yaml
+	kubectl apply -f test/resources/case5_policy_automation/dns-crd.yaml
 
 .PHONY: install-resources
 install-resources:
@@ -272,6 +273,8 @@ install-resources:
 	@echo creating cluster resources
 	kubectl apply -f test/resources/managed1-cluster.yaml
 	kubectl apply -f test/resources/managed2-cluster.yaml
+	@echo setting a Hub cluster DNS name
+	kubectl apply -f test/resources/case5_policy_automation/cluster-dns.yaml
 
 .PHONY: e2e-dependencies
 e2e-dependencies:

controllers/automation/policyautomation_controller.go

@@ -6,21 +6,20 @@ package automation
 import (
 	"context"
 	"fmt"
-	"net"
 	"strconv"
-	"strings"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
@@ -32,8 +31,11 @@ import (
 
 const ControllerName string = "policy-automation"
 
+var dnsGVR = schema.GroupVersionResource{Group: "config.openshift.io", Version: "v1", Resource: "dnses"}
+
 var log = ctrl.Log.WithName(ControllerName)
 
+//+kubebuilder:rbac:groups=config.openshift.io,resources=dnses,resourceNames=cluster,verbs=get
 //+kubebuilder:rbac:groups=policy.open-cluster-management.io,resources=policyautomations,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=policy.open-cluster-management.io,resources=policyautomations/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=policy.open-cluster-management.io,resources=policyautomations/finalizers,verbs=update
@@ -104,41 +106,35 @@ func getTargetListMap(targetList []string) map[string]bool {
 	return targetListMap
 }
 
-// getClusterName will get the hub cluster name information
-func getClusterName() string {
-	hubCluster := ""
-	// Get a config to talk to the apiserver
-	cfg, err := config.GetConfig()
-	if err == nil {
-		hubCluster = cfg.Host
-	} else {
-		log.Error(err, "Unable to get the Kubernetes configuration.")
-	}
+// getClusterDNSName will get the Hub cluster DNS name if the Hub is an OpenShift cluster.
+func (r *PolicyAutomationReconciler) getClusterDNSName(ctx context.Context) (string, error) {
+	dnsCluster, err := r.DynamicClient.Resource(dnsGVR).Get(ctx, "cluster", metav1.GetOptions{})
+	if err != nil {
+		if errors.IsNotFound(err) {
+			// This is a debug log to not spam the logs when the Hub is installed on a Kubernetes distribution other
+			// than OpenShift.
+			log.V(2).Info("The Hub cluster DNS name couldn't be determined")
 
-	if len(hubCluster) > 0 {
-		// remove possible https head
-		hubCluster = strings.ReplaceAll(hubCluster, "https://", "")
-		// remove possible http head
-		hubCluster = strings.ReplaceAll(hubCluster, "http://", "")
-		// remove possible port number
-		hubCluster = strings.SplitN(hubCluster, ":", 2)[0]
-		// convert host name to URL if it is IP address
-		if net.ParseIP(hubCluster) != nil {
-			names, _ := net.LookupAddr(hubCluster)
-			if len(names) != 0 {
-				hubCluster = names[0]
-			}
+			return "", nil
 		}
+
+		return "", err
 	}
 
-	log.V(3).Info("Hub Cluster information: %s", hubCluster)
+	dnsName, _, _ := unstructured.NestedString(dnsCluster.Object, "spec", "baseDomain")
+	if dnsName == "" {
+		log.Info("The OpenShift DNS object named cluster did not contain a valid spec.baseDomain value")
+	} else {
+		log.V(2).Info("The Hub cluster DNS name was found", "name", dnsName)
+	}
 
-	return hubCluster
+	return dnsName, nil
 }
 
 // getViolationContext will put the root policy information into violationContext
 // It also puts the status of the non-compliant replicated policies into violationContext
 func (r *PolicyAutomationReconciler) getViolationContext(
+	ctx context.Context,
 	policy *policyv1.Policy,
 	targetList []string,
 	policyAutomation *policyv1beta1.PolicyAutomation,
@@ -157,7 +153,12 @@ func (r *PolicyAutomationReconciler) getViolationContext(
 	// 3) get the root policy namespace
 	violationContext.PolicyNamespace = policy.GetNamespace()
 	// 4) get the root policy hub cluster name
-	violationContext.HubCluster = getClusterName()
+	var err error
+
+	violationContext.HubCluster, err = r.getClusterDNSName(ctx)
+	if err != nil {
+		return policyv1beta1.ViolationContext{}, err
+	}
 
 	// 5) get the policy sets of the root policy
 	plcPlacement := policy.Status.Placement
@@ -178,7 +179,7 @@ func (r *PolicyAutomationReconciler) getViolationContext(
 
 	replicatedPlcList := &policyv1.PolicyList{}
 
-	err := r.List(
+	err = r.List(
 		context.TODO(),
 		replicatedPlcList,
 		client.MatchingLabels(propagator.LabelsForRootPolicy(policy)),
@@ -308,7 +309,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
 			"Creating an Ansible job", "mode", "manual",
 			"clusterCount", strconv.Itoa(len(targetList)))
 
-		violationContext, _ := r.getViolationContext(policy, targetList, policyAutomation)
+		violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
 
 		err = common.CreateAnsibleJob(
 			policyAutomation,
@@ -359,7 +360,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
 			targetList := common.FindNonCompliantClustersForPolicy(policy)
 			if len(targetList) > 0 {
 				log.Info("Creating An Ansible job", "targetList", targetList)
-				violationContext, _ := r.getViolationContext(policy, targetList, policyAutomation)
+				violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
 				err = common.CreateAnsibleJob(policyAutomation, r.DynamicClient, "scan", violationContext)
 				if err != nil {
 					return reconcile.Result{RequeueAfter: requeueAfter}, err
@@ -381,7 +382,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
 			if len(targetList) > 0 {
 				log.Info("Creating an Ansible job", "targetList", targetList)
 
-				violationContext, _ := r.getViolationContext(policy, targetList, policyAutomation)
+				violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
 				err = common.CreateAnsibleJob(
 					policyAutomation,
 					r.DynamicClient,
@@ -492,7 +493,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
 					trimmedTargetList = append(trimmedTargetList, clusterName)
 				}
 				log.Info("Creating An Ansible job", "trimmedTargetList", trimmedTargetList)
-				violationContext, _ := r.getViolationContext(policy, trimmedTargetList, policyAutomation)
+				violationContext, _ := r.getViolationContext(ctx, policy, trimmedTargetList, policyAutomation)
 				err = common.CreateAnsibleJob(
 					policyAutomation,
 					r.DynamicClient,

deploy/operator.yaml

@@ -73,6 +73,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - config.openshift.io
+  resourceNames:
+  - cluster
+  resources:
+  - dnses
+  verbs:
+  - get
 - apiGroups:
   - ""
   resources:

deploy/rbac/role.yaml

@@ -32,6 +32,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - config.openshift.io
+  resourceNames:
+  - cluster
+  resources:
+  - dnses
+  verbs:
+  - get
 - apiGroups:
   - ""
   resources:

test/e2e/case5_policy_automation_test.go

@@ -172,10 +172,7 @@ var _ = Describe("Test policy automation", func() {
 			extraVars := spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
 			Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
 			Expect(extraVars["namespace"]).To(Equal(testNamespace))
-			// hub_cluster depends on environment so just check if hub_cluster is set
-			// rather than verifying the hub_cluster name
-			By("Check hub_cluster : " + extraVars["hub_cluster"].(string))
-			Expect(len(extraVars["hub_cluster"].(string)) > 0).To(BeTrue())
+			Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
 			Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(1))
 			Expect(extraVars["target_clusters"].([]interface{})[0]).To(Equal("managed1"))
 			Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
@@ -811,10 +808,7 @@ var _ = Describe("Test policy automation", func() {
 			extraVars := spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
 			Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
 			Expect(extraVars["namespace"]).To(Equal(testNamespace))
-			// hub_cluster depends on environment so just check if hub_cluster is set
-			// rather than verifying the hub_cluster name
-			By("Check hub_cluster : " + extraVars["hub_cluster"].(string))
-			Expect(len(extraVars["hub_cluster"].(string)) > 0).To(BeTrue())
+			Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
 			Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(2))
 			Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
 			Expect(extraVars["policy_set"].([]interface{})[0]).To(Equal("case5-test-policyset"))
@@ -872,7 +866,7 @@ var _ = Describe("Test policy automation", func() {
 				return len(ansiblejobList.Items)
 			}, 30, 1).Should(Equal(2))
 
-			By("Check each policy_violation_context is null in extra_vars for the compliant manual run case")
+			By("Check the policy_violation_context is mostly empty for the compliant manual run case")
 			ansiblejobList, err = clientHubDynamic.Resource(gvrAnsibleJob).Namespace(testNamespace).List(
 				context.TODO(), metav1.ListOptions{},
 			)
@@ -887,8 +881,7 @@ var _ = Describe("Test policy automation", func() {
 			extraVars = spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
 			Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
 			Expect(extraVars["namespace"]).To(Equal(testNamespace))
-			By("Check hub_cluster : " + extraVars["hub_cluster"].(string))
-			Expect(len(extraVars["hub_cluster"].(string)) > 0).To(BeTrue())
+			Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
 			Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(0))
 			Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
 			Expect(extraVars["policy_set"].([]interface{})[0]).To(Equal("case5-test-policyset"))

test/resources/case5_policy_automation/cluster-dns.yaml

@@ -0,0 +1,6 @@
+apiVersion: config.openshift.io/v1
+kind: DNS
+metadata:
+  name: cluster
+spec:
+  baseDomain: millienium-falcon.tatooine.local

test/resources/case5_policy_automation/dns-crd.yaml

@@ -0,0 +1,135 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    api-approved.openshift.io: https://github.com/openshift/api/pull/470
+    include.release.openshift.io/ibm-cloud-managed: "true"
+    include.release.openshift.io/self-managed-high-availability: "true"
+    include.release.openshift.io/single-node-developer: "true"
+  creationTimestamp: "2023-01-09T07:07:25Z"
+  generation: 1
+  name: dnses.config.openshift.io
+  ownerReferences:
+  - apiVersion: config.openshift.io/v1
+    kind: ClusterVersion
+    name: version
+    uid: ace25b63-dff4-415f-9c1d-c7583b57e40d
+  resourceVersion: "1100"
+  uid: 37084ebf-13f5-4c39-a9ff-fda238db6a3f
+spec:
+  conversion:
+    strategy: None
+  group: config.openshift.io
+  names:
+    kind: DNS
+    listKind: DNSList
+    plural: dnses
+    singular: dns
+  scope: Cluster
+  versions:
+  - name: v1
+    schema:
+      openAPIV3Schema:
+        description: "DNS holds cluster-wide information about DNS. The canonical
+          name is `cluster` \n Compatibility level 1: Stable within a major release
+          for a minimum of 12 months or 3 minor releases (whichever is longer)."
+        properties:
+          apiVersion:
+            description: 'APIVersion defines the versioned schema of this representation
+              of an object. Servers should convert recognized schemas to the latest
+              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            type: string
+          kind:
+            description: 'Kind is a string value representing the REST resource this
+              object represents. Servers may infer this from the endpoint the client
+              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            type: string
+          metadata:
+            type: object
+          spec:
+            description: spec holds user settable values for configuration
+            properties:
+              baseDomain:
+                description: "baseDomain is the base domain of the cluster. All managed
+                  DNS records will be sub-domains of this base. \n For example, given
+                  the base domain `openshift.example.com`, an API server DNS record
+                  may be created for `cluster-api.openshift.example.com`. \n Once
+                  set, this field cannot be changed."
+                type: string
+              privateZone:
+                description: "privateZone is the location where all the DNS records
+                  that are only available internally to the cluster exist. \n If this
+                  field is nil, no private records should be created. \n Once set,
+                  this field cannot be changed."
+                properties:
+                  id:
+                    description: "id is the identifier that can be used to find the
+                      DNS hosted zone. \n on AWS zone can be fetched using `ID` as
+                      id in [1] on Azure zone can be fetched using `ID` as a pre-determined
+                      name in [2], on GCP zone can be fetched using `ID` as a pre-determined
+                      name in [3]. \n [1]: https://docs.aws.amazon.com/cli/latest/reference/route53/get-hosted-zone.html#options
+                      [2]: https://docs.microsoft.com/en-us/cli/azure/network/dns/zone?view=azure-cli-latest#az-network-dns-zone-show
+                      [3]: https://cloud.google.com/dns/docs/reference/v1/managedZones/get"
+                    type: string
+                  tags:
+                    additionalProperties:
+                      type: string
+                    description: "tags can be used to query the DNS hosted zone. \n
+                      on AWS, resourcegroupstaggingapi [1] can be used to fetch a
+                      zone using `Tags` as tag-filters, \n [1]: https://docs.aws.amazon.com/cli/latest/reference/resourcegroupstaggingapi/get-resources.html#options"
+                    type: object
+                type: object
+              publicZone:
+                description: "publicZone is the location where all the DNS records
+                  that are publicly accessible to the internet exist. \n If this field
+                  is nil, no public records should be created. \n Once set, this field
+                  cannot be changed."
+                properties:
+                  id:
+                    description: "id is the identifier that can be used to find the
+                      DNS hosted zone. \n on AWS zone can be fetched using `ID` as
+                      id in [1] on Azure zone can be fetched using `ID` as a pre-determined
+                      name in [2], on GCP zone can be fetched using `ID` as a pre-determined
+                      name in [3]. \n [1]: https://docs.aws.amazon.com/cli/latest/reference/route53/get-hosted-zone.html#options
+                      [2]: https://docs.microsoft.com/en-us/cli/azure/network/dns/zone?view=azure-cli-latest#az-network-dns-zone-show
+                      [3]: https://cloud.google.com/dns/docs/reference/v1/managedZones/get"
+                    type: string
+                  tags:
+                    additionalProperties:
+                      type: string
+                    description: "tags can be used to query the DNS hosted zone. \n
+                      on AWS, resourcegroupstaggingapi [1] can be used to fetch a
+                      zone using `Tags` as tag-filters, \n [1]: https://docs.aws.amazon.com/cli/latest/reference/resourcegroupstaggingapi/get-resources.html#options"
+                    type: object
+                type: object
+            type: object
+          status:
+            description: status holds observed values from the cluster. They may not
+              be overridden.
+            type: object
+        required:
+        - spec
+        type: object
+    served: true
+    storage: true
+    subresources:
+      status: {}
+status:
+  acceptedNames:
+    kind: DNS
+    listKind: DNSList
+    plural: dnses
+    singular: dns
+  conditions:
+  - lastTransitionTime: "2023-01-09T07:07:25Z"
+    message: no conflicts found
+    reason: NoConflicts
+    status: "True"
+    type: NamesAccepted
+  - lastTransitionTime: "2023-01-09T07:07:25Z"
+    message: the initial names have been accepted
+    reason: InitialNamesAccepted
+    status: "True"
+    type: Established
+  storedVersions:
+  - v1