From 7f2a68fa91ea383a1f8f4481c8ac9eaa64eaa296 Mon Sep 17 00:00:00 2001 From: Anas Khan <83116240+anxkhn@users.noreply.github.com> Date: Mon, 6 Jul 2026 02:19:30 +0530 Subject: [PATCH] object: return an error when reconciled rgw user has no keys In createOrUpdateCephUser, when the desired user carries no explicit keys the reconciler falls back to the live RGW user's keys. If that live user also has zero keys the code intends to fail, but it built the error with errors.Wrapf(err, ...) at a point where err is already nil (the prior SetUserQuota error was handled and returned just above). errors.Wrapf(nil, ...) returns nil, so the failure was swallowed and createOrUpdateCephUser returned success on a user the operator itself flagged as broken. The reconcile then continued to generateCephUserSecret, which indexes userConfig.Keys[0] to populate the Kubernetes secret and panicked on the empty key slice. The reconciler's deferred RecoverAndLogException caught and logged that panic, so the reconcile was abandoned before it reached the Ready status update; and because the recovered Reconcile returns a zero Result with a nil error, the request was not requeued either. The user was left neither marked Ready nor retried. Construct the error with errors.Errorf so the intended failure is surfaced instead of being swallowed. Add a regression test that returns a keyless live user and asserts a non-nil "no keys set" error. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com> Signed-off-by: Joshua Hoblitt --- pkg/operator/ceph/object/user/controller.go | 2 +- .../ceph/object/user/controller_test.go | 73 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/pkg/operator/ceph/object/user/controller.go b/pkg/operator/ceph/object/user/controller.go index a5840095c..3cdb9fa49 100644 --- a/pkg/operator/ceph/object/user/controller.go +++ b/pkg/operator/ceph/object/user/controller.go @@ -507,7 +507,7 @@ func (r *ReconcileObjectStoreUser) createOrUpdateCephUser(u *cephv1.CephObjectSt // use the keys already set on the user & remove all but one key if len(liveUser.Keys) == 0 { // something is wrong, there should be at least one key - return errors.Wrapf(err, "no keys set for user %q", u.Name) + return errors.Errorf("no keys set for user %q", u.Name) } targetUser.Keys = []admin.UserKeySpec{liveUser.Keys[0]} diff --git a/pkg/operator/ceph/object/user/controller_test.go b/pkg/operator/ceph/object/user/controller_test.go index 7bc78ae8a..2768d22f6 100644 --- a/pkg/operator/ceph/object/user/controller_test.go +++ b/pkg/operator/ceph/object/user/controller_test.go @@ -650,6 +650,79 @@ func TestGenerateUserConfigCephVersionChecks(t *testing.T) { }) } +func TestCreateOrUpdateCephUserNoKeys(t *testing.T) { + // Set DEBUG logging + capnslog.SetGlobalLogLevel(capnslog.DEBUG) + + objectUser := &cephv1.CephObjectStoreUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: cephv1.ObjectStoreUserSpec{ + Store: store, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "CephObjectStoreUser", + }, + } + + // A live RGW user the API returned with no access keys. The user spec also + // carries no explicit keys, so createOrUpdateCephUser falls back to the live + // user's keys and must surface an error since there are none. + //nolint:gosec // only test values, not a real secret + userNoKeysJSON := `{ + "user_id": "my-user", + "display_name": "my-user", + "max_buckets": 1000, + "keys": [], + "swift_keys": [], + "caps": [], + "op_mask": "read, write, delete", + "bucket_quota": {"enabled": false, "max_size": -1, "max_objects": -1}, + "user_quota": {"enabled": false, "max_size": -1, "max_objects": -1}, + "type": "rgw" +}` + + mockClient := &cephobject.MockClient{ + MockDo: func(req *http.Request) (*http.Response, error) { + if req.URL.Path != "rook-ceph-rgw-my-store.mycluster.svc/admin/user" { + return nil, fmt.Errorf("unexpected url path %q", req.URL.Path) + } + + if req.Method == http.MethodGet && req.URL.RawQuery == "format=json&uid=my-user" { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte(userNoKeysJSON))), + }, nil + } + + if req.Method == http.MethodPut && req.URL.RawQuery == "enabled=false&format=json&max-objects=-1&max-size=-1"a="a-type=user&uid=my-user" { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader([]byte(userNoKeysJSON))), + }, nil + } + + return nil, fmt.Errorf("unexpected request: %q. method %q. path %q", req.URL.RawQuery, req.Method, req.URL.Path) + }, + } + adminClient, err := admin.New("rook-ceph-rgw-my-store.mycluster.svc", "53S6B9S809NUP19IJ2K3", "1bXPegzsGClvoGAiJdHQD1uOW2sQBLAZM9j9VtXR", mockClient) + assert.NoError(t, err) + r := &ReconcileObjectStoreUser{ + objContext: &cephobject.AdminOpsContext{ + AdminOpsClient: adminClient, + }, + opManagerContext: context.TODO(), + } + + userConfig, err := generateUserConfig(objectUser, cephver.Minimum) + require.NoError(t, err) + err = r.createOrUpdateCephUser(objectUser, userConfig) + require.Error(t, err) + assert.Contains(t, err.Error(), "no keys set for user") +} + func TestValidateUser(t *testing.T) { r := &ReconcileObjectStoreUser{}