Repository: regclient/regclient (go)

Issue:
[Feature] Provide support for defining default settings for hosts inside regctl's config
<!--
Note: Please search to see if an issue already exists for the bug you encountered.
-->

### Current Behavior
When a login to registry command is executed, regctl creates a config.json in user's home directory and saves registry host related config there. Users can themselves also add hosts inside this config and can provide settings like credhelper, reqPerSec, requestConcurrent etc. When we have a huge number of registries these settings are pretty much similar across hosts and we end up having a very big config with repeated information.

Another challenge is that you need to precompile a list of registries that you want to apply custom config like credhelper, reqPerSec, requestConcurrent before actual regctl command is executed. This is mainly problematic when you are pushing regctl to many different linux boxes where applications/developers can make use of it.

### Expected Behavior

Provide a way to set common settings between hosts in a separate element say default under hosts element in config json. Now when a login to registry command is executed, regctl should create a host entry inside hosts element in config.json by using the default values from default element instead.

### Example Solution

hosts: {
  default: {
      credhelper: "path to cred helper binary"
      reqPerSec: 
      reqConcurrent: 
      tls: "enabled"
  },
  mytestacr.azurecr.io: {
   
  },
  docker-hub1.io: {
  
  }
}

### Version

0.7.1

### Environment

- Running as a binary
- Host platform: Linux and Windows
- Registry: ACR and Harbor

### Anything else

<!--
Links? References? Anything that will give us more context about the issue that you are encountering!
-->


Pull request description:
Feat: Add default host config
<!--

