Skip to content

Commit 48d3f9c

Browse files
nmiraschnmirasch
committed
Removed cluster roles and added self-service-notification-enabled to command
Signed-off-by: nmirasch <[email protected]>
1 parent 7cd29b7 commit 48d3f9c

File tree

5 files changed

+9
-183
lines changed

5 files changed

+9
-183
lines changed

‎controllers/argocd/notifications.go‎

Lines changed: 3 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"os"
87
"reflect"
98
"strings"
109

@@ -70,18 +69,6 @@ func (r *ReconcileArgoCD) reconcileNotificationsController(cr *argoproj.ArgoCD)
7069
return err
7170
}
7271

73-
// create clusterrole & clusterrolebinding if cluster-scoped ArgoCD
74-
log.Info("reconciling notifications clusterroles")
75-
clusterrole, err := r.reconcileNotificationsClusterRole(cr)
76-
if err != nil {
77-
return err
78-
}
79-
80-
log.Info("reconciling notifications clusterrolebindings")
81-
if err := r.reconcileNotificationsClusterRoleBinding(cr, clusterrole, sa); err != nil {
82-
return err
83-
}
84-
8572
// reconcile source namespace roles & rolebindings
8673
log.Info("reconciling notifications roles & rolebindings in source namespaces")
8774
if err := r.reconcileNotificationsSourceNamespacesResources(cr); err != nil {
@@ -562,129 +549,6 @@ func (r *ReconcileArgoCD) reconcileNotificationsSecret(cr *argoproj.ArgoCD) erro
562549
return nil
563550
}
564551

