Skip to content

Commit e1d164a

Browse files
mprahlopenshift-merge-robot
mprahl
authored and
openshift-merge-robot
committed
Provide the correct Hub DNS name in the Ansible extra_vars
Using the Kubernetes configuration leads to a local DNS name when running in the cluster. This commit changes it to use the OpenShift dnses custom resource to get the value. Other Kubernetes distributions could have their own implementations added later. Relates: https://issues.redhat.com/browse/ACM-2410 Signed-off-by: mprahl <[email protected]>
1 parent c2d5188 commit e1d164a

File tree

7 files changed

+199
-45
lines changed

7 files changed

+199
-45
lines changed

‎Makefile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ install-crds: manifests
262262
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
263263
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
264264
kubectl apply -f deploy/crds/external/tower.ansible.com_joblaunch_crd.yaml
265+
kubectl apply -f test/resources/case5_policy_automation/dns-crd.yaml
265266

266267
.PHONY: install-resources
267268
install-resources:
@@ -272,6 +273,8 @@ install-resources:
272273
@echo creating cluster resources
273274
kubectl apply -f test/resources/managed1-cluster.yaml
274275
kubectl apply -f test/resources/managed2-cluster.yaml
276+
@echo setting a Hub cluster DNS name
277+
kubectl apply -f test/resources/case5_policy_automation/cluster-dns.yaml
275278

276279
.PHONY: e2e-dependencies
277280
e2e-dependencies:

