Repository: regclient/regclient (go)

Issue:
[Feature] Return a non-zero exit code if login fails
<!--
Note: Please search to see if an issue already exists for the bug you encountered.
-->

### Current Behavior

Calling `regctl registry login` with invalid credentials returns a zero exit code. When used in GitHub Actions, the step passes despite the login failure.

```
$ regctl registry login ghcr.io --user "<username>" --pass "<invalid-password>"
time="2024-05-25T06:01:28Z" level=warning msg="Changing login user for registry" host=docker.io new=<username> orig=githubactions
time="2024-05-25T06:01:28Z" level=warning msg="Changing login password for registry" host=docker.io
time="2024-05-25T06:01:28Z" level=warning msg="Failed to ping registry with credentials" err="failed to ping registry ghcr.io: unauthorized"
```

### Expected Behavior

Return a non-zero exit code so other commands are halted.

### Example Solution

<!--
Example: steps to result in the desired behavior:
1. In this environment...
2. With this config...
3. Run '...'
4. Desired result...
-->

### Version

0.6.1

### Environment

<!--
Please include:
- How you are running the tool: downloaded binary or from within a container
- Your platform: Linux, Windows, or Mac, x86 or ARM.
- Your registry: Docker Hub, ECR, GCR, ACR, Harbor, registry:2, etc.
-->

- Running as binary or container:
- Host platform:
- Registry description:

### Anything else

<!--
Links? References? Anything that will give us more context about the issue that you are encountering!
-->
https://github.com/regclient/regclient/blob/105aee14f726fd3348be77bba5c118cbce40aea9/cmd/regctl/registry.go#L312-L316

Pull request:
Feat: Return non-zero on a failed registry ping
<!--

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 #750
<!-- If this is a bug fix, include "fixes #xxxx", or "closes #xxxx" -->

### Describe the change

On a `regctl registry set` or `regctl registry login`, return a non-zero if the registry ping fails. This is skipped when `--skip-check` is passed.
<!-- 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

Perform `regctl registry set` to a non-existent registry, or `regctl registry login` with an invalid credential.
<!-- Include steps that can be taken to verify the change -->

### Changelog text

- Breaking: `regctl registry set` and `regctl registry login` will return a non-zero if the ping fails.
<!-- 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 a1fad23..79aaf1e 100644
--- a/cmd/regctl/registry.go
+++ b/cmd/regctl/registry.go
@@ -301,7 +301,6 @@ func (registryOpts *registryCmd) runRegistryLogin(cmd *cobra.Command, args []str
 	if err != nil {
 		return err
 	}
-
 	if !registryOpts.skipCheck {
 		r, err := ref.NewHost(args[0])
 		if err != nil {
@@ -310,9 +309,8 @@ func (registryOpts *registryCmd) runRegistryLogin(cmd *cobra.Command, args []str
 		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.Warn("Failed to ping registry, credentials were still stored")
+			return err
 		}
 	}
 	log.WithFields(logrus.Fields{
@@ -453,9 +451,8 @@ func (registryOpts *registryCmd) runRegistrySet(cmd *cobra.Command, args []strin
 		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.Warn("Failed to ping registry, configuration still updated")
+			return err
 		}
 	}
 


Test patch:
diff --git a/cmd/regctl/registry_test.go b/cmd/regctl/registry_test.go
index 369c108..229f068 100644
--- a/cmd/regctl/registry_test.go
+++ b/cmd/regctl/registry_test.go
@@ -11,6 +11,8 @@ import (
 
 	"github.com/olareg/olareg"
 	oConfig "github.com/olareg/olareg/config"
+
+	"github.com/regclient/regclient/types/errs"
 )
 
 func TestRegistry(t *testing.T) {
@@ -30,9 +32,15 @@ func TestRegistry(t *testing.T) {
 	}))
 	tsBadURL, _ := url.Parse(tsBad.URL)
 	tsBadHost := tsBadURL.Host
+	tsUnauth := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+	}))
+	tsUnauthURL, _ := url.Parse(tsUnauth.URL)
+	tsUnauthHost := tsUnauthURL.Host
 	t.Cleanup(func() {
 		tsGood.Close()
 		tsBad.Close()
+		tsUnauth.Close()
 		_ = regHandler.Close()
 	})
 	tempDir := t.TempDir()
@@ -44,53 +52,72 @@ func TestRegistry(t *testing.T) {
 		expectOut   string
 		outContains bool
 	}{
-		// set a good host
+		// disable tls
 		{
 			name:        "set no TLS",
 			args:        []string{"registry", "set", tsGoodHost, "--tls", "disabled"},
 			expectOut:   "",
 			outContains: false,
 		},
-		// set a bad host but skip the check
+		{
+			name:      "set no TLS on unauth host",
+			args:      []string{"registry", "set", tsUnauthHost, "--tls", "disabled"},
+			expectErr: errs.ErrHTTPUnauthorized,
+		},
 		{
 			name:        "set without check",
 			args:        []string{"registry", "set", tsBadHost, "--tls", "disabled", "--skip-check"},
 			expectOut:   "",
 			outContains: false,
 		},
-		// query the good host
+		// query the config change
 		{
 			name:        "query good host",
 			args:        []string{"registry", "config", tsGoodHost},
 			expectOut:   `"tls": "disabled",`,
 			outContains: true,
 		},
-		// query the bad host
+		{
+			name:        "query unauth host",
+			args:        []string{"registry", "config", tsUnauthHost},
+			expectOut:   `"tls": "disabled",`,
+			outContains: true,
+		},
 		{
 			name:        "query bad host",
 			args:        []string{"registry", "config", tsBadHost},
 			expectOut:   `"tls": "disabled",`,
 			outContains: true,
 		},
-		// login to good and bad hosts
+		// login
 		{
 			name:        "login good host",
 			args:        []string{"registry", "login", tsGoodHost, "-u", "testgooduser", "-p", "testpass"},
 			expectOut:   "",
 			outContains: false,
 		},
+		{
+			name:      "login unauth host",
+			args:      []string{"registry", "login", tsUnauthHost, "-u", "testunauthuser", "-p", "testpass"},
+			expectErr: errs.ErrHTTPUnauthorized,
+		},
 		{
 			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
+		// query for user
 		{
 			name:      "query good host",
 			args:      []string{"registry", "config", tsGoodHost, "--format", "{{.User}}"},
 			expectOut: `testgooduser`,
 		},
+		{
+			name:      "query unauth host",
+			args:      []string{"registry", "config", tsUnauthHost, "--format", "{{.User}}"},
+			expectOut: `testunauthuser`,
+		},
 		{
 			name:      "query bad host",
 			args:      []string{"registry", "config", tsBadHost, "--format", "{{.User}}"},
@@ -103,6 +130,12 @@ func TestRegistry(t *testing.T) {
 			expectOut:   "",
 			outContains: false,
 		},
+		{
+			name:        "logout unauth host",
+			args:        []string{"registry", "logout", tsUnauthHost},
+			expectOut:   "",
+			outContains: false,
+		},
 		{
 			name:        "logout bad host",
 			args:        []string{"registry", "logout", tsBadHost},
@@ -115,6 +148,11 @@ func TestRegistry(t *testing.T) {
 			args:      []string{"registry", "config", tsGoodHost, "--format", "{{.User}}"},
 			expectOut: ``,
 		},
+		{
+			name:      "check logout on unauth host",
+			args:      []string{"registry", "config", tsUnauthHost, "--format", "{{.User}}"},
+			expectOut: ``,
+		},
 		{
 			name:      "check logout on bad host",
 			args:      []string{"registry", "config", tsBadHost, "--format", "{{.User}}"},