From c306ed58a0c2ec9d543bbdf2b673718444562e8b Mon Sep 17 00:00:00 2001 From: ricoberger Date: Mon, 27 Dec 2021 21:20:06 +0100 Subject: [PATCH] [elasticsearch] Add tests Add tests for Elasticsearch plugin to improve code coverage. --- CHANGELOG.md | 2 + cmd/kobs/plugins/plugins.go | 2 +- plugins/elasticsearch/elasticsearch.go | 27 ++-- plugins/elasticsearch/elasticsearch_test.go | 120 ++++++++++++++++ .../elasticsearch/pkg/instance/instance.go | 32 +++-- .../pkg/instance/instance_mock.go | 51 +++++++ .../pkg/instance/instance_test.go | 132 ++++++++++++++++++ plugins/grafana/pkg/instance/instance_test.go | 6 +- 8 files changed, 342 insertions(+), 30 deletions(-) create mode 100644 plugins/elasticsearch/elasticsearch_test.go create mode 100644 plugins/elasticsearch/pkg/instance/instance_mock.go create mode 100644 plugins/elasticsearch/pkg/instance/instance_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e9e16077..37ef9dffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ NOTE: As semantic versioning states all 0.y.z releases can contain breaking chan - [#256](https://github.com/kobsio/kobs/pull/256): [grafana] Add tests for Grafana plugin. - [#257](https://github.com/kobsio/kobs/pull/257): Add [codecov.io](https://about.codecov.io) for code coverage. - [#258](https://github.com/kobsio/kobs/pull/258): [rss] Add test for RSS plugin. +- [#261](https://github.com/kobsio/kobs/pull/261): [elasticsearch] Add test for Elasticsearch plugin. ### Fixed @@ -49,6 +50,7 @@ NOTE: As semantic versioning states all 0.y.z releases can contain breaking chan - [#240](https://github.com/kobsio/kobs/pull/240): [core] Switch from `github.com/sirupsen/logrus` to `go.uber.org/zap` for logging and enrich log lines via `context.Context`. - [#241](https://github.com/kobsio/kobs/pull/241): [core] :warning: _Breaking change:_ :warning: Rework authentication / authorization middleware and adjust the Custom Resource Definition for Users and Teams. - [#236](https://github.com/kobsio/kobs/pull/236): [core] Improve filtering in select components for various plugins. +- [#260](https://github.com/kobsio/kobs/pull/260): [opsgenie] Adjust permission handling and add actions for incidents. ## [v0.7.0](https://github.com/kobsio/kobs/releases/tag/v0.7.0) (2021-11-19) diff --git a/cmd/kobs/plugins/plugins.go b/cmd/kobs/plugins/plugins.go index dcf65b50f..559f38b2b 100644 --- a/cmd/kobs/plugins/plugins.go +++ b/cmd/kobs/plugins/plugins.go @@ -87,7 +87,7 @@ func Register(clusters *clusters.Clusters, config Config) chi.Router { usersRouter := users.Register(clusters, router.plugins, config.Users) dashboardsRouter := dashboards.Register(clusters, router.plugins, config.Dashboards) prometheusRouter, prometheusInstances := prometheus.Register(clusters, router.plugins, config.Prometheus) - elasticsearchRouter := elasticsearch.Register(clusters, router.plugins, config.Elasticsearch) + elasticsearchRouter := elasticsearch.Register(router.plugins, config.Elasticsearch) klogsRouter, klogsInstances := klogs.Register(clusters, router.plugins, config.Klogs) jaegerRouter := jaeger.Register(clusters, router.plugins, config.Jaeger) kialiRouter := kiali.Register(clusters, router.plugins, config.Kiali) diff --git a/plugins/elasticsearch/elasticsearch.go b/plugins/elasticsearch/elasticsearch.go index 6eadc4032..9ed780e7b 100644 --- a/plugins/elasticsearch/elasticsearch.go +++ b/plugins/elasticsearch/elasticsearch.go @@ -4,7 +4,6 @@ import ( "net/http" "strconv" - "github.com/kobsio/kobs/pkg/api/clusters" "github.com/kobsio/kobs/pkg/api/middleware/errresponse" "github.com/kobsio/kobs/pkg/api/plugins/plugin" "github.com/kobsio/kobs/pkg/log" @@ -21,16 +20,17 @@ const Route = "/elasticsearch" // Config is the structure of the configuration for the elasticsearch plugin. type Config []instance.Config -// Router implements the router for the resources plugin, which can be registered in the router for our rest api. +// Router implements the router for the Elasticsearch plugin, which can be registered in the router for our rest api. +// Next to the routes for the Elasticsearch plugin it also contains a list of all configured Elasticsearch instances. type Router struct { *chi.Mux - clusters *clusters.Clusters - instances []*instance.Instance + instances []instance.Instance } -func (router *Router) getInstance(name string) *instance.Instance { +// getInstance returns an instance by it's name. +func (router *Router) getInstance(name string) instance.Instance { for _, i := range router.instances { - if i.Name == name { + if i.GetName() == name { return i } } @@ -61,14 +61,14 @@ func (router *Router) getLogs(w http.ResponseWriter, r *http.Request) { parsedTimeStart, err := strconv.ParseInt(timeStart, 10, 64) if err != nil { log.Error(r.Context(), "Could not parse start time.", zap.Error(err)) - errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not parse start time") + errresponse.Render(w, r, err, http.StatusBadRequest, "Could not parse start time") return } parsedTimeEnd, err := strconv.ParseInt(timeEnd, 10, 64) if err != nil { log.Error(r.Context(), "Could not parse end time.", zap.Error(err)) - errresponse.Render(w, r, nil, http.StatusBadRequest, "Could not parse end time") + errresponse.Render(w, r, err, http.StatusBadRequest, "Could not parse end time") return } @@ -83,15 +83,11 @@ func (router *Router) getLogs(w http.ResponseWriter, r *http.Request) { } // Register returns a new router which can be used in the router for the kobs rest api. -func Register(clusters *clusters.Clusters, plugins *plugin.Plugins, config Config) chi.Router { - var instances []*instance.Instance +func Register(plugins *plugin.Plugins, config Config) chi.Router { + var instances []instance.Instance for _, cfg := range config { - instance, err := instance.New(cfg) - if err != nil { - log.Fatal(nil, "Could not create Elasticsearch instance.", zap.Error(err), zap.String("name", cfg.Name)) - } - + instance := instance.New(cfg) instances = append(instances, instance) plugins.Append(plugin.Plugin{ @@ -104,7 +100,6 @@ func Register(clusters *clusters.Clusters, plugins *plugin.Plugins, config Confi router := Router{ chi.NewRouter(), - clusters, instances, } diff --git a/plugins/elasticsearch/elasticsearch_test.go b/plugins/elasticsearch/elasticsearch_test.go new file mode 100644 index 000000000..440eec6bd --- /dev/null +++ b/plugins/elasticsearch/elasticsearch_test.go @@ -0,0 +1,120 @@ +package elasticsearch + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/kobsio/kobs/pkg/api/plugins/plugin" + "github.com/kobsio/kobs/plugins/elasticsearch/pkg/instance" + + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +var testConfig = Config{ + { + Name: "elasticsearch", + DisplayName: "Elasticsearch", + Description: "Elasticsearch can be used for the logs of your application.", + }, +} + +func TestGetLogs(t *testing.T) { + for _, tt := range []struct { + name string + url string + expectedStatusCode int + expectedBody string + prepare func(*instance.MockInstance) + }{ + { + name: "invalid instance name", + url: "/invalidname/logs", + expectedStatusCode: http.StatusBadRequest, + expectedBody: "{\"error\":\"Could not find instance name\"}\n", + prepare: func(mockInstance *instance.MockInstance) { + mockInstance.On("GetName").Return("elasticsearch") + }, + }, + { + name: "parse time start fails", + url: "/elasticsearch/logs?timeStart=test", + expectedStatusCode: http.StatusBadRequest, + expectedBody: "{\"error\":\"Could not parse start time: strconv.ParseInt: parsing \\\"test\\\": invalid syntax\"}\n", + prepare: func(mockInstance *instance.MockInstance) { + mockInstance.On("GetName").Return("elasticsearch") + }, + }, + { + name: "parse time end fails", + url: "/elasticsearch/logs?timeStart=0&timeEnd=test", + expectedStatusCode: http.StatusBadRequest, + expectedBody: "{\"error\":\"Could not parse end time: strconv.ParseInt: parsing \\\"test\\\": invalid syntax\"}\n", + prepare: func(mockInstance *instance.MockInstance) { + mockInstance.On("GetName").Return("elasticsearch") + }, + }, + { + name: "get logs error", + url: "/elasticsearch/logs?timeStart=0&timeEnd=0", + expectedStatusCode: http.StatusInternalServerError, + expectedBody: "{\"error\":\"Could not get logs: bad request\"}\n", + prepare: func(mockInstance *instance.MockInstance) { + mockInstance.On("GetName").Return("elasticsearch") + mockInstance.On("GetLogs", mock.Anything, "", int64(0), int64(0)).Return(nil, fmt.Errorf("bad request")) + }, + }, + { + name: "get logs success", + url: "/elasticsearch/logs?timeStart=0&timeEnd=0", + expectedStatusCode: http.StatusOK, + expectedBody: "{\"took\":0,\"hits\":0,\"documents\":null,\"buckets\":null}\n", + prepare: func(mockInstance *instance.MockInstance) { + mockInstance.On("GetName").Return("elasticsearch") + mockInstance.On("GetLogs", mock.Anything, "", int64(0), int64(0)).Return(&instance.Data{}, nil) + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + mockInstance := &instance.MockInstance{} + mockInstance.AssertExpectations(t) + tt.prepare(mockInstance) + + router := Router{chi.NewRouter(), []instance.Instance{mockInstance}} + router.Route("/{name}", func(r chi.Router) { + r.Get("/logs", router.getLogs) + }) + + req, _ := http.NewRequest(http.MethodGet, tt.url, nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("name", strings.Split(tt.url, "/")[1]) + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + + w := httptest.NewRecorder() + router.getLogs(w, req) + + require.Equal(t, tt.expectedStatusCode, w.Code) + require.Equal(t, tt.expectedBody, string(w.Body.Bytes())) + }) + } +} + +func TestRegister(t *testing.T) { + plugins := &plugin.Plugins{} + router := Register(plugins, testConfig) + + require.NotEmpty(t, router) + require.Equal(t, &plugin.Plugins{ + plugin.Plugin{ + Name: testConfig[0].Name, + DisplayName: testConfig[0].DisplayName, + Description: testConfig[0].Description, + Type: "elasticsearch", + }, + }, plugins) +} diff --git a/plugins/elasticsearch/pkg/instance/instance.go b/plugins/elasticsearch/pkg/instance/instance.go index 135b3f91c..c5073795b 100644 --- a/plugins/elasticsearch/pkg/instance/instance.go +++ b/plugins/elasticsearch/pkg/instance/instance.go @@ -25,16 +25,26 @@ type Config struct { Token string `json:"token"` } -// Instance represents a single Elasticsearch instance, which can be added via the configuration file. -type Instance struct { - Name string +// Instance is the interface which must be implemented by each configured Elasticsearch instance. +type Instance interface { + GetName() string + GetLogs(ctx context.Context, query string, timeStart, timeEnd int64) (*Data, error) +} + +type instance struct { + name string address string client *http.Client } +// Getname returns the name of the Elasticsearch instance, as it was configured by the user. +func (i *instance) GetName() string { + return i.name +} + // GetLogs returns the raw log documents and the buckets for the distribution of the logs accross the selected time // range. We have to pass a query, start and end time to the function. -func (i *Instance) GetLogs(ctx context.Context, query string, timeStart, timeEnd int64) (*Data, error) { +func (i *instance) GetLogs(ctx context.Context, query string, timeStart, timeEnd int64) (*Data, error) { var err error var body []byte var url string @@ -44,7 +54,7 @@ func (i *Instance) GetLogs(ctx context.Context, query string, timeStart, timeEnd log.Debug(ctx, "Run Elasticsearch query.", zap.ByteString("query", body)) - req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(body)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(body)) if err != nil { return nil, err } @@ -94,8 +104,10 @@ func (i *Instance) GetLogs(ctx context.Context, query string, timeStart, timeEnd return nil, fmt.Errorf("%s: %s", res.Error.Type, res.Error.Reason) } -// New returns a new Elasticsearch instance for the given configuration. -func New(config Config) (*Instance, error) { +// New returns a new Elasticsearch instance for the given configuration. If the configuration contains a username and +// password we will add a basic auth header to each request against the Elasticsearch api. If the config contains a +// token we are adding an authentication header with the token. +func New(config Config) Instance { roundTripper := roundtripper.DefaultRoundTripper if config.Username != "" && config.Password != "" { @@ -113,11 +125,11 @@ func New(config Config) (*Instance, error) { } } - return &Instance{ - Name: config.Name, + return &instance{ + name: config.Name, address: config.Address, client: &http.Client{ Transport: roundTripper, }, - }, nil + } } diff --git a/plugins/elasticsearch/pkg/instance/instance_mock.go b/plugins/elasticsearch/pkg/instance/instance_mock.go new file mode 100644 index 000000000..00fcd4aaf --- /dev/null +++ b/plugins/elasticsearch/pkg/instance/instance_mock.go @@ -0,0 +1,51 @@ +// Code generated by mockery v2.9.4. DO NOT EDIT. + +package instance + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// MockInstance is an autogenerated mock type for the Instance type +type MockInstance struct { + mock.Mock +} + +// GetLogs provides a mock function with given fields: ctx, query, timeStart, timeEnd +func (_m *MockInstance) GetLogs(ctx context.Context, query string, timeStart int64, timeEnd int64) (*Data, error) { + ret := _m.Called(ctx, query, timeStart, timeEnd) + + var r0 *Data + if rf, ok := ret.Get(0).(func(context.Context, string, int64, int64) *Data); ok { + r0 = rf(ctx, query, timeStart, timeEnd) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*Data) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string, int64, int64) error); ok { + r1 = rf(ctx, query, timeStart, timeEnd) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetName provides a mock function with given fields: +func (_m *MockInstance) GetName() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} diff --git a/plugins/elasticsearch/pkg/instance/instance_test.go b/plugins/elasticsearch/pkg/instance/instance_test.go new file mode 100644 index 000000000..32f519207 --- /dev/null +++ b/plugins/elasticsearch/pkg/instance/instance_test.go @@ -0,0 +1,132 @@ +package instance + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGetName(t *testing.T) { + instance := &instance{ + name: "elasticsearch", + } + + require.Equal(t, "elasticsearch", instance.GetName()) +} + +func TestGetLogs(t *testing.T) { + for _, tt := range []struct { + name string + address string + ctx context.Context + handlerFunc func(w http.ResponseWriter, r *http.Request) + expectedError bool + expectedData *Data + }{ + { + name: "invalid request", + ctx: nil, + handlerFunc: func(w http.ResponseWriter, r *http.Request) {}, + expectedError: true, + }, + { + name: "invalid request address", + address: "kobs.io", + ctx: context.Background(), + handlerFunc: func(w http.ResponseWriter, r *http.Request) {}, + expectedError: true, + }, + { + name: "invalid request with error response", + ctx: context.Background(), + handlerFunc: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error": {"root_cause": [{"type": "type", "reason": "reason"}], "type": "type", "reason": "reason"}, "status": 400}`)) + }, + expectedError: true, + }, + { + name: "invalid request without error response", + ctx: context.Background(), + handlerFunc: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error": []}`)) + }, + expectedError: true, + }, + { + name: "success request", + ctx: context.Background(), + handlerFunc: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"took": 100, "hits": {"total": {"value": 1000}, "hits": [{"test": "test"}]}, "aggregations": {"logcount": {"buckets": [{"key_as_string": "123", "key": 123, "doc_count": 1000}]}}}`)) + }, + expectedError: false, + expectedData: &Data{Took: 100, Hits: 1000, Documents: []map[string]interface{}{{"test": "test"}}, Buckets: []Bucket{{KeyAsString: "123", Key: 123, DocCount: 1000}}}, + }, + { + name: "success request with invalid json data", + ctx: context.Background(), + handlerFunc: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"took": [100]}`)) + }, + expectedError: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(tt.handlerFunc)) + defer ts.Close() + + if tt.address == "" { + tt.address = ts.URL + } + + instance := &instance{ + name: "elasticsearch", + address: tt.address, + client: ts.Client(), + } + + actualData, actualError := instance.GetLogs(tt.ctx, "", 0, 0) + if tt.expectedError { + require.Error(t, actualError) + } else { + require.NoError(t, actualError) + } + require.Equal(t, tt.expectedData, actualData) + }) + } +} + +func TestNew(t *testing.T) { + for _, tt := range []struct { + name string + config Config + }{ + { + name: "instance without auth", + config: Config{Name: "elasticsearch"}, + }, + { + name: "instance with basic auth", + config: Config{Name: "elasticsearch", Username: "admin", Password: "admin"}, + }, + { + name: "instance with token auth", + config: Config{Name: "elasticsearch", Token: "token"}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + instance := New(tt.config) + require.NotNil(t, instance) + }) + } +} diff --git a/plugins/grafana/pkg/instance/instance_test.go b/plugins/grafana/pkg/instance/instance_test.go index 2884204ba..f5999370f 100644 --- a/plugins/grafana/pkg/instance/instance_test.go +++ b/plugins/grafana/pkg/instance/instance_test.go @@ -257,15 +257,15 @@ func TestNew(t *testing.T) { config Config }{ { - name: "invalid instance name", + name: "instance without auth", config: Config{Name: "grafana"}, }, { - name: "get dashboards failed", + name: "instance with basic auth", config: Config{Name: "grafana", Username: "admin", Password: "admin"}, }, { - name: "get dashboards succeeded", + name: "instance with token auth", config: Config{Name: "grafana", Token: "token"}, }, } {