‎controllers/automation/policyautomation_controller.go

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,20 @@ package automation
66
import (
77
"context"
88
"fmt"
9-
"net"
109
"strconv"
11-
"strings"
1210
"time"
1311

1412
"k8s.io/apimachinery/pkg/api/errors"
1513
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1615
"k8s.io/apimachinery/pkg/runtime"
16+
"k8s.io/apimachinery/pkg/runtime/schema"
1717
"k8s.io/apimachinery/pkg/types"
1818
"k8s.io/client-go/dynamic"
1919
"k8s.io/client-go/tools/record"
2020
ctrl "sigs.k8s.io/controller-runtime"
2121
"sigs.k8s.io/controller-runtime/pkg/builder"
2222
"sigs.k8s.io/controller-runtime/pkg/client"
23-
"sigs.k8s.io/controller-runtime/pkg/client/config"
2423
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2524
"sigs.k8s.io/controller-runtime/pkg/source"
2625

@@ -32,8 +31,11 @@ import (
3231

3332
const ControllerName string = "policy-automation"
3433

34+
var dnsGVR = schema.GroupVersionResource{Group: "config.openshift.io", Version: "v1", Resource: "dnses"}
35+
3536
var log = ctrl.Log.WithName(ControllerName)
3637

38+
//+kubebuilder:rbac:groups=config.openshift.io,resources=dnses,resourceNames=cluster,verbs=get
3739
//+kubebuilder:rbac:groups=policy.open-cluster-management.io,resources=policyautomations,verbs=get;list;watch;create;update;patch;delete
3840
//+kubebuilder:rbac:groups=policy.open-cluster-management.io,resources=policyautomations/status,verbs=get;update;patch
3941
//+kubebuilder:rbac:groups=policy.open-cluster-management.io,resources=policyautomations/finalizers,verbs=update
@@ -104,41 +106,35 @@ func getTargetListMap(targetList []string) map[string]bool {
104106
return targetListMap
105107
}
106108

107-
// getClusterName will get the hub cluster name information
108-
func getClusterName() string {
109-
hubCluster := ""
110-
// Get a config to talk to the apiserver
111-
cfg, err := config.GetConfig()
112-
if err == nil {
113-
hubCluster = cfg.Host
114-
} else {
115-
log.Error(err, "Unable to get the Kubernetes configuration.")
116-
}
109+
// getClusterDNSName will get the Hub cluster DNS name if the Hub is an OpenShift cluster.
110+
func (r *PolicyAutomationReconciler) getClusterDNSName(ctx context.Context) (string, error) {
111+
dnsCluster, err := r.DynamicClient.Resource(dnsGVR).Get(ctx, "cluster", metav1.GetOptions{})
112+
if err != nil {
113+
if errors.IsNotFound(err) {
114+
// This is a debug log to not spam the logs when the Hub is installed on a Kubernetes distribution other
115+
// than OpenShift.
116+
log.V(2).Info("The Hub cluster DNS name couldn't be determined")
117117

118-
if len(hubCluster) > 0 {
119-
// remove possible https head
120-
hubCluster = strings.ReplaceAll(hubCluster, "https://", "")
121-
// remove possible http head
122-
hubCluster = strings.ReplaceAll(hubCluster, "http://", "")
123-
// remove possible port number
124-
hubCluster = strings.SplitN(hubCluster, ":", 2)[0]
125-
// convert host name to URL if it is IP address
126-
if net.ParseIP(hubCluster) != nil {
127-
names, _ := net.LookupAddr(hubCluster)
128-
if len(names) != 0 {
129-
hubCluster = names[0]
130-
}
118+
return "", nil
131119
}
120+
121+
return "", err
132122
}
133123

134-
log.V(3).Info("Hub Cluster information: %s", hubCluster)
124+
dnsName, _, _ := unstructured.NestedString(dnsCluster.Object, "spec", "baseDomain")
125+
if dnsName == "" {
126+
log.Info("The OpenShift DNS object named cluster did not contain a valid spec.baseDomain value")
127+
} else {
128+
log.V(2).Info("The Hub cluster DNS name was found", "name", dnsName)
129+
}
135130

136-
return hubCluster
131+
return dnsName, nil
137132
}
138133

139134
// getViolationContext will put the root policy information into violationContext
140135
// It also puts the status of the non-compliant replicated policies into violationContext
141136
func (r *PolicyAutomationReconciler) getViolationContext(
137+
ctx context.Context,
142138
policy *policyv1.Policy,
143139
targetList []string,
144140
policyAutomation *policyv1beta1.PolicyAutomation,
@@ -157,7 +153,12 @@ func (r *PolicyAutomationReconciler) getViolationContext(
157153
// 3) get the root policy namespace
158154
violationContext.PolicyNamespace = policy.GetNamespace()
159155
// 4) get the root policy hub cluster name
160-
violationContext.HubCluster = getClusterName()
156+
var err error
157+
158+
violationContext.HubCluster, err = r.getClusterDNSName(ctx)
159+
if err != nil {
160+
return policyv1beta1.ViolationContext{}, err
161+
}
161162

162163
// 5) get the policy sets of the root policy
163164
plcPlacement := policy.Status.Placement
@@ -178,7 +179,7 @@ func (r *PolicyAutomationReconciler) getViolationContext(
178179

179180
replicatedPlcList := &policyv1.PolicyList{}
180181

181-
err := r.List(
182+
err = r.List(
182183
context.TODO(),
183184
replicatedPlcList,
184185
client.MatchingLabels(propagator.LabelsForRootPolicy(policy)),
@@ -308,7 +309,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
308309
"Creating an Ansible job", "mode", "manual",
309310
"clusterCount", strconv.Itoa(len(targetList)))
310311

311-
violationContext, _ := r.getViolationContext(policy, targetList, policyAutomation)
312+
violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
312313

313314
err = common.CreateAnsibleJob(
314315
policyAutomation,
@@ -359,7 +360,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
359360
targetList := common.FindNonCompliantClustersForPolicy(policy)
360361
if len(targetList) > 0 {
361362
log.Info("Creating An Ansible job", "targetList", targetList)
362-
violationContext, _ := r.getViolationContext(policy, targetList, policyAutomation)
363+
violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
363364
err = common.CreateAnsibleJob(policyAutomation, r.DynamicClient, "scan", violationContext)
364365
if err != nil {
365366
return reconcile.Result{RequeueAfter: requeueAfter}, err
@@ -381,7 +382,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
381382
if len(targetList) > 0 {
382383
log.Info("Creating an Ansible job", "targetList", targetList)
383384

384-
violationContext, _ := r.getViolationContext(policy, targetList, policyAutomation)
385+
violationContext, _ := r.getViolationContext(ctx, policy, targetList, policyAutomation)
385386
err = common.CreateAnsibleJob(
386387
policyAutomation,
387388
r.DynamicClient,
@@ -492,7 +493,7 @@ func (r *PolicyAutomationReconciler) Reconcile(
492493
trimmedTargetList = append(trimmedTargetList, clusterName)
493494
}
494495
log.Info("Creating An Ansible job", "trimmedTargetList", trimmedTargetList)
495-
violationContext, _ := r.getViolationContext(policy, trimmedTargetList, policyAutomation)
496+
violationContext, _ := r.getViolationContext(ctx, policy, trimmedTargetList, policyAutomation)
496497
err = common.CreateAnsibleJob(
497498
policyAutomation,
498499
r.DynamicClient,

‎deploy/operator.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ rules:
7373
- get
7474
- list
7575
- watch
76+
- apiGroups:
77+
- config.openshift.io
78+
resourceNames:
79+
- cluster
80+
resources:
81+
- dnses
82+
verbs:
83+
- get
7684
- apiGroups:
7785
- ""
7886
resources:

‎deploy/rbac/role.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ rules:
3232
- get
3333
- list
3434
- watch
35+
- apiGroups:
36+
- config.openshift.io
37+
resourceNames:
38+
- cluster
39+
resources:
40+
- dnses
41+
verbs:
42+
- get
3543
- apiGroups:
3644
- ""
3745
resources:

‎test/e2e/case5_policy_automation_test.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,7 @@ var _ = Describe("Test policy automation", func() {
172172
extraVars := spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
173173
Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
174174
Expect(extraVars["namespace"]).To(Equal(testNamespace))
175-
// hub_cluster depends on environment so just check if hub_cluster is set
176-
// rather than verifying the hub_cluster name
177-
By("Check hub_cluster : " + extraVars["hub_cluster"].(string))
178-
Expect(len(extraVars["hub_cluster"].(string)) > 0).To(BeTrue())
175+
Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
179176
Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(1))
180177
Expect(extraVars["target_clusters"].([]interface{})[0]).To(Equal("managed1"))
181178
Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
@@ -811,10 +808,7 @@ var _ = Describe("Test policy automation", func() {
811808
extraVars := spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
812809
Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
813810
Expect(extraVars["namespace"]).To(Equal(testNamespace))
814-
// hub_cluster depends on environment so just check if hub_cluster is set
815-
// rather than verifying the hub_cluster name
816-
By("Check hub_cluster : " + extraVars["hub_cluster"].(string))
817-
Expect(len(extraVars["hub_cluster"].(string)) > 0).To(BeTrue())
811+
Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
818812
Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(2))
819813
Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
820814
Expect(extraVars["policy_set"].([]interface{})[0]).To(Equal("case5-test-policyset"))
@@ -872,7 +866,7 @@ var _ = Describe("Test policy automation", func() {
872866
return len(ansiblejobList.Items)
873867
}, 30, 1).Should(Equal(2))
874868

875-
By("Check each policy_violation_context is null in extra_vars for the compliant manual run case")
869+
By("Check the policy_violation_context is mostly empty for the compliant manual run case")
876870
ansiblejobList, err = clientHubDynamic.Resource(gvrAnsibleJob).Namespace(testNamespace).List(
877871
context.TODO(), metav1.ListOptions{},
878872
)
@@ -887,8 +881,7 @@ var _ = Describe("Test policy automation", func() {
887881
extraVars = spec.(map[string]interface{})["extra_vars"].(map[string]interface{})
888882
Expect(extraVars["policy_name"]).To(Equal("case5-test-policy"))
889883
Expect(extraVars["namespace"]).To(Equal(testNamespace))
890-
By("Check hub_cluster : " + extraVars["hub_cluster"].(string))
891-
Expect(len(extraVars["hub_cluster"].(string)) > 0).To(BeTrue())
884+
Expect(extraVars["hub_cluster"]).To(Equal("millienium-falcon.tatooine.local"))
892885
Expect(len(extraVars["target_clusters"].([]interface{}))).To(Equal(0))
893886
Expect(len(extraVars["policy_set"].([]interface{}))).To(Equal(1))
894887
Expect(extraVars["policy_set"].([]interface{})[0]).To(Equal("case5-test-policyset"))

‎test/resources/case5_policy_automation/cluster-dns.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
apiVersion: config.openshift.io/v1
2+
kind: DNS
3+
metadata:
4+
name: cluster
5+
spec:
6+
baseDomain: millienium-falcon.tatooine.local

‎test/resources/case5_policy_automation/dns-crd.yaml

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
apiVersion: apiextensions.k8s.io/v1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
annotations:
5+
api-approved.openshift.io: https://github.com/openshift/api/pull/470
6+
include.release.openshift.io/ibm-cloud-managed: "true"
7+
include.release.openshift.io/self-managed-high-availability: "true"
8+
include.release.openshift.io/single-node-developer: "true"
9+
creationTimestamp: "2023-01-09T07:07:25Z"
10+
generation: 1
11+
name: dnses.config.openshift.io
12+
ownerReferences:
13+
- apiVersion: config.openshift.io/v1
14+
kind: ClusterVersion
15+
name: version
16+
uid: ace25b63-dff4-415f-9c1d-c7583b57e40d
17+
resourceVersion: "1100"
18+
uid: 37084ebf-13f5-4c39-a9ff-fda238db6a3f
19+
spec:
20+
conversion:
21+
strategy: None
22+
group: config.openshift.io
23+
names:
24+
kind: DNS
25+
listKind: DNSList
26+
plural: dnses
27+
singular: dns
28+
scope: Cluster
29+
versions:
30+
- name: v1
31+
schema:
32+
openAPIV3Schema:
33+
description: "DNS holds cluster-wide information about DNS. The canonical
34+
name is `cluster` \n Compatibility level 1: Stable within a major release
35+
for a minimum of 12 months or 3 minor releases (whichever is longer)."
36+
properties:
37+
apiVersion:
38+
description: 'APIVersion defines the versioned schema of this representation
39+
of an object. Servers should convert recognized schemas to the latest
40+
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
41+
type: string
42+
kind:
43+
description: 'Kind is a string value representing the REST resource this
44+
object represents. Servers may infer this from the endpoint the client
45+
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
46+
type: string
47+
metadata:
48+
type: object
49+
spec:
50+
description: spec holds user settable values for configuration
51+
properties:
52+
baseDomain:
53+
description: "baseDomain is the base domain of the cluster. All managed
54+
DNS records will be sub-domains of this base. \n For example, given
55+
the base domain `openshift.example.com`, an API server DNS record
56+
may be created for `cluster-api.openshift.example.com`. \n Once
57+
set, this field cannot be changed."
58+
type: string
59+
privateZone:
60+
description: "privateZone is the location where all the DNS records
61+
that are only available internally to the cluster exist. \n If this
62+
field is nil, no private records should be created. \n Once set,
63+
this field cannot be changed."
64+
properties:
65+
id:
66+
description: "id is the identifier that can be used to find the
67+
DNS hosted zone. \n on AWS zone can be fetched using `ID` as
68+
id in [1] on Azure zone can be fetched using `ID` as a pre-determined
69+
name in [2], on GCP zone can be fetched using `ID` as a pre-determined
70+
name in [3]. \n [1]: https://docs.aws.amazon.com/cli/latest/reference/route53/get-hosted-zone.html#options
71+
[2]: https://docs.microsoft.com/en-us/cli/azure/network/dns/zone?view=azure-cli-latest#az-network-dns-zone-show
72+
[3]: https://cloud.google.com/dns/docs/reference/v1/managedZones/get"
73+
type: string
74+
tags:
75+
additionalProperties:
76+
type: string
77+
description: "tags can be used to query the DNS hosted zone. \n
78+
on AWS, resourcegroupstaggingapi [1] can be used to fetch a
79+
zone using `Tags` as tag-filters, \n [1]: https://docs.aws.amazon.com/cli/latest/reference/resourcegroupstaggingapi/get-resources.html#options"
80+
type: object
81+
type: object
82+
publicZone:
83+
description: "publicZone is the location where all the DNS records
84+
that are publicly accessible to the internet exist. \n If this field
85+
is nil, no public records should be created. \n Once set, this field
86+
cannot be changed."
87+
properties:
88+
id:
89+
description: "id is the identifier that can be used to find the
90+
DNS hosted zone. \n on AWS zone can be fetched using `ID` as
91+
id in [1] on Azure zone can be fetched using `ID` as a pre-determined
92+
name in [2], on GCP zone can be fetched using `ID` as a pre-determined
93+
name in [3]. \n [1]: https://docs.aws.amazon.com/cli/latest/reference/route53/get-hosted-zone.html#options
94+
[2]: https://docs.microsoft.com/en-us/cli/azure/network/dns/zone?view=azure-cli-latest#az-network-dns-zone-show
95+
[3]: https://cloud.google.com/dns/docs/reference/v1/managedZones/get"
96+
type: string
97+
tags:
98+
additionalProperties:
99+
type: string
100+
description: "tags can be used to query the DNS hosted zone. \n
101+
on AWS, resourcegroupstaggingapi [1] can be used to fetch a
102+
zone using `Tags` as tag-filters, \n [1]: https://docs.aws.amazon.com/cli/latest/reference/resourcegroupstaggingapi/get-resources.html#options"
103+
type: object
104+
type: object
105+
type: object
106+
status:
107+
description: status holds observed values from the cluster. They may not
108+
be overridden.
109+
type: object
110+
required:
111+
- spec
112+
type: object
113+
served: true
114+
storage: true
115+
subresources:
116+
status: {}
117+
status:
118+
acceptedNames:
119+
kind: DNS
120+
listKind: DNSList
121+
plural: dnses
122+
singular: dns
123+
conditions:
124+
- lastTransitionTime: "2023-01-09T07:07:25Z"
125+
message: no conflicts found
126+
reason: NoConflicts
127+
status: "True"
128+
type: NamesAccepted
129+
- lastTransitionTime: "2023-01-09T07:07:25Z"
130+
message: the initial names have been accepted
131+
reason: InitialNamesAccepted
132+
status: "True"
133+
type: Established
134+
storedVersions:
135+
- v1

0 commit comments

Comments
 (0)