565-
// reconcileNotificationsClusterRoleBinding reconciles required clusterrole for notification controller when ArgoCD is cluster-scoped
566-
func (r *ReconcileArgoCD) reconcileNotificationsClusterRole(cr *argoproj.ArgoCD) (*rbacv1.ClusterRole, error) {
567-
568-
allowed := allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES"))
569-
570-
// controller disabled, don't create resources
571-
if !isNotificationsEnabled(cr) {
572-
allowed = false
573-
}
574-
575-
policyRules := policyRuleForNotificationsController()
576-
clusterRole := newClusterRole(common.ArgoCDNotificationsControllerComponent, policyRules, cr)
577-
if err := applyReconcilerHook(cr, clusterRole, ""); err != nil {
578-
return nil, err
579-
}
580-
581-
existingClusterRole := &rbacv1.ClusterRole{}
582-
err := r.Get(context.TODO(), types.NamespacedName{Name: clusterRole.Name}, existingClusterRole)
583-
if err != nil {
584-
if !apierrors.IsNotFound(err) {
585-
return nil, fmt.Errorf("failed to reconcile the cluster role for the service account associated with %s : %s", clusterRole.Name, err)
586-
}
587-
if !allowed {
588-
// Do Nothing
589-
return clusterRole, nil
590-
}
591-
argoutil.LogResourceCreation(log, clusterRole)
592-
return clusterRole, r.Create(context.TODO(), clusterRole)
593-
}
594-
595-
// ArgoCD not cluster scoped, cleanup any existing resource and exit
596-
if !allowed {
597-
argoutil.LogResourceDeletion(log, existingClusterRole, "argocd not cluster scoped")
598-
err := r.Delete(context.TODO(), existingClusterRole)
599-
if err != nil {
600-
if !apierrors.IsNotFound(err) {
601-
return existingClusterRole, err
602-
}
603-
}
604-
return existingClusterRole, nil
605-
}
606-
607-
// if the Rules differ, update the Role
608-
if !reflect.DeepEqual(existingClusterRole.Rules, clusterRole.Rules) {
609-
existingClusterRole.Rules = clusterRole.Rules
610-
argoutil.LogResourceUpdate(log, existingClusterRole, "updating rules")
611-
if err := r.Update(context.TODO(), existingClusterRole); err != nil {
612-
return nil, err
613-
}
614-
}
615-
return existingClusterRole, nil
616-
}
617-
618-
// reconcileNotificationsClusterRoleBinding reconciles required clusterrolebinding for notifications controller when ArgoCD is cluster-scoped
619-
func (r *ReconcileArgoCD) reconcileNotificationsClusterRoleBinding(cr *argoproj.ArgoCD, role *rbacv1.ClusterRole, sa *corev1.ServiceAccount) error {
620-
621-
allowed := allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES"))
622-
623-
// controller disabled, don't create resources
624-
if !isNotificationsEnabled(cr) {
625-
allowed = false
626-
}
627-
628-
clusterRB := newClusterRoleBindingWithname(common.ArgoCDNotificationsControllerComponent, cr)
629-
clusterRB.Subjects = []rbacv1.Subject{
630-
{
631-
Kind: rbacv1.ServiceAccountKind,
632-
Name: sa.Name,
633-
Namespace: cr.Namespace,
634-
},
635-
}
636-
clusterRB.RoleRef = rbacv1.RoleRef{
637-
APIGroup: rbacv1.GroupName,
638-
Kind: "ClusterRole",
639-
Name: role.Name,
640-
}
641-
642-
if err := applyReconcilerHook(cr, clusterRB, ""); err != nil {
643-
return err
644-
}
645-
646-
existingClusterRB := &rbacv1.ClusterRoleBinding{}
647-
err := r.Get(context.TODO(), types.NamespacedName{Name: clusterRB.Name}, existingClusterRB)
648-
if err != nil {
649-
if !apierrors.IsNotFound(err) {
650-
return fmt.Errorf("failed to reconcile the cluster rolebinding for the service account associated with %s : %s", clusterRB.Name, err)
651-
}
652-
if !allowed {
653-
// Do Nothing
654-
return nil
655-
}
656-
argoutil.LogResourceCreation(log, clusterRB)
657-
return r.Create(context.TODO(), clusterRB)
658-
}
659-
660-
// ArgoCD not cluster scoped, cleanup any existing resource and exit
661-
if !allowed {
662-
argoutil.LogResourceDeletion(log, existingClusterRB, "argocd not cluster scoped")
663-
err := r.Delete(context.TODO(), existingClusterRB)
664-
if err != nil {
665-
if !apierrors.IsNotFound(err) {
666-
return err
667-
}
668-
}
669-
return nil
670-
}
671-
672-
// if subj differ, update the rolebinding
673-
if !reflect.DeepEqual(existingClusterRB.Subjects, clusterRB.Subjects) {
674-
existingClusterRB.Subjects = clusterRB.Subjects
675-
argoutil.LogResourceUpdate(log, existingClusterRB, "updating subjects")
676-
if err := r.Update(context.TODO(), existingClusterRB); err != nil {
677-
return err
678-
}
679-
} else if !reflect.DeepEqual(existingClusterRB.RoleRef, clusterRB.RoleRef) {
680-
// RoleRef can't be updated, delete the rolebinding so that it gets recreated
681-
argoutil.LogResourceDeletion(log, existingClusterRB, "roleref changed, deleting rolebinding so it gets recreated")
682-
_ = r.Delete(context.TODO(), existingClusterRB)
683-
return fmt.Errorf("change detected in roleRef for rolebinding %s of Argo CD instance %s in namespace %s", existingClusterRB.Name, cr.Name, existingClusterRB.Namespace)
684-
}
685-
return nil
686-
}
687-
688552
// reconcileNotificationsSourceNamespacesResources creates role & rolebinding in target source namespaces for notifications controller
689553
// Notifications resources are only created if target source ns is subset of apps source namespaces
690554
func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *argoproj.ArgoCD) error {
@@ -774,7 +638,7 @@ func (r *ReconcileArgoCD) reconcileNotificationsSourceNamespacesResources(cr *ar
774638
Namespace: sourceNamespace,
775639
},
776640
RoleRef: rbacv1.RoleRef{
777-
APIGroup: v1.GroupName,
641+
APIGroup: rbacv1.GroupName,
778642
Kind: "Role",
779643
Name: getResourceNameForNotificationsSourceNamespaces(cr),
780644
},
@@ -835,7 +699,8 @@ func (r *ReconcileArgoCD) getNotificationsCommand(cr *argoproj.ArgoCD) []string
835699
}
836700

837701
if len(notificationsSourceNamespaces) > 0 {
838-
cmd = append(cmd, "--application-namespaces", fmt.Sprint(strings.Join(notificationsSourceNamespaces, ",")))
702+
cmd = append(cmd, "--application-namespaces", strings.Join(notificationsSourceNamespaces, ","))
703+
cmd = append(cmd, "--self-service-notification-enabled", "true")
839704
}
840705

841706
return cmd

‎controllers/argocd/notifications_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func TestReconcileNotifications_Deployments_Command(t *testing.T) {
161161
},
162162
SourceNamespaces: []string{"foo", "bar"},
163163
},
164-
expectedCmd: []string{"--application-namespaces", "foo,bar"},
164+
expectedCmd: []string{"--application-namespaces", "foo,bar", "--self-service-notification-enabled", "true"},
165165
},
166166
{
167167
name: "Only notifications contained in spec.sourceNamespaces",
@@ -172,7 +172,7 @@ func TestReconcileNotifications_Deployments_Command(t *testing.T) {
172172
},
173173
SourceNamespaces: []string{"foo", "bar"},
174174
},
175-
expectedCmd: []string{"--application-namespaces", "foo"},
175+
expectedCmd: []string{"--application-namespaces", "foo", "--self-service-notification-enabled", "true"},
176176
},
177177
{
178178
name: "Empty spec.sourceNamespaces, no application namespaces arg",
@@ -184,7 +184,7 @@ func TestReconcileNotifications_Deployments_Command(t *testing.T) {
184184
SourceNamespaces: []string{},
185185
},
186186
expectedCmd: []string{},
187-
notExpectedCmd: []string{"--application-namespaces", "foo"},
187+
notExpectedCmd: []string{"--application-namespaces", "foo", "--self-service-notification-enabled", "true"},
188188
},
189189
}
190190

