mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-12 23:54:01 +09:00
fix(lineage-webhook): keep a rendered false only on create
The cacert controller re-runs admission after a selector change by dropping managed-by-cozystack and keeping the other labels, so the UPDATE carries the "false" this webhook wrote on an earlier admission. Reading that as the chart's narrowing froze the old verdict: a projection, once revoked, could not be granted again. The chart's narrowing only matters on CREATE, since an existing Secret already carries managed-by-cozystack and skips the webhook. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
This commit is contained in:
1 parent
d687e73fd4
commit
730fd5ebf5
2 files changed
+48
-11
No files matched your search
@@ -9,6 +9,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"github.com/cozystack/cozystack/pkg/lineage"
|
||||
admissionv1 "k8s.io/api/admission/v1"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
|
||||
@@ -123,7 +124,7 @@ func (h *LineageControllerWebhook) Handle(ctx context.Context, req admission.Req
|
||||
logger.Error(err, "error computing lineage labels")
|
||||
return admission.Errored(500, fmt.Errorf("error computing lineage labels: %w", err))
|
||||
}
|
||||
labels, err := h.computeLabels(ctx, obj, owner)
|
||||
labels, err := h.computeLabels(ctx, obj, owner, req.Operation)
|
||||
if err != nil {
|
||||
logger.Error(err, "error computing lineage labels")
|
||||
return admission.Errored(500, fmt.Errorf("error computing lineage labels: %w", err))
|
||||
@@ -159,7 +160,7 @@ func (h *LineageControllerWebhook) getOwner(ctx context.Context, o *unstructured
|
||||
return obj, err
|
||||
}
|
||||
|
||||
func (h *LineageControllerWebhook) computeLabels(ctx context.Context, obj *unstructured.Unstructured, owner *unstructured.Unstructured) (map[string]string, error) {
|
||||
func (h *LineageControllerWebhook) computeLabels(ctx context.Context, obj *unstructured.Unstructured, owner *unstructured.Unstructured, op admissionv1.Operation) (map[string]string, error) {
|
||||
if owner == nil {
|
||||
return nil, nil
|
||||
}
|
||||
@@ -194,10 +195,13 @@ func (h *LineageControllerWebhook) computeLabels(ctx context.Context, obj *unstr
|
||||
resourceSelectors := h.getResourceSelectors(obj.GroupVersionKind().GroupKind(), crd)
|
||||
|
||||
selected := matchResourceToExcludeInclude(ctx, obj.GetName(), templateLabels, obj.GetLabels(), resourceSelectors)
|
||||
// The object may narrow the verdict and never widen it: a rendered "false"
|
||||
// is kept even where the definition selects the name, a rendered "true" is
|
||||
// replaced by the computed verdict.
|
||||
if obj.GetLabels()[corev1alpha1.TenantResourceLabelKey] == "false" {
|
||||
// On CREATE the object may narrow the verdict and never widen it: a
|
||||
// rendered "false" is kept even where the definition selects the name, a
|
||||
// rendered "true" is replaced. Only on CREATE: a re-admission (UPDATE with
|
||||
// managed-by-cozystack dropped, as the cacert controller does on a selector
|
||||
// change) carries the "false" this webhook wrote earlier, and keeping it
|
||||
// would block a grant.
|
||||
if op == admissionv1.Create && obj.GetLabels()[corev1alpha1.TenantResourceLabelKey] == "false" {
|
||||
selected = false
|
||||
}
|
||||
labels[corev1alpha1.TenantResourceLabelKey] = "false"
|
||||
|
||||
@@ -16,13 +16,15 @@ import (
|
||||
)
|
||||
|
||||
// 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.
|
||||
// on CREATE and never widen it: a rendered "false" survives a selector that
|
||||
// matches the name, a rendered "true" does not survive one that does not. A
|
||||
// re-admission UPDATE gets the definition's verdict whatever the label says.
|
||||
func TestHandle_TenantResourceLabelNarrowsOnly(t *testing.T) {
|
||||
tenantResourcePath := "/metadata/labels/" + strings.ReplaceAll(corev1alpha1.TenantResourceLabelKey, "/", "~1")
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
op admissionv1.Operation
|
||||
secret string
|
||||
rendered string
|
||||
// want is the value the label must hold once the patch is applied;
|
||||
@@ -31,22 +33,41 @@ func TestHandle_TenantResourceLabelNarrowsOnly(t *testing.T) {
|
||||
}{
|
||||
{
|
||||
name: "a rendered false is kept where the definition selects the name",
|
||||
op: admissionv1.Create,
|
||||
secret: "harbor-demo-credentials",
|
||||
rendered: "false",
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "a rendered true is overwritten where the definition does not select the name",
|
||||
op: admissionv1.Create,
|
||||
secret: "harbor-demo-other",
|
||||
rendered: "true",
|
||||
want: "false",
|
||||
},
|
||||
{
|
||||
name: "no rendered label gets the definition's verdict",
|
||||
op: admissionv1.Create,
|
||||
secret: "harbor-demo-credentials",
|
||||
rendered: "",
|
||||
want: "true",
|
||||
},
|
||||
{
|
||||
// The cacert controller's re-admission after a selector change:
|
||||
// managed-by-cozystack dropped, the old verdict still on the object.
|
||||
name: "a re-admission grants where the definition now selects the name",
|
||||
op: admissionv1.Update,
|
||||
secret: "harbor-demo-credentials",
|
||||
rendered: "false",
|
||||
want: "true",
|
||||
},
|
||||
{
|
||||
name: "a re-admission revokes where the definition no longer selects the name",
|
||||
op: admissionv1.Update,
|
||||
secret: "harbor-demo-other",
|
||||
rendered: "true",
|
||||
want: "false",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
@@ -78,13 +99,25 @@ func TestHandle_TenantResourceLabelNarrowsOnly(t *testing.T) {
|
||||
t.Fatalf("marshal object: %v", err)
|
||||
}
|
||||
|
||||
resp := w.Handle(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{
|
||||
req := admissionv1.AdmissionRequest{
|
||||
Kind: metav1.GroupVersionKind{Version: "v1", Kind: "Secret"},
|
||||
Namespace: "tenant-demo",
|
||||
Name: tt.secret,
|
||||
Operation: admissionv1.Create,
|
||||
Operation: tt.op,
|
||||
Object: runtime.RawExtension{Raw: raw},
|
||||
}})
|
||||
}
|
||||
if tt.op == admissionv1.Update {
|
||||
old := obj.DeepCopy()
|
||||
oldLabels := old.GetLabels()
|
||||
oldLabels[ManagedObjectKey] = "true"
|
||||
old.SetLabels(oldLabels)
|
||||
oldRaw, err := json.Marshal(old)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal old object: %v", err)
|
||||
}
|
||||
req.OldObject = runtime.RawExtension{Raw: oldRaw}
|
||||
}
|
||||
resp := w.Handle(context.Background(), admission.Request{AdmissionRequest: req})
|
||||
if !resp.Allowed {
|
||||
t.Fatalf("expected allowed response, got %+v", resp.Result)
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user