Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(storage): init retry conformance test scaffolding #4711

Merged
merged 36 commits into from Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
a40676c
test(storage): add retry conformance tests [WIP]
tritone Dec 4, 2020
8c45efb
Merge branch 'master' into conf-test
tritone Mar 8, 2021
f606eeb
[WIP] test(storage): add retry conformance tests
tritone Mar 8, 2021
aa61155
separate idempotent/non-idempotent calls
tritone Mar 11, 2021
400a64b
rework schema
tritone Mar 25, 2021
ff9be59
get per-func errors/naming
tritone Mar 25, 2021
0abbad4
use instruction lists
tritone Mar 29, 2021
87f998e
update schema with fixtures
tritone Apr 2, 2021
57cc43e
update schema with comments
tritone Apr 2, 2021
c6385cd
update test cases
tritone Apr 2, 2021
14c25b6
fix id
tritone Apr 2, 2021
5ddf761
change to new emu format [WIP]
tritone Apr 16, 2021
d792232
Merge branch 'master' into conf-test
tritone Apr 30, 2021
4b1b2b9
use new retry_test resource
tritone May 3, 2021
36ae41c
gofmt
tritone May 3, 2021
d8fbc75
add test check and fix endpoints
tritone May 4, 2021
ec4ea20
change fixture to resource
tritone May 4, 2021
d4fd9c1
Merge branch 'master' into conf-scaffolding
BrennaEpp Aug 31, 2021
e0b4753
Merge branch 'master' into conf-scaffolding
BrennaEpp Aug 31, 2021
bd237cc
fix to changes
BrennaEpp Aug 31, 2021
31ebabd
fix
BrennaEpp Aug 31, 2021
74874e5
Merge branch 'master' into conf-scaffolding
BrennaEpp Sep 29, 2021
1974fa8
add some comments
BrennaEpp Sep 29, 2021
d6c80fb
call error from function
BrennaEpp Sep 29, 2021
4e4c1f8
add comments and remove extra url parsing
BrennaEpp Sep 29, 2021
0c98ce0
Merge branch 'master' into conf-scaffolding
BrennaEpp Sep 29, 2021
c157162
retry json align
BrennaEpp Oct 8, 2021
445f24c
Merge branch 'master' into conf-scaffolding
BrennaEpp Oct 8, 2021
31744de
goimports fix
BrennaEpp Oct 8, 2021
8d1ecfb
remove extra func
BrennaEpp Oct 8, 2021
018c255
Merge branch 'master' into conf-scaffolding
BrennaEpp Oct 9, 2021
afda9b6
Merge branch 'master' into conf-scaffolding
BrennaEpp Oct 12, 2021
8ce513a
Merge branch 'master' into conf-scaffolding
BrennaEpp Oct 18, 2021
ed848c2
separate retry tests
BrennaEpp Oct 18, 2021
907c050
Merge branch 'master' into conf-scaffolding
BrennaEpp Oct 18, 2021
acdd446
fix typos
BrennaEpp Oct 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
296 changes: 296 additions & 0 deletions storage/conformance_test.go
Expand Up @@ -16,21 +16,317 @@ package storage

import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"net/http/httputil"
"net/url"
"os"
"strconv"
"strings"
"testing"
"time"

"cloud.google.com/go/internal/uid"
storage_v1_tests "cloud.google.com/go/storage/internal/test/conformance"
"github.com/golang/protobuf/jsonpb"
"github.com/google/go-cmp/cmp"
"google.golang.org/api/option"
raw "google.golang.org/api/storage/v1"
htransport "google.golang.org/api/transport/http"
)

var (
BrennaEpp marked this conversation as resolved.
Show resolved Hide resolved
// Resource vars for retry tests
bucketIDs = uid.NewSpace("bucket", nil)
objectIDs = uid.NewSpace("object", nil)
notificationIDs = uid.NewSpace("notification", nil)
projectID = "my-project-id"
serviceAccountEmail = "my-sevice-account@my-project-id.iam.gserviceaccount.com"
randomBytesToWrite = []byte("abcdef")
)

