Merge pull request #1125 from mumoshu/admin-force-unlock

Enable admins to forcefully unlock deployments

deploy/lock.go

@@ -38,7 +38,7 @@ func NewCoordinator(ns, configMap string) *Coordinator {
 	}
 }
 
-var ErrLocked = fmt.Errorf("deployment is locked")
+var ErrAlreadyLocked = fmt.Errorf("deployment is already locked")
 var ErrAlreadyUnlocked = fmt.Errorf("deployment is already unlocked")
 
 const (
@@ -82,7 +82,7 @@ func (c *Coordinator) lock(ctx context.Context, project, environment, user, reas
 	}
 
 	if value.Locked {
-		return ErrLocked
+		return ErrAlreadyLocked
 	}
 
 	if n := len(value.History); n >= MaxHistoryItems {
@@ -190,7 +190,7 @@ type NotAllowedTounlockError struct {
 }
 
 func (e NotAllowedTounlockError) Error() string {
-	return fmt.Sprintf("user %s is not allowed to unlock", e.User)
+	return fmt.Sprintf("user %s is not allowed to unlock this project", e.User)
 }
 
 func newNotAllowedToUnlockError(user string) NotAllowedTounlockError {

deploy/lock_test.go

@@ -34,8 +34,8 @@ func TestLockUnlock(t *testing.T) {
 	ctx := context.Background()
 
 	require.NoError(t, c.Lock(ctx, "myproject1", "prod", "user1", "for deployment of revision a"))
-	require.ErrorIs(t, c.Lock(ctx, "myproject1", "prod", "user1", "for deployment of revision b"), ErrLocked)
-	require.ErrorIs(t, c.Lock(ctx, "myproject1", "prod", "user2", "for deployment of revision b"), ErrLocked)
+	require.ErrorIs(t, c.Lock(ctx, "myproject1", "prod", "user1", "for deployment of revision b"), ErrAlreadyLocked)
+	require.ErrorIs(t, c.Lock(ctx, "myproject1", "prod", "user2", "for deployment of revision b"), ErrAlreadyLocked)
 	require.ErrorIs(t, c.Unlock(ctx, "myproject1", "prod", "user2", false), newNotAllowedToUnlockError("user2"))
 	require.NoError(t, c.Unlock(ctx, "myproject1", "prod", "user2", true))
 	require.ErrorIs(t, c.Unlock(ctx, "myproject1", "prod", "user1", false), ErrAlreadyUnlocked)

slack.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -260,11 +259,13 @@ func createDeployButtonSection(pj DeployProject, phaseName string) *slack.Sectio
 func (s *SlackListener) runCommand(cmd slackcmd.Command, triggeredBy string, replyIn string) error {
 	var msgOpt slack.MsgOption
 
+	user := s.userList.FindBySlackUserID(triggeredBy)
+
 	switch cmd := cmd.(type) {
 	case *slackcmd.Lock:
-		msgOpt = s.lock(cmd, triggeredBy, replyIn)
+		msgOpt = s.lock(cmd, user, replyIn)
 	case *slackcmd.Unlock:
-		msgOpt = s.unlock(cmd, triggeredBy, replyIn, false)
+		msgOpt = s.unlock(cmd, user, replyIn, false)
 	default:
 		panic("unreachable")
 	}
@@ -277,32 +278,32 @@ func (s *SlackListener) runCommand(cmd slackcmd.Command, triggeredBy string, rep
 }
 
 // lock locks the given project and environment, and replies to the given channel.
-func (s *SlackListener) lock(cmd *slackcmd.Lock, triggeredBy string, replyIn string) slack.MsgOption {
+func (s *SlackListener) lock(cmd *slackcmd.Lock, triggeredBy User, replyIn string) slack.MsgOption {
 	if err := s.validateProjectEnvUser(cmd.Project, cmd.Env, triggeredBy, replyIn); err != nil {
 		return s.errorMessage(err.Error())
 	}
 
-	if err := s.getOrCreateCoordinator().Lock(context.Background(), cmd.Project, cmd.Env, triggeredBy, cmd.Reason); err != nil {
+	if err := s.getOrCreateCoordinator().Lock(context.Background(), cmd.Project, cmd.Env, triggeredBy.SlackUserID, cmd.Reason); err != nil {
 		return s.errorMessage(err.Error())
 	}
 
 	return s.infoMessage(fmt.Sprintf("Locked %s %s", cmd.Project, cmd.Env))
 }
 
 // unlock unlocks the given project and environment, and replies to the given channel.
-func (s *SlackListener) unlock(cmd *slackcmd.Unlock, triggeredBy string, replyIn string, force bool) slack.MsgOption {
+func (s *SlackListener) unlock(cmd *slackcmd.Unlock, triggeredBy User, replyIn string, force bool) slack.MsgOption {
 	if err := s.validateProjectEnvUser(cmd.Project, cmd.Env, triggeredBy, replyIn); err != nil {
 		return s.errorMessage(err.Error())
 	}
 
-	if err := s.getOrCreateCoordinator().Unlock(context.Background(), cmd.Project, cmd.Env, triggeredBy, force); err != nil {
+	if err := s.getOrCreateCoordinator().Unlock(context.Background(), cmd.Project, cmd.Env, triggeredBy.SlackUserID, triggeredBy.IsAdmin()); err != nil {
 		return s.errorMessage(err.Error())
 	}
 
 	return s.infoMessage(fmt.Sprintf("Unlocked %s %s", cmd.Project, cmd.Env))
 }
 
-func (s *SlackListener) validateProjectEnvUser(projectID, env, userID, replyIn string) error {
+func (s *SlackListener) validateProjectEnvUser(projectID, env string, user User, replyIn string) error {
 	pj, err := s.projectList.FindByAlias(projectID)
 	if err != nil {
 		log.Println("[ERROR] ", err)
@@ -321,13 +322,8 @@ func (s *SlackListener) validateProjectEnvUser(projectID, env, userID, replyIn s
 		return fmt.Errorf("find phase %q: %w", env, err)
 	}
 
-	if user := s.userList.FindBySlackUserID(userID); !user.IsDeveloper() {
-		err = errors.New("you are not allowed to lock this project")
-		log.Println("[ERROR] ", err)
-		if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil {
-			log.Println("[ERROR] ", err)
-		}
-		return fmt.Errorf("find by slack user id %q: %w", userID, err)
+	if !user.IsDeveloper() {
+		return fmt.Errorf("you are not allowed to lock/unlock projects: slack user id %q", user.SlackUserID)
 	}
 
 	return nil

slack_test.go

@@ -63,8 +63,10 @@ var rolebindingConfigMap = corev1.ConfigMap{
 	},
 	Data: map[string]string{
 		"Developer": fmt.Sprintf(`%s
-user3
+user2
+user4
 `, user1SlackDisplayName),
+		"Admin": "user4",
 	},
 }
 
@@ -102,7 +104,7 @@ func TestSlackLockUnlock(t *testing.T) {
 		})
 		// List users
 		c.Handle("/users.list", func(w http.ResponseWriter, r *http.Request) {
-			if _, err := w.Write([]byte(`{"ok": true, "members": [{"id": "U1234", "name": "User 1", "profile": {"display_name": "user1"}}]}`)); err != nil {
+			if _, err := w.Write([]byte(`{"ok": true, "members": [{"id": "U1234", "name": "User 1", "profile": {"display_name": "user1"}}, {"id": "U1235", "name": "User 2", "profile": {"display_name": "user2"}}, {"id": "U1237", "name": "User 4", "profile": {"display_name": "user4"}}]}`)); err != nil {
 				t.Logf("failed to write response: %v", err)
 			}
 		})
@@ -219,7 +221,7 @@ func TestSlackLockUnlock(t *testing.T) {
 		Channel: "C1234",
 		Text:    "lock myproject1 production for deployment of revision a",
 	}))
-	require.Equal(t, "deployment is locked", nextMessage().Text())
+	require.Equal(t, "deployment is already locked", nextMessage().Text())
 
 	require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{
 		User:    "U1234",
@@ -235,19 +237,47 @@ func TestSlackLockUnlock(t *testing.T) {
 	}))
 	require.Equal(t, "deployment is already unlocked", nextMessage().Text())
 
+	//
+	// We assume that any users who aren't visible from the Slack API are not allowed to lock/unlock projects.
+	//
+
 	require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{
-		User:    "U1235",
+		User:    "U1236",
 		Channel: "C1234",
 		Text:    "lock myproject1 production for deployment of revision a",
 	}))
-	require.Equal(t, "you are not allowed to lock this project", nextMessage().Text())
+	require.Equal(t, "you are not allowed to lock/unlock projects: slack user id \"\"", nextMessage().Text())
+
+	require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{
+		User:    "U1236",
+		Channel: "C1234",
+		Text:    "unlock myproject1 production",
+	}))
+	require.Equal(t, "you are not allowed to lock/unlock projects: slack user id \"\"", nextMessage().Text())
 
+	// User 2 is a developer so can lock the project
 	require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{
 		User:    "U1235",
 		Channel: "C1234",
+		Text:    "lock myproject1 production for deployment of revision a",
+	}))
+	require.Equal(t, "Locked myproject1 production", nextMessage().Text())
+
+	// User 1 is a developer so cannot unlock the project forcefully
+	require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{
+		User:    "U1234",
+		Channel: "C1234",
+		Text:    "unlock myproject1 production",
+	}))
+	require.Equal(t, "user U1234 is not allowed to unlock this project", nextMessage().Text())
+
+	// User 4 is an admin and can unlock the project forcefully
+	require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{
+		User:    "U1237",
+		Channel: "C1234",
 		Text:    "unlock myproject1 production",
 	}))
