Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable admins to forcefully unlock deployments #1125

Merged
merged 2 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions deploy/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions deploy/lock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 11 additions & 15 deletions slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"log"
"net/http"
Expand Down Expand Up @@ -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")
}
Expand All @@ -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)
Expand All @@ -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
Expand Down
42 changes: 36 additions & 6 deletions slack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ var rolebindingConfigMap = corev1.ConfigMap{
},
Data: map[string]string{
"Developer": fmt.Sprintf(`%s
user3
user2
user4
`, user1SlackDisplayName),
"Admin": "user4",
},
}

Expand Down Expand Up @@ -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)
}
})
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down
46 changes: 40 additions & 6 deletions user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
70 changes: 70 additions & 0 deletions user_test.go
Original file line number Diff line number Diff line change
@@ -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],
)
}
Loading