Skip to content

Commit c1be85d

Browse files
afritzlerafritzler
committed
Harmonize BMCUser reconciler code
1 parent f07a5ff commit c1be85d

File tree

1 file changed

+41
-53
lines changed

1 file changed

+41
-53
lines changed

‎internal/controller/bmcuser_controller.go‎

Lines changed: 41 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package controller
55

66
import (
77
"context"
8+
"errors"
89
"fmt"
910
"time"
1011

@@ -25,7 +26,7 @@ import (
2526
)
2627

2728
const (
28-
BMCUserFinalizer = "metal.ironcore.dev/bmcuser-finalizer"
29+
BMCUserFinalizer = "metal.ironcore.dev/bmcuser"
2930
)
3031

3132
// BMCUserReconciler reconciles a BMCUser object
@@ -53,24 +54,18 @@ func (r *BMCUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
5354

5455
func (r *BMCUserReconciler) reconcileExists(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) (ctrl.Result, error) {
5556
if !user.DeletionTimestamp.IsZero() {
56-
err := r.delete(ctx, log, user)
57-
if err != nil {
58-
return ctrl.Result{}, fmt.Errorf("failed to delete User: %w", err)
59-
}
60-
return ctrl.Result{}, nil
57+
return r.delete(ctx, log, user)
6158
}
6259
return r.reconcile(ctx, log, user)
6360
}
6461

6562
func (r *BMCUserReconciler) reconcile(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) (ctrl.Result, error) {
6663
if user.Spec.BMCRef == nil {
67-
log.Info("No BMC reference set for User, skipping reconciliation", "User", user.Name)
64+
log.Info("No BMC reference set for User, skipping reconciliation")
6865
return ctrl.Result{}, nil
6966
}
7067
bmcObj := &metalv1alpha1.BMC{}
71-
if err := r.Get(ctx, client.ObjectKey{
72-
Name: user.Spec.BMCRef.Name,
73-
}, bmcObj); err != nil {
68+
if err := r.Get(ctx, client.ObjectKey{Name: user.Spec.BMCRef.Name}, bmcObj); err != nil {
7469
return ctrl.Result{}, err
7570
}
7671
if err := r.updateEffectiveSecret(ctx, log, user, bmcObj); err != nil {
@@ -84,26 +79,23 @@ func (r *BMCUserReconciler) reconcile(ctx context.Context, log logr.Logger, user
8479
return ctrl.Result{}, fmt.Errorf("failed to get BMC client: %w", err)
8580
}
8681
defer bmcClient.Logout()
87-
err = r.patchUserStatus(ctx, log, user, bmcClient)
88-
if err != nil {
82+
if err = r.patchUserStatus(ctx, log, user, bmcClient); err != nil {
8983
return ctrl.Result{}, fmt.Errorf("failed to update User status: %w", err)
9084
}
9185

9286
if user.Spec.BMCSecretRef == nil {
93-
log.Info("No BMCSecret reference set for User, creating a new one", "User", user.Name)
87+
log.Info("No BMCSecret reference set for User, creating a new one")
9488
if err := r.ensureBMCSecretForUser(ctx, log, bmcClient, user, bmcObj); err != nil {
9589
return ctrl.Result{}, fmt.Errorf("failed to handle missing BMCSecret reference: %w", err)
9690
}
9791
}
9892
bmcSecret := &metalv1alpha1.BMCSecret{}
99-
if err := r.Get(ctx, client.ObjectKey{
100-
Name: user.Spec.BMCSecretRef.Name,
101-
}, bmcSecret); err != nil {
93+
if err := r.Get(ctx, client.ObjectKey{Name: user.Spec.BMCSecretRef.Name}, bmcSecret); err != nil {
10294
return ctrl.Result{}, err
10395
}
10496

10597
if user.Status.ID == "" {
106-
log.Info("No BMC account ID set in User status, creating or updating BMC account", "User", user.Name)
98+
log.Info("No BMC account ID set in User status, creating or updating BMC account")
10799
_, password, err := bmcutils.GetBMCCredentialsFromSecret(bmcSecret)
108100
if err != nil {
109101
return ctrl.Result{}, fmt.Errorf("failed to get credentials from BMCSecret: %w", err)
@@ -116,7 +108,7 @@ func (r *BMCUserReconciler) reconcile(ctx context.Context, log logr.Logger, user
116108
}
117109
}
118110
if user.Status.EffectiveBMCSecretRef != nil && user.Spec.BMCSecretRef.Name != user.Status.EffectiveBMCSecretRef.Name {
119-
log.Info("BMCSecret reference has changed, updating BMC account", "User", user.Name)
111+
log.Info("BMCSecret reference has changed, updating BMC account")
120112
if err := r.handleUpdatedSecretRef(ctx, log, user, bmcSecret, bmcClient); err != nil {
121113
return ctrl.Result{}, fmt.Errorf("failed to handle updated BMCSecret reference: %w", err)
122114
}
@@ -131,19 +123,19 @@ func (r *BMCUserReconciler) patchUserStatus(ctx context.Context, log logr.Logger
131123
}
132124
for _, account := range accounts {
133125
if account.UserName == user.Spec.UserName {
134-
log.V(1).Info("BMC account already exists", "User", user.Name, "ID", account.ID)
126+
log.V(1).Info("BMC account already exists", "ID", account.ID)
135127
userBase := user.DeepCopy()
136128
user.Status.ID = account.ID
137129
exp, err := time.Parse(time.RFC3339, account.PasswordExpiration)
138130
if err == nil {
139131
user.Status.PasswordExpiration = &metav1.Time{Time: exp}
140132
} else {
141-
log.Error(err, "Failed to parse password expiration time from BMC account", "User", user.Name, "Expiration", account.PasswordExpiration)
133+
log.Error(err, "Failed to parse password expiration time from BMC account", "Expiration", account.PasswordExpiration)
142134
}
143135
if err := r.Status().Patch(ctx, user, client.MergeFrom(userBase)); err != nil {
144136
return fmt.Errorf("failed to patch User status with BMC account ID: %w", err)
145137
}
146-
log.Info("Updated User status with BMC account ID", "User", user.Name, "AccountID", account.ID)
138+
log.Info("Updated User status with BMC account ID", "AccountID", account.ID)
147139
return nil
148140
}
149141
}
@@ -153,7 +145,7 @@ func (r *BMCUserReconciler) patchUserStatus(ctx context.Context, log logr.Logger
153145
func (r *BMCUserReconciler) handleRotatingPassword(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser, bmcObj *metalv1alpha1.BMC, bmcClient bmc.BMC) (ctrl.Result, error) {
154146
forceRotation := false
155147
if user.GetAnnotations() != nil && user.GetAnnotations()[metalv1alpha1.OperationAnnotation] == metalv1alpha1.OperationAnnotationRotateCredentials {
156-
log.Info("User has rotation annotation set, triggering password rotation", "User", user.Name)
148+
log.Info("User has rotation annotation set, triggering password rotation")
157149
forceRotation = true
158150
userBase := user.DeepCopy()
159151
delete(user.Annotations, metalv1alpha1.OperationAnnotation)
@@ -163,26 +155,23 @@ func (r *BMCUserReconciler) handleRotatingPassword(ctx context.Context, log logr
163155
}
164156
if user.Status.PasswordExpiration != nil {
165157
if user.Status.PasswordExpiration.Time.Before(metav1.Now().Time) {
166-
log.Info("BMC user password has expired, rotating password", "User", user.Name)
158+
log.Info("BMC user password has expired, rotating password")
167159
// If the password has expired, we need to rotate it
168160
forceRotation = true
169161
}
170162
}
171163
if user.Spec.RotationPeriod == nil && !forceRotation {
172-
log.V(1).Info("No rotation period set for BMC user, skipping password rotation", "User", user.Name)
164+
log.V(1).Info("No rotation period set for BMC user, skipping password rotation")
173165
return ctrl.Result{}, nil
174166
}
175167
if user.Spec.RotationPeriod != nil &&
176168
user.Status.LastRotation != nil &&
177169
user.Status.LastRotation.Time.Add(user.Spec.RotationPeriod.Duration).After(time.Now()) &&
178170
!forceRotation {
179-
log.V(1).Info("BMC user password rotation is not needed yet", "User", user.Name)
180-
return ctrl.Result{
181-
Requeue: true,
182-
RequeueAfter: user.Spec.RotationPeriod.Duration,
183-
}, nil
171+
log.V(1).Info("BMC user password rotation is not needed yet")
172+
return ctrl.Result{RequeueAfter: user.Spec.RotationPeriod.Duration}, nil
184173
}
185-
log.Info("Rotating BMC user password", "User", user.Name)
174+
log.Info("Rotating BMC user password")
186175
accountService, err := bmcClient.GetAccountService()
187176
if err != nil {
188177
return ctrl.Result{}, fmt.Errorf("failed to get account service: %w", err)
@@ -207,12 +196,12 @@ func (r *BMCUserReconciler) handleRotatingPassword(ctx context.Context, log logr
207196
if err := r.Status().Patch(ctx, user, client.MergeFrom(userBase)); err != nil {
208197
return ctrl.Result{}, fmt.Errorf("failed to patch User status with last rotation time: %w", err)
209198
}
210-
log.Info("Updated last rotation time for BMC user", "User", user.Name)
199+
log.Info("Updated last rotation time for BMC user")
211200
return ctrl.Result{}, nil
212201
}
213202

214203
func (r *BMCUserReconciler) ensureBMCSecretForUser(ctx context.Context, log logr.Logger, bmcClient bmc.BMC, user *metalv1alpha1.BMCUser, bmcObj *metalv1alpha1.BMC) error {
215-
log.Info("No BMCSecret reference set for User, creating a new one", "User", user.Name)
204+
log.Info("No BMCSecret reference set for User, creating a new one")
216205
accountService, err := bmcClient.GetAccountService()
217206
if err != nil {
218207
return fmt.Errorf("failed to get account service: %w", err)
@@ -237,7 +226,7 @@ func (r *BMCUserReconciler) ensureBMCSecretForUser(ctx context.Context, log logr
237226
}
238227

239228
func (r *BMCUserReconciler) handleUpdatedSecretRef(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser, bmcSecret *metalv1alpha1.BMCSecret, bmcClient bmc.BMC) error {
240-
log.Info("BMCSecret credentials have changed, updating BMC user", "User", user.Name)
229+
log.Info("BMCSecret credentials have changed, updating BMC user")
241230
_, password, err := bmcutils.GetBMCCredentialsFromSecret(bmcSecret)
242231
if err != nil {
243232
return fmt.Errorf("failed to get credentials from BMCSecret: %w", err)
@@ -250,7 +239,7 @@ func (r *BMCUserReconciler) handleUpdatedSecretRef(ctx context.Context, log logr
250239
}
251240

252241
func (r *BMCUserReconciler) createBMCSecretForUser(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser, password string) (*metalv1alpha1.BMCSecret, error) {
253-
log.Info("Creating BMCSecret for User", "User", user.Name)
242+
log.Info("Creating BMCSecret for User")
254243
secret := &metalv1alpha1.BMCSecret{
255244
ObjectMeta: metav1.ObjectMeta{
256245
GenerateName: user.Name,
@@ -285,7 +274,7 @@ func (r *BMCUserReconciler) setBMCUserSecretRef(ctx context.Context, log logr.Lo
285274
}
286275

287276
func (r *BMCUserReconciler) setEffectiveSecretRef(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser, secret *metalv1alpha1.BMCSecret) error {
288-
log.Info("Setting effective BMCSecret", "User", user.Name)
277+
log.Info("Setting effective BMCSecret")
289278
userBase := user.DeepCopy()
290279
if user.Status.EffectiveBMCSecretRef == nil {
291280
user.Status.EffectiveBMCSecretRef = &v1.LocalObjectReference{}
@@ -322,14 +311,14 @@ func (r *BMCUserReconciler) updateEffectiveSecret(ctx context.Context, log logr.
322311
return fmt.Errorf("failed to test BMC connection with BMCSecret %s: %w", secret.Name, err)
323312
}
324313
if invalidCredentials {
325-
log.Info("New BMCSecret is invalid, will not update effective BMCSecret", "User", user.Name, "NewBMCSecret", secret.Name)
314+
log.Info("New BMCSecret is invalid, will not update effective BMCSecret", "NewBMCSecret", secret.Name)
326315
return nil
327316
}
328317
if user.Status.EffectiveBMCSecretRef == nil {
329318
if err := r.setEffectiveSecretRef(ctx, log, user, secret); err != nil {
330319
return fmt.Errorf("failed to update effective BMCSecret: %w", err)
331320
}
332-
log.Info("Set effective BMCSecret for User", "User", user.Name)
321+
log.Info("Set effective BMCSecret for User")
333322
return nil
334323
}
335324

@@ -345,11 +334,11 @@ func (r *BMCUserReconciler) updateEffectiveSecret(ctx context.Context, log logr.
345334
return fmt.Errorf("failed to test BMC connection with effectiveSecret %s: %w", effSecret.Name, err)
346335
}
347336
if invalidCredentials {
348-
log.Info("Effective BMCSecret is invalid", "User", user.Name, "EffectiveBMCSecret", effSecret.Name, "NewBMCSecret", secret.Name)
337+
log.Info("Effective BMCSecret is invalid", "EffectiveBMCSecret", effSecret.Name, "NewBMCSecret", secret.Name)
349338
if err := r.setEffectiveSecretRef(ctx, log, user, secret); err != nil {
350339
return fmt.Errorf("failed to update effective BMCSecret: %w", err)
351340
}
352-
log.Info("Updated effective BMCSecret for User", "User", user.Name)
341+
log.Info("Updated effective BMCSecret for User")
353342
}
354343
return nil
355344
}
@@ -362,7 +351,8 @@ func (r *BMCUserReconciler) bmcConnectionTest(ctx context.Context, secret *metal
362351
}
363352
_, err = bmcutils.CreateBMCClient(ctx, r.Client, protocolScheme, bmcObj.Spec.Protocol.Name, address, bmcObj.Spec.Protocol.Port, secret, r.BMCOptions)
364353
if err != nil {
365-
if httpErr, ok := err.(*common.Error); ok {
354+
var httpErr *common.Error
355+
if errors.As(err, &httpErr) {
366356
if httpErr.HTTPReturnedStatusCode == 401 || httpErr.HTTPReturnedStatusCode == 403 {
367357
return true, nil
368358
}
@@ -372,31 +362,29 @@ func (r *BMCUserReconciler) bmcConnectionTest(ctx context.Context, secret *metal
372362
return false, nil
373363
}
374364

375-
func (r *BMCUserReconciler) delete(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) error {
365+
func (r *BMCUserReconciler) delete(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) (ctrl.Result, error) {
376366
if user.Spec.BMCRef == nil {
377-
log.Info("No BMC reference set for User, skipping deletion", "User", user.Name)
378-
return nil
367+
log.Info("No BMC reference set for User, skipping deletion")
368+
return ctrl.Result{}, nil
379369
}
380370
bmcObj := &metalv1alpha1.BMC{}
381-
if err := r.Get(ctx, client.ObjectKey{
382-
Name: user.Spec.BMCRef.Name,
383-
}, bmcObj); err != nil {
384-
return client.IgnoreNotFound(err)
371+
if err := r.Get(ctx, client.ObjectKey{Name: user.Spec.BMCRef.Name}, bmcObj); err != nil {
372+
return ctrl.Result{}, client.IgnoreNotFound(err)
385373
}
386374
bmcClient, err := r.getBMCClient(ctx, bmcObj)
387375
if err != nil {
388-
return fmt.Errorf("failed to get BMC client: %w", err)
376+
return ctrl.Result{}, fmt.Errorf("failed to get BMC client: %w", err)
389377
}
390378
defer bmcClient.Logout()
391-
log.Info("Deleting BMC account for User", "User", user.Name)
379+
log.Info("Deleting BMC account for User")
392380
if err := bmcClient.DeleteAccount(ctx, user.Spec.UserName, user.Status.ID); err != nil {
393-
return fmt.Errorf("failed to delete BMC account: %w", err)
381+
return ctrl.Result{}, fmt.Errorf("failed to delete BMC account: %w", err)
394382
}
395-
log.Info("Successfully deleted BMC account and removed finalizer for User", "User", user.Name)
383+
log.Info("Successfully deleted BMC account and removed finalizer for User")
396384
if modified, err := clientutils.PatchEnsureNoFinalizer(ctx, r.Client, user, BMCUserFinalizer); err != nil || modified {
397-
return err
385+
return ctrl.Result{}, err
398386
}
399-
return nil
387+
return ctrl.Result{}, nil
400388
}
401389

402390
// SetupWithManager sets up the controller with the Manager.

0 commit comments

Comments
 (0)