From 1780e59c25ee40600a31295ff1491a97ecb2c7c7 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 2 Aug 2024 02:56:43 +0000 Subject: [PATCH 01/12] Initial implementation of /lock and /unlock Enables the integration of slackcmd and deploy.Coordinator into the SlackListener, so that it can try to lock/unlock projects and environments when it sees /lock and /unlock respectively. --- project.go | 4 ++ slack.go | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 115 insertions(+), 2 deletions(-) diff --git a/project.go b/project.go index aba9409..92de781 100644 --- a/project.go +++ b/project.go @@ -34,6 +34,10 @@ type DeployPhase struct { Destination Destination `yaml:"destination"` } +func (p DeployPhase) None() bool { + return p.Name == "" +} + type DeployProject struct { // ID is the name of the configmap that defines the project. ID string diff --git a/slack.go b/slack.go index 3c9541e..1f4b4a3 100644 --- a/slack.go +++ b/slack.go @@ -2,16 +2,20 @@ package main import ( "bytes" + "context" "encoding/json" + "errors" "fmt" "log" "net/http" "regexp" "strconv" "strings" + "sync" "github.com/slack-go/slack" "github.com/slack-go/slack/slackevents" + "github.com/zaiminc/gocat/deploy" "github.com/zaiminc/gocat/slackcmd" ) @@ -23,6 +27,9 @@ type SlackListener struct { projectList *ProjectList userList *UserList interactorFactory *InteractorFactory + + coordinator *deploy.Coordinator + mu sync.Mutex } func (s SlackListener) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -182,8 +189,7 @@ func (s *SlackListener) handleMessageEvent(ev *slackevents.AppMentionEvent) erro } if cmd, _ := slackcmd.Parse(ev.Text); cmd != nil { log.Printf("[INFO] %s command is Called", cmd.Name()) - // TODO run the command and post the result to slack - return nil + return s.runCommand(cmd, ev.User, ev.Channel) } return nil } @@ -247,6 +253,109 @@ func createDeployButtonSection(pj DeployProject, phaseName string) *slack.Sectio return section } +// runCommand runs the given command. +// +// triggeredBy is the ID of the Slack user who triggered the command, +// and replyIn is the ID of the Slack channel to reply to. +func (s *SlackListener) runCommand(cmd slackcmd.Command, triggeredBy string, replyIn string) error { + var msgOpt slack.MsgOption + + switch cmd := cmd.(type) { + case *slackcmd.Lock: + msgOpt = s.lock(cmd, triggeredBy, replyIn) + case *slackcmd.Unlock: + msgOpt = s.unlock(cmd, triggeredBy, replyIn, false) + default: + panic("unreachable") + } + + if _, _, err := s.client.PostMessage(replyIn, msgOpt); err != nil { + log.Println("[ERROR] ", err) + } + + return nil +} + +// 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 { + if ok := s.validateProjectEnvUser(cmd.Project, cmd.Env, triggeredBy, replyIn); !ok { + return nil + } + + if err := s.getOrCreateCoordinator().Lock(context.Background(), cmd.Project, cmd.Env, triggeredBy, cmd.Reason); err != nil { + log.Println("[ERROR] ", err) + if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { + log.Println("[ERROR] ", err) + } + return nil + } + + 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 { + if ok := s.validateProjectEnvUser(cmd.Project, cmd.Env, triggeredBy, replyIn); !ok { + return nil + } + + if err := s.getOrCreateCoordinator().Unlock(context.Background(), cmd.Project, cmd.Env, triggeredBy, force); err != nil { + log.Println("[ERROR] ", err) + if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { + log.Println("[ERROR] ", err) + } + return nil + } + + return s.infoMessage(fmt.Sprintf("Unlocked %s %s", cmd.Project, cmd.Env)) +} + +func (s *SlackListener) validateProjectEnvUser(projectID, env, userID, replyIn string) bool { + pj, err := s.projectList.FindByAlias(projectID) + if err != nil { + log.Println("[ERROR] ", err) + if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { + log.Println("[ERROR] ", err) + } + return false + } + + if phase := pj.FindPhase(env); phase.None() { + err = fmt.Errorf("phase %s is not found", env) + log.Println("[ERROR] ", err) + if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { + log.Println("[ERROR] ", err) + } + return false + } + + 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 false + } + + return true +} + +func (s *SlackListener) getOrCreateCoordinator() *deploy.Coordinator { + s.mu.Lock() + defer s.mu.Unlock() + if s.coordinator == nil { + s.coordinator = deploy.NewCoordinator("gocat", "deploylocks") + } + return s.coordinator +} + +func (s *SlackListener) infoMessage(message string) slack.MsgOption { + txt := slack.NewTextBlockObject("mrkdwn", message, false, false) + section := slack.NewSectionBlock(txt, nil, nil) + return slack.MsgOptionBlocks(section) +} + func (s *SlackListener) errorMessage(message string) slack.MsgOption { txt := slack.NewTextBlockObject("mrkdwn", message, false, false) section := slack.NewSectionBlock(txt, nil, nil) From 017dc239b3a898b0ab2eea9720a9a520ac5bbd7e Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 01:45:32 +0000 Subject: [PATCH 02/12] Extract Kubernetes client utility as a library --- deploy/configmap.go | 6 +++--- deploy/kubernetes.go | 32 ++++++++++++++++++++++++++++++-- deploy/lock.go | 3 +-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/deploy/configmap.go b/deploy/configmap.go index a367bd3..a19621c 100644 --- a/deploy/configmap.go +++ b/deploy/configmap.go @@ -72,7 +72,7 @@ func (c *Coordinator) getOrCreateConfigMap(ctx context.Context) (*corev1.ConfigM // getConfigMap creates a Kubernetes API client, and use it to retrieve the ConfigMap. func (c *Coordinator) getConfigMap(ctx context.Context) (*corev1.ConfigMap, error) { - clientset, err := c.kubernetesClientSet() + clientset, err := c.ClientSet() if err != nil { return nil, err } @@ -90,7 +90,7 @@ func (c *Coordinator) getConfigMap(ctx context.Context) (*corev1.ConfigMap, erro } func (c *Coordinator) createConfigMap(ctx context.Context) (*corev1.ConfigMap, error) { - clientset, err := c.kubernetesClientSet() + clientset, err := c.ClientSet() if err != nil { return nil, err } @@ -114,7 +114,7 @@ func (c *Coordinator) createConfigMap(ctx context.Context) (*corev1.ConfigMap, e } func (c *Coordinator) updateConfigMap(ctx context.Context, configMap *corev1.ConfigMap) (*corev1.ConfigMap, error) { - clientset, err := c.kubernetesClientSet() + clientset, err := c.ClientSet() if err != nil { return nil, err } diff --git a/deploy/kubernetes.go b/deploy/kubernetes.go index 71e1cb5..10449b1 100644 --- a/deploy/kubernetes.go +++ b/deploy/kubernetes.go @@ -8,10 +8,38 @@ import ( "k8s.io/client-go/tools/clientcmd" ) +// Kubernetes is a helper struct that provides a way to create and cache a Kubernetes API client. +// The client is created using the KUBECONFIG file if exists, or the in-cluster configuration. +// The client is cached to avoid creating multiple clients. +// +// Usage: +// +// k := Kubernetes{} +// clientset, err := k.ClientSet() +// if err != nil { +// return err +// } +// // Use the clientset +// +// // Or you can embed Kubernetes in your struct: +// type MyStruct struct { +// Kubernetes +// } +// func (m *MyStruct) MyMethod() error { +// clientset, err := m.ClientSet() +// if err != nil { +// return err +// } +// // Use the clientset +// } +type Kubernetes struct { + clientset clientset.Interface +} + // kubeconfigPath returns the path to the KUBECONFIG file, // which is either specified by the KUBECONFIG environment variable, // or the default path ~/.kube/config. -func (c *Coordinator) kubeconfigPath() string { +func (c *Kubernetes) kubeconfigPath() string { kubeconfig := clientcmd.NewDefaultClientConfigLoadingRules().GetDefaultFilename() if path := os.Getenv("KUBECONFIG"); path != "" { kubeconfig = path @@ -22,7 +50,7 @@ func (c *Coordinator) kubeconfigPath() string { // kubernetesClientSet creates a Kubernetes API client // that uses either the KUBECONFIG file if exists, or the in-cluster configuration. -func (c *Coordinator) kubernetesClientSet() (clientset.Interface, error) { +func (c *Kubernetes) ClientSet() (clientset.Interface, error) { if c.clientset != nil { return c.clientset, nil } diff --git a/deploy/lock.go b/deploy/lock.go index 457632a..ba6c494 100644 --- a/deploy/lock.go +++ b/deploy/lock.go @@ -6,7 +6,6 @@ import ( kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clientset "k8s.io/client-go/kubernetes" ) // Coordinator provides a way to lock and unlock deployments made via gocat. @@ -29,7 +28,7 @@ type Coordinator struct { // ConfigMapName is the name of the ConfigMap. ConfigMapName string - clientset clientset.Interface + Kubernetes } func NewCoordinator(ns, configMap string) *Coordinator { From 93776568eca6e624643f9fe2d60829a7ae990850 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 01:46:32 +0000 Subject: [PATCH 03/12] CreateGitHubInstance supports fake GitHub API --- bot.go | 2 +- github.go | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/bot.go b/bot.go index 26e4509..fe683bc 100644 --- a/bot.go +++ b/bot.go @@ -19,7 +19,7 @@ func main() { config.SlackOAuthToken, slack.OptionLog(log.New(os.Stdout, "slack-bot: ", log.Lshortfile|log.LstdFlags)), ) - github := CreateGitHubInstance(config.GitHubAccessToken, config.ManifestRepositoryOrg, config.ManifestRepositoryName, config.GitHubDefaultBranch) + github := CreateGitHubInstance("", config.GitHubAccessToken, config.ManifestRepositoryOrg, config.ManifestRepositoryName, config.GitHubDefaultBranch) git := CreateGitOperatorInstance( config.GitHubUserName, config.GitHubAccessToken, diff --git a/github.go b/github.go index 8d9b610..e05574c 100644 --- a/github.go +++ b/github.go @@ -36,13 +36,18 @@ type GitHubInput struct { Branch string } -func CreateGitHubInstance(token, org, repo, defaultBranch string) GitHub { +func CreateGitHubInstance(url, token, org, repo, defaultBranch string) GitHub { src := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: token}, ) httpClient := oauth2.NewClient(context.Background(), src) - client := githubv4.NewClient(httpClient) + var client *githubv4.Client + if url != "" { + client = githubv4.NewEnterpriseClient(url, httpClient) + } else { + client = githubv4.NewClient(httpClient) + } return GitHub{*client, httpClient, org, repo, defaultBranch} } From 5f80d3d1f08290eb7d637eeaae4d2b769ed7b31c Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 01:48:11 +0000 Subject: [PATCH 04/12] Ensure the test configmap is cleaned up --- deploy/lock_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/deploy/lock_test.go b/deploy/lock_test.go index da99814..9e3c1f1 100644 --- a/deploy/lock_test.go +++ b/deploy/lock_test.go @@ -6,6 +6,8 @@ import ( "os" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/stretchr/testify/require" ) @@ -17,6 +19,11 @@ func TestLockUnlock(t *testing.T) { } c := NewCoordinator("default", "gocat-test") + defer func() { + if c, _ := c.ClientSet(); c != nil { + c.CoreV1().ConfigMaps("default").Delete(context.Background(), "gocat-test", metav1.DeleteOptions{}) + } + }() prevKubeconfig := os.Getenv("KUBECONFIG") os.Setenv("KUBECONFIG", kubeconfigPath) From 9bb907c39fa33724c103f8cea7ab210921a3c712 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 01:48:45 +0000 Subject: [PATCH 05/12] Add integration test for /lock and /unlock --- slack_test.go | 289 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 289 insertions(+) create mode 100644 slack_test.go diff --git a/slack_test.go b/slack_test.go new file mode 100644 index 0000000..869af01 --- /dev/null +++ b/slack_test.go @@ -0,0 +1,289 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "testing" + "time" + + "github.com/slack-go/slack" + "github.com/slack-go/slack/slackevents" + "github.com/slack-go/slack/slacktest" + "github.com/stretchr/testify/require" + "github.com/zaiminc/gocat/deploy" + + corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +var myprojectConfigMap = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myproject1", + Labels: map[string]string{ + "gocat.zaim.net/configmap-type": "project", + }, + }, + Data: map[string]string{ + "Phases": `- name: production +`, + }, +} + +const ( + user1SlackDisplayName = "user1" + user1GitHubLogin = "user1" +) + +var githubuserMappingConfigMap = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "githubuser-mapping", + Labels: map[string]string{ + "gocat.zaim.net/configmap-type": "githubuser-mapping", + }, + }, + Data: map[string]string{ + user1SlackDisplayName: user1GitHubLogin, + }, +} + +var rolebindingConfigMap = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rolebinding", + Labels: map[string]string{ + "gocat.zaim.net/configmap-type": "rolebinding", + }, + }, + Data: map[string]string{ + "Developer": fmt.Sprintf(`%s +user3 +`, user1SlackDisplayName), + }, +} + +// TestSlackLockUnlock tests the lock and unlock commands against +// a pre-configured Kubernetes cluster, fake Slack API, and fake GitHub API. +// It verifies that the lock and unlock commands work as expected, by sending +// messages to the fake Slack API and checking the messages that are sent from gocat. +func TestSlackLockUnlock(t *testing.T) { + if testing.Short() { + t.Skip() + } + + var k = &deploy.Kubernetes{} + clientset, err := k.ClientSet() + require.NoError(t, err) + + setupConfigMaps(t, clientset, myprojectConfigMap, githubuserMappingConfigMap, rolebindingConfigMap) + setupNamespace(t, clientset, "gocat") + + var token = os.Getenv("SLACK_TOKEN") + require.NotEmpty(t, token, "SLACK_TOKEN must be set") + + messages := make(chan message, 10) + nextMessage := func() message { + select { + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for message") + case m := <-messages: + return m + } + return message{} + } + ts := slacktest.NewTestServer(func(c slacktest.Customize) { + c.Handle("/conversations.history", func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(``)) + }) + // List users + c.Handle("/users.list", func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{"ok": true, "members": [{"id": "U1234", "name": "User 1", "profile": {"display_name": "user1"}}]}`)) + }) + // Message posted to the channel + c.Handle("/chat.postMessage", func(w http.ResponseWriter, r *http.Request) { + m := message{ + channel: r.FormValue("channel"), + Blocks: []block{}, + } + blocksValue := r.FormValue("blocks") + if err := json.Unmarshal([]byte(blocksValue), &m.Blocks); err != nil { + t.Logf("failed to unmarshal blocks: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + messages <- m + w.Write([]byte(`{"ok": true}`)) + }) + }) + ts.Start() + defer ts.Stop() + + // ghts is a test HTTP server that serves GitHub API v2 (GraphQL) responses. + ghts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch r.URL.Path { + case "/graphql": + body, err := io.ReadAll(r.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + t.Logf("request body: %s", string(body)) + + // Returns ids and logins of the users who are members of the organization + + w.Write([]byte(`{"data": {"organization": {"membersWithRole": {"nodes": [{"id": "U1234", "login": "user1"}]}}}}`)) + default: + http.Error(w, "not found", http.StatusNotFound) + } + })) + defer ghts.Close() + + s := slack.New(token, + slack.OptionAPIURL(ts.GetAPIURL()), + ) + + config, err := InitConfig() + require.NoError(t, err) + + gh := CreateGitHubInstance( + ghts.URL+"/graphql", + config.GitHubAccessToken, + config.ManifestRepositoryOrg, + config.ManifestRepositoryName, + config.GitHubDefaultBranch, + ) + git := CreateGitOperatorInstance( + config.GitHubUserName, + config.GitHubAccessToken, + config.ManifestRepository, + config.GitHubDefaultBranch, + os.Getenv("GOCAT_GITROOT"), + ) + + userList := UserList{github: gh, slackClient: s} + projectList := NewProjectList() + interactorContext := InteractorContext{ + projectList: &projectList, + userList: &userList, + github: gh, + git: git, + client: s, + config: *config, + } + interactorFactory := NewInteractorFactory(interactorContext) + + var l = &SlackListener{ + client: s, + verificationToken: "token", + projectList: &projectList, + userList: &userList, + interactorFactory: &interactorFactory, + } + + require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{ + User: "U1234", + Channel: "C1234", + Text: "lock myproject1 production for deployment of revision a", + })) + require.Equal(t, "Locked myproject1 production", nextMessage().Text()) + + require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{ + User: "U1234", + Channel: "C1234", + Text: "lock myproject1 production for deployment of revision a", + })) + require.Equal(t, "deployment is locked", nextMessage().Text()) + + require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{ + User: "U1234", + Channel: "C1234", + Text: "unlock myproject1 production", + })) + require.Equal(t, "Unlocked myproject1 production", nextMessage().Text()) + + require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{ + User: "U1234", + Channel: "C1234", + Text: "unlock myproject1 production", + })) + require.Equal(t, "deployment is already unlocked", nextMessage().Text()) + + require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{ + User: "U1235", + 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.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{ + User: "U1235", + 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()) +} + +// Message is a message posted to the fake Slack API's chat.postMessage endpoint +type message struct { + Blocks []block + channel string +} + +func (m message) Text() string { + for _, b := range m.Blocks { + if b.Type == "section" { + return b.Text.Text + } + } + return "" +} + +type block struct { + // For example, "section" + Type string `json:"type"` + Text text `json:"text"` +} + +type text struct { + // For example, "mrkdwn" + Type string `json:"type"` + Text string `json:"text"` +} + +func setupNamespace(t *testing.T, clientset kubernetes.Interface, name string) { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } + _, err := clientset.CoreV1().Namespaces().Create(context.Background(), ns, metav1.CreateOptions{}) + if kerrors.IsAlreadyExists(err) { + _, err = clientset.CoreV1().Namespaces().Update(context.Background(), ns, metav1.UpdateOptions{}) + require.NoError(t, err) + } + t.Cleanup(func() { + clientset.CoreV1().Namespaces().Delete(context.Background(), ns.Name, metav1.DeleteOptions{}) + }) +} + +func setupConfigMaps(t *testing.T, clientset kubernetes.Interface, configMaps ...corev1.ConfigMap) { + for _, cm := range configMaps { + cm := cm + _, err := clientset.CoreV1().ConfigMaps("default").Create(context.Background(), &cm, metav1.CreateOptions{}) + if kerrors.IsAlreadyExists(err) { + _, err = clientset.CoreV1().ConfigMaps("default").Update(context.Background(), &cm, metav1.UpdateOptions{}) + require.NoError(t, err) + } + t.Cleanup(func() { + clientset.CoreV1().ConfigMaps("default").Delete(context.Background(), cm.Name, metav1.DeleteOptions{}) + }) + } +} From dec0920886911a2a5217f67a92fb017180f4c5fb Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 01:49:17 +0000 Subject: [PATCH 06/12] Fix /lock and /unlock error reporting --- slack.go | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/slack.go b/slack.go index 1f4b4a3..e9e879a 100644 --- a/slack.go +++ b/slack.go @@ -278,16 +278,12 @@ 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 { - if ok := s.validateProjectEnvUser(cmd.Project, cmd.Env, triggeredBy, replyIn); !ok { - return nil + 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 { - log.Println("[ERROR] ", err) - if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { - log.Println("[ERROR] ", err) - } - return nil + return s.errorMessage(err.Error()) } return s.infoMessage(fmt.Sprintf("Locked %s %s", cmd.Project, cmd.Env)) @@ -295,29 +291,25 @@ func (s *SlackListener) lock(cmd *slackcmd.Lock, triggeredBy string, replyIn str // 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 { - if ok := s.validateProjectEnvUser(cmd.Project, cmd.Env, triggeredBy, replyIn); !ok { - return nil + 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 { - log.Println("[ERROR] ", err) - if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { - log.Println("[ERROR] ", err) - } - return 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) bool { +func (s *SlackListener) validateProjectEnvUser(projectID, env, userID, replyIn string) error { pj, err := s.projectList.FindByAlias(projectID) if err != nil { log.Println("[ERROR] ", err) if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { log.Println("[ERROR] ", err) } - return false + return fmt.Errorf("find by alias %q: %w", projectID, err) } if phase := pj.FindPhase(env); phase.None() { @@ -326,7 +318,7 @@ func (s *SlackListener) validateProjectEnvUser(projectID, env, userID, replyIn s if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { log.Println("[ERROR] ", err) } - return false + return fmt.Errorf("find phase %q: %w", env, err) } if user := s.userList.FindBySlackUserID(userID); !user.IsDeveloper() { @@ -335,10 +327,10 @@ func (s *SlackListener) validateProjectEnvUser(projectID, env, userID, replyIn s if _, _, err := s.client.PostMessage(replyIn, s.errorMessage(err.Error())); err != nil { log.Println("[ERROR] ", err) } - return false + return fmt.Errorf("find by slack user id %q: %w", userID, err) } - return true + return nil } func (s *SlackListener) getOrCreateCoordinator() *deploy.Coordinator { From 7628dfe254980fd0f8efef7f8d456ee25b1b3244 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 02:10:42 +0000 Subject: [PATCH 07/12] ci: Create kind cluster before running tests --- .github/workflows/test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bfd12f1..fe35d0a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,6 +15,8 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 0 + - name: Create kind cluster + uses: helm/kind-action@v1 - name: Set up Go uses: actions/setup-go@v4 with: From cf58e29a9de1c007bbc13ef2ac37b4a00fd411e3 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 02:14:00 +0000 Subject: [PATCH 08/12] We don't need a slack token in test because it's a fake Slack API --- slack_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/slack_test.go b/slack_test.go index 869af01..b16601b 100644 --- a/slack_test.go +++ b/slack_test.go @@ -83,9 +83,6 @@ func TestSlackLockUnlock(t *testing.T) { setupConfigMaps(t, clientset, myprojectConfigMap, githubuserMappingConfigMap, rolebindingConfigMap) setupNamespace(t, clientset, "gocat") - var token = os.Getenv("SLACK_TOKEN") - require.NotEmpty(t, token, "SLACK_TOKEN must be set") - messages := make(chan message, 10) nextMessage := func() message { select { @@ -146,7 +143,7 @@ func TestSlackLockUnlock(t *testing.T) { })) defer ghts.Close() - s := slack.New(token, + s := slack.New("no-need-to-use-a-token-because-we-are-using-a-fake-server", slack.OptionAPIURL(ts.GetAPIURL()), ) From 424a0e2f552d8dbf34798c7a73dc98d15ded7047 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 02:42:54 +0000 Subject: [PATCH 09/12] Validate that we don't need various gocat envvars for slack_test --- slack_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/slack_test.go b/slack_test.go index b16601b..5b81438 100644 --- a/slack_test.go +++ b/slack_test.go @@ -147,8 +147,16 @@ func TestSlackLockUnlock(t *testing.T) { slack.OptionAPIURL(ts.GetAPIURL()), ) - config, err := InitConfig() - require.NoError(t, err) + // You usually do: + // config, err := InitConfig() + // But for this test, we'll just set the config manually. + config := &CatConfig{ + ManifestRepository: "no-need-to-use-a-token-because-this-test-does-not-use-the-manifest-repository", + ManifestRepositoryOrg: "no-need-to-use-a-token-because-this-test-does-not-use-the-manifest-repository", + GitHubUserName: "no-need-for-github-user-name-because-this-test-does-not-use-the-manifest-repository", + GitHubAccessToken: "no-need-for-github-access-token-because-this-test-does-not-use-the-manifest-repository", + GitHubDefaultBranch: "no-need-for-github-default-branch-because-this-test-does-not-use-the-manifest-repository", + } gh := CreateGitHubInstance( ghts.URL+"/graphql", From 178142b1e93563499fb298e4b6c1558e58166251 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 02:49:49 +0000 Subject: [PATCH 10/12] Address golangci-lint findings --- bot.go | 2 ++ deploy/lock_test.go | 4 +++- slack.go | 2 +- slack_test.go | 22 +++++++++++++++++----- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/bot.go b/bot.go index fe683bc..38f0338 100644 --- a/bot.go +++ b/bot.go @@ -5,6 +5,7 @@ import ( "log" "net/http" "os" + "sync" "github.com/slack-go/slack" ) @@ -44,6 +45,7 @@ func main() { projectList: &projectList, userList: &userList, interactorFactory: &interactorFactory, + mu: &sync.Mutex{}, }) http.Handle("/interaction", interactionHandler{ verificationToken: config.SlackVerificationToken, diff --git a/deploy/lock_test.go b/deploy/lock_test.go index 9e3c1f1..9b0c12f 100644 --- a/deploy/lock_test.go +++ b/deploy/lock_test.go @@ -21,7 +21,9 @@ func TestLockUnlock(t *testing.T) { c := NewCoordinator("default", "gocat-test") defer func() { if c, _ := c.ClientSet(); c != nil { - c.CoreV1().ConfigMaps("default").Delete(context.Background(), "gocat-test", metav1.DeleteOptions{}) + if err := c.CoreV1().ConfigMaps("default").Delete(context.Background(), "gocat-test", metav1.DeleteOptions{}); err != nil { + t.Log(err) + } } }() diff --git a/slack.go b/slack.go index e9e879a..55eae26 100644 --- a/slack.go +++ b/slack.go @@ -29,7 +29,7 @@ type SlackListener struct { interactorFactory *InteractorFactory coordinator *deploy.Coordinator - mu sync.Mutex + mu *sync.Mutex } func (s SlackListener) ServeHTTP(w http.ResponseWriter, r *http.Request) { diff --git a/slack_test.go b/slack_test.go index 5b81438..fbfa0d3 100644 --- a/slack_test.go +++ b/slack_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "os" + "sync" "testing" "time" @@ -95,11 +96,15 @@ func TestSlackLockUnlock(t *testing.T) { } ts := slacktest.NewTestServer(func(c slacktest.Customize) { c.Handle("/conversations.history", func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte(``)) + if _, err := w.Write([]byte(``)); err != nil { + t.Logf("failed to write response: %v", err) + } }) // List users c.Handle("/users.list", func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte(`{"ok": true, "members": [{"id": "U1234", "name": "User 1", "profile": {"display_name": "user1"}}]}`)) + if _, err := w.Write([]byte(`{"ok": true, "members": [{"id": "U1234", "name": "User 1", "profile": {"display_name": "user1"}}]}`)); err != nil { + t.Logf("failed to write response: %v", err) + } }) // Message posted to the channel c.Handle("/chat.postMessage", func(w http.ResponseWriter, r *http.Request) { @@ -114,7 +119,9 @@ func TestSlackLockUnlock(t *testing.T) { return } messages <- m - w.Write([]byte(`{"ok": true}`)) + if _, err := w.Write([]byte(`{"ok": true}`)); err != nil { + t.Logf("failed to write response: %v", err) + } }) }) ts.Start() @@ -191,6 +198,7 @@ func TestSlackLockUnlock(t *testing.T) { projectList: &projectList, userList: &userList, interactorFactory: &interactorFactory, + mu: &sync.Mutex{}, } require.NoError(t, l.handleMessageEvent(&slackevents.AppMentionEvent{ @@ -275,7 +283,9 @@ func setupNamespace(t *testing.T, clientset kubernetes.Interface, name string) { require.NoError(t, err) } t.Cleanup(func() { - clientset.CoreV1().Namespaces().Delete(context.Background(), ns.Name, metav1.DeleteOptions{}) + if err := clientset.CoreV1().Namespaces().Delete(context.Background(), ns.Name, metav1.DeleteOptions{}); err != nil { + t.Logf("failed to delete namespace %s: %v", ns.Name, err) + } }) } @@ -288,7 +298,9 @@ func setupConfigMaps(t *testing.T, clientset kubernetes.Interface, configMaps .. require.NoError(t, err) } t.Cleanup(func() { - clientset.CoreV1().ConfigMaps("default").Delete(context.Background(), cm.Name, metav1.DeleteOptions{}) + if err := clientset.CoreV1().ConfigMaps("default").Delete(context.Background(), cm.Name, metav1.DeleteOptions{}); err != nil { + t.Logf("failed to delete config map %s: %v", cm.Name, err) + } }) } } From 329ff2524938715a078fd30d0041ded8c44ce32d Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 02:53:50 +0000 Subject: [PATCH 11/12] Ensure NewProjectList uses kind in test --- slack_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/slack_test.go b/slack_test.go index fbfa0d3..78d68c7 100644 --- a/slack_test.go +++ b/slack_test.go @@ -181,6 +181,10 @@ func TestSlackLockUnlock(t *testing.T) { ) userList := UserList{github: gh, slackClient: s} + // Set LOCAL so that NewProjectList uses the local kubeconfig + local := os.Getenv("LOCAL") + os.Setenv("LOCAL", "true") + defer os.Setenv("LOCAL", local) projectList := NewProjectList() interactorContext := InteractorContext{ projectList: &projectList, From eb7f860acec29b07e3ee6545fa0f01800f556d15 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 9 Aug 2024 02:58:46 +0000 Subject: [PATCH 12/12] Address golangci-lint findings --- slack_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/slack_test.go b/slack_test.go index 78d68c7..5fce76c 100644 --- a/slack_test.go +++ b/slack_test.go @@ -143,7 +143,9 @@ func TestSlackLockUnlock(t *testing.T) { // Returns ids and logins of the users who are members of the organization - w.Write([]byte(`{"data": {"organization": {"membersWithRole": {"nodes": [{"id": "U1234", "login": "user1"}]}}}}`)) + if _, err := w.Write([]byte(`{"data": {"organization": {"membersWithRole": {"nodes": [{"id": "U1234", "login": "user1"}]}}}}`)); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } default: http.Error(w, "not found", http.StatusNotFound) }