diff --git a/api/v1beta2/capsuleconfiguration_types.go b/api/v1beta2/capsuleconfiguration_types.go index a89fe265..5480f0d6 100644 --- a/api/v1beta2/capsuleconfiguration_types.go +++ b/api/v1beta2/capsuleconfiguration_types.go @@ -173,6 +173,7 @@ type ServiceAccountClient struct { // +kubebuilder:resource:scope=Cluster // +kubebuilder:storageversion // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="Reconcile status" +// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description="Reconcile Message" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" // CapsuleConfiguration is the Schema for the Capsule configuration API. diff --git a/api/v1beta2/customquota_types.go b/api/v1beta2/customquota_types.go index 2a37fa9b..80672325 100644 --- a/api/v1beta2/customquota_types.go +++ b/api/v1beta2/customquota_types.go @@ -64,6 +64,7 @@ type CustomQuotaSpecSourceConfig struct { // +kubebuilder:printcolumn:name="Available",type="string",JSONPath=".status.usage.available",description="The total amount available" // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="Reconcile Status" // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description="Reconcile Message" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Age" type CustomQuota struct { metav1.TypeMeta `json:",inline"` diff --git a/api/v1beta2/globalcustomquota_types.go b/api/v1beta2/globalcustomquota_types.go index d379e597..2dd2eda8 100644 --- a/api/v1beta2/globalcustomquota_types.go +++ b/api/v1beta2/globalcustomquota_types.go @@ -25,6 +25,7 @@ type GlobalCustomQuotaSpec struct { // +kubebuilder:printcolumn:name="Available",type="string",JSONPath=".status.usage.available",description="The total amount available" // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="Reconcile Status" // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description="Reconcile Message" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Age" type GlobalCustomQuota struct { metav1.TypeMeta `json:",inline"` diff --git a/api/v1beta2/quantityledgers_types.go b/api/v1beta2/quantityledgers_types.go index 2719c956..e453f7a6 100644 --- a/api/v1beta2/quantityledgers_types.go +++ b/api/v1beta2/quantityledgers_types.go @@ -75,7 +75,6 @@ type QuantityLedgerSpec struct { // +kubebuilder:printcolumn:name="TargetNamespace",type=string,JSONPath=`.spec.targetRef.namespace` // +kubebuilder:printcolumn:name="TargetName",type=string,JSONPath=`.spec.targetRef.name` // +kubebuilder:printcolumn:name="Reserved",type=string,JSONPath=`.status.reserved` -// +kubebuilder:printcolumn:name="Reservations",type=integer,JSONPath=`.status.reservations.size()` // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` type QuantityLedger struct { metav1.TypeMeta `json:",inline"` diff --git a/api/v1beta2/rule_status_type.go b/api/v1beta2/rule_status_type.go index 0a48540f..0a9183a4 100644 --- a/api/v1beta2/rule_status_type.go +++ b/api/v1beta2/rule_status_type.go @@ -33,6 +33,7 @@ type RuleStatusStatus struct { // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="Ready Status" // +kubebuilder:printcolumn:name="Message",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description="Ready Message" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp",description="Age" type RuleStatus struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitzero"` diff --git a/charts/capsule/crds/capsule.clastix.io_capsuleconfigurations.yaml b/charts/capsule/crds/capsule.clastix.io_capsuleconfigurations.yaml index c023814d..17fbd4e1 100644 --- a/charts/capsule/crds/capsule.clastix.io_capsuleconfigurations.yaml +++ b/charts/capsule/crds/capsule.clastix.io_capsuleconfigurations.yaml @@ -19,6 +19,10 @@ spec: jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string + - description: Reconcile Message + jsonPath: .status.conditions[?(@.type=="Ready")].message + name: Status + type: string - jsonPath: .metadata.creationTimestamp name: Age type: date diff --git a/charts/capsule/crds/capsule.clastix.io_customquotas.yaml b/charts/capsule/crds/capsule.clastix.io_customquotas.yaml index a310c72d..189696bd 100644 --- a/charts/capsule/crds/capsule.clastix.io_customquotas.yaml +++ b/charts/capsule/crds/capsule.clastix.io_customquotas.yaml @@ -37,6 +37,10 @@ spec: jsonPath: .status.conditions[?(@.type=="Ready")].message name: Status type: string + - description: Age + jsonPath: .metadata.creationTimestamp + name: Age + type: date name: v1beta2 schema: openAPIV3Schema: diff --git a/charts/capsule/crds/capsule.clastix.io_globalcustomquotas.yaml b/charts/capsule/crds/capsule.clastix.io_globalcustomquotas.yaml index 3aed23e8..c103def1 100644 --- a/charts/capsule/crds/capsule.clastix.io_globalcustomquotas.yaml +++ b/charts/capsule/crds/capsule.clastix.io_globalcustomquotas.yaml @@ -37,6 +37,10 @@ spec: jsonPath: .status.conditions[?(@.type=="Ready")].message name: Status type: string + - description: Age + jsonPath: .metadata.creationTimestamp + name: Age + type: date name: v1beta2 schema: openAPIV3Schema: diff --git a/charts/capsule/crds/capsule.clastix.io_quantityledgers.yaml b/charts/capsule/crds/capsule.clastix.io_quantityledgers.yaml index e1789033..604db24d 100644 --- a/charts/capsule/crds/capsule.clastix.io_quantityledgers.yaml +++ b/charts/capsule/crds/capsule.clastix.io_quantityledgers.yaml @@ -29,9 +29,6 @@ spec: - jsonPath: .status.reserved name: Reserved type: string - - jsonPath: .status.reservations.size() - name: Reservations - type: integer - jsonPath: .metadata.creationTimestamp name: Age type: date diff --git a/charts/capsule/crds/capsule.clastix.io_rulestatuses.yaml b/charts/capsule/crds/capsule.clastix.io_rulestatuses.yaml index d86e11b1..b5ac6d42 100644 --- a/charts/capsule/crds/capsule.clastix.io_rulestatuses.yaml +++ b/charts/capsule/crds/capsule.clastix.io_rulestatuses.yaml @@ -23,6 +23,10 @@ spec: jsonPath: .status.conditions[?(@.type=="Ready")].message name: Message type: string + - description: Age + jsonPath: .metadata.creationTimestamp + name: Age + type: date name: v1beta2 schema: openAPIV3Schema: @@ -64,46 +68,54 @@ spec: - deny - audit type: string - registries: - description: |- - Define registries which are allowed to be used within this tenant - The rules are aggregated, since you can use Regular Expressions the match registry endpoints - items: - properties: - exp: - description: Expression used to evaluate regex + workloads: + description: Enforcement for Workloads (Pods) + properties: + qosClasses: + description: |- + Define Pod QoS classes matched by this enforcement rule. + Supported values are Guaranteed, Burstable and BestEffort. + items: + description: PodQOSClass defines the supported qos classes + of Pods. type: string - negate: - default: false - description: Negate regular Expression - type: boolean - policy: - description: Allowed PullPolicy for the given registry. - Supplying no value allows all policies. - items: - description: PullPolicy describes a policy for if/when - to pull a container image - type: string - type: array - url: - description: |- - Deprecated: Use exp field - - OCI Registry endpoint, is treated as regular expression. - type: string - validation: - default: - - pod/images + type: array + registries: + description: |- + Define registries which are allowed to be used within this tenant + The rules are aggregated, since you can use Regular Expressions the match registry endpoints + items: + properties: + exp: + description: Expression used to evaluate regex + type: string + negate: + default: false + description: Negate regular Expression + type: boolean + policy: + description: Allowed PullPolicy for the given registry. + Supplying no value allows all policies. + items: + description: PullPolicy describes a policy for if/when + to pull a container image + type: string + type: array + type: object + type: array + targets: + description: |- + Define the enforcement targets this rule applies to. + If empty, each webhook applies its own backwards-compatible default. + items: + enum: + - pod/initcontainers + - pod/ephemeralcontainers + - pod/containers - pod/volumes - description: Requesting Resources - items: - enum: - - pod/images - - pod/volumes - type: string - type: array - type: object - type: array + type: string + type: array + type: object type: object type: object type: array @@ -193,46 +205,54 @@ spec: - deny - audit type: string - registries: - description: |- - Define registries which are allowed to be used within this tenant - The rules are aggregated, since you can use Regular Expressions the match registry endpoints - items: - properties: - exp: - description: Expression used to evaluate regex + workloads: + description: Enforcement for Workloads (Pods) + properties: + qosClasses: + description: |- + Define Pod QoS classes matched by this enforcement rule. + Supported values are Guaranteed, Burstable and BestEffort. + items: + description: PodQOSClass defines the supported qos classes + of Pods. type: string - negate: - default: false - description: Negate regular Expression - type: boolean - policy: - description: Allowed PullPolicy for the given registry. - Supplying no value allows all policies. - items: - description: PullPolicy describes a policy for if/when - to pull a container image - type: string - type: array - url: - description: |- - Deprecated: Use exp field - - OCI Registry endpoint, is treated as regular expression. - type: string - validation: - default: - - pod/images + type: array + registries: + description: |- + Define registries which are allowed to be used within this tenant + The rules are aggregated, since you can use Regular Expressions the match registry endpoints + items: + properties: + exp: + description: Expression used to evaluate regex + type: string + negate: + default: false + description: Negate regular Expression + type: boolean + policy: + description: Allowed PullPolicy for the given registry. + Supplying no value allows all policies. + items: + description: PullPolicy describes a policy for + if/when to pull a container image + type: string + type: array + type: object + type: array + targets: + description: |- + Define the enforcement targets this rule applies to. + If empty, each webhook applies its own backwards-compatible default. + items: + enum: + - pod/initcontainers + - pod/ephemeralcontainers + - pod/containers - pod/volumes - description: Requesting Resources - items: - enum: - - pod/images - - pod/volumes - type: string - type: array - type: object - type: array + type: string + type: array + type: object type: object type: object rules: @@ -258,46 +278,54 @@ spec: - deny - audit type: string - registries: - description: |- - Define registries which are allowed to be used within this tenant - The rules are aggregated, since you can use Regular Expressions the match registry endpoints - items: - properties: - exp: - description: Expression used to evaluate regex + workloads: + description: Enforcement for Workloads (Pods) + properties: + qosClasses: + description: |- + Define Pod QoS classes matched by this enforcement rule. + Supported values are Guaranteed, Burstable and BestEffort. + items: + description: PodQOSClass defines the supported qos + classes of Pods. type: string - negate: - default: false - description: Negate regular Expression - type: boolean - policy: - description: Allowed PullPolicy for the given registry. - Supplying no value allows all policies. - items: - description: PullPolicy describes a policy for if/when - to pull a container image - type: string - type: array - url: - description: |- - Deprecated: Use exp field - - OCI Registry endpoint, is treated as regular expression. - type: string - validation: - default: - - pod/images + type: array + registries: + description: |- + Define registries which are allowed to be used within this tenant + The rules are aggregated, since you can use Regular Expressions the match registry endpoints + items: + properties: + exp: + description: Expression used to evaluate regex + type: string + negate: + default: false + description: Negate regular Expression + type: boolean + policy: + description: Allowed PullPolicy for the given + registry. Supplying no value allows all policies. + items: + description: PullPolicy describes a policy for + if/when to pull a container image + type: string + type: array + type: object + type: array + targets: + description: |- + Define the enforcement targets this rule applies to. + If empty, each webhook applies its own backwards-compatible default. + items: + enum: + - pod/initcontainers + - pod/ephemeralcontainers + - pod/containers - pod/volumes - description: Requesting Resources - items: - enum: - - pod/images - - pod/volumes - type: string - type: array - type: object - type: array + type: string + type: array + type: object type: object type: object type: array diff --git a/charts/capsule/crds/capsule.clastix.io_tenants.yaml b/charts/capsule/crds/capsule.clastix.io_tenants.yaml index 828df24f..a81ec8f3 100644 --- a/charts/capsule/crds/capsule.clastix.io_tenants.yaml +++ b/charts/capsule/crds/capsule.clastix.io_tenants.yaml @@ -2515,46 +2515,54 @@ spec: - deny - audit type: string - registries: - description: |- - Define registries which are allowed to be used within this tenant - The rules are aggregated, since you can use Regular Expressions the match registry endpoints - items: - properties: - exp: - description: Expression used to evaluate regex + workloads: + description: Enforcement for Workloads (Pods) + properties: + qosClasses: + description: |- + Define Pod QoS classes matched by this enforcement rule. + Supported values are Guaranteed, Burstable and BestEffort. + items: + description: PodQOSClass defines the supported qos + classes of Pods. type: string - negate: - default: false - description: Negate regular Expression - type: boolean - policy: - description: Allowed PullPolicy for the given registry. - Supplying no value allows all policies. - items: - description: PullPolicy describes a policy for if/when - to pull a container image - type: string - type: array - url: - description: |- - Deprecated: Use exp field - - OCI Registry endpoint, is treated as regular expression. - type: string - validation: - default: - - pod/images + type: array + registries: + description: |- + Define registries which are allowed to be used within this tenant + The rules are aggregated, since you can use Regular Expressions the match registry endpoints + items: + properties: + exp: + description: Expression used to evaluate regex + type: string + negate: + default: false + description: Negate regular Expression + type: boolean + policy: + description: Allowed PullPolicy for the given + registry. Supplying no value allows all policies. + items: + description: PullPolicy describes a policy for + if/when to pull a container image + type: string + type: array + type: object + type: array + targets: + description: |- + Define the enforcement targets this rule applies to. + If empty, each webhook applies its own backwards-compatible default. + items: + enum: + - pod/initcontainers + - pod/ephemeralcontainers + - pod/containers - pod/volumes - description: Requesting Resources - items: - enum: - - pod/images - - pod/volumes - type: string - type: array - type: object - type: array + type: string + type: array + type: object type: object namespaceSelector: description: Select namespaces which are going to be targeted @@ -2606,7 +2614,7 @@ spec: permissions: description: Permissions for given rule properties: - rules: + promotions: description: |- Define Promotion Rules which distributed additional ClusterRoles across the Tenant for promoted ServiceAccounts. @@ -3125,23 +3133,6 @@ spec: to pull a container image type: string type: array - url: - description: |- - Deprecated: Use exp field - - OCI Registry endpoint, is treated as regular expression. - type: string - validation: - default: - - pod/images - - pod/volumes - description: Requesting Resources - items: - enum: - - pod/images - - pod/volumes - type: string - type: array type: object type: array type: object diff --git a/cmd/controller/main.go b/cmd/controller/main.go index e5aa8d0d..94681d8e 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -564,6 +564,7 @@ func main() { pod.ContainerRegistry(cfg, registryCache), pod.PriorityClass(), pod.RuntimeClass(), + pod.QoSClass(cfg), ), ), route.Ingress(ingress.Class(cfg, kubeVersion), ingress.Hostnames(cfg), ingress.Collision(cfg), ingress.Wildcard()), diff --git a/e2e/replications_globaltenantresource_test.go b/e2e/replications_globaltenantresource_test.go index 292fbdc6..689f836f 100644 --- a/e2e/replications_globaltenantresource_test.go +++ b/e2e/replications_globaltenantresource_test.go @@ -165,6 +165,62 @@ var _ = Describe("GlobalTenantResource", Ordered, Label("replications", "global" }) + It("skips applying resources to terminating namespaces and removes them from processedItems", func() { + terminatingNamespace := tenantANamespaces[2] + releaseNamespace := holdNamespaceTerminating(ctx, terminatingNamespace) + defer releaseNamespace() + + gtr := newRawConfigMapGlobalTenantResource("gtr-skip-terminating-namespace", map[string]string{ + "mode": "active", + }) + + gtr.Spec.TenantSelector = metav1.LabelSelector{ + MatchLabels: map[string]string{"energy": "solar"}, + } + + renameFirstRawConfigMap(gtr, "gtr-skip-terminating") + + EventuallyCreation(func() error { + return k8sClient.Create(ctx, gtr) + }).Should(Succeed()) + + By("verifying non-terminating selected namespaces still receive the resource") + for _, ns := range tenantANamespaces[:2] { + expectConfigMapData(ns, "gtr-skip-terminating", map[string]string{ + "mode": "active", + }) + } + + By("verifying the terminating selected namespace is skipped") + Consistently(func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: "gtr-skip-terminating", + Namespace: terminatingNamespace, + }, &corev1.ConfigMap{}) + }, 2*resyncPeriod.Duration, defaultPollInterval).Should(HaveOccurred()) + + By("verifying non-selected tenants do not receive the resource") + for _, ns := range tenantBNamespaces { + expectConfigMapAbsent(ns, "gtr-skip-terminating") + } + + By("verifying the terminating namespace item is not kept in processedItems") + Eventually(func(g Gomega) { + current := &capsulev1beta2.GlobalTenantResource{} + + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: gtr.Name, + }, current)).To(Succeed()) + + for _, item := range current.Status.ProcessedItems { + g.Expect(item.Name).To(Equal("gtr-skip-terminating")) + + g.Expect(item.Namespace).ToNot(Equal(terminatingNamespace)) + g.Expect(item.Status).To(Equal(metav1.ConditionTrue)) + } + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + }) + It("fails to replicate namespacedItems when the impersonated service account cannot read source resources", func() { saName := "gtr-no-namespaceditem-read" ensureServiceAccount("capsule-system", saName) diff --git a/e2e/replications_tenantresource_test.go b/e2e/replications_tenantresource_test.go index ec01acaf..47e67f6f 100644 --- a/e2e/replications_tenantresource_test.go +++ b/e2e/replications_tenantresource_test.go @@ -134,6 +134,79 @@ var _ = Describe("TenantResource SSA", Ordered, Label("replications", "namespace }) }) + It("skips applying resources to terminating namespaces and removes them from processedItems", func() { + terminatingNamespace := targetNamespaces[2] + releaseNamespace := holdNamespaceTerminating(ctx, terminatingNamespace) + defer releaseNamespace() + + tr := &capsulev1beta2.TenantResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "skip-terminating-namespace", + Namespace: baseNamespace, + }, + Spec: capsulev1beta2.TenantResourceSpec{ + TenantResourceCommonSpec: capsulev1beta2.TenantResourceCommonSpec{ + PruningOnDelete: ptr.To(true), + ResyncPeriod: metav1.Duration{Duration: 5 * time.Second}, + Resources: []capsulev1beta2.ResourceSpec{{ + RawItems: []capsulev1beta2.RawExtension{{ + RawExtension: runtime.RawExtension{ + Object: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "tr-skip-terminating", + }, + Data: map[string]string{ + "mode": "active", + }, + }, + }, + }}, + }}, + }, + }, + } + + EventuallyCreation(func() error { + return k8sClient.Create(ctx, tr) + }).Should(Succeed()) + + By("verifying non-terminating namespaces still receive the resource") + for _, ns := range targetNamespaces[:2] { + expectConfigMapData(ns, "tr-skip-terminating", map[string]string{ + "mode": "active", + }) + } + + By("verifying the terminating namespace is skipped") + Consistently(func() error { + return k8sClient.Get(ctx, types.NamespacedName{ + Name: "tr-skip-terminating", + Namespace: terminatingNamespace, + }, &corev1.ConfigMap{}) + }, 2*resyncPeriod.Duration, defaultPollInterval).Should(HaveOccurred()) + + By("verifying the terminating namespace item is not kept in processedItems") + Eventually(func(g Gomega) { + current := &capsulev1beta2.TenantResource{} + + g.Expect(k8sClient.Get(ctx, types.NamespacedName{ + Name: tr.Name, + Namespace: tr.Namespace, + }, current)).To(Succeed()) + + for _, item := range current.Status.ProcessedItems { + g.Expect(item.Name).To(Equal("tr-skip-terminating")) + + g.Expect(item.Namespace).ToNot(Equal(terminatingNamespace)) + g.Expect(item.Status).To(Equal(metav1.ConditionTrue)) + } + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + }) + Context("generators and template context", func() { It("fails when a templated namespace resolves to a forbidden namespace", func() { diff --git a/e2e/rules_enforce_qos_test.go b/e2e/rules_enforce_qos_test.go new file mode 100644 index 00000000..a6fcbf51 --- /dev/null +++ b/e2e/rules_enforce_qos_test.go @@ -0,0 +1,590 @@ +// Copyright 2020-2026 Project Capsule Authors. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + "fmt" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "sigs.k8s.io/controller-runtime/pkg/client" + + capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" + "github.com/projectcapsule/capsule/pkg/api/meta" + "github.com/projectcapsule/capsule/pkg/api/rbac" + "github.com/projectcapsule/capsule/pkg/api/rules" +) + +var _ = Describe("enforcing pod QoS namespace rules", Ordered, Label("tenant", "rules", "enforce", "workloads", "qos"), func() { + const ownerName = "e2e-rules-qos" + + var tnt *capsulev1beta2.Tenant + + newTenant := func() *capsulev1beta2.Tenant { + return &capsulev1beta2.Tenant{ + ObjectMeta: metav1.ObjectMeta{ + Name: "e2e-rule-qos", + Labels: map[string]string{ + "env": "e2e", + }, + }, + Spec: capsulev1beta2.TenantSpec{ + Owners: rbac.OwnerListSpec{ + { + CoreOwnerSpec: rbac.CoreOwnerSpec{ + UserSpec: rbac.UserSpec{ + Name: ownerName, + Kind: "User", + }, + }, + }, + }, + Rules: []*rules.NamespaceRuleBodyTenant{ + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeDeny, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + QoSClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + }, + }, + }, + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeAudit, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + QoSClasses: []corev1.PodQOSClass{ + corev1.PodQOSBurstable, + }, + }, + }, + }, + }, + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "allow-best-effort": "true", + }, + }, + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeAllow, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + QoSClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + }, + }, + }, + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "deny-burstable": "true", + }, + }, + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeDeny, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + QoSClasses: []corev1.PodQOSClass{ + corev1.PodQOSBurstable, + }, + }, + }, + }, + }, + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "containers-target": "true", + }, + }, + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeDeny, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: []rules.WorkloadValidationTarget{ + rules.ValidateContainers, + }, + QoSClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + }, + }, + }, + }, + }, + } + } + + type expectedQoSStatusRule struct { + action rules.ActionType + targets []rules.WorkloadValidationTarget + qosClasses []corev1.PodQOSClass + } + + expectNamespaceStatusRules := func(nsName string, want []expectedQoSStatusRule) { + Eventually(func(g Gomega) { + nsStatus := &capsulev1beta2.RuleStatus{} + g.Expect(k8sClient.Get( + context.Background(), + client.ObjectKey{Name: meta.NameForManagedRuleStatus(), Namespace: nsName}, + nsStatus, + )).To(Succeed()) + + g.Expect(nsStatus.Status.Rules).To(HaveLen(len(want))) + + for i, expected := range want { + gotRule := nsStatus.Status.Rules[i] + g.Expect(gotRule).NotTo(BeNil()) + g.Expect(gotRule.Enforce.Action).To(Equal(expected.action)) + g.Expect(gotRule.Enforce.Workloads.Targets).To(Equal(expected.targets)) + g.Expect(gotRule.Enforce.Workloads.QoSClasses).To(Equal(expected.qosClasses)) + } + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + } + + createPodAndExpectDenied := func(cs kubernetes.Interface, nsName string, pod *corev1.Pod, substrings ...string) { + base := pod.DeepCopy() + baseName := base.Name + if baseName == "" { + baseName = "pod" + } + + Eventually(func() error { + p := base.DeepCopy() + p.Name = fmt.Sprintf("%s-%d", baseName, time.Now().UnixNano()%1e6) + + _, err := cs.CoreV1().Pods(nsName).Create(context.Background(), p, metav1.CreateOptions{}) + if err == nil { + _ = cs.CoreV1().Pods(nsName).Delete(context.Background(), p.Name, metav1.DeleteOptions{}) + + return fmt.Errorf("expected create to be denied, but it succeeded") + } + + if apierrors.IsAlreadyExists(err) { + return fmt.Errorf("unexpected AlreadyExists: %v", err) + } + + msg := err.Error() + for _, substring := range substrings { + if !strings.Contains(msg, substring) { + return fmt.Errorf("expected error to contain %q, got: %s", substring, msg) + } + } + + return nil + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + } + + createPodAndExpectAllowed := func(cs kubernetes.Interface, nsName string, pod *corev1.Pod) { + EventuallyCreation(func() error { + _, err := cs.CoreV1().Pods(nsName).Create(context.Background(), pod, metav1.CreateOptions{}) + + return err + }).Should(Succeed()) + } + + expectAuditEvent := func(cs kubernetes.Interface, nsName string, podName string, substrings ...string) { + Eventually(func() error { + events, err := cs.CoreV1().Events(nsName).List(context.Background(), metav1.ListOptions{}) + if err != nil { + return err + } + + for _, event := range events.Items { + if event.InvolvedObject.Name != podName { + continue + } + + msg := event.Message + matched := true + + for _, substring := range substrings { + if !strings.Contains(msg, substring) { + matched = false + + break + } + } + + if matched { + return nil + } + } + + return fmt.Errorf("expected audit event for pod %q containing %q", podName, substrings) + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + } + + bestEffortPod := func(name string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "c", + Image: "registry.k8s.io/pause:3.9", + ImagePullPolicy: corev1.PullIfNotPresent, + SecurityContext: restrictedContainerSecurityContext(), + }, + }, + }, + } + } + + burstablePod := func(name string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "c", + Image: "registry.k8s.io/pause:3.9", + ImagePullPolicy: corev1.PullIfNotPresent, + SecurityContext: restrictedContainerSecurityContext(), + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("16Mi"), + }, + }, + }, + }, + }, + } + } + + guaranteedPod := func(name string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "c", + Image: "registry.k8s.io/pause:3.9", + ImagePullPolicy: corev1.PullIfNotPresent, + SecurityContext: restrictedContainerSecurityContext(), + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("16Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("16Mi"), + }, + }, + }, + }, + }, + } + } + + podWithInitContainerQoS := func(name string, initResources corev1.ResourceRequirements, containerResources corev1.ResourceRequirements) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + InitContainers: []corev1.Container{ + { + Name: "init", + Image: "registry.k8s.io/pause:3.9", + ImagePullPolicy: corev1.PullIfNotPresent, + SecurityContext: restrictedContainerSecurityContext(), + Resources: initResources, + }, + }, + Containers: []corev1.Container{ + { + Name: "c", + Image: "registry.k8s.io/pause:3.9", + ImagePullPolicy: corev1.PullIfNotPresent, + SecurityContext: restrictedContainerSecurityContext(), + Resources: containerResources, + }, + }, + }, + } + } + + burstableResources := func() corev1.ResourceRequirements { + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("16Mi"), + }, + } + } + + guaranteedResources := func() corev1.ResourceRequirements { + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("16Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("10m"), + corev1.ResourceMemory: resource.MustParse("16Mi"), + }, + } + } + + JustBeforeEach(func() { + tnt = newTenant() + + EventuallyCreation(func() error { + tnt.ResourceVersion = "" + + return k8sClient.Create(context.TODO(), tnt) + }).Should(Succeed()) + + TenantReady(tnt, metav1.ConditionTrue, defaultTimeoutInterval) + }) + + JustAfterEach(func() { + EventuallyDeletion(tnt) + }) + + It("stores QoS workload rules as independent status rule blocks", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + expectNamespaceStatusRules(ns.GetName(), []expectedQoSStatusRule{ + { + action: rules.ActionTypeDeny, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + { + action: rules.ActionTypeAudit, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBurstable, + }, + }, + }) + }) + + It("stores namespace-selector matched QoS rules as additional status rule blocks", func() { + ns := NewNamespace("", map[string]string{ + "allow-best-effort": "true", + meta.TenantLabel: tnt.GetName(), + }) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + expectNamespaceStatusRules(ns.GetName(), []expectedQoSStatusRule{ + { + action: rules.ActionTypeDeny, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + { + action: rules.ActionTypeAudit, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBurstable, + }, + }, + { + action: rules.ActionTypeAllow, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + }) + }) + + It("denies BestEffort pods by default", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + createPodAndExpectDenied(cs, ns.Name, bestEffortPod("besteffort-denied"), + "BestEffort", + "denied", + ) + }) + + It("allows Guaranteed pods when no QoS rule matches them", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + createPodAndExpectAllowed(cs, ns.Name, guaranteedPod("guaranteed-allowed")) + }) + + It("audits Burstable pods by allowing admission and emitting an event", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + pod := burstablePod("burstable-audited") + + createPodAndExpectAllowed(cs, ns.Name, pod) + + expectAuditEvent(cs, ns.Name, pod.Name, + "Burstable", + "audit QoS rule", + ) + }) + + It("allows BestEffort pods when a later namespace-selected allow rule matches", func() { + ns := NewNamespace("", map[string]string{ + "allow-best-effort": "true", + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + createPodAndExpectAllowed(cs, ns.Name, bestEffortPod("besteffort-allowed")) + }) + + It("denies Burstable pods when a later namespace-selected deny rule overrides an earlier audit rule", func() { + ns := NewNamespace("", map[string]string{ + "deny-burstable": "true", + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + createPodAndExpectDenied(cs, ns.Name, burstablePod("burstable-denied"), + "Burstable", + "denied", + ) + }) + + It("computes QoS across init containers and regular containers", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + pod := podWithInitContainerQoS( + "init-causes-burstable", + burstableResources(), + guaranteedResources(), + ) + + createPodAndExpectAllowed(cs, ns.Name, pod) + + expectAuditEvent(cs, ns.Name, pod.Name, + "Burstable", + "audit QoS rule", + ) + }) + + It("uses empty targets as all targets for QoS rules", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + createPodAndExpectDenied(cs, ns.Name, bestEffortPod("empty-targets-denied"), + "BestEffort", + "denied", + ) + }) + + It("applies QoS rules when explicit workload targets are configured", func() { + ns := NewNamespace("", map[string]string{ + "containers-target": "true", + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + expectNamespaceStatusRules(ns.GetName(), []expectedQoSStatusRule{ + { + action: rules.ActionTypeDeny, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + { + action: rules.ActionTypeAudit, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBurstable, + }, + }, + { + action: rules.ActionTypeDeny, + targets: []rules.WorkloadValidationTarget{ + rules.ValidateContainers, + }, + qosClasses: []corev1.PodQOSClass{ + corev1.PodQOSBestEffort, + }, + }, + }) + + createPodAndExpectDenied(cs, ns.Name, bestEffortPod("explicit-target-denied"), + "BestEffort", + "denied", + ) + }) +}) diff --git a/e2e/rules_registry_test.go b/e2e/rules_enforce_registry_test.go similarity index 54% rename from e2e/rules_registry_test.go rename to e2e/rules_enforce_registry_test.go index 75270686..c585b3e9 100644 --- a/e2e/rules_registry_test.go +++ b/e2e/rules_enforce_registry_test.go @@ -24,6 +24,21 @@ import ( "github.com/projectcapsule/capsule/pkg/api/rules" ) +var ( + targetContainers = []rules.WorkloadValidationTarget{ + "pod/containers", + } + targetEphemeralContainers = []rules.WorkloadValidationTarget{ + "pod/ephemeralcontainers", + } + targetInitContainers = []rules.WorkloadValidationTarget{ + "pod/initcontainers", + } + targetVolumes = []rules.WorkloadValidationTarget{ + "pod/volumes", + } +) + var _ = Describe("enforcing container registry namespace rules", Ordered, Label("tenant", "rules", "images", "registry"), func() { const ownerName = "e2e-rules-registry" @@ -50,15 +65,15 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( }, Rules: []*rules.NamespaceRuleBodyTenant{ { - NamespaceRuleBodyNamespace: rules.NamespaceRuleBodyNamespace{ - Enforce: rules.NamespaceRuleEnforceBody{ + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ Action: rules.ActionTypeAllow, - Registries: []rules.OCIRegistry{ - { - Registry: "harbor/.*", - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, - rules.ValidateVolumes, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "harbor/.*", + }, }, }, }, @@ -66,18 +81,121 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( }, }, { - NamespaceRuleBodyNamespace: rules.NamespaceRuleBodyNamespace{ - Enforce: rules.NamespaceRuleEnforceBody{ + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ Action: rules.ActionTypeDeny, - Registries: []rules.OCIRegistry{ - { - Registry: "harbor/customer/.*", - Policy: []corev1.PullPolicy{ - corev1.PullNever, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetContainers, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "harbor/customer/containers/.*", + }, }, - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, - rules.ValidateVolumes, + }, + }, + }, + }, + }, + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeDeny, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetInitContainers, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "harbor/customer/init/.*", + }, + }, + }, + }, + }, + }, + }, + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeDeny, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetEphemeralContainers, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "harbor/customer/debug/.*", + }, + }, + }, + }, + }, + }, + }, + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeDeny, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetVolumes, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "harbor/customer/volume/.*", + }, + }, + }, + }, + }, + }, + }, + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeAudit, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetContainers, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "audit/containers/.*", + }, + }, + }, + }, + }, + }, + }, + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeAudit, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetVolumes, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "audit/volumes/.*", + }, + }, + }, + }, + }, + }, + }, + { + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rules.ActionTypeAllow, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetContainers, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "policy/.*", + }, + Policy: []corev1.PullPolicy{ + corev1.PullNever, + }, }, }, }, @@ -90,31 +208,16 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( "environment": "prod", }, }, - NamespaceRuleBodyNamespace: rules.NamespaceRuleBodyNamespace{ - Enforce: rules.NamespaceRuleEnforceBody{ + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ Action: rules.ActionTypeAllow, - Registries: []rules.OCIRegistry{ - { - Registry: "harbor/customer/prod-image/.*", - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, - rules.ValidateVolumes, - }, - }, - }, - }, - }, - }, - { - NamespaceRuleBodyNamespace: rules.NamespaceRuleBodyNamespace{ - Enforce: rules.NamespaceRuleEnforceBody{ - Action: rules.ActionTypeAudit, - Registries: []rules.OCIRegistry{ - { - Registry: "audit/.*", - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, - rules.ValidateVolumes, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetContainers, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "harbor/customer/containers/prod/.*", + }, }, }, }, @@ -127,17 +230,17 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( "negate": "true", }, }, - NamespaceRuleBodyNamespace: rules.NamespaceRuleBodyNamespace{ - Enforce: rules.NamespaceRuleEnforceBody{ + NamespaceRuleBodyNamespace: &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ Action: rules.ActionTypeDeny, - Registries: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: "trusted/.*", - Negate: true, - }, - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: targetContainers, + Registries: []rules.OCIRegistry{ + { + RegExpression: api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }, }, }, }, @@ -151,6 +254,7 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( type expectedStatusRule struct { action rules.ActionType + targets []rules.WorkloadValidationTarget expressions []string negated []bool } @@ -158,26 +262,42 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( expectNamespaceStatusRules := func(nsName string, want []expectedStatusRule) { Eventually(func(g Gomega) { nsStatus := &capsulev1beta2.RuleStatus{} + g.Expect(k8sClient.Get( context.Background(), - client.ObjectKey{Name: meta.NameForManagedRuleStatus(), Namespace: nsName}, + client.ObjectKey{ + Name: meta.NameForManagedRuleStatus(), + Namespace: nsName, + }, nsStatus, )).To(Succeed()) g.Expect(nsStatus.Status.Rules).To(HaveLen(len(want))) for i, expected := range want { - gotRule := nsStatus.Status.Rules[i] - g.Expect(gotRule).NotTo(BeNil()) - g.Expect(gotRule.Enforce.Action).To(Equal(expected.action)) - g.Expect(gotRule.Enforce.Registries).To(HaveLen(len(expected.expressions))) + got := nsStatus.Status.Rules[i] + + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Enforce).NotTo(BeNil()) + g.Expect(got.Enforce.Action).To(Equal(expected.action)) + + if len(expected.targets) == 0 { + g.Expect(got.Enforce.Workloads.Targets).To(BeEmpty()) + } else { + g.Expect(got.Enforce.Workloads.Targets).To(Equal(expected.targets)) + } + + g.Expect(got.Enforce.Workloads.Registries).To(HaveLen(len(expected.expressions))) for j, expectedExpression := range expected.expressions { - expr := gotRule.Enforce.Registries[j].Expression() + expr := got.Enforce.Workloads.Registries[j].Expression() + g.Expect(expr.Expression).To(Equal(expectedExpression)) if len(expected.negated) > j { g.Expect(expr.Negate).To(Equal(expected.negated[j])) + } else { + g.Expect(expr.Negate).To(BeFalse()) } } } @@ -317,6 +437,26 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( EventuallyDeletion(tnt) }) + It("denies an allowed registry reference when its pull policy is not permitted", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + pod := restrictedPod("policy-denied", "policy/team/app:1", corev1.PullIfNotPresent) + + createPodAndExpectDenied(cs, ns.Name, pod, + "containers[0]", + "policy/team/app:1", + "pullPolicy=IfNotPresent", + "allowed: Never", + ) + }) + It("stores matching tenant rules as independent status rule blocks", func() { ns := NewNamespace("", map[string]string{ meta.TenantLabel: tnt.GetName(), @@ -332,11 +472,38 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( }, { action: rules.ActionTypeDeny, - expressions: []string{"harbor/customer/.*"}, + targets: targetContainers, + expressions: []string{"harbor/customer/containers/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetInitContainers, + expressions: []string{"harbor/customer/init/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetEphemeralContainers, + expressions: []string{"harbor/customer/debug/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetVolumes, + expressions: []string{"harbor/customer/volume/.*"}, }, { action: rules.ActionTypeAudit, - expressions: []string{"audit/.*"}, + targets: targetContainers, + expressions: []string{"audit/containers/.*"}, + }, + { + action: rules.ActionTypeAudit, + targets: targetVolumes, + expressions: []string{"audit/volumes/.*"}, + }, + { + action: rules.ActionTypeAllow, + targets: targetContainers, + expressions: []string{"policy/.*"}, }, }) }) @@ -357,15 +524,43 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( }, { action: rules.ActionTypeDeny, - expressions: []string{"harbor/customer/.*"}, + targets: targetContainers, + expressions: []string{"harbor/customer/containers/.*"}, }, { - action: rules.ActionTypeAllow, - expressions: []string{"harbor/customer/prod-image/.*"}, + action: rules.ActionTypeDeny, + targets: targetInitContainers, + expressions: []string{"harbor/customer/init/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetEphemeralContainers, + expressions: []string{"harbor/customer/debug/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetVolumes, + expressions: []string{"harbor/customer/volume/.*"}, }, { action: rules.ActionTypeAudit, - expressions: []string{"audit/.*"}, + targets: targetContainers, + expressions: []string{"audit/containers/.*"}, + }, + { + action: rules.ActionTypeAudit, + targets: targetVolumes, + expressions: []string{"audit/volumes/.*"}, + }, + { + action: rules.ActionTypeAllow, + targets: targetContainers, + expressions: []string{"policy/.*"}, + }, + { + action: rules.ActionTypeAllow, + targets: targetContainers, + expressions: []string{"harbor/customer/containers/prod/.*"}, }, }) }) @@ -386,14 +581,42 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( }, { action: rules.ActionTypeDeny, - expressions: []string{"harbor/customer/.*"}, - }, - { - action: rules.ActionTypeAudit, - expressions: []string{"audit/.*"}, + targets: targetContainers, + expressions: []string{"harbor/customer/containers/.*"}, }, { action: rules.ActionTypeDeny, + targets: targetInitContainers, + expressions: []string{"harbor/customer/init/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetEphemeralContainers, + expressions: []string{"harbor/customer/debug/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetVolumes, + expressions: []string{"harbor/customer/volume/.*"}, + }, + { + action: rules.ActionTypeAudit, + targets: targetContainers, + expressions: []string{"audit/containers/.*"}, + }, + { + action: rules.ActionTypeAudit, + targets: targetVolumes, + expressions: []string{"audit/volumes/.*"}, + }, + { + action: rules.ActionTypeAllow, + targets: targetContainers, + expressions: []string{"policy/.*"}, + }, + { + action: rules.ActionTypeDeny, + targets: targetContainers, expressions: []string{"trusted/.*"}, negated: []bool{true}, }, @@ -425,13 +648,13 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) - pod := restrictedPod("customer-denied", "harbor/customer/app:1", corev1.PullIfNotPresent) + pod := restrictedPod("customer-denied", "harbor/customer/containers/app:1", corev1.PullIfNotPresent) createPodAndExpectDenied(cs, ns.Name, pod, "containers[0]", - "harbor/customer/app:1", + "harbor/customer/containers/app:1", "denied", - "harbor/customer/.*", + "harbor/customer/containers/.*", ) }) @@ -450,12 +673,12 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( createPodAndExpectAllowed(cs, ns.Name, pod) updatePodAndExpectDenied(cs, ns.Name, pod.Name, func(pod *corev1.Pod) { - pod.Spec.Containers[0].Image = "harbor/customer/adad:1" + pod.Spec.Containers[0].Image = "harbor/customer/containers/app:1" }, "containers[0]", - "harbor/customer/adad:1", + "harbor/customer/containers/app:1", "denied", - "harbor/customer/.*", + "harbor/customer/containers/.*", ) }) @@ -470,15 +693,15 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) - denied := restrictedPod("prod-customer-denied", "harbor/customer/other-image/app:1", corev1.PullIfNotPresent) + denied := restrictedPod("prod-customer-denied", "harbor/customer/containers/other/app:1", corev1.PullIfNotPresent) createPodAndExpectDenied(cs, ns.Name, denied, "containers[0]", - "harbor/customer/other-image/app:1", + "harbor/customer/containers/other/app:1", "denied", - "harbor/customer/.*", + "harbor/customer/containers/.*", ) - allowed := restrictedPod("prod-customer-allowed", "harbor/customer/prod-image/app:1", corev1.PullIfNotPresent) + allowed := restrictedPod("prod-customer-allowed", "harbor/customer/containers/prod/app:1", corev1.PullIfNotPresent) createPodAndExpectAllowed(cs, ns.Name, allowed) }) @@ -492,13 +715,13 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) - pod := restrictedPod("audit-allowed", "audit/team/app:1", corev1.PullIfNotPresent) + pod := restrictedPod("audit-allowed", "audit/containers/team/app:1", corev1.PullIfNotPresent) createPodAndExpectAllowed(cs, ns.Name, pod) expectAuditEvent(cs, ns.Name, pod.Name, "matched audit registry rule", - "audit/.*", + "audit/containers/.*", ) }) @@ -521,7 +744,7 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( InitContainers: []corev1.Container{ { Name: "init", - Image: "harbor/customer/init:1", + Image: "harbor/customer/init/app:1", ImagePullPolicy: corev1.PullIfNotPresent, SecurityContext: restrictedContainerSecurityContext(), }, @@ -539,9 +762,9 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( createPodAndExpectDenied(cs, ns.Name, pod, "initContainers[0]", - "harbor/customer/init:1", + "harbor/customer/init/app:1", "denied", - "harbor/customer/.*", + "harbor/customer/init/.*", ) }) @@ -574,7 +797,7 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( Name: "imgvol", VolumeSource: corev1.VolumeSource{ Image: &corev1.ImageVolumeSource{ - Reference: "harbor/customer/volume:1", + Reference: "harbor/customer/volume/app:1", PullPolicy: corev1.PullIfNotPresent, }, }, @@ -585,9 +808,55 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( createPodAndExpectDenied(cs, ns.Name, pod, "volumes[0](imgvol)", - "harbor/customer/volume:1", + "harbor/customer/volume/app:1", "denied", - "harbor/customer/.*", + "harbor/customer/volume/.*", + ) + }) + + It("audits image volumes independently from container decisions", Label("skip-on-openshift"), func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "volume-audit-allowed", + }, + Spec: corev1.PodSpec{ + SecurityContext: nobodyPodSecurityContext(), + Containers: []corev1.Container{ + { + Name: "c", + Image: "harbor/platform/app:1", + ImagePullPolicy: corev1.PullIfNotPresent, + SecurityContext: restrictedContainerSecurityContext(), + }, + }, + Volumes: []corev1.Volume{ + { + Name: "imgvol", + VolumeSource: corev1.VolumeSource{ + Image: &corev1.ImageVolumeSource{ + Reference: "audit/volumes/team/app:1", + PullPolicy: corev1.PullIfNotPresent, + }, + }, + }, + }, + }, + } + + createPodAndExpectAllowed(cs, ns.Name, pod) + + expectAuditEvent(cs, ns.Name, pod.Name, + "matched audit registry rule", + "audit/volumes/.*", ) }) @@ -610,7 +879,7 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( ephemeral := corev1.EphemeralContainer{ EphemeralContainerCommon: corev1.EphemeralContainerCommon{ Name: "debug", - Image: "harbor/customer/debug:1", + Image: "harbor/customer/debug/app:1", ImagePullPolicy: corev1.PullIfNotPresent, SecurityContext: restrictedContainerSecurityContext(), }, @@ -622,7 +891,7 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( return err } - current.Spec.EphemeralContainers = append(current.Spec.EphemeralContainers, ephemeral) + current.Spec.EphemeralContainers = []corev1.EphemeralContainer{ephemeral} _, err = cs.CoreV1().Pods(ns.Name).UpdateEphemeralContainers( context.Background(), @@ -631,15 +900,15 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( metav1.UpdateOptions{}, ) if err == nil { - return fmt.Errorf("expected UpdateEphemeralContainers to be denied, but it succeeded") + return fmt.Errorf("expected ephemeral container update to be denied, but it succeeded") } msg := err.Error() for _, substring := range []string{ "ephemeralContainers[0]", - "harbor/customer/debug:1", + "harbor/customer/debug/app:1", "denied", - "harbor/customer/.*", + "harbor/customer/debug/.*", } { if !strings.Contains(msg, substring) { return fmt.Errorf("expected error to contain %q, got: %s", substring, msg) @@ -649,4 +918,42 @@ var _ = Describe("enforcing container registry namespace rules", Ordered, Label( return nil }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) }) + + It("allows an allowed registry reference when its pull policy is permitted", func() { + ns := NewNamespace("", map[string]string{ + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + pod := restrictedPod("policy-allowed", "policy/team/app:1", corev1.PullNever) + + createPodAndExpectAllowed(cs, ns.Name, pod) + }) + + It("applies namespace-selector matched negated regex rules after the base rules", func() { + ns := NewNamespace("", map[string]string{ + "negate": "true", + meta.TenantLabel: tnt.GetName(), + }) + + cs := ownerClient(tnt.Spec.Owners[0].UserSpec) + + NamespaceCreation(ns, tnt.Spec.Owners[0].UserSpec, defaultTimeoutInterval).Should(Succeed()) + NamespaceIsPartOfTenant(tnt, ns).Should(Succeed()) + + denied := restrictedPod("negated-denied", "harbor/platform/app:1", corev1.PullIfNotPresent) + createPodAndExpectDenied(cs, ns.Name, denied, + "containers[0]", + "harbor/platform/app:1", + "denied", + "trusted/.*", + ) + + allowed := restrictedPod("negated-allowed", "trusted/platform/app:1", corev1.PullIfNotPresent) + createPodAndExpectAllowed(cs, ns.Name, allowed) + }) }) diff --git a/e2e/utils_test.go b/e2e/utils_test.go index 1b6063d7..5a6e8bff 100644 --- a/e2e/utils_test.go +++ b/e2e/utils_test.go @@ -1143,3 +1143,62 @@ func EnsureRuntimeClass(ctx context.Context, rtc *nodev1.RuntimeClass) { return err }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) } + +const namespaceTerminationHoldFinalizer = "e2e.projectcapsule.dev/hold-termination" + +func holdNamespaceTerminating(ctx context.Context, name string) func() { + Eventually(func() error { + ns := &corev1.Namespace{} + + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name}, ns); err != nil { + return err + } + + if controllerutil.AddFinalizer(ns, namespaceTerminationHoldFinalizer) { + return k8sClient.Update(ctx, ns) + } + + return nil + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + + Eventually(func() error { + ns := &corev1.Namespace{} + + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name}, ns); err != nil { + return err + } + + if ns.DeletionTimestamp != nil { + return nil + } + + return k8sClient.Delete(ctx, ns) + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + + Eventually(func(g Gomega) { + ns := &corev1.Namespace{} + + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: name}, ns)).To(Succeed()) + g.Expect(ns.DeletionTimestamp).ToNot(BeNil()) + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + + return func() { + Eventually(func() error { + ns := &corev1.Namespace{} + + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name}, ns); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + + return err + } + + if controllerutil.RemoveFinalizer(ns, namespaceTerminationHoldFinalizer) { + return k8sClient.Update(ctx, ns) + } + + return nil + }, defaultTimeoutInterval, defaultPollInterval).Should(Succeed()) + } +} diff --git a/hack/distro/capsule/example-setup/tenants.yaml b/hack/distro/capsule/example-setup/tenants.yaml index 3328b0ad..26ef7b0b 100644 --- a/hack/distro/capsule/example-setup/tenants.yaml +++ b/hack/distro/capsule/example-setup/tenants.yaml @@ -11,7 +11,7 @@ spec: kind: User rules: - permissions: - rules: + promotions: - clusterRoles: - "configmap-replicator" - namespaceSelector: @@ -21,7 +21,7 @@ spec: values: - "test" permissions: - rules: + promotions: - clusterRoles: - "secret-replicator" - namespaceSelector: @@ -31,7 +31,7 @@ spec: values: - "prod" permissions: - rules: + promotions: - clusterRoles: - "sade-boi" permissions: @@ -89,16 +89,29 @@ spec: kind: User name: alice rules: - - enforce: - action: "deny" - registries: - - url: "harbor/.*" + - permissions: + promotions: + - clusterRoles: + - "admin" + selector: + matchLabels: + owner.projectcapsule.dev/role: "replication" + - enforce: action: "allow" - registries: - - url: "harbor/customer/.*" - policy: - - "Never" + workloads: + registries: + - exp: ".*" + - enforce: + action: "deny" + workloads: + registries: + - exp: "harbor/.*" + - enforce: + action: "deny" + workloads: + registries: + - exp: "harbor/customer/.*" - namespaceSelector: matchExpressions: - key: env @@ -107,10 +120,9 @@ spec: - "prod" enforce: action: "allow" - registries: - - url: "harbor/v2/customer-registry/prod-image/.*" - policy: - - "Always" + workloads: + registries: + - exp: "harbor/customer/prod-image/.*" --- apiVersion: capsule.clastix.io/v1beta2 kind: Tenant diff --git a/internal/cache/regex.go b/internal/cache/regex.go index c58561f1..3c3a0609 100644 --- a/internal/cache/regex.go +++ b/internal/cache/regex.go @@ -95,6 +95,10 @@ func (c *RegexCache) GetOrCompile(expr api.RegExpression) (*CompiledRegex, bool, } func (c *RegexCache) Has(id string) bool { + if c == nil { + return false + } + c.mu.RLock() defer c.mu.RUnlock() @@ -104,6 +108,10 @@ func (c *RegexCache) Has(id string) bool { } func (c *RegexCache) Stats() int { + if c == nil { + return 0 + } + c.mu.RLock() defer c.mu.RUnlock() @@ -111,6 +119,10 @@ func (c *RegexCache) Stats() int { } func (c *RegexCache) Reset() { + if c == nil { + return + } + c.mu.Lock() defer c.mu.Unlock() diff --git a/internal/cache/regex_test.go b/internal/cache/regex_test.go index b942aa1f..c25d5c2e 100644 --- a/internal/cache/regex_test.go +++ b/internal/cache/regex_test.go @@ -9,6 +9,71 @@ import ( "github.com/projectcapsule/capsule/pkg/api" ) +func TestCompiledRegexMatchString(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + expression api.RegExpression + value string + want bool + }{ + { + name: "normal expression matches matching value", + expression: api.RegExpression{ + Expression: "trusted/.*", + }, + value: "trusted/team/app:1", + want: true, + }, + { + name: "normal expression does not match non matching value", + expression: api.RegExpression{ + Expression: "trusted/.*", + }, + value: "docker.io/team/app:1", + want: false, + }, + { + name: "negated expression does not match matching value", + expression: api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }, + value: "trusted/team/app:1", + want: false, + }, + { + name: "negated expression matches non matching value", + expression: api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }, + value: "docker.io/team/app:1", + want: true, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cache := NewRegexCache() + + compiled, _, err := cache.GetOrCompile(tt.expression) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if got := compiled.MatchString(tt.value); got != tt.want { + t.Fatalf("MatchString() = %v, want %v", got, tt.want) + } + }) + } +} + func TestRegexCache_GetOrCompile(t *testing.T) { t.Parallel() diff --git a/internal/cache/registries.go b/internal/cache/registries.go index b4d25ad6..552a6809 100644 --- a/internal/cache/registries.go +++ b/internal/cache/registries.go @@ -18,19 +18,15 @@ import ( ) type RuleSet struct { - ID string - Compiled []CompiledRule - HasImages bool - HasVolumes bool + ID string + Compiled []CompiledRule } type CompiledRule struct { Expression api.RegExpression RegexID string - AllowedPolicy map[corev1.PullPolicy]struct{} // nil/empty => allow any - ValidateImages bool - ValidateVolumes bool + AllowedPolicy map[corev1.PullPolicy]struct{} // nil/empty => allow any } func (r *CompiledRule) AllowsPullPolicy(pullPolicy corev1.PullPolicy) bool { @@ -43,17 +39,6 @@ func (r *CompiledRule) AllowsPullPolicy(pullPolicy corev1.PullPolicy) bool { return ok } -func (r *CompiledRule) MatchesTarget(target rules.RegistryValidationTarget) bool { - switch target { - case rules.ValidateImages: - return r.ValidateImages - case rules.ValidateVolumes: - return r.ValidateVolumes - default: - return false - } -} - type RegistryRuleSetCache struct { regexCache *RegexCache @@ -91,7 +76,6 @@ func (c *RegistryRuleSetCache) GetOrBuild(specRules []rules.OCIRegistry) (rs *Ru return rs, true, nil } - // Build outside locks. Regex compilation is delegated to RegexCache. built, err := c.buildRuleSet(id, specRules) if err != nil { return nil, false, err @@ -113,15 +97,12 @@ func (c *RegistryRuleSetCache) GetOrBuild(specRules []rules.OCIRegistry) (rs *Ru return built, false, nil } -// Match matches a reference against target, regex and pullPolicy. -// Admission deny/allow/audit evaluation should usually use MatchReference instead, -// because it needs to distinguish "regex matched but pullPolicy is forbidden" from -// "regex did not match". +// Match matches reference, regex and pullPolicy. +// Admission evaluation should usually use MatchReference instead. func (c *RegistryRuleSetCache) Match( specRules []rules.OCIRegistry, reference string, pullPolicy corev1.PullPolicy, - target rules.RegistryValidationTarget, ) (*CompiledRule, error) { rs, _, err := c.GetOrBuild(specRules) if err != nil { @@ -132,15 +113,13 @@ func (c *RegistryRuleSetCache) Match( return nil, nil } - return c.MatchRuleSet(rs, reference, pullPolicy, target) + return c.MatchRuleSet(rs, reference, pullPolicy) } -// MatchRuleSet matches a reference against target, regex and pullPolicy. func (c *RegistryRuleSetCache) MatchRuleSet( rs *RuleSet, reference string, pullPolicy corev1.PullPolicy, - target rules.RegistryValidationTarget, ) (*CompiledRule, error) { if c == nil { return nil, fmt.Errorf("registry rule set cache is nil") @@ -157,10 +136,6 @@ func (c *RegistryRuleSetCache) MatchRuleSet( for i := range rs.Compiled { rule := &rs.Compiled[i] - if !rule.MatchesTarget(target) { - continue - } - if !rule.AllowsPullPolicy(pullPolicy) { continue } @@ -178,12 +153,11 @@ func (c *RegistryRuleSetCache) MatchRuleSet( return nil, nil } -// MatchReference matches a reference against target and regex only. +// MatchReference matches reference and regex only. // It intentionally does not check pullPolicy. func (c *RegistryRuleSetCache) MatchReference( rs *RuleSet, reference string, - target rules.RegistryValidationTarget, ) (*CompiledRule, error) { if c == nil { return nil, fmt.Errorf("registry rule set cache is nil") @@ -200,10 +174,6 @@ func (c *RegistryRuleSetCache) MatchReference( for i := range rs.Compiled { rule := &rs.Compiled[i] - if !rule.MatchesTarget(target) { - continue - } - compiled, _, err := c.regexCache.GetOrCompile(rule.Expression) if err != nil { return nil, err @@ -228,7 +198,6 @@ func (c *RegistryRuleSetCache) Stats() int { return len(c.rs) } -// activeIDs: set of ids currently referenced by RuleStatus in cluster. func (c *RegistryRuleSetCache) PruneActive(activeIDs map[string]struct{}) int { if c == nil { return 0 @@ -273,13 +242,6 @@ func (c *RegistryRuleSetCache) HashRules(specRules []rules.OCIRegistry) string { sort.Strings(policies) - validations := make([]string, 0, len(r.Validation)) - for _, v := range r.Validation { - validations = append(validations, strings.TrimSpace(string(v))) - } - - sort.Strings(validations) - b.WriteString(strings.TrimSpace(expr.Expression)) b.WriteString(sepField) @@ -299,16 +261,6 @@ func (c *RegistryRuleSetCache) HashRules(specRules []rules.OCIRegistry) string { b.WriteString(p) } - b.WriteString(sepField) - - for i, v := range validations { - if i > 0 { - b.WriteString(sepList) - } - - b.WriteString(v) - } - b.WriteString(sepRule) } @@ -317,7 +269,6 @@ func (c *RegistryRuleSetCache) HashRules(specRules []rules.OCIRegistry) string { return hex.EncodeToString(sum[:]) } -// Has is useful in tests and debugging. func (c *RegistryRuleSetCache) Has(id string) bool { if c == nil { return false @@ -342,8 +293,6 @@ func (c *RegistryRuleSetCache) Reset() { c.rs = make(map[string]*RuleSet) } -// InsertForTest can be behind a build tag if you prefer, but it is fine to keep simple. -// //nolint:unused func (c *RegistryRuleSetCache) insertForTest(id string) { c.mu.Lock() @@ -387,24 +336,6 @@ func (c *RegistryRuleSetCache) buildRuleSet(id string, specRules []rules.OCIRegi } } - if len(r.Validation) == 0 { - cr.ValidateImages = true - cr.ValidateVolumes = true - rs.HasImages = true - rs.HasVolumes = true - } else { - for _, v := range r.Validation { - switch v { - case rules.ValidateImages: - cr.ValidateImages = true - rs.HasImages = true - case rules.ValidateVolumes: - cr.ValidateVolumes = true - rs.HasVolumes = true - } - } - } - rs.Compiled = append(rs.Compiled, cr) } diff --git a/internal/cache/registries_test.go b/internal/cache/registries_test.go index 5c012cf7..b2908d83 100644 --- a/internal/cache/registries_test.go +++ b/internal/cache/registries_test.go @@ -4,6 +4,8 @@ package cache import ( + "strings" + "sync" "testing" corev1 "k8s.io/api/core/v1" @@ -12,645 +14,945 @@ import ( "github.com/projectcapsule/capsule/pkg/api/rules" ) -func TestRegistryRuleSetCache_GetOrBuild(t *testing.T) { +func TestNewRegistryRuleSetCache(t *testing.T) { + t.Parallel() + + t.Run("creates cache with default regex cache", func(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + if c == nil { + t.Fatal("expected cache, got nil") + } + + if c.regexCache == nil { + t.Fatal("expected regex cache to be initialized") + } + + if c.rs == nil { + t.Fatal("expected rule set map to be initialized") + } + + if got := c.Stats(); got != 0 { + t.Fatalf("expected empty cache, got stats=%d", got) + } + }) + + t.Run("uses provided regex cache", func(t *testing.T) { + t.Parallel() + + regexCache := NewRegexCache() + c := NewRegistryRuleSetCache(regexCache) + + if c.regexCache != regexCache { + t.Fatal("expected provided regex cache to be used") + } + }) +} + +func TestRegistryRuleSetCacheGetOrBuild(t *testing.T) { t.Parallel() tests := []struct { - name string - rules []rules.OCIRegistry - wantNil bool - wantErr bool - wantFromCache bool - wantRuleCount int - wantHasImages bool - wantHasVolumes bool + name string + rules []rules.OCIRegistry + wantNil bool + wantCache int }{ { - name: "empty rules return nil", - rules: nil, - wantNil: true, + name: "empty rules return nil ruleset", + rules: nil, + wantNil: true, + wantCache: 0, }, { - name: "build single rule", + name: "single registry builds ruleset", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - Policy: []corev1.PullPolicy{ - corev1.PullIfNotPresent, - }, - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, - }, - }, + registry("harbor/.*"), }, - wantRuleCount: 1, - wantHasImages: true, - wantHasVolumes: false, + wantNil: false, + wantCache: 1, }, { - name: "empty validation defaults to images and volumes", + name: "multiple registries build one ruleset", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - }, + registry("harbor/.*"), + registry("ghcr.io/.*"), }, - wantRuleCount: 1, - wantHasImages: true, - wantHasVolumes: true, + wantNil: false, + wantCache: 1, }, { - name: "invalid regex returns error", + name: "registry with policy builds ruleset", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `[`, - }, - }, + registryWithPolicy("harbor/.*", corev1.PullNever), }, - wantErr: true, + wantNil: false, + wantCache: 1, + }, + { + name: "registry with negated expression builds ruleset", + rules: []rules.OCIRegistry{ + registryWithExpression(api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }), + }, + wantNil: false, + wantCache: 1, }, } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { t.Parallel() - regexCache := NewRegexCache() - registryCache := NewRegistryRuleSetCache(regexCache) - - rs, fromCache, err := registryCache.GetOrBuild(tt.rules) - - if tt.wantErr { - if err == nil { - t.Fatal("expected error, got nil") - } - - if rs != nil { - t.Fatalf("expected nil ruleset on error, got %#v", rs) - } - - return - } + c := NewRegistryRuleSetCache(nil) + rs, fromCache, err := c.GetOrBuild(tt.rules) if err != nil { t.Fatalf("expected no error, got %v", err) } + if fromCache { + t.Fatal("expected first build to not come from cache") + } + if tt.wantNil { if rs != nil { t.Fatalf("expected nil ruleset, got %#v", rs) } - - return - } - - if rs == nil { + } else if rs == nil { t.Fatal("expected ruleset, got nil") } - if fromCache != tt.wantFromCache { - t.Fatalf("expected fromCache=%t, got %t", tt.wantFromCache, fromCache) - } - - if len(rs.Compiled) != tt.wantRuleCount { - t.Fatalf("expected %d compiled rules, got %d", tt.wantRuleCount, len(rs.Compiled)) - } - - if rs.HasImages != tt.wantHasImages { - t.Fatalf("expected HasImages=%t, got %t", tt.wantHasImages, rs.HasImages) - } - - if rs.HasVolumes != tt.wantHasVolumes { - t.Fatalf("expected HasVolumes=%t, got %t", tt.wantHasVolumes, rs.HasVolumes) - } - - if got := registryCache.Stats(); got != 1 { - t.Fatalf("expected 1 registry ruleset cache entry, got %d", got) - } - - if got := regexCache.Stats(); got != tt.wantRuleCount { - t.Fatalf("expected %d regex cache entries, got %d", tt.wantRuleCount, got) + if got := c.Stats(); got != tt.wantCache { + t.Fatalf("expected cache stats=%d, got %d", tt.wantCache, got) } }) } } -func TestRegistryRuleSetCache_GetOrBuild_ReusesCachedRuleSet(t *testing.T) { +func TestRegistryRuleSetCacheGetOrBuildReturnsFromCache(t *testing.T) { t.Parallel() - regexCache := NewRegexCache() - registryCache := NewRegistryRuleSetCache(regexCache) - - rules := []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - }, + c := NewRegistryRuleSetCache(nil) + specRules := []rules.OCIRegistry{ + registry("harbor/.*"), + registryWithPolicy("ghcr.io/.*", corev1.PullIfNotPresent), } - first, fromCache, err := registryCache.GetOrBuild(rules) + first, fromCache, err := c.GetOrBuild(specRules) if err != nil { t.Fatalf("expected no error, got %v", err) } if fromCache { - t.Fatal("expected first lookup to build ruleset, got cache hit") + t.Fatal("expected first call to build ruleset") } - second, fromCache, err := registryCache.GetOrBuild(rules) + second, fromCache, err := c.GetOrBuild(specRules) if err != nil { t.Fatalf("expected no error, got %v", err) } if !fromCache { - t.Fatal("expected second lookup to hit cache") + t.Fatal("expected second call to come from cache") } if first != second { - t.Fatal("expected cached ruleset pointer to be reused") + t.Fatal("expected same ruleset pointer from cache") } - if got := registryCache.Stats(); got != 1 { - t.Fatalf("expected 1 registry ruleset cache entry, got %d", got) - } - - if got := regexCache.Stats(); got != 1 { - t.Fatalf("expected 1 regex cache entry, got %d", got) + if got := c.Stats(); got != 1 { + t.Fatalf("expected one cached ruleset, got %d", got) } } -func TestRegistryRuleSetCache_Match(t *testing.T) { +func TestRegistryRuleSetCacheGetOrBuildConcurrent(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + specRules := []rules.OCIRegistry{ + registry("harbor/.*"), + registryWithPolicy("ghcr.io/.*", corev1.PullIfNotPresent), + } + + const workers = 32 + + var wg sync.WaitGroup + errs := make(chan error, workers) + results := make(chan *RuleSet, workers) + + for i := 0; i < workers; i++ { + wg.Add(1) + + go func() { + defer wg.Done() + + rs, _, err := c.GetOrBuild(specRules) + if err != nil { + errs <- err + + return + } + + results <- rs + }() + } + + wg.Wait() + close(errs) + close(results) + + for err := range errs { + t.Fatalf("unexpected error: %v", err) + } + + var first *RuleSet + for rs := range results { + if rs == nil { + t.Fatal("expected ruleset, got nil") + } + + if first == nil { + first = rs + + continue + } + + if rs != first { + t.Fatal("expected all goroutines to receive the same cached ruleset") + } + } + + if got := c.Stats(); got != 1 { + t.Fatalf("expected one cached ruleset, got %d", got) + } +} + +func TestRegistryRuleSetCacheBuildRuleSet(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + specRules := []rules.OCIRegistry{ + registry("harbor/.*"), + registryWithPolicy("ghcr.io/.*", corev1.PullAlways, corev1.PullIfNotPresent), + registryWithExpression(api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }), + } + + id := c.HashRules(specRules) + + rs, err := c.buildRuleSet(id, specRules) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if rs == nil { + t.Fatal("expected ruleset, got nil") + } + + if rs.ID != id { + t.Fatalf("expected ruleset ID %q, got %q", id, rs.ID) + } + + if len(rs.Compiled) != len(specRules) { + t.Fatalf("expected %d compiled rules, got %d", len(specRules), len(rs.Compiled)) + } + + if rs.Compiled[0].Expression.Expression != "harbor/.*" { + t.Fatalf("expected first expression harbor/.*, got %q", rs.Compiled[0].Expression.Expression) + } + + if len(rs.Compiled[0].AllowedPolicy) != 0 { + t.Fatal("expected first rule to allow any pull policy") + } + + if rs.Compiled[1].Expression.Expression != "ghcr.io/.*" { + t.Fatalf("expected second expression ghcr.io/.*, got %q", rs.Compiled[1].Expression.Expression) + } + + if len(rs.Compiled[1].AllowedPolicy) != 2 { + t.Fatalf("expected two allowed pull policies, got %d", len(rs.Compiled[1].AllowedPolicy)) + } + + if _, ok := rs.Compiled[1].AllowedPolicy[corev1.PullAlways]; !ok { + t.Fatal("expected PullAlways to be allowed") + } + + if _, ok := rs.Compiled[1].AllowedPolicy[corev1.PullIfNotPresent]; !ok { + t.Fatal("expected PullIfNotPresent to be allowed") + } + + if !rs.Compiled[2].Expression.Negate { + t.Fatal("expected third expression to be negated") + } +} + +func TestRegistryRuleSetCacheBuildRuleSetInvalidRegex(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + _, _, err := c.GetOrBuild([]rules.OCIRegistry{ + registry("["), + }) + if err == nil { + t.Fatal("expected invalid regex error, got nil") + } + + if !strings.Contains(err.Error(), "error parsing regexp") { + t.Fatalf("expected regexp parse error, got %v", err) + } +} + +func TestRegistryRuleSetCacheMatchReference(t *testing.T) { t.Parallel() tests := []struct { - name string - rules []rules.OCIRegistry - image string - pullPolicy corev1.PullPolicy - target rules.RegistryValidationTarget - wantMatch bool - wantErr bool + name string + rules []rules.OCIRegistry + reference string + wantMatch bool + wantExpr string }{ { - name: "match image with default validation and any pull policy", + name: "matches first regex", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - }, + registry("harbor/.*"), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: true, + reference: "harbor/team/app:1", + wantMatch: true, + wantExpr: "harbor/.*", }, { - name: "match volume with default validation", + name: "does not match unmatched regex", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - }, + registry("harbor/.*"), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateVolumes, - wantMatch: true, + reference: "ghcr.io/team/app:1", + wantMatch: false, }, { - name: "does not match wrong image", + name: "returns first matching compiled rule", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - }, + registry("harbor/.*"), + registry("harbor/customer/.*"), }, - image: "docker.io/library/nginx:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: false, + reference: "harbor/customer/app:1", + wantMatch: true, + wantExpr: "harbor/.*", }, { - name: "does not match wrong pull policy", + name: "matches later regex when first does not match", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - Policy: []corev1.PullPolicy{ - corev1.PullAlways, - }, - }, + registry("ghcr.io/.*"), + registry("harbor/customer/.*"), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: false, + reference: "harbor/customer/app:1", + wantMatch: true, + wantExpr: "harbor/customer/.*", }, { - name: "matches allowed pull policy", + name: "negated expression matches non-matching reference", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - Policy: []corev1.PullPolicy{ - corev1.PullIfNotPresent, - }, - }, + registryWithExpression(api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: true, + // regexp.MatchString is not anchored by default. Do not use + // "untrusted/..." here, because it contains "trusted/..." as a substring. + reference: "docker.io/team/app:1", + wantMatch: true, + wantExpr: "trusted/.*", }, { - name: "does not match wrong validation target", + name: "negated expression does not match matching reference", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - Validation: []rules.RegistryValidationTarget{ - rules.ValidateVolumes, - }, - }, + registryWithExpression(api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: false, + reference: "trusted/team/app:1", + wantMatch: false, }, { - name: "matches configured validation target", + name: "policy does not affect MatchReference", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, - }, - }, + registryWithPolicy("harbor/.*", corev1.PullNever), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: true, + reference: "harbor/team/app:1", + wantMatch: true, + wantExpr: "harbor/.*", }, { - name: "negated regex matches non matching image", + name: "legacy url fallback is used as expression", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - Negate: true, - }, - }, + registry("legacy/.*"), }, - image: "docker.io/library/nginx:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: true, + reference: "legacy/team/app:1", + wantMatch: true, + wantExpr: "legacy/.*", }, { - name: "negated regex does not match matching image", + name: "nested regex expression wins over legacy url", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - Negate: true, - }, - }, + registryWithExpression(api.RegExpression{ + Expression: "nested/.*", + }), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: false, + reference: "nested/team/app:1", + wantMatch: true, + wantExpr: "nested/.*", }, { - name: "invalid regex returns error", + name: "legacy url is ignored when nested regex expression is set", rules: []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `[`, - }, - }, + registryWithExpression(api.RegExpression{ + Expression: "nested/.*", + }), }, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantErr: true, - }, - { - name: "empty rules do not match", - rules: nil, - image: "ghcr.io/projectcapsule/capsule:latest", - pullPolicy: corev1.PullIfNotPresent, - target: rules.ValidateImages, - wantMatch: false, + reference: "legacy/team/app:1", + wantMatch: false, }, } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { t.Parallel() - registryCache := NewRegistryRuleSetCache(NewRegexCache()) + c := NewRegistryRuleSetCache(nil) - matched, err := registryCache.Match( - tt.rules, - tt.image, - tt.pullPolicy, - tt.target, - ) + rs, _, err := c.GetOrBuild(tt.rules) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } - if tt.wantErr { - if err == nil { - t.Fatal("expected error, got nil") + got, err := c.MatchReference(rs, tt.reference) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if tt.wantMatch { + if got == nil { + t.Fatal("expected match, got nil") + } + + if got.Expression.Expression != tt.wantExpr { + t.Fatalf("expected expression %q, got %q", tt.wantExpr, got.Expression.Expression) } return } - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - if tt.wantMatch && matched == nil { - t.Fatal("expected match, got nil") - } - - if !tt.wantMatch && matched != nil { - t.Fatalf("expected no match, got %#v", matched) + if got != nil { + t.Fatalf("expected no match, got %#v", got) } }) } } -func TestRegistryRuleSetCache_HashRules_NormalizesPolicyAndValidationOrder(t *testing.T) { - t.Parallel() - - c := NewRegistryRuleSetCache(NewRegexCache()) - - a := []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - Policy: []corev1.PullPolicy{ - corev1.PullAlways, - corev1.PullIfNotPresent, - }, - Validation: []rules.RegistryValidationTarget{ - rules.ValidateImages, - rules.ValidateVolumes, - }, - }, - } - - b := []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - Policy: []corev1.PullPolicy{ - corev1.PullIfNotPresent, - corev1.PullAlways, - }, - Validation: []rules.RegistryValidationTarget{ - rules.ValidateVolumes, - rules.ValidateImages, - }, - }, - } - - if c.HashRules(a) != c.HashRules(b) { - t.Fatal("expected equal hashes when policy and validation values only differ by order") - } -} - -func TestRegistryRuleSetCache_HashRules_UsesNegate(t *testing.T) { - t.Parallel() - - c := NewRegistryRuleSetCache(NewRegexCache()) - - positive := []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/.*`, - }, - }, - } - - negative := []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/.*`, - Negate: true, - }, - }, - } - - if c.HashRules(positive) == c.HashRules(negative) { - t.Fatal("expected different hashes for negated and non-negated registry expressions") - } -} - -func TestRegistryRuleSetCache_PruneActive(t *testing.T) { - t.Parallel() - - registryCache := NewRegistryRuleSetCache(NewRegexCache()) - - keepRules := []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - }, - } - - removeRules := []rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^docker\.io/library/.*`, - }, - }, - } - - keep, _, err := registryCache.GetOrBuild(keepRules) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - remove, _, err := registryCache.GetOrBuild(removeRules) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - if got := registryCache.Stats(); got != 2 { - t.Fatalf("expected 2 cache entries before prune, got %d", got) - } - - removed := registryCache.PruneActive(map[string]struct{}{ - keep.ID: {}, - }) - - if removed != 1 { - t.Fatalf("expected 1 pruned cache entry, got %d", removed) - } - - if !registryCache.Has(keep.ID) { - t.Fatalf("expected kept ruleset id %q to remain", keep.ID) - } - - if registryCache.Has(remove.ID) { - t.Fatalf("expected removed ruleset id %q to be pruned", remove.ID) - } - - if got := registryCache.Stats(); got != 1 { - t.Fatalf("expected 1 cache entry after prune, got %d", got) - } -} - -func TestRegistryRuleSetCache_Reset(t *testing.T) { - t.Parallel() - - registryCache := NewRegistryRuleSetCache(NewRegexCache()) - - rs, _, err := registryCache.GetOrBuild([]rules.OCIRegistry{ - { - RegExpression: api.RegExpression{ - Expression: `^ghcr\.io/projectcapsule/.*`, - }, - }, - }) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - if got := registryCache.Stats(); got != 1 { - t.Fatalf("expected 1 cache entry, got %d", got) - } - - registryCache.Reset() - - if got := registryCache.Stats(); got != 0 { - t.Fatalf("expected 0 cache entries after reset, got %d", got) - } - - if registryCache.Has(rs.ID) { - t.Fatalf("expected ruleset id %q to be removed after reset", rs.ID) - } -} - -func TestCompiledRule_AllowsPullPolicy(t *testing.T) { +func TestRegistryRuleSetCacheMatchRuleSetWithPullPolicy(t *testing.T) { t.Parallel() tests := []struct { name string - rule CompiledRule + rules []rules.OCIRegistry + reference string pullPolicy corev1.PullPolicy - want bool + wantMatch bool + wantExpr string }{ { - name: "empty policy allows any", - rule: CompiledRule{}, + name: "matches when no policy is configured", + rules: []rules.OCIRegistry{ + registry("harbor/.*"), + }, + reference: "harbor/team/app:1", pullPolicy: corev1.PullAlways, - want: true, + wantMatch: true, + wantExpr: "harbor/.*", }, { - name: "configured policy allows matching value", - rule: CompiledRule{ - AllowedPolicy: map[corev1.PullPolicy]struct{}{ - corev1.PullIfNotPresent: {}, - }, + name: "matches allowed pull policy", + rules: []rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullNever), }, + reference: "harbor/team/app:1", + pullPolicy: corev1.PullNever, + wantMatch: true, + wantExpr: "harbor/.*", + }, + { + name: "does not match forbidden pull policy", + rules: []rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullNever), + }, + reference: "harbor/team/app:1", pullPolicy: corev1.PullIfNotPresent, - want: true, + wantMatch: false, }, { - name: "configured policy rejects non matching value", - rule: CompiledRule{ - AllowedPolicy: map[corev1.PullPolicy]struct{}{ - corev1.PullIfNotPresent: {}, - }, + name: "does not match empty pull policy when policy is configured", + rules: []rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullNever), }, - pullPolicy: corev1.PullAlways, - want: false, + reference: "harbor/team/app:1", + pullPolicy: "", + wantMatch: false, + }, + { + name: "later rule can match when earlier policy rejects", + rules: []rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullNever), + registryWithPolicy("harbor/customer/.*", corev1.PullIfNotPresent), + }, + reference: "harbor/customer/app:1", + pullPolicy: corev1.PullIfNotPresent, + wantMatch: true, + wantExpr: "harbor/customer/.*", + }, + { + name: "negated expression respects pull policy", + rules: []rules.OCIRegistry{ + registryWithExpressionAndPolicy(api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }, corev1.PullIfNotPresent), + }, + // regexp.MatchString is not anchored by default. Do not use + // "untrusted/..." here, because it contains "trusted/..." as a substring. + reference: "docker.io/team/app:1", + pullPolicy: corev1.PullIfNotPresent, + wantMatch: true, + wantExpr: "trusted/.*", + }, + { + name: "negated expression still rejects forbidden pull policy", + rules: []rules.OCIRegistry{ + registryWithExpressionAndPolicy(api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }, corev1.PullNever), + }, + // regexp.MatchString is not anchored by default. Do not use + // "untrusted/..." here, because it contains "trusted/..." as a substring. + reference: "docker.io/team/app:1", + pullPolicy: corev1.PullIfNotPresent, + wantMatch: false, }, } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := tt.rule.AllowsPullPolicy(tt.pullPolicy); got != tt.want { - t.Fatalf("expected %t, got %t", tt.want, got) + c := NewRegistryRuleSetCache(nil) + + rs, _, err := c.GetOrBuild(tt.rules) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + got, err := c.MatchRuleSet(rs, tt.reference, tt.pullPolicy) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if tt.wantMatch { + if got == nil { + t.Fatal("expected match, got nil") + } + + if got.Expression.Expression != tt.wantExpr { + t.Fatalf("expected expression %q, got %q", tt.wantExpr, got.Expression.Expression) + } + + return + } + + if got != nil { + t.Fatalf("expected no match, got %#v", got) } }) } } -func TestCompiledRule_MatchesTarget(t *testing.T) { +func TestRegistryRuleSetCacheMatch(t *testing.T) { t.Parallel() - tests := []struct { - name string - rule CompiledRule - target rules.RegistryValidationTarget - want bool - }{ - { - name: "matches images", - rule: CompiledRule{ - ValidateImages: true, - }, - target: rules.ValidateImages, - want: true, - }, - { - name: "does not match images when only volumes configured", - rule: CompiledRule{ - ValidateVolumes: true, - }, - target: rules.ValidateImages, - want: false, - }, - { - name: "matches volumes", - rule: CompiledRule{ - ValidateVolumes: true, - }, - target: rules.ValidateVolumes, - want: true, - }, - { - name: "does not match unknown target", - rule: CompiledRule{ - ValidateImages: true, - ValidateVolumes: true, - }, - target: rules.RegistryValidationTarget("unknown"), - want: false, + c := NewRegistryRuleSetCache(nil) + + got, err := c.Match( + []rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullIfNotPresent), }, + "harbor/team/app:1", + corev1.PullIfNotPresent, + ) + if err != nil { + t.Fatalf("expected no error, got %v", err) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() + if got == nil { + t.Fatal("expected match, got nil") + } - if got := tt.rule.MatchesTarget(tt.target); got != tt.want { - t.Fatalf("expected %t, got %t", tt.want, got) - } + if got.Expression.Expression != "harbor/.*" { + t.Fatalf("expected harbor/.*, got %q", got.Expression.Expression) + } +} + +func TestCompiledRuleAllowsPullPolicy(t *testing.T) { + t.Parallel() + + t.Run("nil policy allows any pull policy", func(t *testing.T) { + t.Parallel() + + rule := &CompiledRule{} + + if !rule.AllowsPullPolicy(corev1.PullAlways) { + t.Fatal("expected PullAlways to be allowed") + } + + if !rule.AllowsPullPolicy(corev1.PullIfNotPresent) { + t.Fatal("expected PullIfNotPresent to be allowed") + } + + if !rule.AllowsPullPolicy(corev1.PullNever) { + t.Fatal("expected PullNever to be allowed") + } + + if !rule.AllowsPullPolicy("") { + t.Fatal("expected empty pull policy to be allowed when no policy is configured") + } + }) + + t.Run("configured policy allows only configured values", func(t *testing.T) { + t.Parallel() + + rule := &CompiledRule{ + AllowedPolicy: map[corev1.PullPolicy]struct{}{ + corev1.PullNever: {}, + }, + } + + if !rule.AllowsPullPolicy(corev1.PullNever) { + t.Fatal("expected PullNever to be allowed") + } + + if rule.AllowsPullPolicy(corev1.PullIfNotPresent) { + t.Fatal("expected PullIfNotPresent to be rejected") + } + + if rule.AllowsPullPolicy("") { + t.Fatal("expected empty pull policy to be rejected") + } + }) +} + +func TestRegistryRuleSetCacheHashRules(t *testing.T) { + t.Parallel() + + t.Run("same rules produce same hash", func(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + a := []rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullNever, corev1.PullIfNotPresent), + registry("ghcr.io/.*"), + } + + b := []rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullIfNotPresent, corev1.PullNever), + registry("ghcr.io/.*"), + } + + hashA := c.HashRules(a) + hashB := c.HashRules(b) + + if hashA != hashB { + t.Fatalf("expected equal hash, got %q and %q", hashA, hashB) + } + }) + + t.Run("different expression produces different hash", func(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + hashA := c.HashRules([]rules.OCIRegistry{ + registry("harbor/.*"), }) + + hashB := c.HashRules([]rules.OCIRegistry{ + registry("ghcr.io/.*"), + }) + + if hashA == hashB { + t.Fatalf("expected different hashes, got %q", hashA) + } + }) + + t.Run("different negate value produces different hash", func(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + hashA := c.HashRules([]rules.OCIRegistry{ + registryWithExpression(api.RegExpression{ + Expression: "trusted/.*", + Negate: false, + }), + }) + + hashB := c.HashRules([]rules.OCIRegistry{ + registryWithExpression(api.RegExpression{ + Expression: "trusted/.*", + Negate: true, + }), + }) + + if hashA == hashB { + t.Fatalf("expected different hashes, got %q", hashA) + } + }) + + t.Run("different policy produces different hash", func(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + hashA := c.HashRules([]rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullNever), + }) + + hashB := c.HashRules([]rules.OCIRegistry{ + registryWithPolicy("harbor/.*", corev1.PullIfNotPresent), + }) + + if hashA == hashB { + t.Fatalf("expected different hashes, got %q", hashA) + } + }) + + t.Run("rule order affects hash", func(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + hashA := c.HashRules([]rules.OCIRegistry{ + registry("harbor/.*"), + registry("ghcr.io/.*"), + }) + + hashB := c.HashRules([]rules.OCIRegistry{ + registry("ghcr.io/.*"), + registry("harbor/.*"), + }) + + if hashA == hashB { + t.Fatalf("expected different hashes because rule order matters, got %q", hashA) + } + }) +} + +func TestRegistryRuleSetCacheHasResetPruneActive(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + specA := []rules.OCIRegistry{ + registry("harbor/.*"), + } + + specB := []rules.OCIRegistry{ + registry("ghcr.io/.*"), + } + + idA := c.HashRules(specA) + idB := c.HashRules(specB) + + if c.Has(idA) { + t.Fatal("expected cache not to have idA before build") + } + + if _, _, err := c.GetOrBuild(specA); err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if _, _, err := c.GetOrBuild(specB); err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if !c.Has(idA) { + t.Fatal("expected cache to have idA") + } + + if !c.Has(idB) { + t.Fatal("expected cache to have idB") + } + + if got := c.Stats(); got != 2 { + t.Fatalf("expected two cached rulesets, got %d", got) + } + + removed := c.PruneActive(map[string]struct{}{ + idA: {}, + }) + + if removed != 1 { + t.Fatalf("expected one removed ruleset, got %d", removed) + } + + if !c.Has(idA) { + t.Fatal("expected cache to retain idA") + } + + if c.Has(idB) { + t.Fatal("expected cache to prune idB") + } + + c.Reset() + + if got := c.Stats(); got != 0 { + t.Fatalf("expected cache to be empty after reset, got %d", got) + } +} + +func TestRegistryRuleSetCacheNilReceivers(t *testing.T) { + t.Parallel() + + var c *RegistryRuleSetCache + + if got := c.Stats(); got != 0 { + t.Fatalf("expected nil cache stats to be 0, got %d", got) + } + + if c.Has("missing") { + t.Fatal("expected nil cache Has to be false") + } + + if removed := c.PruneActive(nil); removed != 0 { + t.Fatalf("expected nil cache prune to remove 0, got %d", removed) + } + + c.Reset() + + if _, _, err := c.GetOrBuild([]rules.OCIRegistry{registry("harbor/.*")}); err == nil { + t.Fatal("expected nil cache GetOrBuild error") + } + + if _, err := c.MatchRuleSet(&RuleSet{}, "harbor/app:1", corev1.PullAlways); err == nil { + t.Fatal("expected nil cache MatchRuleSet error") + } + + if _, err := c.MatchReference(&RuleSet{}, "harbor/app:1"); err == nil { + t.Fatal("expected nil cache MatchReference error") + } +} + +func TestRegistryRuleSetCacheNilRegexCache(t *testing.T) { + t.Parallel() + + c := &RegistryRuleSetCache{ + rs: make(map[string]*RuleSet), + } + + if _, err := c.buildRuleSet("id", []rules.OCIRegistry{registry("harbor/.*")}); err == nil { + t.Fatal("expected nil regex cache build error") + } + + if _, err := c.MatchRuleSet(&RuleSet{}, "harbor/app:1", corev1.PullAlways); err == nil { + t.Fatal("expected nil regex cache MatchRuleSet error") + } + + if _, err := c.MatchReference(&RuleSet{}, "harbor/app:1"); err == nil { + t.Fatal("expected nil regex cache MatchReference error") + } +} + +func TestRegistryRuleSetCacheMatchNilRuleSet(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + got, err := c.MatchRuleSet(nil, "harbor/app:1", corev1.PullAlways) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if got != nil { + t.Fatalf("expected nil match, got %#v", got) + } + + got, err = c.MatchReference(nil, "harbor/app:1") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if got != nil { + t.Fatalf("expected nil match, got %#v", got) + } +} + +func TestRegistryRuleSetCacheInsertForTest(t *testing.T) { + t.Parallel() + + c := NewRegistryRuleSetCache(nil) + + c.insertForTest("test-id") + + if !c.Has("test-id") { + t.Fatal("expected inserted test id to exist") + } +} + +func registry(expression string) rules.OCIRegistry { + return rules.OCIRegistry{ + RegExpression: api.RegExpression{ + Expression: expression, + Negate: false, + }, + } +} + +func registryWithPolicy(expression string, policies ...corev1.PullPolicy) rules.OCIRegistry { + return rules.OCIRegistry{ + RegExpression: api.RegExpression{ + Expression: expression, + Negate: false, + }, + Policy: policies, + } +} + +func registryWithExpression(expression api.RegExpression) rules.OCIRegistry { + return rules.OCIRegistry{ + RegExpression: expression, + } +} + +func registryWithExpressionAndPolicy( + expression api.RegExpression, + policies ...corev1.PullPolicy, +) rules.OCIRegistry { + return rules.OCIRegistry{ + RegExpression: expression, + Policy: policies, } } diff --git a/internal/controllers/cfg/invalidator/regex.go b/internal/controllers/cfg/invalidator/regex.go index e3ac0373..7599eb45 100644 --- a/internal/controllers/cfg/invalidator/regex.go +++ b/internal/controllers/cfg/invalidator/regex.go @@ -68,7 +68,11 @@ func collectRegexExpressionsFromNamespaceRule( return } - for _, registry := range rule.Enforce.Registries { + if rule.Enforce == nil { + return + } + + for _, registry := range rule.Enforce.Workloads.Registries { expr := registry.RegExpression if expr.Expression == "" { continue diff --git a/internal/controllers/cfg/invalidator/registries.go b/internal/controllers/cfg/invalidator/registries.go index d4cf59bb..8f312e83 100644 --- a/internal/controllers/cfg/invalidator/registries.go +++ b/internal/controllers/cfg/invalidator/registries.go @@ -36,11 +36,19 @@ func (r *CacheInvalidator) rebuildRuleStatusRegistryCache(ctx context.Context, l item := &rsList.Items[i] for _, rule := range item.Status.Rules { - if rule == nil || len(rule.Enforce.Registries) == 0 { + if rule == nil { continue } - if _, _, err := r.RegistryCache.GetOrBuild(rule.Enforce.Registries); err != nil { + if rule.Enforce == nil { + continue + } + + if len(rule.Enforce.Workloads.Registries) == 0 { + continue + } + + if _, _, err := r.RegistryCache.GetOrBuild(rule.Enforce.Workloads.Registries); err != nil { return fmt.Errorf( "build registry cache for RuleStatus %s/%s: %w", item.Namespace, diff --git a/internal/controllers/resources/global.go b/internal/controllers/resources/global.go index 01ebc830..fdf0c934 100644 --- a/internal/controllers/resources/global.go +++ b/internal/controllers/resources/global.go @@ -130,6 +130,8 @@ func (r *globalResourceController) Reconcile(ctx context.Context, request reconc //nolint:dupl defer func() { + meta.RemoveReconcileTriggerAnnotation(tntResource) + reconcileErr := err if statusErr != nil { reconcileErr = statusErr diff --git a/internal/controllers/resources/namespaced.go b/internal/controllers/resources/namespaced.go index 039412c1..9a7a0a55 100644 --- a/internal/controllers/resources/namespaced.go +++ b/internal/controllers/resources/namespaced.go @@ -167,6 +167,8 @@ func (r *namespacedResourceController) Reconcile(ctx context.Context, request re //nolint:dupl defer func() { + meta.RemoveReconcileTriggerAnnotation(tntResource) + reconcileErr := err if statusErr != nil { reconcileErr = statusErr diff --git a/internal/controllers/rulestatus/manager.go b/internal/controllers/rulestatus/manager.go index e93703b0..ed310a18 100644 --- a/internal/controllers/rulestatus/manager.go +++ b/internal/controllers/rulestatus/manager.go @@ -148,20 +148,31 @@ func (r Manager) reconcile(ctx context.Context, instance *capsulev1beta2.RuleSta continue } - normalized := *rule - normalized.Enforce = rule.Enforce - - normalized.Enforce.Registries = append( - []rules.OCIRegistry(nil), - rule.Enforce.Registries..., - ) - - // Keep status compact: skip empty enforce blocks. - if len(normalized.Enforce.Registries) == 0 { + if rule.Enforce == nil { continue } - ruleStatus = append(ruleStatus, &normalized) + normalized := &rules.NamespaceRuleBodyNamespace{ + Enforce: &rules.NamespaceRuleEnforceBody{ + Action: rule.Enforce.Action, + Workloads: rules.NamespaceRuleEnforceWorkloadsBody{ + Targets: append( + []rules.WorkloadValidationTarget(nil), + rule.Enforce.Workloads.Targets..., + ), + Registries: append( + []rules.OCIRegistry(nil), + rule.Enforce.Workloads.Registries..., + ), + QoSClasses: append( + []corev1.PodQOSClass(nil), + rule.Enforce.Workloads.QoSClasses..., + ), + }, + }, + } + + ruleStatus = append(ruleStatus, normalized) } instance.Status.Rules = ruleStatus diff --git a/internal/webhook/pod/qos.go b/internal/webhook/pod/qos.go new file mode 100644 index 00000000..5ed2a148 --- /dev/null +++ b/internal/webhook/pod/qos.go @@ -0,0 +1,285 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package pod + +import ( + "context" + "fmt" + "net/http" + "slices" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/events" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" + "github.com/projectcapsule/capsule/pkg/api/rules" + ad "github.com/projectcapsule/capsule/pkg/runtime/admission" + "github.com/projectcapsule/capsule/pkg/runtime/configuration" + evt "github.com/projectcapsule/capsule/pkg/runtime/events" + "github.com/projectcapsule/capsule/pkg/runtime/handlers" + "github.com/projectcapsule/capsule/pkg/runtime/workloads" +) + +type qosHandler struct { + configuration configuration.Configuration +} + +func QoSClass(configuration configuration.Configuration) handlers.TypedHandlerWithTenantWithRuleset[*corev1.Pod] { + return &qosHandler{ + configuration: configuration, + } +} + +func (h *qosHandler) OnCreate( + _ client.Client, + _ client.Reader, + pod *corev1.Pod, + _ admission.Decoder, + recorder events.EventRecorder, + tnt *capsulev1beta2.Tenant, + ruleBlocks []*rules.NamespaceRuleBodyNamespace, +) handlers.Func { + return func(ctx context.Context, req admission.Request) *admission.Response { + return h.validate(req, pod, tnt, recorder, ruleBlocks) + } +} + +func (h *qosHandler) OnUpdate( + _ client.Client, + _ client.Reader, + _ *corev1.Pod, + pod *corev1.Pod, + _ admission.Decoder, + recorder events.EventRecorder, + tnt *capsulev1beta2.Tenant, + ruleBlocks []*rules.NamespaceRuleBodyNamespace, +) handlers.Func { + return func(ctx context.Context, req admission.Request) *admission.Response { + return h.validate(req, pod, tnt, recorder, ruleBlocks) + } +} + +func (h *qosHandler) OnDelete( + client.Client, + client.Reader, + *corev1.Pod, + admission.Decoder, + events.EventRecorder, + *capsulev1beta2.Tenant, + []*rules.NamespaceRuleBodyNamespace, +) handlers.Func { + return func(context.Context, admission.Request) *admission.Response { + return nil + } +} + +func (h *qosHandler) validate( + req admission.Request, + pod *corev1.Pod, + tnt *capsulev1beta2.Tenant, + recorder events.EventRecorder, + ruleBlocks []*rules.NamespaceRuleBodyNamespace, +) *admission.Response { + if pod == nil { + resp := admission.Errored(http.StatusInternalServerError, fmt.Errorf("pod is nil")) + + return &resp + } + + if len(ruleBlocks) == 0 { + return nil + } + + qosClass := workloads.GetPodQoSClass(pod) + + evaluation, err := evaluateQoSClass(ruleBlocks, qosClass) + if err != nil { + resp := admission.Errored(http.StatusInternalServerError, err) + + return &resp + } + + if evaluation == nil { + return nil + } + + warnings := make([]string, 0, len(evaluation.Audits)) + + for _, audit := range evaluation.Audits { + msg := fmt.Sprintf( + "pod %q uses QoS class %q and matched audit QoS rule", + pod.Name, + qosClass, + ) + + h.auditWithEvent(recorder, tnt, pod, msg) + warnings = append(warnings, msg) + + _ = audit + } + + if evaluation.Decision == nil { + if len(warnings) > 0 { + resp := admission.Allowed("QoS class audited") + resp.Warnings = append(resp.Warnings, warnings...) + + return &resp + } + + return nil + } + + switch evaluation.Decision.Action { + case rules.ActionTypeAllow: + if len(warnings) > 0 { + resp := admission.Allowed("QoS class allowed with warnings") + resp.Warnings = append(resp.Warnings, warnings...) + + return &resp + } + + return nil + + case rules.ActionTypeDeny: + msg := fmt.Sprintf( + "pod %q uses QoS class %q which is denied by namespace rule", + pod.Name, + qosClass, + ) + + return h.denyWithEvent( + recorder, + tnt, + pod, + evt.ReasonForbiddenPodQoSClass, + msg, + ) + + case rules.ActionTypeAudit: + msg := fmt.Sprintf( + "pod %q uses QoS class %q and matched audit QoS rule", + pod.Name, + qosClass, + ) + + h.auditWithEvent(recorder, tnt, pod, msg) + + resp := admission.Allowed("QoS class audited") + resp.Warnings = append(resp.Warnings, append(warnings, msg)...) + + return &resp + + default: + resp := admission.Errored( + http.StatusInternalServerError, + fmt.Errorf("unsupported namespace rule action %q", evaluation.Decision.Action), + ) + + return &resp + } +} + +type qosDecision struct { + Action rules.ActionType + Rule *rules.NamespaceRuleBodyNamespace + Class corev1.PodQOSClass +} + +type qosEvaluation struct { + Decision *qosDecision + Audits []*qosDecision +} + +func evaluateQoSClass( + ruleBlocks []*rules.NamespaceRuleBodyNamespace, + qosClass corev1.PodQOSClass, +) (*qosEvaluation, error) { + evaluation := &qosEvaluation{} + + for _, rule := range ruleBlocks { + if rule == nil || rule.Enforce == nil { + continue + } + + if len(rule.Enforce.Workloads.QoSClasses) == 0 { + continue + } + + if !rule.Enforce.WorkloadTargetsAny( + rules.ValidateInitContainers, + rules.ValidateEphemeralContainers, + rules.ValidateContainers, + rules.ValidateVolumes, + ) { + continue + } + + if !qosClassMatches(rule.Enforce.Workloads.QoSClasses, qosClass) { + continue + } + + action := rule.Enforce.Action.OrDefault() + + decision := &qosDecision{ + Action: action, + Rule: rule, + Class: qosClass, + } + + switch action { + case rules.ActionTypeAllow, rules.ActionTypeDeny: + // Last matching allow/deny wins. + evaluation.Decision = decision + + case rules.ActionTypeAudit: + evaluation.Audits = append(evaluation.Audits, decision) + + default: + return nil, fmt.Errorf("unsupported namespace rule action %q", action) + } + } + + return evaluation, nil +} + +func qosClassMatches(classes []corev1.PodQOSClass, got corev1.PodQOSClass) bool { + return slices.Contains(classes, got) +} + +func (h *qosHandler) auditWithEvent( + recorder events.EventRecorder, + tnt *capsulev1beta2.Tenant, + pod *corev1.Pod, + msg string, +) { + recorder.Eventf( + pod, + tnt, + corev1.EventTypeWarning, + evt.ReasonForbiddenPodQoSClass, + evt.ActionValidationDenied, + msg, + ) +} + +func (h *qosHandler) denyWithEvent( + recorder events.EventRecorder, + tnt *capsulev1beta2.Tenant, + pod *corev1.Pod, + reason string, + msg string, +) *admission.Response { + recorder.Eventf( + pod, + tnt, + corev1.EventTypeWarning, + reason, + evt.ActionValidationDenied, + msg, + ) + + return ad.Deny(msg) +} diff --git a/internal/webhook/pod/registry.go b/internal/webhook/pod/registry.go index cff1c7ad..1e05100d 100644 --- a/internal/webhook/pod/registry.go +++ b/internal/webhook/pod/registry.go @@ -30,7 +30,10 @@ type registryHandler struct { cache *cache.RegistryRuleSetCache } -func ContainerRegistry(configuration configuration.Configuration, cache *cache.RegistryRuleSetCache) handlers.TypedHandlerWithTenantWithRuleset[*corev1.Pod] { +func ContainerRegistry( + configuration configuration.Configuration, + cache *cache.RegistryRuleSetCache, +) handlers.TypedHandlerWithTenantWithRuleset[*corev1.Pod] { return ®istryHandler{ configuration: configuration, cache: cache, @@ -89,7 +92,19 @@ func (h *registryHandler) validate( ruleBlocks []*rules.NamespaceRuleBodyNamespace, ) *admission.Response { if h.cache == nil { - resp := admission.Errored(http.StatusInternalServerError, fmt.Errorf("registry rule set cache is nil")) + resp := admission.Errored( + http.StatusInternalServerError, + fmt.Errorf("registry rule set cache is nil"), + ) + + return &resp + } + + if pod == nil { + resp := admission.Errored( + http.StatusInternalServerError, + fmt.Errorf("pod is nil"), + ) return &resp } @@ -107,11 +122,23 @@ func (h *registryHandler) validate( warnings := make([]string, 0) - if resp := h.validateContainers(req, pod, tnt, recorder, ruleBlocks, &warnings); resp != nil { + if resp := h.validateContainers( + recorder, + tnt, + pod, + ruleBlocks, + &warnings, + ); resp != nil { return resp } - if resp := h.validateVolumes(req, pod, tnt, recorder, ruleBlocks, &warnings); resp != nil { + if resp := h.validateVolumes( + recorder, + tnt, + pod, + ruleBlocks, + &warnings, + ); resp != nil { return resp } @@ -126,10 +153,9 @@ func (h *registryHandler) validate( } func (h *registryHandler) validateContainers( - req admission.Request, - pod *corev1.Pod, - tnt *capsulev1beta2.Tenant, recorder events.EventRecorder, + tnt *capsulev1beta2.Tenant, + pod *corev1.Pod, ruleBlocks []*rules.NamespaceRuleBodyNamespace, warnings *[]string, ) *admission.Response { @@ -138,11 +164,10 @@ func (h *registryHandler) validateContainers( if resp := h.verifyOCIReference( recorder, - req, tnt, pod, ruleBlocks, - rules.ValidateImages, + rules.ValidateInitContainers, c.Image, c.ImagePullPolicy, fmt.Sprintf("initContainers[%d]", i), @@ -152,35 +177,15 @@ func (h *registryHandler) validateContainers( } } - for i := range pod.Spec.EphemeralContainers { - c := pod.Spec.EphemeralContainers[i] - - if resp := h.verifyOCIReference( - recorder, - req, - tnt, - pod, - ruleBlocks, - rules.ValidateImages, - c.Image, - c.ImagePullPolicy, - fmt.Sprintf("ephemeralContainers[%d]", i), - warnings, - ); resp != nil { - return resp - } - } - for i := range pod.Spec.Containers { c := pod.Spec.Containers[i] if resp := h.verifyOCIReference( recorder, - req, tnt, pod, ruleBlocks, - rules.ValidateImages, + rules.ValidateContainers, c.Image, c.ImagePullPolicy, fmt.Sprintf("containers[%d]", i), @@ -190,14 +195,31 @@ func (h *registryHandler) validateContainers( } } + for i := range pod.Spec.EphemeralContainers { + c := pod.Spec.EphemeralContainers[i] + + if resp := h.verifyOCIReference( + recorder, + tnt, + pod, + ruleBlocks, + rules.ValidateEphemeralContainers, + c.Image, + c.ImagePullPolicy, + fmt.Sprintf("ephemeralContainers[%d]", i), + warnings, + ); resp != nil { + return resp + } + } + return nil } func (h *registryHandler) validateVolumes( - req admission.Request, - pod *corev1.Pod, - tnt *capsulev1beta2.Tenant, recorder events.EventRecorder, + tnt *capsulev1beta2.Tenant, + pod *corev1.Pod, ruleBlocks []*rules.NamespaceRuleBodyNamespace, warnings *[]string, ) *admission.Response { @@ -207,25 +229,13 @@ func (h *registryHandler) validateVolumes( continue } - ref := strings.TrimSpace(v.Image.Reference) - if ref == "" { - return h.denyWithEvent( - recorder, - tnt, - pod, - evt.ReasonForbiddenContainerRegistry, - fmt.Sprintf("volume %q has empty image.reference", v.Name), - ) - } - if resp := h.verifyOCIReference( recorder, - req, tnt, pod, ruleBlocks, rules.ValidateVolumes, - ref, + v.Image.Reference, v.Image.PullPolicy, fmt.Sprintf("volumes[%d](%s)", i, v.Name), warnings, @@ -239,11 +249,10 @@ func (h *registryHandler) validateVolumes( func (h *registryHandler) verifyOCIReference( recorder events.EventRecorder, - req admission.Request, tnt *capsulev1beta2.Tenant, pod *corev1.Pod, ruleBlocks []*rules.NamespaceRuleBodyNamespace, - target rules.RegistryValidationTarget, + target rules.WorkloadValidationTarget, reference string, pullPolicy corev1.PullPolicy, where string, @@ -361,17 +370,25 @@ type registryEvaluation struct { func (h *registryHandler) evaluateOCIReference( ruleBlocks []*rules.NamespaceRuleBodyNamespace, - target rules.RegistryValidationTarget, + target rules.WorkloadValidationTarget, ref string, ) (*registryEvaluation, error) { evaluation := ®istryEvaluation{} for _, rule := range ruleBlocks { - if rule == nil || len(rule.Enforce.Registries) == 0 { + if rule == nil || rule.Enforce == nil { continue } - rs, _, err := h.cache.GetOrBuild(rule.Enforce.Registries) + if len(rule.Enforce.Workloads.Registries) == 0 { + continue + } + + if !rule.Enforce.WorkloadTargetsAny(target) { + continue + } + + rs, _, err := h.cache.GetOrBuild(rule.Enforce.Workloads.Registries) if err != nil { return nil, err } @@ -380,7 +397,10 @@ func (h *registryHandler) evaluateOCIReference( continue } - matched, err := h.cache.MatchReference(rs, ref, target) + // Important: + // Match by image reference only. Pull policy is validated after the + // final allow decision has been selected. + matched, err := h.cache.MatchReference(rs, ref) if err != nil { return nil, err } @@ -389,10 +409,7 @@ func (h *registryHandler) evaluateOCIReference( continue } - action := rule.Enforce.Action - if action == "" { - action = rules.ActionTypeDeny - } + action := rule.Enforce.Action.OrDefault() decision := ®istryDecision{ RuleDecision: rules.RuleDecision{ diff --git a/internal/webhook/tenant/validation/rule_validator.go b/internal/webhook/tenant/validation/rule_validator.go index 1eba8f33..1b7048ef 100644 --- a/internal/webhook/tenant/validation/rule_validator.go +++ b/internal/webhook/tenant/validation/rule_validator.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "regexp" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/events" @@ -70,6 +71,10 @@ func (h *RuleValidationHandler) OnUpdate( } func ValidateRule(tnt *capsulev1beta2.Tenant, req admission.Request) *admission.Response { + if tnt == nil { + return nil + } + if len(tnt.Spec.Rules) == 0 { return nil } @@ -79,6 +84,15 @@ func ValidateRule(tnt *capsulev1beta2.Tenant, req admission.Request) *admission. continue } + body := rule.NamespaceRuleBodyNamespace + if body == nil { + continue + } + + if rule.Enforce == nil { + continue + } + if rule.NamespaceSelector != nil { if _, err := metav1.LabelSelectorAsSelector(rule.NamespaceSelector); err != nil { return ad.Deny( @@ -87,19 +101,19 @@ func ValidateRule(tnt *capsulev1beta2.Tenant, req admission.Request) *admission. } } - for j, registry := range rule.Enforce.Registries { + for j, registry := range rule.Enforce.Workloads.Registries { expr := registry.Expression() - if expr.Expression == "" { + if strings.TrimSpace(expr.Expression) == "" { return ad.Deny( - fmt.Sprintf("rules[%d].enforce.registries[%d].exp must not be empty", i, j), + fmt.Sprintf("rules[%d].enforce.workloads.registries[%d].exp must not be empty", i, j), ) } if _, err := regexp.Compile(expr.Expression); err != nil { return ad.Deny( fmt.Sprintf( - "rules[%d].enforce.registries[%d].exp %q is invalid: %v", + "rules[%d].enforce.workloads.registries[%d].exp %q is invalid: %v", i, j, expr.Expression, diff --git a/pkg/api/meta/annotations.go b/pkg/api/meta/annotations.go index 416a7927..9a93f60c 100644 --- a/pkg/api/meta/annotations.go +++ b/pkg/api/meta/annotations.go @@ -5,6 +5,7 @@ package meta import ( "context" + "maps" "strings" "time" @@ -75,6 +76,24 @@ func TriggerRequestReconcileAnnotation( }) } +func RemoveReconcileTriggerAnnotation( + obj client.Object, +) { + annotations := obj.GetAnnotations() + if _, ok := annotations[ReconcileAnnotation]; !ok { + return + } + + annotations = maps.Clone(annotations) + delete(annotations, ReconcileAnnotation) + + if len(annotations) == 0 { + obj.SetAnnotations(nil) + } else { + obj.SetAnnotations(annotations) + } +} + func annotationRemove(obj client.Object, anno string) { annotations := obj.GetAnnotations() diff --git a/pkg/api/processor/processor_func.go b/pkg/api/processor/processor_func.go index f53ba103..96b7a625 100644 --- a/pkg/api/processor/processor_func.go +++ b/pkg/api/processor/processor_func.go @@ -48,6 +48,8 @@ func (p *Processor) Reconcile( return true } + terminatingNamespaces := map[string]bool{} + for _, i := range *processed { if _, exists := acc[i.GetKey("")]; exists { continue @@ -137,6 +139,31 @@ func (p *Processor) Reconcile( for _, obj := range *item.Objects { fieldOwner := opts.FieldOwnerPrefix + "/" + item.Resource.FieldOwner("") + terminating, namespace, err := p.isNamespaceTerminatingForObject(ctx, obj.Object, terminatingNamespaces) + if err != nil { + hadError = true + or.Status = metav1.ConditionFalse + or.Message = "checking namespace termination failed for item " + obj.Origin.Origin + ": " + err.Error() + + processed.UpdateItem(or) + + continue + } + + if terminating { + log.V(4).Info( + "skipping apply because namespace is terminating", + "item", obj.Origin.Origin, + "namespace", namespace, + "Kind", obj.Object.GetKind(), + "Name", obj.Object.GetName(), + ) + + processed.RemoveItem(or) + + continue + } + ver, created, err := p.Apply( ctx, c, @@ -458,3 +485,81 @@ func (r *Processor) handleCreatedMetadata( existingObject.GetName(), ) } + +func (r *Processor) isNamespaceTerminatingForObject( + ctx context.Context, + obj *unstructured.Unstructured, + cache map[string]bool, +) (terminating bool, namespace string, err error) { + // The Namespace object itself is cluster-scoped, but if Capsule is applying + // a Namespace which is already terminating, we should skip it as well. + if obj.GroupVersionKind().Group == "" && obj.GetKind() == "Namespace" { + namespace = obj.GetName() + + ns := &corev1.Namespace{} + if err := r.GatherClient.Get(ctx, types.NamespacedName{Name: namespace}, ns); err != nil { + if apierrors.IsNotFound(err) { + cache[namespace] = false + + return false, namespace, nil + } + + return false, namespace, err + } + + terminating = ns.DeletionTimestamp != nil || ns.Status.Phase == corev1.NamespaceTerminating + + cache[namespace] = terminating + + return terminating, namespace, nil + } + + mapping, err := r.Mapper.RESTMapping( + obj.GroupVersionKind().GroupKind(), + obj.GroupVersionKind().Version, + ) + if err != nil { + return false, "", err + } + + if mapping.Scope.Name() != k8smeta.RESTScopeNameNamespace { + return false, "", nil + } + + namespace = obj.GetNamespace() + if namespace == "" { + return false, "", nil + } + + return r.isNamespaceTerminating(ctx, namespace, cache) +} + +func (r *Processor) isNamespaceTerminating( + ctx context.Context, + namespace string, + cache map[string]bool, +) (bool, string, error) { + if namespace == "" { + return false, namespace, nil + } + + if terminating, ok := cache[namespace]; ok { + return terminating, namespace, nil + } + + ns := &corev1.Namespace{} + if err := r.GatherClient.Get(ctx, types.NamespacedName{Name: namespace}, ns); err != nil { + if apierrors.IsNotFound(err) { + cache[namespace] = true + + return true, namespace, nil + } + + return false, namespace, err + } + + terminating := ns.DeletionTimestamp != nil || ns.Status.Phase == corev1.NamespaceTerminating + cache[namespace] = terminating + + return terminating, namespace, nil +} diff --git a/pkg/api/rules/action_type.go b/pkg/api/rules/enforce_action_types.go similarity index 100% rename from pkg/api/rules/action_type.go rename to pkg/api/rules/enforce_action_types.go diff --git a/pkg/api/rules/enforce_func.go b/pkg/api/rules/enforce_func.go new file mode 100644 index 00000000..ee36733f --- /dev/null +++ b/pkg/api/rules/enforce_func.go @@ -0,0 +1,30 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import "slices" + +func (a ActionType) OrDefault() ActionType { + if a == "" { + return ActionTypeDeny + } + + return a +} + +func (e NamespaceRuleEnforceBody) GetWorkloadTargets(target WorkloadValidationTarget) bool { + if len(e.Workloads.Targets) == 0 { + return true + } + + return slices.Contains(e.Workloads.Targets, target) +} + +func (e NamespaceRuleEnforceBody) WorkloadTargetsAny(targets ...WorkloadValidationTarget) bool { + if len(e.Workloads.Targets) == 0 { + return true + } + + return slices.ContainsFunc(targets, e.GetWorkloadTargets) +} diff --git a/pkg/api/rules/enforce_types.go b/pkg/api/rules/enforce_types.go index 3a513057..341d6521 100644 --- a/pkg/api/rules/enforce_types.go +++ b/pkg/api/rules/enforce_types.go @@ -12,7 +12,6 @@ type NamespaceRuleEnforceBody struct { //+kubebuilder:default:=deny Action ActionType `json:"action,omitempty"` - // Define registries which are allowed to be used within this tenant - // The rules are aggregated, since you can use Regular Expressions the match registry endpoints - Registries []OCIRegistry `json:"registries,omitempty"` + // Enforcement for Workloads (Pods) + Workloads NamespaceRuleEnforceWorkloadsBody `json:"workloads,omitempty"` } diff --git a/pkg/api/rules/enforce_registry_types.go b/pkg/api/rules/enforce_workloads_registry_types.go similarity index 52% rename from pkg/api/rules/enforce_registry_types.go rename to pkg/api/rules/enforce_workloads_registry_types.go index aa07c655..04daf0ca 100644 --- a/pkg/api/rules/enforce_registry_types.go +++ b/pkg/api/rules/enforce_workloads_registry_types.go @@ -16,40 +16,16 @@ func (i ImagePullPolicySpec) String() string { return string(i) } -// +kubebuilder:validation:Enum=pod/images;pod/volumes -type RegistryValidationTarget string - -const ( - ValidateImages RegistryValidationTarget = "pod/images" - ValidateVolumes RegistryValidationTarget = "pod/volumes" -) - // +kubebuilder:object:generate=true type OCIRegistry struct { api.RegExpression `json:",inline"` - // Deprecated: Use exp field - // - // OCI Registry endpoint, is treated as regular expression. - Registry string `json:"url,omitempty"` - // Allowed PullPolicy for the given registry. Supplying no value allows all policies. // +optional // +kubebuilder:validation:Items:Enum=Always;Never;IfNotPresent Policy []corev1.PullPolicy `json:"policy,omitempty"` - - // Requesting Resources - //+kubebuilder:default:={pod/images,pod/volumes} - Validation []RegistryValidationTarget `json:"validation,omitempty"` } func (r OCIRegistry) Expression() api.RegExpression { - if r.RegExpression.Expression != "" { - return r.RegExpression - } - - return api.RegExpression{ - Expression: r.Registry, - Negate: false, - } + return r.RegExpression } diff --git a/pkg/api/rules/enforce_workloads_types.go b/pkg/api/rules/enforce_workloads_types.go new file mode 100644 index 00000000..c77df7d9 --- /dev/null +++ b/pkg/api/rules/enforce_workloads_types.go @@ -0,0 +1,35 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import corev1 "k8s.io/api/core/v1" + +// +kubebuilder:validation:Enum=pod/initcontainers;pod/ephemeralcontainers;pod/containers;pod/volumes +type WorkloadValidationTarget string + +const ( + DeprecatedValidateImages WorkloadValidationTarget = "pod/images" + + ValidateInitContainers WorkloadValidationTarget = "pod/initcontainers" + ValidateEphemeralContainers WorkloadValidationTarget = "pod/ephemeralcontainers" + ValidateContainers WorkloadValidationTarget = "pod/containers" + ValidateVolumes WorkloadValidationTarget = "pod/volumes" +) + +// +kubebuilder:object:generate=true +type NamespaceRuleEnforceWorkloadsBody struct { + // Define the enforcement targets this rule applies to. + // If empty, each webhook applies its own backwards-compatible default. + // +optional + Targets []WorkloadValidationTarget `json:"targets,omitempty"` + + // Define Pod QoS classes matched by this enforcement rule. + // Supported values are Guaranteed, Burstable and BestEffort. + // +optional + QoSClasses []corev1.PodQOSClass `json:"qosClasses,omitempty"` + + // Define registries which are allowed to be used within this tenant + // The rules are aggregated, since you can use Regular Expressions the match registry endpoints + Registries []OCIRegistry `json:"registries,omitempty"` +} diff --git a/pkg/api/rules/permission_types.go b/pkg/api/rules/permission_types.go index 658048fc..f42097b5 100644 --- a/pkg/api/rules/permission_types.go +++ b/pkg/api/rules/permission_types.go @@ -11,7 +11,7 @@ import ( type NamespaceRulePermissionBody struct { // Define Promotion Rules which distributed additional ClusterRoles across the Tenant // for promoted ServiceAccounts. - Promotions []*NamespaceRulePromotionRule `json:"rules,omitempty"` + Promotions []*NamespaceRulePromotionRule `json:"promotions,omitempty"` } // +kubebuilder:object:generate=true diff --git a/pkg/api/rules/rule_body_types.go b/pkg/api/rules/rule_body_types.go index db1bdf87..fefe54d0 100644 --- a/pkg/api/rules/rule_body_types.go +++ b/pkg/api/rules/rule_body_types.go @@ -12,18 +12,18 @@ import ( type NamespaceRuleBodyNamespace struct { // Enforcement for given rule //+optional - Enforce NamespaceRuleEnforceBody `json:"enforce,omitzero"` + Enforce *NamespaceRuleEnforceBody `json:"enforce,omitzero"` } // Rules Distributed via Tenants // +kubebuilder:object:generate=true type NamespaceRuleBodyTenant struct { - NamespaceRuleBodyNamespace `json:",inline"` + *NamespaceRuleBodyNamespace `json:",inline"` // Select namespaces which are going to be targeted with this rule NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"` // Permissions for given rule //+optional - Permissions NamespaceRulePermissionBody `json:"permissions,omitzero"` + Permissions NamespaceRulePermissionBody `json:"permissions,omitempty"` } diff --git a/pkg/api/rules/zz_generated.deepcopy.go b/pkg/api/rules/zz_generated.deepcopy.go index 9b2e46cd..311f9ca8 100644 --- a/pkg/api/rules/zz_generated.deepcopy.go +++ b/pkg/api/rules/zz_generated.deepcopy.go @@ -15,7 +15,11 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NamespaceRuleBodyNamespace) DeepCopyInto(out *NamespaceRuleBodyNamespace) { *out = *in - in.Enforce.DeepCopyInto(&out.Enforce) + if in.Enforce != nil { + in, out := &in.Enforce, &out.Enforce + *out = new(NamespaceRuleEnforceBody) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceRuleBodyNamespace. @@ -31,7 +35,11 @@ func (in *NamespaceRuleBodyNamespace) DeepCopy() *NamespaceRuleBodyNamespace { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NamespaceRuleBodyTenant) DeepCopyInto(out *NamespaceRuleBodyTenant) { *out = *in - in.NamespaceRuleBodyNamespace.DeepCopyInto(&out.NamespaceRuleBodyNamespace) + if in.NamespaceRuleBodyNamespace != nil { + in, out := &in.NamespaceRuleBodyNamespace, &out.NamespaceRuleBodyNamespace + *out = new(NamespaceRuleBodyNamespace) + (*in).DeepCopyInto(*out) + } if in.NamespaceSelector != nil { in, out := &in.NamespaceSelector, &out.NamespaceSelector *out = new(metav1.LabelSelector) @@ -53,6 +61,32 @@ func (in *NamespaceRuleBodyTenant) DeepCopy() *NamespaceRuleBodyTenant { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NamespaceRuleEnforceBody) DeepCopyInto(out *NamespaceRuleEnforceBody) { *out = *in + in.Workloads.DeepCopyInto(&out.Workloads) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceRuleEnforceBody. +func (in *NamespaceRuleEnforceBody) DeepCopy() *NamespaceRuleEnforceBody { + if in == nil { + return nil + } + out := new(NamespaceRuleEnforceBody) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NamespaceRuleEnforceWorkloadsBody) DeepCopyInto(out *NamespaceRuleEnforceWorkloadsBody) { + *out = *in + if in.Targets != nil { + in, out := &in.Targets, &out.Targets + *out = make([]WorkloadValidationTarget, len(*in)) + copy(*out, *in) + } + if in.QoSClasses != nil { + in, out := &in.QoSClasses, &out.QoSClasses + *out = make([]v1.PodQOSClass, len(*in)) + copy(*out, *in) + } if in.Registries != nil { in, out := &in.Registries, &out.Registries *out = make([]OCIRegistry, len(*in)) @@ -62,12 +96,12 @@ func (in *NamespaceRuleEnforceBody) DeepCopyInto(out *NamespaceRuleEnforceBody) } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceRuleEnforceBody. -func (in *NamespaceRuleEnforceBody) DeepCopy() *NamespaceRuleEnforceBody { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespaceRuleEnforceWorkloadsBody. +func (in *NamespaceRuleEnforceWorkloadsBody) DeepCopy() *NamespaceRuleEnforceWorkloadsBody { if in == nil { return nil } - out := new(NamespaceRuleEnforceBody) + out := new(NamespaceRuleEnforceWorkloadsBody) in.DeepCopyInto(out) return out } @@ -132,11 +166,6 @@ func (in *OCIRegistry) DeepCopyInto(out *OCIRegistry) { *out = make([]v1.PullPolicy, len(*in)) copy(*out, *in) } - if in.Validation != nil { - in, out := &in.Validation, &out.Validation - *out = make([]RegistryValidationTarget, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OCIRegistry. diff --git a/pkg/runtime/events/reasons.go b/pkg/runtime/events/reasons.go index 1f6bf640..500de7c6 100644 --- a/pkg/runtime/events/reasons.go +++ b/pkg/runtime/events/reasons.go @@ -38,6 +38,7 @@ const ( ReasonMissingFQCI string = "MissingFQCI" ReasonForbiddenContainerRegistry string = "ForbiddenContainerRegistry" ReasonForbiddenPullPolicy string = "ForbiddenPullPolicy" + ReasonForbiddenPodQoSClass string = "ForbiddenPodQoSClass" // Ingress. ReasonWildcardDenied string = "WildcardDenied" diff --git a/pkg/runtime/predicates/reconcile_requested.go b/pkg/runtime/predicates/reconcile_requested.go index 9fc7f527..bf4b76e7 100644 --- a/pkg/runtime/predicates/reconcile_requested.go +++ b/pkg/runtime/predicates/reconcile_requested.go @@ -21,18 +21,19 @@ func (ReconcileRequestedPredicate) Update(e event.UpdateEvent) bool { return false } - oldA := e.ObjectOld.GetAnnotations() - newA := e.ObjectNew.GetAnnotations() + oldValue, oldPresent := e.ObjectOld.GetAnnotations()[meta.ReconcileAnnotation] + newValue, newPresent := e.ObjectNew.GetAnnotations()[meta.ReconcileAnnotation] - oldV := "" - if oldA != nil { - oldV = oldA[meta.ReconcileAnnotation] + oldPresent = oldPresent && oldValue != "" + newPresent = newPresent && newValue != "" + + if !newPresent { + return false } - newV := "" - if newA != nil { - newV = newA[meta.ReconcileAnnotation] + if !oldPresent { + return true } - return newV != "" && newV != oldV + return oldValue != newValue } diff --git a/pkg/runtime/quota/utils.go b/pkg/runtime/quota/utils.go index 9a35d8a3..6a23a226 100644 --- a/pkg/runtime/quota/utils.go +++ b/pkg/runtime/quota/utils.go @@ -34,3 +34,7 @@ func NegateQuantity(in resource.Quantity) resource.Quantity { return out } + +func QuantityEqual(a, b resource.Quantity) bool { + return a.Cmp(b) == 0 +} diff --git a/pkg/runtime/quota/utils_test.go b/pkg/runtime/quota/utils_test.go new file mode 100644 index 00000000..aa97565a --- /dev/null +++ b/pkg/runtime/quota/utils_test.go @@ -0,0 +1,333 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package quota + +import ( + "strings" + "testing" + + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestValidateQuantity(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + quantity resource.Quantity + wantErr bool + errContain string + }{ + { + name: "positive integer quantity is valid", + quantity: resource.MustParse("1"), + wantErr: false, + }, + { + name: "positive milli quantity is valid", + quantity: resource.MustParse("100m"), + wantErr: false, + }, + { + name: "positive binary memory quantity is valid", + quantity: resource.MustParse("128Mi"), + wantErr: false, + }, + { + name: "positive decimal memory quantity is valid", + quantity: resource.MustParse("1Gi"), + wantErr: false, + }, + { + name: "zero quantity is invalid", + quantity: resource.MustParse("0"), + wantErr: true, + errContain: "quantity must not be negative or 0", + }, + { + name: "negative integer quantity is invalid", + quantity: resource.MustParse("-1"), + wantErr: true, + errContain: "quantity must not be negative or 0", + }, + { + name: "negative milli quantity is invalid", + quantity: resource.MustParse("-100m"), + wantErr: true, + errContain: "quantity must not be negative or 0", + }, + { + name: "negative binary quantity is invalid", + quantity: resource.MustParse("-128Mi"), + wantErr: true, + errContain: "quantity must not be negative or 0", + }, + { + name: "very small positive milli quantity is valid", + quantity: resource.MustParse("1m"), + wantErr: false, + }, + { + name: "large positive quantity is valid", + quantity: resource.MustParse("999999999999"), + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := ValidateQuantity(tt.quantity) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + + if tt.errContain != "" && !strings.Contains(err.Error(), tt.errContain) { + t.Fatalf("expected error to contain %q, got %q", tt.errContain, err.Error()) + } + + return + } + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) + } +} + +func TestClampQuantityToZero(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in resource.Quantity + want resource.Quantity + }{ + { + name: "positive quantity is unchanged", + in: resource.MustParse("5"), + want: resource.MustParse("5"), + }, + { + name: "positive milli quantity is unchanged", + in: resource.MustParse("250m"), + want: resource.MustParse("250m"), + }, + { + name: "zero quantity is unchanged", + in: resource.MustParse("0"), + want: resource.MustParse("0"), + }, + { + name: "negative integer quantity is clamped to zero", + in: resource.MustParse("-5"), + want: resource.MustParse("0"), + }, + { + name: "negative milli quantity is clamped to zero", + in: resource.MustParse("-250m"), + want: resource.MustParse("0"), + }, + { + name: "negative memory quantity is clamped to zero", + in: resource.MustParse("-1Gi"), + want: resource.MustParse("0"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := tt.in.DeepCopy() + + ClampQuantityToZero(&got) + + if !QuantityEqual(got, tt.want) { + t.Fatalf("expected %q, got %q", tt.want.String(), got.String()) + } + }) + } +} + +func TestNegateQuantity(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in resource.Quantity + want resource.Quantity + }{ + { + name: "positive integer becomes negative", + in: resource.MustParse("5"), + want: resource.MustParse("-5"), + }, + { + name: "negative integer becomes positive", + in: resource.MustParse("-5"), + want: resource.MustParse("5"), + }, + { + name: "zero remains zero", + in: resource.MustParse("0"), + want: resource.MustParse("0"), + }, + { + name: "positive milli becomes negative", + in: resource.MustParse("250m"), + want: resource.MustParse("-250m"), + }, + { + name: "negative milli becomes positive", + in: resource.MustParse("-250m"), + want: resource.MustParse("250m"), + }, + { + name: "positive binary memory becomes negative", + in: resource.MustParse("1Gi"), + want: resource.MustParse("-1Gi"), + }, + { + name: "negative binary memory becomes positive", + in: resource.MustParse("-1Gi"), + want: resource.MustParse("1Gi"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + original := tt.in.DeepCopy() + + got := NegateQuantity(tt.in) + + if !QuantityEqual(got, tt.want) { + t.Fatalf("expected %q, got %q", tt.want.String(), got.String()) + } + + if !QuantityEqual(tt.in, original) { + t.Fatalf("NegateQuantity mutated input: expected original %q, got %q", original.String(), tt.in.String()) + } + }) + } +} + +func TestQuantityEqual(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + a resource.Quantity + b resource.Quantity + want bool + }{ + { + name: "same integer quantities are equal", + a: resource.MustParse("1"), + b: resource.MustParse("1"), + want: true, + }, + { + name: "equivalent decimal and milli quantities are equal", + a: resource.MustParse("1"), + b: resource.MustParse("1000m"), + want: true, + }, + { + name: "equivalent binary quantities are equal", + a: resource.MustParse("1Gi"), + b: resource.MustParse("1024Mi"), + want: true, + }, + { + name: "different integer quantities are not equal", + a: resource.MustParse("1"), + b: resource.MustParse("2"), + want: false, + }, + { + name: "positive and negative quantities are not equal", + a: resource.MustParse("1"), + b: resource.MustParse("-1"), + want: false, + }, + { + name: "zero quantities are equal", + a: resource.MustParse("0"), + b: resource.MustParse("0"), + want: true, + }, + { + name: "equivalent CPU quantities are equal", + a: resource.MustParse("500m"), + b: resource.MustParse("0.5"), + want: true, + }, + { + name: "nearby milli quantities are not equal", + a: resource.MustParse("500m"), + b: resource.MustParse("501m"), + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := QuantityEqual(tt.a, tt.b) + if got != tt.want { + t.Fatalf("expected %t for %q and %q, got %t", tt.want, tt.a.String(), tt.b.String(), got) + } + }) + } +} + +func TestClampQuantityToZeroMutatesOnlyNegativeInput(t *testing.T) { + t.Parallel() + + q := resource.MustParse("-10") + ClampQuantityToZero(&q) + + if !QuantityEqual(q, resource.MustParse("0")) { + t.Fatalf("expected quantity to be clamped to zero, got %q", q.String()) + } + + q = resource.MustParse("10") + ClampQuantityToZero(&q) + + if !QuantityEqual(q, resource.MustParse("10")) { + t.Fatalf("expected positive quantity to remain unchanged, got %q", q.String()) + } +} + +func TestNegateQuantityReturnsIndependentCopy(t *testing.T) { + t.Parallel() + + in := resource.MustParse("10") + out := NegateQuantity(in) + + if !QuantityEqual(in, resource.MustParse("10")) { + t.Fatalf("expected input to remain unchanged, got %q", in.String()) + } + + if !QuantityEqual(out, resource.MustParse("-10")) { + t.Fatalf("expected output to be negated, got %q", out.String()) + } + + ClampQuantityToZero(&out) + + if !QuantityEqual(out, resource.MustParse("0")) { + t.Fatalf("expected output to be clampable independently, got %q", out.String()) + } + + if !QuantityEqual(in, resource.MustParse("10")) { + t.Fatalf("expected input to remain unchanged after mutating output, got %q", in.String()) + } +} diff --git a/pkg/runtime/workloads/qos_class.go b/pkg/runtime/workloads/qos_class.go new file mode 100644 index 00000000..6d575d89 --- /dev/null +++ b/pkg/runtime/workloads/qos_class.go @@ -0,0 +1,155 @@ +// Copyright 2020-2026 Project Capsule Authors +// SPDX-License-Identifier: Apache-2.0 + +package workloads + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func GetPodQoSClass(pod *corev1.Pod) corev1.PodQOSClass { + if pod == nil { + return corev1.PodQOSBestEffort + } + + // Pod Can not change QOSClass during it's lifetime. Therefore we can use the status value if present. + // Docs: The QoS class is determined when the Pod is created and remains unchanged for the lifetime of the Pod. If you later attempt an in-place resize that would result in a different QoS class, the resize is rejected by admission. + if pod.Status.QOSClass != "" { + return pod.Status.QOSClass + } + + return computePodQoSClass(pod) +} + +func computePodQoSClass(pod *corev1.Pod) corev1.PodQOSClass { + if podLevelQoS, ok := computePodLevelQoSClass(pod); ok { + return podLevelQoS + } + + return computeContainerLevelQoSClass(pod) +} + +func computePodLevelQoSClass(pod *corev1.Pod) (corev1.PodQOSClass, bool) { + if pod == nil { + return corev1.PodQOSBestEffort, false + } + + if pod.Spec.Resources == nil { + return corev1.PodQOSBestEffort, false + } + + requests := pod.Spec.Resources.Requests + limits := pod.Spec.Resources.Limits + + if !hasSupportedQoSResource(requests) && !hasSupportedQoSResource(limits) { + return corev1.PodQOSBestEffort, false + } + + cpuRequest, hasCPURequest := positiveResource(requests, corev1.ResourceCPU) + memoryRequest, hasMemoryRequest := positiveResource(requests, corev1.ResourceMemory) + cpuLimit, hasCPULimit := positiveResource(limits, corev1.ResourceCPU) + memoryLimit, hasMemoryLimit := positiveResource(limits, corev1.ResourceMemory) + + if hasCPURequest && + hasMemoryRequest && + hasCPULimit && + hasMemoryLimit && + cpuRequest.Cmp(cpuLimit) == 0 && + memoryRequest.Cmp(memoryLimit) == 0 { + return corev1.PodQOSGuaranteed, true + } + + return corev1.PodQOSBurstable, true +} + +func computeContainerLevelQoSClass(pod *corev1.Pod) corev1.PodQOSClass { + if pod == nil { + return corev1.PodQOSBestEffort + } + + hasSupportedRequestOrLimit := false + isGuaranteed := true + + containers := make([]corev1.Container, 0, + len(pod.Spec.Containers)+ + len(pod.Spec.InitContainers)+ + len(pod.Spec.EphemeralContainers), + ) + + containers = append(containers, pod.Spec.Containers...) + containers = append(containers, pod.Spec.InitContainers...) + + for _, container := range pod.Spec.EphemeralContainers { + containers = append(containers, corev1.Container{ + Name: container.Name, + Resources: container.Resources, + }) + } + + if len(containers) == 0 { + return corev1.PodQOSBestEffort + } + + for _, container := range containers { + requests := container.Resources.Requests + limits := container.Resources.Limits + + cpuRequest, hasCPURequest := positiveResource(requests, corev1.ResourceCPU) + memoryRequest, hasMemoryRequest := positiveResource(requests, corev1.ResourceMemory) + cpuLimit, hasCPULimit := positiveResource(limits, corev1.ResourceCPU) + memoryLimit, hasMemoryLimit := positiveResource(limits, corev1.ResourceMemory) + + if hasCPURequest || hasMemoryRequest || hasCPULimit || hasMemoryLimit { + hasSupportedRequestOrLimit = true + } + + if !hasCPURequest || + !hasMemoryRequest || + !hasCPULimit || + !hasMemoryLimit || + cpuRequest.Cmp(cpuLimit) != 0 || + memoryRequest.Cmp(memoryLimit) != 0 { + isGuaranteed = false + } + } + + if !hasSupportedRequestOrLimit { + return corev1.PodQOSBestEffort + } + + if isGuaranteed { + return corev1.PodQOSGuaranteed + } + + return corev1.PodQOSBurstable +} + +func hasSupportedQoSResource(resources corev1.ResourceList) bool { + for name, quantity := range resources { + if isSupportedQoSComputeResource(name) && quantity.Sign() > 0 { + return true + } + } + + return false +} + +func positiveResource(resources corev1.ResourceList, name corev1.ResourceName) (resource.Quantity, bool) { + quantity, ok := resources[name] + if !ok || quantity.Sign() <= 0 { + return resource.Quantity{}, false + } + + return quantity, true +} + +//nolint:exhaustive +func isSupportedQoSComputeResource(name corev1.ResourceName) bool { + switch name { + case corev1.ResourceCPU, corev1.ResourceMemory: + return true + default: + return false + } +} diff --git a/pkg/runtime/workloads/qos_class_test.go b/pkg/runtime/workloads/qos_class_test.go new file mode 100644 index 00000000..a6c758dd --- /dev/null +++ b/pkg/runtime/workloads/qos_class_test.go @@ -0,0 +1,835 @@ +package workloads + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestGetPodQoSClass(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pod *corev1.Pod + want corev1.PodQOSClass + }{ + { + name: "nil pod returns BestEffort", + pod: nil, + want: corev1.PodQOSBestEffort, + }, + { + name: "status QoS class takes precedence over computed value", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + QOSClass: corev1.PodQOSGuaranteed, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "empty status computes QoS class", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSBestEffort, + }, + { + name: "pod-level Guaranteed takes precedence over BestEffort containers", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: guaranteedPodResourcesPtr("100m", "128Mi"), + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "pod-level Burstable takes precedence over Guaranteed containers", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: requestOnlyPodResourcesPtr("100m", "128Mi"), + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "BestEffort without pod-level or container-level resources", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSBestEffort, + }, + { + name: "Burstable with container requests only", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + requestOnlyContainer("app", "100m", "128Mi"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "Burstable with container limits only", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + limitOnlyContainer("app", "100m", "128Mi"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "Guaranteed with equal CPU and memory requests and limits", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "Burstable when CPU request and limit differ", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + containerWithResources( + "app", + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("200m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + ), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "Burstable when memory request and limit differ", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + containerWithResources( + "app", + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("256Mi"), + }, + ), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "Burstable when one container is BestEffort", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + bestEffortContainer("sidecar"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "Guaranteed with multiple guaranteed containers", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + guaranteedContainer("sidecar", "50m", "64Mi"), + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "Burstable when init container has requests only", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + requestOnlyContainer("init", "100m", "128Mi"), + }, + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "Guaranteed when init and regular containers are guaranteed", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + guaranteedContainer("init", "100m", "128Mi"), + }, + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "Burstable when ephemeral container has requests only", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + }, + EphemeralContainers: []corev1.EphemeralContainer{ + { + EphemeralContainerCommon: corev1.EphemeralContainerCommon{ + Name: "debug", + Resources: requestOnlyRequirements("100m", "128Mi"), + }, + }, + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "Guaranteed when ephemeral container is guaranteed", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + }, + EphemeralContainers: []corev1.EphemeralContainer{ + { + EphemeralContainerCommon: corev1.EphemeralContainerCommon{ + Name: "debug", + Resources: guaranteedRequirements("100m", "128Mi"), + }, + }, + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "zero CPU and memory requests and limits are ignored", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + containerWithResources( + "app", + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("0"), + }, + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("0"), + }, + ), + }, + }, + }, + want: corev1.PodQOSBestEffort, + }, + { + name: "unsupported resources do not influence QoS", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + containerWithResources( + "app", + corev1.ResourceList{ + corev1.ResourceName("example.com/gpu"): resource.MustParse("1"), + }, + corev1.ResourceList{ + corev1.ResourceName("example.com/gpu"): resource.MustParse("1"), + }, + ), + }, + }, + }, + want: corev1.PodQOSBestEffort, + }, + { + name: "hugepages do not influence QoS", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + containerWithResources( + "app", + corev1.ResourceList{ + corev1.ResourceName("hugepages-2Mi"): resource.MustParse("2Mi"), + }, + corev1.ResourceList{ + corev1.ResourceName("hugepages-2Mi"): resource.MustParse("2Mi"), + }, + ), + }, + }, + }, + want: corev1.PodQOSBestEffort, + }, + { + name: "pod-level unsupported resources do not influence QoS", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceName("example.com/gpu"): resource.MustParse("1"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceName("example.com/gpu"): resource.MustParse("1"), + }, + }, + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSBestEffort, + }, + { + name: "pod-level zero resources do not mask container-level Guaranteed", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("0"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("0"), + }, + }, + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "pod-level CPU only request is Burstable", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + }, + }, + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "pod-level memory only limit is Burstable", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + }, + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "pod-level CPU and memory unequal request and limit is Burstable", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("200m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + }, + Containers: []corev1.Container{ + bestEffortContainer("app"), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + { + name: "no containers and no resources is BestEffort", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{}, + }, + want: corev1.PodQOSBestEffort, + }, + { + name: "aggregate container requests and limits equal returns Guaranteed", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + guaranteedContainer("app", "100m", "128Mi"), + guaranteedContainer("sidecar", "200m", "256Mi"), + }, + }, + }, + want: corev1.PodQOSGuaranteed, + }, + { + name: "mismatched container requests/limits are Burstable even if totals match", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + containerWithResources( + "a", + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("200m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + ), + containerWithResources( + "b", + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("200m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + ), + }, + }, + }, + want: corev1.PodQOSBurstable, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := GetPodQoSClass(tt.pod) + if got != tt.want { + t.Fatalf("expected QoS class %q, got %q", tt.want, got) + } + }) + } +} + +func TestComputePodLevelQoSClassNilPodResources(t *testing.T) { + t.Parallel() + + got, ok := computePodLevelQoSClass(&corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: nil, + }, + }) + + if got != corev1.PodQOSBestEffort { + t.Fatalf("expected QoS class %q, got %q", corev1.PodQOSBestEffort, got) + } + + if ok { + t.Fatalf("expected ok=false, got true") + } +} + +func TestComputePodLevelQoSClass(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pod *corev1.Pod + want corev1.PodQOSClass + ok bool + }{ + { + name: "nil pod returns false", + pod: nil, + want: corev1.PodQOSBestEffort, + ok: false, + }, + { + name: "no pod-level resources returns false", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{}, + }, + want: corev1.PodQOSBestEffort, + ok: false, + }, + { + name: "pod-level Guaranteed", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: guaranteedPodResourcesPtr("100m", "128Mi"), + }, + }, + want: corev1.PodQOSGuaranteed, + ok: true, + }, + { + name: "pod-level request only is Burstable", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: requestOnlyPodResourcesPtr("100m", "128Mi"), + }, + }, + want: corev1.PodQOSBurstable, + ok: true, + }, + { + name: "pod-level limit only is Burstable", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: limitOnlyPodResourcesPtr("100m", "128Mi"), + }, + }, + want: corev1.PodQOSBurstable, + ok: true, + }, + { + name: "pod-level CPU only equal request and limit is Burstable", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + }, + }, + }, + }, + want: corev1.PodQOSBurstable, + ok: true, + }, + { + name: "pod-level memory only equal request and limit is Burstable", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + }, + }, + }, + want: corev1.PodQOSBurstable, + ok: true, + }, + { + name: "pod-level zero resources return false", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("0"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("0"), + }, + }, + }, + }, + want: corev1.PodQOSBestEffort, + ok: false, + }, + { + name: "pod-level unsupported resources return false", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + Resources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceName("example.com/gpu"): resource.MustParse("1"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceName("example.com/gpu"): resource.MustParse("1"), + }, + }, + }, + }, + want: corev1.PodQOSBestEffort, + ok: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, ok := computePodLevelQoSClass(tt.pod) + if got != tt.want { + t.Fatalf("expected QoS class %q, got %q", tt.want, got) + } + + if ok != tt.ok { + t.Fatalf("expected ok=%t, got %t", tt.ok, ok) + } + }) + } +} + +func TestQoSHelpers(t *testing.T) { + t.Parallel() + + t.Run("hasSupportedQoSResource", func(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + resources corev1.ResourceList + want bool + }{ + { + name: "nil resources", + resources: nil, + want: false, + }, + { + name: "empty resources", + resources: corev1.ResourceList{}, + want: false, + }, + { + name: "CPU positive", + resources: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("1"), + }, + want: true, + }, + { + name: "memory positive", + resources: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Mi"), + }, + want: true, + }, + { + name: "CPU zero", + resources: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + }, + want: false, + }, + { + name: "unsupported positive", + resources: corev1.ResourceList{ + corev1.ResourceName("example.com/gpu"): resource.MustParse("1"), + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := hasSupportedQoSResource(tt.resources) + if got != tt.want { + t.Fatalf("expected %t, got %t", tt.want, got) + } + }) + } + }) + + t.Run("positiveResource", func(t *testing.T) { + t.Parallel() + + resources := corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("0"), + } + + if got, ok := positiveResource(resources, corev1.ResourceCPU); !ok || got.Cmp(resource.MustParse("100m")) != 0 { + t.Fatalf("expected positive CPU resource, got %q ok=%t", got.String(), ok) + } + + if got, ok := positiveResource(resources, corev1.ResourceMemory); ok { + t.Fatalf("expected zero memory to be ignored, got %q ok=%t", got.String(), ok) + } + + if got, ok := positiveResource(resources, corev1.ResourceStorage); ok { + t.Fatalf("expected missing storage to be ignored, got %q ok=%t", got.String(), ok) + } + }) + + t.Run("isSupportedQoSComputeResource", func(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in corev1.ResourceName + want bool + }{ + {name: "CPU", in: corev1.ResourceCPU, want: true}, + {name: "memory", in: corev1.ResourceMemory, want: true}, + {name: "storage", in: corev1.ResourceStorage, want: false}, + {name: "ephemeral storage", in: corev1.ResourceEphemeralStorage, want: false}, + {name: "extended resource", in: corev1.ResourceName("example.com/gpu"), want: false}, + {name: "hugepage", in: corev1.ResourceName("hugepages-2Mi"), want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := isSupportedQoSComputeResource(tt.in) + if got != tt.want { + t.Fatalf("expected %t, got %t", tt.want, got) + } + }) + } + }) +} + +func bestEffortContainer(name string) corev1.Container { + return corev1.Container{Name: name} +} + +func requestOnlyContainer(name, cpu, memory string) corev1.Container { + return corev1.Container{ + Name: name, + Resources: requestOnlyRequirements(cpu, memory), + } +} + +func limitOnlyContainer(name, cpu, memory string) corev1.Container { + return corev1.Container{ + Name: name, + Resources: limitOnlyRequirements(cpu, memory), + } +} + +func guaranteedContainer(name, cpu, memory string) corev1.Container { + return corev1.Container{ + Name: name, + Resources: guaranteedRequirements(cpu, memory), + } +} + +func containerWithResources( + name string, + requests corev1.ResourceList, + limits corev1.ResourceList, +) corev1.Container { + return corev1.Container{ + Name: name, + Resources: corev1.ResourceRequirements{ + Requests: requests, + Limits: limits, + }, + } +} + +func requestOnlyRequirements(cpu, memory string) corev1.ResourceRequirements { + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpu), + corev1.ResourceMemory: resource.MustParse(memory), + }, + } +} + +func limitOnlyRequirements(cpu, memory string) corev1.ResourceRequirements { + return corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpu), + corev1.ResourceMemory: resource.MustParse(memory), + }, + } +} + +func guaranteedRequirements(cpu, memory string) corev1.ResourceRequirements { + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpu), + corev1.ResourceMemory: resource.MustParse(memory), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpu), + corev1.ResourceMemory: resource.MustParse(memory), + }, + } +} + +func requestOnlyPodResources(cpu, memory string) corev1.ResourceRequirements { + return requestOnlyRequirements(cpu, memory) +} + +func limitOnlyPodResources(cpu, memory string) corev1.ResourceRequirements { + return limitOnlyRequirements(cpu, memory) +} + +func guaranteedPodResources(cpu, memory string) corev1.ResourceRequirements { + return guaranteedRequirements(cpu, memory) +} + +func requestOnlyPodResourcesPtr(cpu, memory string) *corev1.ResourceRequirements { + resources := requestOnlyRequirements(cpu, memory) + + return &resources +} + +func limitOnlyPodResourcesPtr(cpu, memory string) *corev1.ResourceRequirements { + resources := limitOnlyRequirements(cpu, memory) + + return &resources +} + +func guaranteedPodResourcesPtr(cpu, memory string) *corev1.ResourceRequirements { + resources := guaranteedRequirements(cpu, memory) + + return &resources +} diff --git a/pkg/tenant/rules.go b/pkg/tenant/rules.go index 1bf64f9b..dfe4a0c7 100644 --- a/pkg/tenant/rules.go +++ b/pkg/tenant/rules.go @@ -33,9 +33,10 @@ func GetManagedRuleStatus( return obj, err } -// BuildNamespaceRuleBodyForNamespace returns the aggregated rule body that applies to `ns`. +// BuildNamespaceRuleBodyStatus returns the aggregated rule bodies that apply to ns. // - Rules with nil NamespaceSelector match all namespaces. -// - Matching rules are combined in the order they appear in tnt.Spec.Rules (important for "later wins" semantics). +// - Matching rules are returned in the order they appear in tnt.Spec.Rules. +// - Order is important because registry/QoS evaluation uses "later allow/deny wins" semantics. func BuildNamespaceRuleBodyStatus( ctx context.Context, c client.Reader, @@ -46,7 +47,6 @@ func BuildNamespaceRuleBodyStatus( return nil, nil } - // Treat nil labels map as empty. nsLabels := labels.Set{} if ns.Labels != nil { nsLabels = labels.Set(ns.Labels) @@ -70,25 +70,12 @@ func BuildNamespaceRuleBodyStatus( } } - normalized := rules.NamespaceRuleBodyNamespace{ - Enforce: rules.NamespaceRuleEnforceBody{ - Action: rule.Enforce.Action, - Registries: append( - []rules.OCIRegistry(nil), - rule.Enforce.Registries..., - ), - }, - } - - if normalized.Enforce.Action == "" { - normalized.Enforce.Action = rules.ActionTypeDeny - } - - if len(normalized.Enforce.Registries) == 0 { + body := rule.NamespaceRuleBodyNamespace + if body == nil || body.Enforce == nil { continue } - out = append(out, &normalized) + out = append(out, body.DeepCopy()) } return out, nil