Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manager should not panic and should ignore wrong Clusterscoped type setting in HNCConfiguration #72

Merged
merged 1 commit into from
Aug 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/v1alpha2/hnc_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const (
// Condition reasons for BadConfiguration
ReasonMultipleConfigsForType = "MultipleConfigurationsForType"
ReasonResourceNotFound = "ResourceNotFound"
ReasonResourceNotNamespaced = "ResourceNotNamespaced"

// Condition reason for OutOfSync, e.g. errors when creating a reconciler.
ReasonUnknown = "Unknown"
Expand Down
20 changes: 17 additions & 3 deletions internal/reconcilers/hnc_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ type gr2gvkMode map[schema.GroupResource]gvkMode
// gvk2gr keeps track of a group of unique GVKs with the mapping GRs.
type gvk2gr map[schema.GroupVersionKind]schema.GroupResource

type GVKErr struct {
Reason string
Msg string
}

func (e *GVKErr) Error() string {
return e.Msg
}

// checkPeriod is the period that the config reconciler checks if it needs to reconcile the
// `config` singleton.
const checkPeriod = 3 * time.Second
Expand Down Expand Up @@ -188,10 +197,12 @@ func (r *ConfigReconciler) reconcileConfigTypes(inst *api.HNCConfiguration, allR
// Look if the resource exists in the API server.
gvk, err := GVKFor(gr, allRes)
Abirdcfly marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
// If the type is not found, log error and write conditions but don't
// If the type is not found or namespaced, log error and write conditions but don't
// early exit since the other types can still be reconciled.
r.Log.Error(err, "while trying to reconcile the configuration", "type", gr, "mode", rsc.Mode)
r.writeCondition(inst, api.ConditionBadTypeConfiguration, api.ReasonResourceNotFound, err.Error())
if gvkerr, ok := err.(*GVKErr); ok {
r.writeCondition(inst, api.ConditionBadTypeConfiguration, gvkerr.Reason, gvkerr.Msg)
}
continue
}
r.activeGVKMode[gr] = gvkMode{gvk, rsc.Mode}
Expand Down Expand Up @@ -599,6 +610,9 @@ func GVKFor(gr schema.GroupResource, allRes []*restmapper.APIGroupResources) (sc
for _, version := range group.Versions {
for _, resource := range groupedResources.VersionedResources[version.Version] {
if resource.Name == gr.Resource {
if !resource.Namespaced {
return schema.GroupVersionKind{}, &GVKErr{api.ReasonResourceNotNamespaced, fmt.Sprintf("Resource %q is not namespaced", gr)}
}
// Please note that we cannot use resource.group or resource.version
// here because they are preferred group/version and they are default
// to empty to imply this current containing group/version. Therefore,
Expand All @@ -613,5 +627,5 @@ func GVKFor(gr schema.GroupResource, allRes []*restmapper.APIGroupResources) (sc
}
}
}
return schema.GroupVersionKind{}, fmt.Errorf("Resource %q not found", gr)
return schema.GroupVersionKind{}, &GVKErr{api.ReasonResourceNotFound, fmt.Sprintf("Resource %q not found", gr)}
}
8 changes: 8 additions & 0 deletions internal/reconcilers/hnc_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,14 @@ var _ = Describe("HNCConfiguration", func() {
Expect(objectInheritedFrom(ctx, "crontabs", barName, "foo-crontab")).Should(Equal(fooName))
})

It("manager should not panic and ignore wrong Clusterscoped type setting in HNCConfiguration", func() {
// Add a config for a type that hasn't been defined yet.
addToHNCConfig(ctx, api.RBACGroup, "clusterroles", api.Propagate)

Eventually(getHNCConfigCondition(ctx, api.ConditionBadTypeConfiguration, api.ReasonResourceNotNamespaced)).
Should(ContainSubstring("Resource \"clusterroles.rbac.authorization.k8s.io\" is not namespaced"))
})

It("should set NumPropagatedObjects back to 0 after deleting the source object in propagate mode", func() {
addToHNCConfig(ctx, "", "limitranges", api.Propagate)
setParent(ctx, barName, fooName)
Expand Down