Updates CLI to add group support for users

Signed-off-by: PuneetPunamiya <[email protected]>

Author: PuneetPunamiya

pkg/actions/actions.go

@@ -88,7 +88,7 @@ func Update(gr schema.GroupVersionResource, c *cli.Clients, opts *cli.Options) e
 		return err
 	}
 
-	if !containsUsername(at.Spec.Approvers, opts.Username) {
+	if !containsUsername(at.Spec.Approvers, opts) {
 		return fmt.Errorf("Approver: %s, is not present in the approvers list", opts.Username)
 	}
 
@@ -101,6 +101,46 @@ func Update(gr schema.GroupVersionResource, c *cli.Clients, opts *cli.Options) e
 
 func update(gvr *schema.GroupVersionResource, dynamic dynamic.Interface, at *v1alpha1.ApprovalTask, opts *cli.Options) error {
 	for i, approver := range at.Spec.Approvers {
+		switch approver.Type {
+		case "User":
+			if approver.Name == opts.Username {
+				// return true
+				at.Spec.Approvers[i].Input = opts.Input
+				if opts.Message != "" {
+					at.Spec.Approvers[i].Message = opts.Message
+				}
+			}
+		case "Group":
+			for _, groupName := range opts.Groups {
+				if approver.Name == groupName {
+					// return true
+					at.Spec.Approvers[i].Input = opts.Input
+					if opts.Message != "" {
+						at.Spec.Approvers[i].Message = opts.Message
+					}
+
+					userExists := false
+
+					for j, existing := range at.Spec.Approvers[i].Users {
+						if existing.Name == opts.Username {
+							userExists = true
+							if existing.Input != opts.Input {
+								at.Spec.Approvers[i].Users[j].Input = opts.Input
+							}
+							break
+						}
+					}
+					if !userExists {
+						newUser := v1alpha1.UserDetails{
+							Name:  opts.Username,
+							Input: opts.Input,
+						}
+						at.Spec.Approvers[i].Users = append(at.Spec.Approvers[i].Users, newUser)
+					}
+				}
+			}
+		}
+
 		if approver.Name == opts.Username {
 			at.Spec.Approvers[i].Input = opts.Input
 			if opts.Message != "" {
@@ -152,11 +192,26 @@ func InitializeAPIGroupRes(discovery discovery.DiscoveryInterface) error {
 	return nil
 }
 
-func containsUsername(approvers []v1alpha1.ApproverDetails, username string) bool {
+func containsUsername(approvers []v1alpha1.ApproverDetails, user *cli.Options) bool {
 	for _, approver := range approvers {
-		if approver.Name == username {
+		if approver.Name == user.Username {
 			return true
 		}
 	}
+
+	for _, approval := range approvers {
+		switch approval.Type {
+		case "User":
+			if approval.Name == user.Username {
+				return true
+			}
+		case "Group":
+			for _, groupName := range user.Groups {
+				if approval.Name == groupName {
+					return true
+				}
+			}
+		}
+	}
 	return false
 }

pkg/cli/cmd/approve/approve.go

@@ -37,7 +37,7 @@ func Command(p cli.Params) *cobra.Command {
 				ns = ""
 			}
 
-			username, err := p.GetUserInfo()
+			username, groups, err := p.GetUserInfo()
 			if err != nil {
 				return err
 			}
@@ -50,6 +50,7 @@ func Command(p cli.Params) *cobra.Command {
 				Input:     "approve",
 				Username:  username,
 				Message:   message,
+				Groups:    groups,
 			}
 
 			if err := actions.Update(taskGroupResource, cs, opts); err != nil {

pkg/cli/cmd/approve/approve_test.go

@@ -100,6 +100,67 @@ func TestApproveApprovalTask(t *testing.T) {
 				State: "pending",
 			},
 		},
+		// Test case with group approvers
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "at-group-1",
+				Namespace: "foo",
+			},
+			Spec: v1alpha1.ApprovalTaskSpec{
+				Approvers: []v1alpha1.ApproverDetails{
+					{
+						Name:  "admin-group",
+						Input: "pending",
+						Type:  "Group",
+						Users: []v1alpha1.UserDetails{},
+					},
+					{
+						Name:  "dev-team",
+						Input: "pending",
+						Type:  "Group",
+						Users: []v1alpha1.UserDetails{},
+					},
+				},
+				NumberOfApprovalsRequired: 2,
+			},
+			Status: v1alpha1.ApprovalTaskStatus{
+				Approvers: []string{
+					"admin-group",
+					"dev-team",
+				},
+				State: "pending",
+			},
+		},
+		// Test case with mixed user and group approvers
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "at-mixed-1",
+				Namespace: "foo",
+			},
+			Spec: v1alpha1.ApprovalTaskSpec{
+				Approvers: []v1alpha1.ApproverDetails{
+					{
+						Name:  "alice",
+						Input: "pending",
+						Type:  "User",
+					},
+					{
+						Name:  "admin-group",
+						Input: "pending",
+						Type:  "Group",
+						Users: []v1alpha1.UserDetails{},
+					},
+				},
+				NumberOfApprovalsRequired: 2,
+			},
+			Status: v1alpha1.ApprovalTaskStatus{
+				Approvers: []string{
+					"alice",
+					"admin-group",
+				},
+				State: "pending",
+			},
+		},
 	}
 
 	ns := []*corev1.Namespace{
@@ -114,6 +175,8 @@ func TestApproveApprovalTask(t *testing.T) {
 		cb.UnstructuredV1alpha1(approvaltasks[0], "v1alpha1"),
 		cb.UnstructuredV1alpha1(approvaltasks[1], "v1alpha1"),
 		cb.UnstructuredV1alpha1(approvaltasks[2], "v1alpha1"),
+		cb.UnstructuredV1alpha1(approvaltasks[3], "v1alpha1"),
+		cb.UnstructuredV1alpha1(approvaltasks[4], "v1alpha1"),
 	)
 	if err != nil {
 		t.Errorf("unable to create dynamic client: %v", err)
@@ -128,25 +191,61 @@ func TestApproveApprovalTask(t *testing.T) {
 	}{
 		{
 			name:           "approve approval task",
-			command:        command(t, approvaltasks, ns, dc, "tekton"),
+			command:        command(t, approvaltasks, ns, dc, "tekton", []string{}),
 			args:           []string{"at-1", "-n", "foo"},
 			expectedOutput: "ApprovalTask at-1 is approved in foo namespace\n",
 			wantError:      false,
 		},
 		{
 			name:           "invalid username",
-			command:        command(t, approvaltasks, ns, dc, "test-user"),
+			command:        command(t, approvaltasks, ns, dc, "test-user", []string{}),
 			args:           []string{"at-2", "-n", "foo"},
 			expectedOutput: "Error: failed to approve approvalTask from namespace foo: Approver: test-user, is not present in the approvers list\n",
 			wantError:      true,
 		},
 		{
 			name:           "approvaltask not found",
-			command:        command(t, approvaltasks, ns, dc, "tekton"),
+			command:        command(t, approvaltasks, ns, dc, "tekton", []string{}),
 			args:           []string{"at-3", "-n", "test"},
 			expectedOutput: fmt.Sprintf("Error: failed to approve approvalTask from namespace %s: approvaltasks.openshift-pipelines.org \"%s\" not found\n", "test", "at-3"),
 			wantError:      true,
 		},
+		// Group tests
+		{
+			name:           "approve as group member",
+			command:        command(t, approvaltasks, ns, dc, "bob", []string{"admin-group"}),
+			args:           []string{"at-group-1", "-n", "foo"},
+			expectedOutput: "ApprovalTask at-group-1 is approved in foo namespace\n",
+			wantError:      false,
+		},
+		{
+			name:           "user not in any required groups",
+			command:        command(t, approvaltasks, ns, dc, "charlie", []string{"other-group"}),
+			args:           []string{"at-group-1", "-n", "foo"},
+			expectedOutput: "Error: failed to approve approvalTask from namespace foo: Approver: charlie, is not present in the approvers list\n",
+			wantError:      true,
+		},
+		{
+			name:           "approve mixed user and group - as group member",
+			command:        command(t, approvaltasks, ns, dc, "david", []string{"admin-group"}),
+			args:           []string{"at-mixed-1", "-n", "foo"},
+			expectedOutput: "ApprovalTask at-mixed-1 is approved in foo namespace\n",
+			wantError:      false,
+		},
+		{
+			name:           "approve mixed user and group - as direct user",
+			command:        command(t, approvaltasks, ns, dc, "alice", []string{"other-group"}),
+			args:           []string{"at-mixed-1", "-n", "foo"},
+			expectedOutput: "ApprovalTask at-mixed-1 is approved in foo namespace\n",
+			wantError:      false,
+		},
+		{
+			name:           "user in multiple groups but approves through one",
+			command:        command(t, approvaltasks, ns, dc, "eve", []string{"admin-group", "dev-team", "other-group"}),
+			args:           []string{"at-group-1", "-n", "foo"},
+			expectedOutput: "ApprovalTask at-group-1 is approved in foo namespace\n",
+			wantError:      false,
+		},
 	}
 
 	for _, td := range tests {
@@ -163,9 +262,9 @@ func TestApproveApprovalTask(t *testing.T) {
 	}
 }
 
-func command(t *testing.T, approvaltasks []*v1alpha1.ApprovalTask, ns []*corev1.Namespace, dc dynamic.Interface, username string) *cobra.Command {
+func command(t *testing.T, approvaltasks []*v1alpha1.ApprovalTask, ns []*corev1.Namespace, dc dynamic.Interface, username string, groups []string) *cobra.Command {
 	cs, _ := test.SeedTestData(t, test.Data{Approvaltasks: approvaltasks, Namespaces: ns})
-	p := &test.Params{ApprovalTask: cs.ApprovalTask, Kube: cs.Kube, Dynamic: dc, Username: username}
+	p := &test.Params{ApprovalTask: cs.ApprovalTask, Kube: cs.Kube, Dynamic: dc, Username: username, Groups: groups}
 	cs.ApprovalTask.Resources = cb.APIResourceList("v1alpha1", []string{"approvaltask"})
 
 	return Command(p)

pkg/cli/cmd/reject/reject.go

@@ -37,7 +37,7 @@ func Command(p cli.Params) *cobra.Command {
 				ns = ""
 			}
 
-			username, err := p.GetUserInfo()
+			username, groups, err := p.GetUserInfo()
 			if err != nil {
 				return err
 			}
@@ -50,6 +50,7 @@ func Command(p cli.Params) *cobra.Command {
 				Input:     "reject",
 				Username:  username,
 				Message:   message,
+				Groups:    groups,
 			}
 
 			if err := actions.Update(taskGroupResource, cs, opts); err != nil {
@@ -64,7 +65,7 @@ func Command(p cli.Params) *cobra.Command {
 		},
 	}
 
-	c.Flags().StringVarP(&opts.Message, "message", "m", "", "message while approving the approvalTask")
+	c.Flags().StringVarP(&opts.Message, "message", "m", "", "message while rejecting the approvalTask")
 
 	flags.AddOptions(c)