Skip to content

Conversation

@andreas-kupries
Copy link
Contributor

@andreas-kupries andreas-kupries commented Nov 25, 2025

Issue:

Companion to rancher/rancher#52773

Brings the changes to the conversion functions in r/r to the webhook as well.
The code is currently a copy instead of calling the unified function in r/r.
This works, is however also a tech debt.
Unification requires figuring out how to import rancher/pkg/resourcequota into the webhook.

Problem

The webhook has to properly convert a rancher resource quota into a kube resource list, taking the new field extended into account. The webhook's copy of equivalent functions in r/r was behind the times here.

Solution

For now made a copy of the unified r/r function in the webhook.

CheckList

  • Test
  • Docs - No behavioral change. Extension to handle new field properly

note: copied from r/r
todo: unify this and the origin code
@bigkevmcd
Copy link
Contributor

I think we should have a test that asserts that the resource is passed through correctly?

@andreas-kupries
Copy link
Contributor Author

I think we should have a test that asserts that the resource is passed through correctly?

Yes, that is one of the todos

@andreas-kupries
Copy link
Contributor Author

I think we should have a test that asserts that the resource is passed through correctly?

Yes, that is one of the todos

And done (copied from the equivalent r/r tests)

@andreas-kupries
Copy link
Contributor Author

andreas-kupries commented Nov 26, 2025

Validation Template

Context:

Root Cause

As part of extending the rancher resource quota information with a field
extended to support any arbitrary quota a customer may wish to set the webhook
is now the third place where a conversion function from Rancher resource quota to
Kube resource list was found.

The first was dealt with with the original (already merged) PR rancher/rancher#52544
The second was also in r/r, see PR rancher/rancher#52773

All functions originally simply parsed all fields as strings, into a kube
resource quantity. The new field is however not a simple string, but a
map. Parsing as quantity fails.

What was fixed, or what change have occurred

In the original PR the function was extended to handle extended (sic!)
separately. Plus a fix to the mishandling of dotted resource references.
The 52773 PR unifies this and the other function found into a single function.

This PR here has copied the function from r/r into the webhook.

A copy was chosen because various attempts at importing the relevant package
from r/r directly foundered on the cliffs of go modules.

With this webhook now properly converts from quota to lists as well, instead of
failing to parse and rejecting extended quotas.

Areas or cases that should be tested

Verify that Projects are properly stored, with or without data in the extended
field of their resource quotas.

Verify that the dashboard is not broken by the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants