BUGFIX: pushPrefix generation [ISSUE-137]

ivanilves · Apr 20, 2018 · 18f3fe2 · 18f3fe2
1 parent 39d63b4
commit 18f3fe2
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 17 deletions.
diff --git a/api/registry/registry.go b/api/registry/registry.go
@@ -127,7 +127,7 @@ func (c *Container) SeedWithImages(refs ...string) ([]string, error) {
 
 			tag := repo.Tags()[0]
 
-			pushRef := fmt.Sprintf("%s%s/%s:%s", c.hostname, repo.PushPrefix(), repo.Path(), tag)
+			pushRef := fmt.Sprintf("%s%s%s:%s", c.hostname, repo.PushPrefix(), repo.Path(), tag)
 
 			pushRepo, _ := repository.ParseRef(pushRef)
 

diff --git a/api/v1/v1.go b/api/v1/v1.go
@@ -120,6 +120,22 @@ func (api *API) CollectTags(refs ...string) (*collection.Collection, error) {
 	return collection.New(refs, tags)
 }
 
+func getPushPrefix(prefix, defaultPrefix string) string {
+	if prefix == "" {
+		return defaultPrefix
+	}
+
+	if prefix[0:1] != "/" {
+		prefix = "/" + prefix
+	}
+
+	if prefix[len(prefix)-1:] != "/" {
+		prefix = prefix + "/"
+	}
+
+	return prefix
+}
+
 // CollectPushTags blends passed collection with information fetched from [local] "push" registry,
 // makes required comparisons between them and spits organized info back as collection.Collection
 func (api *API) CollectPushTags(cn *collection.Collection, push PushConfig) (*collection.Collection, error) {
@@ -137,16 +153,11 @@ func (api *API) CollectPushTags(cn *collection.Collection, push PushConfig) (*co
 		go func(repo *repository.Repository, i int, done chan error) {
 			refs[i] = repo.Ref()
 
-			pushPrefix := push.Prefix
-			if pushPrefix == "" {
-				pushPrefix = repo.PushPrefix()
-			}
-
-			var pushRepoPath string
-			pushRepoPath = pushPrefix + "/" + repo.Path()
-			pushRepoPath = pushRepoPath[1:] // Leading "/" in prefix should be removed!
-
-			pushRef := fmt.Sprintf("%s/%s~/.*/", push.Registry, pushRepoPath)
+			pushRef := fmt.Sprintf(
+				"%s%s~/.*/",
+				push.Registry,
+				getPushPrefix(push.Prefix, repo.PushPrefix())+repo.Path(),
+			)
 
 			log.Debugf("%s 'push' reference: %+v", fn(repo.Ref()), pushRef)
 
@@ -273,7 +284,7 @@ func (api *API) PushTags(cn *collection.Collection, push PushConfig) error {
 		go func(repo *repository.Repository, tags []*tag.Tag, done chan error) {
 			for _, tg := range tags {
 				srcRef := repo.Name() + ":" + tg.Name()
-				dstRef := push.Registry + push.Prefix + "/" + repo.Path() + ":" + tg.Name()
+				dstRef := push.Registry + getPushPrefix(push.Prefix, repo.PushPrefix()) + repo.Path() + ":" + tg.Name()
 
 				log.Infof("[PULL/PUSH] PUSHING %s => %s", srcRef, dstRef)
 

diff --git a/api/v1/v1_test.go b/api/v1/v1_test.go
@@ -158,3 +158,26 @@ func TestNew_InvalidDockerJSONConfigFile(t *testing.T) {
 
 	assert.NotNil(err)
 }
+
+func TestGetPushPrefix(t *testing.T) {
+	var testCases = map[string]struct {
+		prefix        string
+		defaultPrefix string
+	}{
+		"/quay/io/":         {"", "/quay/io/"},
+		"/":                 {"/", "whatever"},
+		"/maco/":            {"/maco/", ""},
+		"/suau/":            {"suau", ""},
+		"/avegades/perdut/": {"/avegades/perdut", ""},
+		"/mai/fotut/":       {"mai/fotut/", ""},
+		"/entremaliat/":     {"entremaliat", "whatever"},
+	}
+
+	var assert = assert.New(t)
+
+	for expected, input := range testCases {
+		actual := getPushPrefix(input.prefix, input.defaultPrefix)
+
+		assert.Equal(expected, actual)
+	}
+}
diff --git a/repository/repository.go b/repository/repository.go
@@ -165,7 +165,7 @@ func (r *Repository) PushPrefix() string {
 	allParts := strings.Split(r.Registry(), ":")
 	hostPart := allParts[0]
 
-	return "/" + strings.Replace(hostPart, ".", "/", -1)
+	return "/" + strings.Replace(hostPart, ".", "/", -1) + "/"
 }
 
 func validateRef(ref string) (string, error) {

diff --git a/repository/repository_test.go b/repository/repository_test.go
@@ -185,10 +185,10 @@ func TestRepositoryMatchTag(t *testing.T) {
 
 func TestRepositoryPushPrefix(t *testing.T) {
 	testCases := map[string]string{
-		"alpine":                                  "/registry/hub/docker/com",
-		"localhost:5000/nginx":                    "/localhost",
-		"registry.company.com/secutiry/pentest":   "/registry/company/com",
-		"dockerz.hipster.io:8443/hype/kubernetes": "/dockerz/hipster/io",
+		"alpine":                                  "/registry/hub/docker/com/",
+		"localhost:5000/nginx":                    "/localhost/",
+		"registry.company.com/secutiry/pentest":   "/registry/company/com/",
+		"dockerz.hipster.io:8443/hype/kubernetes": "/dockerz/hipster/io/",
 	}
 
 	assert := assert.New(t)