Skip to content

Commit 847eb1f

Browse files
authored
fix: return either non-nil result or non-nil err; move crds from template/ to crds/ (#55)
Signed-off-by: Georgy Khromov <gg.khrmv@gmail.com>
1 parent fcd4e56 commit 847eb1f

11 files changed

+26
-20
lines changed

Taskfile.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ tasks:
7070
desc: Generate code
7171
deps:
7272
- controller-gen
73-
cmd: '{{ .CONTROLLER_GEN }} object:headerFile="hack/boilerplate.go.txt" paths="./..."'
73+
cmds:
74+
- '{{ .CONTROLLER_GEN }} object:headerFile="hack/boilerplate.go.txt" paths="./..."'
75+
- cp -r config/crd/bases/ helm/argocd-rbac-operator/crds
7476

7577
fmt:
7678
desc: Format code

helm/argocd-rbac-operator/templates/rbac-operator.argoproj-labs.io_argocdprojectrolebindings.yaml renamed to helm/argocd-rbac-operator/crds/rbac-operator.argoproj-labs.io_argocdprojectrolebindings.yaml

File renamed without changes.

helm/argocd-rbac-operator/templates/rbac-operator.argoproj-labs.io_argocdprojectroles.yaml renamed to helm/argocd-rbac-operator/crds/rbac-operator.argoproj-labs.io_argocdprojectroles.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ spec:
5858
enum:
5959
- clusters
6060
- applications
61+
- applicationsets
6162
- repositories
6263
- logs
6364
- exec

helm/argocd-rbac-operator/templates/rbac-operator.argoproj-labs.io_argocdrolebindings.yaml renamed to helm/argocd-rbac-operator/crds/rbac-operator.argoproj-labs.io_argocdrolebindings.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ spec:
7272
type: array
7373
required:
7474
- argocdRoleRef
75+
- subjects
7576
type: object
7677
status:
7778
description: ArgoCDRoleBindingStatus defines the observed state of ArgoCDRoleBinding

helm/argocd-rbac-operator/templates/rbac-operator.argoproj-labs.io_argocdroles.yaml renamed to helm/argocd-rbac-operator/crds/rbac-operator.argoproj-labs.io_argocdroles.yaml

File renamed without changes.

internal/controller/argocdprojectrole_controller.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (r *ArgoCDProjectRoleReconciler) Reconcile(ctx context.Context, req ctrl.Re
6060
if err := r.Status().Update(ctx, &projectRole); err != nil {
6161
r.Log.Error(err, "Failed to update ArgoCDProjectRole status", "name", req.Name)
6262
}
63-
return ctrl.Result{RequeueAfter: time.Minute * 2}, err
63+
return ctrl.Result{}, err
6464
}
6565

6666
if projectRole.IsBeingDeleted() {
@@ -69,7 +69,7 @@ func (r *ArgoCDProjectRoleReconciler) Reconcile(ctx context.Context, req ctrl.Re
6969
if err := r.Status().Update(ctx, &projectRole); err != nil {
7070
r.Log.Error(err, "Failed to update ArgoCDProjectRole status during finalizer handling", "name", req.Name)
7171
}
72-
return ctrl.Result{RequeueAfter: time.Minute * 2}, fmt.Errorf("error when handling finalizer: %v", err)
72+
return ctrl.Result{}, fmt.Errorf("error when handling finalizer: %v", err)
7373
}
7474
return ctrl.Result{}, nil
7575
}
@@ -80,7 +80,7 @@ func (r *ArgoCDProjectRoleReconciler) Reconcile(ctx context.Context, req ctrl.Re
8080
if err := r.Status().Update(ctx, &projectRole); err != nil {
8181
r.Log.Error(err, "Failed to update ArgoCDProjectRole status after adding finalizer", "name", req.Name)
8282
}
83-
return ctrl.Result{RequeueAfter: time.Minute * 2}, fmt.Errorf("error when adding finalizer: %v", err)
83+
return ctrl.Result{}, fmt.Errorf("error when adding finalizer: %v", err)
8484
}
8585
return ctrl.Result{RequeueAfter: time.Second}, nil
8686
}
@@ -107,7 +107,7 @@ func (r *ArgoCDProjectRoleReconciler) Reconcile(ctx context.Context, req ctrl.Re
107107
if err := r.Status().Update(ctx, &projectRole); err != nil {
108108
r.Log.Error(err, "Failed to update ArgoCDProjectRole status", "name", req.Name)
109109
}
110-
return ctrl.Result{RequeueAfter: time.Minute * 2}, fmt.Errorf("error fetching ArgoCDProjectRoleBinding: %v", err)
110+
return ctrl.Result{}, fmt.Errorf("error fetching ArgoCDProjectRoleBinding: %v", err)
111111
}
112112
}
113113
return ctrl.Result{RequeueAfter: time.Minute * 5}, nil

