Keystone: User Controller by dlaw4608 · Pull Request #657 · k-orc/openstack-resource-controller

dlaw4608 · 2026-01-28T11:22:52Z

Fixes #583
Add Support for the Identity service's User resource in ORC

winiciusallan

Hi, @dlaw4608, that was a great job!

Left a few comments about using a Project object instead of the ID directly, LMK what you think.

winiciusallan · 2026-02-03T13:48:07Z

api/v1alpha1/user_types.go

+	// +kubebuilder:validation:MinLength:=1
+	// +kubebuilder:validation:MaxLength:=255
+	// +optional
+	DefaultProjectID *string `json:"defaultProjectID,omitempty"`


Isn't it better to use a ProjectRef here so we can better control the resource?

Suggested change

DefaultProjectID *string `json:"defaultProjectID,omitempty"`

DefaultProjectID *ProjectRef `json:"defaultProjectID,omitempty"`

It should indeed be a reference, but would rather be in the form:

Suggested change

DefaultProjectID *string `json:"defaultProjectID,omitempty"`

DefaultProjectRef *KubernetesNameRef `json:"defaultProjectRef,omitempty"`

See https://k-orc.cloud/development/architecture/#api-design-philosophy.

I'd like to add a lint rule to flag these.

Also, since this is an optional dependency for your controller, you should have run the scaffolding tool with -optional-create-dependency Project.

Agreed, went back and recreated the user controller this time with Project included as an optional dependency
go run ./cmd/scaffold-controller -interactive=false \ -kind User \ -gophercloud-client NewIdentityV3 \ -gophercloud-module github.com/gophercloud/gophercloud/v2/openstack/identity/v3/users \ -import-dependency Domain \ -optional-create-dependency Domain \ -optional-create-dependency Project

Changed ProjectRef to DefaultProjectRef to match the OpenStack field naming. The spec only references other ORC objects (like Project) by K8s name, never OpenStack UUIDs. The controller resolves the reference to get the Project's UUID and waits for the Project to be Available before creating the User with its DefaultProjectID.

winiciusallan · 2026-02-03T17:03:59Z

internal/controllers/user/actuator.go

+	}
+}
+
+func handleDefaultProjectIDUpdate(updateOpts *users.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) {


Possibly change this to use a reference instead of an ID as suggested above.

winiciusallan · 2026-02-04T14:00:35Z

internal/controllers/user/tests/user-create-minimal/00-assert.yaml

+
+
+
+
+


Blank lines at the end. I would remove them to make it cleaner.

winiciusallan · 2026-02-04T14:01:35Z

internal/controllers/user/tests/user-create-minimal/00-assert.yaml

+  resource:
+    name: user-create-minimal


I would also add enabled: true here.

thanks for the review @winiciusallan, changes made

mandre · 2026-02-05T09:28:33Z

api/v1alpha1/user_types.go

+	// +kubebuilder:validation:MinLength:=1
+	// +kubebuilder:validation:MaxLength:=255
+	// +optional
+	DefaultProjectID *string `json:"defaultProjectID,omitempty"`


It should indeed be a reference, but would rather be in the form:

Suggested change

DefaultProjectID *string `json:"defaultProjectID,omitempty"`

DefaultProjectRef *KubernetesNameRef `json:"defaultProjectRef,omitempty"`

See https://k-orc.cloud/development/architecture/#api-design-philosophy.

I'd like to add a lint rule to flag these.

Also, since this is an optional dependency for your controller, you should have run the scaffolding tool with -optional-create-dependency Project.

mandre · 2026-02-05T09:37:52Z

api/v1alpha1/user_types.go

+	// +kubebuilder:validation:MinLength:=1
+	// +kubebuilder:validation:MaxLength:=255
+	// +optional
+	Password *string `json:"password,omitempty"`


This must be stored in a secret. See how we do it for user data in the server controller.

Followed the same structure as user data in the server controller, the user_types api uses a SecretRef field to reference a secret containing the password of a user in a secret, thanks for the help!!

go run ./cmd/scaffold-controller -interactive=false \ -kind User \ -gophercloud-client NewIdentityV3 \ -gophercloud-module github.com/gophercloud/gophercloud/v2/openstack/identity/v3/users \ -import-dependency Domain \ -optional-create-dependency Domain \ -optional-create-dependency Project Signed-off-by: Daniel Lawton <dlawton@redhat.com>

- E2E tests included - API configured - Controller Signed-off-by: Daniel Lawton <dlawton@redhat.com>

mandre

Could you also add the User resource to the README ?

mandre · 2026-02-11T15:38:51Z

internal/controllers/user/tests/user-create-full/00-create-resource.yaml

-    projectRef: user-create-full
-    # TODO(scaffolding): Add all fields the resource supports
+    defaultProjectRef: user-create-full
+    enabled: true


Can we also try to set a password?

mandre · 2026-02-11T15:47:34Z

internal/controllers/volume/user-import-error/00-create-resources.yaml

Why is this test placed in the volume controller? This should be removed (I assume this is a result of local tests?).

mandre · 2026-02-11T16:00:12Z

api/v1alpha1/user_types.go

-	// Until you have implemented mutability for the field, you must add a CEL validation
-	// preventing the field being modified:
-	// `// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="<fieldname> is immutable"`
+	SecretRef *KubernetesNameRef `json:"secretRef,omitempty"`


We should add a dependency on this ref in the controller.
Also, we don't use the password at all in the controller. If it's not going to be implemented in this PR, I suggest to drop it from the PR and handle password management (create/update) it in a subsequent PR.

mandre · 2026-02-11T16:01:22Z

internal/controllers/user/tests/user-create-full/01-disbale-domain.yaml

nit: small typo in the file name.

github-actions bot added the semver:major Breaking change label Jan 28, 2026

dlaw4608 force-pushed the keystone_user branch 2 times, most recently from 23295c5 to dcfe645 Compare January 29, 2026 19:28

dlaw4608 marked this pull request as ready for review January 29, 2026 19:32

dlaw4608 force-pushed the keystone_user branch from dcfe645 to 0f90e6a Compare January 29, 2026 20:48

winiciusallan reviewed Feb 4, 2026

View reviewed changes

mandre reviewed Feb 5, 2026

View reviewed changes

dlaw4608 force-pushed the keystone_user branch 3 times, most recently from 9e7925e to c2ebe4c Compare February 11, 2026 10:10

Working User Controller:

cfe154f

- E2E tests included - API configured - Controller Signed-off-by: Daniel Lawton <dlawton@redhat.com>

dlaw4608 force-pushed the keystone_user branch from c2ebe4c to cfe154f Compare February 11, 2026 10:11

mandre reviewed Feb 11, 2026

View reviewed changes

	DefaultProjectID *string `json:"defaultProjectID,omitempty"`
	DefaultProjectID *ProjectRef `json:"defaultProjectID,omitempty"`

	DefaultProjectID *string `json:"defaultProjectID,omitempty"`
	DefaultProjectRef *KubernetesNameRef `json:"defaultProjectRef,omitempty"`

Conversation

dlaw4608 commented Jan 28, 2026

Uh oh!

winiciusallan 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants