Skip to content

Commit 3117b81

Browse files
tkan145tkan145
committed
ApplicationAuth: simplify status reconcilation logic
1 parent 973325a commit 3117b81

2 files changed

Lines changed: 46 additions & 94 deletions

File tree

‎controllers/capabilities/applicationauth_controller.go‎

Lines changed: 45 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ const (
6060
// +kubebuilder:rbac:groups=capabilities.3scale.net,resources=applicationauths/status,verbs=get;update;patch
6161

6262
func (r *ApplicationAuthReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
63-
_ = context.Background()
64-
// _ = r.Log.WithValues("applicationauth", req.NamespacedName)
6563
reqLogger := r.Logger().WithValues("applicationauth", req.NamespacedName)
6664
reqLogger.Info("Reconcile Application Authentication", "Operator version", version.Version)
6765

@@ -84,74 +82,43 @@ func (r *ApplicationAuthReconciler) Reconcile(ctx context.Context, req ctrl.Requ
8482
}
8583
reqLogger.V(1).Info(string(jsonData))
8684
}
87-
// get the application
88-
application := &capabilitiesv1beta1.Application{}
8985

9086
// Retrieve application CR, on failed retrieval update status and requeue
87+
application := &capabilitiesv1beta1.Application{}
9188
err = r.Client().Get(r.Context(), types.NamespacedName{Name: applicationAuth.Spec.ApplicationCRName, Namespace: applicationAuth.Namespace}, application)
9289
if err != nil {
9390
// If the product CR is not found, update status and requeue
9491
if errors.IsNotFound(err) {
95-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
9692
reqLogger.Info("Application CR not found. Ignoring since object must have been deleted")
97-
statusResult, statusErr := statusReconciler.Reconcile()
98-
// Reconcile status first as the reconcilerError might need to be updated to the status section of the CR before requeueing
99-
if statusErr != nil {
100-
return ctrl.Result{}, statusErr
101-
}
102-
if statusResult.Requeue {
103-
reqLogger.Info("Reconciling status not finished. Requeueing.")
104-
return statusResult, nil
105-
}
93+
return r.reconcileStatus(applicationAuth, err, reqLogger)
10694
}
10795

10896
// If API call error, return err
10997
return ctrl.Result{}, err
11098
}
11199

112-
// get the product
100+
// Retrieve DeveloperAccount CR, on failed retrieval update status and requeue
113101
developerAccount := &capabilitiesv1beta1.DeveloperAccount{}
114-
115-
// Retrieve product CR, on failed retrieval update status and requeue
116102
err = r.Client().Get(r.Context(), types.NamespacedName{Name: application.Spec.AccountCR.Name, Namespace: applicationAuth.Namespace}, developerAccount)
117103
if err != nil {
118104
// If the product CR is not found, update status and requeue
119105
if errors.IsNotFound(err) {
120-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
121106
reqLogger.Info("DeveloperAccount CR not found. Ignoring since object must have been deleted")
122-
statusResult, statusErr := statusReconciler.Reconcile()
123-
// Reconcile status first as the reconcilerError might need to be updated to the status section of the CR before requeueing
124-
if statusErr != nil {
125-
return ctrl.Result{}, statusErr
126-
}
127-
if statusResult.Requeue {
128-
reqLogger.Info("Reconciling status not finished. Requeueing.")
129-
return statusResult, nil
130-
}
107+
return r.reconcileStatus(applicationAuth, err, reqLogger)
131108
}
132109

133110
// If API call error, return err
134111
return ctrl.Result{}, err
135112
}
136-
// get the application
137-
product := &capabilitiesv1beta1.Product{}
138113

139-
// Retrieve application CR, on failed retrieval update status and requeue
114+
// Retrieve Product CR, on failed retrieval update status and requeue
115+
product := &capabilitiesv1beta1.Product{}
140116
err = r.Client().Get(r.Context(), types.NamespacedName{Name: application.Spec.ProductCR.Name, Namespace: applicationAuth.Namespace}, product)
141117
if err != nil {
142118
// If the product CR is not found, update status and requeue
143119
if errors.IsNotFound(err) {
144-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
145-
reqLogger.Info("Application CR not found. Ignoring since object must have been deleted")
146-
statusResult, statusErr := statusReconciler.Reconcile()
147-
// Reconcile status first as the reconcilerError might need to be updated to the status section of the CR before requeueing
148-
if statusErr != nil {
149-
return ctrl.Result{}, statusErr
150-
}
151-
if statusResult.Requeue {
152-
reqLogger.Info("Reconciling status not finished. Requeueing.")
153-
return statusResult, nil
154-
}
120+
reqLogger.Info("Product CR not found. Ignoring since object must have been deleted")
121+
return r.reconcileStatus(applicationAuth, err, reqLogger)
155122
}
156123

157124
// If API call error, return err
@@ -171,52 +138,29 @@ func (r *ApplicationAuthReconciler) Reconcile(ctx context.Context, req ctrl.Requ
171138
return ctrl.Result{}, err
172139
}
173140

174-
// get the authSecret
175-
authSecretObj := &corev1.Secret{}
176-
177141
// Retrieve auth secret, on failed retrieval update status and requeue
142+
authSecretObj := &corev1.Secret{}
178143
err = r.Client().Get(r.Context(), types.NamespacedName{Name: applicationAuth.Spec.AuthSecretRef.Name, Namespace: applicationAuth.Namespace}, authSecretObj)
179144
if err != nil {
180145
// If the product CR is not found, update status and requeue
181146
if errors.IsNotFound(err) {
182-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
183147
reqLogger.Info("ApplicationAuth secret not found. Ignoring since object must have been deleted")
184-
statusResult, statusErr := statusReconciler.Reconcile()
185-
// Reconcile status first as the reconcilerError might need to be updated to the status section of the CR before requeueing
186-
if statusErr != nil {
187-
return ctrl.Result{}, statusErr
188-
}
189-
if statusResult.Requeue {
190-
reqLogger.Info("Reconciling status not finished. Requeueing.")
191-
return statusResult, nil
192-
}
148+
return r.reconcileStatus(applicationAuth, err, reqLogger)
193149
}
194150
return ctrl.Result{}, err
195151
}
152+
196153
// populate authSecret struct
197154
authSecret := authSecretReferenceSource(r.Client(), applicationAuth.Namespace, applicationAuth.Spec.AuthSecretRef, reqLogger)
198-
199155
if !applicationAuth.Status.Conditions.IsTrueFor(capabilitiesv1beta1.ApplicationAuthReadyConditionType) {
200-
statusReconciler, reconcileErr := r.applicationAuthReconciler(applicationAuth, developerAccount, application, product, *authSecret, threescaleAPIClient)
201-
if statusReconciler != nil {
202-
statusResult, statusErr := statusReconciler.Reconcile()
203-
204-
if statusErr != nil {
205-
return ctrl.Result{}, statusErr
206-
}
207-
if statusResult.Requeue {
208-
reqLogger.Info("Reconciling status not finished. Requeueing.")
209-
return statusResult, nil
210-
}
211-
// If reconcile error but no status update required, requeue.
212-
if reconcileErr != nil {
213-
return helper.ReconcileErrorHandler(reconcileErr, reqLogger), nil
214-
}
156+
err := r.applicationAuthReconciler(applicationAuth, developerAccount, application, product, *authSecret, threescaleAPIClient)
157+
if err != nil {
158+
return r.reconcileStatus(applicationAuth, err, reqLogger)
215159
}
216160
}
217161
// final return
218162
reqLogger.Info("Successfully reconciled")
219-
return ctrl.Result{}, nil
163+
return r.reconcileStatus(applicationAuth, nil, reqLogger)
220164
}
221165

222166
func (r *ApplicationAuthReconciler) SetupWithManager(mgr ctrl.Manager) error {
@@ -232,7 +176,7 @@ func (r *ApplicationAuthReconciler) applicationAuthReconciler(
232176
product *capabilitiesv1beta1.Product,
233177
authSecret AuthSecret,
234178
threescaleClient *threescaleapi.ThreeScaleClient,
235-
) (*ApplicationAuthStatusReconciler, error) {
179+
) error {
236180
// generate sha base of timestamp
237181
timestamp := time.Now().Unix()
238182
// Write the timestamp string and encode to hash
@@ -253,8 +197,7 @@ func (r *ApplicationAuthReconciler) applicationAuthReconciler(
253197
if application.Status.ID != nil {
254198
_, err := threescaleClient.UpdateApplication(*developerAccount.Status.ID, *application.Status.ID, params)
255199
if err != nil {
256-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
257-
return statusReconciler, err
200+
return err
258201
}
259202
}
260203
}
@@ -264,8 +207,7 @@ func (r *ApplicationAuthReconciler) applicationAuthReconciler(
264207
if application.Status.ID != nil {
265208
foundApplication, err := threescaleClient.CreateApplicationKey(*developerAccount.Status.ID, *application.Status.ID, authSecret.ApplicationKey)
266209
if err != nil {
267-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
268-
return statusReconciler, err
210+
return err
269211
}
270212

271213
authSecret.ApplicationID = foundApplication.ApplicationId
@@ -276,15 +218,13 @@ func (r *ApplicationAuthReconciler) applicationAuthReconciler(
276218
if application.Status.ID != nil {
277219
foundApplication, err := threescaleClient.CreateApplicationRandomKey(*developerAccount.Status.ID, *application.Status.ID)
278220
if err != nil {
279-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
280-
return statusReconciler, err
221+
return err
281222
}
282223
authSecret.ApplicationID = foundApplication.ApplicationId
283224
var foundApplicationKeys []threescaleapi.ApplicationKey
284225
foundApplicationKeys, err = threescaleClient.ApplicationKeys(*developerAccount.Status.ID, *application.Status.ID)
285226
if err != nil {
286-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
287-
return statusReconciler, err
227+
return err
288228
}
289229
lastKey := len(foundApplicationKeys) - 1
290230
authSecret.ApplicationKey = fmt.Sprint(foundApplicationKeys[lastKey].Value)
@@ -300,8 +240,7 @@ func (r *ApplicationAuthReconciler) applicationAuthReconciler(
300240
if err != nil {
301241
// Handle errors gracefully, e.g., log and return or retry
302242
r.Logger().Error(err, "Failed to get existing ApplicationAuthSecret")
303-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
304-
return statusReconciler, err
243+
return err
305244
}
306245
newData := ApplicationAuthSecret.Data
307246
newValues := map[string][]byte{
@@ -317,11 +256,10 @@ func (r *ApplicationAuthReconciler) applicationAuthReconciler(
317256
err = r.Client().Update(r.Context(), ApplicationAuthSecret)
318257
if err != nil {
319258
r.Logger().Error(err, "Failed to update ApplicationAuthSecret")
320-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, err)
321-
return statusReconciler, err
259+
return err
322260
}
323-
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, applicationAuth, nil)
324-
return statusReconciler, nil
261+
262+
return nil
325263
}
326264

327265
func authSecretReferenceSource(cl client.Client, ns string, authSectretRef *corev1.LocalObjectReference, logger logr.Logger) *AuthSecret {
@@ -342,3 +280,24 @@ func authSecretReferenceSource(cl client.Client, ns string, authSectretRef *core
342280

343281
return nil
344282
}
283+
284+
func (r *ApplicationAuthReconciler) reconcileStatus(resource *capabilitiesv1beta1.ApplicationAuth, err error, logger logr.Logger) (ctrl.Result, error) {
285+
statusReconciler := NewApplicationAuthStatusReconciler(r.BaseReconciler, resource, err)
286+
statusResult, statusErr := statusReconciler.Reconcile()
287+
288+
if err != nil {
289+
return ctrl.Result{}, err
290+
}
291+
292+
// Reconcile status first as the reconcilerError might need to be updated to the status section of the CR before requeueing
293+
if statusErr != nil {
294+
return ctrl.Result{}, statusErr
295+
}
296+
297+
if statusResult.Requeue {
298+
logger.Info("Reconciling status not finished. Requeueing.")
299+
return statusResult, nil
300+
}
301+
302+
return ctrl.Result{}, nil
303+
}

‎controllers/capabilities/applicationauth_controller_test.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"bytes"
55
"io"
66
"net/http"
7-
"reflect"
87
"strconv"
98
"testing"
109

@@ -100,17 +99,11 @@ func TestApplicationAuthReconciler_applicationAuthReconciler(t *testing.T) {
10099
r := &ApplicationAuthReconciler{
101100
BaseReconciler: tt.fields.BaseReconciler,
102101
}
103-
got, err := r.applicationAuthReconciler(tt.args.applicationAuth, tt.args.developerAccount, tt.args.application, tt.args.product, tt.args.authSecret, tt.args.threescaleClient)
102+
err := r.applicationAuthReconciler(tt.args.applicationAuth, tt.args.developerAccount, tt.args.application, tt.args.product, tt.args.authSecret, tt.args.threescaleClient)
104103
if (err != nil) != tt.wantErr {
105104
t.Errorf("applicationAuthReconciler() error = %v, wantErr %v", err, tt.wantErr)
106105
return
107106
}
108-
if !reflect.DeepEqual(got.reconcileError, tt.want.reconcileError) {
109-
t.Errorf("applicationAuthReconciler() got = %v, want %v", got.reconcileError, tt.want.reconcileError)
110-
}
111-
if !reflect.DeepEqual(got.resource, tt.want.resource) {
112-
t.Errorf("applicationAuthReconciler() got = %v, want %v", got.resource, tt.want.resource)
113-
}
114107
})
115108
}
116109
}

0 commit comments

Comments
 (0)