diff --git a/query/preauthorizer_test.go b/query/preauthorizer_test.go index 5c7f05b7e8..8335b9ef13 100644 --- a/query/preauthorizer_test.go +++ b/query/preauthorizer_test.go @@ -29,45 +29,56 @@ func newBucketServiceWithOneBucket(bucket platform.Bucket) platform.BucketServic } func TestPreAuthorizer_PreAuthorize(t *testing.T) { - // TODO(adam) add this test back when BucketsAccessed is restored for the from function - // https://github.com/influxdata/flux/issues/114 - t.Skip("https://github.com/influxdata/flux/issues/114") ctx := context.Background() now := time.Now().UTC() - - q := `from(bucket:"my_bucket") |> range(start:-2h) |> yield()` - spec, err := flux.Compile(ctx, q, now) - if err != nil { - t.Fatalf("Error compiling query: %v", err) - } - - // Try to pre-authorize with bucket service with no buckets - // and no authorization + // fresh pre-authorizer auth := &platform.Authorization{Status: platform.Active} emptyBucketService := mock.NewBucketService() preAuthorizer := query.NewPreAuthorizer(emptyBucketService) + // Try to pre-authorize invalid bucketID + q := `from(bucketID:"invalid") |> range(start:-2h) |> yield()` + spec, err := flux.Compile(ctx, q, now) + if err != nil { + t.Fatalf("Error compiling query: %v", err) + } err = preAuthorizer.PreAuthorize(ctx, spec, auth) - if diagnostic := cmp.Diff("Bucket service returned nil bucket", err.Error()); diagnostic != "" { + if diagnostic := cmp.Diff("bucket service returned nil bucket", err.Error()); diagnostic != "" { + t.Errorf("Authorize message mismatch: -want/+got:\n%v", diagnostic) + } + + // Try to pre-authorize a valid from with bucket service with no buckets + // and no authorization + q = `from(bucket:"my_bucket") |> range(start:-2h) |> yield()` + spec, err = flux.Compile(ctx, q, now) + if err != nil { + t.Fatalf("Error compiling query: %v", err) + } + err = preAuthorizer.PreAuthorize(ctx, spec, auth) + if diagnostic := cmp.Diff("bucket service returned nil bucket", err.Error()); diagnostic != "" { t.Errorf("Authorize message mismatch: -want/+got:\n%v", diagnostic) } // Try to authorize with a bucket service that knows about one bucket // (still no authorization) - id, _ := platform.IDFromString("deadbeefdeadbeef") + bucketID, err := platform.IDFromString("deadbeefdeadbeef") + if err != nil { + t.Fatal(err) + } + orgID := platform.ID(1) bucketService := newBucketServiceWithOneBucket(platform.Bucket{ - Name: "my_bucket", - ID: *id, + Name: "my_bucket", + ID: *bucketID, + OrganizationID: orgID, }) preAuthorizer = query.NewPreAuthorizer(bucketService) err = preAuthorizer.PreAuthorize(ctx, spec, auth) - if diagnostic := cmp.Diff(`No read permission for bucket: "my_bucket"`, err.Error()); diagnostic != "" { + if diagnostic := cmp.Diff(`no read permission for bucket: "my_bucket"`, err.Error()); diagnostic != "" { t.Errorf("Authorize message mismatch: -want/+got:\n%v", diagnostic) } - orgID := platform.ID(1) - p, err := platform.NewPermissionAtID(*id, platform.ReadAction, platform.BucketsResourceType, orgID) + p, err := platform.NewPermissionAtID(*bucketID, platform.ReadAction, platform.BucketsResourceType, orgID) if err != nil { t.Fatalf("Error creating read bucket permission query: %v", err) } diff --git a/query/stdlib/influxdata/influxdb/from.go b/query/stdlib/influxdata/influxdb/from.go index cbe9e8092b..ac273ed88b 100644 --- a/query/stdlib/influxdata/influxdb/from.go +++ b/query/stdlib/influxdata/influxdb/from.go @@ -79,6 +79,28 @@ func (s *FromOpSpec) Kind() flux.OperationKind { return FromKind } +// BucketsAccessed makes FromOpSpec a query.BucketAwareOperationSpec +func (s *FromOpSpec) BucketsAccessed() (readBuckets, writeBuckets []platform.BucketFilter) { + bf := platform.BucketFilter{} + if s.Bucket != "" { + bf.Name = &s.Bucket + } + + if len(s.BucketID) > 0 { + if id, err := platform.IDFromString(s.BucketID); err != nil { + invalidID := platform.InvalidID() + bf.ID = &invalidID + } else { + bf.ID = id + } + } + + if bf.ID != nil || bf.Name != nil { + readBuckets = append(readBuckets, bf) + } + return readBuckets, writeBuckets +} + type FromProcedureSpec struct { Bucket string BucketID string @@ -623,9 +645,6 @@ func (FromKeysRule) Rewrite(keysNode plan.PlanNode) (plan.PlanNode, bool, error) return keysNode, true, nil } -// TODO(adam): implement a BucketsAccessed that doesn't depend on flux. -// https://github.com/influxdata/flux/issues/114 - func createFromSource(prSpec plan.ProcedureSpec, dsid execute.DatasetID, a execute.Administration) (execute.Source, error) { spec := prSpec.(*PhysicalFromProcedureSpec) var w execute.Window diff --git a/query/stdlib/influxdata/influxdb/from_test.go b/query/stdlib/influxdata/influxdb/from_test.go index 7be4f3f393..90e8e367b5 100644 --- a/query/stdlib/influxdata/influxdb/from_test.go +++ b/query/stdlib/influxdata/influxdb/from_test.go @@ -1,6 +1,7 @@ package influxdb_test import ( + "fmt" "testing" "time" @@ -122,24 +123,32 @@ func TestFromOperation_Marshaling(t *testing.T) { } func TestFromOpSpec_BucketsAccessed(t *testing.T) { - // TODO(adam) add this test back when BucketsAccessed is restored for the from function - // https://github.com/influxdata/flux/issues/114 - t.Skip("https://github.com/influxdata/flux/issues/114") bucketName := "my_bucket" - bucketID, _ := platform.IDFromString("deadbeef") + bucketIDString := "aaaabbbbccccdddd" + bucketID, err := platform.IDFromString(bucketIDString) + if err != nil { + t.Fatal(err) + } + invalidID := platform.InvalidID() tests := []pquerytest.BucketAwareQueryTestCase{ { Name: "From with bucket", - Raw: `from(bucket:"my_bucket")`, + Raw: fmt.Sprintf(`from(bucket:"%s")`, bucketName), WantReadBuckets: &[]platform.BucketFilter{{Name: &bucketName}}, WantWriteBuckets: &[]platform.BucketFilter{}, }, { Name: "From with bucketID", - Raw: `from(bucketID:"deadbeef")`, + Raw: fmt.Sprintf(`from(bucketID:"%s")`, bucketID), WantReadBuckets: &[]platform.BucketFilter{{ID: bucketID}}, WantWriteBuckets: &[]platform.BucketFilter{}, }, + { + Name: "From invalid bucketID", + Raw: `from(bucketID:"invalid")`, + WantReadBuckets: &[]platform.BucketFilter{{ID: &invalidID}}, + WantWriteBuckets: &[]platform.BucketFilter{}, + }, } for _, tc := range tests { tc := tc diff --git a/query/stdlib/influxdata/influxdb/to.go b/query/stdlib/influxdata/influxdb/to.go index ffdecf6766..d8f04f90c1 100644 --- a/query/stdlib/influxdata/influxdb/to.go +++ b/query/stdlib/influxdata/influxdb/to.go @@ -172,7 +172,13 @@ func (ToOpSpec) Kind() flux.OperationKind { // BucketsAccessed returns the buckets accessed by the spec. func (o *ToOpSpec) BucketsAccessed() (readBuckets, writeBuckets []platform.BucketFilter) { - bf := platform.BucketFilter{Name: &o.Bucket, Organization: &o.Org} + bf := platform.BucketFilter{} + if o.Bucket != "" { + bf.Name = &o.Bucket + } + if o.Org != "" { + bf.Organization = &o.Org + } if o.OrgID != "" { id, err := platform.IDFromString(o.OrgID) if err == nil { diff --git a/query/stdlib/influxdata/influxdb/to_test.go b/query/stdlib/influxdata/influxdb/to_test.go index 641634cdf9..db5718d971 100644 --- a/query/stdlib/influxdata/influxdb/to_test.go +++ b/query/stdlib/influxdata/influxdb/to_test.go @@ -2,6 +2,7 @@ package influxdb_test import ( "context" + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -82,24 +83,25 @@ func TestTo_Query(t *testing.T) { } func TestToOpSpec_BucketsAccessed(t *testing.T) { - // TODO(adam) add this test back when BucketsAccessed is restored for the from function - // https://github.com/influxdata/flux/issues/114 - t.Skip("https://github.com/influxdata/flux/issues/114") bucketName := "my_bucket" orgName := "my_org" - id := platform.ID(1) + orgIDString := "aaaabbbbccccdddd" + orgID, err := platform.IDFromString(orgIDString) + if err != nil { + t.Fatal(err) + } tests := []querytest.BucketAwareQueryTestCase{ { Name: "from() with bucket and to with org and bucket", - Raw: `from(bucket:"my_bucket") |> to(bucket:"my_bucket", org:"my_org")`, + Raw: fmt.Sprintf(`from(bucket:"%s") |> to(bucket:"%s", org:"%s")`, bucketName, bucketName, orgName), WantReadBuckets: &[]platform.BucketFilter{{Name: &bucketName}}, WantWriteBuckets: &[]platform.BucketFilter{{Name: &bucketName, Organization: &orgName}}, }, { Name: "from() with bucket and to with orgID and bucket", - Raw: `from(bucket:"my_bucket") |> to(bucket:"my_bucket", orgID:"0000000000000001")`, + Raw: fmt.Sprintf(`from(bucket:"%s") |> to(bucket:"%s", orgID:"%s")`, bucketName, bucketName, orgIDString), WantReadBuckets: &[]platform.BucketFilter{{Name: &bucketName}}, - WantWriteBuckets: &[]platform.BucketFilter{{Name: &bucketName, OrganizationID: &id}}, + WantWriteBuckets: &[]platform.BucketFilter{{Name: &bucketName, OrganizationID: orgID}}, }, }