From 5dc3f404020d5d6c4e155fe72a3bc2f6f475b00e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 30 Mar 2026 13:27:11 +0200 Subject: [PATCH] user: make passwordRef optional Keystone allows creating passwordless users for authentication via federation, application credentials, or other means. Make passwordRef optional so ORC can create users without a password. A CEL validation on UserResourceSpec prevents removing passwordRef once set, since Keystone does not support clearing a password via the API. The field remains mutable (can be changed to a different Secret). --- api/v1alpha1/user_types.go | 6 +++-- api/v1alpha1/zz_generated.deepcopy.go | 5 ++++ cmd/models-schema/zz_generated.openapi.go | 3 +-- .../bases/openstack.k-orc.cloud_users.yaml | 6 +++-- internal/controllers/user/actuator.go | 10 +++---- internal/controllers/user/controller.go | 4 +-- .../00-create-resource.yaml | 11 +------- .../user/tests/user-create-minimal/README.md | 2 +- .../00-import-resource.yaml | 8 ------ .../01-create-trap-resource.yaml | 3 +-- .../02-create-resource.yaml | 3 +-- .../00-create-resources.yaml | 12 +-------- .../tests/user-import/00-import-resource.yaml | 8 ------ .../user-import/01-create-trap-resource.yaml | 3 +-- .../tests/user-import/02-create-resource.yaml | 3 +-- test/apivalidations/user_test.go | 26 ++++++++++++++----- website/docs/crd-reference.md | 2 +- 17 files changed, 49 insertions(+), 66 deletions(-) diff --git a/api/v1alpha1/user_types.go b/api/v1alpha1/user_types.go index 38acfc92d..e085006d9 100644 --- a/api/v1alpha1/user_types.go +++ b/api/v1alpha1/user_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 // UserResourceSpec contains the desired state of the resource. +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.passwordRef) || has(self.passwordRef)",message="passwordRef may not be removed once set" type UserResourceSpec struct { // name will be the name of the created resource. If not specified, the // name of the ORC object will be used. @@ -45,8 +46,9 @@ type UserResourceSpec struct { // passwordRef is a reference to a Secret containing the password // for this user. The Secret must contain a key named "password". - // +required - PasswordRef KubernetesNameRef `json:"passwordRef,omitempty"` + // If not specified, the user is created without a password. + // +optional + PasswordRef *KubernetesNameRef `json:"passwordRef,omitempty"` } // UserFilter defines an existing resource by its properties diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 4bd75eca4..dffa3ec73 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -5924,6 +5924,11 @@ func (in *UserResourceSpec) DeepCopyInto(out *UserResourceSpec) { *out = new(bool) **out = **in } + if in.PasswordRef != nil { + in, out := &in.PasswordRef, &out.PasswordRef + *out = new(KubernetesNameRef) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserResourceSpec. diff --git a/cmd/models-schema/zz_generated.openapi.go b/cmd/models-schema/zz_generated.openapi.go index 6190bb940..2c1ee063e 100644 --- a/cmd/models-schema/zz_generated.openapi.go +++ b/cmd/models-schema/zz_generated.openapi.go @@ -11395,13 +11395,12 @@ func schema_openstack_resource_controller_v2_api_v1alpha1_UserResourceSpec(ref c }, "passwordRef": { SchemaProps: spec.SchemaProps{ - Description: "passwordRef is a reference to a Secret containing the password for this user. The Secret must contain a key named \"password\".", + Description: "passwordRef is a reference to a Secret containing the password for this user. The Secret must contain a key named \"password\". If not specified, the user is created without a password.", Type: []string{"string"}, Format: "", }, }, }, - Required: []string{"passwordRef"}, }, }, } diff --git a/config/crd/bases/openstack.k-orc.cloud_users.yaml b/config/crd/bases/openstack.k-orc.cloud_users.yaml index 90bc76760..e9dc0fa8c 100644 --- a/config/crd/bases/openstack.k-orc.cloud_users.yaml +++ b/config/crd/bases/openstack.k-orc.cloud_users.yaml @@ -190,12 +190,14 @@ spec: description: |- passwordRef is a reference to a Secret containing the password for this user. The Secret must contain a key named "password". + If not specified, the user is created without a password. maxLength: 253 minLength: 1 type: string - required: - - passwordRef type: object + x-kubernetes-validations: + - message: passwordRef may not be removed once set + rule: '!has(oldSelf.passwordRef) || has(self.passwordRef)' required: - cloudCredentialsRef type: object diff --git a/internal/controllers/user/actuator.go b/internal/controllers/user/actuator.go index b556a6f80..d82014cab 100644 --- a/internal/controllers/user/actuator.go +++ b/internal/controllers/user/actuator.go @@ -140,10 +140,10 @@ func (actuator userActuator) CreateResource(ctx context.Context, obj orcObjectPT } var password string - { + if resource.PasswordRef != nil { secret, secretReconcileStatus := dependency.FetchDependency( ctx, actuator.k8sClient, obj.Namespace, - &resource.PasswordRef, "Secret", + resource.PasswordRef, "Secret", func(*corev1.Secret) bool { return true }, ) reconcileStatus = reconcileStatus.WithReconcileStatus(secretReconcileStatus) @@ -189,11 +189,11 @@ func (actuator userActuator) DeleteResource(ctx context.Context, _ orcObjectPT, func (actuator userActuator) reconcilePassword(ctx context.Context, obj orcObjectPT, osResource *osResourceT) progress.ReconcileStatus { log := ctrl.LoggerFrom(ctx) resource := obj.Spec.Resource - if resource == nil { + if resource == nil || resource.PasswordRef == nil { return nil } - currentRef := string(resource.PasswordRef) + currentRef := string(*resource.PasswordRef) var lastAppliedRef string if obj.Status.Resource != nil { lastAppliedRef = obj.Status.Resource.AppliedPasswordRef @@ -206,7 +206,7 @@ func (actuator userActuator) reconcilePassword(ctx context.Context, obj orcObjec // Read the password from the referenced Secret secret, secretRS := dependency.FetchDependency( ctx, actuator.k8sClient, obj.Namespace, - &resource.PasswordRef, "Secret", + resource.PasswordRef, "Secret", func(*corev1.Secret) bool { return true }, ) if secretRS != nil { diff --git a/internal/controllers/user/controller.go b/internal/controllers/user/controller.go index f69818726..e0f106927 100644 --- a/internal/controllers/user/controller.go +++ b/internal/controllers/user/controller.go @@ -91,10 +91,10 @@ var passwordDependency = dependency.NewDependency[*orcv1alpha1.UserList, *corev1 "spec.resource.passwordRef", func(user *orcv1alpha1.User) []string { resource := user.Spec.Resource - if resource == nil { + if resource == nil || resource.PasswordRef == nil { return nil } - return []string{string(resource.PasswordRef)} + return []string{string(*resource.PasswordRef)} }, ) diff --git a/internal/controllers/user/tests/user-create-minimal/00-create-resource.yaml b/internal/controllers/user/tests/user-create-minimal/00-create-resource.yaml index 72545e48c..c3d2147bf 100644 --- a/internal/controllers/user/tests/user-create-minimal/00-create-resource.yaml +++ b/internal/controllers/user/tests/user-create-minimal/00-create-resource.yaml @@ -1,12 +1,4 @@ --- -apiVersion: v1 -kind: Secret -metadata: - name: user-create-minimal -type: Opaque -stringData: - password: "TestPassword" ---- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: User metadata: @@ -16,5 +8,4 @@ spec: cloudName: openstack-admin secretName: openstack-clouds managementPolicy: managed - resource: - passwordRef: user-create-minimal \ No newline at end of file + resource: {} \ No newline at end of file diff --git a/internal/controllers/user/tests/user-create-minimal/README.md b/internal/controllers/user/tests/user-create-minimal/README.md index 4d3dd61ff..548da3f08 100644 --- a/internal/controllers/user/tests/user-create-minimal/README.md +++ b/internal/controllers/user/tests/user-create-minimal/README.md @@ -2,7 +2,7 @@ ## Step 00 -Create a minimal User, that sets only the required fields, and verify that the observed state corresponds to the spec. +Create a minimal User without a password, and verify that the observed state corresponds to the spec. Also validate that the OpenStack resource uses the name of the ORC object when no name is explicitly specified. diff --git a/internal/controllers/user/tests/user-import-dependency/00-import-resource.yaml b/internal/controllers/user/tests/user-import-dependency/00-import-resource.yaml index 6001315f4..0681c805b 100644 --- a/internal/controllers/user/tests/user-import-dependency/00-import-resource.yaml +++ b/internal/controllers/user/tests/user-import-dependency/00-import-resource.yaml @@ -1,12 +1,4 @@ --- -apiVersion: v1 -kind: Secret -metadata: - name: user-import-dependency-password -type: Opaque -stringData: - password: "TestPassword" ---- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: Domain metadata: diff --git a/internal/controllers/user/tests/user-import-dependency/01-create-trap-resource.yaml b/internal/controllers/user/tests/user-import-dependency/01-create-trap-resource.yaml index 48536462a..7154af7ba 100644 --- a/internal/controllers/user/tests/user-import-dependency/01-create-trap-resource.yaml +++ b/internal/controllers/user/tests/user-import-dependency/01-create-trap-resource.yaml @@ -21,5 +21,4 @@ spec: secretName: openstack-clouds managementPolicy: managed resource: - domainRef: user-import-dependency-not-this-one - passwordRef: user-import-dependency-password \ No newline at end of file + domainRef: user-import-dependency-not-this-one \ No newline at end of file diff --git a/internal/controllers/user/tests/user-import-dependency/02-create-resource.yaml b/internal/controllers/user/tests/user-import-dependency/02-create-resource.yaml index 51c32bb06..ea64cab75 100644 --- a/internal/controllers/user/tests/user-import-dependency/02-create-resource.yaml +++ b/internal/controllers/user/tests/user-import-dependency/02-create-resource.yaml @@ -20,5 +20,4 @@ spec: secretName: openstack-clouds managementPolicy: managed resource: - domainRef: user-import-dependency-external - passwordRef: user-import-dependency-password \ No newline at end of file + domainRef: user-import-dependency-external \ No newline at end of file diff --git a/internal/controllers/user/tests/user-import-error/00-create-resources.yaml b/internal/controllers/user/tests/user-import-error/00-create-resources.yaml index 10e809d1a..498801786 100644 --- a/internal/controllers/user/tests/user-import-error/00-create-resources.yaml +++ b/internal/controllers/user/tests/user-import-error/00-create-resources.yaml @@ -10,14 +10,6 @@ spec: managementPolicy: managed resource: {} --- -apiVersion: v1 -kind: Secret -metadata: - name: user-import-error-password -type: Opaque -stringData: - password: "TestPassword" ---- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: User metadata: @@ -30,7 +22,6 @@ spec: resource: description: User from "import error" test domainRef: user-import-error-domain - passwordRef: user-import-error-password --- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: User @@ -43,5 +34,4 @@ spec: managementPolicy: managed resource: description: User from "import error" test - domainRef: user-import-error-domain - passwordRef: user-import-error-password \ No newline at end of file + domainRef: user-import-error-domain \ No newline at end of file diff --git a/internal/controllers/user/tests/user-import/00-import-resource.yaml b/internal/controllers/user/tests/user-import/00-import-resource.yaml index a80dc427a..d8b7199fa 100644 --- a/internal/controllers/user/tests/user-import/00-import-resource.yaml +++ b/internal/controllers/user/tests/user-import/00-import-resource.yaml @@ -1,12 +1,4 @@ --- -apiVersion: v1 -kind: Secret -metadata: - name: user-import-password -type: Opaque -stringData: - password: "TestPassword" ---- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: Domain metadata: diff --git a/internal/controllers/user/tests/user-import/01-create-trap-resource.yaml b/internal/controllers/user/tests/user-import/01-create-trap-resource.yaml index 18ae8d89a..ea393341f 100644 --- a/internal/controllers/user/tests/user-import/01-create-trap-resource.yaml +++ b/internal/controllers/user/tests/user-import/01-create-trap-resource.yaml @@ -13,5 +13,4 @@ spec: managementPolicy: managed resource: description: User user-import-external from "user-import" test - domainRef: user-import-external - passwordRef: user-import-password \ No newline at end of file + domainRef: user-import-external \ No newline at end of file diff --git a/internal/controllers/user/tests/user-import/02-create-resource.yaml b/internal/controllers/user/tests/user-import/02-create-resource.yaml index ca3cb03fc..43a43ef04 100644 --- a/internal/controllers/user/tests/user-import/02-create-resource.yaml +++ b/internal/controllers/user/tests/user-import/02-create-resource.yaml @@ -10,5 +10,4 @@ spec: managementPolicy: managed resource: description: User user-import-external from "user-import" test - domainRef: user-import-external - passwordRef: user-import-password \ No newline at end of file + domainRef: user-import-external \ No newline at end of file diff --git a/test/apivalidations/user_test.go b/test/apivalidations/user_test.go index f38671d01..983c778ab 100644 --- a/test/apivalidations/user_test.go +++ b/test/apivalidations/user_test.go @@ -41,8 +41,7 @@ func userStub(namespace *corev1.Namespace) *orcv1alpha1.User { } func testUserResource() *applyconfigv1alpha1.UserResourceSpecApplyConfiguration { - return applyconfigv1alpha1.UserResourceSpec(). - WithPasswordRef("user-password") + return applyconfigv1alpha1.UserResourceSpec() } func baseUserPatch(user client.Object) *applyconfigv1alpha1.UserApplyConfiguration { @@ -96,12 +95,10 @@ var _ = Describe("ORC User API validations", func() { user := userStub(namespace) patch := baseUserPatch(user) patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). - WithPasswordRef("user-password"). WithDomainRef("domain-a")) Expect(applyObj(ctx, user, patch)).To(Succeed()) patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). - WithPasswordRef("user-password"). WithDomainRef("domain-b")) Expect(applyObj(ctx, user, patch)).To(MatchError(ContainSubstring("domainRef is immutable"))) }) @@ -110,16 +107,21 @@ var _ = Describe("ORC User API validations", func() { user := userStub(namespace) patch := baseUserPatch(user) patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). - WithPasswordRef("user-password"). WithDefaultProjectRef("project-a")) Expect(applyObj(ctx, user, patch)).To(Succeed()) patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). - WithPasswordRef("user-password"). WithDefaultProjectRef("project-b")) Expect(applyObj(ctx, user, patch)).To(MatchError(ContainSubstring("defaultProjectRef is immutable"))) }) + It("should allow omitting passwordRef", func(ctx context.Context) { + user := userStub(namespace) + patch := baseUserPatch(user) + patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec()) + Expect(applyObj(ctx, user, patch)).To(Succeed()) + }) + It("should have mutable passwordRef", func(ctx context.Context) { user := userStub(namespace) patch := baseUserPatch(user) @@ -131,4 +133,16 @@ var _ = Describe("ORC User API validations", func() { WithPasswordRef("password-b")) Expect(applyObj(ctx, user, patch)).To(Succeed()) }) + + It("should not allow removing passwordRef once set", func(ctx context.Context) { + user := userStub(namespace) + patch := baseUserPatch(user) + patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). + WithPasswordRef("password-a")) + Expect(applyObj(ctx, user, patch)).To(Succeed()) + + patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). + WithDescription("updated")) + Expect(applyObj(ctx, user, patch)).To(MatchError(ContainSubstring("passwordRef may not be removed once set"))) + }) }) diff --git a/website/docs/crd-reference.md b/website/docs/crd-reference.md index 9d15a005a..aee5bb6d7 100644 --- a/website/docs/crd-reference.md +++ b/website/docs/crd-reference.md @@ -4436,7 +4436,7 @@ _Appears in:_ | `domainRef` _[KubernetesNameRef](#kubernetesnameref)_ | domainRef is a reference to the ORC Domain which this resource is associated with. | | MaxLength: 253
MinLength: 1
Optional: \{\}
| | `defaultProjectRef` _[KubernetesNameRef](#kubernetesnameref)_ | defaultProjectRef is a reference to the Default Project which this resource is associated with. | | MaxLength: 253
MinLength: 1
Optional: \{\}
| | `enabled` _boolean_ | enabled defines whether a user is enabled or disabled | | Optional: \{\}
| -| `passwordRef` _[KubernetesNameRef](#kubernetesnameref)_ | passwordRef is a reference to a Secret containing the password
for this user. The Secret must contain a key named "password". | | MaxLength: 253
MinLength: 1
Required: \{\}
| +| `passwordRef` _[KubernetesNameRef](#kubernetesnameref)_ | passwordRef is a reference to a Secret containing the password
for this user. The Secret must contain a key named "password".
If not specified, the user is created without a password. | | MaxLength: 253
MinLength: 1
Optional: \{\}
| #### UserResourceStatus