From 8314d7344f873138a3cad2a2a826b4faa9f6ded9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kosiec?= Date: Mon, 6 Jun 2022 13:37:19 +0200 Subject: [PATCH] Remove global variables, pass context across all components and improve error handling #603 ##### ISSUE TYPE - Feature Pull Request ##### SUMMARY - Use envconfig - Remove global variables related to Kubernetes, logger, and filters - Pass context across all components - support graceful shutdown of all BotKube components - Improve error handling (return errors in almost all places instead of ignoring/logging them) This is just a start, as later (during new features development) we should refactor each component/package in isolation. But after this huge PR it should be possible to do it. Fixes #220 ##### TODO After first review of this PR if the new filter engine approach is accepted: - [x] Update documentation (https://www.botkube.io/filters/) PR: https://github.com/infracloudio/botkube-docs/pull/81 ##### TESTING Tested manually with Mattermost, Discord and Slack. You can use the following image:`pkosiec/botkube:remove-global-vars-v2` --- CONTRIBUTING.md | 12 +- cmd/botkube/main.go | 187 ++++++-- go.mod | 8 +- go.sum | 21 +- helm/botkube/templates/serviceaccount.yaml | 1 - helm/botkube/values.yaml | 4 - pkg/bot/bot.go | 14 +- pkg/bot/discord.go | 60 ++- pkg/bot/lark.go | 61 ++- pkg/bot/mattermost.go | 112 +++-- pkg/bot/slack.go | 154 +++--- pkg/bot/teams.go | 109 +++-- pkg/config/config.go | 9 +- pkg/controller/config_watcher.go | 90 ++++ pkg/controller/controller.go | 442 ++++++++++++------ pkg/controller/controller_test.go | 201 ++++++++ pkg/controller/message.go | 24 + pkg/controller/upgrade.go | 105 +++-- pkg/events/events.go | 27 +- pkg/execute/executor.go | 176 +++---- pkg/execute/factory.go | 54 +++ pkg/execute/resource.go | 61 +++ pkg/filterengine/filterengine.go | 64 +-- pkg/filterengine/filters/image_tag_checker.go | 37 +- pkg/filterengine/filters/ingress_validator.go | 39 +- pkg/filterengine/filters/namespace_checker.go | 45 +- .../filters/node_event_checker.go | 32 +- .../filters/object_annotation_checker.go | 51 +- .../filters/object_annotation_checker_test.go | 11 +- pkg/filterengine/filters/pod_label_checker.go | 36 +- pkg/filterengine/filters/utils.go | 23 +- pkg/filterengine/global.go | 25 - pkg/filterengine/with_all_filters.go | 30 ++ pkg/httpsrv/server.go | 41 ++ pkg/kube/client.go | 35 ++ pkg/log/log.go | 121 ----- pkg/metrics/metrics.go | 33 -- pkg/notify/discord.go | 34 +- pkg/notify/elasticsearch.go | 37 +- pkg/notify/lark.go | 28 +- pkg/notify/mattermost.go | 26 +- pkg/notify/notify.go | 48 +- pkg/notify/slack.go | 26 +- pkg/notify/webhook.go | 28 +- pkg/notify/webhook_test.go | 3 +- pkg/utils/diff.go | 28 +- pkg/utils/diff_test.go | 55 ++- pkg/utils/jsonpath.go | 2 +- pkg/utils/lark.go | 21 +- pkg/utils/utils.go | 279 ++--------- test/e2e/command/botkube.go | 29 +- test/e2e/e2e_test.go | 142 ++++-- test/e2e/env/env.go | 71 +-- test/e2e/filters/filters.go | 2 +- test/e2e/notifier/create/create.go | 4 +- test/e2e/notifier/delete/delete.go | 67 +-- test/e2e/notifier/error/error.go | 84 ---- test/e2e/notifier/update/update.go | 101 ++-- test/e2e/utils/utils.go | 75 ++- test/webhook/server.go | 2 - 60 files changed, 2131 insertions(+), 1616 deletions(-) create mode 100644 pkg/controller/config_watcher.go create mode 100644 pkg/controller/controller_test.go create mode 100644 pkg/controller/message.go create mode 100644 pkg/execute/factory.go create mode 100644 pkg/execute/resource.go delete mode 100644 pkg/filterengine/global.go create mode 100644 pkg/filterengine/with_all_filters.go create mode 100644 pkg/httpsrv/server.go create mode 100644 pkg/kube/client.go delete mode 100644 pkg/log/log.go delete mode 100644 pkg/metrics/metrics.go delete mode 100644 test/e2e/notifier/error/error.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 99fe9bff4..e10027844 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -92,7 +92,13 @@ For faster development, you can also build and run BotKube outside K8s cluster. # From project root directory $ export CONFIG_PATH=$(pwd) ``` -4. Make sure that correct context is set and you are able to access your Kubernetes cluster +4. Export the path to Kubeconfig: + + ```sh + export KUBECONFIG=/Users/$USER/.kube/config # set custom path if necessary + ``` + +5. Make sure that correct context is set and you are able to access your Kubernetes cluster ```console $ kubectl config current-context minikube @@ -100,8 +106,8 @@ For faster development, you can also build and run BotKube outside K8s cluster. Kubernetes master is running at https://192.168.39.233:8443 CoreDNS is running at https://192.168.39.233:8443/api/v1/namespaces/kube-system/services/kube-dns:dns/proxy ... - ``` -5. Run BotKube binary + ``` +6. Run BotKube binary ```sh $ ./botkube ``` diff --git a/cmd/botkube/main.go b/cmd/botkube/main.go index 1f1e45cae..24b6bead3 100644 --- a/cmd/botkube/main.go +++ b/cmd/botkube/main.go @@ -20,113 +20,206 @@ package main import ( + "context" + "fmt" + "log" + "net/http" "os" + "time" + "github.com/google/go-github/v44/github" + "github.com/prometheus/client_golang/prometheus/promhttp" + "github.com/sirupsen/logrus" + "github.com/vrischmann/envconfig" "golang.org/x/sync/errgroup" + "sigs.k8s.io/controller-runtime/pkg/manager/signals" "github.com/infracloudio/botkube/pkg/bot" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/controller" + "github.com/infracloudio/botkube/pkg/execute" "github.com/infracloudio/botkube/pkg/filterengine" - "github.com/infracloudio/botkube/pkg/log" - "github.com/infracloudio/botkube/pkg/metrics" + "github.com/infracloudio/botkube/pkg/httpsrv" + "github.com/infracloudio/botkube/pkg/kube" "github.com/infracloudio/botkube/pkg/notify" - "github.com/infracloudio/botkube/pkg/utils" ) +// Config contains the app configuration parameters. +type Config struct { + MetricsPort string `envconfig:"default=2112"` + LogLevel string `envconfig:"default=error"` + ConfigPath string `envconfig:"optional"` + InformersResyncPeriod time.Duration `envconfig:"default=30m"` + KubeconfigPath string `envconfig:"optional,KUBECONFIG"` +} + const ( - defaultMetricsPort = "2112" + componentLogFieldKey = "component" + botLogFieldKey = "bot" ) -// TODO: -// - Use context to make sure all goroutines shutdowns gracefully: https://github.com/infracloudio/botkube/issues/220 -// - Make the code testable (shorten methods and functions, and reduce level of cyclomatic complexity): https://github.com/infracloudio/botkube/issues/589 - func main() { - // Prometheus metrics - metricsPort, exists := os.LookupEnv("METRICS_PORT") - if !exists { - metricsPort = defaultMetricsPort - } + var appCfg Config + err := envconfig.Init(&appCfg) + exitOnError(err, "while loading app configuration") - // Set up global logger and filter engine - log.SetupGlobal() - filterengine.SetupGlobal() + logger := newLogger(appCfg.LogLevel) + ctx := signals.SetupSignalHandler() + ctx, cancelCtxFn := context.WithCancel(ctx) + defer cancelCtxFn() - errGroup := new(errgroup.Group) + errGroup, ctx := errgroup.WithContext(ctx) + // Prometheus metrics + metricsSrv := newMetricsServer(logger.WithField(componentLogFieldKey, "Metrics server"), appCfg.MetricsPort) errGroup.Go(func() error { - return metrics.ServeMetrics(metricsPort) + return metricsSrv.Serve(ctx) }) - log.Info("Starting controller") - - conf, err := config.New() + conf, err := config.Load(appCfg.ConfigPath) exitOnError(err, "while loading configuration") + if conf == nil { + log.Fatal("while loading configuration: config cannot be nil") + } - // List notifiers - notifiers := notify.ListNotifiers(conf.Communications) + // Prepare K8s clients and mapper + dynamicCli, discoveryCli, mapper, err := kube.SetupK8sClients(appCfg.KubeconfigPath) + exitOnError(err, "while initializing K8s clients") + // Set up the filter engine + filterEngine := filterengine.WithAllFilters(logger, dynamicCli, mapper, conf) + + // List notifiers + notifiers, err := notify.LoadNotifiers(logger, conf.Communications) + exitOnError(err, "while loading notifiers") + + // Create Executor Factory + resMapping, err := execute.LoadResourceMappingIfShould( + logger.WithField(componentLogFieldKey, "Resource Mapping Loader"), + conf, + discoveryCli, + ) + exitOnError(err, "while loading resource mapping") + + executorFactory := execute.NewExecutorFactory( + logger.WithField(componentLogFieldKey, "Executor"), + execute.DefaultCommandRunnerFunc, + *conf, + filterEngine, + resMapping, + ) + + // Run bots if conf.Communications.Slack.Enabled { - log.Info("Starting slack bot") - sb := bot.NewSlackBot(conf) + sb := bot.NewSlackBot(logger.WithField(botLogFieldKey, "Slack"), conf, executorFactory) errGroup.Go(func() error { - return sb.Start() + return sb.Start(ctx) }) } if conf.Communications.Mattermost.Enabled { - log.Info("Starting mattermost bot") - mb := bot.NewMattermostBot(conf) + mb := bot.NewMattermostBot(logger.WithField(botLogFieldKey, "Mattermost"), conf, executorFactory) errGroup.Go(func() error { - return mb.Start() + return mb.Start(ctx) }) } if conf.Communications.Teams.Enabled { - log.Info("Starting MS Teams bot") - tb := bot.NewTeamsBot(conf) + tb := bot.NewTeamsBot(logger.WithField(botLogFieldKey, "MS Teams"), conf, executorFactory) + // TODO: Unify that with other notifiers: Split this into two structs or merge other bots and notifiers into single structs notifiers = append(notifiers, tb) errGroup.Go(func() error { - return tb.Start() + return tb.Start(ctx) }) } if conf.Communications.Discord.Enabled { - log.Info("Starting discord bot") - db := bot.NewDiscordBot(conf) + db := bot.NewDiscordBot(logger.WithField(botLogFieldKey, "Discord"), conf, executorFactory) errGroup.Go(func() error { - return db.Start() + return db.Start(ctx) }) } if conf.Communications.Lark.Enabled { - log.Info("Starting lark bot") - lb := bot.NewLarkBot(conf) + lb := bot.NewLarkBot(logger.WithField(botLogFieldKey, "Lark"), logger.GetLevel(), conf, executorFactory) errGroup.Go(func() error { - return lb.Start() + return lb.Start(ctx) }) } - // Start upgrade notifier + // Start upgrade checker + ghCli := github.NewClient(&http.Client{ + Timeout: 1 * time.Minute, + }) if conf.Settings.UpgradeNotifier { - log.Info("Starting upgrade notifier") + upgradeChecker := controller.NewUpgradeChecker( + logger.WithField(componentLogFieldKey, "Upgrade Checker"), + notifiers, + ghCli.Repositories, + ) errGroup.Go(func() error { - controller.UpgradeNotifier(notifiers) - return nil + return upgradeChecker.Run(ctx) }) } - // Init KubeClient, InformerMap and start controller - utils.InitKubeClient() - utils.InitInformerMap(conf) - utils.InitResourceMap(conf) - controller.RegisterInformers(conf, notifiers) + // Start Config Watcher + if conf.Settings.ConfigWatcher { + cfgWatcher := controller.NewConfigWatcher( + logger.WithField(componentLogFieldKey, "Config Watcher"), + appCfg.ConfigPath, + conf.Settings.ClusterName, + notifiers, + ) + errGroup.Go(func() error { + return cfgWatcher.Do(ctx, cancelCtxFn) + }) + } + + // Start controller + + ctrl := controller.New( + logger.WithField(componentLogFieldKey, "Controller"), + conf, + notifiers, + filterEngine, + appCfg.ConfigPath, + dynamicCli, + mapper, + appCfg.InformersResyncPeriod, + ) + + err = ctrl.Start(ctx) + exitOnError(err, "while starting controller") err = errGroup.Wait() exitOnError(err, "while waiting for goroutines to finish gracefully") } +func newLogger(logLevelStr string) *logrus.Logger { + logger := logrus.New() + // Output to stdout instead of the default stderr + logger.SetOutput(os.Stdout) + + // Only logger the warning severity or above. + logLevel, err := logrus.ParseLevel(logLevelStr) + if err != nil { + // Set Info level as a default + logLevel = logrus.InfoLevel + } + logger.SetLevel(logLevel) + logger.Formatter = &logrus.TextFormatter{ForceColors: true, FullTimestamp: true} + + return logger +} + +func newMetricsServer(log logrus.FieldLogger, metricsPort string) *httpsrv.Server { + addr := fmt.Sprintf(":%s", metricsPort) + mux := http.NewServeMux() + mux.Handle("/metrics", promhttp.Handler()) + + return httpsrv.New(log, addr, mux) +} + func exitOnError(err error, context string) { if err != nil { log.Fatalf("%s: %v", context, err) diff --git a/go.mod b/go.mod index 45a883cee..102d520d4 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ require ( github.com/bwmarrin/discordgo v0.25.0 github.com/fsnotify/fsnotify v1.5.4 github.com/google/go-github/v44 v44.1.0 + github.com/hashicorp/go-multierror v1.1.1 github.com/infracloudio/msbotbuilder-go v0.2.5 github.com/larksuite/oapi-sdk-go v1.1.44 github.com/mattermost/mattermost-server/v5 v5.39.3 @@ -14,6 +15,7 @@ require ( github.com/sirupsen/logrus v1.8.1 github.com/slack-go/slack v0.10.3 github.com/stretchr/testify v1.7.1 + github.com/vrischmann/envconfig v1.3.0 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b k8s.io/api v0.24.0 @@ -21,6 +23,7 @@ require ( k8s.io/client-go v0.24.0 k8s.io/kubectl v0.24.0 k8s.io/sample-controller v0.24.0 + sigs.k8s.io/controller-runtime v0.12.1 ) require ( @@ -61,7 +64,6 @@ require ( github.com/gorilla/websocket v1.5.0 // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect - github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/imdario/mergo v0.3.12 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect @@ -112,7 +114,7 @@ require ( go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect go.uber.org/atomic v1.8.0 // indirect go.uber.org/multierr v1.7.0 // indirect - go.uber.org/zap v1.19.0 // indirect + go.uber.org/zap v1.19.1 // indirect golang.org/x/crypto v0.0.0-20220411220226-7b82a4e95df4 // indirect golang.org/x/net v0.0.0-20220403103023-749bd193bc2b // indirect golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b // indirect @@ -135,7 +137,7 @@ require ( sigs.k8s.io/kustomize/api v0.11.4 // indirect sigs.k8s.io/kustomize/kyaml v0.13.6 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect - sigs.k8s.io/yaml v1.2.0 // indirect + sigs.k8s.io/yaml v1.3.0 // indirect ) go 1.18 diff --git a/go.sum b/go.sum index 3a18ce281..785e4d6f0 100644 --- a/go.sum +++ b/go.sum @@ -821,8 +821,8 @@ github.com/ngdinhtoan/glide-cleanup v0.2.0/go.mod h1:UQzsmiDOb8YV3nOsCxK/c9zPpCZ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/nwaples/rardecode v1.1.0/go.mod h1:5DzqNKiOdpKKBH87u8VlvAnPZMXcGRhxWkRpHbbfGS0= -github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= +github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA= github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= @@ -841,16 +841,16 @@ github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108 github.com/onsi/ginkgo v1.13.0/go.mod h1:+REjRxOmWfHCjfv9TTWB1jD1Frx4XydAD3zm1lskyM0= github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY= github.com/onsi/ginkgo v1.14.1/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY= -github.com/onsi/ginkgo v1.15.0 h1:1V1NfVQR87RtWAgp1lv9JZJ5Jap+XFGKPi00andXGi4= github.com/onsi/ginkgo v1.15.0/go.mod h1:hF8qUzuuC8DJGygJH3726JnCZX4MYbRB8yFfISqnKUg= +github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.9.0/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.10.2/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= -github.com/onsi/gomega v1.10.5 h1:7n6FEkpFmfCoo2t+YYqXH0evK+a9ICQz0xcAy9dYcaQ= github.com/onsi/gomega v1.10.5/go.mod h1:gza4q3jKQJijlu05nKWRCW/GavJumGt8aNRxWg7mt48= +github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= github.com/oov/psd v0.0.0-20210618170533-9fb823ddb631/go.mod h1:GHI1bnmAcbp96z6LNfBJvtrjxhaXGkbsk967utPlvL8= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= @@ -1092,6 +1092,8 @@ github.com/viant/assertly v0.4.8/go.mod h1:aGifi++jvCrUaklKEKT0BU95igDNaqkvz+49u github.com/viant/toolbox v0.24.0/go.mod h1:OxMCG57V0PXuIP2HNQrtJf2CjqdmbrOx5EkMILuUhzM= github.com/vmihailenco/msgpack/v5 v5.3.4/go.mod h1:7xyJ9e+0+9SaZT0Wt1RGleJXzli6Q/V5KbhBonMG9jc= github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds= +github.com/vrischmann/envconfig v1.3.0 h1:4XIvQTXznxmWMnjouj0ST5lFo/WAYf5Exgl3x82crEk= +github.com/vrischmann/envconfig v1.3.0/go.mod h1:bbvxFYJdRSpXrhS63mBFtKJzkDiNkyArOLXtY6q0kuI= github.com/wiggin77/cfg v1.0.2 h1:NBUX+iJRr+RTncTqTNvajHwzduqbhCQjEqxLHr6Fk7A= github.com/wiggin77/cfg v1.0.2/go.mod h1:b3gotba2e5bXTqTW48DwIFoLc+4lWKP7WPi/CdvZ4aE= github.com/wiggin77/merror v1.0.2/go.mod h1:uQTcIU0Z6jRK4OwqganPYerzQxSFJ4GSHM3aurxxQpg= @@ -1169,8 +1171,9 @@ go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.8.0 h1:CUhrE4N1rqSE6FM9ecihEjRkLQu8cDfgDyoOs83mEY4= go.uber.org/atomic v1.8.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/goleak v1.1.10 h1:z+mqJhf6ss6BSfSM671tgKyZBFPTTJM+HLxnhPC3wu0= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= +go.uber.org/goleak v1.1.11-0.20210813005559-691160354723/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= +go.uber.org/goleak v1.1.12 h1:gZAh5/EyT/HQwlpkCy6wTpqfH9H8Lz8zbm3dZh+OyzA= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.7.0 h1:zaiO/rmgFjbmCXdSYJWQcdvOCsthmdaHfr3Gm2Kx4Ec= @@ -1178,8 +1181,9 @@ go.uber.org/multierr v1.7.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95a go.uber.org/zap v1.9.1/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.17.0/go.mod h1:MXVU+bhUf/A7Xi2HNOnopQOrmycQ5Ih87HtOu4q5SSo= -go.uber.org/zap v1.19.0 h1:mZQZefskPPCMIBCSEH0v2/iUqqLrYtaeqwD6FUGUnFE= go.uber.org/zap v1.19.0/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= +go.uber.org/zap v1.19.1 h1:ue41HOKd1vGURxrmeKIgELGb3jPW9DMUDGtsinblHwI= +go.uber.org/zap v1.19.1/go.mod h1:j3DNczoxDZroyBnOT1L/Q79cfUMGZxlv/9dzN7SM1rI= go4.org v0.0.0-20180809161055-417644f6feb5/go.mod h1:MkTOUMDaeVYJUOUsaDXIhWPZYa1yOyC1qaOBpL57BhE= golang.org/x/build v0.0.0-20190111050920-041ab4dc3f9d/go.mod h1:OWs+y06UdEOHN4y+MfF/py+xQ/tYqIWW03b70/CG9Rw= golang.org/x/build v0.0.0-20190314133821-5284462c4bec/go.mod h1:atTaCNAy0f16Ah5aV1gMSwgiKVHwu/JncqDpuRr7lS4= @@ -1240,7 +1244,6 @@ golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f/go.mod h1:5qLYkcX4OjUUV8bRu golang.org/x/lint v0.0.0-20200130185559-910be7a94367/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= -golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug= golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE= golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028/go.mod h1:E/iHnbuqvinMTCcRqshq8CkpyQDoeVncDDYHnLhea+o= @@ -1539,7 +1542,6 @@ golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.1.10-0.20220218145154-897bd77cd717 h1:hI3jKY4Hpf63ns040onEbB3dAkR/H/P83hw1TG8dD3Y= golang.org/x/tools v0.1.10-0.20220218145154-897bd77cd717/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -1787,6 +1789,8 @@ modernc.org/zappy v1.0.0/go.mod h1:hHe+oGahLVII/aTTyWK/b53VDHMAGCBYYeZ9sn83HC4= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= +sigs.k8s.io/controller-runtime v0.12.1 h1:4BJY01xe9zKQti8oRjj/NeHKRXthf1YkYJAgLONFFoI= +sigs.k8s.io/controller-runtime v0.12.1/go.mod h1:BKhxlA4l7FPK4AQcsuL4X6vZeWnKDXez/vp1Y8dxTU0= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 h1:kDi4JBNAsJWfz1aEXhO8Jg87JJaPNLh5tIzYHgStQ9Y= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2/go.mod h1:B+TnT182UBxE84DiCz4CVE26eOSDAeYCpfDnC2kdKMY= sigs.k8s.io/kustomize/api v0.11.4 h1:/0Mr3kfBBNcNPOW5Qwk/3eb8zkswCwnqQxxKtmrTkRo= @@ -1798,8 +1802,9 @@ sigs.k8s.io/kustomize/kyaml v0.13.6/go.mod h1:yHP031rn1QX1lr/Xd934Ri/xdVNG8BE2EC sigs.k8s.io/structured-merge-diff/v4 v4.0.2/go.mod h1:bJZC9H9iH24zzfZ/41RGcq60oK1F7G282QMXDPYydCw= sigs.k8s.io/structured-merge-diff/v4 v4.2.1 h1:bKCqE9GvQ5tiVHn5rfn1r+yao3aLQEaLzkkmAkf+A6Y= sigs.k8s.io/structured-merge-diff/v4 v4.2.1/go.mod h1:j/nl6xW8vLS49O8YvXW1ocPhZawJtm+Yrr7PPRQ0Vg4= -sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q= sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc= +sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo= +sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= sourcegraph.com/sourcegraph/go-diff v0.5.0/go.mod h1:kuch7UrkMzY0X+p9CRK03kfuPQ2zzQcaEFbx8wA8rck= sourcegraph.com/sqs/pbtypes v0.0.0-20180604144634-d3ebe8f20ae4/go.mod h1:ketZ/q3QxT9HOBeFhu6RdvsftgpsbFHBF5Cas6cDKZ0= willnorris.com/go/gifresize v1.0.0/go.mod h1:eBM8gogBGCcaH603vxSpnfjwXIpq6nmnj/jauBDKtAk= diff --git a/helm/botkube/templates/serviceaccount.yaml b/helm/botkube/templates/serviceaccount.yaml index b816ca032..cd460669f 100644 --- a/helm/botkube/templates/serviceaccount.yaml +++ b/helm/botkube/templates/serviceaccount.yaml @@ -12,5 +12,4 @@ metadata: helm.sh/chart: {{ include "botkube.chart" . }} app.kubernetes.io/instance: {{ .Release.Name }} app.kubernetes.io/managed-by: {{ .Release.Service }} -automountServiceAccountToken: {{ .Values.automountServiceAccountToken }} {{- end -}} diff --git a/helm/botkube/values.yaml b/helm/botkube/values.yaml index 930161e5e..9163e08e9 100644 --- a/helm/botkube/values.yaml +++ b/helm/botkube/values.yaml @@ -35,7 +35,6 @@ containerSecurityContext: kubeconfig: # If true, enables overriding the kubernetes auth. - # NOTE: Remember to set automountServiceAccountToken to false. enabled: false # A base64 encoded kubeconfig that will be stored in a secret, mounted to the pod, and specified in the KUBECONFIG environment variable. base64Config: "" @@ -45,9 +44,6 @@ kubeconfig: # config: {base64_encoded_kubeconfig} existingSecret: "" -# Use this to override the default behavior of the service account. It MUST be disabled when using kubeconfig.enabled. -automountServiceAccountToken: true - # set one of the log levels- info, warn, debug, error, fatal, panic logLevel: info diff --git a/pkg/bot/bot.go b/pkg/bot/bot.go index 15806e3a6..a8d127c43 100644 --- a/pkg/bot/bot.go +++ b/pkg/bot/bot.go @@ -19,7 +19,19 @@ package bot +import ( + "context" + + "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/execute" +) + // Bot connects to communication channels and reads/sends messages type Bot interface { - Start() error + Start(ctx context.Context) error +} + +// ExecutorFactory facilitates creation of execute.Executor instances. +type ExecutorFactory interface { + NewDefault(platform config.BotPlatform, isAuthChannel bool, message string) execute.Executor } diff --git a/pkg/bot/discord.go b/pkg/bot/discord.go index e057ff028..8dc896eec 100644 --- a/pkg/bot/discord.go +++ b/pkg/bot/discord.go @@ -20,18 +20,20 @@ package bot import ( + "context" "fmt" "strings" "github.com/bwmarrin/discordgo" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" - "github.com/infracloudio/botkube/pkg/execute" - "github.com/infracloudio/botkube/pkg/log" ) // DiscordBot listens for user's message, execute commands and sends back the response type DiscordBot struct { + log logrus.FieldLogger + executorFactory ExecutorFactory Token string AllowKubectl bool RestrictAccess bool @@ -43,17 +45,21 @@ type DiscordBot struct { // discordMessage contains message details to execute command and send back the result type discordMessage struct { - Event *discordgo.MessageCreate - BotID string - Request string - Response string - IsAuthChannel bool - Session *discordgo.Session + log logrus.FieldLogger + executorFactory ExecutorFactory + Event *discordgo.MessageCreate + BotID string + Request string + Response string + IsAuthChannel bool + Session *discordgo.Session } // NewDiscordBot returns new Bot object -func NewDiscordBot(c *config.Config) Bot { +func NewDiscordBot(log logrus.FieldLogger, c *config.Config, executorFactory ExecutorFactory) *DiscordBot { return &DiscordBot{ + log: log, + executorFactory: executorFactory, Token: c.Communications.Discord.Token, BotID: c.Communications.Discord.BotID, AllowKubectl: c.Settings.Kubectl.Enabled, @@ -65,7 +71,8 @@ func NewDiscordBot(c *config.Config) Bot { } // Start starts the DiscordBot websocket connection and listens for messages -func (b *DiscordBot) Start() error { +func (b *DiscordBot) Start(ctx context.Context) error { + b.log.Info("Starting bot") api, err := discordgo.New("Bot " + b.Token) if err != nil { return fmt.Errorf("while creating Discord session: %w", err) @@ -74,9 +81,11 @@ func (b *DiscordBot) Start() error { // Register the messageCreate func as a callback for MessageCreate events. api.AddHandler(func(s *discordgo.Session, m *discordgo.MessageCreate) { dm := discordMessage{ - Event: m, - BotID: b.BotID, - Session: s, + log: b.log, + executorFactory: b.executorFactory, + Event: m, + BotID: b.BotID, + Session: s, } dm.HandleMessage(b) @@ -88,10 +97,20 @@ func (b *DiscordBot) Start() error { return fmt.Errorf("while opening connection: %w", err) } - log.Info("BotKube connected to Discord!") + b.log.Info("BotKube connected to Discord!") + + <-ctx.Done() + b.log.Info("Shutdown requested. Finishing...") + err = api.Close() + if err != nil { + return fmt.Errorf("while closing connection: %w", err) + } + return nil } +// TODO: refactor - handle and send methods should be defined on Bot level + // HandleMessage handles the incoming messages func (dm *discordMessage) HandleMessage(b *DiscordBot) { // Serve only if starts with mention @@ -115,19 +134,18 @@ func (dm *discordMessage) HandleMessage(b *DiscordBot) { return } - e := execute.NewDefaultExecutor(dm.Request, b.AllowKubectl, b.RestrictAccess, b.DefaultNamespace, - b.ClusterName, config.DiscordBot, b.ChannelID, dm.IsAuthChannel) + e := dm.executorFactory.NewDefault(config.DiscordBot, dm.IsAuthChannel, dm.Request) dm.Response = e.Execute() dm.Send() } func (dm discordMessage) Send() { - log.Debugf("Discord incoming Request: %s", dm.Request) - log.Debugf("Discord Response: %s", dm.Response) + dm.log.Debugf("Discord incoming Request: %s", dm.Request) + dm.log.Debugf("Discord Response: %s", dm.Response) if len(dm.Response) == 0 { - log.Errorf("Invalid request. Dumping the response. Request: %s", dm.Request) + dm.log.Errorf("Invalid request. Dumping the response. Request: %s", dm.Request) return } @@ -143,12 +161,12 @@ func (dm discordMessage) Send() { }, } if _, err := dm.Session.ChannelMessageSendComplex(dm.Event.ChannelID, params); err != nil { - log.Error("Error in uploading file:", err) + dm.log.Error("Error in uploading file:", err) } return } if _, err := dm.Session.ChannelMessageSend(dm.Event.ChannelID, formatCodeBlock(dm.Response)); err != nil { - log.Error("Error in sending message:", err) + dm.log.Error("Error in sending message:", err) } } diff --git a/pkg/bot/lark.go b/pkg/bot/lark.go index da808b748..ce11942e1 100644 --- a/pkg/bot/lark.go +++ b/pkg/bot/lark.go @@ -20,17 +20,19 @@ package bot import ( + "context" "fmt" "net/http" "strings" "github.com/larksuite/oapi-sdk-go/core" "github.com/larksuite/oapi-sdk-go/event" - eventhttpserver "github.com/larksuite/oapi-sdk-go/event/http/native" + eventhttpserver "github.com/larksuite/oapi-sdk-go/event/http" + "github.com/sirupsen/logrus" + + "github.com/infracloudio/botkube/pkg/httpsrv" "github.com/infracloudio/botkube/pkg/config" - "github.com/infracloudio/botkube/pkg/execute" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/utils" ) @@ -69,6 +71,9 @@ const ( // LarkBot listens for user's message, execute commands and sends back the response type LarkBot struct { + log logrus.FieldLogger + executorFactory ExecutorFactory + AllowKubectl bool RestrictAccess bool ClusterName string @@ -79,24 +84,26 @@ type LarkBot struct { } // NewLarkBot returns new Bot object -func NewLarkBot(c *config.Config) Bot { +func NewLarkBot(log logrus.FieldLogger, loggerLevel logrus.Level, c *config.Config, executorFactory ExecutorFactory) *LarkBot { larkConf := c.Communications.Lark appSettings := core.NewInternalAppSettings(core.SetAppCredentials(larkConf.AppID, larkConf.AppSecret), core.SetAppEventKey(larkConf.VerificationToken, larkConf.EncryptKey)) - conf := core.NewConfig(core.Domain(larkConf.Endpoint), appSettings, core.SetLoggerLevel(utils.GetLoggerLevel())) + conf := core.NewConfig(core.Domain(larkConf.Endpoint), appSettings, core.SetLoggerLevel(utils.GetLoggerLevel(loggerLevel))) return &LarkBot{ + log: log, + executorFactory: executorFactory, AllowKubectl: c.Settings.Kubectl.Enabled, RestrictAccess: c.Settings.Kubectl.RestrictAccess, ClusterName: c.Settings.ClusterName, DefaultNamespace: c.Settings.Kubectl.DefaultNamespace, Port: c.Communications.Lark.Port, MessagePath: c.Communications.Lark.MessagePath, - LarkClient: utils.NewLarkClient(conf), + LarkClient: utils.NewLarkClient(log, conf), } } // Execute commands sent by users -func (l *LarkBot) Execute(e map[string]interface{}) error { +func (l *LarkBot) Execute(ctx context.Context, e map[string]interface{}) error { event, err := accessAndTypeCastToMap(larkEvent, e) if err != nil { return fmt.Errorf("while getting event: %w", err) @@ -112,8 +119,7 @@ func (l *LarkBot) Execute(e map[string]interface{}) error { return fmt.Errorf("while getting text: %w", err) } - executor := execute.NewDefaultExecutor(text, l.AllowKubectl, l.RestrictAccess, l.DefaultNamespace, - l.ClusterName, config.LarkBot, "", true) + executor := l.executorFactory.NewDefault(config.LarkBot, true, text) response := executor.Execute() if chatType == larkGroup { @@ -122,7 +128,7 @@ func (l *LarkBot) Execute(e map[string]interface{}) error { return fmt.Errorf("while getting open chat ID: %w", err) } - err = l.LarkClient.SendTextMessage(larkChatID, chatID, response) + err = l.LarkClient.SendTextMessage(ctx, larkChatID, chatID, response) if err != nil { return fmt.Errorf("while sending group chat message: %w", err) } @@ -132,7 +138,7 @@ func (l *LarkBot) Execute(e map[string]interface{}) error { if err != nil { return fmt.Errorf("while getting open ID: %w", err) } - err = l.LarkClient.SendTextMessage(larkOpenID, openID, response) + err = l.LarkClient.SendTextMessage(ctx, larkOpenID, openID, response) if err != nil { return fmt.Errorf("while sending private chat message: %w", err) } @@ -141,13 +147,13 @@ func (l *LarkBot) Execute(e map[string]interface{}) error { } // Start starts the lark server and listens for lark messages -func (l *LarkBot) Start() error { +func (l *LarkBot) Start(ctx context.Context) error { // See: https://open.larksuite.com/document/ukTMukTMukTM/ukjNxYjL5YTM24SO2EjN larkConf := l.LarkClient.Conf helloCallbackFn := func(ctx *core.Context, e map[string]interface{}) error { - err := l.SayHello(e) + err := l.SayHello(ctx, e) if err != nil { - log.Error(err) + l.log.Error(err) return err } @@ -156,9 +162,9 @@ func (l *LarkBot) Start() error { // message event.SetTypeCallback(larkConf, larkMessage, func(ctx *core.Context, e map[string]interface{}) error { - err := l.Execute(e) + err := l.Execute(ctx, e) if err != nil { - log.Error(err) + l.log.Error(err) return err } @@ -174,18 +180,23 @@ func (l *LarkBot) Start() error { // add_user_to_chat event.SetTypeCallback(larkConf, larkAddUserToChat, helloCallbackFn) - eventhttpserver.Register(l.MessagePath, larkConf) + addr := fmt.Sprintf(":%d", l.Port) + mux := http.NewServeMux() + mux.HandleFunc(l.MessagePath, func(responseWriter http.ResponseWriter, request *http.Request) { + eventhttpserver.Handle(larkConf, request, responseWriter) + }) - log.Infof("Starting Lark server on port %d", l.Port) - if err := http.ListenAndServe(fmt.Sprintf(":%d", l.Port), nil); err != nil { - return fmt.Errorf("while listening on port %d: %w", l.Port, err) + srv := httpsrv.New(l.log, addr, mux) + err := srv.Serve(ctx) + if err != nil { + return fmt.Errorf("while running Lark server: %w", err) } return nil } // SayHello send welcome message to new added users -func (l *LarkBot) SayHello(e map[string]interface{}) error { +func (l *LarkBot) SayHello(ctx context.Context, e map[string]interface{}) error { event, err := accessAndTypeCastToMap(larkEvent, e) if err != nil { return fmt.Errorf("while getting event: %w", err) @@ -211,17 +222,17 @@ func (l *LarkBot) SayHello(e map[string]interface{}) error { for _, user := range users { userMap, ok := user.(map[string]interface{}) if !ok { - log.Errorf("while asserting type of user: Failed to convert %T into map[string]interface{}", user) + l.log.Errorf("while asserting type of user: Failed to convert %T into map[string]interface{}", user) continue } openID, err := accessAndTypeCastToString(larkOpenID, userMap) if err != nil { - log.Errorf("while getting open ID: %s", err.Error()) + l.log.Errorf("while getting open ID: %s", err.Error()) continue } username, err := accessAndTypeCastToString(larkUserID, userMap) if err != nil { - log.Errorf("while getting user ID: %s", err.Error()) + l.log.Errorf("while getting user ID: %s", err.Error()) continue } @@ -234,7 +245,7 @@ func (l *LarkBot) SayHello(e map[string]interface{}) error { if err != nil { return fmt.Errorf("while getting chat ID: %w", err) } - err = l.LarkClient.SendTextMessage(larkChatID, chatID, strings.Join(messages, " ")) + err = l.LarkClient.SendTextMessage(ctx, larkChatID, chatID, strings.Join(messages, " ")) if err != nil { return fmt.Errorf("while sending text message: %w", err) } diff --git a/pkg/bot/mattermost.go b/pkg/bot/mattermost.go index 00b45cc13..b74d2065e 100644 --- a/pkg/bot/mattermost.go +++ b/pkg/bot/mattermost.go @@ -20,15 +20,15 @@ package bot import ( + "context" "fmt" "net/url" "strings" "github.com/mattermost/mattermost-server/v5/model" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" - "github.com/infracloudio/botkube/pkg/execute" - "github.com/infracloudio/botkube/pkg/log" ) // mmChannelType to find Mattermost channel type @@ -46,8 +46,15 @@ const ( WebSocketSecureProtocol = "wss://" ) +// TODO: +// - Use latest Mattermost API v6 +// - Remove usage of `log.Fatal` - return error instead + // MMBot listens for user's message, execute commands and sends back the response type MMBot struct { + log logrus.FieldLogger + executorFactory ExecutorFactory + Token string BotName string TeamName string @@ -64,6 +71,9 @@ type MMBot struct { // mattermostMessage contains message details to execute command and send back the result type mattermostMessage struct { + log logrus.FieldLogger + executorFactory ExecutorFactory + Event *model.WebSocketEvent Response string Request string @@ -72,8 +82,10 @@ type mattermostMessage struct { } // NewMattermostBot returns new Bot object -func NewMattermostBot(c *config.Config) Bot { +func NewMattermostBot(log logrus.FieldLogger, c *config.Config, executorFactory ExecutorFactory) *MMBot { return &MMBot{ + log: log, + executorFactory: executorFactory, ServerURL: c.Communications.Mattermost.URL, BotName: c.Communications.Mattermost.BotName, Token: c.Communications.Mattermost.Token, @@ -87,7 +99,8 @@ func NewMattermostBot(c *config.Config) Bot { } // Start establishes mattermost connection and listens for messages -func (b *MMBot) Start() error { +func (b *MMBot) Start(ctx context.Context) error { + b.log.Info("Starting bot") b.APIClient = model.NewAPIv4Client(b.ServerURL) b.APIClient.SetOAuthToken(b.Token) @@ -109,24 +122,28 @@ func (b *MMBot) Start() error { return fmt.Errorf("while pinging Mattermost server %q: %w", b.ServerURL, err) } - go func() { - // It is observed that Mattermost server closes connections unexpectedly after some time. - // For now, we are adding retry logic to reconnect to the server - // https://github.com/infracloudio/botkube/issues/201 - log.Info("BotKube connected to Mattermost!") - for { + // It is observed that Mattermost server closes connections unexpectedly after some time. + // For now, we are adding retry logic to reconnect to the server + // https://github.com/infracloudio/botkube/issues/201 + b.log.Info("BotKube connected to Mattermost!") + for { + select { + case <-ctx.Done(): + b.log.Info("Shutdown requested. Finishing...") + return nil + default: var appErr *model.AppError b.WSClient, appErr = model.NewWebSocketClient4(b.WebSocketURL, b.APIClient.AuthToken) if appErr != nil { - log.Errorf("Error creating WebSocket for Mattermost connectivity. %v", appErr) - return + return fmt.Errorf("while creating WebSocket connection: %w", appErr) } - b.listen() + b.listen(ctx) } - }() - return nil + } } +// TODO: refactor - handle and send methods should be defined on Bot level + // Check incoming message and take action func (mm *mattermostMessage) handleMessage(b MMBot) { post := model.PostFromJson(strings.NewReader(mm.Event.Data["post"].(string))) @@ -143,42 +160,41 @@ func (mm *mattermostMessage) handleMessage(b MMBot) { if mm.Event.Broadcast.ChannelId == b.getChannel().Id { mm.IsAuthChannel = true } - log.Debugf("Received mattermost event: %+v", mm.Event.Data) + mm.log.Debugf("Received mattermost event: %+v", mm.Event.Data) // Trim the @BotKube prefix if exists mm.Request = strings.TrimPrefix(post.Message, "@"+b.BotName+" ") - e := execute.NewDefaultExecutor(mm.Request, b.AllowKubectl, b.RestrictAccess, b.DefaultNamespace, - b.ClusterName, config.MattermostBot, b.ChannelName, mm.IsAuthChannel) + e := mm.executorFactory.NewDefault(config.MattermostBot, mm.IsAuthChannel, mm.Request) mm.Response = e.Execute() mm.sendMessage() } // Send messages to Mattermost func (mm mattermostMessage) sendMessage() { - log.Debugf("Mattermost incoming Request: %s", mm.Request) - log.Debugf("Mattermost Response: %s", mm.Response) + mm.log.Debugf("Mattermost incoming Request: %s", mm.Request) + mm.log.Debugf("Mattermost Response: %s", mm.Response) post := &model.Post{} post.ChannelId = mm.Event.Broadcast.ChannelId if len(mm.Response) == 0 { - log.Infof("Invalid request. Dumping the response. Request: %s", mm.Request) + mm.log.Infof("Invalid request. Dumping the response. Request: %s", mm.Request) return } // Create file if message is too large if len(mm.Response) >= 3990 { res, resp := mm.APIClient.UploadFileAsRequestBody([]byte(mm.Response), mm.Event.Broadcast.ChannelId, mm.Request) if resp.Error != nil { - log.Error("Error occurred while uploading file. Error: ", resp.Error) + mm.log.Error("Error occurred while uploading file. Error: ", resp.Error) } - post.FileIds = []string{string(res.FileInfos[0].Id)} + post.FileIds = []string{res.FileInfos[0].Id} } else { post.Message = formatCodeBlock(mm.Response) } // Create a post in the Channel if _, resp := mm.APIClient.CreatePost(post); resp.Error != nil { - log.Error("Failed to send message. Error: ", resp.Error) + mm.log.Error("Failed to send message. Error: ", resp.Error) } } @@ -201,7 +217,7 @@ func (b MMBot) checkServerConnection() error { func (b MMBot) getTeam() *model.Team { botTeam, resp := b.APIClient.GetTeamByName(b.TeamName, "") if resp.Error != nil { - log.Fatalf("There was a problem finding Mattermost team %s. %s", b.TeamName, resp.Error) + b.log.Fatalf("There was a problem finding Mattermost team %s. %s", b.TeamName, resp.Error) } return botTeam } @@ -210,7 +226,7 @@ func (b MMBot) getTeam() *model.Team { func (b MMBot) getUser() *model.User { users, resp := b.APIClient.AutocompleteUsersInTeam(b.getTeam().Id, b.BotName, 1, "") if resp.Error != nil { - log.Fatalf("There was a problem finding Mattermost user %s. %s", b.BotName, resp.Error) + b.log.Fatalf("There was a problem finding Mattermost user %s. %s", b.BotName, resp.Error) } return users.Users[0] } @@ -220,7 +236,7 @@ func (b MMBot) getChannel() *model.Channel { // Checking if channel exists botChannel, resp := b.APIClient.GetChannelByName(b.ChannelName, b.getTeam().Id, "") if resp.Error != nil { - log.Fatalf("There was a problem finding Mattermost channel %s. %s", b.ChannelName, resp.Error) + b.log.Fatalf("There was a problem finding Mattermost channel %s. %s", b.ChannelName, resp.Error) } // Adding BotKube user to channel @@ -228,30 +244,46 @@ func (b MMBot) getChannel() *model.Channel { return botChannel } -func (b MMBot) listen() { +func (b MMBot) listen(ctx context.Context) { b.WSClient.Listen() defer b.WSClient.Close() for { - if b.WSClient.ListenError != nil { - log.Debugf("Mattermost websocket listen error %s. Reconnecting...", b.WSClient.ListenError) + select { + case <-ctx.Done(): + b.log.Info("Shutdown requested. Finishing...") return - } + case event, ok := <-b.WSClient.EventChannel: + if !ok { + if b.WSClient.ListenError != nil { + b.log.Debugf("while listening on websocket connection: %s", b.WSClient.ListenError.Error()) + } - event := <-b.WSClient.EventChannel - if event == nil { - continue - } - if event.Event == model.WEBSOCKET_EVENT_POSTED { - post := model.PostFromJson(strings.NewReader(event.Data["post"].(string))) + b.log.Info("Incoming events channel closed. Finishing...") + return + } + + if event == nil { + b.log.Info("Nil event, ignoring") + continue + } + + if event.EventType() != model.WEBSOCKET_EVENT_POSTED { + // ignore + continue + } + + post := model.PostFromJson(strings.NewReader(event.GetData()["post"].(string))) // Skip if message posted by BotKube or doesn't start with mention if post.UserId == b.getUser().Id { continue } mm := mattermostMessage{ - Event: event, - IsAuthChannel: false, - APIClient: b.APIClient, + log: b.log, + executorFactory: b.executorFactory, + Event: event, + IsAuthChannel: false, + APIClient: b.APIClient, } mm.handleMessage(b) } diff --git a/pkg/bot/slack.go b/pkg/bot/slack.go index 0ef1ea417..3f55fbd83 100644 --- a/pkg/bot/slack.go +++ b/pkg/bot/slack.go @@ -20,18 +20,21 @@ package bot import ( + "context" "fmt" "strings" + "github.com/sirupsen/logrus" "github.com/slack-go/slack" "github.com/infracloudio/botkube/pkg/config" - "github.com/infracloudio/botkube/pkg/execute" - "github.com/infracloudio/botkube/pkg/log" ) // SlackBot listens for user's message, execute commands and sends back the response type SlackBot struct { + log logrus.FieldLogger + executorFactory ExecutorFactory + Token string AllowKubectl bool RestrictAccess bool @@ -40,12 +43,13 @@ type SlackBot struct { SlackURL string BotID string DefaultNamespace string - - NewExecutorFn func(msg string, allowKubectl, restrictAccess bool, defaultNamespace, clusterName string, platform config.BotPlatform, channelName string, isAuthChannel bool) execute.Executor } // slackMessage contains message details to execute command and send back the result type slackMessage struct { + log logrus.FieldLogger + executorFactory ExecutorFactory + Event *slack.MessageEvent BotID string Request string @@ -56,20 +60,22 @@ type slackMessage struct { } // NewSlackBot returns new Bot object -func NewSlackBot(c *config.Config) Bot { +func NewSlackBot(log logrus.FieldLogger, c *config.Config, executorFactory ExecutorFactory) *SlackBot { return &SlackBot{ + log: log, + executorFactory: executorFactory, Token: c.Communications.Slack.Token, AllowKubectl: c.Settings.Kubectl.Enabled, RestrictAccess: c.Settings.Kubectl.RestrictAccess, ClusterName: c.Settings.ClusterName, ChannelName: c.Communications.Slack.Channel, DefaultNamespace: c.Settings.Kubectl.DefaultNamespace, - NewExecutorFn: execute.NewDefaultExecutor, } } // Start starts the slacknot RTM connection and listens for messages -func (b *SlackBot) Start() error { +func (b *SlackBot) Start(ctx context.Context) error { + b.log.Info("Starting bot") var botID string api := slack.New(b.Token) if len(b.SlackURL) != 0 { @@ -78,57 +84,71 @@ func (b *SlackBot) Start() error { } else { authResp, err := api.AuthTest() if err != nil { - log.Fatal(err) + return fmt.Errorf("while testing the ability to do auth request: %w", err) } botID = authResp.UserID } - RTM := api.NewRTM() - go RTM.ManageConnection() - - for msg := range RTM.IncomingEvents { - switch ev := msg.Data.(type) { - case *slack.ConnectedEvent: - log.Info("BotKube connected to Slack!") - - case *slack.MessageEvent: - // Skip if message posted by BotKube - if ev.User == botID { - continue - } - sm := slackMessage{ - Event: ev, - BotID: botID, - RTM: RTM, - SlackClient: api, + rtm := api.NewRTM() + go rtm.ManageConnection() + + for { + select { + case <-ctx.Done(): + b.log.Info("Shutdown requested. Finishing...") + return rtm.Disconnect() + case msg, ok := <-rtm.IncomingEvents: + if !ok { + b.log.Info("Incoming events channel closed. Finishing...") + return nil } - sm.HandleMessage(b) - - case *slack.RTMError: - log.Errorf("Slack RMT error: %+v", ev.Error()) - - case *slack.ConnectionErrorEvent: - log.Errorf("Slack connection error: %+v", ev.Error()) - - case *slack.IncomingEventError: - log.Errorf("Slack incoming event error: %+v", ev.Error()) - - case *slack.OutgoingErrorEvent: - log.Errorf("Slack outgoing event error: %+v", ev.Error()) - - case *slack.UnmarshallingErrorEvent: - log.Errorf("Slack unmarshalling error: %+v", b.stripUnmarshallingErrEventDetails(ev.Error())) - case *slack.RateLimitedError: - log.Errorf("Slack rate limiting error: %+v", ev.Error()) - - case *slack.InvalidAuthEvent: - return fmt.Errorf("invalid credentials") - - default: + switch ev := msg.Data.(type) { + case *slack.ConnectedEvent: + b.log.Info("BotKube connected to Slack!") + + case *slack.MessageEvent: + // Skip if message posted by BotKube + if ev.User == botID { + continue + } + sm := slackMessage{ + log: b.log, + executorFactory: b.executorFactory, + Event: ev, + BotID: botID, + RTM: rtm, + SlackClient: api, + } + err := sm.HandleMessage(b) + if err != nil { + wrappedErr := fmt.Errorf("while handling message: %w", err) + b.log.Errorf(wrappedErr.Error()) + } + + case *slack.RTMError: + b.log.Errorf("Slack RMT error: %+v", ev.Error()) + + case *slack.ConnectionErrorEvent: + b.log.Errorf("Slack connection error: %+v", ev.Error()) + + case *slack.IncomingEventError: + b.log.Errorf("Slack incoming event error: %+v", ev.Error()) + + case *slack.OutgoingErrorEvent: + b.log.Errorf("Slack outgoing event error: %+v", ev.Error()) + + case *slack.UnmarshallingErrorEvent: + b.log.Errorf("Slack unmarshalling error: %+v", b.stripUnmarshallingErrEventDetails(ev.Error())) + + case *slack.RateLimitedError: + b.log.Errorf("Slack rate limiting error: %+v", ev.Error()) + + case *slack.InvalidAuthEvent: + return fmt.Errorf("invalid credentials") + } } } - return nil } // Temporary workaround until the PR is merged: https://github.com/slack-go/slack/pull/1067 @@ -152,7 +172,9 @@ func (b *SlackBot) stripUnmarshallingErrEventDetails(errMessage string) string { return strings.Join(msgParts[:2], errMsgSeparator) } -func (sm *slackMessage) HandleMessage(b *SlackBot) { +// TODO: refactor - handle and send methods should be defined on Bot level + +func (sm *slackMessage) HandleMessage(b *SlackBot) error { // Check if message posted in authenticated channel info, err := sm.SlackClient.GetConversationInfo(sm.Event.Channel, true) if err == nil { @@ -160,7 +182,8 @@ func (sm *slackMessage) HandleMessage(b *SlackBot) { // Message posted in a channel // Serve only if starts with mention if !strings.HasPrefix(sm.Event.Text, "<@"+sm.BotID+">") { - return + sm.log.Debugf("Ignoring message as it doesn't contain %q prefix", sm.BotID) + return nil } // Serve only if current channel is in config if b.ChannelName == info.Name { @@ -176,18 +199,21 @@ func (sm *slackMessage) HandleMessage(b *SlackBot) { // Trim the @BotKube prefix sm.Request = strings.TrimPrefix(sm.Event.Text, "<@"+sm.BotID+">") - e := b.NewExecutorFn(sm.Request, b.AllowKubectl, b.RestrictAccess, b.DefaultNamespace, - b.ClusterName, config.SlackBot, b.ChannelName, sm.IsAuthChannel) + e := sm.executorFactory.NewDefault(config.SlackBot, sm.IsAuthChannel, sm.Request) sm.Response = e.Execute() - sm.Send() + err = sm.Send() + if err != nil { + return fmt.Errorf("while sending message: %w", err) + } + + return nil } -func (sm *slackMessage) Send() { - log.Debugf("Slack incoming Request: %s", sm.Request) - log.Debugf("Slack Response: %s", sm.Response) +func (sm *slackMessage) Send() error { + sm.log.Debugf("Slack incoming Request: %s", sm.Request) + sm.log.Debugf("Slack Response: %s", sm.Response) if len(sm.Response) == 0 { - log.Infof("Invalid request. Dumping the response. Request: %s", sm.Request) - return + return fmt.Errorf("while reading Slack response: empty response for request %q", sm.Request) } // Upload message as a file if too long if len(sm.Response) >= 3990 { @@ -199,9 +225,9 @@ func (sm *slackMessage) Send() { } _, err := sm.RTM.UploadFile(params) if err != nil { - log.Error("Error in uploading file:", err) + return fmt.Errorf("while uploading file: %w", err) } - return + return nil } var options = []slack.MsgOption{slack.MsgOptionText(formatCodeBlock(sm.Response), false), slack.MsgOptionAsUser(true)} @@ -212,6 +238,8 @@ func (sm *slackMessage) Send() { } if _, _, err := sm.RTM.PostMessage(sm.Event.Channel, options...); err != nil { - log.Error("Error in sending message:", err) + return fmt.Errorf("while posting Slack message: %w", err) } + + return nil } diff --git a/pkg/bot/teams.go b/pkg/bot/teams.go index d36048e12..29adaf4ab 100644 --- a/pkg/bot/teams.go +++ b/pkg/bot/teams.go @@ -32,11 +32,12 @@ import ( "github.com/infracloudio/msbotbuilder-go/core" coreActivity "github.com/infracloudio/msbotbuilder-go/core/activity" "github.com/infracloudio/msbotbuilder-go/schema" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" "github.com/infracloudio/botkube/pkg/execute" - "github.com/infracloudio/botkube/pkg/log" + "github.com/infracloudio/botkube/pkg/httpsrv" ) const ( @@ -57,6 +58,9 @@ var _ Bot = (*Teams)(nil) // Teams contains credentials to start Teams backend server type Teams struct { + log logrus.FieldLogger + executorFactory ExecutorFactory + AppID string AppPassword string MessagePath string @@ -76,7 +80,7 @@ type consentContext struct { } // NewTeamsBot returns Teams instance -func NewTeamsBot(c *config.Config) *Teams { +func NewTeamsBot(log logrus.FieldLogger, c *config.Config, executorFactory ExecutorFactory) *Teams { // Set notifier off by default config.Notify = false port := c.Communications.Teams.Port @@ -88,6 +92,8 @@ func NewTeamsBot(c *config.Config) *Teams { msgPath = "/" } return &Teams{ + log: log, + executorFactory: executorFactory, AppID: c.Communications.Teams.AppID, AppPassword: c.Communications.Teams.AppPassword, NotifType: c.Communications.Teams.NotifType, @@ -101,46 +107,51 @@ func NewTeamsBot(c *config.Config) *Teams { } // Start MS Teams server to serve messages from Teams client -func (t *Teams) Start() error { +func (b *Teams) Start(ctx context.Context) error { + b.log.Info("Starting bot") var err error setting := core.AdapterSetting{ - AppID: t.AppID, - AppPassword: t.AppPassword, + AppID: b.AppID, + AppPassword: b.AppPassword, } - t.Adapter, err = core.NewBotAdapter(setting) + b.Adapter, err = core.NewBotAdapter(setting) if err != nil { return fmt.Errorf("while starting Teams bot: %w", err) } - // Start consent cleanup - http.HandleFunc(t.MessagePath, t.processActivity) - log.Infof("Started MS Teams server on port %s", defaultPort) - if err := http.ListenAndServe(fmt.Sprintf(":%s", t.Port), nil); err != nil { + + addr := fmt.Sprintf(":%s", b.Port) + mux := http.NewServeMux() + mux.HandleFunc(b.MessagePath, b.processActivity) + + srv := httpsrv.New(b.log, addr, mux) + err = srv.Serve(ctx) + if err != nil { return fmt.Errorf("while running MS Teams server: %w", err) } return nil } -func (t *Teams) deleteConsent(ID string, convRef schema.ConversationReference) { - log.Debugf("Deleting activity %s\n", ID) - if err := t.Adapter.DeleteActivity(context.Background(), ID, convRef); err != nil { - log.Errorf("Failed to delete activity. %s", err.Error()) +func (b *Teams) deleteConsent(ctx context.Context, ID string, convRef schema.ConversationReference) { + b.log.Debugf("Deleting activity %s\n", ID) + if err := b.Adapter.DeleteActivity(ctx, ID, convRef); err != nil { + b.log.Errorf("Failed to delete activity. %s", err.Error()) } } -func (t *Teams) processActivity(w http.ResponseWriter, req *http.Request) { - ctx := context.Background() - log.Debugf("Received activity %v\n", req) - activity, err := t.Adapter.ParseRequest(ctx, req) +func (b *Teams) processActivity(w http.ResponseWriter, req *http.Request) { + ctx := req.Context() + b.log.Debugf("Received activity %v\n", req) + activity, err := b.Adapter.ParseRequest(ctx, req) if err != nil { - log.Errorf("Failed to parse Teams request. %s", err.Error()) + b.log.Errorf("Failed to parse Teams request. %s", err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } - err = t.Adapter.ProcessActivity(ctx, activity, coreActivity.HandlerFuncs{ + err = b.Adapter.ProcessActivity(ctx, activity, coreActivity.HandlerFuncs{ OnMessageFunc: func(turn *coreActivity.TurnContext) (schema.Activity, error) { - resp := t.processMessage(turn.Activity) + resp := b.processMessage(turn.Activity) if len(resp) >= maxMessageSize { if turn.Activity.Conversation.ConversationType == convTypePersonal { // send file upload request @@ -159,7 +170,7 @@ func (t *Teams) processActivity(w http.ResponseWriter, req *http.Request) { } return turn.SendActivity(coreActivity.MsgOptionAttachments(attachments)) } - resp = fmt.Sprintf("%s\n```\nCluster: %s\n%s", longRespNotice, t.ClusterName, resp[len(resp)-maxMessageSize:]) + resp = fmt.Sprintf("%s\n```\nCluster: %s\n%s", longRespNotice, b.ClusterName, resp[len(resp)-maxMessageSize:]) } return turn.SendActivity(coreActivity.MsgOptionText(resp)) }, @@ -167,7 +178,7 @@ func (t *Teams) processActivity(w http.ResponseWriter, req *http.Request) { // handle invoke events // https://developer.microsoft.com/en-us/microsoft-teams/blogs/working-with-files-in-your-microsoft-teams-bot/ OnInvokeFunc: func(turn *coreActivity.TurnContext) (schema.Activity, error) { - t.deleteConsent(turn.Activity.ReplyToID, coreActivity.GetCoversationReference(turn.Activity)) + b.deleteConsent(ctx, turn.Activity.ReplyToID, coreActivity.GetCoversationReference(turn.Activity)) if err != nil { return schema.Activity{}, fmt.Errorf("failed to read file: %s", err.Error()) } @@ -202,15 +213,14 @@ func (t *Teams) processActivity(w http.ResponseWriter, req *http.Request) { } msg := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(consentCtx.Command), "BotKube")) - e := execute.NewDefaultExecutor(msg, t.AllowKubectl, t.RestrictAccess, t.DefaultNamespace, - t.ClusterName, config.TeamsBot, "", true) + e := b.executorFactory.NewDefault(config.TeamsBot, true, msg) out := e.Execute() actJSON, _ := json.MarshalIndent(turn.Activity, "", " ") - log.Debugf("Incoming MSTeams Activity: %s", actJSON) + b.log.Debugf("Incoming MSTeams Activity: %s", actJSON) // upload file - err = t.putRequest(uploadInfo.UploadURL, []byte(out)) + err = b.putRequest(uploadInfo.UploadURL, []byte(out)) if err != nil { return schema.Activity{}, fmt.Errorf("failed to upload file: %s", err.Error()) } @@ -231,11 +241,11 @@ func (t *Teams) processActivity(w http.ResponseWriter, req *http.Request) { }, }) if err != nil { - log.Errorf("Failed to process request. %s", err.Error()) + b.log.Errorf("Failed to process request. %s", err.Error()) } } -func (t *Teams) processMessage(activity schema.Activity) string { +func (b *Teams) processMessage(activity schema.Activity) string { // Trim @BotKube prefix msg := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(activity.Text), "BotKube")) @@ -249,23 +259,22 @@ func (t *Teams) processMessage(activity schema.Activity) string { if execute.Start.String() == args[1] { config.Notify = true ref := coreActivity.GetCoversationReference(activity) - t.ConversationRef = &ref + b.ConversationRef = &ref // Remove messageID from the ChannelID if ID, ok := activity.ChannelData["teamsChannelId"]; ok { - t.ConversationRef.ChannelID = ID.(string) - t.ConversationRef.Conversation.ID = ID.(string) + b.ConversationRef.ChannelID = ID.(string) + b.ConversationRef.Conversation.ID = ID.(string) } - return fmt.Sprintf(execute.NotifierStartMsg, t.ClusterName) + return fmt.Sprintf(execute.NotifierStartMsg, b.ClusterName) } } // Multicluster is not supported for Teams - e := execute.NewDefaultExecutor(msg, t.AllowKubectl, t.RestrictAccess, t.DefaultNamespace, - t.ClusterName, config.TeamsBot, "", true) + e := b.executorFactory.NewDefault(config.TeamsBot, true, msg) return formatCodeBlock(e.Execute()) } -func (t *Teams) putRequest(u string, data []byte) (err error) { +func (b *Teams) putRequest(u string, data []byte) (err error) { client := &http.Client{} dec, err := url.QueryUnescape(u) if err != nil { @@ -296,22 +305,22 @@ func (t *Teams) putRequest(u string, data []byte) (err error) { } // SendEvent sends event message via Bot interface -func (t *Teams) SendEvent(event events.Event) error { - card := formatTeamsMessage(event, t.NotifType) - if err := t.sendProactiveMessage(card); err != nil { - log.Errorf("Failed to send notification. %s", err.Error()) +func (b *Teams) SendEvent(ctx context.Context, event events.Event) error { + card := formatTeamsMessage(event, b.NotifType) + if err := b.sendProactiveMessage(ctx, card); err != nil { + b.log.Errorf("Failed to send notification. %s", err.Error()) } - log.Debugf("Event successfully sent to MS Teams >> %+v", event) + b.log.Debugf("Event successfully sent to MS Teams >> %+v", event) return nil } // SendMessage sends message to MsTeams -func (t *Teams) SendMessage(msg string) error { - if t.ConversationRef == nil { - log.Infof("Skipping SendMessage since conversation ref not set") +func (b *Teams) SendMessage(ctx context.Context, msg string) error { + if b.ConversationRef == nil { + b.log.Infof("Skipping SendMessage since conversation ref not set") return nil } - err := t.Adapter.ProactiveMessage(context.TODO(), *t.ConversationRef, coreActivity.HandlerFuncs{ + err := b.Adapter.ProactiveMessage(ctx, *b.ConversationRef, coreActivity.HandlerFuncs{ OnMessageFunc: func(turn *coreActivity.TurnContext) (schema.Activity, error) { return turn.SendActivity(coreActivity.MsgOptionText(msg)) }, @@ -319,16 +328,16 @@ func (t *Teams) SendMessage(msg string) error { if err != nil { return err } - log.Debug("Message successfully sent to MS Teams") + b.log.Debug("Message successfully sent to MS Teams") return nil } -func (t *Teams) sendProactiveMessage(card map[string]interface{}) error { - if t.ConversationRef == nil { - log.Infof("Skipping SendMessage since conversation ref not set") +func (b *Teams) sendProactiveMessage(ctx context.Context, card map[string]interface{}) error { + if b.ConversationRef == nil { + b.log.Infof("Skipping SendMessage since conversation ref not set") return nil } - err := t.Adapter.ProactiveMessage(context.TODO(), *t.ConversationRef, coreActivity.HandlerFuncs{ + err := b.Adapter.ProactiveMessage(ctx, *b.ConversationRef, coreActivity.HandlerFuncs{ OnMessageFunc: func(turn *coreActivity.TurnContext) (schema.Activity, error) { attachments := []schema.Attachment{ { diff --git a/pkg/config/config.go b/pkg/config/config.go index b8d97a94a..3c97af4e4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -253,7 +253,7 @@ func (eventType EventType) String() string { } // NewCommunicationsConfig return new communication config object -func NewCommunicationsConfig() (*Communications, error) { +func NewCommunicationsConfig() (c *Communications, err error) { configPath := os.Getenv("CONFIG_PATH") commCfgFilePath := filepath.Join(configPath, CommunicationConfigFileName) rawCfg, err := os.ReadFile(filepath.Clean(commCfgFilePath)) @@ -268,10 +268,9 @@ func NewCommunicationsConfig() (*Communications, error) { return commCfg, nil } -// New returns new Config -func New() (*Config, error) { - configPath := os.Getenv("CONFIG_PATH") - resCfgFilePath := filepath.Join(configPath, ResourceConfigFileName) +// Load loads new configuration from file. +func Load(dir string) (*Config, error) { + resCfgFilePath := filepath.Join(dir, ResourceConfigFileName) rawCfg, err := os.ReadFile(filepath.Clean(resCfgFilePath)) if err != nil { return nil, err diff --git a/pkg/controller/config_watcher.go b/pkg/controller/config_watcher.go new file mode 100644 index 000000000..c76ffae90 --- /dev/null +++ b/pkg/controller/config_watcher.go @@ -0,0 +1,90 @@ +package controller + +import ( + "context" + "fmt" + "path/filepath" + + "github.com/fsnotify/fsnotify" + "github.com/hashicorp/go-multierror" + "github.com/sirupsen/logrus" + "golang.org/x/sync/errgroup" + + "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/notify" +) + +// ConfigWatcher watches for the config file changes and exits the app. +// TODO: It keeps the previous behavior for now, but it should hot-reload the configuration files without needing a restart. +// Also, it should watch both files. +type ConfigWatcher struct { + log logrus.FieldLogger + configPath string + clusterName string + notifiers []notify.Notifier +} + +// NewConfigWatcher returns new ConfigWatcher instance. +func NewConfigWatcher(log logrus.FieldLogger, configPath string, clusterName string, notifiers []notify.Notifier) *ConfigWatcher { + return &ConfigWatcher{log: log, configPath: configPath, clusterName: clusterName, notifiers: notifiers} +} + +// Do starts watching the configuration file +func (w *ConfigWatcher) Do(ctx context.Context, cancelFunc context.CancelFunc) (err error) { + configFile := filepath.Join(w.configPath, config.ResourceConfigFileName) + + watcher, err := fsnotify.NewWatcher() + if err != nil { + return fmt.Errorf("while creating file watcher: %w", err) + } + defer func() { + deferredErr := watcher.Close() + if deferredErr != nil { + err = multierror.Append(err, deferredErr) + } + }() + + ctx, cancelFn := context.WithCancel(ctx) + defer cancelFn() + + errGroup, _ := errgroup.WithContext(ctx) + errGroup.Go(func() error { + for { + select { + case <-ctx.Done(): + w.log.Info("Shutdown requested. Finishing...") + return nil + case _, ok := <-watcher.Events: + if !ok { + return fmt.Errorf("unexpected file watch end") + } + + w.log.Infof("Config file %q is updated. Sending last message before exit...", configFile) + err := sendMessageToNotifiers(ctx, w.notifiers, fmt.Sprintf(configUpdateMsg, w.clusterName)) + if err != nil { + wrappedErr := fmt.Errorf("while sending message to notifiers: %w", err) + //do not exit yet, cancel the context first + cancelFunc() + return wrappedErr + } + + w.log.Infof("Cancelling the context...") + cancelFunc() + return nil + case err, ok := <-watcher.Errors: + if !ok { + return fmt.Errorf("unexpected file watch end") + } + return fmt.Errorf("while reading events for config file %q: %w", configFile, err) + } + } + }) + + w.log.Infof("Registering watcher on config file %q", configFile) + err = watcher.Add(configFile) + if err != nil { + return fmt.Errorf("while registering watch on config file %q: %w", configFile, err) + } + + return errGroup.Wait() +} diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 152b63cdc..10f7a7dbb 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -20,171 +20,237 @@ package controller import ( + "context" "fmt" - "os" - "os/signal" - "path/filepath" "reflect" "strings" - "syscall" "time" + "github.com/sirupsen/logrus" + coreV1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/dynamic/dynamicinformer" + "k8s.io/client-go/tools/cache" + "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" "github.com/infracloudio/botkube/pkg/filterengine" - - // Register filters - _ "github.com/infracloudio/botkube/pkg/filterengine/filters" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/notify" "github.com/infracloudio/botkube/pkg/utils" - - "github.com/fsnotify/fsnotify" - coreV1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/tools/cache" ) const ( controllerStartMsg = "...and now my watch begins for cluster '%s'! :crossed_swords:" controllerStopMsg = "My watch has ended for cluster '%s'!\nPlease send `@BotKube notifier start` to enable notification once BotKube comes online." configUpdateMsg = "Looks like the configuration is updated for cluster '%s'. I shall halt my watch till I read it." + + finalMessageTimeout = 20 * time.Second ) -var eventGVR = schema.GroupVersionResource{ - Version: "v1", - Resource: "events", +// EventKind defines a map key used for event filtering. +// TODO: Do not export it when E2E tests are refactored (https://github.com/infracloudio/botkube/issues/589) +type EventKind struct { + Resource string + Namespace string + EventType config.EventType } -var startTime time.Time +// KindNS defines a map key used for update event filtering. +// TODO: Do not export it when E2E tests are refactored (https://github.com/infracloudio/botkube/issues/589) +type KindNS struct { + Resource string + Namespace string +} + +// Controller watches Kubernetes resources and send events to notifiers. +type Controller struct { + log logrus.FieldLogger + startTime time.Time + conf *config.Config + notifiers []notify.Notifier + filterEngine filterengine.FilterEngine + configPath string + informersResyncPeriod time.Duration + + dynamicCli dynamic.Interface + mapper meta.RESTMapper + + dynamicKubeInformerFactory dynamicinformer.DynamicSharedInformerFactory + resourceInformerMap map[string]cache.SharedIndexInformer + observedEventKindsMap map[EventKind]bool + observedUpdateEventsMap map[KindNS]config.UpdateSetting +} + +// New create a new Controller instance. +func New(log logrus.FieldLogger, + conf *config.Config, + notifiers []notify.Notifier, + filterEngine filterengine.FilterEngine, + configPath string, + dynamicCli dynamic.Interface, + mapper meta.RESTMapper, + informersResyncPeriod time.Duration, +) *Controller { + return &Controller{ + log: log, + conf: conf, + notifiers: notifiers, + filterEngine: filterEngine, + configPath: configPath, + dynamicCli: dynamicCli, + mapper: mapper, + informersResyncPeriod: informersResyncPeriod, + } +} -// RegisterInformers creates new informer controllers to watch k8s resources -func RegisterInformers(c *config.Config, notifiers []notify.Notifier) { - sendMessage(notifiers, fmt.Sprintf(controllerStartMsg, c.Settings.ClusterName)) - startTime = time.Now() +// Start creates new informer controllers to watch k8s resources +func (c *Controller) Start(ctx context.Context) error { + err := c.initInformerMap() + if err != nil { + return fmt.Errorf("while initializing informer map: %w", err) + } - // Start config file watcher if enabled - if c.Settings.ConfigWatcher { - go configWatcher(c, notifiers) + c.log.Info("Starting controller") + err = sendMessageToNotifiers(ctx, c.notifiers, fmt.Sprintf(controllerStartMsg, c.conf.Settings.ClusterName)) + if err != nil { + return fmt.Errorf("while sending first message: %w", err) } + c.startTime = time.Now() + // Register informers for resource lifecycle events - if len(c.Resources) > 0 { - log.Info("Registering resource lifecycle informer") - for _, r := range c.Resources { - if _, ok := utils.ResourceInformerMap[r.Name]; !ok { + if len(c.conf.Resources) > 0 { + c.log.Info("Registering resource lifecycle informer") + for _, r := range c.conf.Resources { + if _, ok := c.resourceInformerMap[r.Name]; !ok { continue } - log.Infof("Adding informer for resource:%s", r.Name) - utils.ResourceInformerMap[r.Name].AddEventHandler(registerEventHandlers(c, notifiers, r.Name, r.Events)) + c.log.Infof("Adding informer for resource %q", r.Name) + c.resourceInformerMap[r.Name].AddEventHandler(c.registerEventHandlers(ctx, r.Name, r.Events)) } } // Register informers for k8s events - log.Infof("Registering kubernetes events informer for types: %+v", config.WarningEvent.String()) - log.Infof("Registering kubernetes events informer for types: %+v", config.NormalEvent.String()) - utils.DynamicKubeInformerFactory.ForResource(eventGVR).Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - var eventObj coreV1.Event - err := utils.TransformIntoTypedObject(obj.(*unstructured.Unstructured), &eventObj) - if err != nil { - log.Errorf("Unable to transform object type: %v, into type: %v", reflect.TypeOf(obj), reflect.TypeOf(eventObj)) - } - _, err = cache.MetaNamespaceKeyFunc(obj) - if err != nil { - log.Errorf("Failed to get MetaNamespaceKey from event resource") - return - } + c.log.Infof("Registering Kubernetes events informer for types %q and %q", config.WarningEvent.String(), config.NormalEvent.String()) + c.dynamicKubeInformerFactory. + ForResource(schema.GroupVersionResource{Version: "v1", Resource: "events"}). + Informer(). + AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + var eventObj coreV1.Event + err := utils.TransformIntoTypedObject(obj.(*unstructured.Unstructured), &eventObj) + if err != nil { + c.log.Errorf("Unable to transform object type: %v, into type: %v", reflect.TypeOf(obj), reflect.TypeOf(eventObj)) + } + _, err = cache.MetaNamespaceKeyFunc(obj) + if err != nil { + c.log.Errorf("Failed to get MetaNamespaceKey from event resource") + return + } - // Find involved object type - gvr, err := utils.GetResourceFromKind(eventObj.InvolvedObject.GroupVersionKind()) - if err != nil { - log.Errorf("Failed to get involved object: %v", err) - return - } - switch strings.ToLower(eventObj.Type) { - case config.WarningEvent.String(): - // Send WarningEvent as ErrorEvents - sendEvent(obj, nil, c, notifiers, utils.GVRToString(gvr), config.ErrorEvent) - case config.NormalEvent.String(): - // Send NormalEvent as Insignificant InfoEvent - sendEvent(obj, nil, c, notifiers, utils.GVRToString(gvr), config.InfoEvent) - } - }, - }) - stopCh := make(chan struct{}) - defer close(stopCh) + // Find involved object type + gvr, err := utils.GetResourceFromKind(c.mapper, eventObj.InvolvedObject.GroupVersionKind()) + if err != nil { + c.log.Errorf("Failed to get involved object: %v", err) + return + } + switch strings.ToLower(eventObj.Type) { + case config.WarningEvent.String(): + // Send WarningEvent as ErrorEvents + c.sendEvent(ctx, obj, nil, utils.GVRToString(gvr), config.ErrorEvent) + case config.NormalEvent.String(): + // Send NormalEvent as Insignificant InfoEvent + c.sendEvent(ctx, obj, nil, utils.GVRToString(gvr), config.InfoEvent) + } + }, + }) + + stopCh := ctx.Done() - utils.DynamicKubeInformerFactory.Start(stopCh) + c.dynamicKubeInformerFactory.Start(stopCh) + <-stopCh - sigterm := make(chan os.Signal, 1) - signal.Notify(sigterm, syscall.SIGTERM, syscall.SIGINT, syscall.SIGQUIT) + c.log.Info("Shutdown requested. Sending final message...") + finalMsgCtx, cancelFn := context.WithTimeout(context.Background(), finalMessageTimeout) + defer cancelFn() + err = sendMessageToNotifiers(finalMsgCtx, c.notifiers, fmt.Sprintf(controllerStopMsg, c.conf.Settings.ClusterName)) + if err != nil { + return fmt.Errorf("while sending final message: %w", err) + } - <-sigterm - sendMessage(notifiers, fmt.Sprintf(controllerStopMsg, c.Settings.ClusterName)) - // Sleep for some time to send termination notification - time.Sleep(5 * time.Second) + return nil } -func registerEventHandlers(c *config.Config, notifiers []notify.Notifier, resourceType string, events []config.EventType) (handlerFns cache.ResourceEventHandlerFuncs) { +func (c *Controller) registerEventHandlers(ctx context.Context, resourceType string, events []config.EventType) (handlerFns cache.ResourceEventHandlerFuncs) { for _, event := range events { if event == config.AllEvent || event == config.CreateEvent { handlerFns.AddFunc = func(obj interface{}) { - log.Debugf("Processing add to %v", resourceType) - sendEvent(obj, nil, c, notifiers, resourceType, config.CreateEvent) + c.log.Debugf("Processing add to %q", resourceType) + c.sendEvent(ctx, obj, nil, resourceType, config.CreateEvent) } } if event == config.AllEvent || event == config.UpdateEvent { handlerFns.UpdateFunc = func(old, new interface{}) { - log.Debugf("Processing update to %v\n Object: %+v\n", resourceType, new) - sendEvent(new, old, c, notifiers, resourceType, config.UpdateEvent) + c.log.Debugf("Processing update to %q\n Object: %+v\n", resourceType, new) + c.sendEvent(ctx, new, old, resourceType, config.UpdateEvent) } } if event == config.AllEvent || event == config.DeleteEvent { handlerFns.DeleteFunc = func(obj interface{}) { - log.Debugf("Processing delete to %v", resourceType) - sendEvent(obj, nil, c, notifiers, resourceType, config.DeleteEvent) + c.log.Debugf("Processing delete to %q", resourceType) + c.sendEvent(ctx, obj, nil, resourceType, config.DeleteEvent) } } } return handlerFns } -func sendEvent(obj, oldObj interface{}, c *config.Config, notifiers []notify.Notifier, resource string, eventType config.EventType) { +func (c *Controller) sendEvent(ctx context.Context, obj, oldObj interface{}, resource string, eventType config.EventType) { // Filter namespaces - objectMeta := utils.GetObjectMetaData(obj) + objectMeta, err := utils.GetObjectMetaData(ctx, c.dynamicCli, c.mapper, obj) + if err != nil { + c.log.Errorf("while getting object metadata: %s", err.Error()) + return + } switch eventType { case config.InfoEvent: // Skip if ErrorEvent is not configured for the resource - if !utils.CheckOperationAllowed(utils.AllowedEventKindsMap, objectMeta.Namespace, resource, config.ErrorEvent) { - log.Debugf("Ignoring %s to %s/%v in %s namespaces", eventType, resource, objectMeta.Name, objectMeta.Namespace) + if !c.shouldSendEvent(objectMeta.Namespace, resource, config.ErrorEvent) { + c.log.Debugf("Ignoring %q to %s/%v in %q namespace", eventType, resource, objectMeta.Name, objectMeta.Namespace) return } default: - if !utils.CheckOperationAllowed(utils.AllowedEventKindsMap, objectMeta.Namespace, resource, eventType) { - log.Debugf("Ignoring %s to %s/%v in %s namespaces", eventType, resource, objectMeta.Name, objectMeta.Namespace) + if !c.shouldSendEvent(objectMeta.Namespace, resource, eventType) { + c.log.Debugf("Ignoring %q to %s/%v in %q namespace", eventType, resource, objectMeta.Name, objectMeta.Namespace) return } } - log.Debugf("Processing %s to %s/%v in %s namespaces", eventType, resource, objectMeta.Name, objectMeta.Namespace) + c.log.Debugf("Processing %s to %s/%v in %s namespace", eventType, resource, objectMeta.Name, objectMeta.Namespace) // Check if Notify disabled if !config.Notify { - log.Debug("Skipping notification") + c.log.Debug("Skipping notification") return } // Create new event object - event := events.New(obj, eventType, resource, c.Settings.ClusterName) + event, err := events.New(objectMeta, obj, eventType, resource, c.conf.Settings.ClusterName) + if err != nil { + c.log.Errorf("while creating new event: %w", err) + return + } + // Skip older events if !event.TimeStamp.IsZero() { - if event.TimeStamp.Before(startTime) { - log.Debug("Skipping older events") + if event.TimeStamp.Before(c.startTime) { + c.log.Debug("Skipping older events") return } } @@ -193,22 +259,25 @@ func sendEvent(obj, oldObj interface{}, c *config.Config, notifiers []notify.Not if eventType == config.UpdateEvent { var updateMsg string // Check if all namespaces allowed - updateSetting, exist := utils.AllowedUpdateEventsMap[utils.KindNS{Resource: resource, Namespace: "all"}] + updateSetting, exist := c.observedUpdateEventsMap[KindNS{Resource: resource, Namespace: "all"}] if !exist { // Check if specified namespace is allowed - updateSetting, exist = utils.AllowedUpdateEventsMap[utils.KindNS{Resource: resource, Namespace: objectMeta.Namespace}] + updateSetting, exist = c.observedUpdateEventsMap[KindNS{Resource: resource, Namespace: objectMeta.Namespace}] } if exist { // Calculate object diff as per the updateSettings var oldUnstruct, newUnstruct *unstructured.Unstructured var ok bool if oldUnstruct, ok = oldObj.(*unstructured.Unstructured); !ok { - log.Errorf("Failed to typecast object to Unstructured. Skipping event: %#v", event) + c.log.Errorf("Failed to typecast object to Unstructured. Skipping event: %#v", event) } if newUnstruct, ok = obj.(*unstructured.Unstructured); !ok { - log.Errorf("Failed to typecast object to Unstructured. Skipping event: %#v", event) + c.log.Errorf("Failed to typecast object to Unstructured. Skipping event: %#v", event) + } + updateMsg, err = utils.Diff(oldUnstruct.Object, newUnstruct.Object, updateSetting) + if err != nil { + c.log.Errorf("while getting diff: %w", err) } - updateMsg = utils.Diff(oldUnstruct.Object, newUnstruct.Object, updateSetting) } // Send update notification only if fields in updateSetting are changed @@ -218,107 +287,170 @@ func sendEvent(obj, oldObj interface{}, c *config.Config, notifiers []notify.Not } } else { // skipping least significant update - log.Debug("skipping least significant Update event") + c.log.Debug("skipping least significant Update event") event.Skip = true } } // Filter events - event = filterengine.DefaultFilterEngine.Run(obj, event) + event = c.filterEngine.Run(ctx, obj, event) if event.Skip { - log.Debugf("Skipping event: %#v", event) + c.log.Debugf("Skipping event: %#v", event) return } // Skip unpromoted insignificant InfoEvents if event.Type == config.InfoEvent { - log.Debugf("Skipping Insignificant InfoEvent: %#v", event) + c.log.Debugf("Skipping Insignificant InfoEvent: %#v", event) return } if len(event.Kind) <= 0 { - log.Warn("sendEvent received event with Kind nil. Hence skipping.") + c.log.Warn("sendEvent received event with Kind nil. Hence skipping.") return } // check if Recommendations are disabled - if !c.Recommendations { + if !c.conf.Recommendations { event.Recommendations = nil - log.Debug("Skipping Recommendations in Event Notifications") + c.log.Debug("Skipping Recommendations in Event Notifications") } // Send event over notifiers - for _, n := range notifiers { + for _, n := range c.notifiers { go func(n notify.Notifier) { - err := n.SendEvent(event) + err := n.SendEvent(ctx, event) if err != nil { - log.Errorf("while sending event: %s", err.Error()) + c.log.Errorf("while sending event: %s", err.Error()) } }(n) } } -func sendMessage(notifiers []notify.Notifier, msg string) { - if len(msg) <= 0 { - log.Warn("sendMessage received string with length 0. Hence skipping.") - return - } - - // Send message over notifiers - for _, n := range notifiers { - go func(n notify.Notifier) { - err := n.SendMessage(msg) - if err != nil { - log.Errorf("while sending message: %s", err.Error()) - } - }(n) - } -} +func (c *Controller) initInformerMap() error { + // Create dynamic shared informer factory + c.dynamicKubeInformerFactory = dynamicinformer.NewDynamicSharedInformerFactory(c.dynamicCli, c.informersResyncPeriod) -func configWatcher(c *config.Config, notifiers []notify.Notifier) { - configPath := os.Getenv("CONFIG_PATH") - configFile := filepath.Join(configPath, config.ResourceConfigFileName) + // Init maps + c.resourceInformerMap = make(map[string]cache.SharedIndexInformer) + c.observedEventKindsMap = make(map[EventKind]bool) + c.observedUpdateEventsMap = make(map[KindNS]config.UpdateSetting) - watcher, err := fsnotify.NewWatcher() - if err != nil { - log.Fatal("Failed to create file watcher ", err) - } - defer func(watcher *fsnotify.Watcher) { - err := watcher.Close() + for _, v := range c.conf.Resources { + gvr, err := c.parseResourceArg(v.Name) if err != nil { - log.Errorf("while closing watcher: %s", err.Error()) + c.log.Infof("Unable to parse resource: %v\n", v.Name) + continue } - }(watcher) - - done := make(chan bool) - go func() { - for { - select { - case _, ok := <-watcher.Events: - if !ok { - log.Errorf("Error in getting events for config file:%s. Error: %s", configFile, err.Error()) - return + + c.resourceInformerMap[v.Name] = c.dynamicKubeInformerFactory.ForResource(gvr).Informer() + } + // Allowed event kinds map and Allowed Update Events Map + for _, r := range c.conf.Resources { + allEvents := false + for _, e := range r.Events { + if e == config.AllEvent { + allEvents = true + break + } + for _, ns := range r.Namespaces.Include { + c.observedEventKindsMap[EventKind{Resource: r.Name, Namespace: ns, EventType: e}] = true + } + // AllowedUpdateEventsMap entry is created only for UpdateEvent + if e == config.UpdateEvent { + for _, ns := range r.Namespaces.Include { + c.observedUpdateEventsMap[KindNS{Resource: r.Name, Namespace: ns}] = r.UpdateSetting } - log.Infof("Config file %s is updated. Hence restarting the Pod", configFile) - done <- true + } + } - case err, ok := <-watcher.Errors: - if !ok { - log.Errorf("Error in getting events for config file:%s. Error: %s", configFile, err.Error()) - return + // For AllEvent type, add all events to map + if allEvents { + events := []config.EventType{config.CreateEvent, config.UpdateEvent, config.DeleteEvent, config.ErrorEvent} + for _, ev := range events { + for _, ns := range r.Namespaces.Include { + c.observedEventKindsMap[EventKind{Resource: r.Name, Namespace: ns, EventType: ev}] = true + c.observedUpdateEventsMap[KindNS{Resource: r.Name, Namespace: ns}] = r.UpdateSetting } } } - }() - log.Infof("Registering watcher on configfile %s", configFile) - err = watcher.Add(configFile) + } + c.log.Infof("Allowed Events: %+v", c.observedEventKindsMap) + c.log.Infof("Allowed UpdateEvents: %+v", c.observedUpdateEventsMap) + return nil +} + +func (c *Controller) parseResourceArg(arg string) (schema.GroupVersionResource, error) { + gvr, err := c.strToGVR(arg) if err != nil { - log.Errorf("Unable to register watch on config file:%s. Error: %s", configFile, err.Error()) - return + return schema.GroupVersionResource{}, fmt.Errorf("while converting string to GroupVersionReference: %w", err) + } + + // Validate the GVR provided + if _, err := c.mapper.ResourcesFor(gvr); err != nil { + return schema.GroupVersionResource{}, err } - <-done - sendMessage(notifiers, fmt.Sprintf(configUpdateMsg, c.Settings.ClusterName)) - // Wait for Notifier to send message - time.Sleep(5 * time.Second) - os.Exit(0) + return gvr, nil +} + +func (c *Controller) strToGVR(arg string) (schema.GroupVersionResource, error) { + const separator = "/" + gvrStrParts := strings.Split(arg, separator) + switch len(gvrStrParts) { + case 2: + return schema.GroupVersionResource{Group: "", Version: gvrStrParts[0], Resource: gvrStrParts[1]}, nil + case 3: + return schema.GroupVersionResource{Group: gvrStrParts[0], Version: gvrStrParts[1], Resource: gvrStrParts[2]}, nil + default: + return schema.GroupVersionResource{}, fmt.Errorf("invalid string: expected 2 or 3 parts when split by %q", separator) + } +} + +func (c *Controller) shouldSendEvent(namespace string, resource string, eventType config.EventType) bool { + eventMap := c.observedEventKindsMap + if eventMap == nil { + return false + } + + if eventMap[EventKind{Resource: resource, Namespace: "all", EventType: eventType}] { + return true + } + + if eventMap[EventKind{Resource: resource, Namespace: namespace, EventType: eventType}] { + return true + } + + return false +} + +// TODO: These methods are used only for E2E test purposes. Remove them as a part of https://github.com/infracloudio/botkube/issues/589 + +// ShouldSendEvent exports Controller functionality for test purposes. +// Deprecated: This is a temporarily exposed part of internal functionality for testing purposes and shouldn't be used in production code. +func (c *Controller) ShouldSendEvent(namespace string, resource string, eventType config.EventType) bool { + return c.shouldSendEvent(namespace, resource, eventType) +} + +// ObservedEventKindsMap exports Controller functionality for test purposes. +// Deprecated: This is a temporarily exposed part of internal functionality for testing purposes and shouldn't be used in production code. +func (c *Controller) ObservedEventKindsMap() map[EventKind]bool { + return c.observedEventKindsMap +} + +// SetObservedEventKindsMap exports Controller functionality for test purposes. +// Deprecated: This is a temporarily exposed part of internal functionality for testing purposes and shouldn't be used in production code. +func (c *Controller) SetObservedEventKindsMap(observedEventKindsMap map[EventKind]bool) { + c.observedEventKindsMap = observedEventKindsMap +} + +// ObservedUpdateEventsMap exports Controller functionality for test purposes. +// Deprecated: This is a temporarily exposed part of internal functionality for testing purposes and shouldn't be used in production code. +func (c *Controller) ObservedUpdateEventsMap() map[KindNS]config.UpdateSetting { + return c.observedUpdateEventsMap +} + +// SetObservedUpdateEventsMap exports Controller functionality for test purposes. +// Deprecated: This is a temporarily exposed part of internal functionality for testing purposes and shouldn't be used in production code. +func (c *Controller) SetObservedUpdateEventsMap(observedUpdateEventsMap map[KindNS]config.UpdateSetting) { + c.observedUpdateEventsMap = observedUpdateEventsMap } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go new file mode 100644 index 000000000..1aa582009 --- /dev/null +++ b/pkg/controller/controller_test.go @@ -0,0 +1,201 @@ +package controller + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/utils" + testutils "github.com/infracloudio/botkube/test/e2e/utils" +) + +// TODO: Tests moved out straight from E2E test package with minimal changes. +// Refactor them as a part of https://github.com/infracloudio/botkube/issues/589 + +func TestController_ShouldSendEvent_SkipError(t *testing.T) { + observedEventKindsMap := map[EventKind]bool{ + {Resource: "v1/pods", Namespace: "dummy", EventType: "error"}: true, + } + + tests := map[string]testutils.ErrorEvent{ + "skip error event for resources not configured": { + // error event not allowed for Pod resources so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Kind: "Pod", + Namespace: "test", + Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, + }, + "skip error event for namespace not configured": { + // error event not allowed for Pod in test namespace so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Kind: "Pod", + Namespace: "test", + Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, + }, + "skip error event for resources not added in test_config": { + // error event should not be allowed for service resource so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "services"}, + Kind: "Service", + Namespace: "test", + Specs: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test-service-error"}}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + resource := utils.GVRToString(test.GVR) + + c := Controller{ + observedEventKindsMap: observedEventKindsMap, + } + + isAllowed := c.shouldSendEvent(test.Namespace, resource, config.ErrorEvent) + assert.Equal(t, false, isAllowed) + }) + } +} + +func TestController_ShouldSendEvent_SkipUpdate(t *testing.T) { + observedEventKindsMap := map[EventKind]bool{ + {Resource: "v1/pods", Namespace: "dummy", EventType: "delete"}: true, + } + + // test scenarios + tests := map[string]testutils.UpdateObjects{ + "skip update event for namespaces not configured": { + // update operation not allowed for Pod in test namespace so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Kind: "Pod", + Namespace: "test", + Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-update"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, + }, + "skip update event for resources not added": { + // update operation not allowed for namespaces in test_config so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}, + Kind: "Namespace", + Specs: &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "abc"}}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + resource := utils.GVRToString(test.GVR) + + c := Controller{ + observedEventKindsMap: observedEventKindsMap, + } + + isAllowed := c.shouldSendEvent(test.Namespace, resource, config.ErrorEvent) + assert.Equal(t, false, isAllowed) + }) + } +} + +func TestController_ShouldSendEvent_SkipDelete(t *testing.T) { + observedEventKindsMap := map[EventKind]bool{ + {Resource: "v1/pods", Namespace: "dummy", EventType: "delete"}: true, + } + + // test scenarios + tests := map[string]testutils.DeleteObjects{ + "skip delete event for resources not configured": { + // delete operation not allowed for Pod resources so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Kind: "Pod", + Namespace: "test", + Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-delete"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, + }, + "skip delete event for namespace not configured": { + // delete operation not allowed for Pod in test namespace so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Kind: "Pod", + Namespace: "test", + Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-delete"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, + }, + "skip delete event for resources not added in test_config": { + // delete operation not allowed for Pod resources so event should be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "services"}, + Kind: "Service", + Namespace: "test", + Specs: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test-service-delete"}}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + resource := utils.GVRToString(test.GVR) + + c := Controller{ + observedEventKindsMap: observedEventKindsMap, + } + + isAllowed := c.shouldSendEvent(test.Namespace, resource, config.ErrorEvent) + assert.Equal(t, false, isAllowed) + }) + } +} + +func TestController_strToGVR(t *testing.T) { + // test scenarios + tests := []struct { + Name string + Input string + Expected schema.GroupVersionResource + ExpectedErrMessage string + }{ + { + Name: "Without group", + Input: "v1/persistentvolumes", + Expected: schema.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "persistentvolumes", + }, + }, + { + Name: "With group", + Input: "apps/v1/daemonsets", + Expected: schema.GroupVersionResource{ + Group: "apps", + Version: "v1", + Resource: "daemonsets", + }, + }, + { + Name: "With more complex group", + Input: "rbac.authorization.k8s.io/v1/clusterroles", + Expected: schema.GroupVersionResource{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Resource: "clusterroles", + }, + }, + { + Name: "Error", + Input: "foo/bar/baz/qux", + ExpectedErrMessage: "invalid string: expected 2 or 3 parts when split by \"/\"", + }, + } + + for _, testCase := range tests { + t.Run(testCase.Name, func(t *testing.T) { + c := Controller{} + + res, err := c.strToGVR(testCase.Input) + + if testCase.ExpectedErrMessage != "" { + require.Error(t, err) + assert.EqualError(t, err, testCase.ExpectedErrMessage) + return + } + + require.NoError(t, err) + assert.Equal(t, testCase.Expected, res) + }) + } +} diff --git a/pkg/controller/message.go b/pkg/controller/message.go new file mode 100644 index 000000000..3300d6b53 --- /dev/null +++ b/pkg/controller/message.go @@ -0,0 +1,24 @@ +package controller + +import ( + "context" + "fmt" + + "github.com/infracloudio/botkube/pkg/notify" +) + +func sendMessageToNotifiers(ctx context.Context, notifiers []notify.Notifier, msg string) error { + if msg == "" { + return fmt.Errorf("message cannot be empty") + } + + // Send message over notifiers + for _, n := range notifiers { + err := n.SendMessage(ctx, msg) + if err != nil { + return fmt.Errorf("while sending message: %w", err) + } + } + + return nil +} diff --git a/pkg/controller/upgrade.go b/pkg/controller/upgrade.go index 2d93af6d9..42d717000 100644 --- a/pkg/controller/upgrade.go +++ b/pkg/controller/upgrade.go @@ -21,53 +21,100 @@ package controller import ( "context" + "errors" "fmt" "time" "github.com/google/go-github/v44/github" + "github.com/sirupsen/logrus" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/notify" "github.com/infracloudio/botkube/pkg/version" ) -var ( - notified = false - botkubeUpgradeMsg = "Newer version (%s) of BotKube is available :tada:. Please upgrade BotKube backend.\n" + - "Visit botkube.io for more info." +const ( + defaultDuration = 24 * time.Hour + upgradeMsgFmt = "Newer version (%s) of BotKube is available :tada:. Please upgrade BotKube backend.\nVisit botkube.io for more info." + repoOwner = "infracloudio" + repoName = "botkube" ) -func checkRelease(notifiers []notify.Notifier) { - ctx := context.Background() - client := github.NewClient(nil) - release, _, err := client.Repositories.GetLatestRelease(ctx, "infracloudio", "botkube") - if err == nil { - log.Debugf(fmt.Sprintf("Upgrade notifier:: latest release info=%+v", release)) - if release.TagName == nil { - return - } +// GitHubRepoClient describes the client for getting latest release for a given repository. +type GitHubRepoClient interface { + GetLatestRelease(ctx context.Context, owner, repo string) (*github.RepositoryRelease, *github.Response, error) +} - // Send notification if newer version available - if version.Short() != *release.TagName { - sendMessage(notifiers, fmt.Sprintf(botkubeUpgradeMsg, *release.TagName)) - notified = true - } - } +// UpgradeChecker checks for new BotKube releases. +type UpgradeChecker struct { + log logrus.FieldLogger + notifiers []notify.Notifier + ghRepoCli GitHubRepoClient +} + +// NewUpgradeChecker creates a new instance of the Upgrade Checker. +func NewUpgradeChecker(log logrus.FieldLogger, notifiers []notify.Notifier, ghCli GitHubRepoClient) *UpgradeChecker { + return &UpgradeChecker{log: log, notifiers: notifiers, ghRepoCli: ghCli} } -// UpgradeNotifier checks if newer version for BotKube backend available and notifies user -func UpgradeNotifier(notifiers []notify.Notifier) { +// Run runs the Upgrade Checker and checks for new BotKube releases periodically. +func (c *UpgradeChecker) Run(ctx context.Context) error { + c.log.Info("Starting checker") // Check at startup - checkRelease(notifiers) - ticker := time.NewTicker(24 * time.Hour) + notified, err := c.notifyAboutUpgradeIfShould(ctx) + if err != nil { + return fmt.Errorf("while notifying about upgrade if should: %w", err) + } + + if notified { + return nil + } + + ticker := time.NewTicker(defaultDuration) defer ticker.Stop() for { - <-ticker.C - if notified { - return + select { + case <-ctx.Done(): + c.log.Info("Shutdown requested. Finishing...") + return nil + case <-ticker.C: + // Check periodically + notified, err := c.notifyAboutUpgradeIfShould(ctx) + if err != nil { + wrappedErr := fmt.Errorf("while notifying about upgrade if should: %w", err) + c.log.Error(wrappedErr.Error()) + } + + if notified { + return nil + } } - // Check periodically - checkRelease(notifiers) } } + +func (c *UpgradeChecker) notifyAboutUpgradeIfShould(ctx context.Context) (bool, error) { + client := github.NewClient(nil) + release, _, err := client.Repositories.GetLatestRelease(ctx, repoOwner, repoName) + if err != nil { + return false, fmt.Errorf("while getting latest release from GitHub: %w", err) + } + + c.log.Debugf("latest release info: %+v", release) + if release.TagName == nil { + return false, errors.New("release tag is empty") + } + + // Send notification if newer version available + if version.Short() == *release.TagName { + // no new version, finish + return false, nil + } + + err = sendMessageToNotifiers(ctx, c.notifiers, fmt.Sprintf(upgradeMsgFmt, *release.TagName)) + if err != nil { + return false, fmt.Errorf("while sending message about new release: %w", err) + } + + c.log.Infof("Notified about new release %q. Finishing...", *release.TagName) + return true, nil +} diff --git a/pkg/events/events.go b/pkg/events/events.go index ff9d34949..c779697f4 100644 --- a/pkg/events/events.go +++ b/pkg/events/events.go @@ -21,16 +21,15 @@ package events import ( "fmt" - "reflect" "strings" "time" - "github.com/infracloudio/botkube/pkg/config" - log "github.com/infracloudio/botkube/pkg/log" - "github.com/infracloudio/botkube/pkg/utils" - coreV1 "k8s.io/api/core/v1" + metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/utils" ) // Event to store required information from k8s objects @@ -67,10 +66,8 @@ var LevelMap = map[config.EventType]config.Level{ } // New extract required details from k8s object and returns new Event object -func New(object interface{}, eventType config.EventType, resource, clusterName string) Event { +func New(objectMeta metaV1.ObjectMeta, object interface{}, eventType config.EventType, resource, clusterName string) (Event, error) { objectTypeMeta := utils.GetObjectTypeMetaData(object) - objectMeta := utils.GetObjectMetaData(object) - event := Event{ Name: objectMeta.Name, Namespace: objectMeta.Namespace, @@ -107,10 +104,17 @@ func New(object interface{}, eventType config.EventType, resource, clusterName s if objectTypeMeta.Kind == "Event" { var eventObj coreV1.Event - err := utils.TransformIntoTypedObject(object.(*unstructured.Unstructured), &eventObj) + + unstrObj, ok := object.(*unstructured.Unstructured) + if !ok { + return Event{}, fmt.Errorf("cannot convert type %T into *unstructured.Unstructured", object) + } + + err := utils.TransformIntoTypedObject(unstrObj, &eventObj) if err != nil { - log.Errorf("Unable to transform object type: %v, into type: %v", reflect.TypeOf(object), reflect.TypeOf(eventObj)) + return Event{}, fmt.Errorf("while transforming object type %T into type: %T: %w", object, eventObj, err) } + event.Reason = eventObj.Reason event.Messages = append(event.Messages, eventObj.Message) event.Kind = eventObj.InvolvedObject.Kind @@ -126,5 +130,6 @@ func New(object interface{}, eventType config.EventType, resource, clusterName s event.Count = eventObj.Series.Count } } - return event + + return event, nil } diff --git a/pkg/execute/executor.go b/pkg/execute/executor.go index ce8ad0ceb..5477be18a 100644 --- a/pkg/execute/executor.go +++ b/pkg/execute/executor.go @@ -22,16 +22,15 @@ package execute import ( "bytes" "fmt" - "reflect" "strings" "text/tabwriter" "unicode" + "github.com/sirupsen/logrus" "gopkg.in/yaml.v3" "github.com/infracloudio/botkube/pkg/config" - filterengine "github.com/infracloudio/botkube/pkg/filterengine" - "github.com/infracloudio/botkube/pkg/log" + "github.com/infracloudio/botkube/pkg/filterengine" "github.com/infracloudio/botkube/pkg/utils" "github.com/infracloudio/botkube/pkg/version" ) @@ -87,22 +86,17 @@ const ( teamsUnsupportedCmdMsg = "Command not supported. Please visit botkube.io/usage to see supported commands." ) -// Executor is an interface for processes to execute commands -type Executor interface { - Execute() string -} - // DefaultExecutor is a default implementations of Executor type DefaultExecutor struct { - Platform config.BotPlatform - Message string - AllowKubectl bool - RestrictAccess bool - ClusterName string - ChannelName string - IsAuthChannel bool - DefaultNamespace string - runCmdFn CommandRunnerFunc + cfg config.Config + filterEngine filterengine.FilterEngine + log logrus.FieldLogger + runCmdFn CommandRunnerFunc + resMapping ResourceMapping + + Message string + IsAuthChannel bool + Platform config.BotPlatform } // CommandRunnerFunc is a function which runs arbitrary commands @@ -161,77 +155,54 @@ func (action FiltersAction) String() string { return string(action) } -// TODO: Refactor constructors - -// NewDefaultExecutor returns new Executor object -func NewDefaultExecutor(msg string, allowKubectl, restrictAccess bool, defaultNamespace, - clusterName string, platform config.BotPlatform, channelName string, isAuthChannel bool) Executor { - return NewExecutorWithCustomCommandRunner(msg, allowKubectl, restrictAccess, defaultNamespace, clusterName, platform, channelName, isAuthChannel, DefaultCommandRunnerFunc) -} - -// NewExecutorWithCustomCommandRunner returns new Executor object -// msg should not contain the BotId -func NewExecutorWithCustomCommandRunner(msg string, allowKubectl, restrictAccess bool, defaultNamespace, - clusterName string, platform config.BotPlatform, channelName string, isAuthChannel bool, runCmdFn CommandRunnerFunc) Executor { - return &DefaultExecutor{ - Platform: platform, - Message: msg, - AllowKubectl: allowKubectl, - RestrictAccess: restrictAccess, - ClusterName: clusterName, - ChannelName: channelName, - IsAuthChannel: isAuthChannel, - DefaultNamespace: defaultNamespace, - runCmdFn: runCmdFn, - } -} - // Execute executes commands and returns output func (e *DefaultExecutor) Execute() string { + clusterName := e.cfg.Settings.ClusterName // Remove hyperlink if it got added automatically command := utils.RemoveHyperlink(e.Message) args := strings.Fields(strings.TrimSpace(command)) if len(args) == 0 { if e.IsAuthChannel { - return printDefaultMsg(e.Platform) + return e.printDefaultMsg(e.Platform) } return "" // this prevents all bots on all clusters to answer something } - if len(args) >= 1 && utils.AllowedKubectlVerbMap[args[0]] { + if len(args) >= 1 && e.resMapping.AllowedKubectlVerbMap[args[0]] { if validDebugCommands[args[0]] || // Don't check for resource if is a valid debug command - (len(args) >= 2 && (utils.AllowedKubectlResourceMap[args[1]] || // Check if allowed resource - utils.AllowedKubectlResourceMap[utils.KindResourceMap[strings.ToLower(args[1])]] || // Check if matches with kind name - utils.AllowedKubectlResourceMap[utils.ShortnameResourceMap[strings.ToLower(args[1])]])) { // Check if matches with short name + (len(args) >= 2 && (e.resMapping.AllowedKubectlResourceMap[args[1]] || // Check if allowed resource + e.resMapping.AllowedKubectlResourceMap[e.resMapping.KindResourceMap[strings.ToLower(args[1])]] || // Check if matches with kind name + e.resMapping.AllowedKubectlResourceMap[e.resMapping.ShortnameResourceMap[strings.ToLower(args[1])]])) { // Check if matches with short name isClusterNamePresent := strings.Contains(e.Message, "--cluster-name") - if !e.AllowKubectl { - if isClusterNamePresent && e.ClusterName == utils.GetClusterNameFromKubectlCmd(e.Message) { - return fmt.Sprintf(kubectlDisabledMsg, e.ClusterName) + allowKubectl := e.cfg.Settings.Kubectl.Enabled + if !allowKubectl { + if isClusterNamePresent && clusterName == utils.GetClusterNameFromKubectlCmd(e.Message) { + return fmt.Sprintf(kubectlDisabledMsg, clusterName) } return "" } - if e.RestrictAccess && !e.IsAuthChannel && isClusterNamePresent { + if e.cfg.Settings.Kubectl.RestrictAccess && !e.IsAuthChannel && isClusterNamePresent { return "" } return e.runKubectlCommand(args) } } if ValidNotifierCommand[args[0]] { - return e.runNotifierCommand(args, e.ClusterName, e.IsAuthChannel) + return e.runNotifierCommand(args, clusterName, e.IsAuthChannel) } if validPingCommand[args[0]] { - res := e.runVersionCommand(args, e.ClusterName) + res := e.runVersionCommand(args, clusterName) if len(res) == 0 { return "" } - return fmt.Sprintf("pong from cluster '%s'", e.ClusterName) + "\n\n" + res + return fmt.Sprintf("pong from cluster '%s'\n\n%s", clusterName, res) } if validVersionCommand[args[0]] { - return e.runVersionCommand(args, e.ClusterName) + return e.runVersionCommand(args, clusterName) } // Check if filter command if validFilterCommand[args[0]] { - return e.runFilterCommand(args, e.ClusterName, e.IsAuthChannel) + return e.runFilterCommand(args, clusterName, e.IsAuthChannel) } //Check if info command @@ -240,12 +211,12 @@ func (e *DefaultExecutor) Execute() string { } if e.IsAuthChannel { - return printDefaultMsg(e.Platform) + return e.printDefaultMsg(e.Platform) } return "" } -func printDefaultMsg(p config.BotPlatform) string { +func (e *DefaultExecutor) printDefaultMsg(p config.BotPlatform) string { if p == config.TeamsBot { return teamsUnsupportedCmdMsg } @@ -253,7 +224,7 @@ func printDefaultMsg(p config.BotPlatform) string { } // Trim single and double quotes from ends of string -func trimQuotes(clusterValue string) string { +func (e *DefaultExecutor) trimQuotes(clusterValue string) string { return strings.TrimFunc(clusterValue, func(r rune) bool { if r == unicode.SimpleFold('\u0027') || r == unicode.SimpleFold('\u0022') { return true @@ -263,8 +234,8 @@ func trimQuotes(clusterValue string) string { } func (e *DefaultExecutor) runKubectlCommand(args []string) string { - clusterName := e.ClusterName - defaultNamespace := e.DefaultNamespace + clusterName := e.cfg.Settings.ClusterName + defaultNamespace := e.cfg.Settings.Kubectl.DefaultNamespace isAuthChannel := e.IsAuthChannel // run commands in namespace specified under Config.Settings.DefaultNamespace field if !utils.Contains(args, "-n") && !utils.Contains(args, "--namespace") && len(defaultNamespace) != 0 { @@ -272,7 +243,7 @@ func (e *DefaultExecutor) runKubectlCommand(args []string) string { } // Remove unnecessary flags - finalArgs := []string{} + var finalArgs []string isClusterNameArg := false for index, arg := range args { if isClusterNameArg { @@ -289,12 +260,12 @@ func (e *DefaultExecutor) runKubectlCommand(args []string) string { if strings.HasPrefix(arg, ClusterFlag.String()) { // Check if flag value in current or next argument and compare with config.settings.clustername if arg == ClusterFlag.String() { - if index == len(args)-1 || trimQuotes(args[index+1]) != clusterName { + if index == len(args)-1 || e.trimQuotes(args[index+1]) != clusterName { return "" } isClusterNameArg = true } else { - if trimQuotes(strings.SplitAfterN(arg, ClusterFlag.String()+"=", 2)[1]) != clusterName { + if e.trimQuotes(strings.SplitAfterN(arg, ClusterFlag.String()+"=", 2)[1]) != clusterName { return "" } } @@ -309,7 +280,7 @@ func (e *DefaultExecutor) runKubectlCommand(args []string) string { // Get command runner out, err := e.runCmdFn(kubectlBinary, finalArgs) if err != nil { - log.Error("Error in executing kubectl command: ", err) + e.log.Error("Error in executing kubectl command: ", err) return fmt.Sprintf("Cluster: %s\n%s", clusterName, out+err.Error()) } return fmt.Sprintf("Cluster: %s\n%s", clusterName, out) @@ -327,11 +298,11 @@ func (e *DefaultExecutor) runNotifierCommand(args []string, clusterName string, switch args[1] { case Start.String(): config.Notify = true - log.Info("Notifier enabled") + e.log.Info("Notifier enabled") return fmt.Sprintf(NotifierStartMsg, clusterName) case Stop.String(): config.Notify = false - log.Info("Notifier disabled") + e.log.Info("Notifier disabled") return fmt.Sprintf(notifierStopMsg, clusterName) case Status.String(): if !config.Notify { @@ -339,14 +310,14 @@ func (e *DefaultExecutor) runNotifierCommand(args []string, clusterName string, } return fmt.Sprintf("Notifications are on for cluster '%s'", clusterName) case ShowConfig.String(): - out, err := showControllerConfig() + out, err := e.showControllerConfig() if err != nil { - log.Error("Error in executing showconfig command: ", err) + e.log.Error("Error in executing showconfig command: ", err) return "Error in getting configuration!" } return fmt.Sprintf("Showing config for cluster '%s'\n\n%s", clusterName, out) } - return printDefaultMsg(e.Platform) + return e.printDefaultMsg(e.Platform) } // runFilterCommand to list, enable or disable filters @@ -360,16 +331,16 @@ func (e *DefaultExecutor) runFilterCommand(args []string, clusterName string, is switch args[1] { case FilterList.String(): - log.Debug("List filters") - return makeFiltersList() + e.log.Debug("List filters") + return e.makeFiltersList() // Enable filter case FilterEnable.String(): if len(args) < 3 { - return fmt.Sprintf(filterNameMissing, makeFiltersList()) + return fmt.Sprintf(filterNameMissing, e.makeFiltersList()) } - log.Debug("Enable filters", args[2]) - if err := filterengine.DefaultFilterEngine.SetFilter(args[2], true); err != nil { + e.log.Debug("Enable filters", args[2]) + if err := e.filterEngine.SetFilter(args[2], true); err != nil { return err.Error() } return fmt.Sprintf(filterEnabled, args[2], clusterName) @@ -377,15 +348,15 @@ func (e *DefaultExecutor) runFilterCommand(args []string, clusterName string, is // Disable filter case FilterDisable.String(): if len(args) < 3 { - return fmt.Sprintf(filterNameMissing, makeFiltersList()) + return fmt.Sprintf(filterNameMissing, e.makeFiltersList()) } - log.Debug("Disabled filters", args[2]) - if err := filterengine.DefaultFilterEngine.SetFilter(args[2], false); err != nil { + e.log.Debug("Disabled filters", args[2]) + if err := e.filterEngine.SetFilter(args[2], false); err != nil { return err.Error() } return fmt.Sprintf(filterDisabled, args[2], clusterName) } - return printDefaultMsg(e.Platform) + return e.printDefaultMsg(e.Platform) } //runInfoCommand to list allowed commands @@ -397,28 +368,25 @@ func (e *DefaultExecutor) runInfoCommand(args []string, isAuthChannel bool) stri return IncompleteCmdMsg } - if len(args) > 3 && args[2] == ClusterFlag.String() && args[3] != e.ClusterName { + clusterName := e.cfg.Settings.ClusterName + if len(args) > 3 && args[2] == ClusterFlag.String() && args[3] != clusterName { return fmt.Sprintf(WrongClusterCmdMsg, args[3]) } - return makeCommandInfoList() -} - -func makeCommandInfoList() string { - allowedVerbs := utils.GetStringInYamlFormat("allowed verbs:", utils.AllowedKubectlVerbMap) - allowedResources := utils.GetStringInYamlFormat("allowed resources:", utils.AllowedKubectlResourceMap) + allowedVerbs := utils.GetStringInYamlFormat("allowed verbs:", e.resMapping.AllowedKubectlVerbMap) + allowedResources := utils.GetStringInYamlFormat("allowed resources:", e.resMapping.AllowedKubectlResourceMap) return allowedVerbs + allowedResources } // Use tabwriter to display string in tabular form // https://golang.org/pkg/text/tabwriter -func makeFiltersList() string { +func (e *DefaultExecutor) makeFiltersList() string { buf := new(bytes.Buffer) w := tabwriter.NewWriter(buf, 5, 0, 1, ' ', 0) fmt.Fprintln(w, "FILTER\tENABLED\tDESCRIPTION") - for k, v := range filterengine.DefaultFilterEngine.ShowFilters() { - fmt.Fprintf(w, "%s\t%v\t%s\n", reflect.TypeOf(k).Name(), v, k.Describe()) + for k, v := range e.filterEngine.ShowFilters() { + fmt.Fprintf(w, "%s\t%v\t%s\n", k.Name(), v, k.Describe()) } w.Flush() @@ -430,7 +398,7 @@ func (e *DefaultExecutor) findBotKubeVersion() (versions string) { // Returns "Server Version: xxxx" k8sVersion, err := e.runCmdFn("sh", args) if err != nil { - log.Warn(fmt.Sprintf("Failed to get Kubernetes version: %s", err.Error())) + e.log.Warn(fmt.Sprintf("Failed to get Kubernetes version: %s", err.Error())) k8sVersion = "Server Version: Unknown\n" } @@ -463,21 +431,25 @@ func (e *DefaultExecutor) runVersionCommand(args []string, clusterName string) s return e.findBotKubeVersion() } -func showControllerConfig() (configYaml string, err error) { - c, err := config.New() - if err != nil { - log.Fatal(fmt.Sprintf("Error in loading configuration. Error:%s", err.Error())) - } +const redactedSecretStr = "*** REDACTED ***" - // hide sensitive info - c.Communications.Slack.Token = "" - c.Communications.ElasticSearch.Password = "" +func (e *DefaultExecutor) showControllerConfig() (string, error) { + cfg := e.cfg - b, err := yaml.Marshal(c) + // hide sensitive info + // TODO: Refactor - split config into two files and avoid printing sensitive data + // without need to resetting them manually (which is an error-prone approach) + cfg.Communications.Slack.Token = redactedSecretStr + cfg.Communications.ElasticSearch.Password = redactedSecretStr + cfg.Communications.Discord.Token = redactedSecretStr + cfg.Communications.Mattermost.Token = redactedSecretStr + cfg.Communications.Lark.AppSecret = redactedSecretStr + cfg.Communications.Lark.EncryptKey = redactedSecretStr + + b, err := yaml.Marshal(cfg) if err != nil { - return configYaml, err + return "", err } - configYaml = string(b) - return configYaml, nil + return string(b), nil } diff --git a/pkg/execute/factory.go b/pkg/execute/factory.go new file mode 100644 index 000000000..39c8f851b --- /dev/null +++ b/pkg/execute/factory.go @@ -0,0 +1,54 @@ +package execute + +import ( + "github.com/sirupsen/logrus" + + "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/filterengine" +) + +// DefaultExecutorFactory facilitates creation of the Executor instances. +type DefaultExecutorFactory struct { + log logrus.FieldLogger + runCmdFn CommandRunnerFunc + cfg config.Config + filterEngine filterengine.FilterEngine + resMapping ResourceMapping +} + +// Executor is an interface for processes to execute commands +type Executor interface { + Execute() string +} + +// NewExecutorFactory creates new DefaultExecutorFactory. +func NewExecutorFactory( + log logrus.FieldLogger, + runCmdFn CommandRunnerFunc, + cfg config.Config, + filterEngine filterengine.FilterEngine, + resMapping ResourceMapping, +) *DefaultExecutorFactory { + return &DefaultExecutorFactory{ + log: log, + runCmdFn: runCmdFn, + cfg: cfg, + filterEngine: filterEngine, + resMapping: resMapping, + } +} + +// NewDefault creates new Default Executor. +func (f *DefaultExecutorFactory) NewDefault(platform config.BotPlatform, isAuthChannel bool, message string) Executor { + return &DefaultExecutor{ + log: f.log, + runCmdFn: f.runCmdFn, + cfg: f.cfg, + resMapping: f.resMapping, + + filterEngine: f.filterEngine, + IsAuthChannel: isAuthChannel, + Message: message, + Platform: platform, + } +} diff --git a/pkg/execute/resource.go b/pkg/execute/resource.go new file mode 100644 index 000000000..24be6207d --- /dev/null +++ b/pkg/execute/resource.go @@ -0,0 +1,61 @@ +package execute + +import ( + "fmt" + "strings" + + "github.com/sirupsen/logrus" + "k8s.io/client-go/discovery" + + "github.com/infracloudio/botkube/pkg/config" +) + +// ResourceMapping contains helper maps for kubectl execution. +type ResourceMapping struct { + KindResourceMap map[string]string + ShortnameResourceMap map[string]string + AllowedKubectlResourceMap map[string]bool + AllowedKubectlVerbMap map[string]bool +} + +// LoadResourceMappingIfShould initializes helper maps to allow kubectl execution for required resources. +// If Kubectl support is disabled, it returns empty ResourceMapping without an error. +func LoadResourceMappingIfShould(log logrus.FieldLogger, conf *config.Config, discoveryCli discovery.DiscoveryInterface) (ResourceMapping, error) { + if !conf.Settings.Kubectl.Enabled { + log.Infof("Kubectl disabled. Finishing...") + return ResourceMapping{}, nil + } + + resMapping := ResourceMapping{ + KindResourceMap: make(map[string]string), + ShortnameResourceMap: make(map[string]string), + AllowedKubectlResourceMap: make(map[string]bool), + AllowedKubectlVerbMap: make(map[string]bool), + } + + for _, r := range conf.Settings.Kubectl.Commands.Resources { + resMapping.AllowedKubectlResourceMap[r] = true + } + for _, r := range conf.Settings.Kubectl.Commands.Verbs { + resMapping.AllowedKubectlVerbMap[r] = true + } + + _, resourceList, err := discoveryCli.ServerGroupsAndResources() + if err != nil { + return ResourceMapping{}, fmt.Errorf("while getting resource list from K8s cluster: %w", err) + } + for _, resource := range resourceList { + for _, r := range resource.APIResources { + // Ignore subresources + if strings.Contains(r.Name, "/") { + continue + } + resMapping.KindResourceMap[strings.ToLower(r.Kind)] = r.Name + for _, sn := range r.ShortNames { + resMapping.ShortnameResourceMap[sn] = r.Name + } + } + } + log.Infof("Loaded resource mapping: %+v", resMapping) + return resMapping, nil +} diff --git a/pkg/filterengine/filterengine.go b/pkg/filterengine/filterengine.go index 381d2feee..5337ac504 100644 --- a/pkg/filterengine/filterengine.go +++ b/pkg/filterengine/filterengine.go @@ -20,71 +20,83 @@ package filterengine import ( + "context" "fmt" - "reflect" + + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) +// DefaultFilterEngine is a default implementation of the Filter Engine +type DefaultFilterEngine struct { + log logrus.FieldLogger + + // TODO: Change map key to filter name to be able to SetFilter without iterating through the whole map + FiltersMap map[Filter]bool +} + // FilterEngine has methods to register and run filters type FilterEngine interface { - Run(interface{}, events.Event) events.Event + Run(context.Context, interface{}, events.Event) events.Event Register(...Filter) ShowFilters() map[Filter]bool SetFilter(string, bool) error } -type defaultFilters struct { - FiltersMap map[Filter]bool -} - // Filter has method to run filter type Filter interface { - Run(interface{}, *events.Event) + Run(context.Context, interface{}, *events.Event) error + Name() string Describe() string } -// NewDefaultFilter creates new DefaultFilter object -func NewDefaultFilter() FilterEngine { - var df defaultFilters - df.FiltersMap = make(map[Filter]bool) - return &df +// New creates new DefaultFilterEngine object +func New(log logrus.FieldLogger) *DefaultFilterEngine { + return &DefaultFilterEngine{ + log: log, + FiltersMap: make(map[Filter]bool), + } } -// Run run the filters -func (f *defaultFilters) Run(object interface{}, event events.Event) events.Event { - log.Debug("Filterengine running filters") - // Run registered filters - for k, v := range f.FiltersMap { - if v { - k.Run(object, &event) +// Run runs the registered filters +func (f *DefaultFilterEngine) Run(ctx context.Context, object interface{}, event events.Event) events.Event { + f.log.Debug("Running registered filters") + for filter, enabled := range f.FiltersMap { + if !enabled { + continue + } + + err := filter.Run(ctx, object, &event) + if err != nil { + f.log.Errorf("while running filter %q: %w", filter.Name(), err) } } return event } // Register filter(s) to engine -func (f *defaultFilters) Register(filters ...Filter) { +func (f *DefaultFilterEngine) Register(filters ...Filter) { for _, filter := range filters { - log.Infof("Registering filter %q", reflect.TypeOf(filter).Name()) + f.log.Infof("Registering filter %q", filter.Name()) f.FiltersMap[filter] = true } } // ShowFilters return map of filter name and status -func (f defaultFilters) ShowFilters() map[Filter]bool { +// TODO: This method is used for printing filters, but the order of the list is not preserved. Fix it. +func (f DefaultFilterEngine) ShowFilters() map[Filter]bool { return f.FiltersMap } // SetFilter sets filter value in FilterMap to enable or disable filter -func (f *defaultFilters) SetFilter(name string, flag bool) error { +func (f *DefaultFilterEngine) SetFilter(name string, flag bool) error { // Find filter struct name for k := range f.FiltersMap { - if reflect.TypeOf(k).Name() == name { + if k.Name() == name { f.FiltersMap[k] = flag return nil } } - return fmt.Errorf("Invalid filter name %s", name) + return fmt.Errorf("couldn't find filter with name %q", name) } diff --git a/pkg/filterengine/filters/image_tag_checker.go b/pkg/filterengine/filters/image_tag_checker.go index b68f9a564..9ee9bb52f 100644 --- a/pkg/filterengine/filters/image_tag_checker.go +++ b/pkg/filterengine/filters/image_tag_checker.go @@ -20,33 +20,38 @@ package filters import ( + "context" "fmt" - "reflect" "strings" + "github.com/sirupsen/logrus" + coreV1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/utils" - - coreV1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) // ImageTagChecker add recommendations to the event object if latest image tag is used in pod containers type ImageTagChecker struct { - Description string + log logrus.FieldLogger +} + +// NewImageTagChecker creates a new ImageTagChecker instance +func NewImageTagChecker(log logrus.FieldLogger) *ImageTagChecker { + return &ImageTagChecker{log: log} } // Run filers and modifies event struct -func (f ImageTagChecker) Run(object interface{}, event *events.Event) { +func (f *ImageTagChecker) Run(_ context.Context, object interface{}, event *events.Event) error { if event.Kind != "Pod" || event.Type != config.CreateEvent || utils.GetObjectTypeMetaData(object).Kind == "Event" { - return + return nil } var podObj coreV1.Pod err := utils.TransformIntoTypedObject(object.(*unstructured.Unstructured), &podObj) if err != nil { - log.Errorf("Unable to transform object type: %v, into type: %v", reflect.TypeOf(object), reflect.TypeOf(podObj)) + return fmt.Errorf("while transforming object type %T into type: %T: %w", object, podObj, err) } // Check image tag in initContainers @@ -64,10 +69,16 @@ func (f ImageTagChecker) Run(object interface{}, event *events.Event) { event.Recommendations = append(event.Recommendations, fmt.Sprintf(":latest tag used in image '%s' of Container '%s' should be avoided.", c.Image, c.Name)) } } - log.Debug("Image tag filter successful!") + f.log.Debug("Image tag filter successful!") + return nil +} + +// Name returns the filter's name +func (f *ImageTagChecker) Name() string { + return "ImageTagChecker" } -// Describe filter -func (f ImageTagChecker) Describe() string { - return f.Description +// Describe describes the filter +func (f *ImageTagChecker) Describe() string { + return "Checks and adds recommendation if 'latest' image tag is used for container image." } diff --git a/pkg/filterengine/filters/ingress_validator.go b/pkg/filterengine/filters/ingress_validator.go index b700e8a1a..567483413 100644 --- a/pkg/filterengine/filters/ingress_validator.go +++ b/pkg/filterengine/filters/ingress_validator.go @@ -22,32 +22,38 @@ package filters import ( "context" "fmt" - "reflect" + "github.com/sirupsen/logrus" networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/dynamic" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/utils" ) // IngressValidator checks if service and tls secret used in ingress specs is already present // and adds recommendations to event struct accordingly type IngressValidator struct { - Description string + log logrus.FieldLogger + dynamicCli dynamic.Interface +} + +// NewIngressValidator creates a new IngressValidator instance +func NewIngressValidator(log logrus.FieldLogger, dynamicCli dynamic.Interface) *IngressValidator { + return &IngressValidator{log: log, dynamicCli: dynamicCli} } // Run filers and modifies event struct -func (iv IngressValidator) Run(object interface{}, event *events.Event) { +func (f *IngressValidator) Run(ctx context.Context, object interface{}, event *events.Event) error { if event.Kind != "Ingress" || event.Type != config.CreateEvent || utils.GetObjectTypeMetaData(object).Kind == "Event" { - return + return nil } var ingressObj networkingv1.Ingress err := utils.TransformIntoTypedObject(object.(*unstructured.Unstructured), &ingressObj) if err != nil { - log.Errorf("Unable to transform object type: %v, into type: %v", reflect.TypeOf(object), reflect.TypeOf(ingressObj)) + return fmt.Errorf("while transforming object type %T into type: %T: %w", object, ingressObj, err) } ingNs := ingressObj.ObjectMeta.Namespace @@ -56,7 +62,8 @@ func (iv IngressValidator) Run(object interface{}, event *events.Event) { for _, rule := range ingressObj.Spec.Rules { for _, path := range rule.IngressRuleValue.HTTP.Paths { if path.Backend.Service == nil { - return + // TODO: Support path.Backend.Resource as well + continue } serviceName := path.Backend.Service.Name servicePort := path.Backend.Service.Port.Number @@ -64,7 +71,7 @@ func (iv IngressValidator) Run(object interface{}, event *events.Event) { if ns == "default" { ns = ingNs } - _, err := ValidServicePort(context.Background(), serviceName, ns, int32(servicePort)) + _, err := ValidServicePort(ctx, f.dynamicCli, serviceName, ns, servicePort) if err != nil { event.Warnings = append(event.Warnings, fmt.Sprintf("Service '%s' used in ingress '%s' config does not exist or port '%v' not exposed", serviceName, ingressObj.Name, servicePort)) } @@ -73,15 +80,21 @@ func (iv IngressValidator) Run(object interface{}, event *events.Event) { // Check if tls secret exists for _, tls := range ingressObj.Spec.TLS { - _, err := ValidSecret(context.Background(), tls.SecretName, ingNs) + _, err := ValidSecret(ctx, f.dynamicCli, tls.SecretName, ingNs) if err != nil { event.Recommendations = append(event.Recommendations, fmt.Sprintf("TLS secret %s does not exist", tls.SecretName)) } } - log.Debug("Ingress Validator filter successful!") + f.log.Debug("Ingress Validator filter successful!") + return nil +} + +// Name returns the filter's name +func (f *IngressValidator) Name() string { + return "IngressValidator" } -// Describe filter -func (iv IngressValidator) Describe() string { - return iv.Description +// Describe describes the filter +func (f *IngressValidator) Describe() string { + return "Checks if services and tls secrets used in ingress specs are available." } diff --git a/pkg/filterengine/filters/namespace_checker.go b/pkg/filterengine/filters/namespace_checker.go index c1167f1af..0c26e3701 100644 --- a/pkg/filterengine/filters/namespace_checker.go +++ b/pkg/filterengine/filters/namespace_checker.go @@ -20,36 +20,35 @@ package filters import ( + "context" "regexp" "strings" + "github.com/sirupsen/logrus" + "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) // NamespaceChecker ignore events from blocklisted namespaces type NamespaceChecker struct { - Description string + log logrus.FieldLogger + configuredResources []config.Resource +} + +// NewNamespaceChecker creates a new NamespaceChecker instance +func NewNamespaceChecker(log logrus.FieldLogger, configuredResources []config.Resource) *NamespaceChecker { + return &NamespaceChecker{log: log, configuredResources: configuredResources} } // Run filters and modifies event struct -func (f NamespaceChecker) Run(_ interface{}, event *events.Event) { +func (f *NamespaceChecker) Run(_ context.Context, _ interface{}, event *events.Event) error { // Skip filter for cluster scoped resource if len(event.Namespace) == 0 { - return - } - // load config.yaml - botkubeConfig, err := config.New() - if err != nil { - log.Errorf("Error in loading configuration. %s", err.Error()) - return + return nil } - if botkubeConfig == nil { - log.Errorf("Error in loading configuration.") - return - } - for _, resource := range botkubeConfig.Resources { + + for _, resource := range f.configuredResources { if event.Resource != resource.Name { continue } @@ -57,15 +56,21 @@ func (f NamespaceChecker) Run(_ interface{}, event *events.Event) { event.Skip = shouldSkipEvent break } - log.Debug("Ignore Namespaces filter successful!") + f.log.Debug("Ignore Namespaces filter successful!") + return nil +} + +// Name returns the filter's name +func (f *NamespaceChecker) Name() string { + return "NamespaceChecker" } -// Describe filter -func (f NamespaceChecker) Describe() string { - return f.Description +// Describe describes the filter +func (f *NamespaceChecker) Describe() string { + return "Checks if event belongs to blocklisted namespaces and filter them." } -// isNamespaceIgnored checks if a event to be ignored from user config +// isNamespaceIgnored checks if an event to be ignored from user config func isNamespaceIgnored(resourceNamespaces config.Namespaces, eventNamespace string) bool { if len(resourceNamespaces.Include) == 1 && resourceNamespaces.Include[0] == "all" { if len(resourceNamespaces.Ignore) > 0 { diff --git a/pkg/filterengine/filters/node_event_checker.go b/pkg/filterengine/filters/node_event_checker.go index 5aa997ac1..b3583c769 100644 --- a/pkg/filterengine/filters/node_event_checker.go +++ b/pkg/filterengine/filters/node_event_checker.go @@ -22,9 +22,12 @@ package filters import ( + "context" + + "github.com/sirupsen/logrus" + "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/utils" ) @@ -37,19 +40,24 @@ const ( // NodeEventsChecker checks job status and adds message in the events structure type NodeEventsChecker struct { - Description string + log logrus.FieldLogger +} + +// NewNodeEventsChecker creates a new NodeEventsChecker instance +func NewNodeEventsChecker(log logrus.FieldLogger) *NodeEventsChecker { + return &NodeEventsChecker{log: log} } // Run filers and modifies event struct -func (f NodeEventsChecker) Run(object interface{}, event *events.Event) { +func (f *NodeEventsChecker) Run(_ context.Context, object interface{}, event *events.Event) error { // Check for Event object if utils.GetObjectTypeMetaData(object).Kind == "Event" { - return + return nil } // Run filter only on Node events if event.Kind != "Node" { - return + return nil } // Update event details @@ -66,10 +74,16 @@ func (f NodeEventsChecker) Run(object interface{}, event *events.Event) { event.Skip = true } - log.Debug("Node Critical Event filter successful!") + f.log.Debug("Node Critical Event filter successful!") + return nil +} + +// Name returns the filter's name +func (f *NodeEventsChecker) Name() string { + return "NodeEventsChecker" } -// Describe filter -func (f NodeEventsChecker) Describe() string { - return f.Description +// Describe describes the filter +func (f *NodeEventsChecker) Describe() string { + return "Sends notifications on node level critical events." } diff --git a/pkg/filterengine/filters/object_annotation_checker.go b/pkg/filterengine/filters/object_annotation_checker.go index 9c7d998b5..1d8f41dbc 100644 --- a/pkg/filterengine/filters/object_annotation_checker.go +++ b/pkg/filterengine/filters/object_annotation_checker.go @@ -20,10 +20,15 @@ package filters import ( + "context" + "fmt" + + "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/api/meta" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/utils" ) @@ -36,38 +41,54 @@ const ( // ObjectAnnotationChecker add recommendations to the event object if pod created without any labels type ObjectAnnotationChecker struct { - Description string + log logrus.FieldLogger + dynamicCli dynamic.Interface + mapper meta.RESTMapper +} + +// NewObjectAnnotationChecker creates a new ObjectAnnotationChecker instance +func NewObjectAnnotationChecker(log logrus.FieldLogger, dynamicCli dynamic.Interface, mapper meta.RESTMapper) *ObjectAnnotationChecker { + return &ObjectAnnotationChecker{log: log, dynamicCli: dynamicCli, mapper: mapper} } // Run filters and modifies event struct -func (f ObjectAnnotationChecker) Run(object interface{}, event *events.Event) { +func (f *ObjectAnnotationChecker) Run(ctx context.Context, object interface{}, event *events.Event) error { // get objects metadata - obj := utils.GetObjectMetaData(object) + obj, err := utils.GetObjectMetaData(ctx, f.dynamicCli, f.mapper, object) + if err != nil { + return fmt.Errorf("while getting object metadata: %w", err) + } // Check annotations in object - if isObjectNotifDisabled(obj) { + if f.isObjectNotifDisabled(obj) { event.Skip = true - log.Debug("Object Notification Disable through annotations") + f.log.Debug("Object Notification Disable through annotations") } - if channel, ok := reconfigureChannel(obj); ok { + if channel, ok := f.reconfigureChannel(obj); ok { event.Channel = channel - log.Debugf("Redirecting Event Notifications to channel: %s", channel) + f.log.Debugf("Redirecting Event Notifications to channel: %s", channel) } - log.Debug("Object annotations filter successful!") + f.log.Debug("Object annotations filter successful!") + return nil +} + +// Name returns the filter's name +func (f *ObjectAnnotationChecker) Name() string { + return "ObjectAnnotationChecker" } -// Describe filter -func (f ObjectAnnotationChecker) Describe() string { - return f.Description +// Describe describes the filter +func (f *ObjectAnnotationChecker) Describe() string { + return "Checks if annotations botkube.io/* present in object specs and filters them." } // isObjectNotifDisabled checks annotation botkube.io/disable // annotation botkube.io/disable disables the event notifications from objects -func isObjectNotifDisabled(obj metaV1.ObjectMeta) bool { +func (f *ObjectAnnotationChecker) isObjectNotifDisabled(obj metaV1.ObjectMeta) bool { if obj.Annotations[DisableAnnotation] == "true" { - log.Debug("Skipping Disabled Event Notifications!") + f.log.Debug("Skipping Disabled Event Notifications!") return true } return false @@ -77,7 +98,7 @@ func isObjectNotifDisabled(obj metaV1.ObjectMeta) bool { // annotation botkube.io/channel directs event notifications to channels // based on the channel names present in them // Note: Add botkube app into the desired channel to receive notifications -func reconfigureChannel(obj metaV1.ObjectMeta) (string, bool) { +func (f *ObjectAnnotationChecker) reconfigureChannel(obj metaV1.ObjectMeta) (string, bool) { // redirect messages to channels based on annotations if channel, ok := obj.Annotations[ChannelAnnotation]; ok { return channel, true diff --git a/pkg/filterengine/filters/object_annotation_checker_test.go b/pkg/filterengine/filters/object_annotation_checker_test.go index 46b149a6d..7bfe0fb86 100644 --- a/pkg/filterengine/filters/object_annotation_checker_test.go +++ b/pkg/filterengine/filters/object_annotation_checker_test.go @@ -22,6 +22,7 @@ package filters import ( "testing" + logtest "github.com/sirupsen/logrus/hooks/test" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -38,7 +39,10 @@ func TestIsObjectNotifDisabled(t *testing.T) { for name, test := range tests { name, test := name, test t.Run(name, func(t *testing.T) { - if actual := isObjectNotifDisabled(test.annotation); actual != test.expected { + log, _ := logtest.NewNullLogger() + f := NewObjectAnnotationChecker(log, nil, nil) + + if actual := f.isObjectNotifDisabled(test.annotation); actual != test.expected { t.Errorf("expected: %+v != actual: %+v\n", test.expected, actual) } }) @@ -59,7 +63,10 @@ func TestReconfigureChannel(t *testing.T) { for name, test := range tests { name, test := name, test t.Run(name, func(t *testing.T) { - if actualChannel, actualBool := reconfigureChannel(test.objectMeta); actualBool != test.expectedBool { + log, _ := logtest.NewNullLogger() + f := NewObjectAnnotationChecker(log, nil, nil) + + if actualChannel, actualBool := f.reconfigureChannel(test.objectMeta); actualBool != test.expectedBool { if actualChannel != test.expectedChannel { t.Errorf("expected: %+v != actual: %+v\n", test.expectedChannel, actualChannel) } diff --git a/pkg/filterengine/filters/pod_label_checker.go b/pkg/filterengine/filters/pod_label_checker.go index 4996fa3f7..0f58c3e99 100644 --- a/pkg/filterengine/filters/pod_label_checker.go +++ b/pkg/filterengine/filters/pod_label_checker.go @@ -20,35 +20,55 @@ package filters import ( + "context" "fmt" + "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/dynamic" + "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/utils" ) // PodLabelChecker add recommendations to the event object if pod created without any labels type PodLabelChecker struct { - Description string + log logrus.FieldLogger + dynamicCli dynamic.Interface + mapper meta.RESTMapper +} + +// NewPodLabelChecker creates a new PodLabelChecker instance +func NewPodLabelChecker(log logrus.FieldLogger, dynamicCli dynamic.Interface, mapper meta.RESTMapper) *PodLabelChecker { + return &PodLabelChecker{log: log, dynamicCli: dynamicCli, mapper: mapper} } // Run filters and modifies event struct -func (f PodLabelChecker) Run(object interface{}, event *events.Event) { +func (f PodLabelChecker) Run(ctx context.Context, object interface{}, event *events.Event) error { if event.Kind != "Pod" || event.Type != config.CreateEvent { - return + return nil } - podObjectMeta := utils.GetObjectMetaData(object) + podObjectMeta, err := utils.GetObjectMetaData(ctx, f.dynamicCli, f.mapper, object) + if err != nil { + return fmt.Errorf("while getting object metadata: %w", err) + } // Check labels in pod if len(podObjectMeta.Labels) == 0 { event.Recommendations = append(event.Recommendations, fmt.Sprintf("pod '%s' creation without labels should be avoided.", podObjectMeta.Name)) } - log.Debug("Pod label filter successful!") + f.log.Debug("Pod label filter successful!") + return nil +} + +// Name returns the filter's name +func (f *PodLabelChecker) Name() string { + return "PodLabelChecker" } -// Describe filter +// Describe describes the filter func (f PodLabelChecker) Describe() string { - return f.Description + return "Checks and adds recommendations if labels are missing in the pod specs." } diff --git a/pkg/filterengine/filters/utils.go b/pkg/filterengine/filters/utils.go index ebbe68039..890b43a7c 100644 --- a/pkg/filterengine/filters/utils.go +++ b/pkg/filterengine/filters/utils.go @@ -27,6 +27,7 @@ import ( coreV1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" "github.com/infracloudio/botkube/pkg/utils" ) @@ -42,23 +43,9 @@ var ( } ) -// ValidService returns Service object is service given service exists in the given namespace -func ValidService(ctx context.Context, name, namespace string) (*coreV1.Service, error) { - unstructuredService, err := utils.DynamicKubeClient.Resource(serviceGVR).Namespace(namespace).Get(ctx, name, metaV1.GetOptions{}) - if err != nil { - return nil, err - } - var serviceObject coreV1.Service - err = utils.TransformIntoTypedObject(unstructuredService, &serviceObject) - if err != nil { - return nil, err - } - return &serviceObject, nil -} - // ValidServicePort returns valid Service object if given service with the port exists in the given namespace -func ValidServicePort(ctx context.Context, name, namespace string, port int32) (*coreV1.Service, error) { - unstructuredService, err := utils.DynamicKubeClient.Resource(serviceGVR).Namespace(namespace).Get(ctx, name, metaV1.GetOptions{}) +func ValidServicePort(ctx context.Context, dynamicCli dynamic.Interface, name, namespace string, port int32) (*coreV1.Service, error) { + unstructuredService, err := dynamicCli.Resource(serviceGVR).Namespace(namespace).Get(ctx, name, metaV1.GetOptions{}) if err != nil { return nil, err } @@ -76,8 +63,8 @@ func ValidServicePort(ctx context.Context, name, namespace string, port int32) ( } // ValidSecret return Secret object if the secret is present in the specified object -func ValidSecret(ctx context.Context, name, namespace string) (*coreV1.Secret, error) { - unstructuredSecret, err := utils.DynamicKubeClient.Resource(secretGVR).Namespace(namespace).Get(ctx, name, metaV1.GetOptions{}) +func ValidSecret(ctx context.Context, dynamicCli dynamic.Interface, name, namespace string) (*coreV1.Secret, error) { + unstructuredSecret, err := dynamicCli.Resource(secretGVR).Namespace(namespace).Get(ctx, name, metaV1.GetOptions{}) if err != nil { return nil, err } diff --git a/pkg/filterengine/global.go b/pkg/filterengine/global.go deleted file mode 100644 index 7826b2294..000000000 --- a/pkg/filterengine/global.go +++ /dev/null @@ -1,25 +0,0 @@ -package filterengine - -import "github.com/infracloudio/botkube/pkg/filterengine/filters" - -var ( - // DefaultFilterEngine contains default implementation for FilterEngine - DefaultFilterEngine FilterEngine -) - -// SetupGlobal set ups a global filter engine with all default filters enabled -// TODO: Convert it to local instance shared by all BotKube components -// See: https://github.com/infracloudio/botkube/issues/589 -func SetupGlobal() { - filtersToRegister := []Filter{ - filters.ImageTagChecker{Description: "Checks and adds recommendation if 'latest' image tag is used for container image."}, - filters.IngressValidator{Description: "Checks if services and tls secrets used in ingress specs are available."}, - filters.ObjectAnnotationChecker{Description: "Checks if annotations botkube.io/* present in object specs and filters them."}, - filters.PodLabelChecker{Description: "Checks and adds recommendations if labels are missing in the pod specs."}, - filters.NamespaceChecker{Description: "Checks if event belongs to blocklisted namespaces and filter them."}, - filters.NodeEventsChecker{Description: "Sends notifications on node level critical events."}, - } - - DefaultFilterEngine = NewDefaultFilter() - DefaultFilterEngine.Register(filtersToRegister...) -} diff --git a/pkg/filterengine/with_all_filters.go b/pkg/filterengine/with_all_filters.go new file mode 100644 index 000000000..aeb526c0e --- /dev/null +++ b/pkg/filterengine/with_all_filters.go @@ -0,0 +1,30 @@ +package filterengine + +import ( + "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/dynamic" + + "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/filterengine/filters" +) + +const ( + filterLogFieldKey = "filter" + componentLogFieldKey = "component" +) + +// WithAllFilters returns new DefaultFilterEngine instance with all filters registered. +func WithAllFilters(logger *logrus.Logger, dynamicCli dynamic.Interface, mapper meta.RESTMapper, conf *config.Config) *DefaultFilterEngine { + filterEngine := New(logger.WithField(componentLogFieldKey, "Filter Engine")) + filterEngine.Register([]Filter{ + filters.NewImageTagChecker(logger.WithField(filterLogFieldKey, "Image Tag Checker")), + filters.NewIngressValidator(logger.WithField(filterLogFieldKey, "Ingress Validator"), dynamicCli), + filters.NewObjectAnnotationChecker(logger.WithField(filterLogFieldKey, "Object Annotation Checker"), dynamicCli, mapper), + filters.NewPodLabelChecker(logger.WithField(filterLogFieldKey, "Pod Label Checker"), dynamicCli, mapper), + filters.NewNamespaceChecker(logger.WithField(filterLogFieldKey, "Namespace Checker"), conf.Resources), + filters.NewNodeEventsChecker(logger.WithField(filterLogFieldKey, "Node Events Checker")), + }...) + + return filterEngine +} diff --git a/pkg/httpsrv/server.go b/pkg/httpsrv/server.go new file mode 100644 index 000000000..627127d4d --- /dev/null +++ b/pkg/httpsrv/server.go @@ -0,0 +1,41 @@ +package httpsrv + +import ( + "context" + "fmt" + "net/http" + + "github.com/sirupsen/logrus" +) + +// Server provides functionality to start HTTP server with a cancelable context. +type Server struct { + srv *http.Server + log logrus.FieldLogger +} + +// New creates a new HTTP server. +func New(log logrus.FieldLogger, addr string, mux *http.ServeMux) *Server { + return &Server{ + srv: &http.Server{Addr: addr, Handler: mux}, + log: log, + } +} + +// Serve starts the HTTP server and blocks unil the channel is closed or an error occurs. +func (s *Server) Serve(ctx context.Context) error { + go func() { + <-ctx.Done() + s.log.Info("Shutdown requested. Finishing...") + if err := s.srv.Shutdown(context.Background()); err != nil { + s.log.Error("while shutting down server: %w", err) + } + }() + + s.log.Infof("Starting server on address %q", s.srv.Addr) + if err := s.srv.ListenAndServe(); err != http.ErrServerClosed { + return fmt.Errorf("while starting server: %w", err) + } + + return nil +} diff --git a/pkg/kube/client.go b/pkg/kube/client.go new file mode 100644 index 000000000..83407f6ce --- /dev/null +++ b/pkg/kube/client.go @@ -0,0 +1,35 @@ +package kube + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/client-go/discovery" + cacheddiscovery "k8s.io/client-go/discovery/cached" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/restmapper" + "k8s.io/client-go/tools/clientcmd" +) + +// SetupK8sClients creates K8s client from provided kubeconfig OR service account to interact with apiserver +func SetupK8sClients(kubeConfigPath string) (dynamic.Interface, discovery.DiscoveryInterface, meta.RESTMapper, error) { + kubeConfig, err := clientcmd.BuildConfigFromFlags("", kubeConfigPath) + if err != nil { + return nil, nil, nil, fmt.Errorf("while loading K8s config: %w", err) + } + + // Initiate discovery client for REST resource mapping + discoveryClient, err := discovery.NewDiscoveryClientForConfig(kubeConfig) + if err != nil { + return nil, nil, nil, fmt.Errorf("while creating discovery client: %w", err) + } + + dynamicK8sCli, err := dynamic.NewForConfig(kubeConfig) + if err != nil { + return nil, nil, nil, fmt.Errorf("while creating dynamic K8s client: %w", err) + } + + discoCacheClient := cacheddiscovery.NewMemCacheClient(discoveryClient) + mapper := restmapper.NewDeferredDiscoveryRESTMapper(discoCacheClient) + return dynamicK8sCli, discoveryClient, mapper, nil +} diff --git a/pkg/log/log.go b/pkg/log/log.go deleted file mode 100644 index 87e433e2b..000000000 --- a/pkg/log/log.go +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright (c) 2019 InfraCloud Technologies -// -// Permission is hereby granted, free of charge, to any person obtaining a copy of -// this software and associated documentation files (the "Software"), to deal in -// the Software without restriction, including without limitation the rights to -// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of -// the Software, and to permit persons to whom the Software is furnished to do so, -// subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS -// FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR -// COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER -// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN -// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - -package log - -import ( - "os" - - "github.com/sirupsen/logrus" -) - -// log object for logging across the pkg/ -var log = logrus.New() - -// SetupGlobal set ups a global logger -// TODO: Convert it to local instance shared by all BotKube components -// See: https://github.com/infracloudio/botkube/issues/589 -func SetupGlobal() { - // Output to stdout instead of the default stderr - log.SetOutput(os.Stdout) - - // Only log the warning severity or above. - logLevel, err := logrus.ParseLevel(os.Getenv("LOG_LEVEL")) - if err != nil { - // Set Info level as a default - logLevel = logrus.InfoLevel - } - log.SetLevel(logLevel) - log.Formatter = &logrus.TextFormatter{ForceColors: true, FullTimestamp: true} -} - -// Info map logrus.Info func to log.Info -func Info(message ...interface{}) { - log.Info(message...) -} - -// Trace map logrus.Trace func to log.Trace -func Trace(message ...interface{}) { - log.Trace(message...) -} - -// Debug map logrus.Debug func to log.Debug -func Debug(message ...interface{}) { - log.Debug(message...) -} - -// Warn map logrus.Warn func to log.Warn -func Warn(message ...interface{}) { - log.Warn(message...) -} - -// Error map logrus.Error func to log.Error -func Error(message ...interface{}) { - log.Error(message...) -} - -// Fatal map logrus.Fatal func to log.Fatal -func Fatal(message ...interface{}) { - log.Fatal(message...) -} - -// Panic map logrus.Panic func to log.Panic -func Panic(message ...interface{}) { - log.Panic(message...) -} - -// Infof map logrus.Infof func to log.Infof -func Infof(format string, v ...interface{}) { - log.Infof(format, v...) -} - -// Tracef map logrus.Tracef func to log.Tracef -func Tracef(format string, v ...interface{}) { - log.Tracef(format, v...) -} - -// Debugf map logrus.Debugf func to log.Debugf -func Debugf(format string, v ...interface{}) { - log.Debugf(format, v...) -} - -// Warnf map logrus.Warnf func to log.Warnf -func Warnf(format string, v ...interface{}) { - log.Warnf(format, v...) -} - -// Errorf map logrus.Errorf func to log.Errorf -func Errorf(format string, v ...interface{}) { - log.Errorf(format, v...) -} - -// Fatalf map logrus.Fatalf func to log.Fatalf -func Fatalf(format string, v ...interface{}) { - log.Fatalf(format, v...) -} - -// Panicf map logrus.Panicf func to log.Panicf -func Panicf(format string, v ...interface{}) { - log.Panicf(format, v...) -} - -//GetLevel logrus logLevel -func GetLevel() logrus.Level { - return log.GetLevel() -} diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go deleted file mode 100644 index 58981b826..000000000 --- a/pkg/metrics/metrics.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2019 InfraCloud Technologies -// -// Permission is hereby granted, free of charge, to any person obtaining a copy of -// this software and associated documentation files (the "Software"), to deal in -// the Software without restriction, including without limitation the rights to -// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of -// the Software, and to permit persons to whom the Software is furnished to do so, -// subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS -// FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR -// COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER -// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN -// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - -package metrics - -import ( - "net/http" - - "github.com/prometheus/client_golang/prometheus/promhttp" -) - -// ServeMetrics exposes metrics in Prometheus format -// TODO: Make sure server gracefully shut downs on signal -func ServeMetrics(metricsPort string) error { - http.Handle("/metrics", promhttp.Handler()) - return http.ListenAndServe(":"+metricsPort, nil) -} diff --git a/pkg/notify/discord.go b/pkg/notify/discord.go index 031766903..0ded29873 100644 --- a/pkg/notify/discord.go +++ b/pkg/notify/discord.go @@ -20,13 +20,14 @@ package notify import ( + "context" "fmt" "github.com/bwmarrin/discordgo" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) // customTimeFormat holds custom time format string @@ -42,14 +43,17 @@ var embedColor = map[config.Level]int{ // Discord contains URL and ClusterName type Discord struct { + log logrus.FieldLogger + Token string ChannelID string NotifType config.NotifType } // NewDiscord returns new Discord object -func NewDiscord(c config.Discord) Notifier { +func NewDiscord(log logrus.FieldLogger, c config.Discord) *Discord { return &Discord{ + log: log, Token: c.Token, ChannelID: c.Channel, NotifType: c.NotifType, @@ -57,38 +61,36 @@ func NewDiscord(c config.Discord) Notifier { } // SendEvent sends event notification to Discord Channel -func (d *Discord) SendEvent(event events.Event) (err error) { - log.Debug(fmt.Sprintf(">> Sending to discord: %+v", event)) +// Context is not supported by client: See https://github.com/bwmarrin/discordgo/issues/752 +func (d *Discord) SendEvent(_ context.Context, event events.Event) (err error) { + d.log.Debugf(">> Sending to discord: %+v", event) api, err := discordgo.New("Bot " + d.Token) if err != nil { - log.Error("error creating Discord session,", err) - return err + return fmt.Errorf("while creating Discord session: %w", err) } messageSend := formatDiscordMessage(event, d.NotifType) if _, err := api.ChannelMessageSendComplex(d.ChannelID, &messageSend); err != nil { - log.Errorf("Error in sending message: %+v", err) - return err + return fmt.Errorf("while sending Discord message to channel %q: %w", d.ChannelID, err) } - log.Debugf("Event successfully sent to channel %s", d.ChannelID) + d.log.Debugf("Event successfully sent to channel %s", d.ChannelID) return nil } // SendMessage sends message to Discord Channel -func (d *Discord) SendMessage(msg string) error { - log.Debug(fmt.Sprintf(">> Sending to discord: %+v", msg)) +// Context is not supported by client: See https://github.com/bwmarrin/discordgo/issues/752 +func (d *Discord) SendMessage(_ context.Context, msg string) error { + d.log.Debugf(">> Sending to discord: %+v", msg) api, err := discordgo.New("Bot " + d.Token) if err != nil { - log.Error("error creating Discord session,", err) - return err + return fmt.Errorf("while creating Discord session: %w", err) } if _, err := api.ChannelMessageSend(d.ChannelID, msg); err != nil { - log.Error("Error in sending message:", err) - return err + return fmt.Errorf("while sending Discord message to channel %q: %w", d.ChannelID, err) } - log.Debugf("Event successfully sent to Discord %v", msg) + d.log.Debugf("Event successfully sent to Discord %v", msg) return nil } diff --git a/pkg/notify/elasticsearch.go b/pkg/notify/elasticsearch.go index 69bd178a8..ecf90a993 100644 --- a/pkg/notify/elasticsearch.go +++ b/pkg/notify/elasticsearch.go @@ -35,10 +35,10 @@ import ( "github.com/aws/aws-sdk-go/service/sts" "github.com/olivere/elastic" "github.com/sha1sum/aws_signing_client" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) const ( @@ -55,6 +55,7 @@ const ( // ElasticSearch contains auth cred and index setting type ElasticSearch struct { + log logrus.FieldLogger ELSClient *elastic.Client Server string SkipTLSVerify bool @@ -65,7 +66,7 @@ type ElasticSearch struct { } // NewElasticSearch returns new ElasticSearch object -func NewElasticSearch(c config.ElasticSearch) (Notifier, error) { +func NewElasticSearch(log logrus.FieldLogger, c config.ElasticSearch) (*ElasticSearch, error) { var elsClient *elastic.Client var err error var creds *credentials.Credentials @@ -89,7 +90,7 @@ func NewElasticSearch(c config.ElasticSearch) (Notifier, error) { signer := v4.NewSigner(creds) awsClient, err := aws_signing_client.New(signer, nil, awsService, c.AWSSigning.AWSRegion) if err != nil { - return nil, err + return nil, fmt.Errorf("while creating new AWS Signing client: %w", err) } elsClient, err = elastic.NewClient( elastic.SetURL(c.Server), @@ -100,7 +101,7 @@ func NewElasticSearch(c config.ElasticSearch) (Notifier, error) { elastic.SetGzip(false), ) if err != nil { - return nil, err + return nil, fmt.Errorf("while creating new Elastic client: %w", err) } } else { elsClientParams := []elastic.ClientOptionFunc{ @@ -122,10 +123,11 @@ func NewElasticSearch(c config.ElasticSearch) (Notifier, error) { // create elasticsearch client elsClient, err = elastic.NewClient(elsClientParams...) if err != nil { - return nil, err + return nil, fmt.Errorf("while creating new Elastic client: %w", err) } } return &ElasticSearch{ + log: log, ELSClient: elsClient, Index: c.Index.Name, Type: c.Index.Type, @@ -152,8 +154,7 @@ func (e *ElasticSearch) flushIndex(ctx context.Context, event interface{}) error // Create index if not exists exists, err := e.ELSClient.IndexExists(indexName).Do(ctx) if err != nil { - log.Error(fmt.Sprintf("Failed to get index. Error:%s", err.Error())) - return err + return fmt.Errorf("while getting index: %w", err) } if !exists { // Create a new index. @@ -167,30 +168,26 @@ func (e *ElasticSearch) flushIndex(ctx context.Context, event interface{}) error } _, err := e.ELSClient.CreateIndex(indexName).BodyJson(mapping).Do(ctx) if err != nil { - log.Error(fmt.Sprintf("Failed to create index. Error:%s", err.Error())) - return err + return fmt.Errorf("while creating index: %w", err) } } // Send event to els _, err = e.ELSClient.Index().Index(indexName).Type(e.Type).BodyJson(event).Do(ctx) if err != nil { - log.Error(fmt.Sprintf("Failed to post data to els. Error:%s", err.Error())) - return err + return fmt.Errorf("while posting data to ELS: %w", err) } _, err = e.ELSClient.Flush().Index(indexName).Do(ctx) if err != nil { - log.Error(fmt.Sprintf("Failed to flush data to els. Error:%s", err.Error())) - return err + return fmt.Errorf("while flushing data in ELS: %w", err) } - log.Debugf("Event successfully sent to ElasticSearch index %s", indexName) + e.log.Debugf("Event successfully sent to ElasticSearch index %s", indexName) return nil } -// SendEvent sends event notification to slack -func (e *ElasticSearch) SendEvent(event events.Event) (err error) { - log.Debug(fmt.Sprintf(">> Sending to ElasticSearch: %+v", event)) - ctx := context.Background() +// SendEvent sends event notification to ElasticSearch +func (e *ElasticSearch) SendEvent(ctx context.Context, event events.Event) (err error) { + e.log.Debugf(">> Sending to ElasticSearch: %+v", event) // Create index if not exists if err := e.flushIndex(ctx, event); err != nil { @@ -199,7 +196,7 @@ func (e *ElasticSearch) SendEvent(event events.Event) (err error) { return nil } -// SendMessage sends message to slack channel -func (e *ElasticSearch) SendMessage(msg string) error { +// SendMessage is no-op +func (e *ElasticSearch) SendMessage(_ context.Context, _ string) error { return nil } diff --git a/pkg/notify/lark.go b/pkg/notify/lark.go index 13fc73624..c6bd36df0 100644 --- a/pkg/notify/lark.go +++ b/pkg/notify/lark.go @@ -20,11 +20,13 @@ package notify import ( + "context" + "github.com/larksuite/oapi-sdk-go/core" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/utils" ) @@ -35,27 +37,31 @@ const ( // Lark contains LarkClient for communication with lark and receiver group name to send notification to type Lark struct { + log logrus.FieldLogger LarkClient *utils.LarkClient ReceiverGroup string } // NewLark returns new Lark object -func NewLark(c config.CommunicationsConfig) Notifier { +func NewLark(log logrus.FieldLogger, loggerLevel logrus.Level, c config.CommunicationsConfig) *Lark { appSettings := core.NewInternalAppSettings(core.SetAppCredentials(c.Lark.AppID, c.Lark.AppSecret), core.SetAppEventKey(c.Lark.VerificationToken, c.Lark.EncryptKey)) - conf := core.NewConfig(core.Domain(c.Lark.Endpoint), appSettings, core.SetLoggerLevel(utils.GetLoggerLevel())) - return &Lark{LarkClient: utils.NewLarkClient(conf), - ReceiverGroup: c.Lark.ChatGroup} + conf := core.NewConfig(core.Domain(c.Lark.Endpoint), appSettings, core.SetLoggerLevel(utils.GetLoggerLevel(loggerLevel))) + return &Lark{ + log: log, + LarkClient: utils.NewLarkClient(log, conf), + ReceiverGroup: c.Lark.ChatGroup, + } } // SendEvent sends event notification to lark chart group -func (l *Lark) SendEvent(event events.Event) error { - log.Debugf(">> Sending to lark: %+v", event) - return l.LarkClient.SendTextMessage(ChatID, l.ReceiverGroup, FormatShortMessage(event)) +func (l *Lark) SendEvent(ctx context.Context, event events.Event) error { + l.log.Debugf(">> Sending to lark: %+v", event) + return l.LarkClient.SendTextMessage(ctx, ChatID, l.ReceiverGroup, FormatShortMessage(event)) } // SendMessage sends message to lark chart group -func (l *Lark) SendMessage(msg string) error { - log.Debugf(">> Sending to lark: %+v", msg) - return l.LarkClient.SendTextMessage(ChatID, l.ReceiverGroup, msg) +func (l *Lark) SendMessage(ctx context.Context, msg string) error { + l.log.Debugf(">> Sending to lark: %+v", msg) + return l.LarkClient.SendTextMessage(ctx, ChatID, l.ReceiverGroup, msg) } diff --git a/pkg/notify/mattermost.go b/pkg/notify/mattermost.go index a22f5eccd..07edb3f73 100644 --- a/pkg/notify/mattermost.go +++ b/pkg/notify/mattermost.go @@ -20,27 +20,29 @@ package notify import ( + "context" "encoding/json" "fmt" "strconv" "github.com/hashicorp/go-multierror" "github.com/mattermost/mattermost-server/v5/model" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) // Mattermost contains server URL and token type Mattermost struct { + log logrus.FieldLogger Client *model.Client4 Channel string NotifType config.NotifType } // NewMattermost returns new Mattermost object -func NewMattermost(c config.Mattermost) (Notifier, error) { +func NewMattermost(log logrus.FieldLogger, c config.Mattermost) (*Mattermost, error) { // Set configurations for Mattermost server client := model.NewAPIv4Client(c.URL) client.SetOAuthToken(c.Token) @@ -54,6 +56,7 @@ func NewMattermost(c config.Mattermost) (Notifier, error) { } return &Mattermost{ + log: log, Client: client, Channel: botChannel.Id, NotifType: c.NotifType, @@ -61,8 +64,8 @@ func NewMattermost(c config.Mattermost) (Notifier, error) { } // SendEvent sends event notification to Mattermost -func (m *Mattermost) SendEvent(event events.Event) error { - log.Info(fmt.Sprintf(">> Sending to Mattermost: %+v", event)) +func (m *Mattermost) SendEvent(ctx context.Context, event events.Event) error { + m.log.Debugf(">> Sending to Mattermost: %+v", event) var fields []*model.SlackAttachmentField @@ -113,7 +116,7 @@ func (m *Mattermost) SendEvent(event events.Event) error { // send error message to default channel msg := fmt.Sprintf("Unable to send message to Channel `%s`: `%s`\n```add Botkube app to the Channel %s\nMissed events follows below:```", targetChannel, resp.Error, targetChannel) - sendMessageErr := m.SendMessage(msg) + sendMessageErr := m.SendMessage(ctx, msg) if sendMessageErr != nil { return multierror.Append(createPostWrappedErr, sendMessageErr) } @@ -121,7 +124,7 @@ func (m *Mattermost) SendEvent(event events.Event) error { // sending missed event to default channel // reset event.Channel and send event event.Channel = "" - sendEventErr := m.SendEvent(event) + sendEventErr := m.SendEvent(ctx, event) if sendEventErr != nil { return multierror.Append(createPostWrappedErr, sendEventErr) } @@ -129,15 +132,16 @@ func (m *Mattermost) SendEvent(event events.Event) error { return createPostWrappedErr } - log.Debugf("Event successfully sent to channel %q", post.ChannelId) + m.log.Debugf("Event successfully sent to channel %q", post.ChannelId) return nil } // SendMessage sends message to Mattermost channel -func (m *Mattermost) SendMessage(msg string) error { - post := &model.Post{} - post.ChannelId = m.Channel - post.Message = msg +func (m *Mattermost) SendMessage(_ context.Context, msg string) error { + post := &model.Post{ + ChannelId: m.Channel, + Message: msg, + } if _, resp := m.Client.CreatePost(post); resp.Error != nil { return fmt.Errorf("while creating a post: %w", resp.Error) } diff --git a/pkg/notify/notify.go b/pkg/notify/notify.go index c2aa01484..e942a00b2 100644 --- a/pkg/notify/notify.go +++ b/pkg/notify/notify.go @@ -20,47 +20,59 @@ package notify import ( + "context" "fmt" + "github.com/sirupsen/logrus" + "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) +const notifierLogFieldKey = "notifier" + // Notifier to send event notification on the communication channels type Notifier interface { - SendEvent(events.Event) error - SendMessage(string) error + SendEvent(context.Context, events.Event) error + SendMessage(context.Context, string) error } -// ListNotifiers returns list of configured notifiers -func ListNotifiers(conf config.CommunicationsConfig) []Notifier { +// LoadNotifiers returns list of configured notifiers +func LoadNotifiers(log *logrus.Logger, conf config.CommunicationsConfig) ([]Notifier, error) { var notifiers []Notifier if conf.Slack.Enabled { - notifiers = append(notifiers, NewSlack(conf.Slack)) + notifiers = append(notifiers, NewSlack(log.WithField(notifierLogFieldKey, "Slack"), conf.Slack)) } + if conf.Mattermost.Enabled { - if notifier, err := NewMattermost(conf.Mattermost); err == nil { - notifiers = append(notifiers, notifier) - } else { - log.Error(fmt.Sprintf("Failed to create Mattermost client. Error: %v", err)) + mmNotifier, err := NewMattermost(log.WithField(notifierLogFieldKey, "Mattermost"), conf.Mattermost) + if err != nil { + return nil, fmt.Errorf("while creating Mattermost client: %w", err) } + + notifiers = append(notifiers, mmNotifier) } + if conf.Discord.Enabled { - notifiers = append(notifiers, NewDiscord(conf.Discord)) + notifiers = append(notifiers, NewDiscord(log.WithField(notifierLogFieldKey, "Discord"), conf.Discord)) } + if conf.ElasticSearch.Enabled { - if els, err := NewElasticSearch(conf.ElasticSearch); err == nil { - notifiers = append(notifiers, els) - } else { - log.Error(fmt.Sprintf("Failed to create els client. Error: %v", err)) + esNotifier, err := NewElasticSearch(log.WithField(notifierLogFieldKey, "ElasticSearch"), conf.ElasticSearch) + if err != nil { + return nil, fmt.Errorf("while creating ElasticSearch client: %w", err) } + + notifiers = append(notifiers, esNotifier) } + if conf.Webhook.Enabled { - notifiers = append(notifiers, NewWebhook(conf)) + notifiers = append(notifiers, NewWebhook(log.WithField(notifierLogFieldKey, "Webhook"), conf)) } + if conf.Lark.Enabled { - notifiers = append(notifiers, NewLark(conf)) + notifiers = append(notifiers, NewLark(log.WithField(notifierLogFieldKey, "Lark"), log.GetLevel(), conf)) } - return notifiers + + return notifiers, nil } diff --git a/pkg/notify/slack.go b/pkg/notify/slack.go index eaa3ac90e..37a679815 100644 --- a/pkg/notify/slack.go +++ b/pkg/notify/slack.go @@ -20,17 +20,17 @@ package notify import ( + "context" "encoding/json" "fmt" "strconv" "github.com/hashicorp/go-multierror" - + "github.com/sirupsen/logrus" "github.com/slack-go/slack" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) const sendFailureMessageFmt = "Unable to send message to Channel `%s`: `%s`\n```add Botkube app to the Channel %s\nMissed events follows below:```" @@ -46,14 +46,16 @@ var attachmentColor = map[config.Level]string{ // Slack contains Token for authentication with slack and Channel name to send notification to type Slack struct { + log logrus.FieldLogger Channel string NotifType config.NotifType Client *slack.Client } // NewSlack returns new Slack object -func NewSlack(c config.Slack) Notifier { +func NewSlack(log logrus.FieldLogger, c config.Slack) *Slack { return &Slack{ + log: log, Channel: c.Channel, NotifType: c.NotifType, Client: slack.New(c.Token), @@ -61,8 +63,8 @@ func NewSlack(c config.Slack) Notifier { } // SendEvent sends event notification to slack -func (s *Slack) SendEvent(event events.Event) error { - log.Debug(fmt.Sprintf(">> Sending to slack: %+v", event)) +func (s *Slack) SendEvent(ctx context.Context, event events.Event) error { + s.log.Debugf(">> Sending to slack: %+v", event) attachment := formatSlackMessage(event, s.NotifType) targetChannel := event.Channel @@ -84,7 +86,7 @@ func (s *Slack) SendEvent(event events.Event) error { // send error message to default channel msg := fmt.Sprintf(sendFailureMessageFmt, targetChannel, err.Error(), targetChannel) - sendMessageErr := s.SendMessage(msg) + sendMessageErr := s.SendMessage(ctx, msg) if sendMessageErr != nil { return multierror.Append(postMessageWrappedErr, sendMessageErr) } @@ -92,26 +94,26 @@ func (s *Slack) SendEvent(event events.Event) error { // sending missed event to default channel // reset event.Channel and send event event.Channel = "" - sendEventErr := s.SendEvent(event) + sendEventErr := s.SendEvent(ctx, event) if sendEventErr != nil { return multierror.Append(postMessageWrappedErr, sendEventErr) } return postMessageWrappedErr } - log.Debugf("Event successfully sent to channel %q at %s", channelID, timestamp) + s.log.Debugf("Event successfully sent to channel %q at %s", channelID, timestamp) return nil } // SendMessage sends message to slack channel -func (s *Slack) SendMessage(msg string) error { - log.Debug(fmt.Sprintf(">> Sending to slack: %+v", msg)) - channelID, timestamp, err := s.Client.PostMessage(s.Channel, slack.MsgOptionText(msg, false), slack.MsgOptionAsUser(true)) +func (s *Slack) SendMessage(ctx context.Context, msg string) error { + s.log.Debugf(">> Sending to slack: %+v", msg) + channelID, timestamp, err := s.Client.PostMessageContext(ctx, s.Channel, slack.MsgOptionText(msg, false), slack.MsgOptionAsUser(true)) if err != nil { return fmt.Errorf("while sending Slack message to channel %q: %w", s.Channel, err) } - log.Debugf("Message successfully sent to channel %s at %s", channelID, timestamp) + s.log.Debugf("Message successfully sent to channel %s at %s", channelID, timestamp) return nil } diff --git a/pkg/notify/webhook.go b/pkg/notify/webhook.go index 71aa3ba82..689a16bd1 100644 --- a/pkg/notify/webhook.go +++ b/pkg/notify/webhook.go @@ -21,20 +21,24 @@ package notify import ( "bytes" + "context" "encoding/json" "fmt" "net/http" "time" "github.com/hashicorp/go-multierror" + "github.com/sirupsen/logrus" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/events" - "github.com/infracloudio/botkube/pkg/log" ) +const defaultHTTPCliTimeout = 30 * time.Second + // Webhook contains URL type Webhook struct { + log logrus.FieldLogger URL string } @@ -66,14 +70,15 @@ type EventStatus struct { } // NewWebhook returns new Webhook object -func NewWebhook(c config.CommunicationsConfig) Notifier { +func NewWebhook(log logrus.FieldLogger, c config.CommunicationsConfig) *Webhook { return &Webhook{ + log: log, URL: c.Webhook.URL, } } // SendEvent sends event notification to Webhook url -func (w *Webhook) SendEvent(event events.Event) (err error) { +func (w *Webhook) SendEvent(ctx context.Context, event events.Event) (err error) { jsonPayload := &WebhookPayload{ EventMeta: EventMeta{ Kind: event.Kind, @@ -94,35 +99,34 @@ func (w *Webhook) SendEvent(event events.Event) (err error) { Warnings: event.Warnings, } - err = w.PostWebhook(jsonPayload) + err = w.PostWebhook(ctx, jsonPayload) if err != nil { - log.Error(err.Error()) - log.Debugf("Event Not Sent to Webhook %v", event) + return fmt.Errorf("while sending event to webhook: %w", err) } - log.Debugf("Event successfully sent to Webhook %v", event) + w.log.Debugf("Event successfully sent to Webhook: %+v", event) return nil } -// SendMessage sends message to Webhook url -func (w *Webhook) SendMessage(msg string) error { +// SendMessage is no-op +func (w *Webhook) SendMessage(_ context.Context, _ string) error { return nil } // PostWebhook posts webhook to listener -func (w *Webhook) PostWebhook(jsonPayload *WebhookPayload) (err error) { +func (w *Webhook) PostWebhook(ctx context.Context, jsonPayload *WebhookPayload) (err error) { message, err := json.Marshal(jsonPayload) if err != nil { return err } - req, err := http.NewRequest("POST", w.URL, bytes.NewBuffer(message)) + req, err := http.NewRequestWithContext(ctx, "POST", w.URL, bytes.NewBuffer(message)) if err != nil { return err } req.Header.Add("Content-Type", "application/json") - client := &http.Client{} + client := &http.Client{Timeout: defaultHTTPCliTimeout} resp, err := client.Do(req) if err != nil { return err diff --git a/pkg/notify/webhook_test.go b/pkg/notify/webhook_test.go index a470f4d05..cdf4d4d7e 100644 --- a/pkg/notify/webhook_test.go +++ b/pkg/notify/webhook_test.go @@ -20,6 +20,7 @@ package notify import ( + "context" "fmt" "net/http" "net/http/httptest" @@ -57,7 +58,7 @@ func TestPostWebhook(t *testing.T) { URL: ts.URL, } - err := w.PostWebhook(&WebhookPayload{}) + err := w.PostWebhook(context.Background(), &WebhookPayload{}) assert.Equal(t, test.expected, err) }) } diff --git a/pkg/utils/diff.go b/pkg/utils/diff.go index dfd4fef60..4ea630413 100644 --- a/pkg/utils/diff.go +++ b/pkg/utils/diff.go @@ -21,43 +21,47 @@ package utils import ( "fmt" + "strings" "github.com/infracloudio/botkube/pkg/config" - "github.com/infracloudio/botkube/pkg/log" ) type diffReporter struct { field string } -func (d diffReporter) exec(x, y interface{}) (string, bool) { +func (d diffReporter) exec(x, y interface{}) (string, error) { vx, err := parseJsonpath(x, d.field) if err != nil { // Happens when the fields were not set by the time event was issued, do not return in that case - log.Debugf("Failed to find value from jsonpath: %s, object: %+v. Error: %v", d.field, x, err) + return "", fmt.Errorf("while finding value from jsonpath: %q, object: %+v: %w", d.field, x, err) } vy, err := parseJsonpath(y, d.field) if err != nil { - log.Debugf("Failed to find value from jsonpath: %s, object: %+v, Error: %v", d.field, y, err) + return "", fmt.Errorf("while finding value from jsonpath: %q, object: %+v: %w", d.field, y, err) } // treat and false as same fields if vx == vy || (vx == "" && vy == "false") { - return "", false + return "", nil } - return fmt.Sprintf("%s:\n\t-: %+v\n\t+: %+v\n", d.field, vx, vy), true + return fmt.Sprintf("%s:\n\t-: %+v\n\t+: %+v\n", d.field, vx, vy), nil } // Diff provides differences between two objects spec -func Diff(x, y interface{}, updatesetting config.UpdateSetting) string { - msg := "" - for _, val := range updatesetting.Fields { +func Diff(x, y interface{}, updateSetting config.UpdateSetting) (string, error) { + strBldr := new(strings.Builder) + for _, val := range updateSetting.Fields { var d diffReporter d.field = val - if diff, ok := d.exec(x, y); ok { - msg = msg + diff + diff, err := d.exec(x, y) + if err != nil { + return "", err } + + strBldr.WriteString(diff) } - return msg + + return strBldr.String(), nil } diff --git a/pkg/utils/diff_test.go b/pkg/utils/diff_test.go index d6f5a53dd..d63f691da 100644 --- a/pkg/utils/diff_test.go +++ b/pkg/utils/diff_test.go @@ -23,6 +23,9 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/infracloudio/botkube/pkg/config" ) @@ -76,10 +79,11 @@ type ExpectedDiff struct { func TestDiff(t *testing.T) { tests := map[string]struct { - old Object - new Object - update config.UpdateSetting - expected ExpectedDiff + old Object + new Object + update config.UpdateSetting + expected ExpectedDiff + expectedErrMessage string }{ `Spec Diff`: { old: Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "bar"}}, @@ -92,10 +96,10 @@ func TestDiff(t *testing.T) { }, }, `Non Spec Diff`: { - old: Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "bar"}}, - new: Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "boo"}}, - update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true}, - expected: ExpectedDiff{}, + old: Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "bar"}}, + new: Object{Spec: Spec{Containers: []Container{{Image: "nginx:1.14"}}}, Other: Other{Foo: "boo"}}, + update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true}, + expectedErrMessage: "while finding value from jsonpath: \"metadata.name\", object: {Spec:{Port:0 Containers:[{Image:nginx:1.14}]} Status:{Replicas:0} Data:{Properties:} Rules:{Verbs:} Other:{Foo:bar Annotations:map[]}}: metadata is not found", }, `Annotations changed`: { old: Object{Other: Other{Annotations: map[string]string{"app.kubernetes.io/version": "1"}}}, @@ -118,10 +122,10 @@ func TestDiff(t *testing.T) { }, }, `Non Status Diff`: { - old: Object{Status: Status{Replicas: 1}, Other: Other{Foo: "bar"}}, - new: Object{Status: Status{Replicas: 1}, Other: Other{Foo: "boo"}}, - update: config.UpdateSetting{Fields: []string{"metadata.labels"}, IncludeDiff: true}, - expected: ExpectedDiff{}, + old: Object{Status: Status{Replicas: 1}, Other: Other{Foo: "bar"}}, + new: Object{Status: Status{Replicas: 1}, Other: Other{Foo: "boo"}}, + update: config.UpdateSetting{Fields: []string{"metadata.labels"}, IncludeDiff: true}, + expectedErrMessage: "while finding value from jsonpath: \"metadata.labels\", object: {Spec:{Port:0 Containers:[]} Status:{Replicas:1} Data:{Properties:} Rules:{Verbs:} Other:{Foo:bar Annotations:map[]}}: metadata is not found", }, `Data Diff`: { old: Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "bar"}}, @@ -134,10 +138,10 @@ func TestDiff(t *testing.T) { }, }, `Non Data Diff`: { - old: Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "bar"}}, - new: Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "boo"}}, - update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true}, - expected: ExpectedDiff{}, + old: Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "bar"}}, + new: Object{Data: Data{Properties: "color: blue"}, Other: Other{Foo: "boo"}}, + update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true}, + expectedErrMessage: "while finding value from jsonpath: \"metadata.name\", object: {Spec:{Port:0 Containers:[]} Status:{Replicas:0} Data:{Properties:color: blue} Rules:{Verbs:} Other:{Foo:bar Annotations:map[]}}: metadata is not found", }, `Rules Diff`: { old: Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "bar"}}, @@ -150,18 +154,25 @@ func TestDiff(t *testing.T) { }, }, `Non Rules Diff`: { - old: Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "bar"}}, - new: Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "boo"}}, - update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true}, - expected: ExpectedDiff{}, + old: Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "bar"}}, + new: Object{Rules: Rules{Verbs: "list"}, Other: Other{Foo: "boo"}}, + update: config.UpdateSetting{Fields: []string{"metadata.name"}, IncludeDiff: true}, + expectedErrMessage: "while finding value from jsonpath: \"metadata.name\", object: {Spec:{Port:0 Containers:[]} Status:{Replicas:0} Data:{Properties:} Rules:{Verbs:list} Other:{Foo:bar Annotations:map[]}}: metadata is not found", }, } for name, test := range tests { name, test := name, test t.Run(name, func(t *testing.T) { - if actual := Diff(test.old, test.new, test.update); actual != test.expected.MockDiff() { - t.Errorf("expected: %+v != actual: %+v\n", test.expected.MockDiff(), actual) + actual, err := Diff(test.old, test.new, test.update) + + if test.expectedErrMessage != "" { + require.Error(t, err) + assert.Equal(t, test.expectedErrMessage, err.Error()) + return } + + require.NoError(t, err) + assert.Equal(t, test.expected.MockDiff(), actual) }) } } diff --git a/pkg/utils/jsonpath.go b/pkg/utils/jsonpath.go index fba2202aa..a47082b6c 100644 --- a/pkg/utils/jsonpath.go +++ b/pkg/utils/jsonpath.go @@ -44,7 +44,7 @@ func parseJsonpath(obj interface{}, jsonpathStr string) (string, error) { return "", err } - valueStrings := []string{} + var valueStrings []string if len(values) == 0 || len(values[0]) == 0 { valueStrings = append(valueStrings, "") } diff --git a/pkg/utils/lark.go b/pkg/utils/lark.go index 2a098b81a..52b177dec 100644 --- a/pkg/utils/lark.go +++ b/pkg/utils/lark.go @@ -27,12 +27,12 @@ import ( "github.com/larksuite/oapi-sdk-go/core" larkconfig "github.com/larksuite/oapi-sdk-go/core/config" im "github.com/larksuite/oapi-sdk-go/service/im/v1" - - "github.com/infracloudio/botkube/pkg/log" + "github.com/sirupsen/logrus" ) // LarkClient the client to communication with lark open platform type LarkClient struct { + log logrus.FieldLogger Conf *larkconfig.Config Service *im.Service } @@ -43,9 +43,9 @@ type TextMessage struct { } // NewLarkClient returns new Lark client -func NewLarkClient(conf *larkconfig.Config) *LarkClient { +func NewLarkClient(log logrus.FieldLogger, conf *larkconfig.Config) *LarkClient { imService := im.NewService(conf) - return &LarkClient{Conf: conf, Service: imService} + return &LarkClient{log: log, Conf: conf, Service: imService} } func (lark *LarkClient) marshalTextMessage(message string) (string, error) { @@ -59,12 +59,12 @@ func (lark *LarkClient) marshalTextMessage(message string) (string, error) { // SendTextMessage sending text message to lark group // See: https://open.larksuite.com/document/ukTMukTMukTM/uUjNz4SN2MjL1YzM -func (lark *LarkClient) SendTextMessage(receiveType, receiveID, msg string) error { +func (lark *LarkClient) SendTextMessage(ctx context.Context, receiveType, receiveID, msg string) error { content, err := lark.marshalTextMessage(msg) if err != nil { - return fmt.Errorf("Error in sending marshal text message: %s error: %s", msg, err.Error()) + return fmt.Errorf("while sending text message %q: %w", msg, err) } - coreCtx := core.WrapContext(context.Background()) + coreCtx := core.WrapContext(ctx) reqCall := lark.Service.Messages.Create(coreCtx, &im.MessageCreateReqBody{ ReceiveId: receiveID, Content: content, @@ -75,14 +75,13 @@ func (lark *LarkClient) SendTextMessage(receiveType, receiveID, msg string) erro if err != nil { return fmt.Errorf("Error in sending lark message: %s error: %s", msg, err.Error()) } - log.Debugf("Message successfully sent to channel: %s with message: %+v", receiveID, ret) + lark.log.Debugf("Message successfully sent to channel: %s with message: %+v", receiveID, ret) return nil } //GetLoggerLevel Unified Log Levels -func GetLoggerLevel() core.LoggerLevel { - logrusLevel := log.GetLevel() - switch int(logrusLevel) { +func GetLoggerLevel(loggerLevel logrus.Level) core.LoggerLevel { + switch int(loggerLevel) { case 0, 1, 2: return core.LoggerLevelError case 3: diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 22abdc6d8..160e39699 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -23,181 +23,26 @@ import ( "bytes" "context" "fmt" - "os" - "reflect" "regexp" - "strconv" "strings" - "time" - - "github.com/infracloudio/botkube/pkg/config" - log "github.com/infracloudio/botkube/pkg/log" coreV1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/discovery" - cacheddiscovery "k8s.io/client-go/discovery/cached" "k8s.io/client-go/dynamic" - "k8s.io/client-go/dynamic/dynamicinformer" - "k8s.io/client-go/rest" - "k8s.io/client-go/restmapper" - "k8s.io/client-go/tools/cache" - "k8s.io/client-go/tools/clientcmd" -) - -// TODO: Do not use global variables and convert them to local ones passed via references into all BotKube components -// See https://github.com/infracloudio/botkube/issues/589 -var ( - // ResourceInformerMap is a map of resource name to resource Getter interface - ResourceInformerMap map[string]cache.SharedIndexInformer - // AllowedEventKindsMap is a map to filter valid event kinds - AllowedEventKindsMap map[EventKind]bool - // AllowedUpdateEventsMap is a map of resource and namespace to updateconfig - AllowedUpdateEventsMap map[KindNS]config.UpdateSetting - // AllowedKubectlResourceMap is map of allowed resources with kubectl command - AllowedKubectlResourceMap map[string]bool - // AllowedKubectlVerbMap is map of allowed verb with kubectl command - AllowedKubectlVerbMap map[string]bool - // KindResourceMap contains resource name to kind mapping - KindResourceMap map[string]string - // ShortnameResourceMap contains resource name to short name mapping - ShortnameResourceMap map[string]string - // DynamicKubeClient is a global dynamic kubernetes client to communicate to apiserver - DynamicKubeClient dynamic.Interface - // DynamicKubeInformerFactory is a global DynamicSharedInformerFactory object to watch resources - DynamicKubeInformerFactory dynamicinformer.DynamicSharedInformerFactory - // Mapper is a global DeferredDiscoveryRESTMapper object, which maps all resources present on - // the cluster, and create relation between GVR, and GVK - Mapper *restmapper.DeferredDiscoveryRESTMapper - // DiscoveryClient implements - DiscoveryClient discovery.DiscoveryInterface ) const hyperlinkRegex = `(?m)` -// InitKubeClient creates K8s client from provided kubeconfig OR service account to interact with apiserver -func InitKubeClient() { - kubeConfig, err := rest.InClusterConfig() - if err != nil { - kubeconfigPath := os.Getenv("KUBECONFIG") - if kubeconfigPath == "" { - kubeconfigPath = os.Getenv("HOME") + "/.kube/config" - } - botkubeConf, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath) - if err != nil { - log.Fatal(err) - } - // Initiate discovery client for REST resource mapping - DiscoveryClient, err = discovery.NewDiscoveryClientForConfig(botkubeConf) - if err != nil { - log.Fatalf("Unable to create Discovery Client") - } - DynamicKubeClient, err = dynamic.NewForConfig(botkubeConf) - if err != nil { - log.Fatal(err) - } - } else { - // Initiate discovery client for REST resource mapping - DiscoveryClient, err = discovery.NewDiscoveryClientForConfig(kubeConfig) - if err != nil { - log.Fatal(err) - } - DynamicKubeClient, err = dynamic.NewForConfig(kubeConfig) - if err != nil { - log.Fatal(err) - } - } - - discoCacheClient := cacheddiscovery.NewMemCacheClient(DiscoveryClient) - discoCacheClient.Invalidate() - Mapper = restmapper.NewDeferredDiscoveryRESTMapper(discoCacheClient) -} - -// EventKind used in AllowedEventKindsMap to filter event kinds -type EventKind struct { - Resource string - Namespace string - EventType config.EventType -} - -// KindNS used in AllowedUpdateEventsMap -type KindNS struct { - Resource string - Namespace string -} - -// InitInformerMap initializes helper maps to filter events -func InitInformerMap(conf *config.Config) { - // Get resync period - rsyncTimeStr, ok := os.LookupEnv("INFORMERS_RESYNC_PERIOD") - if !ok { - rsyncTimeStr = "30" - } - rsyncTime, err := strconv.Atoi(rsyncTimeStr) - if err != nil { - log.Fatal("Error in reading INFORMERS_RESYNC_PERIOD env var.", err) - } - - // Create dynamic shared informer factory - DynamicKubeInformerFactory = dynamicinformer.NewDynamicSharedInformerFactory(DynamicKubeClient, time.Duration(rsyncTime)*time.Minute) - - // Init maps - ResourceInformerMap = make(map[string]cache.SharedIndexInformer) - AllowedEventKindsMap = make(map[EventKind]bool) - AllowedUpdateEventsMap = make(map[KindNS]config.UpdateSetting) - - for _, v := range conf.Resources { - gvr, err := ParseResourceArg(v.Name) - if err != nil { - log.Infof("Unable to parse resource: %v\n", v.Name) - continue - } - - ResourceInformerMap[v.Name] = DynamicKubeInformerFactory.ForResource(gvr).Informer() - } - // Allowed event kinds map and Allowed Update Events Map - for _, r := range conf.Resources { - allEvents := false - for _, e := range r.Events { - if e == config.AllEvent { - allEvents = true - break - } - for _, ns := range r.Namespaces.Include { - AllowedEventKindsMap[EventKind{Resource: r.Name, Namespace: ns, EventType: e}] = true - } - // AllowedUpdateEventsMap entry is created only for UpdateEvent - if e == config.UpdateEvent { - for _, ns := range r.Namespaces.Include { - AllowedUpdateEventsMap[KindNS{Resource: r.Name, Namespace: ns}] = r.UpdateSetting - } - } - } - - // For AllEvent type, add all events to map - if allEvents { - events := []config.EventType{config.CreateEvent, config.UpdateEvent, config.DeleteEvent, config.ErrorEvent} - for _, ev := range events { - for _, ns := range r.Namespaces.Include { - AllowedEventKindsMap[EventKind{Resource: r.Name, Namespace: ns, EventType: ev}] = true - AllowedUpdateEventsMap[KindNS{Resource: r.Name, Namespace: ns}] = r.UpdateSetting - } - } - } - } - log.Infof("Allowed Events - %+v", AllowedEventKindsMap) - log.Infof("Allowed UpdateEvents - %+v", AllowedUpdateEventsMap) -} - // GetObjectMetaData returns metadata of the given object -func GetObjectMetaData(obj interface{}) metaV1.ObjectMeta { +func GetObjectMetaData(ctx context.Context, dynamicCli dynamic.Interface, mapper meta.RESTMapper, obj interface{}) (metaV1.ObjectMeta, error) { unstructuredObject, ok := obj.(*unstructured.Unstructured) if !ok { - return metaV1.ObjectMeta{} + return metaV1.ObjectMeta{}, fmt.Errorf("cannot convert type %T into *unstructured.Unstructured", obj) } unstructuredObject = unstructuredObject.DeepCopy() objectMeta := metaV1.ObjectMeta{ @@ -219,18 +64,23 @@ func GetObjectMetaData(obj interface{}) metaV1.ObjectMeta { var eventObj coreV1.Event err := TransformIntoTypedObject(obj.(*unstructured.Unstructured), &eventObj) if err != nil { - log.Errorf("Unable to transform object type: %v, into type: %v", reflect.TypeOf(obj), reflect.TypeOf(eventObj)) + return metaV1.ObjectMeta{}, fmt.Errorf("while transforming object type: %T into type %T: %w", obj, eventObj, err) } - if len(objectMeta.Annotations) == 0 { - objectMeta.Annotations = ExtractAnnotationsFromEvent(&eventObj) - } else { - // Append InvolvedObject`s annotations to existing event object`s annotations map - for key, value := range ExtractAnnotationsFromEvent(&eventObj) { - objectMeta.Annotations[key] = value - } + + eventAnnotations, err := ExtractAnnotationsFromEvent(ctx, dynamicCli, mapper, &eventObj) + if err != nil { + return metaV1.ObjectMeta{}, err + } + + if objectMeta.Annotations == nil { + objectMeta.Annotations = make(map[string]string) + } + + for key, value := range eventAnnotations { + objectMeta.Annotations[key] = value } } - return objectMeta + return objectMeta, nil } // GetObjectTypeMetaData returns typemetadata of the given object @@ -248,7 +98,7 @@ func GetObjectTypeMetaData(obj interface{}) metaV1.TypeMeta { // DeleteDoubleWhiteSpace returns slice that removing whitespace from a arg slice func DeleteDoubleWhiteSpace(slice []string) []string { - result := []string{} + var result []string for _, s := range slice { if len(s) != 0 { result = append(result, s) @@ -258,8 +108,8 @@ func DeleteDoubleWhiteSpace(slice []string) []string { } // GetResourceFromKind returns resource name for given Kind -func GetResourceFromKind(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) { - mapping, err := Mapper.RESTMapping(gvk.GroupKind(), gvk.Version) +func GetResourceFromKind(mapper meta.RESTMapper, gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) { + mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { return schema.GroupVersionResource{}, fmt.Errorf("Error while creating REST Mapping for Event Involved Object: %v", err) } @@ -267,62 +117,20 @@ func GetResourceFromKind(gvk schema.GroupVersionKind) (schema.GroupVersionResour } // ExtractAnnotationsFromEvent returns annotations of InvolvedObject for the given event -func ExtractAnnotationsFromEvent(obj *coreV1.Event) map[string]string { - gvr, err := GetResourceFromKind(obj.InvolvedObject.GroupVersionKind()) +func ExtractAnnotationsFromEvent(ctx context.Context, dynamicCli dynamic.Interface, mapper meta.RESTMapper, obj *coreV1.Event) (map[string]string, error) { + gvr, err := GetResourceFromKind(mapper, obj.InvolvedObject.GroupVersionKind()) if err != nil { - log.Error(err) - return nil + return nil, err } - annotations, err := DynamicKubeClient.Resource(gvr).Namespace(obj.InvolvedObject.Namespace).Get(context.Background(), obj.InvolvedObject.Name, metaV1.GetOptions{}) + annotations, err := dynamicCli.Resource(gvr).Namespace(obj.InvolvedObject.Namespace).Get(ctx, obj.InvolvedObject.Name, metaV1.GetOptions{}) if err != nil { // IgnoreNotFound returns nil on NotFound errors. if apierrors.IsNotFound(err) { - return nil - } - log.Error(err) - return nil - } - return annotations.GetAnnotations() -} - -// InitResourceMap initializes helper maps to allow kubectl execution for required resources -func InitResourceMap(conf *config.Config) { - if !conf.Settings.Kubectl.Enabled { - return - } - KindResourceMap = make(map[string]string) - ShortnameResourceMap = make(map[string]string) - AllowedKubectlResourceMap = make(map[string]bool) - AllowedKubectlVerbMap = make(map[string]bool) - - for _, r := range conf.Settings.Kubectl.Commands.Resources { - AllowedKubectlResourceMap[r] = true - } - for _, r := range conf.Settings.Kubectl.Commands.Verbs { - AllowedKubectlVerbMap[r] = true - } - - _, resourceList, err := DiscoveryClient.ServerGroupsAndResources() - if err != nil { - log.Errorf("Failed to get resource list in k8s cluster. %v", err) - return - } - for _, resource := range resourceList { - for _, r := range resource.APIResources { - // Ignore subresources - if strings.Contains(r.Name, "/") { - continue - } - KindResourceMap[strings.ToLower(r.Kind)] = r.Name - for _, sn := range r.ShortNames { - ShortnameResourceMap[sn] = r.Name - } + return nil, nil } + return nil, err } - log.Infof("AllowedKubectlResourceMap - %+v", AllowedKubectlResourceMap) - log.Infof("AllowedKubectlVerbMap - %+v", AllowedKubectlVerbMap) - log.Infof("KindResourceMap - %+v", KindResourceMap) - log.Infof("ShortnameResourceMap - %+v", ShortnameResourceMap) + return annotations.GetAnnotations(), nil } //GetClusterNameFromKubectlCmd this will return cluster name from kubectl command @@ -337,24 +145,6 @@ func GetClusterNameFromKubectlCmd(cmd string) string { return s } -// ParseResourceArg parses the group/version/resource args and create a schema.GroupVersionResource -func ParseResourceArg(arg string) (schema.GroupVersionResource, error) { - var gvr schema.GroupVersionResource - if strings.Count(arg, "/") >= 2 { - s := strings.SplitN(arg, "/", 3) - gvr = schema.GroupVersionResource{Group: s[0], Version: s[1], Resource: s[2]} - } else if strings.Count(arg, "/") == 1 { - s := strings.SplitN(arg, "/", 2) - gvr = schema.GroupVersionResource{Group: "", Version: s[0], Resource: s[1]} - } - - // Validate the GVR provided - if _, err := Mapper.ResourcesFor(gvr); err != nil { - return schema.GroupVersionResource{}, err - } - return gvr, nil -} - // GVRToString converts GVR formats to string func GVRToString(gvr schema.GroupVersionResource) string { if gvr.Group == "" { @@ -380,21 +170,6 @@ func GetStringInYamlFormat(header string, commands map[string]bool) string { return b.String() } -// CheckOperationAllowed checks whether operation are allowed -func CheckOperationAllowed(eventMap map[EventKind]bool, namespace string, resource string, eventType config.EventType) bool { - if eventMap != nil && (eventMap[EventKind{ - Resource: resource, - Namespace: "all", - EventType: eventType}] || - eventMap[EventKind{ - Resource: resource, - Namespace: namespace, - EventType: eventType}]) { - return true - } - return false -} - // Contains tells whether a contains x. func Contains(a []string, x string) bool { for _, n := range a { diff --git a/test/e2e/command/botkube.go b/test/e2e/command/botkube.go index 0312ed1e0..fe185b4a2 100644 --- a/test/e2e/command/botkube.go +++ b/test/e2e/command/botkube.go @@ -124,15 +124,13 @@ func (c *context) testBotkubeCommand(t *testing.T) { assert.Equal(t, c.Config.Communications.Slack.Channel, m.Channel) switch test.command { case "filters list": - fl := compareFilters(strings.Split(test.expected, "\n"), strings.Split(strings.TrimSpace(strings.Trim(m.Text, "`")), "\n")) - assert.Equal(t, fl, true) + assert.ElementsMatch(t, strings.Split(test.expected, "\n"), strings.Split(strings.TrimSpace(strings.Trim(m.Text, "`")), "\n")) case "commands list --cluster-name test-cluster-2": fallthrough case "commands list --cluster-name test-cluster-1": fallthrough case "commands list": - cl := compareFilters(strings.Split(test.expected, "\n"), strings.Split(strings.TrimSpace(strings.Trim(m.Text, "`")), "\n")) - assert.Equal(t, cl, true) + assert.ElementsMatch(t, strings.Split(test.expected, "\n"), strings.Split(strings.TrimSpace(strings.Trim(m.Text, "`")), "\n")) default: assert.Equal(t, test.expected, m.Text) } @@ -141,27 +139,6 @@ func (c *context) testBotkubeCommand(t *testing.T) { } } -func compareFilters(expected, actual []string) bool { - if len(expected) != len(actual) { - return false - } - - // Compare slices - for _, a := range actual { - found := false - for _, e := range expected { - if a == e { - found = true - break - } - } - if !found { - return false - } - } - return true -} - // Test disable notification with BotKube notifier command // - disable notifier with '@BotKube notifier stop' // - create pod and verify BotKube doesn't send notification @@ -198,7 +175,7 @@ func (c *context) testNotifierCommand(t *testing.T) { } t.Run("create resource", func(t *testing.T) { // Inject an event into the fake client. - utils.CreateResource(t, pod) + utils.CreateResource(t, c.DynamicCli, pod) if c.TestEnv.Config.Communications.Slack.Enabled { // Get last seen slack message diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 769b175cd..2f78b8831 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -20,79 +20,135 @@ package e2e import ( + "context" "testing" "time" + "github.com/sirupsen/logrus" + logtest "github.com/sirupsen/logrus/hooks/test" "github.com/slack-go/slack" "github.com/stretchr/testify/require" + "github.com/vrischmann/envconfig" + "golang.org/x/sync/errgroup" "github.com/infracloudio/botkube/pkg/bot" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/controller" "github.com/infracloudio/botkube/pkg/execute" "github.com/infracloudio/botkube/pkg/filterengine" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/pkg/notify" - "github.com/infracloudio/botkube/pkg/utils" "github.com/infracloudio/botkube/test/e2e/command" "github.com/infracloudio/botkube/test/e2e/env" "github.com/infracloudio/botkube/test/e2e/filters" "github.com/infracloudio/botkube/test/e2e/notifier/create" "github.com/infracloudio/botkube/test/e2e/notifier/delete" - notifierErr "github.com/infracloudio/botkube/test/e2e/notifier/error" "github.com/infracloudio/botkube/test/e2e/notifier/update" "github.com/infracloudio/botkube/test/e2e/welcome" ) +const ( + componentLogFieldKey = "component" + botLogFieldKey = "bot" +) + +// Config contains the test configuration parameters. +type Config struct { + ConfigPath string `envconfig:"optional"` + InformersResyncPeriod time.Duration `envconfig:"default=30m"` + KubeconfigPath string `envconfig:"optional,KUBECONFIG"` +} + // TestRun run e2e integration tests func TestRun(t *testing.T) { + var appCfg Config + err := envconfig.Init(&appCfg) + require.NoError(t, err, "while loading app configuration") + // New Environment to run integration tests - testEnv, err := env.New() + testEnv, err := env.New(appCfg.ConfigPath) require.NoError(t, err) - // Set up global logger and filter engine - log.SetupGlobal() - filterengine.SetupGlobal() + ctx, cancelCtxFn := context.WithCancel(context.Background()) + defer cancelCtxFn() + + logger, _ := logtest.NewNullLogger() + errGroup := new(errgroup.Group) + + // Filter engine + filterEngine := filterengine.WithAllFilters(logger, testEnv.DynamicCli, testEnv.Mapper, testEnv.Config) // Fake notifiers var notifiers []notify.Notifier if testEnv.Config.Communications.Slack.Enabled { - fakeSlackNotifier := ¬ify.Slack{ - Channel: testEnv.Config.Communications.Slack.Channel, - NotifType: testEnv.Config.Communications.Slack.NotifType, - Client: slack.New(testEnv.Config.Communications.Slack.Token, slack.OptionAPIURL(testEnv.SlackServer.GetAPIURL())), - } + t.Log("Starting test Slack server") + testEnv.SetupFakeSlack() + fakeSlackNotifier := notify.NewSlack(logger, testEnv.Config.Communications.Slack) + fakeSlackNotifier.Client = slack.New(testEnv.Config.Communications.Slack.Token, slack.OptionAPIURL(testEnv.SlackServer.GetAPIURL())) notifiers = append(notifiers, fakeSlackNotifier) } if testEnv.Config.Communications.Webhook.Enabled { - fakeWebhookNotifier := ¬ify.Webhook{ - URL: testEnv.WebhookServer.GetAPIURL(), - } + t.Log("Starting fake webhook server") + testEnv.SetupFakeWebhook() + + fakeWebhookNotifier := notify.NewWebhook(logger, config.CommunicationsConfig{ + Webhook: config.Webhook{ + URL: testEnv.WebhookServer.GetAPIURL(), + }, + }) notifiers = append(notifiers, fakeWebhookNotifier) } - utils.DynamicKubeClient = testEnv.K8sClient - utils.DiscoveryClient = testEnv.DiscoFake - utils.Mapper = testEnv.Mapper - utils.InitInformerMap(testEnv.Config) - utils.InitResourceMap(testEnv.Config) + resMapping, err := execute.LoadResourceMappingIfShould( + logger.WithField(componentLogFieldKey, "Resource Mapping Loader"), + testEnv.Config, + testEnv.DiscoveryCli, + ) + require.NoError(t, err) - // Start controller with fake notifiers - go controller.RegisterInformers(testEnv.Config, notifiers) - t.Run("Welcome", welcome.E2ETests(testEnv)) + executorFactory := execute.NewExecutorFactory( + logger.WithField(componentLogFieldKey, "Executor"), + command.FakeCommandRunnerFunc, + *testEnv.Config, + filterEngine, + resMapping, + ) + + // Controller + + ctrl := controller.New( + logger.WithField(componentLogFieldKey, "Controller"), + testEnv.Config, + notifiers, + filterEngine, + appCfg.ConfigPath, + testEnv.DynamicCli, + testEnv.Mapper, + appCfg.InformersResyncPeriod, + ) + + testEnv.Ctrl = ctrl + + // Start test components + + errGroup.Go(func() error { + t.Log("Starting controller") + return ctrl.Start(ctx) + }) if testEnv.Config.Communications.Slack.Enabled { - // Start fake Slack bot - sb := NewFakeSlackBot(testEnv) - go func(t *testing.T) { - err := sb.Start() - require.NoError(t, err) - }(t) + t.Log("Starting fake Slack bot") + sb := NewFakeSlackBot(logger.WithField(botLogFieldKey, "Slack"), executorFactory, testEnv) + errGroup.Go(func() error { + return sb.Start(ctx) + }) } + // Start controller with fake notifiers + t.Run("Welcome", welcome.E2ETests(testEnv)) + time.Sleep(time.Second) // Make test suite @@ -102,30 +158,24 @@ func TestRun(t *testing.T) { "filters": filters.E2ETests(testEnv), "update": update.E2ETests(testEnv), "delete": delete.E2ETests(testEnv), - "error": notifierErr.E2ETests(testEnv), } // Run test suite for name, test := range suite { t.Run(name, test.Run) } + + t.Log("Cancelling context") + cancelCtxFn() + err = errGroup.Wait() + require.NoError(t, err) } // NewFakeSlackBot creates new mocked Slack bot -func NewFakeSlackBot(testenv *env.TestEnv) *bot.SlackBot { - log.Info("Starting fake Slack bot") - - return &bot.SlackBot{ - Token: testenv.Config.Communications.Slack.Token, - AllowKubectl: testenv.Config.Settings.Kubectl.Enabled, - RestrictAccess: testenv.Config.Settings.Kubectl.RestrictAccess, - ClusterName: testenv.Config.Settings.ClusterName, - ChannelName: testenv.Config.Communications.Slack.Channel, - SlackURL: testenv.SlackServer.GetAPIURL(), - BotID: testenv.SlackServer.BotID, - DefaultNamespace: testenv.Config.Settings.Kubectl.DefaultNamespace, - NewExecutorFn: func(msg string, allowKubectl, restrictAccess bool, defaultNamespace, clusterName string, platform config.BotPlatform, channelName string, isAuthChannel bool) execute.Executor { - return execute.NewExecutorWithCustomCommandRunner(msg, allowKubectl, restrictAccess, defaultNamespace, clusterName, platform, channelName, isAuthChannel, command.FakeCommandRunnerFunc) - }, - } +func NewFakeSlackBot(log logrus.FieldLogger, executorFactory bot.ExecutorFactory, testenv *env.TestEnv) *bot.SlackBot { + slackBot := bot.NewSlackBot(log, testenv.Config, executorFactory) + slackBot.SlackURL = testenv.SlackServer.GetAPIURL() + slackBot.BotID = testenv.SlackServer.BotID + + return slackBot } diff --git a/test/e2e/env/env.go b/test/e2e/env/env.go index c20310e6f..289ca7c48 100644 --- a/test/e2e/env/env.go +++ b/test/e2e/env/env.go @@ -20,6 +20,7 @@ package env import ( + "errors" "fmt" "testing" "time" @@ -28,6 +29,7 @@ import ( "github.com/slack-go/slack/slacktest" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/discovery" cacheddiscovery "k8s.io/client-go/discovery/cached" @@ -38,23 +40,37 @@ import ( samplev1alpha1 "k8s.io/sample-controller/pkg/apis/samplecontroller/v1alpha1" "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/controller" "github.com/infracloudio/botkube/test/e2e/utils" "github.com/infracloudio/botkube/test/webhook" ) // TestEnv to store objects required for e2e testing -// K8sClient : Fake K8s client to mock resource creation -// SlackServer : Fake Slack server -// SlackMessages: Channel to store incoming Slack messages from BotKube -// Config : BotKube config provided with config.yaml type TestEnv struct { - DiscoFake discovery.DiscoveryInterface - K8sClient dynamic.Interface - SlackServer *slacktest.Server - WebhookServer *webhook.Server + + // Ctrl is a pointer to the BotKube controller + Ctrl *controller.Controller + + // DiscoveryCli is a fake Discovery client + DiscoveryCli discovery.DiscoveryInterface + + // DynamicCli is a fake K8s client to mock resource creation + DynamicCli dynamic.Interface + + // Config is provided with config.yaml + Config *config.Config + + // SlackServer is a fake Slack server + SlackServer *slacktest.Server + + // SlackMessages is a channel to store incoming Slack messages from BotKube SlackMessages chan *slack.MessageEvent - Config *config.Config - Mapper *restmapper.DeferredDiscoveryRESTMapper + + // WebhookServer is a fake Webhook server + WebhookServer *webhook.Server + + // Mapper is a K8s resources mapper that uses DiscoveryCli + Mapper meta.RESTMapper } // E2ETest interface to run tests @@ -63,15 +79,16 @@ type E2ETest interface { } // New creates TestEnv and populate required objects -func New() (*TestEnv, error) { - testEnv := &TestEnv{} - +func New(configPath string) (*TestEnv, error) { // Loads test configuration for Integration Testing - conf, err := config.New() + conf, err := config.Load(configPath) if err != nil { return nil, fmt.Errorf("while loading configuration: %w", err) } - testEnv.Config = conf + + if conf == nil { + return nil, errors.New("while loading configuration: config cannot be nil") + } s := runtime.NewScheme() @@ -88,24 +105,20 @@ func New() (*TestEnv, error) { return nil, err } - testEnv.K8sClient = fake.NewSimpleDynamicClient(s) - testEnv.DiscoFake = kubeFake.NewSimpleClientset().Discovery() - discoCacheClient := cacheddiscovery.NewMemCacheClient(FakeCachedDiscoveryInterface()) - testEnv.Mapper = restmapper.NewDeferredDiscoveryRESTMapper(discoCacheClient) - - if testEnv.Config.Communications.Slack.Enabled { - testEnv.SlackMessages = make(chan *slack.MessageEvent, 1) - testEnv.SetupFakeSlack() - } - if testEnv.Config.Communications.Webhook.Enabled { - testEnv.SetupFakeWebhook() - } - - return testEnv, nil + return &TestEnv{ + Config: conf, + DynamicCli: fake.NewSimpleDynamicClient(s), + DiscoveryCli: kubeFake.NewSimpleClientset().Discovery(), + Mapper: restmapper.NewDeferredDiscoveryRESTMapper( + cacheddiscovery.NewMemCacheClient(FakeCachedDiscoveryInterface()), + ), + }, nil } // SetupFakeSlack create fake Slack server to mock Slack func (e *TestEnv) SetupFakeSlack() { + e.SlackMessages = make(chan *slack.MessageEvent, 1) + s := slacktest.NewTestServer() s.SetBotName("BotKube") go s.Start() diff --git a/test/e2e/filters/filters.go b/test/e2e/filters/filters.go index e88ab03b8..501e8956d 100644 --- a/test/e2e/filters/filters.go +++ b/test/e2e/filters/filters.go @@ -92,7 +92,7 @@ func (c *context) testFilters(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { // Inject an event into the fake client. - utils.CreateResource(t, test) + utils.CreateResource(t, c.DynamicCli, test) if c.TestEnv.Config.Communications.Slack.Enabled { // Get last seen slack message diff --git a/test/e2e/notifier/create/create.go b/test/e2e/notifier/create/create.go index af8099b4c..0620c3c53 100644 --- a/test/e2e/notifier/create/create.go +++ b/test/e2e/notifier/create/create.go @@ -105,7 +105,7 @@ func (c *context) testCreateResource(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { // Inject an event into the fake client. - testutils.CreateResource(t, test) + testutils.CreateResource(t, c.DynamicCli, test) if c.TestEnv.Config.Communications.Slack.Enabled { // Get last seen slack message @@ -128,7 +128,7 @@ func (c *context) testCreateResource(t *testing.T) { } resource := utils.GVRToString(test.GVR) - isAllowed := utils.CheckOperationAllowed(utils.AllowedEventKindsMap, test.Namespace, resource, config.CreateEvent) + isAllowed := c.Ctrl.ShouldSendEvent(test.Namespace, resource, config.CreateEvent) assert.Equal(t, isAllowed, true) }) } diff --git a/test/e2e/notifier/delete/delete.go b/test/e2e/notifier/delete/delete.go index 0cb01c0a2..81b104c31 100644 --- a/test/e2e/notifier/delete/delete.go +++ b/test/e2e/notifier/delete/delete.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/controller" "github.com/infracloudio/botkube/pkg/notify" "github.com/infracloudio/botkube/pkg/utils" "github.com/infracloudio/botkube/test/e2e/env" @@ -21,65 +22,16 @@ type context struct { *env.TestEnv } -func (c *context) testSKipDeleteEvent(t *testing.T) { - // Modifying AllowedEventKindsMap to remove delete event for pod resource - delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: "delete"}) - // Modifying AllowedEventKindsMap to add delete event for only dummy namespace and ignore everything - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "dummy", EventType: "delete"}] = true - // Modifying AllowedEventKindsMap to remove all event for service resource - events := []config.EventType{"delete", "error", "create"} - for _, event := range events { - delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/services", Namespace: "all", EventType: event}) - } - // test scenarios - tests := map[string]testutils.DeleteObjects{ - "skip delete event for resources not configured": { - // delete operation not allowed for Pod resources so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Kind: "Pod", - Namespace: "test", - Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-delete"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, - }, - "skip delete event for namespace not configured": { - // delete operation not allowed for Pod in test namespace so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Kind: "Pod", - Namespace: "test", - Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-delete"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, - }, - "skip delete event for resources not added in test_config": { - // delete operation not allowed for Pod resources so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "services"}, - Kind: "Service", - Namespace: "test", - Specs: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test-service-delete"}}, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - resource := utils.GVRToString(test.GVR) - // checking if delete operation is skipped - isAllowed := utils.CheckOperationAllowed(utils.AllowedEventKindsMap, test.Namespace, resource, config.DeleteEvent) - assert.Equal(t, isAllowed, false) - }) - } - // Resetting original configuration as per test_config - defer delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/pods", Namespace: "dummy", EventType: "delete"}) - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: "delete"}] = true - // Resetting original configuration as per test_config adding service resource - for _, event := range events { - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/services", Namespace: "all", EventType: event}] = true - } -} - func (c *context) testDeleteEvent(t *testing.T) { events := []config.EventType{"update", "error", "create"} // Modifying AllowedEventKindsMap to remove events other than delete for pod resource + observedEventKindsMap := c.Ctrl.ObservedEventKindsMap() for _, event := range events { - delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: event}) + delete(observedEventKindsMap, controller.EventKind{Resource: "v1/pods", Namespace: "all", EventType: event}) } + c.Ctrl.SetObservedEventKindsMap(observedEventKindsMap) + // test scenarios tests := map[string]testutils.DeleteObjects{ "perform delete operation and configure BotKube to listen only delete events": { @@ -101,10 +53,10 @@ func (c *context) testDeleteEvent(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { resource := utils.GVRToString(test.GVR) - isAllowed := utils.CheckOperationAllowed(utils.AllowedEventKindsMap, test.Namespace, resource, config.DeleteEvent) + isAllowed := c.Ctrl.ShouldSendEvent(test.Namespace, resource, config.DeleteEvent) assert.Equal(t, isAllowed, true) - testutils.DeleteResource(t, test) + testutils.DeleteResource(t, c.DynamicCli, test) if c.TestEnv.Config.Communications.Slack.Enabled { // Get last seen slack message @@ -131,14 +83,15 @@ func (c *context) testDeleteEvent(t *testing.T) { }) } // Resetting original configuration as per test_config + observedEventKindsMap = c.Ctrl.ObservedEventKindsMap() for _, event := range events { - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: event}] = true + observedEventKindsMap[controller.EventKind{Resource: "v1/pods", Namespace: "all", EventType: event}] = true } + c.Ctrl.SetObservedEventKindsMap(observedEventKindsMap) } // Run tests func (c *context) Run(t *testing.T) { - t.Run("skip delete event", c.testSKipDeleteEvent) t.Run("delete resource", c.testDeleteEvent) } diff --git a/test/e2e/notifier/error/error.go b/test/e2e/notifier/error/error.go deleted file mode 100644 index 8176ddda4..000000000 --- a/test/e2e/notifier/error/error.go +++ /dev/null @@ -1,84 +0,0 @@ -package error - -import ( - "testing" - - "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/infracloudio/botkube/pkg/config" - "github.com/infracloudio/botkube/pkg/utils" - "github.com/infracloudio/botkube/test/e2e/env" - testutils "github.com/infracloudio/botkube/test/e2e/utils" -) - -type context struct { - *env.TestEnv -} - -func (c *context) testSKipErrorEvent(t *testing.T) { - // Modifying AllowedEventKindsMap to add error event for only dummy namespace and ignore everything - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "dummy", EventType: "error"}] = true - - serviceEvent := utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/services", Namespace: "all", EventType: "error"}] - podEvent := utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: "error"}] - - // Modifying AllowedEventKindsMap to remove error event for pod resource - delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: "error"}) - - // Modifying AllowedEventKindsMap to remove error event for service resource - delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/services", Namespace: "all", EventType: "error"}) - - tests := map[string]testutils.ErrorEvent{ - "skip error event for resources not configured": { - // error event not allowed for Pod resources so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Kind: "Pod", - Namespace: "test", - Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, - }, - "skip error event for namespace not configured": { - // error event not allowed for Pod in test namespace so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Kind: "Pod", - Namespace: "test", - Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, - }, - "skip error event for resources not added in test_config": { - // error event should not be allowed for service resource so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "services"}, - Kind: "Service", - Namespace: "test", - Specs: &v1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test-service-error"}}, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - resource := utils.GVRToString(test.GVR) - // checking if error operation is skipped - isAllowed := utils.CheckOperationAllowed(utils.AllowedEventKindsMap, test.Namespace, resource, config.ErrorEvent) - assert.Equal(t, isAllowed, false) - }) - } - // Resetting original configuration as per test_config - defer func() { - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/services", Namespace: "all", EventType: "error"}] = serviceEvent - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: "error"}] = podEvent - delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/pods", Namespace: "dummy", EventType: "error"}) - }() -} - -// Run tests -func (c *context) Run(t *testing.T) { - t.Run("skip error event", c.testSKipErrorEvent) -} - -// E2ETests runs error notification tests -func E2ETests(testEnv *env.TestEnv) env.E2ETest { - return &context{ - testEnv, - } -} diff --git a/test/e2e/notifier/update/update.go b/test/e2e/notifier/update/update.go index ddbc1f2cb..117e772ee 100644 --- a/test/e2e/notifier/update/update.go +++ b/test/e2e/notifier/update/update.go @@ -6,11 +6,13 @@ import ( "github.com/slack-go/slack" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "github.com/infracloudio/botkube/pkg/config" + "github.com/infracloudio/botkube/pkg/controller" "github.com/infracloudio/botkube/pkg/notify" "github.com/infracloudio/botkube/pkg/utils" "github.com/infracloudio/botkube/test/e2e/env" @@ -98,20 +100,24 @@ func (c *context) testUpdateResource(t *testing.T) { t.Run(name, func(t *testing.T) { resource := utils.GVRToString(test.GVR) // checking if update operation is true - isAllowed := utils.AllowedEventKindsMap[utils.EventKind{ + observedEventKindsMap := c.Ctrl.ObservedEventKindsMap() + isAllowed := observedEventKindsMap[controller.EventKind{ Resource: resource, Namespace: "all", EventType: config.UpdateEvent}] || - utils.AllowedEventKindsMap[utils.EventKind{ + observedEventKindsMap[controller.EventKind{ Resource: resource, Namespace: test.Namespace, EventType: config.UpdateEvent}] assert.Equal(t, isAllowed, true) // modifying the update setting value as per testcases - utils.AllowedUpdateEventsMap[utils.KindNS{Resource: "v1/pods", Namespace: "all"}] = test.UpdateSetting + observedUpdateEventKindsMap := c.Ctrl.ObservedUpdateEventsMap() + observedUpdateEventKindsMap[controller.KindNS{Resource: "v1/pods", Namespace: "all"}] = test.UpdateSetting + c.Ctrl.SetObservedUpdateEventsMap(observedUpdateEventKindsMap) // getting the updated and old object - oldObj, newObj := testutils.UpdateResource(t, test) - updateMsg := utils.Diff(oldObj.Object, newObj.Object, test.UpdateSetting) + oldObj, newObj := testutils.UpdateResource(t, c.DynamicCli, test) + updateMsg, err := utils.Diff(oldObj.Object, newObj.Object, test.UpdateSetting) + require.NoError(t, err) assert.Equal(t, test.Diff, updateMsg) // Inject an event into the fake client. if c.TestEnv.Config.Communications.Slack.Enabled { @@ -143,7 +149,6 @@ func (c *context) testUpdateResource(t *testing.T) { // Run tests func (c *context) Run(t *testing.T) { t.Run("update resource", c.testUpdateResource) - t.Run("skip update event", c.testSKipUpdateEvent) t.Run("skip update event for wrong setting", c.testSkipWrongSetting) } @@ -154,52 +159,20 @@ func E2ETests(testEnv *env.TestEnv) env.E2ETest { } } -func (c *context) testSKipUpdateEvent(t *testing.T) { - // Modifying AllowedEventKindsMap configure dummy namespace for update event and ignore all - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "dummy", EventType: "update"}] = true - delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: "update"}) - // reset to original test config - defer delete(utils.AllowedEventKindsMap, utils.EventKind{Resource: "v1/pods", Namespace: "dummy", EventType: "update"}) - - // test scenarios - tests := map[string]testutils.UpdateObjects{ - "skip update event for namespaces not configured": { - // update operation not allowed for Pod in test namespace so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Kind: "Pod", - Namespace: "test", - Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-update"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, - }, - "skip update event for resources not added": { - // update operation not allowed for namespaces in test_config so event should be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}, - Kind: "Namespace", - Specs: &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "abc"}}, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - resource := utils.GVRToString(test.GVR) - // checking if update operation is true - isAllowed := utils.CheckOperationAllowed(utils.AllowedEventKindsMap, test.Namespace, resource, config.UpdateEvent) - assert.Equal(t, isAllowed, false) - }) - } - // Resetting original configuration as per test_config - utils.AllowedEventKindsMap[utils.EventKind{Resource: "v1/pods", Namespace: "all", EventType: "update"}] = true -} - func (c *context) testSkipWrongSetting(t *testing.T) { // test scenarios - tests := map[string]testutils.UpdateObjects{ + tests := map[string]struct { + updateObj testutils.UpdateObjects + expectedErrMessage string + }{ "skip update event for wrong updateSettings value": { - // update event given with wrong value of updateSettings which doesn't exist would be skipped - GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Kind: "Pod", - Namespace: "test", - Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-update-skip"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, - Patch: []byte(`{ + updateObj: testutils.UpdateObjects{ + // update event given with wrong value of updateSettings which doesn't exist would be skipped + GVR: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Kind: "Pod", + Namespace: "test", + Specs: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "test-pod-update-skip"}, Spec: v1.PodSpec{Containers: []v1.Container{{Name: "test-pod-container", Image: "tomcat:9.0.34"}}}}, + Patch: []byte(`{ "apiVersion": "v1", "kind": "Pod", "metadata": { @@ -216,25 +189,35 @@ func (c *context) testSkipWrongSetting(t *testing.T) { } } `), - // adding wrong field - UpdateSetting: config.UpdateSetting{Fields: []string{"spec.invalid"}, IncludeDiff: true}, - // diff calcuted should be empty because of error - Diff: "", + // adding wrong field + UpdateSetting: config.UpdateSetting{Fields: []string{"spec.invalid"}, IncludeDiff: true}, + // diff calcuted should be empty because of error + Diff: "", + }, + expectedErrMessage: "while finding value from jsonpath: \"spec.invalid\", object: map[apiVersion:v1 kind:Pod metadata:map[creationTimestamp: name:test-pod-update-skip namespace:test] spec:map[containers:[map[image:tomcat:9.0.34 name:test-pod-container resources:map[]]]] status:map[]]: invalid is not found", }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - resource := utils.GVRToString(test.GVR) + resource := utils.GVRToString(test.updateObj.GVR) // checking if update operation is true - isAllowed := utils.CheckOperationAllowed(utils.AllowedEventKindsMap, test.Namespace, resource, config.UpdateEvent) + isAllowed := c.Ctrl.ShouldSendEvent(test.updateObj.Namespace, resource, config.UpdateEvent) assert.Equal(t, isAllowed, true) // modifying the update setting value as per testcases - utils.AllowedUpdateEventsMap[utils.KindNS{Resource: "v1/pods", Namespace: "all"}] = test.UpdateSetting + + observedUpdateEventKindsMap := c.Ctrl.ObservedUpdateEventsMap() + observedUpdateEventKindsMap[controller.KindNS{Resource: "v1/pods", Namespace: "all"}] = test.updateObj.UpdateSetting + c.Ctrl.SetObservedUpdateEventsMap(observedUpdateEventKindsMap) + // getting the updated and old object - oldObj, newObj := testutils.UpdateResource(t, test) - updateMsg := utils.Diff(oldObj.Object, newObj.Object, test.UpdateSetting) - assert.Equal(t, test.Diff, updateMsg) + oldObj, newObj := testutils.UpdateResource(t, c.DynamicCli, test.updateObj) + updateMsg, err := utils.Diff(oldObj.Object, newObj.Object, test.updateObj.UpdateSetting) + if test.expectedErrMessage != "" { + require.EqualError(t, err, test.expectedErrMessage) + } + + assert.Equal(t, test.updateObj.Diff, updateMsg) }) } } diff --git a/test/e2e/utils/utils.go b/test/e2e/utils/utils.go index 8f367ba33..0cdf357d3 100644 --- a/test/e2e/utils/utils.go +++ b/test/e2e/utils/utils.go @@ -25,15 +25,16 @@ import ( "time" "github.com/slack-go/slack" + "github.com/stretchr/testify/require" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/dynamic" "github.com/infracloudio/botkube/pkg/config" "github.com/infracloudio/botkube/pkg/notify" - "github.com/infracloudio/botkube/pkg/utils" ) // SlackMessage structure @@ -98,66 +99,60 @@ type ErrorEvent struct { } // CreateResource with fake client -func CreateResource(t *testing.T, obj CreateObjects) { - // convert the runtime.Object to unstructured.Unstructured +func CreateResource(t *testing.T, dynamicCli dynamic.Interface, obj CreateObjects) { + t.Helper() + s := unstructured.Unstructured{} - k, ok := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.Specs) - if ok != nil { - t.Fatalf("Failed to convert pod object into unstructured") - } + k, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.Specs) + require.NoError(t, err, "while converting pod object into unstructured") + s.Object = k s.SetGroupVersionKind(obj.GVR.GroupVersion().WithKind(obj.Kind)) // Create resource - _, err := utils.DynamicKubeClient.Resource(obj.GVR).Namespace(obj.Namespace).Create(context.TODO(), &s, v1.CreateOptions{}) - if err != nil { - t.Fatalf("Failed to create %s: %v", obj.GVR.Resource, err) - } + _, err = dynamicCli.Resource(obj.GVR).Namespace(obj.Namespace).Create(context.TODO(), &s, v1.CreateOptions{}) + require.NoError(t, err, "while creating %q", obj.GVR.Resource) } // UpdateResource Create and update the obj and return old and new obj -func UpdateResource(t *testing.T, obj UpdateObjects) (*unstructured.Unstructured, *unstructured.Unstructured) { +func UpdateResource(t *testing.T, dynamicCli dynamic.Interface, obj UpdateObjects) (*unstructured.Unstructured, *unstructured.Unstructured) { + t.Helper() + s := unstructured.Unstructured{} - k, ok := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.Specs) - if ok != nil { - t.Fatalf("Failed to convert pod object into unstructured") - } + k, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.Specs) + require.NoError(t, err, "while converting pod object into unstructured") + s.Object = k s.SetGroupVersionKind(obj.GVR.GroupVersion().WithKind(obj.Kind)) // Create resource and get the old object - oldObj, err := utils.DynamicKubeClient.Resource(obj.GVR).Namespace(obj.Namespace).Create(context.TODO(), &s, v1.CreateOptions{}) - if err != nil { - t.Fatalf("Failed to create %s: %v", obj.GVR.Resource, err) - } + oldObj, err := dynamicCli.Resource(obj.GVR).Namespace(obj.Namespace).Create(context.TODO(), &s, v1.CreateOptions{}) + require.NoError(t, err, "while creating %q", obj.GVR.Resource) + // Mock the time delay involved in listening, filtering, and notifying events to all notifiers - time.Sleep(30 * time.Second) + time.Sleep(10 * time.Second) // TODO: Is that really needed? Improve it in https://github.com/infracloudio/botkube/issues/589 // Applying patch - newObj, err := utils.DynamicKubeClient.Resource(obj.GVR).Namespace(obj.Namespace).Patch(context.TODO(), s.GetName(), types.MergePatchType, obj.Patch, v1.PatchOptions{}) - if err != nil { - t.Fatalf("Failed to update %s: %v", obj.GVR.Resource, err) - } + newObj, err := dynamicCli.Resource(obj.GVR).Namespace(obj.Namespace).Patch(context.TODO(), s.GetName(), types.MergePatchType, obj.Patch, v1.PatchOptions{}) + require.NoError(t, err, "while updating %q", obj.GVR.Resource) + // Mock the time delay involved in listening, filtering, and notifying events to all notifiers - time.Sleep(30 * time.Second) + time.Sleep(10 * time.Second) // TODO: Is that really needed? Improve it in https://github.com/infracloudio/botkube/issues/589 return oldObj, newObj } // DeleteResource deletes the obj with fake client -func DeleteResource(t *testing.T, obj DeleteObjects) { +func DeleteResource(t *testing.T, dynamicCli dynamic.Interface, obj DeleteObjects) { + t.Helper() + s := unstructured.Unstructured{} - k, ok := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.Specs) - if ok != nil { - t.Fatalf("Failed to convert pod object into unstructured") - } + k, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.Specs) + require.NoError(t, err, "while converting pod object into unstructured") + s.Object = k s.SetGroupVersionKind(obj.GVR.GroupVersion().WithKind(obj.Kind)) - _, err := utils.DynamicKubeClient.Resource(obj.GVR).Namespace(obj.Namespace).Create(context.TODO(), &s, v1.CreateOptions{}) - if err != nil { - t.Fatalf("Failed to create %s: %v", obj.GVR.Resource, err) - } - // Delete resource - err = utils.DynamicKubeClient.Resource(obj.GVR).Namespace(obj.Namespace).Delete(context.TODO(), s.GetName(), v1.DeleteOptions{}) + _, err = dynamicCli.Resource(obj.GVR).Namespace(obj.Namespace).Create(context.TODO(), &s, v1.CreateOptions{}) + require.NoError(t, err, "while creating %q", obj.GVR.Resource) - if err != nil { - t.Fatalf("Failed to delete %s: %v", obj.GVR.Resource, err) - } + // Delete resource + err = dynamicCli.Resource(obj.GVR).Namespace(obj.Namespace).Delete(context.TODO(), s.GetName(), v1.DeleteOptions{}) + require.NoError(t, err, "while deleting %q", obj.GVR.Resource) } diff --git a/test/webhook/server.go b/test/webhook/server.go index bd54cb2c6..b9d3ae1ef 100644 --- a/test/webhook/server.go +++ b/test/webhook/server.go @@ -24,7 +24,6 @@ import ( "net/http" "net/http/httptest" - "github.com/infracloudio/botkube/pkg/log" "github.com/infracloudio/botkube/test/e2e/utils" ) @@ -61,7 +60,6 @@ func NewTestServer() *Server { // Start starts the test server func (s *Server) Start() { - log.Info("starting Mock Webhook server") s.server.Start() }