From 28a09a40e006e2baf99d849e010914d192e25ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 30 Mar 2026 11:43:23 +0200 Subject: [PATCH] user: implement password mutability via passwordRef Make the passwordRef field mutable so users can update passwords by pointing to a different Secret. Track the applied password reference in a new status field (appliedPasswordRef) to detect when an update is needed. The reconciler compares spec.resource.passwordRef with status.resource.appliedPasswordRef. On first reconcile after creation, it sets the status field without calling UpdateUser (CreateResource already set the initial password). On subsequent changes, it reads the new Secret, calls UpdateUser, and updates the status field via a MergePatch that coexists with the main SSA status update. --- api/v1alpha1/user_types.go | 7 +- cmd/models-schema/zz_generated.openapi.go | 7 ++ .../bases/openstack.k-orc.cloud_users.yaml | 9 ++- internal/controllers/user/actuator.go | 73 +++++++++++++++++++ .../user/tests/user-update/00-assert.yaml | 1 + .../user/tests/user-update/01-assert.yaml | 1 + .../user-update/01-updated-resource.yaml | 9 +++ .../user/tests/user-update/02-assert.yaml | 1 + .../user/tests/user-update/README.md | 2 +- .../api/v1alpha1/userresourcestatus.go | 21 ++++-- .../applyconfiguration/internal/internal.go | 3 + test/apivalidations/user_test.go | 4 +- website/docs/crd-reference.md | 1 + 13 files changed, 126 insertions(+), 13 deletions(-) diff --git a/api/v1alpha1/user_types.go b/api/v1alpha1/user_types.go index ddccb96e2..38acfc92d 100644 --- a/api/v1alpha1/user_types.go +++ b/api/v1alpha1/user_types.go @@ -46,7 +46,6 @@ 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 - // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="passwordRef is immutable" PasswordRef KubernetesNameRef `json:"passwordRef,omitempty"` } @@ -92,4 +91,10 @@ type UserResourceStatus struct { // +kubebuilder:validation:MaxLength:=1024 // +optional PasswordExpiresAt string `json:"passwordExpiresAt,omitempty"` + + // appliedPasswordRef is the name of the Secret containing the + // password that was last applied to the OpenStack resource. + // +kubebuilder:validation:MaxLength=1024 + // +optional + AppliedPasswordRef string `json:"appliedPasswordRef,omitempty"` } diff --git a/cmd/models-schema/zz_generated.openapi.go b/cmd/models-schema/zz_generated.openapi.go index e13e9899b..6190bb940 100644 --- a/cmd/models-schema/zz_generated.openapi.go +++ b/cmd/models-schema/zz_generated.openapi.go @@ -11456,6 +11456,13 @@ func schema_openstack_resource_controller_v2_api_v1alpha1_UserResourceStatus(ref Format: "", }, }, + "appliedPasswordRef": { + SchemaProps: spec.SchemaProps{ + Description: "appliedPasswordRef is the name of the Secret containing the password that was last applied to the OpenStack resource.", + Type: []string{"string"}, + Format: "", + }, + }, }, }, }, diff --git a/config/crd/bases/openstack.k-orc.cloud_users.yaml b/config/crd/bases/openstack.k-orc.cloud_users.yaml index 201bb5eba..90bc76760 100644 --- a/config/crd/bases/openstack.k-orc.cloud_users.yaml +++ b/config/crd/bases/openstack.k-orc.cloud_users.yaml @@ -193,9 +193,6 @@ spec: maxLength: 253 minLength: 1 type: string - x-kubernetes-validations: - - message: passwordRef is immutable - rule: self == oldSelf required: - passwordRef type: object @@ -302,6 +299,12 @@ spec: description: resource contains the observed state of the OpenStack resource. properties: + appliedPasswordRef: + description: |- + appliedPasswordRef is the name of the Secret containing the + password that was last applied to the OpenStack resource. + maxLength: 1024 + type: string defaultProjectID: description: defaultProjectID is the ID of the Default Project to which the user is associated with. diff --git a/internal/controllers/user/actuator.go b/internal/controllers/user/actuator.go index 4b4683fe5..b556a6f80 100644 --- a/internal/controllers/user/actuator.go +++ b/internal/controllers/user/actuator.go @@ -22,6 +22,7 @@ import ( "github.com/gophercloud/gophercloud/v2/openstack/identity/v3/users" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -31,8 +32,10 @@ import ( "github.com/k-orc/openstack-resource-controller/v2/internal/controllers/generic/progress" "github.com/k-orc/openstack-resource-controller/v2/internal/logging" "github.com/k-orc/openstack-resource-controller/v2/internal/osclients" + "github.com/k-orc/openstack-resource-controller/v2/internal/util/applyconfigs" "github.com/k-orc/openstack-resource-controller/v2/internal/util/dependency" orcerrors "github.com/k-orc/openstack-resource-controller/v2/internal/util/errors" + orcapplyconfigv1alpha1 "github.com/k-orc/openstack-resource-controller/v2/pkg/clients/applyconfiguration/api/v1alpha1" ) // OpenStack resource types @@ -183,6 +186,75 @@ func (actuator userActuator) DeleteResource(ctx context.Context, _ orcObjectPT, return progress.WrapError(actuator.osClient.DeleteUser(ctx, resource.ID)) } +func (actuator userActuator) reconcilePassword(ctx context.Context, obj orcObjectPT, osResource *osResourceT) progress.ReconcileStatus { + log := ctrl.LoggerFrom(ctx) + resource := obj.Spec.Resource + if resource == nil { + return nil + } + + currentRef := string(resource.PasswordRef) + var lastAppliedRef string + if obj.Status.Resource != nil { + lastAppliedRef = obj.Status.Resource.AppliedPasswordRef + } + + if lastAppliedRef == currentRef { + return nil + } + + // Read the password from the referenced Secret + secret, secretRS := dependency.FetchDependency( + ctx, actuator.k8sClient, obj.Namespace, + &resource.PasswordRef, "Secret", + func(*corev1.Secret) bool { return true }, + ) + if secretRS != nil { + return secretRS + } + + passwordBytes, ok := secret.Data["password"] + if !ok { + return progress.NewReconcileStatus().WithProgressMessage("Password secret does not contain \"password\" key") + } + password := string(passwordBytes) + + // Only call UpdateUser if this is not the first reconcile after creation. + // CreateResource already set the initial password. + if lastAppliedRef != "" { + log.V(logging.Info).Info("Updating password") + _, err := actuator.osClient.UpdateUser(ctx, osResource.ID, users.UpdateOpts{ + Password: password, + }) + + if orcerrors.IsConflict(err) { + err = orcerrors.Terminal(orcv1alpha1.ConditionReasonInvalidConfiguration, "invalid configuration updating resource: "+err.Error(), err) + } + if err != nil { + return progress.WrapError(err) + } + } + + // Update the lastAppliedPasswordRef status field via a MergePatch. + // MergePatch sets only the specified fields without claiming SSA + // ownership, so the main SSA status update won't remove this field. + statusApply := orcapplyconfigv1alpha1.UserResourceStatus(). + WithAppliedPasswordRef(currentRef) + applyConfig := orcapplyconfigv1alpha1.User(obj.Name, obj.Namespace). + WithUID(obj.UID). + WithStatus(orcapplyconfigv1alpha1.UserStatus(). + WithResource(statusApply)) + if err := actuator.k8sClient.Status().Patch(ctx, obj, + applyconfigs.Patch(types.MergePatchType, applyConfig)); err != nil { + return progress.WrapError(err) + } + + if lastAppliedRef != "" { + return progress.NeedsRefresh() + } + return nil +} + func (actuator userActuator) updateResource(ctx context.Context, obj orcObjectPT, osResource *osResourceT) progress.ReconcileStatus { log := ctrl.LoggerFrom(ctx) resource := obj.Spec.Resource @@ -259,6 +331,7 @@ func handleEnabledUpdate(updateOpts *users.UpdateOpts, resource *resourceSpecT, func (actuator userActuator) GetResourceReconcilers(ctx context.Context, orcObject orcObjectPT, osResource *osResourceT, controller interfaces.ResourceController) ([]resourceReconciler, progress.ReconcileStatus) { return []resourceReconciler{ + actuator.reconcilePassword, actuator.updateResource, }, nil } diff --git a/internal/controllers/user/tests/user-update/00-assert.yaml b/internal/controllers/user/tests/user-update/00-assert.yaml index c7a2749fc..e30fd7137 100644 --- a/internal/controllers/user/tests/user-update/00-assert.yaml +++ b/internal/controllers/user/tests/user-update/00-assert.yaml @@ -12,6 +12,7 @@ assertAll: - celExpr: "!has(user.status.resource.defaultProjectID)" # passwordExpiresAt depends on the Keystone security_compliance # configuration and is not asserted here. + - celExpr: "user.status.resource.appliedPasswordRef == 'user-update'" --- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: User diff --git a/internal/controllers/user/tests/user-update/01-assert.yaml b/internal/controllers/user/tests/user-update/01-assert.yaml index 0a9fa6937..cf594b6ee 100644 --- a/internal/controllers/user/tests/user-update/01-assert.yaml +++ b/internal/controllers/user/tests/user-update/01-assert.yaml @@ -8,6 +8,7 @@ status: name: user-update-updated description: user-update-updated enabled: false + appliedPasswordRef: user-update-password-updated conditions: - type: Available status: "True" diff --git a/internal/controllers/user/tests/user-update/01-updated-resource.yaml b/internal/controllers/user/tests/user-update/01-updated-resource.yaml index dea4f5476..dd7727629 100644 --- a/internal/controllers/user/tests/user-update/01-updated-resource.yaml +++ b/internal/controllers/user/tests/user-update/01-updated-resource.yaml @@ -1,4 +1,12 @@ --- +apiVersion: v1 +kind: Secret +metadata: + name: user-update-password-updated +type: Opaque +stringData: + password: "TestPasswordUpdated" +--- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: User metadata: @@ -8,3 +16,4 @@ spec: name: user-update-updated description: user-update-updated enabled: false + passwordRef: user-update-password-updated diff --git a/internal/controllers/user/tests/user-update/02-assert.yaml b/internal/controllers/user/tests/user-update/02-assert.yaml index c2c14d837..7682f1636 100644 --- a/internal/controllers/user/tests/user-update/02-assert.yaml +++ b/internal/controllers/user/tests/user-update/02-assert.yaml @@ -10,6 +10,7 @@ assertAll: - celExpr: "!has(user.status.resource.description)" # passwordExpiresAt depends on the Keystone security_compliance # configuration and is not asserted here. + - celExpr: "user.status.resource.appliedPasswordRef == 'user-update'" --- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: User diff --git a/internal/controllers/user/tests/user-update/README.md b/internal/controllers/user/tests/user-update/README.md index 13da45548..160b0122d 100644 --- a/internal/controllers/user/tests/user-update/README.md +++ b/internal/controllers/user/tests/user-update/README.md @@ -6,7 +6,7 @@ Create a User using only mandatory fields. ## Step 01 -Update all mutable fields. +Update all mutable fields, including passwordRef (pointing to a new Secret). ## Step 02 diff --git a/pkg/clients/applyconfiguration/api/v1alpha1/userresourcestatus.go b/pkg/clients/applyconfiguration/api/v1alpha1/userresourcestatus.go index c23b0b6cf..db56adfbf 100644 --- a/pkg/clients/applyconfiguration/api/v1alpha1/userresourcestatus.go +++ b/pkg/clients/applyconfiguration/api/v1alpha1/userresourcestatus.go @@ -21,12 +21,13 @@ package v1alpha1 // UserResourceStatusApplyConfiguration represents a declarative configuration of the UserResourceStatus type for use // with apply. type UserResourceStatusApplyConfiguration struct { - Name *string `json:"name,omitempty"` - Description *string `json:"description,omitempty"` - DomainID *string `json:"domainID,omitempty"` - DefaultProjectID *string `json:"defaultProjectID,omitempty"` - Enabled *bool `json:"enabled,omitempty"` - PasswordExpiresAt *string `json:"passwordExpiresAt,omitempty"` + Name *string `json:"name,omitempty"` + Description *string `json:"description,omitempty"` + DomainID *string `json:"domainID,omitempty"` + DefaultProjectID *string `json:"defaultProjectID,omitempty"` + Enabled *bool `json:"enabled,omitempty"` + PasswordExpiresAt *string `json:"passwordExpiresAt,omitempty"` + AppliedPasswordRef *string `json:"appliedPasswordRef,omitempty"` } // UserResourceStatusApplyConfiguration constructs a declarative configuration of the UserResourceStatus type for use with @@ -82,3 +83,11 @@ func (b *UserResourceStatusApplyConfiguration) WithPasswordExpiresAt(value strin b.PasswordExpiresAt = &value return b } + +// WithAppliedPasswordRef sets the AppliedPasswordRef field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the AppliedPasswordRef field is set to the value of the last call. +func (b *UserResourceStatusApplyConfiguration) WithAppliedPasswordRef(value string) *UserResourceStatusApplyConfiguration { + b.AppliedPasswordRef = &value + return b +} diff --git a/pkg/clients/applyconfiguration/internal/internal.go b/pkg/clients/applyconfiguration/internal/internal.go index c641e66f4..84688dc8a 100644 --- a/pkg/clients/applyconfiguration/internal/internal.go +++ b/pkg/clients/applyconfiguration/internal/internal.go @@ -3415,6 +3415,9 @@ var schemaYAML = typed.YAMLObject(`types: - name: com.github.k-orc.openstack-resource-controller.v2.api.v1alpha1.UserResourceStatus map: fields: + - name: appliedPasswordRef + type: + scalar: string - name: defaultProjectID type: scalar: string diff --git a/test/apivalidations/user_test.go b/test/apivalidations/user_test.go index 15057eb85..f38671d01 100644 --- a/test/apivalidations/user_test.go +++ b/test/apivalidations/user_test.go @@ -120,7 +120,7 @@ var _ = Describe("ORC User API validations", func() { Expect(applyObj(ctx, user, patch)).To(MatchError(ContainSubstring("defaultProjectRef is immutable"))) }) - It("should have immutable passwordRef", func(ctx context.Context) { + It("should have mutable passwordRef", func(ctx context.Context) { user := userStub(namespace) patch := baseUserPatch(user) patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). @@ -129,6 +129,6 @@ var _ = Describe("ORC User API validations", func() { patch.Spec.WithResource(applyconfigv1alpha1.UserResourceSpec(). WithPasswordRef("password-b")) - Expect(applyObj(ctx, user, patch)).To(MatchError(ContainSubstring("passwordRef is immutable"))) + Expect(applyObj(ctx, user, patch)).To(Succeed()) }) }) diff --git a/website/docs/crd-reference.md b/website/docs/crd-reference.md index 07dbfbe88..9d15a005a 100644 --- a/website/docs/crd-reference.md +++ b/website/docs/crd-reference.md @@ -4458,6 +4458,7 @@ _Appears in:_ | `defaultProjectID` _string_ | defaultProjectID is the ID of the Default Project to which the user is associated with. | | MaxLength: 1024
Optional: \{\}
| | `enabled` _boolean_ | enabled defines whether a user is enabled or disabled | | Optional: \{\}
| | `passwordExpiresAt` _string_ | passwordExpiresAt is the timestamp at which the user's password expires. | | MaxLength: 1024
Optional: \{\}
| +| `appliedPasswordRef` _string_ | appliedPasswordRef is the name of the Secret containing the
password that was last applied to the OpenStack resource. | | MaxLength: 1024
Optional: \{\}
| #### UserSpec