internal/controller/argocdprojectrolebinding_controller.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (r *ArgoCDProjectRoleBindingReconciler) Reconcile(ctx context.Context, req
6161
projectRoleBinding.SetConditions(rbacoperatorv1alpha1.ReconcileError(err))
6262
if err := r.Status().Update(ctx, &projectRoleBinding); err != nil {
6363
r.Log.Error(err, "Failed to update ArgoCDProjectRoleBinding status", "name", req.Name)
64-
return ctrl.Result{RequeueAfter: time.Minute * 2}, err
64+
return ctrl.Result{}, err
6565
}
6666
}
6767

@@ -71,7 +71,7 @@ func (r *ArgoCDProjectRoleBindingReconciler) Reconcile(ctx context.Context, req
7171
if err := r.Status().Update(ctx, &projectRoleBinding); err != nil {
7272
r.Log.Error(err, "Failed to update ArgoCDProjectRoleBinding status during finalizer handling", "name", req.Name)
7373
}
74-
return ctrl.Result{RequeueAfter: time.Minute * 2}, fmt.Errorf("error when handling finalizer: %v", err)
74+
return ctrl.Result{}, fmt.Errorf("error when handling finalizer: %v", err)
7575
}
7676
return ctrl.Result{}, nil
7777
}
@@ -82,7 +82,7 @@ func (r *ArgoCDProjectRoleBindingReconciler) Reconcile(ctx context.Context, req
8282
if err := r.Status().Update(ctx, &projectRoleBinding); err != nil {
8383
r.Log.Error(err, "Failed to update ArgoCDProjectRoleBinding status after adding finalizer", "name", req.Name)
8484
}
85-
return ctrl.Result{RequeueAfter: time.Minute * 2}, fmt.Errorf("error when adding finalizer: %v", err)
85+
return ctrl.Result{}, fmt.Errorf("error when adding finalizer: %v", err)
8686
}
8787
return ctrl.Result{RequeueAfter: time.Second}, nil
8888
}
@@ -106,7 +106,7 @@ func (r *ArgoCDProjectRoleBindingReconciler) Reconcile(ctx context.Context, req
106106
if err := r.Status().Update(ctx, &projectRoleBinding); err != nil {
107107
r.Log.Error(err, "Failed to update ArgoCDProjectRoleBinding status after project role not found", "name", req.Name)
108108
}
109-
return ctrl.Result{RequeueAfter: time.Minute * 2}, fmt.Errorf("error when getting ArgoCDProjectRole: %v", err)
109+
return ctrl.Result{}, fmt.Errorf("error when getting ArgoCDProjectRole: %v", err)
110110
}
111111

112112
if !projectRole.HasArgoCDProjectRoleBindingRef() {
@@ -132,7 +132,7 @@ func (r *ArgoCDProjectRoleBindingReconciler) Reconcile(ctx context.Context, req
132132
}
133133
r.Log.Error(err, "Failed to remove role from AppProject", "appProject", boundAppProject, "role", projectRoleName)
134134
projectRoleBinding.SetConditions(rbacoperatorv1alpha1.ReconcileError(err))
135-
return ctrl.Result{RequeueAfter: time.Second}, fmt.Errorf("error when removing role from AppProject: %v", err)
135+
return ctrl.Result{}, fmt.Errorf("error when removing role from AppProject: %v", err)
136136
}
137137
r.Log.Info("Role removed from AppProject", "appProject", boundAppProject, "role", projectRoleName)
138138
projectRoleBinding.Status.AppProjectsBound = removeStringFromSlice(projectRoleBinding.Status.AppProjectsBound, boundAppProject)
@@ -163,15 +163,15 @@ func (r *ArgoCDProjectRoleBindingReconciler) Reconcile(ctx context.Context, req
163163
if err := r.Status().Update(ctx, &projectRoleBinding); err != nil {
164164
r.Log.Error(err, "Failed to update ArgoCDProjectRoleBinding status after patching AppProject", "name", req.Name)
165165
}
166-
return ctrl.Result{RequeueAfter: time.Second}, fmt.Errorf("error when patching AppProject: %v", err)
166+
return ctrl.Result{}, fmt.Errorf("error when patching AppProject: %v", err)
167167
}
168168
r.Log.Info("AppProject patched successfully", "appProject", appProjectRef)
169169
if !isAppProjectInStatus(projectRoleBinding.Status.AppProjectsBound, appProjectRef) {
170170
projectRoleBinding.Status.AppProjectsBound = append(projectRoleBinding.Status.AppProjectsBound, appProjectRef)
171171
r.Log.Info("AppProject added to status", "appProject", appProjectRef)
172172
if err := r.Status().Update(ctx, &projectRoleBinding); err != nil {
173173
r.Log.Error(err, "Failed to update ArgoCDProjectRoleBinding status after patching AppProject", "name", req.Name)
174-
return ctrl.Result{RequeueAfter: time.Second}, fmt.Errorf("error when updating status: %v", err)
174+
return ctrl.Result{}, fmt.Errorf("error when updating status: %v", err)
175175
}
176176
}
177177
}

