pkg/registry: improve logging, tests & (test) readability

pkg/registry/auth/auth.go

@@ -4,7 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/url"
 	"strings"
@@ -26,7 +26,7 @@ func GetToken(container types.Container, registryAuth string) (string, error) {
 	}
 
 	URL := GetChallengeURL(normalizedRef)
-	logrus.WithField("URL", URL.String()).Debug("Building challenge URL")
+	logrus.WithField("URL", URL.String()).Debug("Built challenge URL")
 
 	var req *http.Request
 	if req, err = GetChallengeRequest(URL); err != nil {
@@ -99,7 +99,7 @@ func GetBearerHeader(challenge string, imageRef ref.Named, registryAuth string)
 		return "", err
 	}
 
-	body, _ := ioutil.ReadAll(authResponse.Body)
+	body, _ := io.ReadAll(authResponse.Body)
 	tokenResponse := &types.TokenResponse{}
 
 	err = json.Unmarshal(body, tokenResponse)

pkg/registry/auth/auth_test.go

@@ -2,14 +2,14 @@ package auth_test
 
 import (
 	"fmt"
-	"github.com/containrrr/watchtower/internal/actions/mocks"
-	"github.com/containrrr/watchtower/pkg/registry/auth"
 	"net/url"
 	"os"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/containrrr/watchtower/internal/actions/mocks"
+	"github.com/containrrr/watchtower/pkg/registry/auth"
 	wtTypes "github.com/containrrr/watchtower/pkg/types"
 	ref "github.com/docker/distribution/reference"
 	. "github.com/onsi/ginkgo"
@@ -53,7 +53,7 @@ var _ = Describe("the auth module", func() {
 		mockCreated,
 		mockDigest)
 
-	When("getting an auth url", func() {
+	Describe("GetToken", func() {
 		It("should parse the token from the response",
 			SkipIfCredentialsEmpty(GHCRCredentials, func() {
 				creds := fmt.Sprintf("%s:%s", GHCRCredentials.Username, GHCRCredentials.Password)
@@ -62,37 +62,39 @@ var _ = Describe("the auth module", func() {
 				Expect(token).NotTo(Equal(""))
 			}),
 		)
+	})
 
+	Describe("GetAuthURL", func() {
 		It("should create a valid auth url object based on the challenge header supplied", func() {
-			input := `bearer realm="https://ghcr.io/token",service="ghcr.io",scope="repository:user/image:pull"`
+			challenge := `bearer realm="https://ghcr.io/token",service="ghcr.io",scope="repository:user/image:pull"`
+			imageRef, _ := ref.ParseNormalizedNamed("containrrr/watchtower")
 			expected := &url.URL{
 				Host:     "ghcr.io",
 				Scheme:   "https",
 				Path:     "/token",
 				RawQuery: "scope=repository%3Acontainrrr%2Fwatchtower%3Apull&service=ghcr.io",
 			}
-			imageRef, _ := ref.ParseNormalizedNamed("containrrr/watchtower")
+
-			res, err := auth.GetAuthURL(input, imageRef)
+			URL, err := auth.GetAuthURL(challenge, imageRef)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(res).To(Equal(expected))
+			Expect(URL).To(Equal(expected))
 		})
-		It("should create a valid auth url object based on the challenge header supplied", func() {
+
-			input := `bearer realm="https://ghcr.io/token"`
+		When("given an invalid challenge header", func() {
-			imageRef, _ := ref.ParseNormalizedNamed("containrrr/watchtower")
+			It("should return an error", func() {
-			res, err := auth.GetAuthURL(input, imageRef)
+				challenge := `bearer realm="https://ghcr.io/token"`
-			Expect(err).To(HaveOccurred())
+				imageRef, _ := ref.ParseNormalizedNamed("containrrr/watchtower")
-			Expect(res).To(BeNil())
+				URL, err := auth.GetAuthURL(challenge, imageRef)
+				Expect(err).To(HaveOccurred())
+				Expect(URL).To(BeNil())
+			})
 		})
 
 		When("deriving the auth scope from an image name", func() {
 			It("should prepend official dockerhub images with \"library/\"", func() {
-				Expect(getScopeFromImageAuthURL("docker.io/registry")).To(Equal("library/registry"))
+				Expect(getScopeFromImageAuthURL("registry")).To(Equal("library/registry"))
 				Expect(getScopeFromImageAuthURL("docker.io/registry")).To(Equal("library/registry"))
 				Expect(getScopeFromImageAuthURL("index.docker.io/registry")).To(Equal("library/registry"))
-				Expect(getScopeFromImageAuthURL("index.docker.io/registry")).To(Equal("library/registry"))
-
-				Expect(getScopeFromImageAuthURL("registry")).To(Equal("library/registry"))
-				Expect(getScopeFromImageAuthURL("registry")).To(Equal("library/registry"))
 			})
 			It("should not include vanity hosts\"", func() {
 				Expect(getScopeFromImageAuthURL("docker.io/containrrr/watchtower")).To(Equal("containrrr/watchtower"))
@@ -100,26 +102,27 @@ var _ = Describe("the auth module", func() {
 			})
 			It("should not destroy three segment image names\"", func() {
 				Expect(getScopeFromImageAuthURL("piksel/containrrr/watchtower")).To(Equal("piksel/containrrr/watchtower"))
-				Expect(getScopeFromImageAuthURL("piksel/containrrr/watchtower")).To(Equal("piksel/containrrr/watchtower"))
+				Expect(getScopeFromImageAuthURL("ghcr.io/piksel/containrrr/watchtower")).To(Equal("piksel/containrrr/watchtower"))
 			})
-			It("should not add \"library/\" for one segment image names if they're not on dockerhub", func() {
+			It("should not prepend library/ to image names if they're not on dockerhub", func() {
 				Expect(getScopeFromImageAuthURL("ghcr.io/watchtower")).To(Equal("watchtower"))
-				// Expect(getScopeFromImageName("watchtower")).To(Equal("watchtower"))
+				Expect(getScopeFromImageAuthURL("ghcr.io/containrrr/watchtower")).To(Equal("containrrr/watchtower"))
 			})
 		})
 	})
-	When("getting a challenge url", func() {
+
+	Describe("GetChallengeURL", func() {
 		It("should create a valid challenge url object based on the image ref supplied", func() {
 			expected := url.URL{Host: "ghcr.io", Scheme: "https", Path: "/v2/"}
 			imageRef, _ := ref.ParseNormalizedNamed("ghcr.io/containrrr/watchtower:latest")
 			Expect(auth.GetChallengeURL(imageRef)).To(Equal(expected))
 		})
-		It("should assume docker hub if the image ref is not fully qualified", func() {
+		It("should assume Docker Hub for image refs with no explicit registry", func() {
 			expected := url.URL{Host: "index.docker.io", Scheme: "https", Path: "/v2/"}
 			imageRef, _ := ref.ParseNormalizedNamed("containrrr/watchtower:latest")
 			Expect(auth.GetChallengeURL(imageRef)).To(Equal(expected))
 		})
-		It("should use docker hub if the image ref specifies docker.io", func() {
+		It("should use index.docker.io if the image ref specifies docker.io", func() {
 			expected := url.URL{Host: "index.docker.io", Scheme: "https", Path: "/v2/"}
 			imageRef, _ := ref.ParseNormalizedNamed("docker.io/containrrr/watchtower:latest")
 			Expect(auth.GetChallengeURL(imageRef)).To(Equal(expected))
@@ -130,9 +133,8 @@ var _ = Describe("the auth module", func() {
 var scopeImageRegexp = MatchRegexp("^repository:[a-z0-9]+(/[a-z0-9]+)*:pull$")
 
 func getScopeFromImageAuthURL(imageName string) string {
-	challenge := `bearer realm="https://dummy.host/token",service="dummy.host",scope="repository:user/image:pull"`
-
 	normalizedRef, _ := ref.ParseNormalizedNamed(imageName)
+	challenge := `bearer realm="https://dummy.host/token",service="dummy.host",scope="repository:user/image:pull"`
 	URL, _ := auth.GetAuthURL(challenge, normalizedRef)
 
 	scope := URL.Query().Get("scope")

pkg/registry/helpers/helpers.go

@@ -4,6 +4,7 @@ import (
 	"github.com/docker/distribution/reference"
 )
 
+// domains for Docker Hub, the default registry
 const (
 	DefaultRegistryDomain       = "docker.io"
 	DefaultRegistryHost         = "index.docker.io"

pkg/registry/helpers/helpers_test.go

@@ -18,15 +18,19 @@ var _ = Describe("the helpers", func() {
 			_, err := GetRegistryAddress("")
 			Expect(err).To(HaveOccurred())
 		})
-		It("should return index.docker.io if passed an image name with no explicit domain", func() {
+		It("should return index.docker.io for image refs with no explicit registry", func() {
 			Expect(GetRegistryAddress("watchtower")).To(Equal("index.docker.io"))
 			Expect(GetRegistryAddress("containrrr/watchtower")).To(Equal("index.docker.io"))
 		})
+		It("should return index.docker.io for image refs with docker.io domain", func() {
+			Expect(GetRegistryAddress("docker.io/watchtower")).To(Equal("index.docker.io"))
+			Expect(GetRegistryAddress("docker.io/containrrr/watchtower")).To(Equal("index.docker.io"))
+		})
 		It("should return the host if passed an image name containing a local host", func() {
 			Expect(GetRegistryAddress("henk:80/watchtower")).To(Equal("henk:80"))
 			Expect(GetRegistryAddress("localhost/watchtower")).To(Equal("localhost"))
 		})
-		It("should return the server name if passed a fully qualified image name", func() {
+		It("should return the server address if passed a fully qualified image name", func() {
 			Expect(GetRegistryAddress("github.com/containrrr/config")).To(Equal("github.com"))
 		})
 	})

pkg/registry/manifest/manifest.go

@@ -18,12 +18,12 @@ func BuildManifestURL(container types.Container) (string, error) {
 		return "", err
 	}
 	normalizedTaggedRef, isTagged := normalizedRef.(ref.NamedTagged)
-		if !isTagged {
+	if !isTagged {
-			return "", errors.New("Parsed container image ref has no tag: " + normalizedRef.String())
+		return "", errors.New("Parsed container image ref has no tag: " + normalizedRef.String())
-		}
+	}
 
 	host, _ := helpers.GetRegistryAddress(normalizedTaggedRef.Name())
-	img, tag := ExtractImageAndTag(normalizedTaggedRef)
+	img, tag := ref.Path(normalizedTaggedRef), normalizedTaggedRef.Tag()
 
 	logrus.WithFields(logrus.Fields{
 		"image":      img,
@@ -43,8 +43,3 @@ func BuildManifestURL(container types.Container) (string, error) {
 	}
 	return url.String(), nil
 }
-
-// ExtractImageAndTag from image reference
-func ExtractImageAndTag(namedTaggedRef ref.NamedTagged) (string, string) {
-	return ref.Path(namedTaggedRef), namedTaggedRef.Tag()
-}

pkg/registry/manifest/manifest_test.go

@@ -19,30 +19,34 @@ func TestManifest(t *testing.T) {
 var _ = Describe("the manifest module", func() {
 	Describe("BuildManifestURL", func() {
 		It("should return a valid url given a fully qualified image", func() {
-			expected := "https://ghcr.io/v2/containrrr/watchtower/manifests/latest"
+			imageRef := "ghcr.io/containrrr/watchtower:mytag"
+			expected := "https://ghcr.io/v2/containrrr/watchtower/manifests/mytag"
 
-			URL, err := buildMockContainerManifestURL("ghcr.io/containrrr/watchtower:latest")
+			URL, err := buildMockContainerManifestURL(imageRef)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(URL).To(Equal(expected))
 		})
-		It("should assume Docker Hub for non-qualified images", func() {
+		It("should assume Docker Hub for image refs with no explicit registry", func() {
+			imageRef := "containrrr/watchtower:latest"
 			expected := "https://index.docker.io/v2/containrrr/watchtower/manifests/latest"
 
-			URL, err := buildMockContainerManifestURL("containrrr/watchtower:latest")
+			URL, err := buildMockContainerManifestURL(imageRef)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(URL).To(Equal(expected))
 		})
-		It("should assume latest for image refs without an explicit tag", func() {
+		It("should assume latest for image refs with no explicit tag", func() {
+			imageRef := "containrrr/watchtower"
 			expected := "https://index.docker.io/v2/containrrr/watchtower/manifests/latest"
 
-			URL, err := buildMockContainerManifestURL("containrrr/watchtower")
+			URL, err := buildMockContainerManifestURL(imageRef)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(URL).To(Equal(expected))
 		})
 		It("should not prepend library/ for single-part container names in registries other than Docker Hub", func() {
+			imageRef := "docker-registry.domain/imagename:latest"
 			expected := "https://docker-registry.domain/v2/imagename/manifests/latest"
 
-			URL, err := buildMockContainerManifestURL("docker-registry.domain/imagename:latest")
+			URL, err := buildMockContainerManifestURL(imageRef)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(URL).To(Equal(expected))
 		})

pkg/registry/trust.go

@@ -18,7 +18,7 @@ import (
 // loaded from environment variables or docker config
 // as available in that order
 func EncodedAuth(ref string) (string, error) {
-	auth, err := EncodedEnvAuth(ref)
+	auth, err := EncodedEnvAuth()
 	if err != nil {
 		auth, err = EncodedConfigAuth(ref)
 	}
@@ -28,7 +28,7 @@ func EncodedAuth(ref string) (string, error) {
 // EncodedEnvAuth returns an encoded auth config for the given registry
 // loaded from environment variables
 // Returns an error if authentication environment variables have not been set
-func EncodedEnvAuth(ref string) (string, error) {
+func EncodedEnvAuth() (string, error) {
 	username := os.Getenv("REPO_USER")
 	password := os.Getenv("REPO_PASS")
 	if username != "" && password != "" {
@@ -36,7 +36,7 @@ func EncodedEnvAuth(ref string) (string, error) {
 			Username: username,
 			Password: password,
 		}
-		log.Debugf("Loaded auth credentials for user %s on registry %s", auth.Username, ref)
+		log.Debugf("Loaded auth credentials for registry user %s from environment", auth.Username)
 		log.Tracef("Using auth password %s", auth.Password)
 		return EncodeAuth(auth)
 	}
@@ -60,7 +60,7 @@ func EncodedConfigAuth(imageRef string) (string, error) {
 	}
 	configFile, err := cliconfig.Load(configDir)
 	if err != nil {
-		log.Errorf("Unable to find default config file %s", err)
+		log.Errorf("Unable to find default config file: %s", err)
 		return "", err
 	}
 	credStore := CredentialsStore(*configFile)

pkg/registry/trust_test.go

@@ -1,41 +1,49 @@
 package registry
 
 import (
+	"os"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"os"
 )
 
-var _ = Describe("Testing with Ginkgo", func() {
+var _ = Describe("Registry credential helpers", func() {
-	It("encoded env auth_ should return an error if repo envs are unset", func() {
+	Describe("EncodedAuth", func() {
-		_ = os.Unsetenv("REPO_USER")
+		It("should return repo credentials from env when set", func() {
-		_ = os.Unsetenv("REPO_PASS")
+			var err error
+			expected := "eyJ1c2VybmFtZSI6ImNvbnRhaW5ycnItdXNlciIsInBhc3N3b3JkIjoiY29udGFpbnJyci1wYXNzIn0="
 
-		_, err := EncodedEnvAuth("")
+			err = os.Setenv("REPO_USER", "containrrr-user")
-		Expect(err).To(HaveOccurred())
+			Expect(err).NotTo(HaveOccurred())
+
+			err = os.Setenv("REPO_PASS", "containrrr-pass")
+			Expect(err).NotTo(HaveOccurred())
+
+			config, err := EncodedEnvAuth()
+			Expect(config).To(Equal(expected))
+			Expect(err).NotTo(HaveOccurred())
+		})
 	})
-	It("encoded env auth_ should return auth hash if repo envs are set", func() {
-		var err error
-		expectedHash := "eyJ1c2VybmFtZSI6ImNvbnRhaW5ycnItdXNlciIsInBhc3N3b3JkIjoiY29udGFpbnJyci1wYXNzIn0="
 
-		err = os.Setenv("REPO_USER", "containrrr-user")
+	Describe("EncodedEnvAuth", func() {
-		Expect(err).NotTo(HaveOccurred())
+		It("should return an error if repo envs are unset", func() {
+			_ = os.Unsetenv("REPO_USER")
+			_ = os.Unsetenv("REPO_PASS")
 
-		err = os.Setenv("REPO_PASS", "containrrr-pass")
+			_, err := EncodedEnvAuth()
-		Expect(err).NotTo(HaveOccurred())
+			Expect(err).To(HaveOccurred())
-
+		})
-		config, err := EncodedEnvAuth("")
-		Expect(config).To(Equal(expectedHash))
-		Expect(err).NotTo(HaveOccurred())
 	})
-	It("encoded config auth_ should return an error if file is not present", func() {
-		var err error
 
-		err = os.Setenv("DOCKER_CONFIG", "/dev/null/should-fail")
+	Describe("EncodedConfigAuth", func() {
-		Expect(err).NotTo(HaveOccurred())
+		It("should return an error if file is not present", func() {
+			var err error
 
-		_, err = EncodedConfigAuth("")
+			err = os.Setenv("DOCKER_CONFIG", "/dev/null/should-fail")
-		Expect(err).To(HaveOccurred())
+			Expect(err).NotTo(HaveOccurred())
-	})
+
+			_, err = EncodedConfigAuth("")
+			Expect(err).To(HaveOccurred())
+		})
 	})
 })