(feat) Support for creating users on cluster init by utdrmac · Pull Request #82 · valkey-io/valkey-operator

utdrmac · 2026-02-10T17:28:23Z

Resolves #36

This feature allows for the creation of Valkey users on cluster initialization. The feature abstracts Valkey ACL permissions into several objects, giving better readability, and creation flexibility to users that may not be intimately familiar with Valkey ACLs.

Signed-off-by: utdrmac <matthew.boehm@percona.com>

utdrmac · 2026-02-10T17:29:35Z

e2e tests pass in my local kind cluster

=== RUN   TestE2E
  Starting valkey-operator e2e test suite
Running Suite: e2e suite - /home/utdrmac/dockers/valkey-operator/test/e2e
=======================================================================
Random Seed: 1770743918

Will run 10 of 10 specs
------------------------------
...
Ran 10 of 10 Specs in 135.056 seconds
SUCCESS! -- 10 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestE2E (135.06s)
PASS
ok  	valkey.io/valkey-operator/test/e2e	135.065s
make cleanup-test-e2e
...

utdrmac · 2026-02-10T17:32:10Z

 var _ = BeforeSuite(func() {
-	By("building the manager image")
-	cmd := exec.Command("make", "docker-build", fmt.Sprintf("IMG=%s", managerImage))
+	By("purging old events")


K8S holds on to events for an extended period of time, even after a cluster is deleted. This call to purge events was added because subsequent e2e tests would fail if a previous test failed due to the e2e test fetching all events.

Good catch. We could move it to valkeycluster_test.go to have it close to where its needed.
This is also the only testcase/context where we use involvedObject.name=valkeycluster-sample.
I guess it needs to be in a BeforeEach instead then.

utdrmac · 2026-02-10T17:33:37Z


 .PHONY: build
-build: manifests generate fmt vet ## Build manager binary.
+build: manifests generate fmt vet lint ## Build manager binary.


The golangci-lint was only ran during github e2e tests, and it was frustrating for this to fail remotely. Simple change to check this locally first.

utdrmac

I added some comments for 2 changes outside the scope of the PR.

jdheyburn

This is great to see, thank you for putting the time in! Just left some comments for clarity

sandeepkunusoth · 2026-02-11T17:37:10Z

+	// Do not apply a password to this user
+	// +kubebuilder:default=false
+	NoPassword bool `json:"nopass,omitempty"`
+


i think passwordEnabled or enabledPassword would be good naming here instead of NoPassword. Also i am not sure this is general use case of having nopass attached to user. if they really want to acheieve this they can always use rawacl.

It might not be general or recommended, but if Valkey exposes it as an option it might be useful to abstract it away.

https://valkey.io/topics/acl/#configure-acls-with-the-acl-command

This boolean is used in the PR in a few places. If we were to remove NoPassword here, would we change those conditions to be 'nopass' in RawAcl (pseudocode) ?

I think the idea was since the ACL keyword is nopass then this follows-ish 1:1. Since the default is false if not specified, is there a strong reason to remove it? As the code is now, if removed, then passwords are always searched for even in a nopass=true case. The code would then need to check rawAcl for presence of nopass to achieve same logic.

Signed-off-by: utdrmac <matthew.boehm@percona.com>

jdheyburn · 2026-02-12T10:45:25Z

I think we're almost there with this PR, just pending @sandeepkunusoth approval from their remaining comments

bjosv · 2026-02-12T10:07:06Z

 var _ = BeforeSuite(func() {
-	By("building the manager image")
-	cmd := exec.Command("make", "docker-build", fmt.Sprintf("IMG=%s", managerImage))
+	By("purging old events")


Good catch. We could move it to valkeycluster_test.go to have it close to where its needed.
This is also the only testcase/context where we use involvedObject.name=valkeycluster-sample.
I guess it needs to be in a BeforeEach instead then.

bjosv · 2026-02-12T10:26:30Z

 	for _, shard := range s.Shards {
 		for _, slot := range shard.Slots {
-			var next []SlotsRange
+			var next []SlotsRange //nolint:prealloc


Interesting that this isn't showing in CI, our current main (given by kubebuilder) uses

make lint ... Downloading github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.7.2 0 issues.

which is using
go: downloading github.com/alexkohler/prealloc v1.0.0

Is this seen with newer versions of golangci-lint/prealloc?

Must be. I'm using golangci-lint has version 2.8.0 built with go1.25.5

bjosv

LGTM, my previous comments are nits

Signed-off-by: utdrmac <matthew.boehm@percona.com>

utdrmac · 2026-02-19T03:32:53Z

Resolved conflict with c679809

Signed-off-by: utdrmac <matthew.boehm@percona.com>

jdheyburn · 2026-02-23T17:18:01Z

@utdrmac I think there is a bad merge pulling in some functions that no longer exist. Would you mind double checking to see what can be removed? Once that's done I'll approve and merge. Thanks again!

utdrmac · 2026-02-26T15:10:09Z

Hmm, I didn't run into any issues with git merge main. 🤷 Anyooo, I merged main and reran test suite:

...
  STEP: uninstalling CRDs @ 02/26/26 15:08:00.503
  running: "make uninstall"
  STEP: removing manager namespace @ 02/26/26 15:08:01.891
  running: "kubectl delete ns valkey-operator-system"
[AfterSuite] PASSED [26.256 seconds]
------------------------------

Ran 11 of 11 Specs in 220.267 seconds
SUCCESS! -- 11 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestE2E (220.27s)
PASS
ok  	valkey.io/valkey-operator/test/e2e	220.277s

Signed-off-by: utdrmac <matthew.boehm@percona.com>

utdrmac · 2026-02-26T15:59:00Z

f976ef7 is my forgotten sign-off of 318c67e

318c67e is a quick refactor to remove the extra boolean variable, and move around the logic so it's a bit cleaner

jdheyburn · 2026-02-26T17:00:10Z

+// primary, regardless of its node-index label. This handles the post-failover
+// case where node-index=1 (or higher) was promoted by Valkey.
+// Returns ("", "") if no primary is found.
+func findShardPrimary(state *valkey.ClusterState, shardIndex int, pods *corev1.PodList) (nodeID, ip string) {


Is this, and the other functions above it, the result of a bad merge?

upsertAnnotation is added by this PR. findShardPrimary was added in d777c83 I'm not seeing any merge issues.

My apologies, the comparison I was making made it appear as something different :(

jdheyburn · 2026-02-27T12:06:25Z

Thank for your time and effort on this @utdrmac !

jdheyburn · 2026-02-27T17:32:45Z

Hey @utdrmac, I just deployed the sample cluster to a test cluster with this commit.

I am wanting to check on the expectation for the charlie user which does not have a password (although they are disabled). The logs are erroring on that there is a missing password key for the user.

2026-02-27T17:29:09Z    ERROR   missing password key in secret  {"controller": "valkeycluster", "controllerGroup": "valkey.io", "controllerKind": "ValkeyCluster", "ValkeyCluster": {"name":"valkeycluster-sample","namespace":"valkey-operator-system"}, "namespace": "valkey-operator-system", "name": "valkeycluster-sample", "reconcileID": "d110d30b-4d32-45ff-adae-023fbb440395", "user": "charlie", "secret": "valkeycluster-sample-users", "key": "charlie"}

utdrmac · 2026-02-27T19:23:54Z

@jdheyburn That is correct due to the logic flow. The first thing checked for is a password. If NoPassword is true, fetchUserPasswords exits early, and then then continues building the user. If NoPassword is false, (as is the case for charlie), the search inside the Secret takes place. This returns error because we can't add a user without password. If charlie did have a password, or NoPassword was true, he would get added in disabled ("user charlie off").

I guess it comes down to a design decision: Should users that are disabled appear in the ACL file? If yes, (current behavior), then the error is correct. If no, then the logic can be changed to completely skip users which are disabled (and print info message that user was skipped due to being disabled).

Resolves valkey-io#36 This feature allows for the creation of Valkey users on cluster initialization. The feature abstracts Valkey ACL permissions into several objects, giving better readability, and creation flexibility to users that may not be intimately familiar with Valkey ACLs. --------- Signed-off-by: utdrmac <matthew.boehm@percona.com>

hieu2102 · 2026-03-10T03:48:21Z

@utdrmac I think we should still create users if no password is provided. Otherwise modules that override the authentication flow (i.e valkey-ldap) will not works

sandeepkunusoth · 2026-03-10T04:18:36Z

@utdrmac I think we should still create users if no password is provided. Otherwise modules that override the authentication flow (i.e valkey-ldap) will not works

for nopass users we need to allow users with resetpass

jdheyburn · 2026-03-10T09:13:41Z

for nopass users we need to allow users with resetpass

@sandeepkunusoth Can you expand on the workflow you're describing and how it might be used?

@utdrmac @hieu2102 I think we should still create the user, but perhaps the logs could be tidied. If NoPass is defined then we don't need to error out, because we're not expecting a password.

utdrmac · 2026-03-10T19:22:37Z

@hieu2102 Gotcha; easy enough to change
@jdheyburn That makes sense on the logs.

create users on cluster init

6851fe9

Signed-off-by: utdrmac <matthew.boehm@percona.com>

utdrmac commented Feb 10, 2026

View reviewed changes

sandeepkunusoth reviewed Feb 11, 2026

View reviewed changes

Comment thread internal/controller/valkeycluster_controller.go

jdheyburn reviewed Feb 11, 2026

View reviewed changes

sandeepkunusoth reviewed Feb 11, 2026

View reviewed changes

Comment thread internal/controller/utils.go Outdated

updates from feedback

68a0a18

Signed-off-by: utdrmac <matthew.boehm@percona.com>

bjosv reviewed Feb 12, 2026

View reviewed changes

bjosv previously approved these changes Feb 13, 2026

View reviewed changes

merge origin/main

c679809

Signed-off-by: utdrmac <matthew.boehm@percona.com>

fmt fix

09cc75c

Signed-off-by: utdrmac <matthew.boehm@percona.com>

utdrmac dismissed bjosv’s stale review via 09cc75c February 19, 2026 14:44

Merge branch 'main' into users-cluster-init

c60ae86

refactor

f976ef7

Signed-off-by: utdrmac <matthew.boehm@percona.com>

utdrmac force-pushed the users-cluster-init branch from 318c67e to f976ef7 Compare February 26, 2026 15:57

jdheyburn reviewed Feb 26, 2026

View reviewed changes

jdheyburn approved these changes Feb 27, 2026

View reviewed changes

jdheyburn merged commit ed15c7f into valkey-io:main Feb 27, 2026
4 checks passed

utdrmac deleted the users-cluster-init branch March 30, 2026 22:07

hieu2102 mentioned this pull request Apr 13, 2026

Allow creating users with resetpass flag #134

Open

This was referenced Apr 15, 2026

reconcileUsersAcl silently skips users with missing password secrets #138

Closed

reconcileUsersAcl silently skips users with missing password secrets #139

Closed

Conversation

utdrmac commented Feb 10, 2026

Uh oh!

utdrmac commented Feb 10, 2026

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!

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

jdheyburn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

jdheyburn commented Feb 12, 2026

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!

bjosv left a comment

Choose a reason for hiding this comment

Uh oh!

utdrmac commented Feb 19, 2026

Uh oh!

jdheyburn commented Feb 23, 2026

Uh oh!

utdrmac commented Feb 26, 2026

Uh oh!

utdrmac commented Feb 26, 2026

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!

jdheyburn commented Feb 27, 2026

Uh oh!

jdheyburn commented Feb 27, 2026

Uh oh!

utdrmac commented Feb 27, 2026

Uh oh!

hieu2102 commented Mar 10, 2026

Uh oh!

sandeepkunusoth commented Mar 10, 2026

Uh oh!

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

Uh oh!

Uh oh!

utdrmac commented Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

utdrmac left a comment •

edited

Loading

jdheyburn commented Mar 10, 2026 •

edited

Loading