From 38c66a60228a86816f2010021f15320ab9a936ac Mon Sep 17 00:00:00 2001 From: Pavel Ageev Date: Mon, 26 Nov 2018 22:04:58 +0300 Subject: [PATCH 1/2] Add correct validation fo duties list in config, fix panic "Out of range" on empty duties list in config, fix .gitignore file (to ignore compiled binary), update Makefile (remove useless checks), --- .dockerignore | 2 -- .gitignore | 2 +- Makefile | 20 +++-------------- ROADMAP.md | 2 +- main.go | 62 ++++++++++++++++++++++++++++++++++++++++++--------- 5 files changed, 56 insertions(+), 32 deletions(-) diff --git a/.dockerignore b/.dockerignore index f83b45c..de472a9 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,4 +1,2 @@ * - -!.docker/docker-entrypoint !slack-duty-bot diff --git a/.gitignore b/.gitignore index d71234d..40ff779 100644 --- a/.gitignore +++ b/.gitignore @@ -6,5 +6,5 @@ *.out config.yaml slack-duty-bot -!/slack-duty-bot +!./**/slack-duty-bot .kubernetes/deploy.yaml diff --git a/Makefile b/Makefile index eb326d8..9060e44 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ all: build -.PHONY : all package +.PHONY: all # prevent run if docker not found ifeq (, $(shell which docker)) @@ -18,17 +18,6 @@ ifeq ($(MOD_NAME),) override MOD_NAME=slack-duty-bot endif -# vendor variables -FORCE_INIT=0 -ifeq (,$(wildcard ./go.mod)) - FORCE_INIT=1 -endif - -FORCE_VENDOR=0 -ifeq (,$(wildcard ./vendor/.*)) - FORCE_VENDOR=1 -endif - # build go binary variables GO_VERSION=1.11 GOOS?=$(shell go env GOOS || echo linux) @@ -43,24 +32,21 @@ DOCKER_USER?= DOCKER_PASSWORD?= init: -ifeq ($(FORCE_INIT), 1) docker run --rm \ -v ${ROOT_DIR}:/project \ -w /project \ -e GO111MODULE=on \ golang:${GO_VERSION} \ - go mod init ${MOD_NAME} -endif + go mod init ${MOD_NAME} || true vendor: init -ifeq ($(FORCE_VENDOR), 1) + rm -r vendor || true docker run --rm \ -v ${ROOT_DIR}:/project \ -w /project \ -e GO111MODULE=on \ golang:${GO_VERSION} \ go mod vendor -endif test: vendor docker run --rm \ diff --git a/ROADMAP.md b/ROADMAP.md index 3a5f40b..254874c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -6,7 +6,7 @@ ROADMAP * HTTP rest API for edit duties configuration * Persistent storage for duties configuration * Multiple instance mode with ex-locks -* HELM support +* ~~HELM support~~ * Healtchecks * Kubernetes livenessprobe/readinessprobe * Support prometheus metrics diff --git a/main.go b/main.go index 3af6d87..9f964e6 100644 --- a/main.go +++ b/main.go @@ -135,9 +135,44 @@ func validateArguments() error { if len(viper.GetStringSlice("slack.keyword")) == 0 { return fmt.Errorf("parameter slack.keyword is required") } + var ( + duties = parseDutiesList() + index = getCurrentDayIndex() + cfgGroup = viper.GetString("slack.group.id") != "" && viper.GetString("slack.group.name") != "" + length = len(duties) + ) + switch length > 0 { + case true: + if length < 7 { + return fmt.Errorf("duties list is not empty, but indexes count %d is less then 7 (weekdays)", length) + } + if len(duties[index]) == 0 && !cfgGroup { + return fmt.Errorf("empty duties list for current day (%d) index and slack.group info is not exist in config", index) + } + break + case false: + if !cfgGroup { + return fmt.Errorf("empty duties list in config and slack.group info is not exist in config") + } + break + } return nil } +func parseDutiesList() [][]string { + var ( + config = struct { + Duties [][]string // we need this hack cause viper cannot resolve slice of slice + }{} + ) + viper.Unmarshal(&config) + return config.Duties +} + +func getCurrentDayIndex() int { + return int(time.Now().Weekday()) +} + func handleMessageEvent(rtm *slack.RTM, event *slack.MessageEvent) error { if err := checkMessageEvent(event); err != nil { logrus.Debugf("Incoming message check error: %+v", err) @@ -158,23 +193,27 @@ func handleMessageEvent(rtm *slack.RTM, event *slack.MessageEvent) error { } } var ( - config = struct { - Duties [][]string // we need this hack cause viper cannot resolve slice of slice - }{} duties []string + cfg = parseDutiesList() + index = getCurrentDayIndex() ) - viper.Unmarshal(&config) - for _, username := range config.Duties[int(time.Now().Weekday())] { - userId, ok := userIds[username] - if !ok { - logrus.Errorf("Failed to get user id by username %s", username) + if len(cfg) > index { + for _, username := range cfg[index] { + userId, ok := userIds[username] + if !ok { + logrus.Errorf("Failed to get user id by username %s", username) + } + duties = append(duties, fmt.Sprintf("<@%s|%s>", userId, username)) } - duties = append(duties, fmt.Sprintf("<@%s|%s>", userId, username)) } if len(duties) == 0 && viper.GetString("slack.group.id") != "" && viper.GetString("slack.group.name") != "" { duties = append(duties, fmt.Sprintf("", viper.GetString("slack.group.id"), viper.GetString("slack.group.name"))) } - // send message + if len(duties) == 0 { + return fmt.Errorf("failed to collect duties list for incomming message") + } + logrus.Debugf("Final duties list for call: %+v", duties) + //send message var outgoing = rtm.NewOutgoingMessage(strings.Join(duties, ", "), event.Channel) if viper.GetBool("slack.threads") == true { outgoing.ThreadTimestamp = event.Timestamp @@ -185,8 +224,9 @@ func handleMessageEvent(rtm *slack.RTM, event *slack.MessageEvent) error { } func checkMessageEvent(event *slack.MessageEvent) error { + // skip topic messages if event.Topic != "" { - return fmt.Errorf("inocming message about topic change") + return fmt.Errorf("incoming message about topic change") } // check text if event.Text == "" { From 69db9bd15f041455e66be3bed235709535ef938f Mon Sep 17 00:00:00 2001 From: Pavel Ageev Date: Tue, 27 Nov 2018 21:33:09 +0300 Subject: [PATCH 2/2] Spell check fix (core review PR) --- main.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/main.go b/main.go index 9f964e6..f5a7353 100644 --- a/main.go +++ b/main.go @@ -144,15 +144,15 @@ func validateArguments() error { switch length > 0 { case true: if length < 7 { - return fmt.Errorf("duties list is not empty, but indexes count %d is less then 7 (weekdays)", length) + return fmt.Errorf("duties list is not empty, but indexes count %d is less than 7 (weekdays)", length) } if len(duties[index]) == 0 && !cfgGroup { - return fmt.Errorf("empty duties list for current day (%d) index and slack.group info is not exist in config", index) + return fmt.Errorf("empty duties list for current day (%d) index and slack.group info does not exist in config", index) } break case false: if !cfgGroup { - return fmt.Errorf("empty duties list in config and slack.group info is not exist in config") + return fmt.Errorf("empty duties list in config and slack.group info does not exist in config") } break } @@ -210,7 +210,7 @@ func handleMessageEvent(rtm *slack.RTM, event *slack.MessageEvent) error { duties = append(duties, fmt.Sprintf("", viper.GetString("slack.group.id"), viper.GetString("slack.group.name"))) } if len(duties) == 0 { - return fmt.Errorf("failed to collect duties list for incomming message") + return fmt.Errorf("failed to collect duties list for incoming message") } logrus.Debugf("Final duties list for call: %+v", duties) //send message @@ -226,7 +226,7 @@ func handleMessageEvent(rtm *slack.RTM, event *slack.MessageEvent) error { func checkMessageEvent(event *slack.MessageEvent) error { // skip topic messages if event.Topic != "" { - return fmt.Errorf("incoming message about topic change") + return fmt.Errorf("the incoming message about topic change") } // check text if event.Text == "" {