Keystone: User CreateOpts for password field by dlaw4608 · Pull Request #709 · k-orc/openstack-resource-controller

dlaw4608 · 2026-03-12T17:05:26Z

Fixes: #703

Follow up for User Controller PR that was recently merged #657,
Current Progress:

User CreateOpts included for password field works fine (E2E tests passing, password when created will be stored as a SecretRef.

winiciusallan

Hi @dlaw4608, thanks for adding this, a very important field for the user controller. I would like to highlight a few things:

I believe we need to change the logic to reconcile the password. Testing locally on my environment, I was not able to change the user password by simply changing the value of the password key in the secret. We need to identify a change in the secret upfront and then perform a PATCH operation[1] to properly update the password. We may want to add a new reconciler to the resource. Does it make sense?
From a user perspective, I believe it would be good to give the user the ability to pass the password encoded with base64. Testing locally, I chose to use the data key instead of stringData, and the password wasn't changed as expected. Not sure if this is a real requirement, but I would like to highlight this.

[1] https://docs.openstack.org/api-ref/identity/v3/index.html#update-user

edit: I just realized you intended to add only the Create option for passwords. If so, you can ignore the first bullet point at this moment.

winiciusallan · 2026-03-16T22:15:37Z

config/samples/openstack_v1alpha1_user.yaml

+  name: user-sample
+spec:
+  cloudCredentialsRef:
+    cloudName: devstack-admin


Maybe a change during development. Let's stick with openstack-admin for consistency. Wdyt?

winiciusallan · 2026-03-16T22:15:42Z

config/samples/openstack_v1alpha1_user.yaml

+  name: user-sample
+spec:
+  cloudCredentialsRef:
+    cloudName: devstack-admin


winiciusallan · 2026-03-16T22:15:55Z

config/samples/openstack_v1alpha1_user.yaml

 spec:
  cloudCredentialsRef:
-    cloudName: openstack-admin
+    cloudName: devstack-admin


And here too :).

winiciusallan · 2026-03-16T22:19:40Z

config/samples/openstack_v1alpha1_user.yaml

+  apiVersion: v1
+  kind: Secret
+  metadata:
+    name: user-sample
+  type: Opaque
+  stringData:
+    password: "TestPassword"


Suggested change

apiVersion: v1

kind: Secret

metadata:

name: user-sample

type: Opaque

stringData:

password: "TestPassword"

apiVersion: v1

kind: Secret

metadata:

name: user-sample

type: Opaque

stringData:

password: "TestPassword"

A few blank spaces at the beginning. Let's remove them for better indentation.

winiciusallan · 2026-03-16T22:30:16Z

api/v1alpha1/user_types.go

+
+	// password is the password set for the user
+	// +optional
+	Password *PasswordSpec `json:"password,omitempty"`


Why don't we keep it simple and pass the SecretRef directly here? Any advantage of using a struct?

+1 on this, not sure there is a point in warping it in an additional struct

winiciusallan · 2026-03-16T22:35:22Z

internal/controllers/user/tests/user-dependency/01-create-dependencies.yaml

+apiVersion: v1
+kind: Secret
+metadata:
+  name: user-create-full
+type: Opaque
+stringData:
+  password: "TestPassword"


Probably you also need to add a new user-dependency-no-password-secret or something like that at 00-create-resources-missing-deps.yaml to properly test the dependency with this secret, and complete the deletion later as well.

winiciusallan · 2026-03-16T22:58:51Z

website/docs/crd-reference.md

+
+
+
+
+


ooc: why do we have so many blank lines here? 🧐

that's just how the github.com/elastic/crd-ref-docs package works AFAIK.

BTW, I updated it in #716 and now we'll need to re-generate the assets.

eshulman2

very quick and shallow review (need to come back to this one)

eshulman2 · 2026-03-17T12:23:06Z

internal/controllers/user/actuator.go


 	handleNameUpdate(&updateOpts, obj, osResource)
 	handleDescriptionUpdate(&updateOpts, resource, osResource)
 	handleEnabledUpdate(&updateOpts, resource, osResource)


Missing password update logic, I would suggest adding it here to allow password changing and address what @winiciusallan was mentioning about password not changing when changing the secret

We can leave the password update for a later time if this gets too complicated in this PR. We would just need to make the field immutable for now.

