Keystone: Implement RegisteredLimit Controller by gndrmnn · Pull Request #782 · k-orc/openstack-resource-controller

gndrmnn · 2026-05-11T14:22:38Z

WIP

No support for RegionID as k-orc does not have that resource currently
RegisteredLimits are unnamed, but have a unique constraint over ResourceName, RegionID, and DefaultValue leading to some interesting challengens. E.g. Declaring two yaml files with two RegisteredLimits with the same RegionID and ResourceName will race to change the same entry.

github-actions · 2026-05-11T14:28:38Z

Failed to assess the semver bump. See logs for details.

gndrmnn · 2026-07-15T07:02:27Z

Hi, this PR is more or less good to be reviewed. I am currently running into a strange problem though: In the registeredlimit-import test we are creating a mandatory Service to use in the import filter. In some cases, the CI pipeline will fail with Waiting for service/<service name> to be ready. Other times, it successfully proceeds to Waiting for OpenStack resource to be created externally. As you can see right now, two CI tests failed while the epoxy one passed.

I have already setup a local keystone instance to test this against, and I can not reproduce the Waiting for service/<service name> to be ready problem. So it looks like this is some kind of timing problem in the test maybe?

What makes this especially weird is, I have looked at the Endpoint actuator and used it as inspiration for this PR. The Endpoint also requires a mandatory ServiceRef and even declares it the same way as I do in the endpoint-import CI test. That test however does not flap at all. So I seem to be doing something wrong, I can not put my finger on right now.

mandre · 2026-07-17T01:54:56Z

@mandre

Hi, this PR is more or less good to be reviewed. I am currently running into a strange problem though: In the registeredlimit-import test we are creating a mandatory Service to use in the import filter. In some cases, the CI pipeline will fail with Waiting for service/<service name> to be ready. Other times, it successfully proceeds to Waiting for OpenStack resource to be created externally. As you can see right now, two CI tests failed while the epoxy one passed.

Hi @gndrmnn, I won't be able to look at this right away. Perhaps @eshulman2, @dlaw4608, or @winiciusallan can do a first round of reviews, and might have ideas on what could be causing the tests to fail. I'll look at this when I'm back.

winiciusallan

Hi @gndrmnn, try the suggestion and let's see how the tests behave. In the meantime, I'll review the rest of your code. Thanks for working on this.

$ go run ./cmd/scaffold-controller -interactive=false \ -kind=RegisteredLimit \ -gophercloud-client=NewIdentityV3 \ -gophercloud-module=github.com/gophercloud/gophercloud/v2/openstack/identity/v3/registeredlimits \ -gophercloud-type=RegisteredLimit \ -openstack-json-object=registered_limits \ -required-create-dependency=Service \ -import-dependency=Service On-behalf-of: SAP nils.gondermann@sap.com

Register with the resource generator On-behalf-of: SAP nils.gondermann@sap.com

Add the OpenStack client to scope On-behalf-of: SAP nils.gondermann@sap.com

Register the controller On-behalf-of: SAP nils.gondermann@sap.com

On-behalf-of: SAP nils.gondermann@sap.com

dlaw4608

Looks Good, currently going through the E2E tests, also don't forget to add this controller the the README

winiciusallan · 2026-07-20T12:42:08Z

-		// TODO(scaffolding): Add more fields
+		ServiceID:    serviceID,
+		ResourceName: resource.ResourceName,
+		DefaultLimit: int(*resource.DefaultLimit),


I'm afraid that we have a bug in Gophercloud: we can't use the default limit as 0. Have you tested this in one of your runs?

I can confirm a default value of 0 will cause a invalid configuration creating resource error. defaultLimit is required in Gophercloud, but a zero value will fail validation.

gophercloud/gophercloud#3866

Thanks for opening this @gndrmnn

It only seems to be affecting creation, because the field in UpdateOpts doesn't have the required:true marker.

So let's return a terminal error here when DefaultLimit is 0, pointing to the gophercloud bug.

On-behalf-of: SAP nils.gondermann@sap.com

Generate the OLM bundle On-behalf-of: SAP nils.gondermann@sap.com

On-behalf-of: SAP nils.gondermann@sap.com

mandre

Again, this looks very good. Just a few cleanup needed before we can merge it.

mandre · 2026-08-12T08:10:30Z

+	// +kubebuilder:validation:MaxLength:=255
+	// +required
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="resourceName is immutable"
+	ResourceName string `json:"resourceName,omitempty"`


It looks like keystone enforces names with non-whitespaces chars only:

name: dict[str, Any] = { "type": "string", "minLength": 1, "maxLength": 255, "pattern": r"[\S]+", "description": "The resource name.", }

https://github.com/openstack/keystone/blob/50d266e/keystone/api/validation/parameter_types.py#L19-L25

We should also have the same validation pattern otherwise we may end up in a situation where we get endless reconciles that can never succeed if someone sets a name with a whitespace.

In other words: our API for resource specs can be stricter than the OpenStack API, never the other way around.

mandre · 2026-08-12T09:00:18Z

+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="resourceName is immutable"
+	ResourceName string `json:"resourceName,omitempty"`
+
+	// defaultLimit is limit of the specified resource in the given context.


Should we perhaps explain that -1 means unlimited (no quota), while 0 means hard blocking the usage of this resource? It's easy to wrongly assume that 0 means unlimited.

We should also note the limitation of setting 0 at creation pointing to the gophercloud issue.

