fix: validate only changed metadata on updates (#2118)

Signed-off-by: Oliver Baehler <oliver@sudo-i.net>
This commit is contained in:
Oliver Bähler
2026-09-03 13:54:31 +02:00
committed by GitHub
parent e5318792c6
commit e50bacffd7
5 changed files with 514 additions and 33 deletions
+287 -1
View File
@@ -5,6 +5,7 @@ package e2e
import (
"context"
"encoding/json"
"fmt"
"strings"
"time"
@@ -29,7 +30,14 @@ import (
)
var _ = Describe("enforcing generic metadata namespace rules", Ordered, Label("tenant", "rules", "enforce", "metadata", "generic"), func() {
const ownerName = "e2e-rules-metadata"
const (
ownerName = "e2e-rules-metadata"
secondOwnerName = "e2e-rules-metadata-second"
ownerLabel = "example.corp/label-a"
secondOwnerLabel = "example.corp/label-b"
ownerAnnotation = "example.corp/annotation-a"
secondOwnerAnnotation = "example.corp/annotation-b"
)
var (
tnt *capsulev1beta2.Tenant
@@ -112,6 +120,65 @@ var _ = Describe("enforcing generic metadata namespace rules", Ordered, Label("t
return rule
}
audienceRule := func(
audience rules.Audience,
rule *rules.NamespaceRuleBodyTenant,
) *rules.NamespaceRuleBodyTenant {
rule.Audience = []rules.Audience{audience}
return rule
}
isolatedMetadataRules := func() []*rules.NamespaceRuleBodyTenant {
return []*rules.NamespaceRuleBodyTenant{
audienceRule(
rules.Audience{
Kind: rules.AudienceKindCustom,
Name: string(rules.CustomAudienceCapsuleUser),
},
metadataRule(
rules.ActionTypeDeny,
"v1",
[]string{"ConfigMap"},
map[string]rules.MetadataValueRule{
".*": metadataValueRule(false, metadataByExpression(".*")),
},
map[string]rules.MetadataValueRule{
".*": metadataValueRule(false, metadataByExpression(".*")),
},
),
),
audienceRule(
rules.Audience{Kind: rules.AudienceKindUser, Name: ownerName},
metadataRule(
rules.ActionTypeAllow,
"v1",
[]string{"ConfigMap"},
map[string]rules.MetadataValueRule{
ownerLabel: metadataValueRule(false, metadataByExact("owner-a")),
},
map[string]rules.MetadataValueRule{
ownerAnnotation: metadataValueRule(false, metadataByExact("owner-a")),
},
),
),
audienceRule(
rules.Audience{Kind: rules.AudienceKindUser, Name: secondOwnerName},
metadataRule(
rules.ActionTypeAllow,
"v1",
[]string{"ConfigMap"},
map[string]rules.MetadataValueRule{
secondOwnerLabel: metadataValueRule(false, metadataByExact("owner-b")),
},
map[string]rules.MetadataValueRule{
secondOwnerAnnotation: metadataValueRule(false, metadataByExact("owner-b")),
},
),
),
}
}
baseTenantRules := func() []*rules.NamespaceRuleBodyTenant {
return []*rules.NamespaceRuleBodyTenant{
metadataRule(
@@ -190,6 +257,14 @@ var _ = Describe("enforcing generic metadata namespace rules", Ordered, Label("t
},
},
},
{
CoreOwnerSpec: rbac.CoreOwnerSpec{
UserSpec: rbac.UserSpec{
Name: secondOwnerName,
Kind: "User",
},
},
},
},
Rules: tenantRules,
},
@@ -574,6 +649,62 @@ var _ = Describe("enforcing generic metadata namespace rules", Ordered, Label("t
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
}
patchConfigMapMetadata := func(
cs kubernetes.Interface,
nsName string,
cmName string,
labels map[string]string,
annotations map[string]string,
) error {
metadataPatch := map[string]any{}
if labels != nil {
metadataPatch["labels"] = labels
}
if annotations != nil {
metadataPatch["annotations"] = annotations
}
patch, err := json.Marshal(map[string]any{"metadata": metadataPatch})
if err != nil {
return err
}
_, err = cs.CoreV1().ConfigMaps(nsName).Patch(
context.Background(),
cmName,
k8stypes.MergePatchType,
patch,
metav1.PatchOptions{},
)
return err
}
patchConfigMapMetadataAndExpectDenied := func(
cs kubernetes.Interface,
nsName string,
cmName string,
labels map[string]string,
annotations map[string]string,
substrings ...string,
) {
Eventually(func() error {
err := patchConfigMapMetadata(cs, nsName, cmName, labels, annotations)
if err == nil {
return fmt.Errorf("expected configmap metadata patch to be denied, but it succeeded")
}
message := err.Error()
for _, substring := range substrings {
if !strings.Contains(message, substring) {
return fmt.Errorf("expected error to contain %q, got: %s", substring, message)
}
}
return nil
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
}
createServiceAndExpectAllowed := func(cs kubernetes.Interface, nsName string, svc *corev1.Service) {
EventuallyCreation(func() error {
_, err := cs.CoreV1().Services(nsName).Create(context.Background(), svc, metav1.CreateOptions{})
@@ -719,6 +850,25 @@ var _ = Describe("enforcing generic metadata namespace rules", Ordered, Label("t
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
}
prepareIsolatedMetadataTest := func(name string) (*corev1.Namespace, kubernetes.Interface, kubernetes.Interface, *corev1.ConfigMap) {
updateTenantRules(isolatedMetadataRules())
ns := createNamespace(nil)
waitForProjectedMetadata(ns.Name, secondOwnerLabel, nil, nil)
owner := ownerClient(tnt.Spec.Owners[0].UserSpec)
secondOwner := ownerClient(tnt.Spec.Owners[1].UserSpec)
cm := configMap(
name,
map[string]string{ownerLabel: "owner-a"},
map[string]string{ownerAnnotation: "owner-a"},
)
createConfigMapAndExpectAllowed(owner, ns.Name, cm)
return ns, owner, secondOwner, cm
}
BeforeEach(func() {
tenantRules = baseTenantRules()
})
@@ -1358,6 +1508,142 @@ var _ = Describe("enforcing generic metadata namespace rules", Ordered, Label("t
)
})
It("allows an audience to patch its metadata while another audience's metadata is unchanged", func() {
ns, _, secondOwner, cm := prepareIsolatedMetadataTest("metadata-update-isolated-allowed")
By("patching only the label and annotation assigned to the second owner")
Eventually(func() error {
return patchConfigMapMetadata(
secondOwner,
ns.Name,
cm.Name,
map[string]string{secondOwnerLabel: "owner-b"},
map[string]string{secondOwnerAnnotation: "owner-b"},
)
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
By("preserving both audiences' metadata")
Eventually(func(g Gomega) {
current, err := secondOwner.CoreV1().ConfigMaps(ns.Name).Get(
context.Background(),
cm.Name,
metav1.GetOptions{},
)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(current.Labels).To(HaveKeyWithValue(ownerLabel, "owner-a"))
g.Expect(current.Labels).To(HaveKeyWithValue(secondOwnerLabel, "owner-b"))
g.Expect(current.Annotations).To(HaveKeyWithValue(ownerAnnotation, "owner-a"))
g.Expect(current.Annotations).To(HaveKeyWithValue(secondOwnerAnnotation, "owner-b"))
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
})
It("denies patches to metadata assigned to another audience", func() {
ns, _, secondOwner, cm := prepareIsolatedMetadataTest("metadata-update-other-audience-denied")
By("denying a change to the first owner's label")
patchConfigMapMetadataAndExpectDenied(
secondOwner,
ns.Name,
cm.Name,
map[string]string{ownerLabel: "tampered"},
nil,
ownerLabel,
"tampered",
"denied",
)
By("denying a change to the first owner's annotation")
patchConfigMapMetadataAndExpectDenied(
secondOwner,
ns.Name,
cm.Name,
nil,
map[string]string{ownerAnnotation: "tampered"},
ownerAnnotation,
"tampered",
"denied",
)
By("preserving the first owner's metadata")
Eventually(func(g Gomega) {
current, err := secondOwner.CoreV1().ConfigMaps(ns.Name).Get(
context.Background(),
cm.Name,
metav1.GetOptions{},
)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(current.Labels).To(HaveKeyWithValue(ownerLabel, "owner-a"))
g.Expect(current.Annotations).To(HaveKeyWithValue(ownerAnnotation, "owner-a"))
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
})
It("denies disallowed values for metadata assigned to the updating audience", func() {
ns, _, secondOwner, cm := prepareIsolatedMetadataTest("metadata-update-own-value-denied")
By("denying a disallowed value for the second owner's label")
patchConfigMapMetadataAndExpectDenied(
secondOwner,
ns.Name,
cm.Name,
map[string]string{secondOwnerLabel: "blocked"},
nil,
secondOwnerLabel,
"blocked",
"denied",
)
By("denying a disallowed value for the second owner's annotation")
patchConfigMapMetadataAndExpectDenied(
secondOwner,
ns.Name,
cm.Name,
nil,
map[string]string{secondOwnerAnnotation: "blocked"},
secondOwnerAnnotation,
"blocked",
"denied",
)
})
It("allows non-metadata updates when another audience's metadata is unchanged", func() {
ns, _, secondOwner, cm := prepareIsolatedMetadataTest("metadata-update-data-allowed")
By("updating ConfigMap data as the second owner")
Eventually(func() error {
current, err := secondOwner.CoreV1().ConfigMaps(ns.Name).Get(
context.Background(),
cm.Name,
metav1.GetOptions{},
)
if err != nil {
return err
}
current.Data["key"] = "updated"
_, err = secondOwner.CoreV1().ConfigMaps(ns.Name).Update(
context.Background(),
current,
metav1.UpdateOptions{},
)
return err
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
By("preserving the metadata and the data update")
Eventually(func(g Gomega) {
current, err := secondOwner.CoreV1().ConfigMaps(ns.Name).Get(
context.Background(),
cm.Name,
metav1.GetOptions{},
)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(current.Data).To(HaveKeyWithValue("key", "updated"))
g.Expect(current.Labels).To(HaveKeyWithValue(ownerLabel, "owner-a"))
g.Expect(current.Annotations).To(HaveKeyWithValue(ownerAnnotation, "owner-a"))
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
})
It("denies an update when a metadata value becomes invalid", func() {
updateTenantRules([]*rules.NamespaceRuleBodyTenant{
metadataRule(
@@ -45,9 +45,10 @@ func evaluateGenericRules[R any](
}
type genericRuleValidator func(
genericObject,
schema.GroupVersionKind,
[]*apirules.NamespaceRuleEnforceBody,
oldObj genericObject,
obj genericObject,
gvk schema.GroupVersionKind,
enforceBodies []*apirules.NamespaceRuleEnforceBody,
) (*ruleengine.Evaluation, error)
type genericRules struct {
@@ -94,7 +95,7 @@ func (h *genericRules) OnCreate(
enforceBodies := ruleengine.EnforceBodiesFromNamespaceRules(bodies)
if err := h.validateGenericRules(ctx, req, obj, gvk, tnt, recorder, enforceBodies); err != nil {
if err := h.validateGenericRules(ctx, req, nil, obj, gvk, tnt, recorder, enforceBodies); err != nil {
return ad.Deny(err.Error())
}
@@ -105,7 +106,7 @@ func (h *genericRules) OnCreate(
func (h *genericRules) OnUpdate(
_ client.Client,
_ client.Reader,
_ genericObject,
oldObj genericObject,
obj genericObject,
_ admission.Decoder,
recorder events.EventRecorder,
@@ -120,7 +121,7 @@ func (h *genericRules) OnUpdate(
enforceBodies := ruleengine.EnforceBodiesFromNamespaceRules(bodies)
if err := h.validateGenericRules(ctx, req, obj, gvk, tnt, recorder, enforceBodies); err != nil {
if err := h.validateGenericRules(ctx, req, oldObj, obj, gvk, tnt, recorder, enforceBodies); err != nil {
return ad.Deny(err.Error())
}
@@ -145,6 +146,7 @@ func (h *genericRules) OnDelete(
func (h *genericRules) validateGenericRules(
ctx context.Context,
req admission.Request,
oldObj genericObject,
obj genericObject,
gvk schema.GroupVersionKind,
tnt *capsulev1beta2.Tenant,
@@ -162,7 +164,7 @@ func (h *genericRules) validateGenericRules(
}
for _, evaluate := range h.rules {
evaluation, err := evaluate(obj, gvk, enforceBodies)
evaluation, err := evaluate(oldObj, obj, gvk, enforceBodies)
if err != nil {
return err
}
@@ -157,7 +157,7 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
t.Fatalf("validator must not be called")
return nil, nil
@@ -171,6 +171,7 @@ func TestValidateGenericRules(t *testing.T) {
context.Background(),
admission.Request{},
nil,
nil,
baseGVK,
testTenant(),
testEventRecorder{},
@@ -188,7 +189,7 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(obj genericObject, got schema.GroupVersionKind, _ []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(_ genericObject, obj genericObject, got schema.GroupVersionKind, _ []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
if got != baseGVK {
t.Fatalf("expected gvk %s, got %s", baseGVK.String(), got.String())
}
@@ -211,6 +212,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
obj,
baseGVK,
testTenant(),
@@ -244,6 +246,7 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(
genericObject,
genericObject,
schema.GroupVersionKind,
[]*apirules.NamespaceRuleEnforceBody,
@@ -263,6 +266,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
obj,
coreGVK("ConfigMap"),
&capsulev1beta2.Tenant{
@@ -293,7 +297,7 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
called = true
return nil, nil
@@ -306,6 +310,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
genericMetadataObject(map[string]string{
"managed-by": "human",
}, nil),
@@ -329,12 +334,12 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
calls = append(calls, "first")
return nil, nil
},
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
calls = append(calls, "second")
return &ruleengine.Evaluation{}, nil
@@ -347,6 +352,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
genericMetadataObject(nil, nil),
baseGVK,
testTenant(),
@@ -376,7 +382,7 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(_ genericObject, _ schema.GroupVersionKind, got []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(_ genericObject, _ genericObject, _ schema.GroupVersionKind, got []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
if len(got) != len(enforceBodies) {
t.Fatalf("expected %d enforce bodies, got %d", len(enforceBodies), len(got))
}
@@ -391,6 +397,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
genericMetadataObject(nil, nil),
baseGVK,
testTenant(),
@@ -410,10 +417,10 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
return nil, expected
},
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
secondCalled = true
return nil, nil
@@ -426,6 +433,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
genericMetadataObject(nil, nil),
baseGVK,
testTenant(),
@@ -447,7 +455,7 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
calls++
return &ruleengine.Evaluation{
@@ -465,7 +473,7 @@ func TestValidateGenericRules(t *testing.T) {
},
}, nil
},
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
calls++
return nil, nil
@@ -478,6 +486,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
genericMetadataObject(nil, nil),
baseGVK,
testTenant(),
@@ -499,7 +508,7 @@ func TestValidateGenericRules(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
return &ruleengine.Evaluation{
Blocking: &ruleengine.Decision{
SetName: "metadata label",
@@ -513,7 +522,7 @@ func TestValidateGenericRules(t *testing.T) {
},
}, nil
},
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
secondCalled = true
return nil, nil
@@ -526,6 +535,7 @@ func TestValidateGenericRules(t *testing.T) {
err := h.validateGenericRules(
context.Background(),
admission.Request{},
nil,
genericMetadataObject(nil, nil),
baseGVK,
testTenant(),
@@ -561,7 +571,7 @@ func TestGenericRulesOnCreate(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
called = true
return nil, nil
@@ -638,7 +648,7 @@ func TestGenericRulesOnCreate(t *testing.T) {
h := &genericRules{
rules: []genericRuleValidator{
func(genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
func(genericObject, genericObject, schema.GroupVersionKind, []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
return nil, errors.New("validator failed")
},
},
@@ -672,18 +682,18 @@ func TestGenericRulesOnCreate(t *testing.T) {
func TestGenericRulesOnUpdate(t *testing.T) {
t.Parallel()
t.Run("uses new object and allows when validators return nil", func(t *testing.T) {
t.Run("uses old and new objects and allows when validators return nil", func(t *testing.T) {
t.Parallel()
calledWithNewObject := false
calledWithObjects := false
oldObj := genericMetadataObject(map[string]string{"old": "true"}, nil)
newObj := genericMetadataObject(map[string]string{"new": "true"}, nil)
h := &genericRules{
rules: []genericRuleValidator{
func(obj genericObject, _ schema.GroupVersionKind, _ []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
calledWithNewObject = obj.GetLabels()["new"] == "true"
func(old, obj genericObject, _ schema.GroupVersionKind, _ []*apirules.NamespaceRuleEnforceBody) (*ruleengine.Evaluation, error) {
calledWithObjects = old.GetLabels()["old"] == "true" && obj.GetLabels()["new"] == "true"
return nil, nil
},
@@ -707,8 +717,8 @@ func TestGenericRulesOnUpdate(t *testing.T) {
if resp != nil {
t.Fatalf("expected nil response, got %#v", resp)
}
if !calledWithNewObject {
t.Fatalf("expected validator to receive the new object")
if !calledWithObjects {
t.Fatalf("expected validator to receive the old and new objects")
}
})
@@ -1041,6 +1051,7 @@ func TestGenericRulesWithRealMetadataValidatorSmoke(t *testing.T) {
h := GenericRules(cache.NewRegexCache()).(*genericRules)
evaluation, err := h.validateMetadata(
nil,
genericMetadataObject(map[string]string{"env": "prod"}, nil),
schema.GroupVersionKind{
Group: "",
@@ -33,6 +33,7 @@ type metadataEntry struct {
}
func (h *genericRules) validateMetadata(
oldObj genericObject,
obj genericObject,
gvk schema.GroupVersionKind,
enforceBodies []*apirules.NamespaceRuleEnforceBody,
@@ -41,7 +42,7 @@ func (h *genericRules) validateMetadata(
return nil, nil
}
entries, err := h.controlledMetadataEntries(obj, gvk, enforceBodies)
entries, err := h.controlledMetadataEntries(oldObj, obj, gvk, enforceBodies)
if err != nil {
return nil, err
}
@@ -178,6 +179,7 @@ func (h *genericRules) metadataSet(
//nolint:gocognit
func (h *genericRules) controlledMetadataEntries(
oldObj genericObject,
obj genericObject,
gvk schema.GroupVersionKind,
enforceBodies []*apirules.NamespaceRuleEnforceBody,
@@ -185,6 +187,15 @@ func (h *genericRules) controlledMetadataEntries(
labels := obj.GetLabels()
annotations := obj.GetAnnotations()
// Creation has no old object, so all present metadata is evaluated. During
// updates, unchanged entries still satisfy Required but skip value matching.
var oldLabels, oldAnnotations map[string]string
if oldObj != nil {
oldLabels = oldObj.GetLabels()
oldAnnotations = oldObj.GetAnnotations()
}
seen := make(map[string]metadataEntry)
for _, enforce := range enforceBodies {
@@ -224,7 +235,14 @@ func (h *genericRules) controlledMetadataEntries(
matchedAny = true
h.addMetadataEntry(seen, metadataFieldLabel, key, value, true, required)
h.addMetadataEntryIfChanged(
seen,
oldLabels,
metadataFieldLabel,
key,
value,
required,
)
}
if !matchedAny && required {
@@ -256,7 +274,14 @@ func (h *genericRules) controlledMetadataEntries(
matchedAny = true
h.addMetadataEntry(seen, metadataFieldAnnotation, key, value, true, required)
h.addMetadataEntryIfChanged(
seen,
oldAnnotations,
metadataFieldAnnotation,
key,
value,
required,
)
}
if !matchedAny && required {
@@ -282,6 +307,31 @@ func (h *genericRules) controlledMetadataEntries(
return out, nil
}
func metadataValueUnchanged(
oldMetadata map[string]string,
key string,
value string,
) bool {
oldValue, ok := oldMetadata[key]
return ok && oldValue == value
}
func (h *genericRules) addMetadataEntryIfChanged(
seen map[string]metadataEntry,
oldMetadata map[string]string,
field metadataField,
key string,
value string,
required bool,
) {
if metadataValueUnchanged(oldMetadata, key, value) {
return
}
h.addMetadataEntry(seen, field, key, value, true, required)
}
func (h *genericRules) matchesMetadataKey(selector, key string) (bool, error) {
return h.regexCache.MatchRegex(apirules.MetadataKeyExpression(selector), key)
}
@@ -356,7 +356,7 @@ func TestValidateMetadata(t *testing.T) {
h := newMetadataTestRules(nil, nil)
got, err := h.validateMetadata(tt.obj, tt.gvk, tt.enforceBodies)
got, err := h.validateMetadata(nil, tt.obj, tt.gvk, tt.enforceBodies)
if tt.wantMessage == "error" {
if err == nil {
t.Fatalf("expected error")
@@ -419,6 +419,137 @@ func TestValidateMetadata(t *testing.T) {
}
}
func TestValidateMetadataUpdate(t *testing.T) {
t.Parallel()
denyAll := enforceMetadata(
apirules.ActionTypeDeny,
[]string{"*"},
[]string{"ConfigMap"},
map[string]apirules.MetadataValueRule{
".*": metadataPolicy(false, expression(".*")),
},
map[string]apirules.MetadataValueRule{
".*": metadataPolicy(false, expression(".*")),
},
)
allowB := enforceMetadata(
apirules.ActionTypeAllow,
[]string{"*"},
[]string{"ConfigMap"},
map[string]apirules.MetadataValueRule{
"label-b": metadataPolicy(false, expression(".*")),
},
map[string]apirules.MetadataValueRule{
"annotation-b": metadataPolicy(false, expression(".*")),
},
)
requireEnvironment := enforceMetadata(
apirules.ActionTypeAllow,
[]string{"*"},
[]string{"ConfigMap"},
map[string]apirules.MetadataValueRule{
"env": metadataPolicy(true, exact("prod")),
},
nil,
)
tests := []struct {
name string
oldObj genericObject
obj genericObject
enforceBodies []*apirules.NamespaceRuleEnforceBody
wantNil bool
wantBlocking bool
wantPath string
}{
{
name: "allows changed metadata authorized for the caller while ignoring unchanged metadata",
oldObj: metadataObject(
map[string]string{"label-a": "old"},
map[string]string{"annotation-a": "old"},
),
obj: metadataObject(
map[string]string{"label-a": "old", "label-b": "new"},
map[string]string{"annotation-a": "old", "annotation-b": "new"},
),
enforceBodies: []*apirules.NamespaceRuleEnforceBody{denyAll, allowB},
},
{
name: "denies a changed label not authorized for the caller",
oldObj: metadataObject(map[string]string{"label-a": "old"}, nil),
obj: metadataObject(map[string]string{"label-a": "new"}, nil),
enforceBodies: []*apirules.NamespaceRuleEnforceBody{denyAll, allowB},
wantBlocking: true,
wantPath: `metadata.labels["label-a"]`,
},
{
name: "ignores an unchanged required value",
oldObj: metadataObject(map[string]string{"env": "legacy"}, nil),
obj: metadataObject(map[string]string{"env": "legacy", "other": "new"}, nil),
enforceBodies: []*apirules.NamespaceRuleEnforceBody{requireEnvironment},
wantNil: true,
},
{
name: "denies a changed required value that is invalid",
oldObj: metadataObject(map[string]string{"env": "prod"}, nil),
obj: metadataObject(map[string]string{"env": "stage"}, nil),
enforceBodies: []*apirules.NamespaceRuleEnforceBody{requireEnvironment},
wantBlocking: true,
wantPath: `metadata.labels["env"]`,
},
{
name: "denies removal of required metadata",
oldObj: metadataObject(map[string]string{"env": "prod"}, nil),
obj: metadataObject(nil, nil),
enforceBodies: []*apirules.NamespaceRuleEnforceBody{requireEnvironment},
wantBlocking: true,
wantPath: `metadata.labels["env"]`,
},
{
name: "ignores removal of optional denied metadata",
oldObj: metadataObject(map[string]string{"label-a": "old"}, nil),
obj: metadataObject(nil, nil),
enforceBodies: []*apirules.NamespaceRuleEnforceBody{denyAll},
wantNil: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
h := newMetadataTestRules(nil, nil)
got, err := h.validateMetadata(tt.oldObj, tt.obj, coreGVK("ConfigMap"), tt.enforceBodies)
if err != nil {
t.Fatalf("expected no error, got %v", err)
}
if tt.wantNil {
if got != nil {
t.Fatalf("expected nil evaluation, got %#v", got)
}
return
}
if got == nil {
t.Fatal("expected evaluation")
}
blockingErr := got.BlockingError()
if tt.wantBlocking && blockingErr == nil {
t.Fatal("expected blocking error")
}
if !tt.wantBlocking && blockingErr != nil {
t.Fatalf("expected no blocking error, got %v", blockingErr)
}
if tt.wantPath != "" && got.Blocking.Value.Path != tt.wantPath {
t.Fatalf("expected path %q, got %q", tt.wantPath, got.Blocking.Value.Path)
}
})
}
}
func TestControlledMetadataEntriesMatchesKeyPatternsWithRegexCache(t *testing.T) {
t.Parallel()
@@ -439,6 +570,7 @@ func TestControlledMetadataEntriesMatchesKeyPatternsWithRegexCache(t *testing.T)
}}
entries, err := h.controlledMetadataEntries(
nil,
obj,
schema.GroupVersionKind{Version: "v1", Kind: "Namespace"},
enforce,
@@ -966,7 +1098,7 @@ func TestControlledMetadataEntries(t *testing.T) {
h := newMetadataTestRules(tt.managedLabels, tt.managedAnnotations)
got, err := h.controlledMetadataEntries(tt.obj, tt.gvk, tt.enforceBodies)
got, err := h.controlledMetadataEntries(nil, tt.obj, tt.gvk, tt.enforceBodies)
if err != nil {
t.Fatalf("controlledMetadataEntries() error = %v", err)
}