wip: add back leading slash on azure driver

also support no leading slash, since some customers are running the
registry with the previous code that removed it.

registry/storage/driver/azure/azure.go

@@ -66,6 +66,7 @@ func New(params *Parameters) (*Driver, error) {
 		client:        client,
 		rootDirectory: params.RootDirectory,
 	}
+	driver := &Driver{baseEmbed: baseEmbed{Base: base.Base{StorageDriver: d}}}
 
 	// the previous code removed the leading "/" on paths.
 	// this caused backwards incompatible upgrades between v2 and v3
@@ -77,34 +78,47 @@ func New(params *Parameters) (*Driver, error) {
 	// paths start with a leading slach by default in v2 - to be backwards
 	// compatible we default to that and set it to false if we detect the
 	// existing storage does not use it.
-	d.useLeadingSlash = true
+
+	// installations that upgraded from v2 to v3 (previous to this patch)
+	// might have data in / in their storage account, but will actually be
+	// using data from a path without the leading /.
+	// to make sure there is no data loss, we first check for data in the
+	// path with no leading slash, and only when we do not find data there,
+	// we default to the leading slash
+	d.useLeadingSlash = false
 
 	// we use listBlobs instead of List because List is recursive,
 	// and we only need to look at the root.
-	blobs, err := d.listBlobs(context.TODO(), "/")
-	// // TODO: revisit error handling?
-	if err != nil {
+	blobRef := d.client.NewBlobClient("/")
+	_, err = blobRef.GetProperties(context.TODO(), nil)
+	// TODO: revisit error handling?
+	if err != nil && !is404(err) {
 		return nil, err
 	}
 
 	// a fresh registry install will contain no blobs in its root dir.
 	// a registry that used the previous v3 path handling however might
 	// also return 0 blobs under "/", but actually contain blobs under "".
-	if len(blobs) == 0 {
-		d.useLeadingSlash = false
-		blobs, err := d.listBlobs(context.TODO(), "/")
+	if is404(err) {
+		fmt.Println("GetProperties of empty blob returned 404")
+		d.useLeadingSlash = true
+		blobRef := d.client.NewBlobClient("//")
+		_, err := blobRef.GetProperties(context.TODO(), nil)
 		// TODO: revisit error handling?
-		if err != nil {
+		if err != nil && !is404(err) {
 			return nil, err
 		}
-		if len(blobs) == 0 {
-			// no blobs found with and without a leading slash, this
-			// means this is a new registry.
-			d.useLeadingSlash = true
-		}
+		// blobs, err := d.listBlobs(context.TODO(), "/")
+		// // TODO: revisit error handling?
+		// if err != nil {
+		// 	return nil, err
+		// }
+		// if len(blobs) == 0 {
+		// 	d.useLeadingSlash = false
+		// }
 	}
 
-	return &Driver{baseEmbed: baseEmbed{Base: base.Base{StorageDriver: d}}}, nil
+	return driver, nil
 }
 
 // Implement the storagedriver.StorageDriver interface.
@@ -200,6 +214,7 @@ func (d *driver) Reader(ctx context.Context, path string, offset int64) (io.Read
 // at the location designated by "path" after the call to Commit.
 func (d *driver) Writer(ctx context.Context, path string, append bool) (storagedriver.FileWriter, error) {
 	blobName := d.blobName(path)
+	fmt.Println("Writer blobName:", blobName)
 	blobRef := d.client.NewBlobClient(blobName)
 
 	props, err := blobRef.GetProperties(ctx, nil)
@@ -502,6 +517,9 @@ func (d *driver) blobName(path string) string {
 	trimmedRoot := strings.TrimRight(d.rootDirectory, "/")
 	trimmedPath := strings.TrimLeft(path, "/")
 	if d.useLeadingSlash {
+		if !strings.HasPrefix(trimmedRoot, "/") {
+			trimmedRoot = "/" + trimmedRoot
+		}
 		return trimmedRoot + "/" + trimmedPath
 	}
 

registry/storage/driver/azure/azure_test.go

@@ -91,44 +91,123 @@ func randStringRunes(n int) string {
 	return string(b)
 }
 
+func writeBlob(ctx context.Context, driver storagedriver.StorageDriver, contents, path string, append_ bool) error {
+	writer, err := driver.Writer(ctx, path, append_)
+	if err != nil {
+		return fmt.Errorf("unexpected error from driver.Writer: %v", err)
+	}
+	_, err = writer.Write([]byte(contents))
+	if err != nil {
+		return fmt.Errorf("writer.Write: unexpected error: %v", err)
+	}
+
+	err = writer.Commit()
+	if err != nil {
+		return fmt.Errorf("writer.Commit: unexpected error: %v", err)
+	}
+	err = writer.Close()
+	if err != nil {
+		return fmt.Errorf("writer.Close: unexpected error: %v", err)
+	}
+	return nil
+}
+
 func TestUseTrailingSlash(t *testing.T) {
+	ctx := context.Background()
+	fmt.Println("================================== empty storage ======================================")
 	d, err := azureDriverConstructor()
 	if err != nil {
 		t.Fatalf("unexpected error creating azure driver: %v", err)
 	}
 	storageDriver := d.(*Driver)
 	drvr := storageDriver.Base.StorageDriver.(*driver)
-	fmt.Println(drvr.useLeadingSlash)
-}
 
-func TestListBlobs(t *testing.T) {
-	driver, err := azureDriverConstructor()
-	if err != nil {
-		t.Fatalf("unexpected error creating azure driver: %v", err)
+	// at this point there should be no data in the storage with or without
+	// a leading slash. this simulates a fresh install of the registry, so we
+	// expect the default useLeadingSlash to be used (true).
+	if !drvr.useLeadingSlash {
+		t.Fatal("expected a fresh install of the registry to use a leading slash on storage paths by default")
 	}
+	fmt.Println("========================================================================")
 
+	fmt.Println("================================== /docker/<etc> ======================================")
+	// create files in storage with leading slash
+	// this should detect that files starting with a /
+	// exist, which will set useLeadingSlash to true
 	contents := randStringRunes(4 * 1024 * 1024)
-	path := "/file"
-	ctx := context.Background()
+	path1 := "/docker/registry/v2/repositories/test/hello/_layers/sha256/abc123/link"
 
-	defer driver.Delete(ctx, path)
+	defer d.Delete(ctx, path1)
+	if err := writeBlob(ctx, d, contents, path1, false); err != nil {
+		t.Fatal(err)
+	}
 
-	writer, err := driver.Writer(ctx, path, false)
+	// create another instance of the storage driver. this is the only way
+	// to reset the value of useLeadingSlash
+	d, err = azureDriverConstructor()
 	if err != nil {
-		t.Fatalf("unexpected error from driver.Writer: %v", err)
+		t.Fatalf("unexpected error creating azure driver: %v", err)
 	}
-	_, err = writer.Write([]byte(contents))
-	if err != nil {
-		t.Fatalf("writer.Write: unexpected error: %v", err)
+	storageDriver = d.(*Driver)
+	drvr = storageDriver.Base.StorageDriver.(*driver)
+
+	// when files exist in a path with a leading / the registry is using
+	// existing storage and it should follow the /, useLeadingSlash should
+	// be true.
+	if !drvr.useLeadingSlash {
+		t.Error("expected an existing install with data under `/docker` to use a leading slash on storage paths by default")
+		t.Logf("sample blob name: %s", drvr.blobName("/docker/registry/v2/foo"))
+	}
+	fmt.Println("========================================================================")
+
+	fmt.Println("========================================================================")
+	fmt.Println("================================== docker/<etc> ======================================")
+	// now we'll simulate a case where a user updated from a v2 to a v3 registry,
+	// then pushed data into the registry again. in this situation, users will have
+	// data both, under /docker and under docker (without a leading /).
+	// in this case we assume the user has recovered from the apparent data loss,
+	// and wants to keep using the docker path (without a leading /).
+	// this means useLeadingSlash should be false.
+	contents = randStringRunes(4 * 1024 * 1024)
+	path2 := "/docker/registry/v2/repositories/test/hello/_layers/sha256/abc123/link"
+
+	defer d.Delete(ctx, path2)
+	// we have to manually set useLeadingSlash to false so that the path ends up
+	// not containing the leading slash - the driver framework doesn't allow us to
+	// create blobs that do not start with /
+	drvr.useLeadingSlash = false
+	if err := writeBlob(ctx, d, contents, path2, false); err != nil {
+		t.Fatal(err)
 	}
 
-	err = writer.Commit()
+	// create another instance of the storage driver. this is the only way
+	// to reset the value of useLeadingSlash
+	d, err = azureDriverConstructor()
 	if err != nil {
-		t.Fatalf("writer.Commit: unexpected error: %v", err)
+		t.Fatalf("unexpected error creating azure driver: %v", err)
 	}
-	err = writer.Close()
+	storageDriver = d.(*Driver)
+	drvr = storageDriver.Base.StorageDriver.(*driver)
+
+	if drvr.useLeadingSlash {
+		t.Error("expected a existing install of the registry with data under `docker` (without a leading slash) and `/docker` (with a leading slash) to default to use `docker` (without leading slash) but it default to `/docker`")
+	}
+	fmt.Println("========================================================================")
+}
+
+func TestListBlobs(t *testing.T) {
+	driver, err := azureDriverConstructor()
 	if err != nil {
-		t.Fatalf("writer.Close: unexpected error: %v", err)
+		t.Fatalf("unexpected error creating azure driver: %v", err)
+	}
+
+	contents := randStringRunes(4 * 1024 * 1024)
+	path := "/file"
+	ctx := context.Background()
+
+	defer driver.Delete(ctx, path)
+	if err := writeBlob(ctx, driver, contents, path, false); err != nil {
+		t.Fatal(err)
 	}
 
 	blobNames, err := driver.List(ctx, "/")