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

Add per-object-level filtering based on labels (excluding Rancher-generated resources) #125

Merged
merged 1 commit into from
Jan 24, 2022
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
8 changes: 8 additions & 0 deletions docs/user-guide/concepts.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,14 @@ adding annotations is not possible and HNC will by default exclude them.
Similarly, Kubernetes ServiceAccount Secrets will also by default be excluded
from propagation.

In addition, propagation exclusions are also used for Rancher-managed Kubernetes
clusters. Rancher uses a "project" concept that bundles namespaces and thus sets
roles, rolebindings, etc. for all namespaces of a project. This leads to
conflicts with HNC, so all resources created by Rancher (which are automatically
labeled with `"cattle.io/creator": "norman"` by Rancher, cf. [their
docs](https://rancher.com/docs/rancher/v2.6/en/system-tools/#remove)) are
excluded from propagation.

<a name="admin"/>

## Administration
Expand Down
21 changes: 17 additions & 4 deletions internal/integtest/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,9 @@ func MakeObject(ctx context.Context, resource string, nsName, name string) {
createdObjects = append(createdObjects, inst)
}

// MakeObjectWithAnnotation creates an empty object with annotation given kind in a specific
// MakeObjectWithAnnotations creates an empty object with annotation given kind in a specific
// namespace. The kind and its corresponding GVK should be included in the GVKs map.
func MakeObjectWithAnnotation(ctx context.Context, resource string, nsName,
func MakeObjectWithAnnotations(ctx context.Context, resource string, nsName,
name string, a map[string]string) {
inst := &unstructured.Unstructured{}
inst.SetGroupVersionKind(GVKs[resource])
Expand All @@ -313,9 +313,22 @@ func MakeObjectWithAnnotation(ctx context.Context, resource string, nsName,
createdObjects = append(createdObjects, inst)
}

// UpdateObjectWithAnnotation gets an object given it's kind, nsName and name, adds the annotation
// MakeObjectWithLabels creates an empty object with label given kind in a specific
// namespace. The kind and its corresponding GVK should be included in the GVKs map.
func MakeObjectWithLabels(ctx context.Context, resource string, nsName,
name string, l map[string]string) {
inst := &unstructured.Unstructured{}
inst.SetGroupVersionKind(GVKs[resource])
inst.SetNamespace(nsName)
inst.SetName(name)
inst.SetLabels(l)
ExpectWithOffset(1, K8sClient.Create(ctx, inst)).Should(Succeed())
createdObjects = append(createdObjects, inst)
}

// UpdateObjectWithAnnotations gets an object given it's kind, nsName and name, adds the annotation
// and updates this object
func UpdateObjectWithAnnotation(ctx context.Context, resource string, nsName,
func UpdateObjectWithAnnotations(ctx context.Context, resource string, nsName,
name string, a map[string]string) error {
nnm := types.NamespacedName{Namespace: nsName, Name: name}
inst := &unstructured.Unstructured{}
Expand Down
35 changes: 28 additions & 7 deletions internal/objects/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ var _ = Describe("Exceptions", func() {
tc.treeSelector = ReplaceStrings(tc.treeSelector, names)

// Create a Role with the selector and treeSelector annotation
MakeObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{
MakeObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{
api.AnnotationSelector: tc.selector,
api.AnnotationTreeSelector: tc.treeSelector,
api.AnnotationNoneSelector: tc.noneSelector,
Expand Down Expand Up @@ -215,7 +215,7 @@ var _ = Describe("Exceptions", func() {
}

// update the role with the selector and treeSelector annotation
UpdateObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{
UpdateObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{
api.AnnotationSelector: tc.selector,
api.AnnotationTreeSelector: tc.treeSelector,
api.AnnotationNoneSelector: tc.noneSelector,
Expand All @@ -232,7 +232,7 @@ var _ = Describe("Exceptions", func() {
}

// remove the annotation and verify that the object is back for every namespace
UpdateObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{})
UpdateObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{})
for _, ns := range names {
Eventually(HasObject(ctx, api.RoleResource, ns, "testrole")).Should(BeTrue(), "When propagating testrole to %s", ns)
}
Expand Down Expand Up @@ -304,7 +304,7 @@ var _ = Describe("Exceptions", func() {
Eventually(HasObject(ctx, api.RoleResource, names[nolabelchild], "testrole")).Should(BeTrue(), "When propagating testrole to %s", names[nolabelchild])
// Add `select` exception annotation with propagate label and verify the
// object is only propagated to children with the label.
UpdateObjectWithAnnotation(ctx, api.RoleResource, names[p], "testrole", map[string]string{
UpdateObjectWithAnnotations(ctx, api.RoleResource, names[p], "testrole", map[string]string{
api.AnnotationSelector: label,
})
Eventually(HasObject(ctx, api.RoleResource, names[nolabelchild], "testrole")).Should(BeFalse(), "When propagating testrole to %s", names[nolabelchild])
Expand Down Expand Up @@ -398,7 +398,7 @@ var _ = Describe("Basic propagation", func() {
Expect(ObjectInheritedFrom(ctx, "configmaps", barName, "foo-config")).Should(Equal(fooName))
})

It("should not propagate builtin exclusions", func() {
It("should not propagate builtin exclusions by name", func() {
SetParent(ctx, barName, fooName)
MakeObject(ctx, "configmaps", fooName, "istio-ca-root-cert")
MakeObject(ctx, "configmaps", fooName, "kube-root-ca.crt")
Expand All @@ -411,6 +411,27 @@ var _ = Describe("Basic propagation", func() {
Eventually(HasObject(ctx, "configmaps", barName, "kube-root-ca.crt")).Should(BeFalse())
})

It("should not propagate builtin exclusions by labels", func() {
SetParent(ctx, barName, fooName)
MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-blocked", map[string]string{"cattle.io/creator": "norman"})
MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-wrong-value", map[string]string{"cattle.io/creator": "testme"})
MakeObjectWithLabels(ctx, "roles", fooName, "role-with-labels-something", map[string]string{"app": "hello-world"})
AddToHNCConfig(ctx, api.RBACGroup, api.RoleKind, api.Propagate)

// the first one should not propagate, everything else should
Consistently(HasObject(ctx, "roles", barName, "role-with-labels-blocked")).Should(BeFalse())
Eventually(HasObject(ctx, "roles", barName, "role-with-labels-wrong-value")).Should(BeTrue())
Eventually(HasObject(ctx, "roles", barName, "role-with-labels-something")).Should(BeTrue())

// lets try the same with config maps, they are also filtered and should not propagate
MakeObjectWithLabels(ctx, "configmaps", fooName, "cm-with-label-1", map[string]string{"cattle.io/creator": "norman"})
MakeObjectWithLabels(ctx, "configmaps", fooName, "cm-with-label-2", map[string]string{"app": "hello-world"})
AddToHNCConfig(ctx, "", "configmaps", api.Propagate)

Consistently(HasObject(ctx, "configmaps", barName, "cm-with-label-1")).Should(BeFalse())
Eventually(HasObject(ctx, "configmaps", barName, "cm-with-label-2")).Should(BeTrue())
})

It("should be removed if the hierarchy changes", func() {
SetParent(ctx, barName, fooName)
SetParent(ctx, bazName, barName)
Expand Down Expand Up @@ -658,7 +679,7 @@ var _ = Describe("Basic propagation", func() {

It("should avoid propagating banned annotations", func() {
SetParent(ctx, barName, fooName)
MakeObjectWithAnnotation(ctx, "roles", fooName, "foo-annot-role", map[string]string{
MakeObjectWithAnnotations(ctx, "roles", fooName, "foo-annot-role", map[string]string{
"annot-a": "value-a",
"annot-b": "value-b",
})
Expand All @@ -682,7 +703,7 @@ var _ = Describe("Basic propagation", func() {

// Tell the HNC config not to propagate annot-a and verify that this time, it's not annotated
config.UnpropagatedAnnotations = []string{"annot-a"}
MakeObjectWithAnnotation(ctx, "roles", fooName, "foo-annot-role", map[string]string{
MakeObjectWithAnnotations(ctx, "roles", fooName, "foo-annot-role", map[string]string{
"annot-a": "value-a",
"annot-b": "value-b",
})
Expand Down
33 changes: 30 additions & 3 deletions internal/selectors/selectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,19 +158,46 @@ func GetNoneSelector(inst *unstructured.Unstructured) (bool, error) {
return noneSelector, nil
}

// cmExclusions are known (istio and kube-root) CA configmap which are excluded from propagation
var cmExclusions = []string{"istio-ca-root-cert", "kube-root-ca.crt"}
// cmExclusionsByName are known (istio and kube-root) CA configmap which are excluded from propagation
var cmExclusionsByName = []string{"istio-ca-root-cert", "kube-root-ca.crt"}

// A label as a key, value pair, used to exclude resources matching this label (both key and value).
type ExclusionByLabelsSpec struct {
Key string
Value string
}

// ExclusionByLabelsSpec are known label key-value pairs which are excluded from propagation. Right
// now only used to exclude resources created by Rancher, see "System Tools > Remove"
// (https://rancher.com/docs/rancher/v2.6/en/system-tools/#remove)
var exclusionByLabels = []ExclusionByLabelsSpec{
{Key: "cattle.io/creator", Value: "norman"},
}

// isExcluded returns true to indicate that this object is excluded from being propagated
func isExcluded(inst *unstructured.Unstructured) (bool, error) {
name := inst.GetName()
kind := inst.GetKind()
group := inst.GroupVersionKind().Group
labels := inst.GetLabels()

for _, excludedResourceName := range cmExclusions {
// exclusion by name
for _, excludedResourceName := range cmExclusionsByName {
if group == "" && kind == "ConfigMap" && name == excludedResourceName {
return true, nil
}
}

// exclusion by labels
for _, res := range exclusionByLabels {
gotLabelValue, ok := labels[res.Key]
nobbs marked this conversation as resolved.
Show resolved Hide resolved
// check for presence has to be explicit, as empty label values are allowed and a
// nonexisting key in the `labels` map will also return an empty string ("") - potentially
// causing false matches if `ok` is not checked
if ok && gotLabelValue == res.Value {
return true, nil
}
}

return false, nil
}