From 138a186294a620431f330962ba7e6f780c6e6867 Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Fri, 13 Nov 2020 14:11:51 +1100 Subject: [PATCH 1/6] feat: Empty `Test` that elides writes or a default db / rp This is useful for validating InfluxQL DDL queries that don't typically require writes or may have more complex setup requirements. --- influxql/_v1tests/server_helpers.go | 65 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/influxql/_v1tests/server_helpers.go b/influxql/_v1tests/server_helpers.go index 02e6edbc252..1f4dda11bbd 100644 --- a/influxql/_v1tests/server_helpers.go +++ b/influxql/_v1tests/server_helpers.go @@ -88,12 +88,14 @@ type Write struct { type Writes []*Write type Test struct { - orgID influxdb.ID - bucketID influxdb.ID - db string - rp string - writes Writes - queries []*Query + orgID influxdb.ID + bucketID influxdb.ID + db string + rp string + writes Writes + queries []*Query + noDefaultMapping bool + noWrites bool } func NewTest(db, rp string) Test { @@ -103,6 +105,12 @@ func NewTest(db, rp string) Test { } } +// NewEmptyTest creates an empty test without a default database and retention policy mapping or +// any expected writes. +func NewEmptyTest() Test { + return Test{noDefaultMapping: true, noWrites: true} +} + func (qt *Test) Run(ctx context.Context, t *testing.T, p *tests.DefaultPipeline) { t.Helper() fx, auth := qt.init(ctx, t, p) @@ -146,37 +154,32 @@ func (qt *Test) addQueries(q ...*Query) { func (qt *Test) init(ctx context.Context, t *testing.T, p *tests.DefaultPipeline) (fx pipeline.BaseFixture, auth *influxdb.Authorization) { t.Helper() - require.Greater(t, len(qt.writes), 0) - qt.orgID = p.DefaultOrgID qt.bucketID = p.DefaultBucketID fx = pipeline.NewBaseFixture(t, p.Pipeline, qt.orgID, qt.bucketID) - qt.writeTestData(ctx, t, fx.Admin) - p.Flush() - - writeOrg, err := influxdb.NewPermissionAtID(qt.orgID, influxdb.WriteAction, influxdb.OrgsResourceType, qt.orgID) - require.NoError(t, err) - - bucketWritePerm, err := influxdb.NewPermissionAtID(qt.bucketID, influxdb.WriteAction, influxdb.BucketsResourceType, qt.orgID) - require.NoError(t, err) - - bucketReadPerm, err := influxdb.NewPermissionAtID(qt.bucketID, influxdb.ReadAction, influxdb.BucketsResourceType, qt.orgID) - require.NoError(t, err) + if !qt.noWrites { + require.GreaterOrEqual(t, len(qt.writes), 0) + qt.writeTestData(ctx, t, fx.Admin) + p.Flush() + } - auth = tests.MakeAuthorization(qt.orgID, p.DefaultUserID, []influxdb.Permission{*writeOrg, *bucketWritePerm, *bucketReadPerm}) - ctx = icontext.SetAuthorizer(ctx, auth) - err = p.Launcher. - DBRPMappingServiceV2(). - Create(ctx, &influxdb.DBRPMappingV2{ - Database: qt.db, - RetentionPolicy: qt.rp, - Default: true, - OrganizationID: qt.orgID, - BucketID: qt.bucketID, - }) - require.NoError(t, err) + auth = tests.MakeAuthorization(qt.orgID, p.DefaultUserID, influxdb.OperPermissions()) + + if !qt.noDefaultMapping { + ctx = icontext.SetAuthorizer(ctx, auth) + err := p.Launcher. + DBRPMappingServiceV2(). + Create(ctx, &influxdb.DBRPMappingV2{ + Database: qt.db, + RetentionPolicy: qt.rp, + Default: true, + OrganizationID: qt.orgID, + BucketID: qt.bucketID, + }) + require.NoError(t, err) + } return } From 98a7fc789ce42716395338dff580a36eb16360e2 Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Fri, 13 Nov 2020 14:26:22 +1100 Subject: [PATCH 2/6] chore: Test to validate expected behavior of SHOW DATABASES --- influxql/_v1tests/query_test.go | 62 +++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/influxql/_v1tests/query_test.go b/influxql/_v1tests/query_test.go index 5b0af6ee869..941909ed4b5 100644 --- a/influxql/_v1tests/query_test.go +++ b/influxql/_v1tests/query_test.go @@ -7,6 +7,11 @@ import ( "strings" "testing" "time" + + "github.com/influxdata/influxdb/v2" + icontext "github.com/influxdata/influxdb/v2/context" + "github.com/influxdata/influxdb/v2/tests" + "github.com/stretchr/testify/require" ) // Ensure parameterized queries can be executed @@ -82,3 +87,60 @@ func TestServer_Query_Chunked(t *testing.T) { ctx := context.Background() test.Run(ctx, t, s) } + +func TestServer_Query_ShowDatabases(t *testing.T) { + t.Parallel() + s := OpenServer(t) + defer s.MustClose() + + ctx := context.Background() + ctx = icontext.SetAuthorizer(ctx, tests.MakeAuthorization(s.DefaultOrgID, s.DefaultUserID, influxdb.OperPermissions())) + + // create some buckets and mappings + buckets := []struct { + name string + db string + rp string + }{ + {"my-bucket", "my-bucket", "autogen"}, + {"telegraf/autogen", "telegraf", "autogen"}, + {"telegraf/1_week", "telegraf", "1_week"}, + {"telegraf/1_month", "telegraf", "1_month"}, + } + + for _, bi := range buckets { + b := influxdb.Bucket{ + OrgID: s.DefaultOrgID, + Type: influxdb.BucketTypeUser, + Name: bi.name, + RetentionPeriod: 0, + } + err := s.Launcher. + Launcher. + BucketService(). + CreateBucket(ctx, &b) + require.NoError(t, err) + + err = s.Launcher. + DBRPMappingServiceV2(). + Create(ctx, &influxdb.DBRPMappingV2{ + Database: bi.db, + RetentionPolicy: bi.rp, + Default: true, + OrganizationID: s.DefaultOrgID, + BucketID: b.ID, + }) + require.NoError(t, err) + } + + test := NewEmptyTest() + test.addQueries( + &Query{ + name: "show databases does not return duplicates", + command: "SHOW DATABASES", + exp: `{"results":[{"statement_id":0,"series":[{"name":"databases","columns":["name"],"values":[["my-bucket"],["telegraf"]]}]}]}`, + }, + ) + + test.Run(context.Background(), t, s) +} From 93825e6eb7ca101fd59e9d6d8afdd681bfb88258 Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Fri, 13 Nov 2020 14:26:37 +1100 Subject: [PATCH 3/6] fix: Track seen databases in map and skip duplicates --- v1/coordinator/statement_executor.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/v1/coordinator/statement_executor.go b/v1/coordinator/statement_executor.go index fdaebb606a6..c48bc416aae 100644 --- a/v1/coordinator/statement_executor.go +++ b/v1/coordinator/statement_executor.go @@ -317,14 +317,19 @@ func (e *StatementExecutor) createIterators(ctx context.Context, stmt *influxql. func (e *StatementExecutor) executeShowDatabasesStatement(ctx context.Context, q *influxql.ShowDatabasesStatement, ectx *query.ExecutionContext) (models.Rows, error) { row := &models.Row{Name: "databases", Columns: []string{"name"}} - // TODO(gianarb): How pagination works here? dbrps, _, err := e.DBRP.FindMany(ctx, influxdb.DBRPMappingFilterV2{ OrgID: &ectx.OrgID, }) if err != nil { return nil, err } + + seenDbs := make(map[string]struct{}, len(dbrps)) for _, dbrp := range dbrps { + if _, ok := seenDbs[dbrp.Database]; ok { + continue + } + perm, err := influxdb.NewPermissionAtID(dbrp.BucketID, influxdb.ReadAction, influxdb.BucketsResourceType, dbrp.OrganizationID) if err != nil { return nil, err @@ -336,6 +341,7 @@ func (e *StatementExecutor) executeShowDatabasesStatement(ctx context.Context, q } return nil, err } + seenDbs[dbrp.Database] = struct{}{} row.Values = append(row.Values, []interface{}{dbrp.Database}) } return []*models.Row{row}, nil From 2df34042f4a8363d9921b21270f381047ec7cae9 Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Fri, 13 Nov 2020 14:26:47 +1100 Subject: [PATCH 4/6] chore: Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6f253aea7d..4ef291498ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ 1. [19991](https://github.com/influxdata/influxdb/pull/19991): Use --skip-verify flag for backup/restore CLI command. 1. [19995](https://github.com/influxdata/influxdb/pull/19995): Don't auto-print help on influxd errors 1. [20012](https://github.com/influxdata/influxdb/pull/20012): Validate input paths to `influxd upgrade` up-front +1. [20017](https://github.com/influxdata/influxdb/pull/20017): Don't include duplicates for SHOW DATABASES ## v2.0.1 [2020-11-10] From 8ab8a5523ae347d0458e57059966a98f889d0a29 Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Mon, 16 Nov 2020 09:24:36 +1100 Subject: [PATCH 5/6] fix: PR Feedback --- influxql/_v1tests/server_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/influxql/_v1tests/server_helpers.go b/influxql/_v1tests/server_helpers.go index 1f4dda11bbd..0ec05985507 100644 --- a/influxql/_v1tests/server_helpers.go +++ b/influxql/_v1tests/server_helpers.go @@ -160,7 +160,7 @@ func (qt *Test) init(ctx context.Context, t *testing.T, p *tests.DefaultPipeline fx = pipeline.NewBaseFixture(t, p.Pipeline, qt.orgID, qt.bucketID) if !qt.noWrites { - require.GreaterOrEqual(t, len(qt.writes), 0) + require.Greater(t, len(qt.writes), 0) qt.writeTestData(ctx, t, fx.Admin) p.Flush() } From 90b9d15af1222ba8d57f5802247c17ca7d5d9ee7 Mon Sep 17 00:00:00 2001 From: Stuart Carnie Date: Tue, 17 Nov 2020 08:51:33 +1100 Subject: [PATCH 6/6] fix: Update tests.Client to use new HTTP client APIs --- tests/client.go | 50 ++++++++++++++----------------------------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/tests/client.go b/tests/client.go index 56a03b180f8..1a648df5533 100644 --- a/tests/client.go +++ b/tests/client.go @@ -11,8 +11,10 @@ import ( "github.com/influxdata/flux/csv" "github.com/influxdata/influxdb/v2" + "github.com/influxdata/influxdb/v2/authorization" influxhttp "github.com/influxdata/influxdb/v2/http" "github.com/influxdata/influxdb/v2/pkg/httpc" + "github.com/influxdata/influxdb/v2/tenant" ) type ClientConfig struct { @@ -32,6 +34,11 @@ type Client struct { Client *httpc.Client *influxhttp.Service + *authorization.AuthorizationClientService + *tenant.BucketClientService + *tenant.OrgClientService + *tenant.UserClientService + ClientConfig } @@ -52,9 +59,13 @@ func NewClient(endpoint string, config ClientConfig) (*Client, error) { return nil, err } return &Client{ - Client: hc, - Service: svc, - ClientConfig: config, + Client: hc, + Service: svc, + AuthorizationClientService: &authorization.AuthorizationClientService{Client: hc}, + BucketClientService: &tenant.BucketClientService{Client: hc}, + OrgClientService: &tenant.OrgClientService{Client: hc}, + UserClientService: &tenant.UserClientService{Client: hc}, + ClientConfig: config, }, nil } @@ -186,27 +197,6 @@ func (c *Client) MustCreateLabel(t *testing.T) influxdb.ID { return l.ID } -// MustCreateDocument creates a document or is a fatal error. -// Used in tests where the content of the document does not matter. -func (c *Client) MustCreateDocument(t *testing.T) influxdb.ID { - t.Helper() - - d := &influxdb.Document{ - ID: influxdb.ID(12123434), - Meta: influxdb.DocumentMeta{ - Name: "n1", - Type: "t1", - }, - Content: "howdy", - } - - err := c.CreateDocument(context.Background(), c.DocumentsNamespace, c.OrgID, d) - if err != nil { - t.Fatalf("unable to create document: %v", err) - } - return d.ID -} - // MustCreateCheck creates a check or is a fatal error. // Used in tests where the content of the check does not matter. func (c *Client) MustCreateCheck(t *testing.T) influxdb.ID { @@ -372,8 +362,6 @@ func (c *Client) MustCreateResource(t *testing.T, r influxdb.ResourceType) influ return c.MustCreateLabel(t) case influxdb.ViewsResourceType: // 12 t.Skip("Are views still a thing?") - case influxdb.DocumentsResourceType: // 13 - return c.MustCreateDocument(t) case influxdb.NotificationRuleResourceType: // 14 return c.MustCreateNotificationRule(t) case influxdb.NotificationEndpointResourceType: // 15 @@ -416,8 +404,6 @@ func (c *Client) DeleteResource(t *testing.T, r influxdb.ResourceType, id influx return c.DeleteLabel(ctx, id) case influxdb.ViewsResourceType: // 12 t.Skip("Are views still a thing?") - case influxdb.DocumentsResourceType: // 13 - return c.DeleteDocument(ctx, c.DocumentsNamespace, id) case influxdb.NotificationRuleResourceType: // 14 return c.DeleteNotificationRule(ctx, id) case influxdb.NotificationEndpointResourceType: // 15 @@ -514,14 +500,6 @@ func (c *Client) FindAll(t *testing.T, r influxdb.ResourceType) ([]influxdb.ID, } case influxdb.ViewsResourceType: // 12 t.Skip("Are views still a thing?") - case influxdb.DocumentsResourceType: // 13 - rs, err := c.GetDocuments(ctx, c.DocumentsNamespace, c.OrgID) - if err != nil { - return nil, err - } - for _, r := range rs { - ids = append(ids, r.ID) - } case influxdb.NotificationRuleResourceType: // 14 rs, _, err := c.FindNotificationRules(ctx, influxdb.NotificationRuleFilter{OrgID: &c.OrgID}) if err != nil {