Repository: regclient/regclient (go)

Issue:
[Issue] Pinging registry slows down when registry is not available
### Current Behavior

Pinging the registry before the registry is configured is slowing down operations due to sleeping backoff and wait.  Local registries / configuration may not be available at the first pass (certificate setup, authentication, proxies, etc.)

### Expected Behavior

No ping / configuration check, or allow it to be skipped / disabled

### Steps To Reproduce

1. Configure regctl like below `regctl registry set localhost:5000 --tls enabled --req-concurrent 15 --req-per-sec 15`
1. Sleep / backoff starts
1. Start a local registry: `docker run -dp 5000:5000 --restart=always --name registry registry`


Ideally something like:
regctl registry set localhost:5000 --tls enabled --req-concurrent 15 --req-per-sec 15 --skip-ping-check

### Version
[https://github.com/regclient/regclient/releases/tag/v0.5.6](v0.5.6)



Pull request description:
Allow registry check to be skipped on set and login
<!--

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 #643 
<!-- If this is a bug fix, include "fixes #xxxx", or "closes #xxxx" -->

### Describe the change

This adds a `--skip-check` flag to `regctl registry set` and `regctl registry login` commands.
<!-- 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 registry set ... --skip-check
regctl registry logini ... --skip-check
```
<!-- Include steps that can be taken to verify the change -->

### Changelog text

- Add a `--skip-check` option to `regctl registry set` and `regctl registry login`.
<!-- 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/registry.go b/cmd/regctl/registry.go
index 64f9cb2..01f039a 100644
--- a/cmd/regctl/registry.go
+++ b/cmd/regctl/registry.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"os"
 	"strings"
 	"syscall"
 
@@ -33,6 +32,7 @@ type registryCmd struct {
 	blobChunk, blobMax   int64
 	reqPerSec            float64
 	reqConcurrent        int64
+	skipCheck            bool
 	apiOpts              []string
 	scheme               string   // TODO: remove
 	dns                  []string // TODO: remove
@@ -84,25 +84,27 @@ the contents of the file, e.g. --cacert "$(cat reg-ca.crt)"`,
 
 	registryLoginCmd.Flags().StringVarP(&registryOpts.user, "user", "u", "", "Username")
 	registryLoginCmd.Flags().StringVarP(&registryOpts.pass, "pass", "p", "", "Password")
-	registryLoginCmd.Flags().BoolVarP(&registryOpts.passStdin, "pass-stdin", "", false, "Read password from stdin")
+	registryLoginCmd.Flags().BoolVar(&registryOpts.passStdin, "pass-stdin", false, "Read password from stdin")
+	registryLoginCmd.Flags().BoolVar(&registryOpts.skipCheck, "skip-check", false, "Skip checking connectivity to the registry")
 	_ = registryLoginCmd.RegisterFlagCompletionFunc("user", completeArgNone)
 	_ = registryLoginCmd.RegisterFlagCompletionFunc("pass", completeArgNone)
 
-	registrySetCmd.Flags().StringVarP(&registryOpts.credHelper, "cred-helper", "", "", "Credential helper (full binary name, including docker-credential- prefix)")
-	registrySetCmd.Flags().StringVarP(&registryOpts.cacert, "cacert", "", "", "CA Certificate (not a filename, use \"$(cat ca.pem)\" to use a file)")
-	registrySetCmd.Flags().StringVarP(&registryOpts.clientCert, "client-cert", "", "", "Client certificate for mTLS (not a filename, use \"$(cat client.pem)\" to use a file)")
-	registrySetCmd.Flags().StringVarP(&registryOpts.clientKey, "client-key", "", "", "Client key for mTLS (not a filename, use \"$(cat client.key)\" to use a file)")
-	registrySetCmd.Flags().StringVarP(&registryOpts.tls, "tls", "", "", "TLS (enabled, insecure, disabled)")
-	registrySetCmd.Flags().StringVarP(&registryOpts.hostname, "hostname", "", "", "Hostname or ip with port")
-	registrySetCmd.Flags().StringVarP(&registryOpts.pathPrefix, "path-prefix", "", "", "Prefix to all repositories")
-	registrySetCmd.Flags().StringArrayVarP(&registryOpts.mirrors, "mirror", "", nil, "List of mirrors (registry names)")
-	registrySetCmd.Flags().UintVarP(&registryOpts.priority, "priority", "", 0, "Priority (for sorting mirrors)")
-	registrySetCmd.Flags().BoolVarP(&registryOpts.repoAuth, "repo-auth", "", false, "Separate auth requests per repository instead of per registry")
-	registrySetCmd.Flags().Int64VarP(&registryOpts.blobChunk, "blob-chunk", "", 0, "Blob chunk size")
-	registrySetCmd.Flags().Int64VarP(&registryOpts.blobMax, "blob-max", "", 0, "Blob size before switching to chunked push, -1 to disable")
-	registrySetCmd.Flags().Float64VarP(&registryOpts.reqPerSec, "req-per-sec", "", 0, "Requests per second")
-	registrySetCmd.Flags().Int64VarP(&registryOpts.reqConcurrent, "req-concurrent", "", 0, "Concurrent requests")
-	registrySetCmd.Flags().StringArrayVarP(&registryOpts.apiOpts, "api-opts", "", nil, "List of options (key=value))")
+	registrySetCmd.Flags().StringVar(&registryOpts.credHelper, "cred-helper", "", "Credential helper (full binary name, including docker-credential- prefix)")
+	registrySetCmd.Flags().StringVar(&registryOpts.cacert, "cacert", "", "CA Certificate (not a filename, use \"$(cat ca.pem)\" to use a file)")
+	registrySetCmd.Flags().StringVar(&registryOpts.clientCert, "client-cert", "", "Client certificate for mTLS (not a filename, use \"$(cat client.pem)\" to use a file)")
+	registrySetCmd.Flags().StringVar(&registryOpts.clientKey, "client-key", "", "Client key for mTLS (not a filename, use \"$(cat client.key)\" to use a file)")
+	registrySetCmd.Flags().StringVar(&registryOpts.tls, "tls", "", "TLS (enabled, insecure, disabled)")
+	registrySetCmd.Flags().StringVar(&registryOpts.hostname, "hostname", "", "Hostname or ip with port")
+	registrySetCmd.Flags().StringVar(&registryOpts.pathPrefix, "path-prefix", "", "Prefix to all repositories")
+	registrySetCmd.Flags().StringArrayVar(&registryOpts.mirrors, "mirror", nil, "List of mirrors (registry names)")
+	registrySetCmd.Flags().UintVar(&registryOpts.priority, "priority", 0, "Priority (for sorting mirrors)")
+	registrySetCmd.Flags().BoolVar(&registryOpts.repoAuth, "repo-auth", false, "Separate auth requests per repository instead of per registry")
+	registrySetCmd.Flags().Int64Var(&registryOpts.blobChunk, "blob-chunk", 0, "Blob chunk size")
+	registrySetCmd.Flags().Int64Var(&registryOpts.blobMax, "blob-max", 0, "Blob size before switching to chunked push, -1 to disable")
+	registrySetCmd.Flags().Float64Var(&registryOpts.reqPerSec, "req-per-sec", 0, "Requests per second")
+	registrySetCmd.Flags().Int64Var(&registryOpts.reqConcurrent, "req-concurrent", 0, "Concurrent requests")
+	registrySetCmd.Flags().BoolVar(&registryOpts.skipCheck, "skip-check", false, "Skip checking connectivity to the registry")
+	registrySetCmd.Flags().StringArrayVar(&registryOpts.apiOpts, "api-opts", nil, "List of options (key=value))")
 	_ = registrySetCmd.RegisterFlagCompletionFunc("cacert", completeArgNone)
 	_ = registrySetCmd.RegisterFlagCompletionFunc("tls", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		return []string{
@@ -119,8 +121,8 @@ the contents of the file, e.g. --cacert "$(cat reg-ca.crt)"`,
 	_ = registrySetCmd.RegisterFlagCompletionFunc("blob-max", completeArgNone)
 
 	// TODO: eventually remove
-	registrySetCmd.Flags().StringVarP(&registryOpts.scheme, "scheme", "", "", "[Deprecated] Scheme (http, https)")
-	registrySetCmd.Flags().StringArrayVarP(&registryOpts.dns, "dns", "", nil, "[Deprecated] DNS hostname or ip with port")
+	registrySetCmd.Flags().StringVar(&registryOpts.scheme, "scheme", "", "[Deprecated] Scheme (http, https)")
+	registrySetCmd.Flags().StringArrayVar(&registryOpts.dns, "dns", nil, "[Deprecated] DNS hostname or ip with port")
 	_ = registrySetCmd.Flags().MarkHidden("scheme")
 	_ = registrySetCmd.Flags().MarkHidden("dns")
 
@@ -176,7 +178,8 @@ func (registryOpts *registryCmd) runRegistryConfig(cmd *cobra.Command, args []st
 		}
 	}
 
-	fmt.Println(string(hj))
+	// TODO: enable formatted/template output
+	fmt.Fprintf(cmd.OutOrStdout(), "%s\n", hj)
 	return nil
 }
 
@@ -201,7 +204,7 @@ func (registryOpts *registryCmd) runRegistryLogin(cmd *cobra.Command, args []str
 		return fmt.Errorf("user must be provided to read password from stdin")
 	} else {
 		// prompt for username
-		reader := bufio.NewReader(os.Stdin)
+		reader := bufio.NewReader(cmd.InOrStdin())
 		defUser := ""
 		if h.User != "" {
 			defUser = " [" + h.User + "]"
@@ -219,7 +222,7 @@ func (registryOpts *registryCmd) runRegistryLogin(cmd *cobra.Command, args []str
 	if flagChanged(cmd, "pass") {
 		h.Pass = registryOpts.pass
 	} else if registryOpts.passStdin {
-		pass, err := io.ReadAll(os.Stdin)
+		pass, err := io.ReadAll(cmd.InOrStdin())
 		if err != nil {
 			return fmt.Errorf("failed to read password from stdin: %w", err)
 		}
@@ -259,18 +262,19 @@ func (registryOpts *registryCmd) runRegistryLogin(cmd *cobra.Command, args []str
 		return err
 	}
 
-	r, err := ref.NewHost(args[0])
-	if err != nil {
-		return err
-	}
-	rc := registryOpts.rootOpts.newRegClient()
-	_, err = rc.Ping(ctx, r)
-	if err != nil {
-		log.WithFields(logrus.Fields{
-			"err": err,
-		}).Warn("Failed to ping registry with credentials")
+	if !registryOpts.skipCheck {
+		r, err := ref.NewHost(args[0])
+		if err != nil {
+			return err
+		}
+		rc := registryOpts.rootOpts.newRegClient()
+		_, err = rc.Ping(ctx, r)
+		if err != nil {
+			log.WithFields(logrus.Fields{
+				"err": err,
+			}).Warn("Failed to ping registry with credentials")
+		}
 	}
-
 	log.WithFields(logrus.Fields{
 		"registry": args[0],
 	}).Info("Credentials set")
@@ -401,16 +405,18 @@ func (registryOpts *registryCmd) runRegistrySet(cmd *cobra.Command, args []strin
 		return err
 	}
 
-	r, err := ref.NewHost(args[0])
-	if err != nil {
-		return err
-	}
-	rc := registryOpts.rootOpts.newRegClient()
-	_, err = rc.Ping(ctx, r)
-	if err != nil {
-		log.WithFields(logrus.Fields{
-			"err": err,
-		}).Warn("Failed to ping registry with settings")
+	if !registryOpts.skipCheck {
+		r, err := ref.NewHost(args[0])
+		if err != nil {
+			return err
+		}
+		rc := registryOpts.rootOpts.newRegClient()
+		_, err = rc.Ping(ctx, r)
+		if err != nil {
+			log.WithFields(logrus.Fields{
+				"err": err,
+			}).Warn("Failed to ping registry with settings")
+		}
 	}
 
 	log.WithFields(logrus.Fields{


Test patch:
diff --git a/cmd/regctl/registry_test.go b/cmd/regctl/registry_test.go
new file mode 100644
index 0000000..d402541
--- /dev/null
+++ b/cmd/regctl/registry_test.go
@@ -0,0 +1,146 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/olareg/olareg"
+	oConfig "github.com/olareg/olareg/config"
+)
+
+func TestRegistry(t *testing.T) {
+	// t.Parallel() // this is not parallel due to environment variable settings
+	regHandler := olareg.New(oConfig.Config{
+		Storage: oConfig.ConfigStorage{
+			StoreType: oConfig.StoreMem,
+			RootDir:   "./testdata",
+		},
+	})
+	tsGood := httptest.NewServer(regHandler)
+	tsGoodURL, _ := url.Parse(tsGood.URL)
+	tsGoodHost := tsGoodURL.Host
+	tsBad := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Errorf("request was made to the bad host")
+		w.WriteHeader(http.StatusForbidden)
+	}))
+	tsBadURL, _ := url.Parse(tsBad.URL)
+	tsBadHost := tsBadURL.Host
+	tempDir := t.TempDir()
+	t.Setenv(ConfigEnv, filepath.Join(tempDir, "config.json"))
+	tt := []struct {
+		name        string
+		args        []string
+		expectErr   error
+		expectOut   string
+		outContains bool
+	}{
+		// set a good host
+		{
+			name:        "set no TLS",
+			args:        []string{"registry", "set", tsGoodHost, "--tls", "disabled"},
+			expectOut:   "",
+			outContains: false,
+		},
+		// set a bad host but skip the check
+		{
+			name:        "set without check",
+			args:        []string{"registry", "set", tsBadHost, "--tls", "disabled", "--skip-check"},
+			expectOut:   "",
+			outContains: false,
+		},
+		// query the good host
+		{
+			name:        "query good host",
+			args:        []string{"registry", "config", tsGoodHost},
+			expectOut:   `"tls": "disabled",`,
+			outContains: true,
+		},
+		// query the bad host
+		{
+			name:        "query bad host",
+			args:        []string{"registry", "config", tsBadHost},
+			expectOut:   `"tls": "disabled",`,
+			outContains: true,
+		},
+		// login to good and bad hosts
+		{
+			name:        "login good host",
+			args:        []string{"registry", "login", tsGoodHost, "-u", "testgooduser", "-p", "testpass"},
+			expectOut:   "",
+			outContains: false,
+		},
+		{
+			name:        "login bad host",
+			args:        []string{"registry", "login", tsBadHost, "-u", "testbaduser", "-p", "testpass", "--skip-check"},
+			expectOut:   "",
+			outContains: false,
+		},
+		// query good and bad to ensure user is set
+		{
+			name:        "query good host",
+			args:        []string{"registry", "config", tsGoodHost},
+			expectOut:   `"user": "testgooduser",`,
+			outContains: true,
+		},
+		{
+			name:        "query bad host",
+			args:        []string{"registry", "config", tsBadHost},
+			expectOut:   `"user": "testbaduser",`,
+			outContains: true,
+		},
+		// logout
+		{
+			name:        "logout good host",
+			args:        []string{"registry", "logout", tsGoodHost},
+			expectOut:   "",
+			outContains: false,
+		},
+		{
+			name:        "logout bad host",
+			args:        []string{"registry", "logout", tsBadHost},
+			expectOut:   "",
+			outContains: false,
+		},
+		// verify logout
+		{
+			name: "check logout on good host",
+			args: []string{"registry", "config", tsGoodHost},
+			expectOut: fmt.Sprintf(`"hostname": "%s",
+  "credHost": "",`, tsGoodHost),
+			outContains: true,
+		},
+		{
+			name: "check logout on bad host",
+			args: []string{"registry", "config", tsBadHost},
+			expectOut: fmt.Sprintf(`"hostname": "%s",
+  "credHost": "",`, tsBadHost),
+			outContains: true,
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			out, err := cobraTest(t, nil, tc.args...)
+			if tc.expectErr != nil {
+				if err == nil {
+					t.Errorf("did not receive expected error: %v", tc.expectErr)
+				} else if !errors.Is(err, tc.expectErr) && err.Error() != tc.expectErr.Error() {
+					t.Errorf("unexpected error, received %v, expected %v", err, tc.expectErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Errorf("returned unexpected error: %v", err)
+				return
+			}
+			if (!tc.outContains && out != tc.expectOut) || (tc.outContains && !strings.Contains(out, tc.expectOut)) {
+				t.Errorf("unexpected output, expected %s, received %s", tc.expectOut, out)
+			}
+		})
+	}
+}