‎controllers/argocd/policyrule.go‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -607,28 +607,6 @@ func policyRuleForServerApplicationSetSourceNamespaces() []v1.PolicyRule {
607607
},
608608
}
609609
}
610-
func policyRuleForServerNotificationsSourceNamespaces() []v1.PolicyRule {
611-
return []v1.PolicyRule{
612-
{
613-
APIGroups: []string{
614-
"argoproj.io",
615-
},
616-
Resources: []string{
617-
"applications",
618-
"appprojects",
619-
},
620-
Verbs: []string{
621-
"create",
622-
"get",
623-
"list",
624-
"patch",
625-
"update",
626-
"watch",
627-
"delete",
628-
},
629-
},
630-
}
631-
}
632610

633611
func policyRuleForRoleForImageUpdaterController() []v1.PolicyRule {
634612
return []v1.PolicyRule{

‎controllers/argocd/role.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin
230230
}
231231
// do not reconcile roles for namespaces already containing managed-by label
232232
// as it already contains roles with permissions to manipulate application resources
233-
// reconciled during reconciliation of ManagedNamespaces
233+
// reconciled during reconcilation of ManagedNamespaces
234234
if value, ok := namespace.Labels[common.ArgoCDManagedByLabel]; ok && value != "" {
235235
log.Info(fmt.Sprintf("Skipping reconciling resources for namespace %s as it is already managed-by namespace %s.", namespace.Name, value))
236236
// if managed-by-cluster-argocd label is also present, remove the namespace from the ManagedSourceNamespaces.
@@ -260,9 +260,7 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin
260260
if contains(r.getApplicationSetSourceNamespaces(cr), sourceNamespace) {
261261
role.Rules = append(role.Rules, policyRuleForServerApplicationSetSourceNamespaces()...)
262262
}
263-
if contains(r.getNotificationsSourceNamespaces(cr), sourceNamespace) {
264-
role.Rules = append(role.Rules, policyRuleForServerNotificationsSourceNamespaces()...)
265-
}
263+
266264
created := false
267265
existingRole := v1.Role{}
268266
err := r.Get(context.TODO(), types.NamespacedName{Name: role.Name, Namespace: namespace.Name}, &existingRole)

‎controllers/argocd/role_test.go‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,6 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing.
205205
ApplicationSet: &argoproj.ArgoCDApplicationSet{
206206
SourceNamespaces: []string{"tmp"},
207207
},
208-
Notifications: argoproj.ArgoCDNotifications{
209-
Enabled: true,
210-
SourceNamespaces: []string{"tmp"},
211-
},
212208
}
213209

214210
resObjs := []client.Object{a}
@@ -241,18 +237,12 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing.
241237
ApplicationSet: &argoproj.ArgoCDApplicationSet{
242238
SourceNamespaces: []string{"tmp", sourceNamespace},
243239
},
244-
Notifications: argoproj.ArgoCDNotifications{
245-
Enabled: true,
246-
SourceNamespaces: []string{"tmp", sourceNamespace},
247-
},
248240
}
249241
err = r.reconcileRoleForApplicationSourceNamespaces(workloadIdentifier, expectedRules, a)
250242
assert.NoError(t, err)
251243
reconciledRole = &v1.Role{}
252244
assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: sourceNamespace}, reconciledRole))
253-
expectedRules = append(expectedRules, policyRuleForServerApplicationSetSourceNamespaces()...)
254-
expectedRules = append(expectedRules, policyRuleForServerNotificationsSourceNamespaces()...)
255-
assert.Equal(t, expectedRules, reconciledRole.Rules)
245+
assert.Equal(t, append(expectedRules, policyRuleForServerApplicationSetSourceNamespaces()...), reconciledRole.Rules)
256246

257247
// check if appset rules are removed for server-role when appset namespace is removed from the list
258248
a.Spec = argoproj.ArgoCDSpec{
@@ -262,12 +252,7 @@ func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing.
262252
ApplicationSet: &argoproj.ArgoCDApplicationSet{
263253
SourceNamespaces: []string{"tmp"},
264254
},
265-
Notifications: argoproj.ArgoCDNotifications{
266-
Enabled: true,
267-
SourceNamespaces: []string{"tmp"},
268-
},
269255
}
270-
expectedRules = policyRuleForServerApplicationSourceNamespaces()
271256
err = r.reconcileRoleForApplicationSourceNamespaces(workloadIdentifier, expectedRules, a)
272257
assert.NoError(t, err)
273258
reconciledRole = &v1.Role{}

0 commit comments

Comments
 (0)