Add ability to override default registry and tag in pkg/name (#824)

* Add name Option functions

Adds a name OptionFn type that allows for overriding default option
values. Also introduces two new option fields, defaultRegistry and
defaultTag. These are set to the previously used defaults unless
overridden by their respective OptionFns.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

* Set default registry when building the registry object

Moves registry name defaulting to construction rather than defaulting in
registry methods. This allows for a user to override the configured
default via name options.

Note that we still rewrite docker.io to index.docker.io if it is
explictly provided. This also means that if the default is set to
docker.io by a user and no registry is provided, we will rewrite the
defeault to index.docker.io.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

* Set default tag when building tag object

Moves tag defaulting to construction of the tag object rather than
defaulting in tag methods.

Note that we assume a default tag is valid in the eyes of the user and
do not check the default for validity if it is overridden. This is
consistent with the current behavior as defaulting was previously
happening much later than the check for a valid tag.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

* Add ParseReference tests for registry and tag defaulting

Adds tests for paring full image references with registry and tag
defaulting.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

pkg/name/options.go

@@ -14,13 +14,29 @@
 
 package name
 
+const (
+	// DefaultRegistry is the registry name that will be used if no registry
+	// provided and the default is not overridden.
+	DefaultRegistry      = "index.docker.io"
+	defaultRegistryAlias = "docker.io"
+
+	// DefaultTag is the tag name that will be used if no tag provided and the
+	// default is not overridden.
+	DefaultTag = "latest"
+)
+
 type options struct {
-	strict   bool // weak by default
-	insecure bool // secure by default
+	strict          bool // weak by default
+	insecure        bool // secure by default
+	defaultRegistry string
+	defaultTag      string
 }
 
 func makeOptions(opts ...Option) options {
-	opt := options{}
+	opt := options{
+		defaultRegistry: DefaultRegistry,
+		defaultTag:      DefaultTag,
+	}
 	for _, o := range opts {
 		o(&opt)
 	}
@@ -47,3 +63,21 @@ func WeakValidation(opts *options) {
 func Insecure(opts *options) {
 	opts.insecure = true
 }
+
+// OptionFn is a function that returns an option.
+type OptionFn func() Option
+
+// WithDefaultRegistry sets the default registry that will be used if one is not
+// provided.
+func WithDefaultRegistry(r string) Option {
+	return func(opts *options) {
+		opts.defaultRegistry = r
+	}
+}
+
+// WithDefaultTag sets the default tag that will be used if one is not provided.
+func WithDefaultTag(t string) Option {
+	return func(opts *options) {
+		opts.defaultTag = t
+	}
+}

pkg/name/ref_test.go

@@ -18,6 +18,35 @@ import (
 	"testing"
 )
 
+var (
+	testDefaultRegistry = "registry.upbound.io"
+	testDefaultTag      = "stable"
+	inputDefaultNames   = []string{
+		"crossplane/provider-gcp",
+		"crossplane/provider-gcp:v0.14.0",
+		"ubuntu",
+		"gcr.io/crossplane/provider-gcp:latest",
+	}
+	outputDefaultNames = []string{
+		"registry.upbound.io/crossplane/provider-gcp:stable",
+		"registry.upbound.io/crossplane/provider-gcp:v0.14.0",
+		"registry.upbound.io/ubuntu:stable",
+		"gcr.io/crossplane/provider-gcp:latest",
+	}
+)
+
+func TestParseReferenceDefaulting(t *testing.T) {
+	for i, name := range inputDefaultNames {
+		ref, err := ParseReference(name, WithDefaultRegistry(testDefaultRegistry), WithDefaultTag(testDefaultTag))
+		if err != nil {
+			t.Errorf("ParseReference(%q); %v", name, err)
+		}
+		if ref.Name() != outputDefaultNames[i] {
+			t.Errorf("ParseReference(%q); got %v, want %v", name, ref.String(), outputDefaultNames[i])
+		}
+	}
+}
+
 func TestParseReference(t *testing.T) {
 	for _, name := range goodWeakValidationDigestNames {
 		ref, err := ParseReference(name, WeakValidation)

pkg/name/registry.go

@@ -21,12 +21,6 @@ import (
 	"strings"
 )
 
-const (
-	// DefaultRegistry is Docker Hub, assumed when a hostname is omitted.
-	DefaultRegistry      = "index.docker.io"
-	defaultRegistryAlias = "docker.io"
-)
-
 // Detect more complex forms of local references.
 var reLocal = regexp.MustCompile(`.*\.local(?:host)?(?::\d{1,5})?$`)
 
@@ -44,10 +38,7 @@ type Registry struct {
 
 // RegistryStr returns the registry component of the Registry.
 func (r Registry) RegistryStr() string {
-	if r.registry != "" {
-		return r.registry
-	}
-	return DefaultRegistry
+	return r.registry
 }
 
 // Name returns the name from which the Registry was derived.
@@ -124,6 +115,9 @@ func NewRegistry(name string, opts ...Option) (Registry, error) {
 		return Registry{}, err
 	}
 
+	if name == "" {
+		name = opt.defaultRegistry
+	}
 	// Rewrite "docker.io" to "index.docker.io".
 	// See: https://github.com/google/go-containerregistry/issues/68
 	if name == defaultRegistryAlias {

pkg/name/registry_test.go

@@ -108,6 +108,24 @@ func TestDefaultRegistryNames(t *testing.T) {
 	}
 }
 
+func TestOverrideDefaultRegistryNames(t *testing.T) {
+	testRegistries := []string{"docker.io", ""}
+	expectedRegistries := []string{"index.docker.io", "gcr.io"}
+	overrideDefault := "gcr.io"
+
+	for i, testRegistry := range testRegistries {
+		registry, err := NewRegistry(testRegistry, WeakValidation, WithDefaultRegistry(overrideDefault))
+		if err != nil {
+			t.Fatalf("`%s` should be a valid Registry name, got error: %v", testRegistry, err)
+		}
+
+		actualRegistry := registry.RegistryStr()
+		if actualRegistry != expectedRegistries[i] {
+			t.Errorf("RegistryStr() was incorrect for %v. Wanted: `%s` Got: `%s`", registry, expectedRegistries[i], actualRegistry)
+		}
+	}
+}
+
 func TestRegistryComponents(t *testing.T) {
 	t.Parallel()
 	testRegistry := "gcr.io"

pkg/name/tag.go

@@ -19,7 +19,6 @@ import (
 )
 
 const (
-	defaultTag = "latest"
 	// TODO(dekkagaijin): use the docker/distribution regexes for validation.
 	tagChars = "abcdefghijklmnopqrstuvwxyz0123456789_-.ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 	tagDelim = ":"
@@ -47,10 +46,7 @@ func (t Tag) Identifier() string {
 
 // TagStr returns the tag component of the Tag.
 func (t Tag) TagStr() string {
-	if t.tag != "" {
-		return t.tag
-	}
-	return defaultTag
+	return t.tag
 }
 
 // Name returns the name from which the Tag was derived.
@@ -96,6 +92,10 @@ func NewTag(name string, opts ...Option) (Tag, error) {
 		}
 	}
 
+	if tag == "" {
+		tag = opt.defaultTag
+	}
+
 	repo, err := NewRepository(base, opts...)
 	if err != nil {
 		return Tag{}, err

pkg/name/tag_test.go

@@ -146,3 +146,17 @@ func TestAllDefaults(t *testing.T) {
 		t.Errorf("Name() was incorrect for %v. Wanted: `%s` Got: `%s`", tag, expectedName, actualName)
 	}
 }
+
+func TestOverrideDefault(t *testing.T) {
+	tagNameStr := "ubuntu"
+	tag, err := NewTag(tagNameStr, WeakValidation, WithDefaultTag("other"))
+	if err != nil {
+		t.Fatalf("`%s` should be a valid Tag name, got error: %v", tagNameStr, err)
+	}
+
+	expectedName := "index.docker.io/library/ubuntu:other"
+	actualName := tag.Name()
+	if actualName != expectedName {
+		t.Errorf("Name() was incorrect for %v. Wanted: `%s` Got: `%s`", tag, expectedName, actualName)
+	}
+}