// Holds the resources for a particular test case. Only the necessary fields will
// be populated; others will be nil.
type resources struct {
bucket *BucketAttrs
object *ObjectAttrs
notification *Notification
hmacKey *HMACKey
}

func (fs *resources) populate(ctx context.Context, c *Client, resource storage_v1_tests.Resource) error {
switch resource {
case storage_v1_tests.Resource_BUCKET:
bkt := c.Bucket(bucketIDs.New())
if err := bkt.Create(ctx, projectID, &BucketAttrs{}); err != nil {
return fmt.Errorf("creating bucket: %v", err)
}
attrs, err := bkt.Attrs(ctx)
if err != nil {
return fmt.Errorf("getting bucket attrs: %v", err)
}
fs.bucket = attrs
case storage_v1_tests.Resource_OBJECT:
// Assumes bucket has been populated first.
obj := c.Bucket(fs.bucket.Name).Object(objectIDs.New())
w := obj.NewWriter(ctx)
if _, err := w.Write(randomBytesToWrite); err != nil {
return fmt.Errorf("writing object: %v", err)
}
if err := w.Close(); err != nil {
return fmt.Errorf("closing object: %v", err)
}
attrs, err := obj.Attrs(ctx)
if err != nil {
return fmt.Errorf("getting object attrs: %v", err)
}
fs.object = attrs
case storage_v1_tests.Resource_NOTIFICATION:
// Assumes bucket has been populated first.
n, err := c.Bucket(fs.bucket.Name).AddNotification(ctx, &Notification{
TopicProjectID: projectID,
TopicID: notificationIDs.New(),
PayloadFormat: JSONPayload,
})
if err != nil {
return fmt.Errorf("adding notification: %v", err)
}
fs.notification = n
case storage_v1_tests.Resource_HMAC_KEY:
key, err := c.CreateHMACKey(ctx, projectID, serviceAccountEmail)
if err != nil {
return fmt.Errorf("creating HMAC key: %v", err)
}
fs.hmacKey = key
}
return nil
}

type retryFunc func(ctx context.Context, c *Client, fs *resources, preconditions bool) error

// Methods to retry. This is a map whose keys are a string describing a standard
// API call (e.g. storage.objects.get) and values are a list of functions which
// wrap library methods that implement these calls. There may be multiple values
// because multiple library methods may use the same call (e.g. get could be a
// read or just a metadata get).
var methods = map[string][]retryFunc{
"storage.buckets.get": {
func(ctx context.Context, c *Client, fs *resources, _ bool) error {
_, err := c.Bucket(fs.bucket.Name).Attrs(ctx)
return err
},
},
}

