From 958518a6976d434d0ab0ba1f60b9ce71c9cfb52a Mon Sep 17 00:00:00 2001 From: Neel Dalsania Date: Thu, 11 Aug 2022 12:23:22 +0530 Subject: [PATCH] added checks on workspace & deployment roles and ids for team list (#683) --- software/deployment/teams.go | 24 ++++++++++++++++++-- software/deployment/teams_test.go | 37 ++++++++++++++++++++++++++++++- software/workspace/teams.go | 24 ++++++++++++++++++-- software/workspace/teams_test.go | 37 ++++++++++++++++++++++++++++++- 4 files changed, 116 insertions(+), 6 deletions(-) diff --git a/software/deployment/teams.go b/software/deployment/teams.go index 1f1408041..5d1c685bd 100644 --- a/software/deployment/teams.go +++ b/software/deployment/teams.go @@ -30,8 +30,9 @@ func ListTeamRoles(deploymentID string, client houston.ClientInterface, out io.W // Build rows for i := range deploymentTeams { - for j := range deploymentTeams[i].RoleBindings { - tab.AddRow([]string{deploymentID, deploymentTeams[i].ID, deploymentTeams[i].Name, deploymentTeams[i].RoleBindings[j].Role}, false) + role := getDeploymentLevelRole(deploymentTeams[i].RoleBindings, deploymentID) + if role != houston.NoneTeamRole { + tab.AddRow([]string{deploymentID, deploymentTeams[i].ID, deploymentTeams[i].Name, role}, false) } } @@ -100,3 +101,22 @@ func RemoveTeam(deploymentID, teamID string, client houston.ClientInterface, out return nil } + +// isValidDeploymentLevelRole checks if the role is amongst valid workspace roles +func isValidDeploymentLevelRole(role string) bool { + switch role { + case houston.DeploymentAdminRole, houston.DeploymentEditorRole, houston.DeploymentViewerRole, houston.NoneTeamRole: + return true + } + return false +} + +// getDeploymentLevelRole returns the first system level role from a slice of roles +func getDeploymentLevelRole(roles []houston.RoleBinding, deploymentID string) string { + for i := range roles { + if isValidDeploymentLevelRole(roles[i].Role) && roles[i].Deployment.ID == deploymentID { + return roles[i].Role + } + } + return houston.NoneTeamRole +} diff --git a/software/deployment/teams_test.go b/software/deployment/teams_test.go index b40a38804..5bf07b17c 100644 --- a/software/deployment/teams_test.go +++ b/software/deployment/teams_test.go @@ -64,7 +64,11 @@ func TestRemoveTeam(t *testing.T) { func TestListTeamRoles(t *testing.T) { t.Run("success", func(t *testing.T) { mock := new(houston_mocks.ClientInterface) - mock.On("ListDeploymentTeamsAndRoles", "deployment-id").Return([]houston.Team{{ID: "test-id-1", Name: "test-name-1", RoleBindings: []houston.RoleBinding{{Role: "test-role-1"}}}, {ID: "test-id-2", Name: "test-name-2", RoleBindings: []houston.RoleBinding{{Role: "test-role-2"}}}}, nil) + mock.On("ListDeploymentTeamsAndRoles", "deployment-id").Return( + []houston.Team{ + {ID: "test-id-1", Name: "test-name-1", RoleBindings: []houston.RoleBinding{{Role: houston.DeploymentViewerRole, Deployment: houston.Deployment{ID: "deployment-id"}}}}, + {ID: "test-id-2", Name: "test-name-2", RoleBindings: []houston.RoleBinding{{Role: houston.DeploymentAdminRole, Deployment: houston.Deployment{ID: "deployment-id"}}}}, + }, nil) buf := new(bytes.Buffer) err := ListTeamRoles("deployment-id", mock, buf) @@ -113,3 +117,34 @@ func TestUpdateTeamRole(t *testing.T) { mock.AssertExpectations(t) }) } + +func TestGetDeploymentLevelRole(t *testing.T) { + tests := []struct { + roleBinding []houston.RoleBinding + deploymentID string + result string + }{ + { + roleBinding: []houston.RoleBinding{ + {Role: houston.SystemAdminRole}, + {Role: houston.DeploymentAdminRole, Deployment: houston.Deployment{ID: "test-id-1"}}, + {Role: houston.DeploymentEditorRole, Deployment: houston.Deployment{ID: "test-id-2"}}, + }, + deploymentID: "test-id-1", + result: houston.DeploymentAdminRole, + }, + { + roleBinding: []houston.RoleBinding{ + {Role: houston.SystemAdminRole}, + {Role: houston.DeploymentEditorRole, Deployment: houston.Deployment{ID: "test-id-2"}}, + }, + deploymentID: "test-id-1", + result: houston.NoneTeamRole, + }, + } + + for _, tt := range tests { + resp := getDeploymentLevelRole(tt.roleBinding, tt.deploymentID) + assert.Equal(t, tt.result, resp, "expected: %v, actual: %v", tt.result, resp) + } +} diff --git a/software/workspace/teams.go b/software/workspace/teams.go index 523f4a2df..f8995945d 100644 --- a/software/workspace/teams.go +++ b/software/workspace/teams.go @@ -64,8 +64,9 @@ func ListTeamRoles(workspaceID string, client houston.ClientInterface, out io.Wr Header: []string{"WORKSPACE ID", "TEAM ID", "TEAM NAME", "ROLE"}, } for i := range workspaceTeams { - for j := range workspaceTeams[i].RoleBindings { - tab.AddRow([]string{workspaceID, workspaceTeams[i].ID, workspaceTeams[i].Name, workspaceTeams[i].RoleBindings[j].Role}, false) + role := getWorkspaceLevelRole(workspaceTeams[i].RoleBindings, workspaceID) + if role != houston.NoneTeamRole { + tab.AddRow([]string{workspaceID, workspaceTeams[i].ID, workspaceTeams[i].Name, role}, false) } } tab.Print(out) @@ -103,3 +104,22 @@ func UpdateTeamRole(workspaceID, teamID, role string, client houston.ClientInter fmt.Fprintf(out, "Role has been changed from %s to %s for team %s\n", rb.Role, newRole, teamID) return nil } + +// isValidWorkspaceLevelRole checks if the role is amongst valid workspace roles +func isValidWorkspaceLevelRole(role string) bool { + switch role { + case houston.WorkspaceAdminRole, houston.WorkspaceEditorRole, houston.WorkspaceViewerRole, houston.NoneTeamRole: + return true + } + return false +} + +// getWorkspaceLevelRole returns the first system level role from a slice of roles +func getWorkspaceLevelRole(roles []houston.RoleBinding, workspaceID string) string { + for i := range roles { + if isValidWorkspaceLevelRole(roles[i].Role) && roles[i].Workspace.ID == workspaceID { + return roles[i].Role + } + } + return houston.NoneTeamRole +} diff --git a/software/workspace/teams_test.go b/software/workspace/teams_test.go index 91d6ad0d9..7ada5c17b 100644 --- a/software/workspace/teams_test.go +++ b/software/workspace/teams_test.go @@ -64,7 +64,11 @@ func TestRemoveTeam(t *testing.T) { func TestListTeamRoles(t *testing.T) { t.Run("success", func(t *testing.T) { mock := new(houston_mocks.ClientInterface) - mock.On("ListWorkspaceTeamsAndRoles", "workspace-id").Return([]houston.Team{{ID: "test-id-1", Name: "test-name-1", RoleBindings: []houston.RoleBinding{{Role: "test-role-1"}}}, {ID: "test-id-2", Name: "test-name-2", RoleBindings: []houston.RoleBinding{{Role: "test-role-2"}}}}, nil) + mock.On("ListWorkspaceTeamsAndRoles", "workspace-id").Return( + []houston.Team{ + {ID: "test-id-1", Name: "test-name-1", RoleBindings: []houston.RoleBinding{{Role: houston.WorkspaceViewerRole, Workspace: houston.Workspace{ID: "workspace-id"}}}}, + {ID: "test-id-2", Name: "test-name-2", RoleBindings: []houston.RoleBinding{{Role: houston.WorkspaceAdminRole, Workspace: houston.Workspace{ID: "workspace-id"}}}}, + }, nil) buf := new(bytes.Buffer) err := ListTeamRoles("workspace-id", mock, buf) @@ -148,3 +152,34 @@ func TestUpdateTeamRole(t *testing.T) { mock.AssertExpectations(t) }) } + +func TestGetWorkspaceLevelRole(t *testing.T) { + tests := []struct { + roleBinding []houston.RoleBinding + workspaceID string + result string + }{ + { + roleBinding: []houston.RoleBinding{ + {Role: houston.SystemAdminRole}, + {Role: houston.WorkspaceAdminRole, Workspace: houston.Workspace{ID: "test-id-1"}}, + {Role: houston.WorkspaceEditorRole, Workspace: houston.Workspace{ID: "test-id-2"}}, + }, + workspaceID: "test-id-1", + result: houston.WorkspaceAdminRole, + }, + { + roleBinding: []houston.RoleBinding{ + {Role: houston.SystemAdminRole}, + {Role: houston.WorkspaceEditorRole, Workspace: houston.Workspace{ID: "test-id-2"}}, + }, + workspaceID: "test-id-1", + result: houston.NoneTeamRole, + }, + } + + for _, tt := range tests { + resp := getWorkspaceLevelRole(tt.roleBinding, tt.workspaceID) + assert.Equal(t, tt.result, resp, "expected: %v, actual: %v", tt.result, resp) + } +}