Non-namespaced resource limits #12916

tobschli · 2025-09-09T09:03:07Z

How to categorize this PR?

/area robustness
/kind enhancement

What this PR does / why we need it:

This PR adds the possibility to restrict the total amount of non-namespaced resources in the virtual garden cluster.
To achieve this, I enhanced the already existing resource-size webhook to also allow setting a count field.

When set, the webhook checks the total amount of already existing resources in the virtual-garden cluster and permits creating resources above this limit.

Just like the size limit, Kubernetes subjects can be configured that are exempt from this limit.

In addition to that, I added some minor fixes that improve developer experience.

Making the admission-local plugin debuggable
Fixing one condition in a Helm template that could result in invalid YAML

Which issue(s) this PR fixes:
Fixes #10878

Special notes for your reviewer:

There are two points, where I would like to know your opinion about:

In order to allow this webhook to be usable for non-namespaced resources, I needed to remove the namespaceSelector for it.
This is not really clean in my opinion, but apart from moving this logic in a standalone webhook (which I also did not prefer), I could not come up with a better solution at the moment
The name of the webhook now does not 100% reflect it's usage, but I am not entirely sure about the potential side effects of renaming the endpoint. Should we keep it as-is? Or should we rename it?

Release note:

It is now possible to restrict the total count of objects for non-namespaced resources. You can set it through the admission controller configuration's `server.resourceAdmissionConfiguration.limits[].count` field.

timuthy · 2025-09-10T06:54:02Z

/assign

timuthy

Thanks for the feature! Besides the comments, could you please enhance the documentation?

Regarding your second question:

The name of the webhook now does not 100% reflect it's usage, but I am not entirely sure about the potential side effects of renaming the endpoint. Should we keep it as-is? Or should we rename it?

Renaming of the webhook should be fine, but renaming the endpoint isn't disruption free. If we want to completely rename, a new webhook needs to be introduced and the old one removed.

pkg/admissioncontroller/webhook/admission/resourcesize/handler.go

...rdener/controlplane/charts/application/templates/validatingwebhook-admission-controller.yaml

tobschli · 2025-09-16T11:35:30Z

I just noticed that it would probably also make sense to add an integration test for this.
The unit tests for this feature only test the case where the count limit is 0.
This works because there is no actual rest mapper, which means the resolution to the GVR fails, returning a count of 0, which violates the limit.

I would implement this, once we decided to use a separate webhook or not :)

tobschli · 2025-09-18T11:34:51Z

In a talk with @timuthy we decided to move the namespace selector into the component-config of this webhook. - non-namespaced resources will be size / count checked all the time - namespaced resources will be ignored in case they don't match the namespaceSelector
We gain:

We do not need to introduce a new webhook, where we would need to drop all namespaced resources

We can also size restrict non-namespaced resources now 🥳

It turns out, that non-namespaced resources are not skipped, even though there is a namespaceSelector present.
This is also explained in the doc-string of the ValidatingWebhoolConfiguration's NamespaceSelector (ref):

If the object is another cluster scoped resource, it never skips the webhook.

I will update this PR accordingly in the near future.

timuthy

Thanks for the adjustments!

docs/concepts/admission-controller.md

pkg/admissioncontroller/apis/config/v1alpha1/types.go

pkg/admissioncontroller/webhook/admission/resourcesize/handler.go

tobschli · 2025-09-22T12:28:01Z

Because I somehow cannot comment on the one PR review comment:

Can we only log this in case a limit is really configured for the affected resource?

We already return earlier here

tobschli · 2025-09-22T13:52:32Z

BTW. pull-gardener-e2e-kind-ha-multi-zone now failed 5 times in a row.. I am beginning to think that this is not flaky behavior, but something related to this PR. But the test logs, as well as the artifacts do not really give me a clue on what this could be...

When checking the diff between the commit where it was working and where the failing started, I also cannot point out anything that looks off: https://github.com/tobschli/gardener/compare/5991c265aa5732529b289347e376e9459c5b5d7c..924f3e9db980e5594090792d6888a78c4692669c