func TestRetryConformance(t *testing.T) {
host := os.Getenv("STORAGE_EMULATOR_HOST")
if host == "" {
// This test is currently skipped in CI as the env variable is not set
// TODO: Add test to CI
t.Skip("This test must use the testbench emulator; set STORAGE_EMULATOR_HOST to run.")
BrennaEpp marked this conversation as resolved.
Show resolved Hide resolved
}
endpoint, err := url.Parse(host)
if err != nil {
t.Fatalf("error parsing emulator host (make sure it includes the scheme such as http://host): %v", err)
}

ctx := context.Background()

// Create non-wrapped client to use for setup steps.
client, err := NewClient(ctx)
if err != nil {
t.Fatalf("storage.NewClient: %v", err)
}

_, _, testFiles := parseFiles(t)

for _, testFile := range testFiles {
for _, retryTest := range testFile.RetryTests {
for _, instructions := range retryTest.Cases {
for _, method := range retryTest.Methods {
if len(methods[method.Name]) == 0 {
t.Logf("No tests for operation %v", method.Name)
}
for i, fn := range methods[method.Name] {
testName := fmt.Sprintf("%v-%v-%v-%v", retryTest.Id, instructions.Instructions, method.Name, i)
t.Run(testName, func(t *testing.T) {

// Create the retry subtest
subtest := &retrySubtest{T: t, name: testName, host: endpoint}
subtest.create(map[string][]string{
method.Name: instructions.Instructions,
})

// Create necessary test resources in the emulator
fs := &resources{}
for _, resource := range method.Resources {
if err := fs.populate(ctx, client, resource); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should populating the resources be a method on the subtest as well? Not required but it feels slightly cleaner I guess.

t.Fatalf("creating test resources: %v", err)
}
}

// Test
err = fn(ctx, subtest.wrappedClient, fs, retryTest.PreconditionProvided)
if retryTest.ExpectSuccess && err != nil {
t.Errorf("want success, got %v", err)
}
if !retryTest.ExpectSuccess && err == nil {
t.Errorf("want failure, got success")
}

// Verify that all instructions were used up during the test
// (indicates that the client sent the correct requests).
subtest.check()

// Close out test in emulator.
subtest.delete()
})
}

}
}
}
}

}

type retrySubtest struct {
*testing.T
name string
id string // ID to pass as a header in the test execution
host *url.URL // set the path when using; path is not guaranteed betwen calls
wrappedClient *Client
}

// create creates a retry test resource in the emulator
func (rt *retrySubtest) create(instructions map[string][]string) {
c := http.DefaultClient
data := struct {
Instructions map[string][]string `json:"instructions"`
}{
Instructions: instructions,
}

buf := new(bytes.Buffer)
if err := json.NewEncoder(buf).Encode(data); err != nil {
rt.Fatalf("encoding request: %v", err)
}

rt.host.Path = "retry_test"
resp, err := c.Post(rt.host.String(), "application/json", buf)
if err != nil || resp.StatusCode != 200 {
rt.Fatalf("creating retry test: err: %v, resp: %+v", err, resp)
}
defer func() {
closeErr := resp.Body.Close()
if err == nil {
err = closeErr
}
}()
testRes := struct {
TestID string `json:"id"`
}{}
if err := json.NewDecoder(resp.Body).Decode(&testRes); err != nil {
rt.Fatalf("decoding test ID: %v", err)
}

rt.id = testRes.TestID

// Create wrapped client which will send emulator instructions
rt.host.Path = ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Curious is the main purpose of creating a wrappedClient in here to populate the retrySubtest struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrappedClient adds the retry test header to each request, prints the request if the test fails and also adds the DevstorageFullControlScope to the https client transport. Correct, we do create it here to populate the struct

client, err := wrappedClient(rt.T, rt.host.String(), rt.id)
if err != nil {
rt.Fatalf("creating wrapped client: %v", err)
}
rt.wrappedClient = client
}

// check verifies that all instructions for a given retry testID have been used up
func (rt *retrySubtest) check() {
rt.host.Path = strings.Join([]string{"retry_test", rt.id}, "/")
c := http.DefaultClient
resp, err := c.Get(rt.host.String())
if err != nil || resp.StatusCode != 200 {
rt.Errorf("getting retry test: err: %v, resp: %+v", err, resp)
}
defer func() {
closeErr := resp.Body.Close()
if err == nil {
err = closeErr
}
}()
testRes := struct {
Instructions map[string][]string
Completed bool
}{}
if err := json.NewDecoder(resp.Body).Decode(&testRes); err != nil {
rt.Errorf("decoding response: %v", err)
}
if !testRes.Completed {
rt.Errorf("test not completed; unused instructions: %+v", testRes.Instructions)
}
}

// Delete a retry test resource.
func (rt *retrySubtest) delete() {
rt.host.Path = strings.Join([]string{"retry_test", rt.id}, "/")
c := http.DefaultClient
req, err := http.NewRequest("DELETE", rt.host.String(), nil)
if err != nil {
rt.Errorf("creating request: %v", err)
}
resp, err := c.Do(req)
if err != nil || resp.StatusCode != 200 {
rt.Errorf("deleting test: err: %v, resp: %+v", err, resp)
}
}

