diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4397097de..65bfe81bc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -189,7 +189,7 @@ For faster development, you can also build and run Botkube outside K8s cluster. > Each time you make a change to the [source](cmd/source) or [executors](cmd/executor) plugins re-run the above command. > **Note** - > To build specific plugin binaries, use `PLUGIN_TARGETS`. For example `PLUGIN_TARGETS="x, kubectl" make build-plugins-single`. + > To build specific plugin binaries, use `PLUGIN_TARGETS`. For example `PLUGIN_TARGETS="kubernetes,echo" make build-plugins-single`. ## Making A Change diff --git a/Makefile b/Makefile index f7767b57a..ff7caa4a0 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,13 @@ .DEFAULT_GOAL := build -.PHONY: container-image test test-integration-slack test-integration-discord build pre-build publish lint lint-fix go-import-fmt system-check save-images load-and-push-images gen-grpc-resources gen-plugins-index build-plugins build-plugins-single gen-docs-cli gen-plugins-goreleaser +.PHONY: container-image test test-integration-slack test-integration-discord build pre-build publish lint lint-fix go-import-fmt system-check save-images load-and-push-images gen-grpc-resources gen-plugins-index build-plugins build-plugins-single gen-docs-cli gen-plugins-goreleaser serve-local-plugins # Show this help. help: @awk '/^#/{c=substr($$0,3);next}c&&/^[[:alpha:]][[:alnum:]_-]+:/{print substr($$1,1,index($$1,":")),c}1{c=0}' $(MAKEFILE_LIST) | column -s: -t +serve-local-plugins: ## Serve local plugins + go run hack/target/serve-plugins/main.go + lint-fix: go-import-fmt @go mod tidy @go mod verify diff --git a/internal/source/kubernetes/config/config.go b/internal/source/kubernetes/config/config.go index 0b036998f..88755f8f5 100644 --- a/internal/source/kubernetes/config/config.go +++ b/internal/source/kubernetes/config/config.go @@ -1,6 +1,7 @@ package config import ( + "errors" "fmt" "regexp" "strings" @@ -38,10 +39,40 @@ type ( DisplayName string `yaml:"displayName"` } Trigger struct { - Type []string `yaml:"type"` + Type []EventType `yaml:"type"` } ) +func (b *ExtraButtons) NormalizeAndValidate() error { + // the multierr pkg is not used as it breaks the final error indent making it hard to read + var issues []string + if b.Button.DisplayName == "" { + issues = append(issues, "displayName cannot be empty") + } + + if b.Button.CommandTpl == "" { + issues = append(issues, "commandTpl cannot be empty") + } + + if b.Trigger.Type == nil { + issues = append(issues, "trigger.type cannot be empty") + } + for idx, t := range b.Trigger.Type { + // we can't normalize it on custom 'UnmarshalYAML' because koanf uses map[string]any + // that causes it to drop Go struct as custom UnmarshalYAML. + t = EventType(strings.ToLower(string(t))) + if !t.IsValid() { + issues = append(issues, fmt.Sprintf("unknown trigger.type[%q]", t)) + } + b.Trigger.Type[idx] = t + } + + if len(issues) > 0 { + return errors.New(strings.Join(issues, ", ")) + } + return nil +} + // Commands contains allowed verbs and resources type Commands struct { Verbs []string `yaml:"verbs"` @@ -179,8 +210,24 @@ const ( AllEvent EventType = "all" ) -func (eventType EventType) String() string { - return string(eventType) +// IsValid checks if the event type is valid. +func (eventType *EventType) IsValid() bool { + if eventType == nil { + return false + } + switch *eventType { + case CreateEvent, UpdateEvent, DeleteEvent, ErrorEvent, WarningEvent, NormalEvent, InfoEvent, AllEvent: + return true + } + return false +} + +// String returns the string representation of the event type. +func (eventType *EventType) String() string { + if eventType == nil { + return "" + } + return string(*eventType) } // Resource contains resources to watch diff --git a/internal/source/kubernetes/filterengine/filterengine.go b/internal/source/kubernetes/filterengine/filterengine.go index e7f3dc014..31c99dc78 100644 --- a/internal/source/kubernetes/filterengine/filterengine.go +++ b/internal/source/kubernetes/filterengine/filterengine.go @@ -69,7 +69,7 @@ func (f *DefaultFilterEngine) Run(ctx context.Context, event event.Event) event. // Register filter(s) to engine. func (f *DefaultFilterEngine) Register(filters ...RegisteredFilter) { for _, filter := range filters { - f.log.Infof("Registering filter %q (enabled: %t)...", filter.Name(), filter.Enabled) + f.log.Debugf("Registering filter %q (enabled: %t)...", filter.Name(), filter.Enabled) f.filters[filter.Name()] = filter } } diff --git a/internal/source/kubernetes/msg.go b/internal/source/kubernetes/msg.go index 9fa05ade7..66864977a 100644 --- a/internal/source/kubernetes/msg.go +++ b/internal/source/kubernetes/msg.go @@ -7,12 +7,13 @@ import ( sprig "github.com/go-task/slim-sprig" "github.com/sirupsen/logrus" - "k8s.io/kubectl/pkg/util/slice" + "golang.org/x/exp/slices" "github.com/kubeshop/botkube/internal/source/kubernetes/commander" "github.com/kubeshop/botkube/internal/source/kubernetes/config" "github.com/kubeshop/botkube/internal/source/kubernetes/event" "github.com/kubeshop/botkube/pkg/api" + multierrx "github.com/kubeshop/botkube/pkg/multierror" ) var emojiForLevel = map[config.Level]string{ @@ -53,12 +54,12 @@ func (m *MessageBuilder) FromEvent(event event.Event, actions []config.ExtraButt cmdSection, err := m.getCommandSelectIfShould(event) if err != nil { - return api.Message{}, err + m.log.Errorf("Failed to get commands buttons assigned to %q event. Those buttons will be omitted. Issues:\n%s", event.Type.String(), err) } - btns, err := m.getExternalActions(actions, event) + btns, err := m.getExtraButtonsAssignedToEvent(actions, event) if err != nil { - return api.Message{}, err + m.log.Errorf("Failed to convert extra buttons assigned to %q event. Those buttons will be omitted. Issues:\n%s", event.Type.String(), err) } if cmdSection != nil || len(btns) > 0 { msg.Sections = append(msg.Sections, api.Section{ @@ -70,24 +71,33 @@ func (m *MessageBuilder) FromEvent(event event.Event, actions []config.ExtraButt return msg, nil } -func (m *MessageBuilder) getExternalActions(actions []config.ExtraButtons, e event.Event) (api.Buttons, error) { +func (m *MessageBuilder) getExtraButtonsAssignedToEvent(actions []config.ExtraButtons, e event.Event) (api.Buttons, error) { var actBtns api.Buttons - for _, act := range actions { + issues := multierrx.New() + for idx, act := range actions { if !act.Enabled { continue } - if !slice.ContainsString(act.Trigger.Type, e.Type.String(), nil) { + + err := act.NormalizeAndValidate() + if err != nil { + issues = multierrx.Append(issues, fmt.Errorf("invalid extraButtons[%d]: %s", idx, err)) + continue + } + + if !slices.Contains(act.Trigger.Type, e.Type) { continue } btn, err := m.renderActionButton(act, e) if err != nil { - return nil, err + issues = multierrx.Append(issues, fmt.Errorf("invalid extraButtons[%d].commandTpl: %s", idx, err)) + continue } actBtns = append(actBtns, btn) } - return actBtns, nil + return actBtns, issues.ErrorOrNil() } func ptrSection(s *api.Selects) api.Selects { @@ -175,7 +185,7 @@ func (m *MessageBuilder) appendBulletListIfNotEmpty(bulletLists api.BulletLists, } func (m *MessageBuilder) renderActionButton(act config.ExtraButtons, e event.Event) (api.Button, error) { - tmpl, err := template.New("example").Funcs(sprig.FuncMap()).Parse(act.Button.CommandTpl) + tmpl, err := template.New(act.Button.DisplayName).Funcs(sprig.FuncMap()).Parse(act.Button.CommandTpl) if err != nil { return api.Button{}, err } diff --git a/internal/source/kubernetes/msg_test.go b/internal/source/kubernetes/msg_test.go new file mode 100644 index 000000000..b58adea46 --- /dev/null +++ b/internal/source/kubernetes/msg_test.go @@ -0,0 +1,81 @@ +package kubernetes + +import ( + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/kubeshop/botkube/internal/source/kubernetes/config" + "github.com/kubeshop/botkube/internal/source/kubernetes/event" +) + +func TestGetExtraButtonsAssignedToEvent(t *testing.T) { + // given + builder := MessageBuilder{} + givenButtons := []config.ExtraButtons{ + { + // This is fully valid + Enabled: true, + Trigger: config.Trigger{ + Type: []config.EventType{"error"}, + }, + Button: config.Button{ + DisplayName: "Ask AI", + CommandTpl: "ai --resource={{ .Namespace }}/{{ .Kind | lower }}/{{ .Name }} --error={{ .Reason }} --bk-cmd-header='AI assistance'", + }, + }, + { + // This is valid, as the 'ERROR' type should be normalized to "error" + Enabled: true, + Trigger: config.Trigger{ + Type: []config.EventType{"ERROR"}, + }, + Button: config.Button{ + DisplayName: "Get", + CommandTpl: "kubectl get {{ .Kind | lower }}", + }, + }, + { + // This is invalid, as we can't render `.Event` + Enabled: true, + Trigger: config.Trigger{ + Type: []config.EventType{"error"}, + }, + Button: config.Button{ + DisplayName: "Ask AI v2", + CommandTpl: "ai {{.Event.Namespace}} this one is wrong", + }, + }, + { + // This is invalid, as the DisplayName and Trigger is not set + Enabled: true, + Trigger: config.Trigger{}, + Button: config.Button{ + CommandTpl: "ai {{.Event.Namespace}} this one is wrong", + }, + }, + { + // This is invalid, but should be ignored as it's disabled + Enabled: false, + Trigger: config.Trigger{}, + Button: config.Button{ + CommandTpl: "ai {{.Event.Namespace}} this one is wrong", + }, + }, + } + + // when + gotBtns, err := builder.getExtraButtonsAssignedToEvent(givenButtons, event.Event{Type: "error"}) + assert.EqualError(t, err, heredoc.Doc(` + 2 errors occurred: + * invalid extraButtons[2].commandTpl: template: Ask AI v2:1:11: executing "Ask AI v2" at <.Event.Namespace>: can't evaluate field Event in type event.Event + * invalid extraButtons[3]: displayName cannot be empty, trigger.type cannot be empty`)) + + // then + require.Len(t, gotBtns, 2) + for idx, btn := range gotBtns { + assert.Equal(t, givenButtons[idx].Button.DisplayName, btn.Name) + } +} diff --git a/internal/source/kubernetes/source.go b/internal/source/kubernetes/source.go index 961c305a0..03711bda7 100644 --- a/internal/source/kubernetes/source.go +++ b/internal/source/kubernetes/source.go @@ -84,13 +84,19 @@ func (*Source) Stream(ctx context.Context, input source.StreamInput) (source.Str if err != nil { return source.StreamOutput{}, fmt.Errorf("while merging input configs: %w", err) } + + // In Kubernetes, we have an "info" level by default. We should aim to minimize info logging and consider using + // the debug level instead. This approach will prevent flooding the Agent logs with irrelevant information, + // as the Agent logs everything that plugin writes to stderr. + log := loggerx.NewStderr(pkgConfig.Logger{ + Level: cfg.Log.Level, + }) + s := Source{ - startTime: time.Now(), - eventCh: make(chan source.Event), - config: cfg, - logger: loggerx.New(pkgConfig.Logger{ - Level: cfg.Log.Level, - }), + startTime: time.Now(), + eventCh: make(chan source.Event), + config: cfg, + logger: log, clusterName: input.Context.ClusterName, kubeConfig: input.Context.KubeConfig, isInteractivitySupported: input.Context.IsInteractivitySupported, @@ -135,7 +141,7 @@ func consumeEvents(ctx context.Context, s Source) { }, func(resource string) (cache.SharedIndexInformer, error) { gvr, err := parseResourceArg(resource, client.mapper) if err != nil { - s.logger.Infof("Unable to parse resource: %s to register with informer\n", resource) + s.logger.Errorf("Unable to parse resource: %s to register with informer\n", resource) return nil, err } return dynamicKubeInformerFactory.ForResource(gvr).Informer(), nil diff --git a/pkg/config/config.go b/pkg/config/config.go index 7670a0e29..6e84a06aa 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -620,8 +620,8 @@ const ( // FormatterText text formatter for logging FormatterText Formatter = "text" - // FormatterJson json formatter for logging - FormatterJson Formatter = "json" + // FormatterJSON json formatter for logging + FormatterJSON Formatter = "json" ) // Logger holds logger configuration parameters. diff --git a/pkg/loggerx/logger.go b/pkg/loggerx/logger.go index 2f109613f..0b199a49b 100644 --- a/pkg/loggerx/logger.go +++ b/pkg/loggerx/logger.go @@ -1,6 +1,7 @@ package loggerx import ( + "io" "os" "github.com/sirupsen/logrus" @@ -9,10 +10,24 @@ import ( ) // New returns a new logger based on a given configuration. +// It logs to stdout by default. It's a helper function to maintain backward compatibility. func New(cfg config.Logger) logrus.FieldLogger { + return NewStdout(cfg) +} + +// NewStderr returns a new logger based on a given configuration. It logs to stderr. +func NewStderr(cfg config.Logger) logrus.FieldLogger { + return newWithOutput(cfg, os.Stderr) +} + +// NewStdout returns a new logger based on a given configuration. It logs to stdout. +func NewStdout(cfg config.Logger) logrus.FieldLogger { + return newWithOutput(cfg, os.Stdout) +} + +func newWithOutput(cfg config.Logger, output io.Writer) logrus.FieldLogger { logger := logrus.New() - // Output to stdout instead of the default stderr - logger.SetOutput(os.Stdout) + logger.SetOutput(output) // Only logger the warning severity or above. logLevel, err := logrus.ParseLevel(cfg.Level) @@ -21,7 +36,7 @@ func New(cfg config.Logger) logrus.FieldLogger { logLevel = logrus.InfoLevel } logger.SetLevel(logLevel) - if cfg.Formatter == config.FormatterJson { + if cfg.Formatter == config.FormatterJSON { logger.Formatter = &logrus.JSONFormatter{} } else { logger.Formatter = &logrus.TextFormatter{FullTimestamp: true, DisableColors: cfg.DisableColors, ForceColors: true}