pkg/admissioncontroller/webhook/admission/resourcesize/handler.go

timuthy

/lgtm

gardener-prow · 2025-09-23T15:48:08Z

LGTM label has been added.

Git tree hash: b1ee9c4886f6e35412e29a0bbb6e177cdcdcc268

tobschli · 2025-09-24T06:36:16Z

I will take a look later on why the pull-gardener-e2e-kind-ha-multi-zone consistently fails.

tobschli · 2025-09-24T07:30:33Z

Booting up the scenario locally works without a problem.
My suspicion is that this behavior is related to this flake: #12919

I will take a look why this change here provokes this even more.

…ze` limit

gardener-prow · 2025-10-02T11:27:22Z

New changes are detected. LGTM label has been removed.

tobschli · 2025-10-06T07:11:05Z

/test pull-gardener-e2e-kind-ha-multi-zone

rfranzke · 2025-10-08T08:21:55Z

pkg/admissioncontroller/apis/config/v1alpha1/types.go

+	Size *resource.Quantity `json:"size,omitempty"`
+	// Count specifies the maximum number of resources of the given kind.
+	// +optional
+	Count *int64 `json:"count,omitempty"`


You could adapt L72 now

rfranzke · 2025-10-08T08:22:14Z

pkg/admissioncontroller/apis/config/v1alpha1/types.go

+	Size *resource.Quantity `json:"size,omitempty"`
+	// Count specifies the maximum number of resources of the given kind.
+	// +optional
+	Count *int64 `json:"count,omitempty"`


Also adapt https://github.com/gardener/gardener/blob/master/example/20-componentconfig-gardener-admission-controller.yaml

rfranzke · 2025-10-08T08:22:39Z

pkg/admissioncontroller/apis/config/v1alpha1/validation/validation.go

+		if limit.Count != nil && apivalidation.ValidateNonnegativeField(*limit.Count, fld.Child("count")).ToAggregate() != nil {
+			allErrs = append(allErrs, field.Invalid(fld.Child("count"), limit.Count, "value must not be negative"))
+		}
+
+		if limit.Size != nil && limit.Size.Cmp(resource.Quantity{}) < 0 {


I guess one of count or size should be set (both must not be unset at the same time).

rfranzke · 2025-10-08T08:23:14Z

pkg/apis/operator/v1alpha1/conversion/conversion.go

 			APIVersions: limit.APIVersions,
 			Resources:   limit.Resources,
-			Size:        limit.Size,
+			Size:        &limit.Size,


Don't we need to add Count here now as well?

The ResourceLimit type seems to be equally defined in two places:

https://github.com/tobschli/gardener/blob/13363fa6facf8e75776416b567d4a8e6ea9bf27b/pkg/admissioncontroller/apis/config/v1alpha1/types.go#L73-L88

gardener/pkg/apis/operator/v1alpha1/types_garden.go

Lines 561 to 572 in 6a9a9d5

type ResourceLimit struct {

// APIGroups is the name of the APIGroup that contains the limited resource. WildcardAll represents all groups.

// +optional

APIGroups []string `json:"apiGroups,omitempty"`

// APIVersions is the version of the resource. WildcardAll represents all versions.

// +optional

APIVersions []string `json:"apiVersions,omitempty"`

// Resources is the name of the resource this rule applies to. WildcardAll represents all resources.

Resources []string `json:"resources"`

// Size specifies the imposed limit.

Size resource.Quantity `json:"size"`

}

Do we want to drop one?

I think we didn't want to import the component config API into the operator.gardener.cloud/v1alpha1 package, hence the duplication.

rfranzke · 2025-10-08T08:24:23Z

skaffold.yaml

+# TEST: Check if this causes the flake in CI
+#  insecureRegistries:
+#    - garden.local.gardener.cloud:5001


Let's remove this again :)

rfranzke · 2025-10-08T08:26:22Z

pkg/admissioncontroller/webhook/admission/resourcesize/handler.go

+}
+
+func (h *Handler) getKindFromGVR(gvr *metav1.GroupVersionResource) (string, error) {
+	if h.RESTMapper == nil {


Is this check really needed? Usually, we don't do these checks if we control the field 👀

Perhaps not, but I thought it would be better to have it, than to not have it.
In order keep everything aligned, I will remove it :)

rfranzke · 2025-10-08T08:27:04Z

pkg/admissioncontroller/webhook/admission/resourcesize/handler.go

+}
+
+func (h *Handler) existingResourcesExceedLimit(_ admission.Request, gvr *metav1.GroupVersionResource, limit int64) (bool, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)


Why don't we pass the context.Context from the Handle function down here? This would be better instead of using context.Background

rfranzke · 2025-10-08T08:28:06Z

pkg/admissioncontroller/webhook/admission/resourcesize/handler.go

+	})
+
+	// List all resources of this type cluster-wide (since we only count non-namespaced resources)
+	if err := h.Client.List(ctx, list, client.Limit(limit+1)); err != nil {


Nit: Would it make sense to use an APIReader (that is uncached)? Otherwise, if many objects were created at the same time, the GAC's cache might be stale and (unintentionally) allow exceeding the configured resource limit?

Does the GAC's manager use a cached client? I could not find the cache settings for it, so I would assume it is uncached?

Yes, the controller-runtime client is cached by default, unless you explicitly disable it.

rfranzke · 2025-10-08T08:28:45Z

pkg/admissioncontroller/webhook/admission/resourcesize/handler.go

+		return fmt.Errorf("failed to determine if count exceeds limit: %w", err)
+	}
+
+	if exceedsLimit {


NIt: You could make an early exit (to prevent indenting) if exceedsLimit is false.

rfranzke · 2025-10-08T08:30:01Z

...er/controlplane/charts/runtime/templates/admission-controller/configmap-componentconfig.yaml

        port: {{ required ".Values.global.admission.config.server.metrics.port is required" .Values.global.admission.config.server.metrics.port }}
      {{- if .Values.global.admission.config.server.resourceAdmissionConfiguration }}
      resourceAdmissionConfiguration:
+        {{- if .Values.global.admission.config.server.resourceAdmissionConfiguration.limits }}


Nit: You could adapt the example in

gardener/charts/gardener/controlplane/values.yaml

Lines 310 to 315 in bbc46e3

# resourceAdmissionConfiguration:

# limits:

# - apiGroups: ["core.gardener.cloud"]

# apiVersions: ["*"]

# resources: ["shoots"]

# size: 100Ki

…en values are empty. Before this, when not specifying e.g. `unrestrictedSubjects`, the key would be added to the YAML, leading to invalid YAML

This also included some refactorings, as we did not return an error when the evaluation of the current count of objects fails, to prevent this feature from blocking operations on the objects.

…resources

gardener-prow · 2025-10-09T11:30:41Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from rfranzke. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gardener-prow · 2025-10-09T12:12:58Z

@tobschli: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gardener-e2e-kind-operator-seed	`87ff3b4`	link	true	`/test pull-gardener-e2e-kind-operator-seed`
pull-gardener-apidiff	`a7a3d09`	link	false	`/test pull-gardener-apidiff`
pull-gardener-unit	`a7a3d09`	link	true	`/test pull-gardener-unit`
pull-gardener-e2e-kind-migration	`a7a3d09`	link	true	`/test pull-gardener-e2e-kind-migration`

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

gardener-prow bot added area/robustness Robustness, reliability, resilience related kind/enhancement Enhancement, improvement, extension labels Sep 9, 2025

gardener-prow bot requested review from ary1992 and marc1404 September 9, 2025 09:03

gardener-prow bot added cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 9, 2025

tobschli force-pushed the project-quota branch from fb02468 to 335af25 Compare September 9, 2025 09:04

gardener-prow bot assigned timuthy Sep 10, 2025

timuthy reviewed Sep 16, 2025

View reviewed changes

tobschli force-pushed the project-quota branch from adea887 to aec5f86 Compare September 16, 2025 11:33

tobschli force-pushed the project-quota branch from aec5f86 to 5991c26 Compare September 16, 2025 11:37

timuthy requested changes Sep 19, 2025

View reviewed changes

timuthy reviewed Sep 22, 2025

View reviewed changes

timuthy reviewed Sep 23, 2025

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Sep 23, 2025

tobschli added 2 commits October 2, 2025 13:23

Add optional Count parameter to resource limit webhook

3e21577

Adapt components, tests and conversions to take the, now optional `si…

