From 3c32d46e2379651a13e0669710bbd47e89d8afb2 Mon Sep 17 00:00:00 2001
From: David Beck <david.beck@invitae.com>
Date: Wed, 15 May 2024 14:00:01 -0700
Subject: [PATCH] feat(sync): preserve subpath in repositories when registry is
 defined

  - Add `RegistryName` field to `repoDescriptor` for better registry handling.
  - Introduce `getSubPath` function to cleanly handle subpath extraction.
  - Update functions `imagesToCopyFromRegistry` and `filterSourceReferences` to include `RegistryName`.
  - Modify `syncOptions.run` to use `getSubPath`.
  - Add unit tests for `getSubPath`.

Resolves
  - https://github.com/containers/skopeo/issues/1992
---
 cmd/skopeo/sync.go      | 29 +++++++++++++++++++++--------
 cmd/skopeo/sync_test.go | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/cmd/skopeo/sync.go b/cmd/skopeo/sync.go
index a5639f5bc9..c66c9217d5 100644
--- a/cmd/skopeo/sync.go
+++ b/cmd/skopeo/sync.go
@@ -56,9 +56,10 @@ type syncOptions struct {
 
 // repoDescriptor contains information of a single repository used as a sync source.
 type repoDescriptor struct {
-	DirBasePath string                 // base path when source is 'dir'
-	ImageRefs   []types.ImageReference // List of tagged image found for the repository
-	Context     *types.SystemContext   // SystemContext for the sync command
+	DirBasePath  string                 // base path when source is 'dir'
+	RegistryName string                 // Name of the registry
+	ImageRefs    []types.ImageReference // List of tagged image found for the repository
+	Context      *types.SystemContext   // SystemContext for the sync command
 }
 
 // tlsVerifyConfig is an implementation of the Unmarshaler interface, used to
@@ -374,8 +375,9 @@ func imagesToCopyFromRegistry(registryName string, cfg registrySyncConfig, sourc
 			continue
 		}
 		repoDescList = append(repoDescList, repoDescriptor{
-			ImageRefs: sourceReferences,
-			Context:   serverCtx})
+			RegistryName: registryName,
+			ImageRefs:    sourceReferences,
+			Context:      serverCtx})
 	}
 
 	// include repository descriptors for cfg.ImagesByTagRegex
@@ -453,8 +455,9 @@ func filterSourceReferences(sys *types.SystemContext, registryName string, colle
 		}
 
 		repoDescList = append(repoDescList, repoDescriptor{
-			ImageRefs: filteredSourceReferences,
-			Context:   sys,
+			RegistryName: registryName,
+			ImageRefs:    filteredSourceReferences,
+			Context:      sys,
 		})
 	}
 	return repoDescList
@@ -593,6 +596,16 @@ func imagesToCopy(source string, transport string, sourceCtx *types.SystemContex
 	return descriptors, nil
 }
 
+// getSubPath takes a full and base path string. Normally this looks like
+// fullPath = "registry.fedoraproject.org/f38/flatpak-runtime:f38"
+// basePath = "registry.fedoraproject.org"
+// It returns a string with the basePath removed for example "f38/flatpak-runtime:f38".
+func getSubPath(fullPath, basePath string) string {
+	cleanBasePath := filepath.Clean(basePath)
+	cleanFullPath := filepath.Clean(fullPath)
+	return strings.TrimPrefix(cleanFullPath, cleanBasePath+"/")
+}
+
 func (opts *syncOptions) run(args []string, stdout io.Writer) (retErr error) {
 	if len(args) != 2 {
 		return errorShouldDisplayUsage{errors.New("Exactly two arguments expected")}
@@ -741,7 +754,7 @@ func (opts *syncOptions) run(args []string, stdout io.Writer) (retErr error) {
 			}
 
 			if !opts.scoped {
-				destSuffix = path.Base(destSuffix)
+				destSuffix = getSubPath(destSuffix, srcRepo.RegistryName)
 			}
 
 			destRef, err := destinationReference(path.Join(destination, destSuffix)+opts.appendSuffix, opts.destination)
diff --git a/cmd/skopeo/sync_test.go b/cmd/skopeo/sync_test.go
index 734abfedd1..d263928698 100644
--- a/cmd/skopeo/sync_test.go
+++ b/cmd/skopeo/sync_test.go
@@ -44,3 +44,44 @@ func TestTLSVerifyConfig(t *testing.T) {
 	err := yaml.Unmarshal([]byte(`tls-verify: "not a valid bool"`), &config)
 	assert.Error(t, err)
 }
+
+func TestGetSubPath(t *testing.T) {
+	tests := []struct {
+		name     string
+		registry string
+		fullPath string
+		expected string
+	}{
+		{
+			name:     "Registry",
+			registry: "registry.fedoraproject.org",
+			fullPath: "registry.fedoraproject.org/f38/flatpak-runtime:f38",
+			expected: "f38/flatpak-runtime:f38",
+		},
+		{
+			name:     "Registry with Subpath",
+			registry: "registry.fedoraproject.org/f38",
+			fullPath: "registry.fedoraproject.org/f38/flatpak-runtime:f38",
+			expected: "flatpak-runtime:f38",
+		},
+		{
+			name:     "Dir without registry",
+			registry: "",
+			fullPath: "/media/usb/",
+			expected: "/media/usb",
+		},
+		{
+			name:     "Repo without registy",
+			registry: "",
+			fullPath: "flatpak-runtime:f38",
+			expected: "flatpak-runtime:f38",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := getSubPath(tt.fullPath, tt.registry)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}