From 4c4d927bea321f411f85c09b23ae713c53936481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20B=C3=A4hler?= <26610571+oliverbaehler@users.noreply.github.com> Date: Fri, 5 Jun 2026 14:46:37 +0200 Subject: [PATCH] fix: use different match strategy for truthy and match (#1953) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(controller): decode old object for delete requests Signed-off-by: Oliver Bähler * chore: modernize golang Signed-off-by: Oliver Bähler * chore: modernize golang Signed-off-by: Oliver Bähler * chore: modernize golang Signed-off-by: Oliver Bähler * fix: preserve ca-bundles injected from external providers Signed-off-by: Oliver Baehler * fix: best effort patch reconciling status Signed-off-by: Oliver Baehler * fix: use different match strategy for truthy and match Signed-off-by: Oliver Baehler --------- Signed-off-by: Oliver Bähler Signed-off-by: Oliver Baehler --- .../capsule/example-setup/custom-quotas.yaml | 17 + internal/controllers/customquotas/utils.go | 34 +- pkg/runtime/jsonpath/truth.go | 112 ++++ pkg/runtime/jsonpath/truth_test.go | 590 ++++++++++++++++++ pkg/runtime/selectors/fields.go | 17 +- pkg/utils/compiled_target.go | 51 ++ 6 files changed, 814 insertions(+), 7 deletions(-) create mode 100644 pkg/runtime/jsonpath/truth_test.go create mode 100644 pkg/utils/compiled_target.go diff --git a/hack/distro/capsule/example-setup/custom-quotas.yaml b/hack/distro/capsule/example-setup/custom-quotas.yaml index 2819df86..7ccc77d0 100644 --- a/hack/distro/capsule/example-setup/custom-quotas.yaml +++ b/hack/distro/capsule/example-setup/custom-quotas.yaml @@ -1,6 +1,23 @@ --- apiVersion: capsule.clastix.io/v1beta2 kind: GlobalCustomQuota +metadata: + name: service-aggregate +spec: + limit: 5 + namespaceSelectors: + - matchLabels: + capsule.clastix.io/tenant: wind + sources: + - apiVersion: v1 + kind: Service + op: count + selectors: + - fieldSelectors: + - .spec.type=="ClusterIP" +--- +apiVersion: capsule.clastix.io/v1beta2 +kind: GlobalCustomQuota metadata: name: storage-aggregate spec: diff --git a/internal/controllers/customquotas/utils.go b/internal/controllers/customquotas/utils.go index 3a985846..5cf85e38 100644 --- a/internal/controllers/customquotas/utils.go +++ b/internal/controllers/customquotas/utils.go @@ -28,6 +28,7 @@ import ( "github.com/projectcapsule/capsule/pkg/runtime/jsonpath" "github.com/projectcapsule/capsule/pkg/runtime/quota" "github.com/projectcapsule/capsule/pkg/runtime/selectors" + "github.com/projectcapsule/capsule/pkg/utils" ) const immediatePendingDeleteRequeue = 500 * time.Millisecond @@ -112,7 +113,7 @@ func MatchesCompiledSelectorsWithFields( allFieldsMatch := true for _, matcher := range sel.FieldMatchers { - ok, err := jsonpath.EvaluateTruthyFromCompiled(u, matcher) + ok, err := evaluateCompiledFieldSelector(u, matcher) if err != nil { return false, err } @@ -132,6 +133,27 @@ func MatchesCompiledSelectorsWithFields( return false, nil } +func evaluateCompiledFieldSelector( + u unstructured.Unstructured, + matcher selectors.CompiledFieldSelector, +) (bool, error) { + switch matcher.Operator { + case selectors.FieldSelectorTruthy: + return jsonpath.EvaluateTruthyFromCompiled(u, matcher.Compiled) + + case selectors.FieldSelectorEquals: + actual, err := matcher.Compiled.Execute(u) + if err != nil { + return false, err + } + + return strings.TrimSpace(actual) == matcher.Value, nil + + default: + return false, fmt.Errorf("unsupported field selector operator %q", matcher.Operator) + } +} + func MakeCustomQuotaCacheKey(namespace, name string) string { return namespace + "/" + name } @@ -162,15 +184,15 @@ func CompileSelectorsWithFields( lblSel = compiled } - fieldMatchers := make([]*jsonpath.CompiledJSONPath, 0, len(selector.FieldSelectors)) + fieldMatchers := make([]selectors.CompiledFieldSelector, 0, len(selector.FieldSelectors)) - for _, path := range selector.FieldSelectors { - compiledPath, err := cache.GetOrCompile(path) + for _, raw := range selector.FieldSelectors { + compiledSelector, err := utils.CompileFieldSelector(cache, raw) if err != nil { - return nil, fmt.Errorf("compile field selector path %q: %w", path, err) + return nil, fmt.Errorf("compile field selector %q: %w", raw, err) } - fieldMatchers = append(fieldMatchers, compiledPath) + fieldMatchers = append(fieldMatchers, compiledSelector) } out = append(out, selectors.CompiledSelectorWithFields{ diff --git a/pkg/runtime/jsonpath/truth.go b/pkg/runtime/jsonpath/truth.go index 36de0b8b..90663d69 100644 --- a/pkg/runtime/jsonpath/truth.go +++ b/pkg/runtime/jsonpath/truth.go @@ -39,3 +39,115 @@ func EvaluateTruthyFromCompiled(u unstructured.Unstructured, compiled *CompiledJ return true, nil } } + +func SplitFieldSelectorEquals(raw string) (path string, value string, ok bool) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", "", false + } + + idx, width := findTopLevelEquals(raw) + if idx < 0 { + return "", "", false + } + + path = strings.TrimSpace(raw[:idx]) + value = strings.TrimSpace(raw[idx+width:]) + + if path == "" || value == "" { + return "", "", false + } + + value = trimMatchingQuotes(value) + + return path, value, true +} + +func findTopLevelEquals(raw string) (idx int, width int) { + var ( + bracketDepth int + braceDepth int + parenDepth int + quote byte + escaped bool + ) + + for i := range len(raw) { + ch := raw[i] + + if escaped { + escaped = false + + continue + } + + if quote != 0 { + switch ch { + case '\\': + escaped = true + case quote: + quote = 0 + } + + continue + } + + switch ch { + case '\'', '"': + quote = ch + + case '[': + bracketDepth++ + case ']': + if bracketDepth > 0 { + bracketDepth-- + } + + case '{': + braceDepth++ + case '}': + if braceDepth > 0 { + braceDepth-- + } + + case '(': + parenDepth++ + case ')': + if parenDepth > 0 { + parenDepth-- + } + + case '=': + if bracketDepth != 0 || braceDepth != 0 || parenDepth != 0 { + continue + } + + if i+1 < len(raw) && raw[i+1] == '=' { + return i, 2 + } + + return i, 1 + } + } + + return -1, 0 +} + +func trimMatchingQuotes(value string) string { + if len(value) < 2 { + return value + } + + first := value[0] + last := value[len(value)-1] + + if first != last { + return value + } + + if first != '"' && first != '\'' { + return value + } + + return strings.TrimSpace(value[1 : len(value)-1]) +} diff --git a/pkg/runtime/jsonpath/truth_test.go b/pkg/runtime/jsonpath/truth_test.go new file mode 100644 index 00000000..4de27046 --- /dev/null +++ b/pkg/runtime/jsonpath/truth_test.go @@ -0,0 +1,590 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package jsonpath + +import ( + "strings" + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestEvaluateTruthyFromCompiled(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + object unstructured.Unstructured + want bool + wantErr bool + }{ + { + name: "empty missing path is false", + path: ".missing", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "type": "ClusterIP", + }, + }), + want: false, + }, + { + name: "empty string is false", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": "", + }, + }), + want: false, + }, + { + name: "whitespace string is false", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": " ", + }, + }), + want: false, + }, + { + name: "false string is false", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": "false", + }, + }), + want: false, + }, + { + name: "false string case insensitive is false", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": "FALSE", + }, + }), + want: false, + }, + { + name: "false string with whitespace is false", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": " false ", + }, + }), + want: false, + }, + { + name: "zero string is false", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": "0", + }, + }), + want: false, + }, + { + name: "true string is true", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": "true", + }, + }), + want: true, + }, + { + name: "non-empty scalar string is true", + path: ".spec.type", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "type": "ClusterIP", + }, + }), + want: true, + }, + { + name: "one string is true", + path: ".spec.value", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "value": "1", + }, + }), + want: true, + }, + { + name: "jsonpath filter match is true", + path: ".spec.accessModes[?(@==\"ReadWriteOnce\")]", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "accessModes": []any{ + "ReadWriteOnce", + "ReadOnlyMany", + }, + }, + }), + want: true, + }, + { + name: "jsonpath filter no match is false", + path: ".spec.accessModes[?(@==\"ReadWriteMany\")]", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "accessModes": []any{ + "ReadWriteOnce", + "ReadOnlyMany", + }, + }, + }), + want: false, + }, + { + name: "invalid jsonpath execution returns error", + path: ".spec.accessModes[999]", + object: newUnstructured(map[string]any{ + "spec": map[string]any{ + "accessModes": []any{ + "ReadWriteOnce", + }, + }, + }), + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + compiled, err := CompileJSONPath(tt.path) + if err != nil { + t.Fatalf("expected jsonpath %q to compile, got %v", tt.path, err) + } + + got, err := EvaluateTruthyFromCompiled(tt.object, compiled) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + + return + } + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if got != tt.want { + t.Fatalf("expected %t, got %t", tt.want, got) + } + }) + } +} + +func TestEvaluateTruthyFromCompiledRejectsNilCompiledJSONPath(t *testing.T) { + t.Parallel() + + got, err := EvaluateTruthyFromCompiled(newUnstructured(nil), nil) + if err == nil { + t.Fatal("expected error, got nil") + } + + if got { + t.Fatal("expected false result for nil compiled jsonpath") + } + + if !strings.Contains(err.Error(), "compiled jsonpath is nil") { + t.Fatalf("expected nil compiled jsonpath error, got %v", err) + } +} + +func TestSplitFieldSelectorEquals(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + wantPath string + wantValue string + wantOK bool + }{ + { + name: "single equals", + raw: `.spec.type=ClusterIP`, + wantPath: `.spec.type`, + wantValue: `ClusterIP`, + wantOK: true, + }, + { + name: "double equals", + raw: `.spec.type==ClusterIP`, + wantPath: `.spec.type`, + wantValue: `ClusterIP`, + wantOK: true, + }, + { + name: "double equals with quoted value", + raw: `.spec.type=="ClusterIP"`, + wantPath: `.spec.type`, + wantValue: `ClusterIP`, + wantOK: true, + }, + { + name: "double equals with single quoted value", + raw: `.spec.type=='ClusterIP'`, + wantPath: `.spec.type`, + wantValue: `ClusterIP`, + wantOK: true, + }, + { + name: "trims whitespace around expression", + raw: ` .spec.type == "ClusterIP" `, + wantPath: `.spec.type`, + wantValue: `ClusterIP`, + wantOK: true, + }, + { + name: "keeps spaces inside quoted value", + raw: `.metadata.annotations["example.com/value"]=="hello world"`, + wantPath: `.metadata.annotations["example.com/value"]`, + wantValue: `hello world`, + wantOK: true, + }, + { + name: "quoted value containing equals", + raw: `.metadata.annotations["example.com/check"]=="a=b"`, + wantPath: `.metadata.annotations["example.com/check"]`, + wantValue: `a=b`, + wantOK: true, + }, + { + name: "single quoted value containing equals", + raw: `.metadata.annotations["example.com/check"]=='a=b'`, + wantPath: `.metadata.annotations["example.com/check"]`, + wantValue: `a=b`, + wantOK: true, + }, + { + name: "top level equality after bracket expression", + raw: `.metadata.labels["app.kubernetes.io/name"]=="nginx"`, + wantPath: `.metadata.labels["app.kubernetes.io/name"]`, + wantValue: `nginx`, + wantOK: true, + }, + { + name: "jsonpath filter equality is not top level equality", + raw: `.spec.accessModes[?(@=="ReadWriteOnce")]`, + wantOK: false, + }, + { + name: "jsonpath filter single equals is not top level equality", + raw: `.spec.accessModes[?(@="ReadWriteOnce")]`, + wantOK: false, + }, + { + name: "truthy path without equals", + raw: `.spec.storageClassName`, + wantOK: false, + }, + { + name: "empty string", + raw: ``, + wantOK: false, + }, + { + name: "whitespace string", + raw: ` `, + wantOK: false, + }, + { + name: "missing path", + raw: `=ClusterIP`, + wantOK: false, + }, + { + name: "missing value", + raw: `.spec.type=`, + wantOK: false, + }, + { + name: "missing value with double equals", + raw: `.spec.type==`, + wantOK: false, + }, + { + name: "does not split equals inside double quoted path segment", + raw: `.metadata.annotations["example.com/a=b"]=="value"`, + wantPath: `.metadata.annotations["example.com/a=b"]`, + wantValue: `value`, + wantOK: true, + }, + { + name: "does not split equals inside single quoted path segment", + raw: `.metadata.annotations['example.com/a=b']=="value"`, + wantPath: `.metadata.annotations['example.com/a=b']`, + wantValue: `value`, + wantOK: true, + }, + { + name: "does not split equals inside parentheses", + raw: `.spec.values(@=="ignored")=="value"`, + wantPath: `.spec.values(@=="ignored")`, + wantValue: `value`, + wantOK: true, + }, + { + name: "does not split equals inside braces", + raw: `.spec.values{"a==b"}=="value"`, + wantPath: `.spec.values{"a==b"}`, + wantValue: `value`, + wantOK: true, + }, + { + name: "escaped quote inside quoted value", + raw: `.metadata.annotations["example.com/check"]=="a\"=b"`, + wantPath: `.metadata.annotations["example.com/check"]`, + wantValue: `a\"=b`, + wantOK: true, + }, + { + name: "unmatched quote after equality remains part of value", + raw: `.spec.type=="ClusterIP`, + wantPath: `.spec.type`, + wantValue: `"ClusterIP`, + wantOK: true, + }, + { + name: "unmatched quote before equality suppresses split", + raw: `.metadata.annotations["broken]==value`, + wantOK: false, + }, + { + name: "unmatched bracket suppresses split", + raw: `.metadata.annotations["key"=="value"`, + wantOK: false, + }, + { + name: "top level equality before later malformed content still splits", + raw: `.spec.type=ClusterIP[?(@=="ignored")]`, + wantPath: `.spec.type`, + wantValue: `ClusterIP[?(@=="ignored")]`, + wantOK: true, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotPath, gotValue, gotOK := SplitFieldSelectorEquals(tt.raw) + if gotOK != tt.wantOK { + t.Fatalf("expected ok=%t, got %t", tt.wantOK, gotOK) + } + + if gotPath != tt.wantPath { + t.Fatalf("expected path %q, got %q", tt.wantPath, gotPath) + } + + if gotValue != tt.wantValue { + t.Fatalf("expected value %q, got %q", tt.wantValue, gotValue) + } + }) + } +} + +func TestFindTopLevelEquals(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + wantIdx int + wantWidth int + }{ + { + name: "single equals", + raw: `.spec.type=ClusterIP`, + wantIdx: len(`.spec.type`), + wantWidth: 1, + }, + { + name: "double equals", + raw: `.spec.type==ClusterIP`, + wantIdx: len(`.spec.type`), + wantWidth: 2, + }, + { + name: "ignores equals inside brackets", + raw: `.spec.accessModes[?(@=="ReadWriteOnce")]`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "finds top level equals after brackets", + raw: `.metadata.labels["app.kubernetes.io/name"]=="nginx"`, + wantIdx: len(`.metadata.labels["app.kubernetes.io/name"]`), + wantWidth: 2, + }, + { + name: "ignores equals inside double quotes", + raw: `.metadata.annotations["a=b"]`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores equals inside single quotes", + raw: `.metadata.annotations['a=b']`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores escaped quote before equals inside quoted string", + raw: `.metadata.annotations["a\"=b"]`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores equals inside parentheses", + raw: `.spec.values(@=="ignored")`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores equals inside braces", + raw: `.spec.values{"a==b"}`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "finds first top level equals", + raw: `.spec.type=ClusterIP=ignored`, + wantIdx: len(`.spec.type`), + wantWidth: 1, + }, + { + name: "empty string", + raw: ``, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "no equals", + raw: `.spec.type`, + wantIdx: -1, + wantWidth: 0, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotIdx, gotWidth := findTopLevelEquals(tt.raw) + if gotIdx != tt.wantIdx { + t.Fatalf("expected idx %d, got %d", tt.wantIdx, gotIdx) + } + + if gotWidth != tt.wantWidth { + t.Fatalf("expected width %d, got %d", tt.wantWidth, gotWidth) + } + }) + } +} + +func TestTrimMatchingQuotes(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value string + want string + }{ + { + name: "empty", + value: "", + want: "", + }, + { + name: "single character", + value: `"`, + want: `"`, + }, + { + name: "double quoted", + value: `"ClusterIP"`, + want: "ClusterIP", + }, + { + name: "single quoted", + value: `'ClusterIP'`, + want: "ClusterIP", + }, + { + name: "trims inner whitespace", + value: `" ClusterIP "`, + want: "ClusterIP", + }, + { + name: "unmatched starting quote", + value: `"ClusterIP`, + want: `"ClusterIP`, + }, + { + name: "unmatched ending quote", + value: `ClusterIP"`, + want: `ClusterIP"`, + }, + { + name: "different quote types", + value: `"ClusterIP'`, + want: `"ClusterIP'`, + }, + { + name: "unquoted", + value: `ClusterIP`, + want: `ClusterIP`, + }, + { + name: "quoted value containing equals", + value: `"a=b"`, + want: `a=b`, + }, + { + name: "quoted value containing escaped quote", + value: `"a\"=b"`, + want: `a\"=b`, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if got := trimMatchingQuotes(tt.value); got != tt.want { + t.Fatalf("expected %q, got %q", tt.want, got) + } + }) + } +} + +func newUnstructured(object map[string]any) unstructured.Unstructured { + return unstructured.Unstructured{ + Object: object, + } +} diff --git a/pkg/runtime/selectors/fields.go b/pkg/runtime/selectors/fields.go index 0dc78bef..fab68612 100644 --- a/pkg/runtime/selectors/fields.go +++ b/pkg/runtime/selectors/fields.go @@ -10,6 +10,13 @@ import ( "github.com/projectcapsule/capsule/pkg/runtime/jsonpath" ) +type FieldSelectorOperator string + +const ( + FieldSelectorTruthy FieldSelectorOperator = "truthy" + FieldSelectorEquals FieldSelectorOperator = "equals" +) + // +kubebuilder:object:generate=true type SelectorWithFields struct { // Select Items based on their labels. @@ -23,5 +30,13 @@ type SelectorWithFields struct { type CompiledSelectorWithFields struct { LabelSelector labels.Selector - FieldMatchers []*jsonpath.CompiledJSONPath + FieldMatchers []CompiledFieldSelector +} + +type CompiledFieldSelector struct { + Raw string + Path string + Operator FieldSelectorOperator + Value string + Compiled *jsonpath.CompiledJSONPath } diff --git a/pkg/utils/compiled_target.go b/pkg/utils/compiled_target.go new file mode 100644 index 00000000..84a6d178 --- /dev/null +++ b/pkg/utils/compiled_target.go @@ -0,0 +1,51 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "fmt" + "strings" + + "github.com/projectcapsule/capsule/internal/cache" + "github.com/projectcapsule/capsule/pkg/runtime/jsonpath" + "github.com/projectcapsule/capsule/pkg/runtime/selectors" +) + +func CompileFieldSelector( + cache *cache.JSONPathCache, + raw string, +) (selectors.CompiledFieldSelector, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return selectors.CompiledFieldSelector{}, fmt.Errorf("field selector must not be empty") + } + + path, value, ok := jsonpath.SplitFieldSelectorEquals(raw) + if !ok { + compiledPath, err := cache.GetOrCompile(raw) + if err != nil { + return selectors.CompiledFieldSelector{}, err + } + + return selectors.CompiledFieldSelector{ + Raw: raw, + Path: raw, + Operator: selectors.FieldSelectorTruthy, + Compiled: compiledPath, + }, nil + } + + compiledPath, err := cache.GetOrCompile(path) + if err != nil { + return selectors.CompiledFieldSelector{}, err + } + + return selectors.CompiledFieldSelector{ + Raw: raw, + Path: path, + Operator: selectors.FieldSelectorEquals, + Value: value, + Compiled: compiledPath, + }, nil +}