From 7b214464ee88e6ae67782cc8b114ebe4cd33660d Mon Sep 17 00:00:00 2001 From: Timofei Larkin Date: Wed, 14 Oct 2026 11:58:49 +0200 Subject: [PATCH] fix(lineage-webhook): keep an explicit tenantresource false The webhook computes the tenantresource verdict from the ApplicationDefinition selectors on first admission and copies it over whatever label the chart rendered, so a chart cannot take a Secret the definition still selects off the tenantsecrets projection. The credentials conversion in design-proposals/user-secrets-api needs exactly that during its migration window: the converted chart renders the platform-only credentials Secret with tenantresource false while the old definition still selects the name. A rendered false is now kept; a rendered true is still replaced by the computed verdict, so the object can only narrow what the definition grants, never widen it. Assisted-by: LLM Signed-off-by: Timofei Larkin --- internal/lineagecontrollerwebhook/webhook.go | 19 ++- .../webhook_tenantresource_test.go | 115 ++++++++++++++++++ 2 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 internal/lineagecontrollerwebhook/webhook_tenantresource_test.go diff --git a/internal/lineagecontrollerwebhook/webhook.go b/internal/lineagecontrollerwebhook/webhook.go index ea8b0d01b..d2ef5f938 100644 --- a/internal/lineagecontrollerwebhook/webhook.go +++ b/internal/lineagecontrollerwebhook/webhook.go @@ -193,12 +193,19 @@ func (h *LineageControllerWebhook) computeLabels(ctx context.Context, obj *unstr crd := cfg.appCRDMap[appRef{gv.Group, owner.GetKind()}] resourceSelectors := h.getResourceSelectors(obj.GroupVersionKind().GroupKind(), crd) - labels[corev1alpha1.TenantResourceLabelKey] = func(b bool) string { - if b { - return corev1alpha1.TenantResourceLabelValue - } - return "false" - }(matchResourceToExcludeInclude(ctx, obj.GetName(), templateLabels, obj.GetLabels(), resourceSelectors)) + selected := matchResourceToExcludeInclude(ctx, obj.GetName(), templateLabels, obj.GetLabels(), resourceSelectors) + // The verdict can be narrowed by the object and never widened: a chart that + // renders tenantresource "false" keeps it even where the definition selects + // the name, while a chart-set "true" is replaced by the computed verdict. + // This is what lets a chart take a Secret the definition still selects off + // the tenantsecrets projection while the definition catches up. + if obj.GetLabels()[corev1alpha1.TenantResourceLabelKey] == "false" { + selected = false + } + labels[corev1alpha1.TenantResourceLabelKey] = "false" + if selected { + labels[corev1alpha1.TenantResourceLabelKey] = corev1alpha1.TenantResourceLabelValue + } return labels, nil } diff --git a/internal/lineagecontrollerwebhook/webhook_tenantresource_test.go b/internal/lineagecontrollerwebhook/webhook_tenantresource_test.go new file mode 100644 index 000000000..4fe9f6af8 --- /dev/null +++ b/internal/lineagecontrollerwebhook/webhook_tenantresource_test.go @@ -0,0 +1,115 @@ +package lineagecontrollerwebhook + +import ( + "context" + "encoding/json" + "strings" + "testing" + + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" + corev1alpha1 "github.com/cozystack/cozystack/pkg/apis/core/v1alpha1" + admissionv1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// The tenantresource verdict is the definition's, but an object may narrow it +// and never widen it: a rendered "false" survives a selector that matches the +// name, a rendered "true" does not survive one that does not. +func TestHandle_TenantResourceLabelNarrowsOnly(t *testing.T) { + tenantResourcePath := "/metadata/labels/" + strings.ReplaceAll(corev1alpha1.TenantResourceLabelKey, "/", "~1") + + tests := []struct { + name string + secret string + rendered string + // want is the value the label must hold once the patch is applied; + // "" means the response must not touch the label at all. + want string + }{ + { + name: "a rendered false is kept where the definition selects the name", + secret: "harbor-demo-credentials", + rendered: "false", + want: "", + }, + { + name: "a rendered true is overwritten where the definition does not select the name", + secret: "harbor-demo-other", + rendered: "true", + want: "false", + }, + { + name: "no rendered label gets the definition's verdict", + secret: "harbor-demo-credentials", + rendered: "", + want: "true", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := newHandleTestWebhook(t) + w.config.Store(&runtimeConfig{appCRDMap: map[appRef]*cozyv1alpha1.ApplicationDefinition{ + {"apps.cozystack.io", "Harbor"}: {Spec: cozyv1alpha1.ApplicationDefinitionSpec{ + Application: cozyv1alpha1.ApplicationDefinitionApplication{Kind: "Harbor"}, + Secrets: cozyv1alpha1.ApplicationDefinitionResources{ + Include: []*cozyv1alpha1.ApplicationDefinitionResourceSelector{ + {ResourceNames: []string{"{{ .kind }}-{{ .name }}-credentials"}}, + }, + }, + }}, + }}) + + labels := map[string]string{"helm.toolkit.fluxcd.io/name": "harbor-demo"} + if tt.rendered != "" { + labels[corev1alpha1.TenantResourceLabelKey] = tt.rendered + } + obj := &unstructured.Unstructured{} + obj.SetAPIVersion("v1") + obj.SetKind("Secret") + obj.SetNamespace("tenant-demo") + obj.SetName(tt.secret) + obj.SetLabels(labels) + raw, err := json.Marshal(obj) + if err != nil { + t.Fatalf("marshal object: %v", err) + } + + resp := w.Handle(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{ + Kind: metav1.GroupVersionKind{Version: "v1", Kind: "Secret"}, + Namespace: "tenant-demo", + Name: tt.secret, + Operation: admissionv1.Create, + Object: runtime.RawExtension{Raw: raw}, + }}) + if !resp.Allowed { + t.Fatalf("expected allowed response, got %+v", resp.Result) + } + + var got string + var touched bool + for _, p := range resp.Patches { + if p.Path != tenantResourcePath { + continue + } + touched = true + v, ok := p.Value.(string) + if !ok { + t.Fatalf("patch %s carries %T, want string", p.Path, p.Value) + } + got = v + } + switch { + case tt.want == "" && touched: + t.Errorf("expected the label untouched, got patch value %q", got) + case tt.want != "" && !touched: + t.Errorf("expected a patch setting the label to %q, got none", tt.want) + case tt.want != "" && got != tt.want: + t.Errorf("label patched to %q, want %q", got, tt.want) + } + }) + } +}