fix(registry): Answer 400 when the endpoint health check cannot reach the URL

POST /registries with url "ftp://example.invalid" or "not-a-url" returned 500.
lib.ValidateURL does run on this path and accepts both — ftp is in its default
scheme set, and a scheme-less string is normalised to http — so the request
reached the health check and the transport error came back untouched:

  Get "ftp://example.invalid/api/version": unsupported protocol scheme "ftp"
  Get "http://not-a-url/api/version": dial tcp: lookup not-a-url ... no such host

The endpoint comes from the request body, so failing to reach it is a bad
request. Wrap the health-check error accordingly, keeping the underlying
reason in the message. The same error path backs POST /registries/ping and
PUT /registries/{id}, which answered 500 for the same input.

The scheme allowlist is deliberately left alone: storage-backed registry types
use s3:// and sftp:// URLs on this path.

Signed-off-by: Prasanth Baskar <prasanth@8gears.com>

Author: bupd

src/controller/registry/controller.go

@@ -176,7 +176,12 @@ func (c *controller) IsHealthy(ctx context.Context, registry *model.Registry) (b
 	}
 	status, err := adapter.HealthCheck()
 	if err != nil {
-		return false, err
+		// The endpoint comes from the request, so a transport failure is a
+		// problem with what the caller supplied, not an internal one. Without
+		// this the raw error ("unsupported protocol scheme \"ftp\"", "no such
+		// host") reached the client as a 500.
+		return false, errors.New(nil).WithCode(errors.BadRequestCode).
+			WithMessagef("failed to check the health of the registry %s: %v", registry.URL, err)
 	}
 	return status == model.Healthy, nil
 }

src/controller/registry/controller_test.go

@@ -16,12 +16,14 @@ package registry
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/suite"
 
 	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/lib/config"
+	"github.com/goharbor/harbor/src/lib/errors"
 	_ "github.com/goharbor/harbor/src/pkg/config/inmemory"
 	"github.com/goharbor/harbor/src/pkg/reg/model"
 	"github.com/goharbor/harbor/src/testing/mock"
@@ -151,6 +153,44 @@ func (r *registryTestSuite) TestValidate() {
 	r.adapter.AssertExpectations(r.T())
 }
 
+// The endpoint comes from the request body, so a transport failure while
+// health-checking it must be reported as a bad request, not a 500.
+func (r *registryTestSuite) TestValidateHealthCheckTransportError() {
+	for _, tc := range []struct {
+		name string
+		url  string
+		err  error
+	}{
+		{
+			name: "unsupported scheme",
+			url:  "ftp://example.invalid",
+			err:  fmt.Errorf(`Get "ftp://example.invalid/api/version": unsupported protocol scheme "ftp"`),
+		},
+		{
+			name: "unresolvable host",
+			url:  "not-a-url",
+			err:  fmt.Errorf(`Get "http://not-a-url/api/version": dial tcp: lookup not-a-url: no such host`),
+		},
+	} {
+		r.Run(tc.name, func() {
+			r.SetupTest()
+			mock.OnAnything(r.regMgr, "CreateAdapter").Return(r.adapter, nil)
+			mock.OnAnything(r.adapter, "HealthCheck").Return("", tc.err)
+
+			err := r.ctl.validate(context.Background(), &model.Registry{
+				Name: "endpoint01",
+				URL:  tc.url,
+			})
+
+			r.Require().NotNil(err)
+			r.True(errors.IsErr(err, errors.BadRequestCode), "want a bad request error, got %v", err)
+			r.Contains(err.Error(), tc.err.Error())
+			r.regMgr.AssertExpectations(r.T())
+			r.adapter.AssertExpectations(r.T())
+		})
+	}
+}
+
 func (r *registryTestSuite) TestDelete() {
 	// referenced by replication policy
 	mock.OnAnything(r.repMgr, "Count").Return(int64(1), nil)

src/server/v2.0/handler/registry_test.go

@@ -20,6 +20,7 @@ import (
 	testifymock "github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/pkg/reg/model"
 	"github.com/goharbor/harbor/src/server/v2.0/models"
 	"github.com/goharbor/harbor/src/server/v2.0/restapi"
@@ -225,6 +226,24 @@ func (suite *RegistryTestSuite) TestUpdateRegistryStorageSchemeURLAccepted() {
 	}
 }
 
+// A URL the health check cannot reach is a problem with the request body, so
+// the ping must answer 400 rather than surfacing the transport error as a 500.
+func (suite *RegistryTestSuite) TestPingRegistryUnreachableURLRejected() {
+	suite.Security.On("IsAuthenticated").Return(true).Once()
+	suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once()
+
+	unreachable := errors.New(nil).WithCode(errors.BadRequestCode).
+		WithMessage(`failed to check the health of the registry ftp://example.invalid: Get "ftp://example.invalid/api/version": unsupported protocol scheme "ftp"`)
+	suite.regCtl.On("IsHealthy", mock.Anything, mock.Anything).Return(false, unreachable).Once()
+
+	res, err := suite.PostJSON("/registries/ping", &models.RegistryPing{
+		Type: suite.ptrStr("harbor"),
+		URL:  suite.ptrStr("ftp://example.invalid"),
+	})
+	suite.NoError(err)
+	suite.Equal(400, res.StatusCode)
+}
+
 func TestRegistryTestSuite(t *testing.T) {
 	suite.Run(t, &RegistryTestSuite{})
 }