It seems we can't reliably write a CEL validation that would prevent that (we need the CRDValidationRatcheting gate that's only enabled by default starting from kube 1.30, and we target 1.29 as our floor version). So we would need to add a check in the actuator, at resource creation.

mandre · 2026-08-12T09:25:53Z

-		// TODO(scaffolding): Add more fields
+		ServiceID:    serviceID,
+		ResourceName: resource.ResourceName,
+		DefaultLimit: int(*resource.DefaultLimit),


It only seems to be affecting creation, because the field in UpdateOpts doesn't have the required:true marker.

So let's return a terminal error here when DefaultLimit is 0, pointing to the gophercloud bug.

mandre · 2026-08-12T09:44:26Z

+		return nil, err
+	}
+
+	return &batch[0], nil


There's no guarantee that the slice is non-empty when err == nil, we should check for len(batch) == 0 and return an error if that's the case.

mandre · 2026-08-12T09:46:45Z

+	// +optional
+	RegionID string `json:"regionID,omitempty"`
+
+	// serviceID is a reference to the ORC Service which this resource is associated with.


Copy & paste error I assume :)
It should be

Suggested change

// serviceID is a reference to the ORC Service which this resource is associated with.

// serviceID is the UUID of the service to which the registered limit belongs.

gndrmnn force-pushed the implement_registeredlimit branch from 22a08f7 to e46ac56 Compare May 12, 2026 14:31

github-actions Bot added the semver:major Breaking change label May 12, 2026

gndrmnn force-pushed the implement_registeredlimit branch from e46ac56 to 4964e1d Compare May 13, 2026 13:40

gndrmnn force-pushed the implement_registeredlimit branch 4 times, most recently from 8379469 to 027f4aa Compare June 1, 2026 14:23

gndrmnn force-pushed the implement_registeredlimit branch 4 times, most recently from b6de206 to e46d4a0 Compare June 29, 2026 14:10

gndrmnn force-pushed the implement_registeredlimit branch 11 times, most recently from 4c2f1fe to 8c90198 Compare July 14, 2026 12:02

gndrmnn marked this pull request as ready for review July 15, 2026 07:02

winiciusallan reviewed Jul 17, 2026

View reviewed changes

Comment thread internal/controllers/registeredlimit/controller.go

gndrmnn marked this pull request as draft July 20, 2026 07:09

chenwng mentioned this pull request Jul 20, 2026

Limits Controller #851

Open

gndrmnn added 4 commits July 20, 2026 11:30

Scaffolding for the RegisteredLimit controller

a9cffb5

Register with the resource generator On-behalf-of: SAP nils.gondermann@sap.com

Scaffolding for the RegisteredLimit controller

83e8d40

Add the OpenStack client to scope On-behalf-of: SAP nils.gondermann@sap.com

Scaffolding for the RegisteredLimit controller

17e8b9d

Register the controller On-behalf-of: SAP nils.gondermann@sap.com

RegisteredLimit Adapt to BatchCreate API

7191209

On-behalf-of: SAP nils.gondermann@sap.com

gndrmnn force-pushed the implement_registeredlimit branch 2 times, most recently from 85d8901 to 1221836 Compare July 20, 2026 10:32

RegisteredLimit Type Declaration

20e2cbb

On-behalf-of: SAP nils.gondermann@sap.com

gndrmnn force-pushed the implement_registeredlimit branch from 1221836 to 7ba4ab5 Compare July 20, 2026 11:04

gndrmnn marked this pull request as ready for review July 20, 2026 11:33

gndrmnn requested a review from winiciusallan July 20, 2026 11:33

dlaw4608 reviewed Jul 20, 2026

View reviewed changes

Comment thread internal/controllers/registeredlimit/tests/registeredlimit-update/01-updated-resource.yaml Outdated

dlaw4608 reviewed Jul 20, 2026

View reviewed changes

Comment thread internal/controllers/registeredlimit/status.go

winiciusallan reviewed Jul 20, 2026

View reviewed changes

gndrmnn force-pushed the implement_registeredlimit branch from 7ba4ab5 to 77b0b7a Compare July 21, 2026 10:18

gndrmnn added 3 commits July 22, 2026 09:19

RegisteredLimit Actuator and Status

3c224e0

On-behalf-of: SAP nils.gondermann@sap.com

RegisteredLimit Tests and Config Samples

e5d3ef4

On-behalf-of: SAP nils.gondermann@sap.com

Scaffolding for the RegisteredLimit controller

fd45397

Generate the OLM bundle On-behalf-of: SAP nils.gondermann@sap.com

gndrmnn force-pushed the implement_registeredlimit branch from 77b0b7a to 6147781 Compare July 22, 2026 07:20

Add RegisteredLimits to README

d5b3186

On-behalf-of: SAP nils.gondermann@sap.com

gndrmnn force-pushed the implement_registeredlimit branch from 6147781 to d5b3186 Compare July 22, 2026 07:54

gndrmnn mentioned this pull request Jul 22, 2026

RegisteredLimit does not allow a defaultLimit of 0 gophercloud/gophercloud#3866

Open

mandre reviewed Aug 12, 2026

View reviewed changes

	// serviceID is a reference to the ORC Service which this resource is associated with.
	// serviceID is the UUID of the service to which the registered limit belongs.

Uh oh!

Conversation

gndrmnn commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

gndrmnn commented Jul 15, 2026

Uh oh!

mandre commented Jul 17, 2026

Uh oh!

winiciusallan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dlaw4608 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

mandre left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gndrmnn commented May 11, 2026 •

edited

Loading