storagedriver: Adding an interface conformance test for Walk

registry/storage/driver/conformance/conformance.go

@@ -0,0 +1,171 @@
+package conformance
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	storagedriver "github.com/docker/distribution/registry/storage/driver"
+	"strings"
+	"testing"
+)
+
+var fileset = map[string][]string{
+	"/":        {"/file1", "/folder1", "/folder2"},
+	"/folder1": {"/folder1/file1"},
+	"/folder2": {"/folder2/file1"},
+}
+
+type walkConformance struct {
+	driver  storagedriver.StorageDriver
+	created []string
+}
+
+func (wc *walkConformance) init() error {
+	for _, paths := range fileset {
+		for _, path := range paths {
+			if _, isDir := fileset[path]; isDir {
+				continue // skip directories
+			}
+			err := wc.driver.PutContent(context.Background(), path, []byte("content "+path))
+			if err != nil {
+				return errors.New(fmt.Sprintf("unable to create file %s: %s", path, err))
+			}
+			wc.created = append(wc.created, path)
+		}
+	}
+	return nil
+}
+
+func (wc *walkConformance) cleanup() error {
+	var lastError error
+	for _, path := range wc.created {
+		err := wc.driver.Delete(context.Background(), path)
+		if err != nil {
+			_ = fmt.Errorf("cleanup failed for path %s: %s", path, err)
+			lastError = err
+		}
+	}
+	return lastError
+}
+
+func (wc *walkConformance) isDir(path string) bool {
+	_, isDir := fileset[path]
+	return isDir
+}
+
+func Run(driver storagedriver.StorageDriver, t *testing.T) error {
+	wc := walkConformance{driver: driver}
+	defer func() {
+		err := wc.cleanup()
+		if err != nil {
+			t.Fatalf("cleanup failed: %s", err)
+		}
+	}()
+	err := wc.init()
+	if err != nil {
+		return err
+	}
+
+	noopFn := func(fileInfo storagedriver.FileInfo) error { return nil }
+
+	tcs := []struct {
+		name     string
+		fn       storagedriver.WalkFn
+		from     string
+		expected []string
+		err      bool
+	}{
+		{
+			name: "walk all",
+			fn:   noopFn,
+			expected: []string{
+				"/",
+				"/file1",
+				"/folder1",
+				"/folder1/file1",
+				"/folder2",
+				"/folder2/file1",
+			},
+		},
+		{
+			name: "skip directory",
+			fn: func(fileInfo storagedriver.FileInfo) error {
+				if fileInfo.Path() == "/folder1" {
+					return storagedriver.ErrSkipDir
+				}
+				if strings.Contains(fileInfo.Path(), "/folder1") {
+					t.Fatalf("skipped dir %s and should not walk %s", "/folder1", fileInfo.Path())
+				}
+				return nil
+			},
+			expected: []string{
+				"/",
+				"/file1",
+				"/folder1", // return ErrSkipDir, skip anything under /folder1
+				// skip /folder1/file1
+				"/folder2",
+				"/folder2/file1",
+			},
+		},
+		{
+			name: "stop early",
+			fn: func(fileInfo storagedriver.FileInfo) error {
+				if fileInfo.Path() == "/folder1/file1" {
+					return storagedriver.ErrSkipDir
+				}
+				return nil
+			},
+			expected: []string{
+				"/",
+				"/file1",
+				"/folder1",
+				"/folder1/file1",
+				// stop early
+			},
+		},
+		{
+			name: "from folder",
+			fn:   noopFn,
+			expected: []string{
+				"/folder1",
+				"/folder1/file1",
+			},
+			from: "/folder1",
+		},
+	}
+
+	for _, tc := range tcs {
+		var walked []string
+		if tc.from == "" {
+			tc.from = "/"
+		}
+		t.Run(tc.name, func(t *testing.T) {
+			err := wc.driver.Walk(context.Background(), tc.from, func(fileInfo storagedriver.FileInfo) error {
+				walked = append(walked, fileInfo.Path())
+				if fileInfo.IsDir() != wc.isDir(fileInfo.Path()) {
+					t.Fatalf("fileInfo isDir not matching file system: expected %t actual %t", wc.isDir(fileInfo.Path()), fileInfo.IsDir())
+				}
+				return tc.fn(fileInfo)
+			})
+			if tc.err && err == nil {
+				t.Fatalf("expected err")
+			}
+			if !tc.err && err != nil {
+				t.Fatalf(err.Error())
+			}
+			compareWalked(t, tc.expected, walked)
+		})
+	}
+	return nil
+}
+
+func compareWalked(t *testing.T, expected, walked []string) {
+	if len(walked) != len(expected) {
+		t.Fatalf("Mismatch number of fileInfo walked %d expected %d; walked %s; expected %s;", len(walked), len(expected), walked, expected)
+	}
+	for i := range walked {
+		if walked[i] != expected[i] {
+			t.Fatalf("expected walked to come in order expected: walked %s", walked)
+		}
+	}
+}

registry/storage/driver/s3-aws/s3_test.go

@@ -2,6 +2,7 @@ package s3
 
 import (
 	"bytes"
+	"github.com/docker/distribution/registry/storage/driver/conformance"
 	"io/ioutil"
 	"math/rand"
 	"os"
@@ -106,7 +107,7 @@ func init() {
 
 	// Skip S3 storage driver tests if environment variable parameters are not provided
 	skipS3 = func() string {
-		if accessKey == "" || secretKey == "" || region == "" || bucket == "" || encrypt == "" {
+		if accessKey == "" || secretKey == "" || bucket == "" || encrypt == "" {
 			return "Must set AWS_ACCESS_KEY, AWS_SECRET_KEY, AWS_REGION, S3_BUCKET, and S3_ENCRYPT to run S3 tests"
 		}
 		return ""
@@ -240,6 +241,28 @@ func TestStorageClass(t *testing.T) {
 
 }
 
+func TestConformance(t *testing.T) {
+	if skipS3() != "" {
+		t.Skip(skipS3())
+	}
+
+	rootDir, err := ioutil.TempDir("", "driver-")
+	if err != nil {
+		t.Fatalf("unexpected error creating temporary directory: %v", err)
+	}
+	defer os.Remove(rootDir)
+
+	standardDriver, err := s3DriverConstructor(rootDir, s3.StorageClassStandard)
+	if err != nil {
+		t.Fatalf("unexpected error creating driver with standard storage: %v", err)
+	}
+
+	err = conformance.Run(standardDriver, t)
+	if err != nil {
+		t.Fatalf("conformance failed: %v", err)
+	}
+}
+
 func TestOverThousandBlobs(t *testing.T) {
 	if skipS3() != "" {
 		t.Skip(skipS3())