blob: Implement HTTP replays for GCS (#205)

This modifies the GCS test to use HTTP replays.

To do this, a new recorder is added, some client side validation is stripped away (not necessary as you have to hit GCS anyway to do anything useful) and a JSON parser dependency added to redact things from the saved replays. As the JSON parser redaction technique works differently (and far better) to the previous regex method, I regenerated the AWS replays as well out of an abundance of caution.

Fixes #92

blob/gcsblob/gcsblob.go

@@ -18,10 +18,7 @@ package gcsblob
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"regexp"
-	"unicode/utf8"
 
 	"github.com/google/go-cloud/blob"
 	"github.com/google/go-cloud/blob/driver"
@@ -34,9 +31,6 @@ import (
 
 // OpenBucket returns a GCS Bucket that communicates using the given HTTP client.
 func OpenBucket(ctx context.Context, bucketName string, client *gcp.HTTPClient) (*blob.Bucket, error) {
-	if err := validateBucketChar(bucketName); err != nil {
-		return nil, err
-	}
 	if client == nil {
 		return nil, fmt.Errorf("NewBucket requires an HTTP client to communicate using")
 	}
@@ -89,9 +83,6 @@ func (b *bucket) NewRangeReader(ctx context.Context, key string, offset, length
 //
 // The caller must call Close on the returned Writer when done writing.
 func (b *bucket) NewTypedWriter(ctx context.Context, key string, contentType string, opts *driver.WriterOptions) (driver.Writer, error) {
-	if err := validateObjectChar(key); err != nil {
-		return nil, err
-	}
 	bkt := b.client.Bucket(b.name)
 	obj := bkt.Object(key)
 	w := obj.NewWriter(ctx)
@@ -116,33 +107,6 @@ func (b *bucket) Delete(ctx context.Context, key string) error {
 
 const namingRuleURL = "https://cloud.google.com/storage/docs/naming"
 
-// validateBucketChar checks whether character set and length meet the general requirement
-// of bucket naming rule. See https://cloud.google.com/storage/docs/naming for
-// the full requirements and best practice.
-func validateBucketChar(name string) error {
-	v := regexp.MustCompile(`^[a-z0-9][a-z0-9-_.]{1,220}[a-z0-9]$`)
-	if !v.MatchString(name) {
-		return fmt.Errorf("invalid bucket name, see %s for detailed requirements", namingRuleURL)
-	}
-	return nil
-}
-
-// validateObjectChar checks whether name is a valid UTF-8 encoded string, and its
-// length is between 1-1024 bytes. See https://cloud.google.com/storage/docs/naming
-// for the full requirements and best practice.
-func validateObjectChar(name string) error {
-	if name == "" {
-		return errors.New("object name is empty")
-	}
-	if !utf8.ValidString(name) {
-		return fmt.Errorf("object name is not valid UTF-8, see %s for detailed requirements", namingRuleURL)
-	}
-	if len(name) > 1024 {
-		return fmt.Errorf("object name is longer than 1024 bytes, see %s for detailed requirements", namingRuleURL)
-	}
-	return nil
-}
-
 func bufferSize(size int) int {
 	if size == 0 {
 		return googleapi.DefaultUploadChunkSize

blob/gcsblob/gcsblob_test.go

@@ -16,63 +16,167 @@ package gcsblob
 
 import (
 	"context"
+	"flag"
 	"fmt"
+	"io"
 	"net/http"
 	"strings"
 	"testing"
 
+	"cloud.google.com/go/storage"
+	"github.com/dnaeon/go-vcr/recorder"
 	"github.com/google/go-cloud/gcp"
-
+	"github.com/google/go-cloud/internal/testing/replay"
 	"google.golang.org/api/googleapi"
+	"google.golang.org/api/option"
 )
 
-func TestValidateBucketChar(t *testing.T) {
-	t.Parallel()
+const bucketPrefix = "go-cloud"
+
+var projectID = flag.String("project", "", "GCP project ID (string, not project number) to run tests against")
+
+func TestNewBucketNaming(t *testing.T) {
 	tests := []struct {
-		name  string
-		valid bool
+		name, bucketName string
+		wantErr          bool
 	}{
-		{"bucket-name", true},
-		{"8ucket_nam3", true},
-		{"bn", false},
-		{"_bucketname_", false},
-		{"bucketnameUpper", false},
-		{"bucketname?invalidchar", false},
+		{
+			name:       "A good bucket name should pass",
+			bucketName: "bucket-name",
+		},
+		{
+			name:       "A name with leading digits should pass",
+			bucketName: "8ucket_nam3",
+		},
+		{
+			name:       "A name with a leading underscore should fail",
+			bucketName: "_bucketname_",
+			wantErr:    true,
+		},
+		{
+			name:       "A name with an uppercase character should fail",
+			bucketName: "bucketnameUpper",
+			wantErr:    true,
+		},
+		{
+			name:       "A name with an invalid character should fail",
+			bucketName: "bucketname?invalidchar",
+			wantErr:    true,
+		},
+		{
+			name:       "A name that's too long should fail",
+			bucketName: strings.Repeat("a", 64),
+			wantErr:    true,
+		},
 	}
 
-	for i, test := range tests {
-		err := validateBucketChar(test.name)
-		if test.valid && err != nil {
-			t.Errorf("%d) got %v, want nil", i, err)
-		} else if !test.valid && err == nil {
-			t.Errorf("%d) got nil, want invalid error", i)
-		}
+	ctx := context.Background()
+	gcsC, done, err := newGCSClient(ctx, t.Logf, "test-naming")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer done()
+	c, err := storage.NewClient(ctx, option.WithHTTPClient(&gcsC.Client))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			b := c.Bucket(fmt.Sprintf("%s-%s", bucketPrefix, tc.bucketName))
+			err = b.Create(ctx, *projectID, nil)
+
+			switch {
+			case err != nil && !tc.wantErr:
+				t.Errorf("got %q; want nil", err)
+			case err == nil && tc.wantErr:
+				t.Errorf("got nil error; want error")
+			case !tc.wantErr:
+				_ = b.Delete(ctx)
+			}
+		})
 	}
 }
 
-func TestValidateObjectChar(t *testing.T) {
-	t.Parallel()
+func TestNewWriterObjectNaming(t *testing.T) {
 	tests := []struct {
-		name  string
-		valid bool
+		name, objName string
+		wantErr       bool
 	}{
-		{"object-name", true},
-		{"文件名", true},
-		{"ファイル名", true},
-		{"", false},
-		{"\xF4\x90\x80\x80", false},
-		{strings.Repeat("a", 1024), true},
-		{strings.Repeat("a", 1025), false},
-		{strings.Repeat("☺", 342), false},
+		{
+			name:    "An ASCII name should pass",
+			objName: "object-name",
+		},
+		{
+			name:    "A Unicode name should pass",
+			objName: "文件名",
+		},
+
+		{
+			name:    "An empty name should fail",
+			wantErr: true,
+		},
+		{
+			name:    "A name of escaped chars should fail",
+			objName: "\xF4\x90\x80\x80",
+			wantErr: true,
+		},
+		{
+			name:    "A name of 1024 chars should succeed",
+			objName: strings.Repeat("a", 1024),
+		},
+		{
+			name:    "A name of 1025 chars should fail",
+			objName: strings.Repeat("a", 1025),
+			wantErr: true,
+		},
+		{
+			name:    "A long name of Unicode chars should fail",
+			objName: strings.Repeat("☺", 342),
+			wantErr: true,
+		},
 	}
 
-	for i, test := range tests {
-		err := validateObjectChar(test.name)
-		if test.valid && err != nil {
-			t.Errorf("%d) got %v, want nil", i, err)
-		} else if !test.valid && err == nil {
-			t.Errorf("%d) got nil, want invalid error", i)
-		}
+	ctx := context.Background()
+	gcsC, done, err := newGCSClient(ctx, t.Logf, "test-obj-naming")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer done()
+	c, err := storage.NewClient(ctx, option.WithHTTPClient(&gcsC.Client))
+	if err != nil {
+		t.Fatal(err)
+	}
+	bkt := fmt.Sprintf("%s-%s", bucketPrefix, "test-obj-naming")
+	b := c.Bucket(bkt)
+	defer func() { _ = b.Delete(ctx) }()
+	_ = b.Create(ctx, *projectID, nil)
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			b, err := OpenBucket(ctx, bkt, gcsC)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			w, err := b.NewWriter(ctx, tc.objName, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			_, err = io.WriteString(w, "foo")
+			if err != nil {
+				t.Fatal(err)
+			}
+			err = w.Close()
+
+			switch {
+			case err != nil && !tc.wantErr:
+				t.Errorf("got %q; want nil", err)
+			case err == nil && tc.wantErr:
+				t.Errorf("got nil error; want error")
+			}
+		})
 	}
 }
 
@@ -131,3 +235,29 @@ func TestHTTPClientOpt(t *testing.T) {
 		t.Errorf("got %v; want %v", ts.called, "true")
 	}
 }
+
+func newGCSClient(ctx context.Context, logf func(string, ...interface{}), filepath string) (*gcp.HTTPClient, func(), error) {
+
+	mode := recorder.ModeRecording
+	if testing.Short() {
+		mode = recorder.ModeReplaying
+	}
+	r, done, err := replay.NewGCSRecorder(logf, mode, filepath)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	c := &gcp.HTTPClient{Client: http.Client{Transport: r}}
+	if mode == recorder.ModeRecording {
+		creds, err := gcp.DefaultCredentials(ctx)
+		if err != nil {
+			return nil, nil, err
+		}
+		c, err = gcp.NewHTTPClient(r, gcp.CredentialsTokenSource(creds))
+		if err != nil {
+			return nil, nil, err
+		}
+	}
+
+	return c, done, err
+}