Commits must be signed indicating your agreement to the [DCO](https://developercertificate.org/).
See [DCO missing](https://github.com/src-d/guide/blob/master/developer-community/fix-DCO.md) for steps to fix a missing signoff.

-->

### Fixes issue

Fixes #802.
<!-- If this is a bug fix, include "fixes #xxxx", or "closes #xxxx" -->

### Describe the change

This adds support for a default host config in regclient. In `regctl`, this exposes a flag to set the default credential helper, however additional options can be directly edited in the config file.
<!-- Include the type of change: bug fix, new feature, breaking change, documentation update -->
<!-- Describe what was changed, why the change was made, and how it was implemented -->

### How to verify it

```shell
regctl config set --default-cred-helper "some-helper"
regctl tag ls $registry
```
<!-- Include steps that can be taken to verify the change -->

### Changelog text

- Feat: Add default host config.
<!-- If the release changelog should have an entry for this, include it here -->

### Please verify and check that the pull request fulfills the following requirements

<!-- Mark the following with an [X] to verify they are included -->

- [X] Tests have been added or not applicable
- [X] Documentation has been added, updated, or not applicable
- [X] Changes have been rebased to main
- [X] Multiple commits to the same code have been squashed

<!-- markdownlint-disable-file MD041 -->


Patch:
diff --git a/cmd/regctl/config.go b/cmd/regctl/config.go
index 4f89352..6e4fc93 100644
--- a/cmd/regctl/config.go
+++ b/cmd/regctl/config.go
@@ -28,18 +28,20 @@ var (
 type Config struct {
 	Filename      string                  `json:"-"`                 // filename that was loaded
 	Version       int                     `json:"version,omitempty"` // version the file in case the config file syntax changes in the future
-	Hosts         map[string]*config.Host `json:"hosts"`
+	Hosts         map[string]*config.Host `json:"hosts,omitempty"`
+	HostDefault   *config.Host            `json:"hostDefault,omitempty"`
 	BlobLimit     int64                   `json:"blobLimit,omitempty"`
 	IncDockerCert *bool                   `json:"incDockerCert,omitempty"`
 	IncDockerCred *bool                   `json:"incDockerCred,omitempty"`
 }
 
 type configCmd struct {
-	rootOpts   *rootCmd
-	blobLimit  int64
-	dockerCert bool
-	dockerCred bool
-	format     string
+	rootOpts      *rootCmd
+	blobLimit     int64
+	defCredHelper string
+	dockerCert    bool
+	dockerCred    bool
+	format        string
 }
 
 func NewConfigCmd(rootOpts *rootCmd) *cobra.Command {
@@ -82,6 +84,7 @@ regctl config set --docker-cred`,
 	configSetCmd.Flags().Int64Var(&configOpts.blobLimit, "blob-limit", 0, "limit for blob chunks, this is stored in memory")
 	configSetCmd.Flags().BoolVar(&configOpts.dockerCert, "docker-cert", false, "load certificates from docker")
 	configSetCmd.Flags().BoolVar(&configOpts.dockerCred, "docker-cred", false, "load credentials from docker")
+	configSetCmd.Flags().StringVar(&configOpts.defCredHelper, "default-cred-helper", "", "default credential helper")
 
 	configTopCmd.AddCommand(configGetCmd)
 	configTopCmd.AddCommand(configSetCmd)
@@ -110,6 +113,16 @@ func (configOpts *configCmd) runConfigSet(cmd *cobra.Command, args []string) err
 	if flagChanged(cmd, "blob-limit") {
 		c.BlobLimit = configOpts.blobLimit
 	}
+	if flagChanged(cmd, "default-cred-helper") {
+		if c.HostDefault != nil {
+			c.HostDefault.CredHelper = configOpts.defCredHelper
+		}
+		if c.HostDefault == nil && configOpts.defCredHelper != "" {
+			c.HostDefault = &config.Host{
+				CredHelper: configOpts.defCredHelper,
+			}
+		}
+	}
 	if flagChanged(cmd, "docker-cert") {
 		if !configOpts.dockerCert {
 			c.IncDockerCert = &configOpts.dockerCert
@@ -125,6 +138,10 @@ func (configOpts *configCmd) runConfigSet(cmd *cobra.Command, args []string) err
 		}
 	}
 
+	if c.HostDefault != nil && c.HostDefault.IsZero() {
+		c.HostDefault = nil
+	}
+
 	err = c.ConfigSave()
 	if err != nil {
 		return err
diff --git a/cmd/regctl/root.go b/cmd/regctl/root.go
index b0f6154..f043f6f 100644
--- a/cmd/regctl/root.go
+++ b/cmd/regctl/root.go
@@ -172,6 +172,9 @@ func (rootOpts *rootCmd) newRegClient() *regclient.RegClient {
 	if conf.IncDockerCert == nil || *conf.IncDockerCert {
 		rcOpts = append(rcOpts, regclient.WithDockerCerts())
 	}
+	if conf.HostDefault != nil {
+		rcOpts = append(rcOpts, regclient.WithConfigHostDefault(*conf.HostDefault))
+	}
 
 	rcHosts := []config.Host{}
 	for name, host := range conf.Hosts {
diff --git a/config/host.go b/config/host.go
index 37269a8..17d4e20 100644
--- a/config/host.go
+++ b/config/host.go
@@ -110,7 +110,7 @@ type Host struct {
 	Token         string            `json:"token,omitempty" yaml:"token"`                 // token, experimental for specific APIs
 	CredHelper    string            `json:"credHelper,omitempty" yaml:"credHelper"`       // credential helper command for requesting logins
 	CredExpire    timejson.Duration `json:"credExpire,omitempty" yaml:"credExpire"`       // time until credential expires
-	CredHost      string            `json:"credHost" yaml:"credHost"`                     // used when a helper hostname doesn't match Hostname
+	CredHost      string            `json:"credHost,omitempty" yaml:"credHost"`           // used when a helper hostname doesn't match Hostname
 	PathPrefix    string            `json:"pathPrefix,omitempty" yaml:"pathPrefix"`       // used for mirrors defined within a repository namespace
 	Mirrors       []string          `json:"mirrors,omitempty" yaml:"mirrors"`             // list of other Host Names to use as mirrors
 	Priority      uint              `json:"priority,omitempty" yaml:"priority"`           // priority when sorting mirrors, higher priority attempted first
@@ -141,16 +141,48 @@ func HostNew() *Host {
 	return &h
 }
 
-// HostNewName creates a default Host with a hostname.
-func HostNewName(name string) *Host {
-	h := HostNew()
+// HostNewDefName creates a host using provided defaults and hostname.
+func HostNewDefName(def *Host, name string) *Host {
+	var h Host
+	if def == nil {
+		h = *HostNew()
+	} else {
+		h = *def
+		// configure required defaults
+		if h.TLS == TLSUndefined {
+			h.TLS = TLSEnabled
+		}
+		if h.APIOpts == nil {
+			h.APIOpts = map[string]string{}
+		}
+		if h.ReqConcurrent == 0 {
+			h.ReqConcurrent = int64(defaultConcurrent)
+		}
+		if h.ReqPerSec == 0 {
+			h.ReqPerSec = float64(defaultReqPerSec)
+		}
+		// copy any fields that are not passed by value
+		if len(h.APIOpts) > 0 {
+			orig := h.APIOpts
+			h.APIOpts = map[string]string{}
+			for k, v := range orig {
+				h.APIOpts[k] = v
+			}
+		}
+		if h.Mirrors != nil {
+			orig := h.Mirrors
+			h.Mirrors = make([]string, len(orig))
+			copy(h.Mirrors, orig)
+		}
+	}
+	// configure host
 	origName := name
 	// Docker Hub is a special case
 	if name == DockerRegistryAuth || name == DockerRegistryDNS || name == DockerRegistry {
 		h.Name = DockerRegistry
 		h.Hostname = DockerRegistryDNS
 		h.CredHost = DockerRegistryAuth
-		return h
+		return &h
 	}
 	// handle http/https prefix
 	i := strings.Index(name, "://")
@@ -171,7 +203,12 @@ func HostNewName(name string) *Host {
 	if origName != name {
 		h.CredHost = origName
 	}
-	return h
+	return &h
+}
+
+// HostNewName creates a default Host with a hostname.
+func HostNewName(name string) *Host {
+	return HostNewDefName(nil, name)
 }
 
 // GetCred returns the credential, fetching from a credential helper if needed.
@@ -200,6 +237,35 @@ func (host *Host) refreshHelper() {
 	}
 }
 
+// IsZero returns true if the struct is set to the zero value or the result of [HostNew].
+func (host Host) IsZero() bool {
+	if host.Name != "" ||
+		(host.TLS != TLSUndefined && host.TLS != TLSEnabled) ||
+		host.RegCert != "" ||
+		host.ClientCert != "" ||
+		host.ClientKey != "" ||
+		host.Hostname != "" ||
+		host.User != "" ||
+		host.Pass != "" ||
+		host.Token != "" ||
+		host.CredHelper != "" ||
+		host.CredExpire != 0 ||
+		host.CredHost != "" ||
+		host.PathPrefix != "" ||
+		len(host.Mirrors) != 0 ||
+		host.Priority != 0 ||
+		host.RepoAuth ||
+		len(host.APIOpts) != 0 ||
+		host.BlobChunk != 0 ||
+		host.BlobMax != 0 ||
+		(host.ReqPerSec != 0 && host.ReqPerSec != float64(defaultReqPerSec)) ||
+		(host.ReqConcurrent != 0 && host.ReqConcurrent != int64(defaultConcurrent)) ||
+		!host.credRefresh.IsZero() {
+		return false
+	}
+	return true
+}
+
 // Merge adds fields from a new config host entry.
 func (host *Host) Merge(newHost Host, log *logrus.Logger) error {
 	name := newHost.Name
diff --git a/regclient.go b/regclient.go
index d02f44f..0ac4be8 100644
--- a/regclient.go
+++ b/regclient.go
@@ -31,12 +31,12 @@ const (
 
 // RegClient is used to access OCI distribution-spec registries.
 type RegClient struct {
-	hosts map[string]*config.Host
-	log   *logrus.Logger
-	// mu        sync.Mutex
-	regOpts   []reg.Opts
-	schemes   map[string]scheme.API
-	userAgent string
+	hosts       map[string]*config.Host
+	hostDefault *config.Host
+	log         *logrus.Logger
+	regOpts     []reg.Opts
+	schemes     map[string]scheme.API
+	userAgent   string
 }
 
 // Opt functions are used by [New] to create a [*RegClient].
@@ -47,10 +47,9 @@ func New(opts ...Opt) *RegClient {
 	var rc = RegClient{
 		hosts:     map[string]*config.Host{},
 		userAgent: DefaultUserAgent,
-		// logging is disabled by default
-		log:     &logrus.Logger{Out: io.Discard},
-		regOpts: []reg.Opts{},
-		schemes: map[string]scheme.API{},
+		log:       &logrus.Logger{Out: io.Discard},
+		regOpts:   []reg.Opts{},
+		schemes:   map[string]scheme.API{},
 	}
 
 	info := version.GetInfo()
@@ -74,6 +73,7 @@ func New(opts ...Opt) *RegClient {
 	}
 	rc.regOpts = append(rc.regOpts,
 		reg.WithConfigHosts(hostList),
+		reg.WithConfigHostDefault(rc.hostDefault),
 		reg.WithLog(rc.log),
 		reg.WithUserAgent(rc.userAgent),
 	)
@@ -126,6 +126,13 @@ func WithConfigHost(configHost ...config.Host) Opt {
 	}
 }
 
+// WithConfigHostDefault adds default settings for new hosts.
+func WithConfigHostDefault(configHost config.Host) Opt {
+	return func(rc *RegClient) {
+		rc.hostDefault = &configHost
+	}
+}
+
 // WithConfigHosts adds a list of config host settings.
 //
 // Deprecated: replace with [WithConfigHost].
@@ -249,12 +256,9 @@ func (rc *RegClient) hostLoad(src string, hosts []config.Host) {
 func (rc *RegClient) hostSet(newHost config.Host) error {
 	name := newHost.Name
 	var err error
-	// hostSet should only run on New, which single threaded
-	// rc.mu.Lock()
-	// defer rc.mu.Unlock()
 	if _, ok := rc.hosts[name]; !ok {
 		// merge newHost with default host settings
-		rc.hosts[name] = config.HostNewName(name)
+		rc.hosts[name] = config.HostNewDefName(rc.hostDefault, name)
 		err = rc.hosts[name].Merge(newHost, nil)
 	} else {
 		// merge newHost with existing settings
diff --git a/scheme/reg/reg.go b/scheme/reg/reg.go
index 2cf9c90..5672ba6 100644
--- a/scheme/reg/reg.go
+++ b/scheme/reg/reg.go
@@ -45,6 +45,7 @@ type Reg struct {
 	reghttpOpts     []reghttp.Opts
 	log             *logrus.Logger
 	hosts           map[string]*config.Host
+	hostDefault     *config.Host
 	features        map[featureKey]*featureVal
 	blobChunkSize   int64
 	blobChunkLimit  int64
@@ -115,7 +116,7 @@ func (reg *Reg) hostGet(hostname string) *config.Host {
 	reg.muHost.Lock()
 	defer reg.muHost.Unlock()
 	if _, ok := reg.hosts[hostname]; !ok {
-		newHost := config.HostNewName(hostname)
+		newHost := config.HostNewDefName(reg.hostDefault, hostname)
 		// check for normalized hostname
 		if newHost.Name != hostname {
 			hostname = newHost.Name
@@ -201,6 +202,13 @@ func WithCertFiles(files []string) Opts {
 	}
 }
 
+// WithConfigHostDefault provides default settings for hosts.
+func WithConfigHostDefault(ch *config.Host) Opts {
+	return func(r *Reg) {
+		r.hostDefault = ch
+	}
+}
+
 // WithConfigHosts adds host configs for credentials
 func WithConfigHosts(configHosts []*config.Host) Opts {
 	return func(r *Reg) {


Test patch:
diff --git a/cmd/regctl/config_test.go b/cmd/regctl/config_test.go
index 9db07a8..4d590ef 100644
--- a/cmd/regctl/config_test.go
+++ b/cmd/regctl/config_test.go
@@ -15,8 +15,8 @@ func TestConfig(t *testing.T) {
 	if err != nil {
 		t.Errorf("failed to run config get: %v", err)
 	}
-	if out != `{"hosts":{}}` {
-		t.Errorf("unexpected output from empty config, expected: %s, received: %s", `{"hosts":{}}`, out)
+	if out != `{}` {
+		t.Errorf("unexpected output from empty config, expected: %s, received: %s", `{}`, out)
 	}
 
 	// set options
@@ -52,10 +52,27 @@ func TestConfig(t *testing.T) {
 		t.Errorf("unexpected output for docker-cred, expected: false, received: %s", out)
 	}
 
+	// set a default credential helper
+	out, err = cobraTest(t, nil, "config", "set", "--default-cred-helper", "test-helper")
+	if err != nil {
+		t.Errorf("failed to set credential helper: %v", err)
+	}
+	if out != "" {
+		t.Errorf("unexpected output from set: %s", out)
+	}
+
+	out, err = cobraTest(t, nil, "config", "get", "--format", "{{ .HostDefault.CredHelper }}")
+	if err != nil {
+		t.Errorf("failed to run config get on default cred helper: %v", err)
+	}
+	if out != "test-helper" {
+		t.Errorf("unexpected output for default cred helper, expected: test-helper, received: %s", out)
+	}
+
 	// reset back to zero values
-	out, err = cobraTest(t, nil, "config", "set", "--blob-limit", "0", "--docker-cert", "--docker-cred")
+	out, err = cobraTest(t, nil, "config", "set", "--blob-limit", "0", "--docker-cert", "--docker-cred", "--default-cred-helper", "")
 	if err != nil {
-		t.Errorf("failed to set blob-limit: %v", err)
+		t.Errorf("failed to set default values: %v", err)
 	}
 	if out != "" {
 		t.Errorf("unexpected output from set: %s", out)
@@ -66,8 +83,7 @@ func TestConfig(t *testing.T) {
 	if err != nil {
 		t.Errorf("failed to run config get: %v", err)
 	}
-	if out != `{"hosts":{}}` {
-		t.Errorf("unexpected output from empty config, expected: %s, received: %s", `{"hosts":{}}`, out)
+	if out != `{}` {
+		t.Errorf("unexpected output from empty config, expected: %s, received: %s", `{}`, out)
 	}
-
 }
diff --git a/config/host_test.go b/config/host_test.go
index 18684d8..0eeb091 100644
--- a/config/host_test.go
+++ b/config/host_test.go
@@ -21,10 +21,21 @@ func TestConfig(t *testing.T) {
 	t.Setenv("PATH", filepath.Join(cwd, "testdata")+string(os.PathListSeparator)+curPath)
 
 	// generate new/blank
-	blankHostP := HostNew()
+	newHostP := HostNew()
 
 	// generate new/hostname
-	emptyHostP := HostNewName("host.example.org")
+	newHostNameP := HostNewName("host.example.org")
+
+	defMirror := Host{
+		Mirrors: []string{"mirror.example.org"},
+	}
+	defCredHelper := Host{
+		CredHelper: "docker-credential-test",
+	}
+
+	newHostDefNil := HostNewDefName(nil, "host.example.org")
+	newHostDefMirror := HostNewDefName(&defMirror, "host.example.org")
+	newHostDefCredHelper := HostNewDefName(&defCredHelper, "host.example.org")
 
 	caCert := string(`-----BEGIN CERTIFICATE-----
 	MIIC/zCCAeegAwIBAgIUPrFPsUzINvS75tp6kIdsycXrrSQwDQYJKoZIhvcNAQEL
@@ -151,7 +162,7 @@ func TestConfig(t *testing.T) {
 	}
 
 	// merge blank with json
-	exMergeBlank := *blankHostP
+	exMergeBlank := *newHostP
 	err = (&exMergeBlank).Merge(exHost, nil)
 	if err != nil {
 		t.Errorf("failed to merge blank host with exHost: %v", err)
@@ -161,7 +172,7 @@ func TestConfig(t *testing.T) {
 	if err != nil {
 		t.Errorf("failed to merge ex host with exHost2: %v", err)
 	}
-	exMergeCredHelper := *blankHostP
+	exMergeCredHelper := *newHostP
 	err = (&exMergeCredHelper).Merge(exHostCredHelper, nil)
 	if err != nil {
 		t.Errorf("failed to merge blank host with exHostCredHelper: %v", err)
@@ -183,19 +194,38 @@ func TestConfig(t *testing.T) {
 		host       Host
 		hostExpect Host
 		credExpect Cred
+		isZero     bool
 	}{
 		{
-			name: "blank",
-			host: *blankHostP,
+			name:       "empty",
+			host:       Host{},
+			hostExpect: Host{},
+			credExpect: Cred{},
+			isZero:     true,
+		},
+		{
+			name: "new",
+			host: *newHostP,
 			hostExpect: Host{
 				TLS:     TLSEnabled,
 				APIOpts: map[string]string{},
 			},
 			credExpect: Cred{},
+			isZero:     true,
+		},
+		{
+			name: "new-name",
+			host: *newHostNameP,
+			hostExpect: Host{
+				TLS:      TLSEnabled,
+				Hostname: "host.example.org",
+				APIOpts:  map[string]string{},
+			},
+			credExpect: Cred{},
 		},
 		{
-			name: "empty",
-			host: *emptyHostP,
+			name: "new-default-nil",
+			host: *newHostDefNil,
 			hostExpect: Host{
 				TLS:      TLSEnabled,
 				Hostname: "host.example.org",
@@ -203,6 +233,31 @@ func TestConfig(t *testing.T) {
 			},
 			credExpect: Cred{},
 		},
+		{
+			name: "new-default-mirror",
+			host: *newHostDefMirror,
+			hostExpect: Host{
+				TLS:      TLSEnabled,
+				Hostname: "host.example.org",
+				APIOpts:  map[string]string{},
+				Mirrors:  []string{"mirror.example.org"},
+			},
+			credExpect: Cred{},
+		},
+		{
+			name: "new-default-cred-helper",
+			host: *newHostDefCredHelper,
+			hostExpect: Host{
+				TLS:        TLSEnabled,
+				Hostname:   "host.example.org",
+				APIOpts:    map[string]string{},
+				CredHelper: "docker-credential-test",
+			},
+			credExpect: Cred{
+				User:     "hello",
+				Password: "world",
+			},
+		},
 		{
 			name: "exHost",
 			host: exHost,
@@ -363,6 +418,9 @@ func TestConfig(t *testing.T) {
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
+			if tc.host.IsZero() != tc.isZero {
+				t.Errorf("IsZero did not return %t", tc.isZero)
+			}
 			// check each field
 			if tc.host.TLS != tc.hostExpect.TLS {
 				expect, _ := tc.hostExpect.TLS.MarshalText()
diff --git a/config/testdata/docker-credential-test b/config/testdata/docker-credential-test
index 2812d06..3fe42be 100755
--- a/config/testdata/docker-credential-test
+++ b/config/testdata/docker-credential-test
@@ -3,6 +3,7 @@
 list='{
   "https://index.docker.io/v1/": "hubuser",
   "http://http.example.com/": "hello",
+  "host.example.org": "hello",
   "testhost.example.com": "hello",
   "testtoken.example.com": "<token>"
 }'
@@ -19,6 +20,12 @@ registry_http='
   "Secret": "universe"
 }
 '
+registry_host_org='
+{ "ServerURL": "host.example.org",
+  "Username": "hello",
+  "Secret": "world"
+}
+'
 registry_testhost='
 { "ServerURL": "testhost.example.com",
   "Username": "hello",
@@ -43,6 +50,10 @@ if [ "$1" = "get" ]; then
       echo "${registry_http}"
       exit 0
       ;;
+    host.example.org)
+      echo "${registry_host_org}"
+      exit 0
+      ;;
     testhost.example.com)
       echo "${registry_testhost}"
       exit 0
diff --git a/regclient_test.go b/regclient_test.go
index 2bef173..3611bec 100644
--- a/regclient_test.go
+++ b/regclient_test.go
@@ -68,6 +68,7 @@ func TestNew(t *testing.T) {
 			},
 		},
 	}
+	defaultRegOptCount := 4
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
 			result := New(tc.opts...)
@@ -90,8 +91,8 @@ func TestNew(t *testing.T) {
 				}
 			}
 			if len(tc.expect.regOpts) > 0 {
-				if len(tc.expect.regOpts)+3 != len(result.regOpts) {
-					t.Errorf("regOpts length mismatch, expected %d, received %d", len(tc.expect.regOpts), len(result.regOpts))
+				if len(tc.expect.regOpts)+defaultRegOptCount != len(result.regOpts) {
+					t.Errorf("regOpts length mismatch, expected %d, received %d", len(tc.expect.regOpts)+defaultRegOptCount, len(result.regOpts))
 				}
 				// TODO: can content of each regOpt be compared?
 			}