Merge pull request #11894 from influxdata/fix/buckets-accessed

fix(query/stdlib/influxdb): make FromOpSpec a BucketAwareSpec

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()
-
+	// fresh pre-authorizer
-	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
 	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",
+		Name:           "my_bucket",
-		ID:   *id,
+		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(*bucketID, platform.ReadAction, platform.BucketsResourceType, orgID)
-	p, err := platform.NewPermissionAtID(*id, platform.ReadAction, platform.BucketsResourceType, orgID)
 	if err != nil {
 		t.Fatalf("Error creating read bucket permission query: %v", err)
 	}

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

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

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 {

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}},
 		},
 	}