6c22301

…ze` limit

Adapt resourcesize handler to also handle count limit

955b98e

tobschli force-pushed the project-quota branch from 87ff3b4 to cb4f189 Compare October 2, 2025 11:27

gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 2, 2025

gardener-prow bot requested a review from timuthy October 2, 2025 11:27

rfranzke requested changes Oct 8, 2025

View reviewed changes

gardener-prow bot assigned rfranzke Oct 8, 2025

tobschli added 12 commits October 9, 2025 13:29

Add tests for count limit handling

378eb75

Add validation for count field

1541f2f

Add tests for count validation

8cc1f05

--- SEPARATOR ---

5352aa6

Adapt admission-controller ConfigMap template to not add keys, wh…

6cc6480

…en values are empty. Before this, when not specifying e.g. `unrestrictedSubjects`, the key would be added to the YAML, leading to invalid YAML

Adapt documentation

fe5032d

Limit the list to countLimit + 1

562502a

This also included some refactorings, as we did not return an error when the evaluation of the current count of objects fails, to prevent this feature from blocking operations on the objects.

Add logging to hint that limits are only enforced for non-namespaced …

28958ec

…resources

Fix test by adding rest mapper

0e69ad5

Implement PR feedback

103d966

Implement PR feedback

e972bd2

Implement PR feedback

a7a3d09

tobschli force-pushed the project-quota branch from 9612d0a to a7a3d09 Compare October 9, 2025 11:30

	type ResourceLimit struct {
	// APIGroups is the name of the APIGroup that contains the limited resource. WildcardAll represents all groups.
	// +optional
	APIGroups []string `json:"apiGroups,omitempty"`
	// APIVersions is the version of the resource. WildcardAll represents all versions.
	// +optional
	APIVersions []string `json:"apiVersions,omitempty"`
	// Resources is the name of the resource this rule applies to. WildcardAll represents all resources.
	Resources []string `json:"resources"`
	// Size specifies the imposed limit.
	Size resource.Quantity `json:"size"`
	}

	# resourceAdmissionConfiguration:
	# limits:
	# - apiGroups: ["core.gardener.cloud"]
	# apiVersions: ["*"]
	# resources: ["shoots"]
	# size: 100Ki

Non-namespaced resource limits #12916

Are you sure you want to change the base?

Non-namespaced resource limits #12916

Uh oh!

Conversation

tobschli commented Sep 9, 2025

Uh oh!

timuthy commented Sep 10, 2025

Uh oh!

timuthy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tobschli commented Sep 16, 2025

Uh oh!

tobschli commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

timuthy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tobschli commented Sep 22, 2025

Uh oh!

tobschli commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timuthy left a comment

Choose a reason for hiding this comment

Uh oh!

gardener-prow bot commented Sep 23, 2025

Uh oh!

tobschli commented Sep 24, 2025

Uh oh!

tobschli commented Sep 24, 2025

Uh oh!

gardener-prow bot commented Oct 2, 2025

Uh oh!

tobschli commented Oct 6, 2025

Uh oh!

rfranzke Oct 8, 2025 • edited by tobschli Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

timuthy left a comment •

edited

Loading

tobschli commented Sep 18, 2025 •

edited

Loading

tobschli commented Sep 22, 2025 •

edited

Loading

rfranzke Oct 8, 2025 •

edited by tobschli

Loading