However, in case we want to permit the creation of users without providing a password as it is now, then we must return the one that OpenStack generated for us (in a secret). Otherwise, we should make the password field required (this would actually .

eshulman2 · 2026-03-17T12:24:19Z

api/v1alpha1/user_types.go

+
+	// password is the password set for the user
+	// +optional
+	Password *PasswordSpec `json:"password,omitempty"`


+1 on this, not sure there is a point in warping it in an additional struct

mandre

Right now the user controller is not very useful, and it can only be used when importing existing users (we can create users too, but they can't be used because we don't expose the generated password).

My recommendation for this PR: let's focus on making managed Users more useful by making the new password field required. We can switch it to optional later, once we have a mechanism for exposing the generated password. Let's also make the password field immutable for now until we're ready to update it. It may be a bit tricky because we'll need to watch both the passwordRef and the referenced password.

mandre · 2026-03-19T14:22:38Z

website/docs/crd-reference.md

+
+
+
+
+


that's just how the github.com/elastic/crd-ref-docs package works AFAIK.

mandre · 2026-03-27T09:40:37Z

website/docs/crd-reference.md

+
+
+
+
+


BTW, I updated it in #716 and now we'll need to re-generate the assets.

mandre · 2026-03-27T09:45:45Z

api/v1alpha1/user_types.go

 	// +optional
 	Enabled bool `json:"enabled,omitempty"`
+
+	// passwordExpiresAt filters the response based on expriing passwords.


This does not properly describe the passwordExpiresAt status field

Also, it looks like we're not populating this new field in status.go.

mandre · 2026-03-27T10:03:31Z

internal/controllers/user/actuator.go


 	handleNameUpdate(&updateOpts, obj, osResource)
 	handleDescriptionUpdate(&updateOpts, resource, osResource)
 	handleEnabledUpdate(&updateOpts, resource, osResource)


We can leave the password update for a later time if this gets too complicated in this PR. We would just need to make the field immutable for now.

However, in case we want to permit the creation of users without providing a password as it is now, then we must return the one that OpenStack generated for us (in a secret). Otherwise, we should make the password field required (this would actually .

Add a `passwordRef` field to `UserResourceSpec` that references a Secret containing the user's password. Make this field required until we add the ability to expose auto-generated passwords from Keystone. This field is also currently immutable. Also populate `passwordExpiresAt` in status when Keystone has password expiration configured. Co-Authored-By: Martin André <m.andre@redhat.com>

mandre · 2026-03-27T14:27:17Z

@dlaw4608 I took the liberty to update your patch so that we can merge it soon, hope you don't mind. I chose to make the passwordRef field required until there's a way to expose a generated secret, and also made it immutable. We can change both of these things later. PTAL at the changes.

dlaw4608

@mandre thanks for getting this over the line, your change making the passwordRef field mandatory makes sense especially for this first iteration, the next step now is to include the ability to update the password of a user, but again that is for a future PR. LGTM I'd say it's ready to go 👍

github-actions bot added the semver:minor Backwards-compatible change label Mar 12, 2026

dlaw4608 changed the title ~~Keystone: User CreateOpts and UpdateOpts for password field~~ Keystone: User CreateOpts for password field Mar 16, 2026

dlaw4608 force-pushed the user_password branch from c76b0e8 to bc7d429 Compare March 16, 2026 22:08

dlaw4608 marked this pull request as ready for review March 16, 2026 22:09

winiciusallan reviewed Mar 16, 2026

View reviewed changes

eshulman2 reviewed Mar 17, 2026

View reviewed changes

mandre mentioned this pull request Mar 25, 2026

[Question] Maturity of the project #718

Closed

mandre modified the milestone: Release 2.5 Mar 26, 2026

mandre reviewed Mar 27, 2026

View reviewed changes

mandre force-pushed the user_password branch from bc7d429 to c77882f Compare March 27, 2026 14:22

dlaw4608 commented Mar 27, 2026

View reviewed changes

Conversation

dlaw4608 commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

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!

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

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!

mandre commented Mar 27, 2026

Uh oh!

dlaw4608 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dlaw4608 commented Mar 12, 2026 •

edited

Loading

winiciusallan left a comment •

edited

Loading