diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a1e0ea42..7b3bea2c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -140,6 +140,8 @@ jobs: run: bash hack/test/hyg_07_nightly_race_test.sh - name: Verify manager RBAC grants core Events (not events.k8s.io) run: bash hack/test/core_events_rbac_test.sh + - name: Verify manager RBAC can read KollectClusterScope + run: bash hack/test/cluster_scope_rbac_test.sh - name: Verify changelog-sync release guard (fbb5196a3 regression lock) run: bash hack/test/changelog_sync_release_guard_test.sh # LAB-DEKIND: this suite is the enforcement mechanism for the lab substrate allowlist diff --git a/charts/kollect/templates/clusterrole.yaml b/charts/kollect/templates/clusterrole.yaml index d565ae3f..0e47ea52 100644 --- a/charts/kollect/templates/clusterrole.yaml +++ b/charts/kollect/templates/clusterrole.yaml @@ -54,7 +54,7 @@ rules: - kollecttargets/status verbs: [get, patch, update] - apiGroups: [kollect.dev] - resources: [kollectprofiles, kollectscopes] + resources: [kollectclusterscopes, kollectprofiles, kollectscopes] verbs: [get, list, watch] - apiGroups: [cert-manager.io] resources: [certificates] diff --git a/config/olm/template/manifests/kollect.clusterserviceversion.yaml b/config/olm/template/manifests/kollect.clusterserviceversion.yaml index e2ad26d0..5b0f287e 100644 --- a/config/olm/template/manifests/kollect.clusterserviceversion.yaml +++ b/config/olm/template/manifests/kollect.clusterserviceversion.yaml @@ -229,6 +229,7 @@ spec: - apiGroups: - kollect.dev resources: + - kollectclusterscopes - kollectprofiles - kollectscopes verbs: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index e50d570d..266c81a2 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -96,6 +96,7 @@ rules: - apiGroups: - kollect.dev resources: + - kollectclusterscopes - kollectprofiles - kollectscopes verbs: diff --git a/docs/operator-manual/upgrading.md b/docs/operator-manual/upgrading.md index 4ece9cd0..b5e66b45 100644 --- a/docs/operator-manual/upgrading.md +++ b/docs/operator-manual/upgrading.md @@ -87,6 +87,15 @@ Pin `image.tag` to a specific release (or use the release-pinned `install.yaml`) chart default resolves to `v` — the image shipped with that chart version — rather than a floating `latest` tag. +!!! warning "Upgrade chart and image together" + RBAC is Helm-managed, so a pinned `image.tag` newer than the installed chart runs against the + older ClusterRole. Releases after **v0.18.0** add `kollectclusterscopes` `get`/`list`/`watch` to + the manager ClusterRole and make the controller *watch* that type. A manager that cannot watch a + type it registered fails its cache sync and exits, so pairing the new image with the old + ClusterRole crash-loops the operator instead of degrading one controller. Bump the chart in the + same change as the image — or, on the raw-manifest path, re-apply `install.yaml` from the same + release. + ### 4. Wait for rollout ```sh diff --git a/hack/test/cluster_scope_rbac_test.sh b/hack/test/cluster_scope_rbac_test.sh new file mode 100755 index 00000000..c70ce43d --- /dev/null +++ b/hack/test/cluster_scope_rbac_test.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# Regression lock: scope.LoadCluster lists KollectClusterScope on every +# KollectClusterTarget/KollectClusterInventory reconcile, and both cluster-kind +# webhooks call it with failurePolicy=fail. Without get/list/watch on +# kollectclusterscopes the manager cache cannot sync that type, so reconcile +# errors and admission rejects every cluster-kind write. The grant was missing +# from the generated role and the chart until this lock landed. +# +# Third surface: the OLM CSV template carries its own copy of these rules. +# It is not re-asserted here — hack/test/dist_olm_bundle_test.sh already fails +# on any drift between config/rbac/role.yaml and the CSV clusterPermissions. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +ROLE="${ROOT}/config/rbac/role.yaml" +CLUSTERROLE_TMPL="${ROOT}/charts/kollect/templates/clusterrole.yaml" +CONTROLLERS=( + "${ROOT}/internal/controller/kollectclustertarget_controller.go" + "${ROOT}/internal/controller/kollectclusterinventory_controller.go" +) + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +pass() { + echo "ok - $*" +} + +[[ -f "${ROLE}" ]] || fail "missing ${ROLE}" +[[ -f "${CLUSTERROLE_TMPL}" ]] || fail "missing ${CLUSTERROLE_TMPL}" + +# Controllers that reach scope.LoadCluster must carry the marker; controller-gen +# aggregates them into the single manager role. +for f in "${CONTROLLERS[@]}"; do + [[ -f "${f}" ]] || fail "missing controller ${f}" + if ! grep -Eq 'kubebuilder:rbac:groups=kollect\.dev,resources=kollectclusterscopes,verbs=get;list;watch' "${f}"; then + fail "$(basename "${f}"): missing kollectclusterscopes get;list;watch marker" + fi + pass "$(basename "${f}") kollectclusterscopes marker" +done + +python3 - "${ROLE}" <<'PY' || fail "config/rbac/role.yaml missing kollectclusterscopes get/list/watch" +import sys, yaml +doc = yaml.safe_load(open(sys.argv[1])) +for rule in doc.get("rules") or []: + groups = set(rule.get("apiGroups") or []) + resources = set(rule.get("resources") or []) + verbs = set(rule.get("verbs") or []) + if "kollect.dev" in groups and "kollectclusterscopes" in resources and {"get", "list", "watch"} <= verbs: + print("ok - config/rbac/role.yaml kollectclusterscopes get/list/watch") + break +else: + sys.exit(1) +PY + +# The chart ClusterRole is hand-maintained and only rendered outside tenantMode, +# which is exactly where the cluster kinds are served. +if ! grep -Eq '^\s*resources: \[[^]]*kollectclusterscopes[^]]*\]' "${CLUSTERROLE_TMPL}"; then + fail "$(basename "${CLUSTERROLE_TMPL}"): no kollectclusterscopes in any resources list" +fi +if ! awk ' + /resources: \[[^]]*kollectclusterscopes[^]]*\]/ { found=1; next } + found && /verbs: \[get, list, watch\]/ { ok=1 } + found && /verbs:/ && !ok { exit 1 } + END { exit ok ? 0 : 1 } +' "${CLUSTERROLE_TMPL}"; then + fail "$(basename "${CLUSTERROLE_TMPL}"): kollectclusterscopes rule lacks verbs [get, list, watch]" +fi +pass "$(basename "${CLUSTERROLE_TMPL}") kollectclusterscopes get/list/watch" + +echo "All cluster_scope_rbac tests passed." diff --git a/internal/controller/kollectclusterinventory_controller.go b/internal/controller/kollectclusterinventory_controller.go index 024edfb5..77bdc205 100644 --- a/internal/controller/kollectclusterinventory_controller.go +++ b/internal/controller/kollectclusterinventory_controller.go @@ -59,6 +59,7 @@ type KollectClusterInventoryReconciler struct { // +kubebuilder:rbac:groups=kollect.dev,resources=kollectclusterinventories/finalizers,verbs=update // +kubebuilder:rbac:groups=kollect.dev,resources=kollectclustertargets,verbs=get;list;watch // +kubebuilder:rbac:groups=kollect.dev,resources=kollectsnapshotsinks;kollectdatabasesinks;kollecteventsinks,verbs=get;list;watch +// +kubebuilder:rbac:groups=kollect.dev,resources=kollectclusterscopes,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch diff --git a/internal/controller/kollectclustertarget_controller.go b/internal/controller/kollectclustertarget_controller.go index 0cce13d2..5fee9481 100644 --- a/internal/controller/kollectclustertarget_controller.go +++ b/internal/controller/kollectclustertarget_controller.go @@ -38,6 +38,7 @@ type KollectClusterTargetReconciler struct { // +kubebuilder:rbac:groups=kollect.dev,resources=kollectclustertargets/status,verbs=get;update;patch // +kubebuilder:rbac:groups=kollect.dev,resources=kollectclustertargets/finalizers,verbs=update // +kubebuilder:rbac:groups=kollect.dev,resources=kollectprofiles,verbs=get;list;watch +// +kubebuilder:rbac:groups=kollect.dev,resources=kollectclusterscopes,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch @@ -353,6 +354,10 @@ func (r *KollectClusterTargetReconciler) SetupWithManager(mgr ctrl.Manager) erro &kollectdevv1alpha1.KollectProfile{}, handler.EnqueueRequestsFromMapFunc(r.mapProfileToClusterTargets), ). + Watches( + &kollectdevv1alpha1.KollectClusterScope{}, + handler.EnqueueRequestsFromMapFunc(r.mapClusterScopeToClusterTargets), + ). Named("kollectclustertarget"). Complete(r) } @@ -376,6 +381,33 @@ func (r *KollectClusterTargetReconciler) mapNamespaceToClusterTargets( return reqs } +// mapClusterScopeToClusterTargets re-reconciles every KollectClusterTarget on any +// KollectClusterScope write. It does not filter to the enforced scope on purpose: +// scope.LoadCluster resolves the ceiling as the lowest-named object of all of them, +// so creating or renaming any KollectClusterScope can change which one is enforced. +func (r *KollectClusterTargetReconciler) mapClusterScopeToClusterTargets( + ctx context.Context, + obj client.Object, +) []reconcile.Request { + if _, ok := obj.(*kollectdevv1alpha1.KollectClusterScope); !ok { + return nil + } + + var list kollectdevv1alpha1.KollectClusterTargetList + if err := r.List(ctx, &list); err != nil { + return nil + } + + reqs := make([]reconcile.Request, 0, len(list.Items)) + for i := range list.Items { + reqs = append(reqs, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: list.Items[i].Name}, + }) + } + + return reqs +} + func (r *KollectClusterTargetReconciler) mapProfileToClusterTargets( ctx context.Context, obj client.Object, diff --git a/internal/controller/kollectclustertarget_map_test.go b/internal/controller/kollectclustertarget_map_test.go index dff9ccfd..f3fd7e6c 100644 --- a/internal/controller/kollectclustertarget_map_test.go +++ b/internal/controller/kollectclustertarget_map_test.go @@ -55,3 +55,41 @@ func TestKollectClusterTargetReconciler_mapFunctions(t *testing.T) { t.Fatalf("non-profile object should return nil, got %#v", got) } } + +func TestKollectClusterTargetReconciler_mapClusterScopeToClusterTargets(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + if err := kollectdevv1alpha1.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + first := &kollectdevv1alpha1.KollectClusterTarget{ObjectMeta: metav1.ObjectMeta{Name: "ct-a"}} + second := &kollectdevv1alpha1.KollectClusterTarget{ObjectMeta: metav1.ObjectMeta{Name: "ct-b"}} + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(first, second).Build() + r := &KollectClusterTargetReconciler{Client: cl} + + // Any KollectClusterScope write can change the enforced ceiling — LoadCluster + // picks the lowest-named object of all of them — so every target re-reconciles + // regardless of which scope object was written. + scopeObj := &kollectdevv1alpha1.KollectClusterScope{ObjectMeta: metav1.ObjectMeta{Name: "zz-not-enforced"}} + reqs := r.mapClusterScopeToClusterTargets(context.Background(), scopeObj) + if len(reqs) != 2 { + t.Fatalf("cluster scope map reqs = %#v, want one per cluster target", reqs) + } + + names := map[string]bool{} + for _, req := range reqs { + if req.Namespace != "" { + t.Fatalf("cluster-scoped request must not carry a namespace: %#v", req) + } + names[req.Name] = true + } + if !names["ct-a"] || !names["ct-b"] { + t.Fatalf("cluster scope map reqs = %#v, want ct-a and ct-b", reqs) + } + + if got := r.mapClusterScopeToClusterTargets(context.Background(), first); got != nil { + t.Fatalf("non-scope object should return nil, got %#v", got) + } +}