Skip to content
This repository was archived by the owner on Apr 17, 2025. It is now read-only.

Commit 572c781

Browse files
committed
fix: Ignore other HNC configurations than the first one even when given irregular GroupResources.
Cases: - GroupResource is specified in a singular form - GroupResource is specified without group
1 parent 854e022 commit 572c781

File tree

2 files changed

+45
-11
lines changed

2 files changed

+45
-11
lines changed

internal/hncconfig/reconciler.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,20 @@ func (r *Reconciler) reconcileConfigTypes(inst *api.HNCConfiguration) {
148148
// Get valid settings in the spec.resources of the `config` singleton.
149149
for _, rsc := range inst.Spec.Resources {
150150
gr := schema.GroupResource{Group: rsc.Group, Resource: rsc.Resource}
151-
// If there are multiple configurations of the same type, we will follow the
151+
// Look if the resource exists in the API server.
152+
gvk, err := r.resourceMapper.NamespacedKindFor(gr)
153+
if err != nil {
154+
// Log error and write conditions, but don't early exit since the other types can still be reconciled.
155+
r.Log.Error(err, "while trying to reconcile the configuration", "type", gr, "mode", rsc.Mode)
156+
r.writeCondition(inst, api.ConditionBadTypeConfiguration, reasonForGVKError(err), err.Error())
157+
continue
158+
}
159+
160+
// If there are multiple configurations of the same GVK, we will follow the
152161
// first configuration and ignore the rest.
153-
if gvkMode, exist := r.activeGVKMode[gr]; exist {
154-
log := r.Log.WithValues("resource", gr, "appliedMode", gvkMode.mode)
162+
if firstGR, exist := r.activeGR[gvk]; exist {
163+
gvkMode := r.activeGVKMode[firstGR]
164+
log := r.Log.WithValues("resource", gr, "appliedMode", rsc.Mode)
155165
msg := ""
156166
// Set a different message if the type is enforced by HNC.
157167
if api.IsEnforcedType(rsc) {
@@ -165,14 +175,6 @@ func (r *Reconciler) reconcileConfigTypes(inst *api.HNCConfiguration) {
165175
continue
166176
}
167177

168-
// Look if the resource exists in the API server.
169-
gvk, err := r.resourceMapper.NamespacedKindFor(gr)
170-
if err != nil {
171-
// Log error and write conditions, but don't early exit since the other types can still be reconciled.
172-
r.Log.Error(err, "while trying to reconcile the configuration", "type", gr, "mode", rsc.Mode)
173-
r.writeCondition(inst, api.ConditionBadTypeConfiguration, reasonForGVKError(err), err.Error())
174-
continue
175-
}
176178
r.activeGVKMode[gr] = gvkMode{gvk, rsc.Mode}
177179
r.activeGR[gvk] = gr
178180
}

internal/hncconfig/reconciler_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,22 @@ var _ = Describe("HNCConfiguration", func() {
8989
Eventually(GetHNCConfigCondition(ctx, api.ConditionBadTypeConfiguration, api.ReasonMultipleConfigsForType)).Should(ContainSubstring(api.RoleResource))
9090
})
9191

92+
It("should ignore the `roles` configuration in the spec and set `MultipleConfigurationsForType` condition even when given in a singular form", func() {
93+
AddToHNCConfig(ctx, api.RBACGroup, "role", api.Ignore)
94+
95+
Eventually(typeSpecMode(ctx, api.RBACGroup, "role")).Should(Equal(api.Ignore))
96+
Eventually(typeStatusMode(ctx, api.RBACGroup, api.RoleResource)).Should(Equal(api.Propagate))
97+
Eventually(GetHNCConfigCondition(ctx, api.ConditionBadTypeConfiguration, api.ReasonMultipleConfigsForType)).Should(ContainSubstring("role"))
98+
})
99+
100+
It("should ignore the `roles` configuration in the spec and set `MultipleConfigurationsForType` condition even when specified without group", func() {
101+
AddToHNCConfig(ctx, "", api.RoleResource, api.Ignore)
102+
103+
Eventually(typeSpecMode(ctx, "", api.RoleResource)).Should(Equal(api.Ignore))
104+
Eventually(typeStatusMode(ctx, api.RBACGroup, api.RoleResource)).Should(Equal(api.Propagate))
105+
Eventually(GetHNCConfigCondition(ctx, api.ConditionBadTypeConfiguration, api.ReasonMultipleConfigsForType)).Should(ContainSubstring(api.RoleResource))
106+
})
107+
92108
It("should propagate RoleBindings by default", func() {
93109
SetParent(ctx, barName, fooName)
94110
// Give foo a role binding.
@@ -106,6 +122,22 @@ var _ = Describe("HNCConfiguration", func() {
106122
Eventually(GetHNCConfigCondition(ctx, api.ConditionBadTypeConfiguration, api.ReasonMultipleConfigsForType)).Should(ContainSubstring(api.RoleBindingResource))
107123
})
108124

125+
It("should ignore the `rolebindings` configuration in the spec and set `MultipleConfigurationsForType` condition even when given in a singular form", func() {
126+
AddToHNCConfig(ctx, api.RBACGroup, "rolebinding", api.Ignore)
127+
128+
Eventually(typeSpecMode(ctx, api.RBACGroup, "rolebinding")).Should(Equal(api.Ignore))
129+
Eventually(typeStatusMode(ctx, api.RBACGroup, api.RoleBindingResource)).Should(Equal(api.Propagate))
130+
Eventually(GetHNCConfigCondition(ctx, api.ConditionBadTypeConfiguration, api.ReasonMultipleConfigsForType)).Should(ContainSubstring("rolebinding"))
131+
})
132+
133+
It("should ignore the `rolebindings` configuration in the spec and set `MultipleConfigurationsForType` condition even when specified without group", func() {
134+
AddToHNCConfig(ctx, "", api.RoleBindingResource, api.Ignore)
135+
136+
Eventually(typeSpecMode(ctx, "", api.RoleBindingResource)).Should(Equal(api.Ignore))
137+
Eventually(typeStatusMode(ctx, api.RBACGroup, api.RoleBindingResource)).Should(Equal(api.Propagate))
138+
Eventually(GetHNCConfigCondition(ctx, api.ConditionBadTypeConfiguration, api.ReasonMultipleConfigsForType)).Should(ContainSubstring(api.RoleBindingResource))
139+
})
140+
109141
It("should unset ResourceNotFound condition if a bad type spec is removed", func() {
110142
// Group of ConfigMap should be ""
111143
AddToHNCConfig(ctx, "wrong", "configmaps", api.Propagate)

0 commit comments

Comments
 (0)