diff --git a/e2e/customquota_global_test.go b/e2e/customquota_global_test.go index 38cd86c3..742e8e0a 100644 --- a/e2e/customquota_global_test.go +++ b/e2e/customquota_global_test.go @@ -19,6 +19,7 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" "github.com/projectcapsule/capsule/pkg/api/meta" @@ -3365,4 +3366,280 @@ var _ = Describe("when GlobalCustomQuota uses ledger-backed reconciliation", Ord expectGlobalQuotaUsedAndClaims(ctx, cpuQuota.GetName(), "200m", 2) expectGlobalQuotaUsedAndClaims(ctx, emptyDirQuota.GetName(), "2Gi", 2) }) + + It("excludes succeeded pods from cpu limit quota using not-equals field selector", func() { + q := &capsulev1beta2.GlobalCustomQuota{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gq-not-equals-succeeded", + Labels: map[string]string{ + "e2e.capsule.dev/test-suite": "globalcustomquota-ledger", + }, + }, + Spec: capsulev1beta2.GlobalCustomQuotaSpec{ + CustomQuotaSpec: capsulev1beta2.CustomQuotaSpec{ + Limit: resource.MustParse("2"), + Sources: []capsulev1beta2.CustomQuotaSpecSource{ + { + VersionKind: runtime.VersionKind{ + APIVersion: "v1", + Kind: "Pod", + }, + CustomQuotaSpecSourceConfig: capsulev1beta2.CustomQuotaSpecSourceConfig{ + Operation: quota.OpAdd, + Path: ".spec.containers[*].resources.limits.cpu", + Selectors: []selectors.SelectorWithFields{ + { + FieldSelectors: []string{ + ".status.phase!=Succeeded", + ".status.phase!=Failed", + ".status.phase!=Unknown", + }, + }, + }, + }, + }, + }, + }, + NamespaceSelectors: []selectors.NamespaceSelector{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + tenantLabel: tenantValue, + }, + }, + }, + }, + }, + } + + EventuallyCreation(func() error { + return k8sClient.Create(ctx, q) + }).Should(Succeed()) + awaitGlobalQuotaReady(ctx, q.GetName()) + + // Running pod with 1 CPU limit — should be counted. + runningPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ne-running", + Namespace: testNamespace, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "main", + Image: "nginx:1.27.0", + SecurityContext: restrictedContainerSecurityContext(), + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + }, + }, + }, + RestartPolicy: corev1.RestartPolicyAlways, + }, + } + EventuallyCreation(func() error { + runningPod.ResourceVersion = "" + return k8sClient.Create(ctx, runningPod) + }).Should(Succeed()) + expectGlobalQuotaUsedAndClaims(ctx, q.GetName(), "1", 1) + + // Succeeded pod with 1 CPU limit — must be excluded after completion. + succeededPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ne-succeeded", + Namespace: testNamespace, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "main", + Image: "busybox:1.36", + Command: []string{"sh", "-c", "exit 0"}, + SecurityContext: restrictedContainerSecurityContext(), + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + }, + }, + }, + RestartPolicy: corev1.RestartPolicyNever, + TerminationGracePeriodSeconds: ptr.To(int64(0)), + }, + } + EventuallyCreation(func() error { + succeededPod.ResourceVersion = "" + return k8sClient.Create(ctx, succeededPod) + }).Should(Succeed()) + + // Wait for the pod to complete. + Eventually(func(g Gomega) { + obj := &corev1.Pod{} + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: succeededPod.Name, + Namespace: testNamespace, + }, obj)).To(Succeed()) + g.Expect(obj.Status.Phase).To(Equal(corev1.PodSucceeded)) + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + + // Succeeded pod must be excluded; only the running pod should count. + expectGlobalQuotaUsedAndClaims(ctx, q.GetName(), "1", 1) + expectLedgerSettled(ctx, ControllerNamespace, q.GetName()) + + // Admission: 1 existing CPU + 2 requested > limit 2 — must be denied. + expectPodCreationDeniedContaining(func(name string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: testNamespace}, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "main", + Image: "nginx:1.27.0", + SecurityContext: restrictedContainerSecurityContext(), + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + }, + }, + }, + RestartPolicy: corev1.RestartPolicyAlways, + }, + } + }, q.GetName()) + }) + + It("excludes failed pods from cpu limit quota using not-equals field selector", func() { + q := &capsulev1beta2.GlobalCustomQuota{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gq-not-equals-failed", + Labels: map[string]string{ + "e2e.capsule.dev/test-suite": "globalcustomquota-ledger", + }, + }, + Spec: capsulev1beta2.GlobalCustomQuotaSpec{ + CustomQuotaSpec: capsulev1beta2.CustomQuotaSpec{ + Limit: resource.MustParse("2"), + Sources: []capsulev1beta2.CustomQuotaSpecSource{ + { + VersionKind: runtime.VersionKind{ + APIVersion: "v1", + Kind: "Pod", + }, + CustomQuotaSpecSourceConfig: capsulev1beta2.CustomQuotaSpecSourceConfig{ + Operation: quota.OpAdd, + Path: ".spec.containers[*].resources.limits.cpu", + Selectors: []selectors.SelectorWithFields{ + { + FieldSelectors: []string{ + ".status.phase!=Succeeded", + ".status.phase!=Failed", + ".status.phase!=Unknown", + }, + }, + }, + }, + }, + }, + }, + NamespaceSelectors: []selectors.NamespaceSelector{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + tenantLabel: tenantValue, + }, + }, + }, + }, + }, + } + + EventuallyCreation(func() error { + return k8sClient.Create(ctx, q) + }).Should(Succeed()) + awaitGlobalQuotaReady(ctx, q.GetName()) + + // Running pod with 1 CPU limit — should be counted. + runningPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ne-failed-running", + Namespace: testNamespace, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "main", + Image: "nginx:1.27.0", + SecurityContext: restrictedContainerSecurityContext(), + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + }, + }, + }, + RestartPolicy: corev1.RestartPolicyAlways, + }, + } + EventuallyCreation(func() error { + runningPod.ResourceVersion = "" + return k8sClient.Create(ctx, runningPod) + }).Should(Succeed()) + expectGlobalQuotaUsedAndClaims(ctx, q.GetName(), "1", 1) + + // Failed pod with 1 CPU limit — must be excluded after failure. + failedPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ne-failed-terminal", + Namespace: testNamespace, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "main", + Image: "busybox:1.36", + Command: []string{"sh", "-c", "exit 1"}, + SecurityContext: restrictedContainerSecurityContext(), + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + }, + }, + }, + RestartPolicy: corev1.RestartPolicyNever, + TerminationGracePeriodSeconds: ptr.To(int64(0)), + }, + } + EventuallyCreation(func() error { + failedPod.ResourceVersion = "" + return k8sClient.Create(ctx, failedPod) + }).Should(Succeed()) + + // Wait for the pod to fail. + Eventually(func(g Gomega) { + obj := &corev1.Pod{} + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: failedPod.Name, + Namespace: testNamespace, + }, obj)).To(Succeed()) + g.Expect(obj.Status.Phase).To(Equal(corev1.PodFailed)) + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + + // Failed pod must be excluded; only the running pod should count. + expectGlobalQuotaUsedAndClaims(ctx, q.GetName(), "1", 1) + expectLedgerSettled(ctx, ControllerNamespace, q.GetName()) + }) }) diff --git a/internal/controllers/customquotas/utils.go b/internal/controllers/customquotas/utils.go index ba80da9f..1d58b54d 100644 --- a/internal/controllers/customquotas/utils.go +++ b/internal/controllers/customquotas/utils.go @@ -157,6 +157,14 @@ func evaluateCompiledFieldSelector( return strings.TrimSpace(actual) == matcher.Value, nil + case selectors.FieldSelectorNotEquals: + 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) } diff --git a/pkg/runtime/jsonpath/truth.go b/pkg/runtime/jsonpath/truth.go index 90663d69..53ccf908 100644 --- a/pkg/runtime/jsonpath/truth.go +++ b/pkg/runtime/jsonpath/truth.go @@ -133,6 +133,101 @@ func findTopLevelEquals(raw string) (idx int, width int) { return -1, 0 } +// SplitFieldSelectorNotEquals parses a raw field selector of the form "path!=value" +// and returns the path, value, and whether the not-equal operator was found at the top level. +func SplitFieldSelectorNotEquals(raw string) (path string, value string, ok bool) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", "", false + } + + idx, width := findTopLevelNotEquals(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 +} + +// findTopLevelNotEquals finds the index and width of a top-level "!=" operator, +// skipping any operators nested inside brackets, braces, parentheses or quotes. +func findTopLevelNotEquals(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 -1, 0 +} + func trimMatchingQuotes(value string) string { if len(value) < 2 { return value diff --git a/pkg/runtime/jsonpath/truth_test.go b/pkg/runtime/jsonpath/truth_test.go index 4de27046..a5715b2d 100644 --- a/pkg/runtime/jsonpath/truth_test.go +++ b/pkg/runtime/jsonpath/truth_test.go @@ -583,6 +583,283 @@ func TestTrimMatchingQuotes(t *testing.T) { } } +func TestSplitFieldSelectorNotEquals(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + wantPath string + wantValue string + wantOK bool + }{ + { + name: "simple not-equals", + raw: `.status.phase!=Succeeded`, + wantPath: `.status.phase`, + wantValue: `Succeeded`, + wantOK: true, + }, + { + name: "not-equals with double-quoted value", + raw: `.status.phase!="Succeeded"`, + wantPath: `.status.phase`, + wantValue: `Succeeded`, + wantOK: true, + }, + { + name: "not-equals with single-quoted value", + raw: `.status.phase!='Succeeded'`, + wantPath: `.status.phase`, + wantValue: `Succeeded`, + wantOK: true, + }, + { + name: "trims whitespace around expression", + raw: ` .status.phase != "Succeeded" `, + wantPath: `.status.phase`, + wantValue: `Succeeded`, + 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: "value containing not-equals in quotes", + raw: `.metadata.annotations["example.com/check"]!="a!=b"`, + wantPath: `.metadata.annotations["example.com/check"]`, + wantValue: `a!=b`, + wantOK: true, + }, + { + name: "top level not-equals after bracket expression", + raw: `.metadata.labels["app.kubernetes.io/name"]!="nginx"`, + wantPath: `.metadata.labels["app.kubernetes.io/name"]`, + wantValue: `nginx`, + wantOK: true, + }, + { + name: "bang without equals is not not-equals", + raw: `.spec.type!ClusterIP`, + wantOK: false, + }, + { + name: "truthy path without not-equals", + raw: `.spec.storageClassName`, + wantOK: false, + }, + { + name: "plain equals is not not-equals", + raw: `.spec.type=ClusterIP`, + wantOK: false, + }, + { + name: "double equals is not not-equals", + raw: `.spec.type==ClusterIP`, + wantOK: false, + }, + { + name: "empty string", + raw: ``, + wantOK: false, + }, + { + name: "whitespace string", + raw: ` `, + wantOK: false, + }, + { + name: "missing path", + raw: `!=Succeeded`, + wantOK: false, + }, + { + name: "missing value", + raw: `.status.phase!=`, + wantOK: false, + }, + { + name: "does not split not-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 not-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 not-equals inside brackets", + raw: `.spec.accessModes[?(@!="ReadWriteOnce")]!="value"`, + wantPath: `.spec.accessModes[?(@!="ReadWriteOnce")]`, + wantValue: `value`, + wantOK: true, + }, + { + name: "does not split not-equals inside parentheses", + raw: `.spec.values(@!="ignored")!="value"`, + wantPath: `.spec.values(@!="ignored")`, + wantValue: `value`, + wantOK: true, + }, + { + name: "does not split not-equals inside braces", + raw: `.spec.values{"a!=b"}!="value"`, + wantPath: `.spec.values{"a!=b"}`, + wantValue: `value`, + wantOK: true, + }, + { + name: "unmatched bracket suppresses split", + raw: `.metadata.annotations["key"!="value"`, + wantOK: false, + }, + { + name: "unmatched quote after not-equals remains part of value", + raw: `.status.phase!="Succeeded`, + wantPath: `.status.phase`, + wantValue: `"Succeeded`, + wantOK: true, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotPath, gotValue, gotOK := SplitFieldSelectorNotEquals(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 TestFindTopLevelNotEquals(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + wantIdx int + wantWidth int + }{ + { + name: "simple not-equals", + raw: `.status.phase!=Succeeded`, + wantIdx: len(`.status.phase`), + wantWidth: 2, + }, + { + name: "ignores not-equals inside brackets", + raw: `.spec.accessModes[?(@!="ReadWriteOnce")]`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "finds top level not-equals after brackets", + raw: `.metadata.labels["app.kubernetes.io/name"]!="nginx"`, + wantIdx: len(`.metadata.labels["app.kubernetes.io/name"]`), + wantWidth: 2, + }, + { + name: "ignores not-equals inside double quotes", + raw: `.metadata.annotations["a!=b"]`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores not-equals inside single quotes", + raw: `.metadata.annotations['a!=b']`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores escaped quote before not-equals inside quoted string", + raw: `.metadata.annotations["a\"!=b"]`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores not-equals inside parentheses", + raw: `.spec.values(@!="ignored")`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "ignores not-equals inside braces", + raw: `.spec.values{"a!=b"}`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "finds first top level not-equals", + raw: `.status.phase!=Succeeded!=ignored`, + wantIdx: len(`.status.phase`), + wantWidth: 2, + }, + { + name: "bang without equals is not not-equals", + raw: `.spec.type!ClusterIP`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "plain equals is not found", + raw: `.spec.type=ClusterIP`, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "empty string", + raw: ``, + wantIdx: -1, + wantWidth: 0, + }, + { + name: "no operator", + 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 := findTopLevelNotEquals(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 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 fab68612..50778c0e 100644 --- a/pkg/runtime/selectors/fields.go +++ b/pkg/runtime/selectors/fields.go @@ -13,8 +13,9 @@ import ( type FieldSelectorOperator string const ( - FieldSelectorTruthy FieldSelectorOperator = "truthy" - FieldSelectorEquals FieldSelectorOperator = "equals" + FieldSelectorTruthy FieldSelectorOperator = "truthy" + FieldSelectorEquals FieldSelectorOperator = "equals" + FieldSelectorNotEquals FieldSelectorOperator = "not-equals" ) // +kubebuilder:object:generate=true diff --git a/pkg/utils/compiled_target.go b/pkg/utils/compiled_target.go index 84a6d178..563833e6 100644 --- a/pkg/utils/compiled_target.go +++ b/pkg/utils/compiled_target.go @@ -21,6 +21,22 @@ func CompileFieldSelector( return selectors.CompiledFieldSelector{}, fmt.Errorf("field selector must not be empty") } + // Check != before == so that "!=" is not misidentified as a bare "=" match. + if path, value, ok := jsonpath.SplitFieldSelectorNotEquals(raw); ok { + compiledPath, err := cache.GetOrCompile(path) + if err != nil { + return selectors.CompiledFieldSelector{}, err + } + + return selectors.CompiledFieldSelector{ + Raw: raw, + Path: path, + Operator: selectors.FieldSelectorNotEquals, + Value: value, + Compiled: compiledPath, + }, nil + } + path, value, ok := jsonpath.SplitFieldSelectorEquals(raw) if !ok { compiledPath, err := cache.GetOrCompile(raw)