Improve error message under 401 condition with Azure ACR (radius-proj…

…ect#6974)

# Description

Improve error message returned when publishing to a private ACR

## Type of change

<!--

Please select **one** of the following options that describes your
change and delete the others. Clearly identifying the type of change you
are making will help us review your PR faster, and is used in authoring
release notes.

If you are making a bug fix or functionality change to Radius and do not
have an associated issue link please create one now.

-->

- This pull request is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
(issue link optional).

<!--

Please update the following to link the associated issue. This is
required for some kinds of changes (see above).

-->

Fixes: radius-project#6902 radius-project#6297

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

copilot:all

---------

Signed-off-by: Garry Taylor <[email protected]>
Co-authored-by: Ryan Nowak <[email protected]>
Signed-off-by: willdavsmith <[email protected]>

pkg/cli/clierrors/types.go

@@ -16,6 +16,8 @@ limitations under the License.
 
 package clierrors
 
+import "strings"
+
 // FriendlyError defines an interface for errors that should be gracefully handled by the CLI and
 // display a friendly error message to the user.
 type FriendlyError interface {
@@ -42,7 +44,7 @@ func (e *ErrorMessage) Error() string {
 		return e.Message
 	}
 
-	return e.Message + " Cause: " + e.Cause.Error() + "."
+	return e.Message + " Cause: " + strings.TrimSpace(e.Cause.Error()) + "."
 }
 
 // IsFriendlyError returns true for ErrorMessage. These errors are always handled gracefully by the CLI.

pkg/cli/cmd/bicep/publish/publish.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -30,6 +31,8 @@ import (
 	"github.com/radius-project/radius/pkg/cli/framework"
 	"github.com/radius-project/radius/pkg/cli/output"
 
+	"net/http"
+
 	"github.com/opencontainers/go-digest"
 	specs "github.com/opencontainers/image-spec/specs-go"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -39,6 +42,7 @@ import (
 	"oras.land/oras-go/v2/registry"
 	"oras.land/oras-go/v2/registry/remote"
 	"oras.land/oras-go/v2/registry/remote/auth"
+	"oras.land/oras-go/v2/registry/remote/errcode"
 	"oras.land/oras-go/v2/registry/remote/retry"
 )
 
@@ -167,14 +171,40 @@ func (r *Runner) Run(ctx context.Context) error {
 	r.Destination = dest
 
 	err = r.publish(ctx)
-	if err != nil {
-		return clierrors.MessageWithCause(err, "Failed to publish Bicep file %q to %q.", r.File, r.Target)
+	var httpErr *errcode.ErrorResponse
+	if errors.As(err, &httpErr) {
+		message := fmt.Sprintf("Failed to publish Bicep file %q to %q", r.File, r.Target)
+		return handleErrorResponse(httpErr, message)
+	} else if err != nil {
+		return clierrors.MessageWithCause(err, "Failed to publish Bicep file %q to %q", r.File, r.Target)
 	}
 
 	r.Output.LogInfo("Successfully published Bicep file %q to %q", r.File, r.Target)
 	return nil
 }
 
+func handleErrorResponse(httpErr *errcode.ErrorResponse, message string) error {
+	acrInfo := ""
+	if strings.Contains(httpErr.URL.Host, "azurecr.io") {
+		acrInfo = "For more details visit: https://learn.microsoft.com/en-us/azure/container-registry/container-registry-authentication?tabs=azure-cli"
+	}
+
+	switch httpErr.StatusCode {
+	case http.StatusUnauthorized:
+		if acrInfo == "" {
+			return clierrors.MessageWithCause(httpErr, "%s\nUnauthorized: Please login to %q", message, httpErr.URL.Host)
+		} else {
+			return clierrors.MessageWithCause(httpErr, "%s\nUnauthorized: Please login to %q\n%s", message, httpErr.URL.Host, acrInfo)
+		}
+	case http.StatusForbidden:
+		return clierrors.MessageWithCause(httpErr, "%s\nForbidden: You don't have permission to push to %q", message, httpErr.URL.Host)
+	case http.StatusNotFound:
+		return clierrors.MessageWithCause(httpErr, "%s\nNot Found: Unable to find registry %q", message, httpErr.URL.Host)
+	default:
+		return clierrors.MessageWithCause(httpErr, "%s\nSomething went wrong", message)
+	}
+}
+
 func (r *Runner) publish(ctx context.Context) error {
 	// Prepare Source
 	src, err := r.prepareSource(ctx)

pkg/cli/cmd/bicep/publish/publish_test.go

@@ -19,6 +19,9 @@ package bicep
 import (
 	"context"
 	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/url"
 	"reflect"
 	"testing"
 
@@ -32,6 +35,7 @@ import (
 	"oras.land/oras-go/v2/content/memory"
 	"oras.land/oras-go/v2/registry"
 	"oras.land/oras-go/v2/registry/remote"
+	"oras.land/oras-go/v2/registry/remote/errcode"
 )
 
 func TestRunner_extractDestination(t *testing.T) {
@@ -313,3 +317,48 @@ func TestRunner_Validate(t *testing.T) {
 	}
 	radcli.SharedValidateValidation(t, NewCommand, tests)
 }
+
+func getError(registerUrl string, statusCode int) *errcode.ErrorResponse  {
+
+	err := &errcode.ErrorResponse{
+		URL:        &url.URL{Host: registerUrl},
+		StatusCode: statusCode,
+	}
+
+	return err
+}
+
+func TestHandleErrorResponse(t *testing.T) {
+
+	httpErrA := getError("myregistry.azurecr.io", http.StatusUnauthorized)
+	httpErrB := getError("otherregistry.gcr.io", http.StatusUnauthorized)
+	httpErrC := getError("myregistry.azurecr.io", http.StatusForbidden)
+	httpErrD := getError("myregistry.azurecr.io", http.StatusNotFound)
+	httpErrE := getError("myregistry.azurecr.io", http.StatusInternalServerError)
+
+	testCases := []struct {
+		httpErr       *errcode.ErrorResponse
+		message       string
+		expectedError string
+	}{
+		{httpErrA, "Unauthorized - Includes Azure login info", fmt.Sprintf("Unauthorized: Please login to %q\nFor more details visit: https://learn.microsoft.com/en-us/azure/container-registry/container-registry-authentication?tabs=azure-cli Cause: \"//myregistry.azurecr.io\": response status code 401: Unauthorized.", httpErrA.URL.Host)},
+		{httpErrB, "Standard unauthorized message", fmt.Sprintf("Unauthorized: Please login to %q Cause: %q: response status code 401: Unauthorized.", httpErrB.URL.Host, "//"+httpErrB.URL.Host)},
+		{httpErrC, "Forbidden error", fmt.Sprintf("Forbidden: You don't have permission to push to %q Cause: %q: response status code 403: Forbidden.", httpErrC.URL.Host, "//"+httpErrC.URL.Host)},
+		{httpErrD, "Not found error", fmt.Sprintf("Not Found: Unable to find registry %q Cause: %q: response status code 404: Not Found.", httpErrD.URL.Host, "//"+httpErrD.URL.Host)},
+		{httpErrE, "Internal server error", fmt.Sprintf("Something went wrong Cause: %q: response status code 500: Internal Server Error.", "//"+httpErrE.URL.Host)},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.message, func(t *testing.T) {
+			httpErr := &errcode.ErrorResponse{
+				Method:     tc.httpErr.Method,
+				URL:        tc.httpErr.URL,
+				StatusCode: tc.httpErr.StatusCode,
+			}
+
+			result := handleErrorResponse(httpErr, tc.message)
+			expected := fmt.Sprintf("%s\n%s", tc.message, tc.expectedError)
+			require.Equal(t, expected, result.Error())
+		})
+	}
+}