-	require.Equal(t, "find by slack user id \"U1235\": you are not allowed to lock this project", nextMessage().Text())
+	require.Equal(t, "Unlocked myproject1 production", nextMessage().Text())
 }
 
 // Message is a message posted to the fake Slack API's chat.postMessage endpoint

user.go

@@ -5,6 +5,20 @@ import (
 	"strings"
 
 	"github.com/slack-go/slack"
+	v1 "k8s.io/api/core/v1"
+)
+
+// Role is a type to represent a role of a user.
+//
+// A user can have multiple roles.
+// But we currently assume any Admin users are also Developer users.
+// So, a consumer of this type should be able to check if a user has an equal or higher role than
+// Developer by checking if the user has the Developer role.
+type Role string
+
+const (
+	RoleDeveloper Role = "Developer"
+	RoleAdmin     Role = "Admin"
 )
 
 type User struct {
@@ -13,12 +27,20 @@ type User struct {
 	GitHubUserName   string
 	GitHubNodeID     string
 	isDeveloper      bool
+	isAdmin          bool
 }
 
 func (u User) IsDeveloper() bool {
 	return u.isDeveloper
 }
 
+// IsAdmin returns a flag to indicate whether the user is an admin or not.
+// An admin can force-unlock a project/phase using `@bot unlock` command,
+// even if the project/phase is locked by other users.
+func (u User) IsAdmin() bool {
+	return u.isAdmin
+}
+
 type UserList struct {
 	Items       []User
 	github      GitHub
@@ -42,6 +64,7 @@ func (ul *UserList) Reload() {
 
 	cml := getConfigMapList("githubuser-mapping")
 	rolebindings := getConfigMapList("rolebinding")
+	userNamesInGroups := ul.createUserNamesInGroups(rolebindings)
 
 	for _, slackUser := range slackUsers {
 		if slackUser.IsBot || slackUser.Deleted {
@@ -55,18 +78,29 @@ func (ul *UserList) Reload() {
 			}
 		}
 		user.GitHubNodeID = githubUsers[user.GitHubUserName]
-		for _, rolebinding := range rolebindings.Items {
-			raw := rolebinding.Data["Developer"]
+		_, user.isDeveloper = userNamesInGroups[RoleDeveloper][user.SlackDisplayName]
+		_, user.isAdmin = userNamesInGroups[RoleAdmin][user.SlackDisplayName]
+		ul.Items = append(ul.Items, user)
+	}
+}
+
+func (ul UserList) createUserNamesInGroups(rolebindings *v1.ConfigMapList) map[Role]map[string]struct{} {
+	userNamesInGroups := map[Role]map[string]struct{}{
+		RoleDeveloper: {},
+		RoleAdmin:     {},
+	}
+	for _, rolebinding := range rolebindings.Items {
+		for group, users := range userNamesInGroups {
+			raw := rolebinding.Data[string(group)]
 			userNames := strings.Split(raw, "\n")
 			for _, userName := range userNames {
-				if user.SlackDisplayName == userName {
-					user.isDeveloper = true
-					break
+				if userName != "" {
+					users[userName] = struct{}{}
 				}
 			}
 		}
-		ul.Items = append(ul.Items, user)
 	}
+	return userNamesInGroups
 }
 
 func (ul UserList) FindBySlackUserID(slackUserID string) User {

user_test.go

@@ -0,0 +1,70 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestCreateUserNamesInGroups(t *testing.T) {
+	var (
+		ul UserList
+
+		rolebindings = &v1.ConfigMapList{
+			Items: []v1.ConfigMap{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rolebinding1",
+						Labels: map[string]string{
+							"gocat.zaim.net/configmap-type": "rolebinding",
+						},
+					},
+					Data: map[string]string{
+						"Developer": "user1\nuser2\n",
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rolebinding2",
+						Labels: map[string]string{
+							"gocat.zaim.net/configmap-type": "rolebinding",
+						},
+					},
+					Data: map[string]string{
+						"Admin": "user3\nuser4",
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rolebinding3",
+						Labels: map[string]string{
+							"gocat.zaim.net/configmap-type": "rolebinding",
+						},
+					},
+					Data: map[string]string{
+						"Developer": "user5",
+						"Admin":     "user6",
+					},
+				},
+			},
+		}
+	)
+
+	userNamesInGroups := ul.createUserNamesInGroups(rolebindings)
+
+	if len(userNamesInGroups) != 2 {
+		t.Fatalf("userNamesInGroups length is not 2")
+	}
+
+	require.Equal(t,
+		map[string]struct{}{"user1": {}, "user2": {}, "user5": {}},
+		userNamesInGroups[RoleDeveloper],
+	)
+
+	require.Equal(t,
+		map[string]struct{}{"user3": {}, "user4": {}, "user6": {}},
+		userNamesInGroups[RoleAdmin],
+	)
+}