internal/controller/argocdrole_controller.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (r *ArgoCDRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request)
104104
if err := r.Client.Status().Update(ctx, &role); err != nil {
105105
r.Log.Error(err, "Failed to update ArgoCDRole status", "name", req.Name)
106106
}
107-
return ctrl.Result{Requeue: true, RequeueAfter: time.Second}, fmt.Errorf("ConfigMap not found")
107+
return ctrl.Result{}, fmt.Errorf("ConfigMap not found")
108108
}
109109

110110
if role.HasArgoCDRoleBindingRef() {
@@ -131,7 +131,7 @@ func (r *ArgoCDRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request)
131131
if err := r.Client.Status().Update(ctx, &role); err != nil {
132132
r.Log.Error(err, "Failed to update ArgoCDRole status", "name", req.Name)
133133
}
134-
return ctrl.Result{Requeue: true, RequeueAfter: time.Second}, err
134+
return ctrl.Result{}, err
135135
}
136136

137137
role.SetConditions(rbacoperatorv1alpha1.ReconcileSuccess().WithObservedGeneration(role.GetGeneration()))
@@ -151,7 +151,7 @@ func (r *ArgoCDRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request)
151151
if err := r.Client.Status().Update(ctx, &role); err != nil {
152152
r.Log.Error(err, "Failed to update ArgoCDRole status", "name", req.Name)
153153
}
154-
return ctrl.Result{Requeue: true, RequeueAfter: time.Second}, err
154+
return ctrl.Result{}, err
155155
}
156156

157157
role.SetConditions(rbacoperatorv1alpha1.ReconcileSuccess().WithObservedGeneration(role.GetGeneration()))

internal/controller/argocdrole_controller_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ func TestArgoCDRoleReconciler_CMNotFound(t *testing.T) {
150150

151151
res, err := reconciler.Reconcile(context.TODO(), req)
152152
assert.Error(t, err)
153-
assert.True(t, res.RequeueAfter > 0, "expected requeue after to be greater than 0")
153+
assert.False(t, res.Requeue, "requeue should be false when error is returned")
154+
assert.Equal(t, time.Duration(0), res.RequeueAfter, "requeue after should be 0 when error is returned")
154155
}
155156

156157
func TestArgoCDRoleReconciler_HandleFinalizer(t *testing.T) {

internal/controller/argocdrolebinding_controller.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (r *ArgoCDRoleBindingReconciler) Reconcile(ctx context.Context, req ctrl.Re
102102
if err := r.Client.Status().Update(ctx, &rb); err != nil {
103103
r.Log.Error(err, "Failed to update ArgoCDRoleBinding status", "name", req.Name)
104104
}
105-
return ctrl.Result{Requeue: true, RequeueAfter: time.Second}, fmt.Errorf("ConfigMap not found")
105+
return ctrl.Result{}, fmt.Errorf("ConfigMap not found")
106106
}
107107

108108
roleName := rb.Spec.ArgoCDRoleRef.Name
@@ -132,7 +132,7 @@ func (r *ArgoCDRoleBindingReconciler) Reconcile(ctx context.Context, req ctrl.Re
132132
if err := r.Client.Status().Update(ctx, &rb); err != nil {
133133
r.Log.Error(err, "Failed to update ArgoCDRoleBinding status", "name", req.Name)
134134
}
135-
return ctrl.Result{Requeue: true, RequeueAfter: time.Second}, err
135+
return ctrl.Result{}, err
136136
}
137137

138138
if !role.HasArgoCDRoleBindingRef() {
@@ -169,7 +169,7 @@ func (r *ArgoCDRoleBindingReconciler) Reconcile(ctx context.Context, req ctrl.Re
169169
if err := r.Client.Status().Update(ctx, &rb); err != nil {
170170
r.Log.Error(err, "Failed to update ArgoCDRoleBinding status", "name", req.Name)
171171
}
172-
return ctrl.Result{Requeue: true, RequeueAfter: time.Second}, err
172+
return ctrl.Result{}, err
173173
}
174174

175175
rb.SetConditions(rbacoperatorv1alpha1.ReconcileSuccess().WithObservedGeneration(rb.GetGeneration()))

0 commit comments

Comments
 (0)