// retryTestRoundTripper sends the retry test ID to the emulator with each request
type retryTestRoundTripper struct {
*testing.T
rt http.RoundTripper
testID string
}

func (wt *retryTestRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) {
r.Header.Set("x-retry-test-id", wt.testID)

requestDump, err := httputil.DumpRequest(r, false)
if err != nil {
wt.Logf("error creating request dump: %v", err)
}

resp, err := wt.rt.RoundTrip(r)
if err != nil {
wt.Logf("roundtrip error (may be expected): %v\nrequest: %s", err, requestDump)
}
return resp, err
}

// Create custom client that sends instructions to the storage testbench Retry Test API
func wrappedClient(t *testing.T, host, testID string) (*Client, error) {
ctx := context.Background()
base := http.DefaultTransport
trans, err := htransport.NewTransport(ctx, base, option.WithScopes(raw.DevstorageFullControlScope),
option.WithUserAgent("custom-user-agent"))
if err != nil {
return nil, fmt.Errorf("failed to create http client: %v", err)
}
c := http.Client{Transport: trans}

// Add RoundTripper to the created HTTP client
wrappedTrans := &retryTestRoundTripper{rt: c.Transport, testID: testID, T: t}
c.Transport = wrappedTrans

// Supply this client to storage.NewClient
client, err := NewClient(ctx, option.WithHTTPClient(&c), option.WithEndpoint(host+"/storage/v1/"))
return client, err
}

func TestPostPolicyV4Conformance(t *testing.T) {
oldUTCNow := utcNow
defer func() {
Expand Down
45 changes: 45 additions & 0 deletions storage/internal/test/conformance/retry_tests.json
@@ -0,0 +1,45 @@
{
"retryTests": [
{
"id": 1,
"description": "always_idempotent",
"cases": [
{
"instructions": ["return-503", "return-503"]
},
{
"instructions": ["return-reset-connection", "return-reset-connection"]
},
{
"instructions": ["return-reset-connection", "return-503"]
}
],
"methods": [
{"name": "storage.bucket_acl.get", "resources": ["BUCKET"]},
{"name": "storage.bucket_acl.list", "resources": ["BUCKET"]},
{"name": "storage.buckets.delete", "resources": ["BUCKET"]},
{"name": "storage.buckets.get", "resources": ["BUCKET"]},
{"name": "storage.buckets.getIamPolicy", "resources": ["BUCKET"]},
{"name": "storage.buckets.insert", "resources": []},
{"name": "storage.buckets.list", "resources": ["BUCKET"]},
{"name": "storage.buckets.lockRetentionPolicy", "resources": ["BUCKET"]},
{"name": "storage.buckets.testIamPermissions", "resources": ["BUCKET"]},
{"name": "storage.default_object_acl.get", "resources": ["BUCKET"]},
{"name": "storage.default_object_acl.list", "resources": ["BUCKET"]},
{"name": "storage.hmacKey.delete", "resources": ["HMAC_KEY"]},
{"name": "storage.hmacKey.get", "resources": ["HMAC_KEY"]},
{"name": "storage.hmacKey.list", "resources": ["HMAC_KEY"]},
{"name": "storage.notifications.delete", "resources": ["BUCKET", "NOTIFICATION"]},
{"name": "storage.notifications.get", "resources": ["BUCKET", "NOTIFICATION"]},
{"name": "storage.notifications.list", "resources": ["BUCKET", "NOTIFICATION"]},
{"name": "storage.object_acl.get", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.object_acl.list", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.objects.get", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.objects.list", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.serviceaccount.get", "resources": []}
],
"preconditionProvided": false,
"expectSuccess": true
}
]
}