Fix and improve acorn login validation check (#922)

- Add --skip-checks as an escape hatch for trying to authenticate during `acorn login
- Rename `acorn install --checks` to `acorn install --skip-checks` for symmetry
- Fix the check itself so that authing against quay.io works (by removing the pull/push scopes from the authentication call)
- Move the check server side

Signed-off-by: Joshua Silverio <[email protected]>
Signed-off-by: Craig Jellick <[email protected]>
Co-authored-by: Craig Jellick <[email protected]>

docs/docs/100-reference/01-command-line/acorn_credential_login.md

@@ -22,6 +22,7 @@ acorn login ghcr.io
   -h, --help              help for login
   -p, --password string   Password
       --password-stdin    Take the password from stdin
+      --skip-checks       Bypass login validation checks
   -u, --username string   Username
 ```
 

docs/docs/100-reference/01-command-line/acorn_install.md

@@ -23,7 +23,6 @@ acorn install
       --acorn-dns-endpoint string             The URL to access the Acorn DNS service
       --api-server-replicas int               acorn-api deployment replica count
       --auto-upgrade-interval string          For apps configured with automatic upgrades enabled, the interval at which to check for new versions. Upgrade intervals configured at the application level cannot be smaller than this. (default '5m' - 5 minutes)
-      --checks                                Disable preflight checks with --checks=false
       --cluster-domain strings                The externally addressable cluster domain (default .on-acorn.io)
       --controller-replicas int               acorn-controller deployment replica count
       --default-publish-mode string           If no publish mode is set default to this value (default user)
@@ -37,6 +36,7 @@ acorn install
   -o, --output string                         Output manifests instead of applying them (json, yaml)
       --pod-security-enforce-profile string   The name of the PodSecurity profile to set (default baseline)
       --set-pod-security-enforce-profile      Set the PodSecurity profile on created namespaces (default true)
+      --skip-checks                           Bypass installation checks
 ```
 
 ### Options inherited from parent commands

docs/docs/100-reference/01-command-line/acorn_login.md

@@ -22,6 +22,7 @@ acorn login ghcr.io
   -h, --help              help for login
   -p, --password string   Password
       --password-stdin    Take the password from stdin
+      --skip-checks       Bypass login validation checks
   -u, --username string   Username
 ```
 

integration/client/credentials/credentials_test.go

@@ -28,7 +28,7 @@ func TestCredentialCreate(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	cred, err := c.CredentialCreate(ctx, reg, "user", "pass")
+	cred, err := c.CredentialCreate(ctx, reg, "user", "pass", false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -38,7 +38,7 @@ func TestCredentialCreate(t *testing.T) {
 	assert.Equal(t, "user", cred.Username)
 	assert.Nil(t, cred.Password)
 
-	cred1, err := c.CredentialCreate(ctx, reg1, "user2", "pass2")
+	cred1, err := c.CredentialCreate(ctx, reg1, "user2", "pass2", false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -74,12 +74,12 @@ func TestCredentialList(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	cred1, err := c.CredentialCreate(ctx, reg, "user", "pass")
+	cred1, err := c.CredentialCreate(ctx, reg, "user", "pass", false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	cred2, err := c.CredentialCreate(ctx, reg1, "user2", "pass2")
+	cred2, err := c.CredentialCreate(ctx, reg1, "user2", "pass2", false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -115,12 +115,12 @@ func TestCredentialGet(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err = c.CredentialCreate(ctx, reg, "user", "pass")
+	_, err = c.CredentialCreate(ctx, reg, "user", "pass", false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	cred1, err := c.CredentialCreate(ctx, reg1, "user2", "pass2")
+	cred1, err := c.CredentialCreate(ctx, reg1, "user2", "pass2", false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -151,17 +151,17 @@ func TestCredentialUpdate(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err = c.CredentialCreate(ctx, reg, "user", "pass")
+	_, err = c.CredentialCreate(ctx, reg, "user", "pass", false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	_, err = c.CredentialCreate(ctx, reg1, "user2", "pass2")
+	_, err = c.CredentialCreate(ctx, reg1, "user2", "pass2", false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	cred1New, err := c.CredentialUpdate(ctx, reg1, "user3", "pass3")
+	cred1New, err := c.CredentialUpdate(ctx, reg1, "user3", "pass3", false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -194,12 +194,12 @@ func TestCredentialDelete(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err = c.CredentialCreate(ctx, reg, "user", "pass")
+	_, err = c.CredentialCreate(ctx, reg, "user", "pass", false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	_, err = c.CredentialCreate(ctx, reg1, "user2", "pass2")
+	_, err = c.CredentialCreate(ctx, reg1, "user2", "pass2", false)
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/apis/api.acorn.io/v1/types.go

@@ -242,6 +242,7 @@ type Credential struct {
 	ServerAddress string  `json:"serverAddress,omitempty"`
 	Username      string  `json:"username,omitempty"`
 	Password      *string `json:"password,omitempty"`
+	SkipChecks    bool    `json:"skipChecks,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/cli/credential_login.go

@@ -30,6 +30,7 @@ acorn login ghcr.io`,
 }
 
 type CredentialLogin struct {
+	SkipChecks    bool   `usage:"Bypass login validation checks"`
 	PasswordStdin bool   `usage:"Take the password from stdin"`
 	Password      string `usage:"Password" short:"p"`
 	Username      string `usage:"Username" short:"u"`
@@ -71,7 +72,7 @@ func (a *CredentialLogin) Run(cmd *cobra.Command, args []string) error {
 
 	existing, err := client.CredentialGet(cmd.Context(), args[0])
 	if apierror.IsNotFound(err) {
-		cred, err := client.CredentialCreate(cmd.Context(), args[0], a.Username, a.Password)
+		cred, err := client.CredentialCreate(cmd.Context(), args[0], a.Username, a.Password, a.SkipChecks)
 		if err != nil {
 			return err
 		}
@@ -82,7 +83,7 @@ func (a *CredentialLogin) Run(cmd *cobra.Command, args []string) error {
 
 	existing.Username = a.Username
 	existing.Password = &a.Password
-	cred, err := client.CredentialUpdate(cmd.Context(), args[0], a.Username, a.Password)
+	cred, err := client.CredentialUpdate(cmd.Context(), args[0], a.Username, a.Password, a.SkipChecks)
 	if err != nil {
 		return err
 	}

pkg/cli/install.go

@@ -20,7 +20,7 @@ acorn install`,
 }
 
 type Install struct {
-	Checks *bool `usage:"Disable preflight checks with --checks=false"`
+	SkipChecks bool `usage:"Bypass installation checks"`
 
 	Image  string `usage:"Override the default image used for the deployment"`
 	Output string `usage:"Output manifests instead of applying them (json, yaml)" short:"o"`
@@ -38,7 +38,7 @@ func (i *Install) Run(cmd *cobra.Command, args []string) error {
 	}
 
 	return install.Install(cmd.Context(), image, &install.Options{
-		Checks:             i.Checks,
+		SkipChecks:         i.SkipChecks,
 		OutputFormat:       i.Output,
 		Config:             i.Config,
 		APIServerReplicas:  i.APIServerReplicas,

pkg/client/client.go

@@ -194,10 +194,10 @@ type Client interface {
 	AppLog(ctx context.Context, name string, opts *LogOptions) (<-chan apiv1.LogMessage, error)
 	AppConfirmUpgrade(ctx context.Context, name string) error
 
-	CredentialCreate(ctx context.Context, serverAddress, username, password string) (*apiv1.Credential, error)
+	CredentialCreate(ctx context.Context, serverAddress, username, password string, skipChecks bool) (*apiv1.Credential, error)
 	CredentialList(ctx context.Context) ([]apiv1.Credential, error)
 	CredentialGet(ctx context.Context, serverAddress string) (*apiv1.Credential, error)
-	CredentialUpdate(ctx context.Context, serverAddress, username, password string) (*apiv1.Credential, error)
+	CredentialUpdate(ctx context.Context, serverAddress, username, password string, skipChecks bool) (*apiv1.Credential, error)
 	CredentialDelete(ctx context.Context, serverAddress string) (*apiv1.Credential, error)
 
 	SecretCreate(ctx context.Context, name, secretType string, data map[string][]byte) (*apiv1.Secret, error)

pkg/client/credentials.go

@@ -2,23 +2,15 @@ package client
 
 import (
 	"context"
-	"net/http"
 	"sort"
 
 	apiv1 "github.com/acorn-io/acorn/pkg/apis/api.acorn.io/v1"
-	"github.com/google/go-containerregistry/pkg/authn"
-	"github.com/google/go-containerregistry/pkg/name"
-	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func (c *client) CredentialCreate(ctx context.Context, serverAddress, username, password string) (*apiv1.Credential, error) {
-	if err := credentialValidate(ctx, username, password, serverAddress); err != nil {
-		return nil, err
-	}
-
+func (c *client) CredentialCreate(ctx context.Context, serverAddress, username, password string, skipChecks bool) (*apiv1.Credential, error) {
 	credential := &apiv1.Credential{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      serverAddress,
@@ -27,6 +19,7 @@ func (c *client) CredentialCreate(ctx context.Context, serverAddress, username,
 		ServerAddress: serverAddress,
 		Username:      username,
 		Password:      &password,
+		SkipChecks:    skipChecks,
 	}
 	return credential, c.Client.Create(ctx, credential)
 }
@@ -39,11 +32,7 @@ func (c *client) CredentialGet(ctx context.Context, serverAddress string) (*apiv
 	}, credential)
 }
 
-func (c *client) CredentialUpdate(ctx context.Context, serverAddress, username, password string) (*apiv1.Credential, error) {
-	if err := credentialValidate(ctx, username, password, serverAddress); err != nil {
-		return nil, err
-	}
-
+func (c *client) CredentialUpdate(ctx context.Context, serverAddress, username, password string, skipChecks bool) (*apiv1.Credential, error) {
 	credential := &apiv1.Credential{}
 	err := c.Client.Get(ctx, kclient.ObjectKey{
 		Name:      serverAddress,
@@ -55,6 +44,7 @@ func (c *client) CredentialUpdate(ctx context.Context, serverAddress, username,
 
 	credential.Username = username
 	credential.Password = &password
+	credential.SkipChecks = skipChecks
 	return credential, c.Client.Update(ctx, credential)
 }
 
@@ -96,23 +86,3 @@ func (c *client) CredentialDelete(ctx context.Context, serverAddress string) (*a
 	}
 	return credential, err
 }
-
-// credentialValidate takes a username, password and serverAddress string to validate
-// whether their combination is valid and will succeed login for pushes/pulls.
-func credentialValidate(ctx context.Context, username, password, serverAddress string) error {
-	// Build a registry struct for the host
-	reg, err := name.NewRegistry(serverAddress)
-	if err != nil {
-		return err
-	}
-
-	// Build a new transport for the registry which validates authentication
-	scopes := []string{transport.PullScope, transport.PushScope}
-	auth := &authn.Basic{Username: username, Password: password}
-	_, err = transport.NewWithContext(ctx, reg, auth, http.DefaultTransport, scopes)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}

pkg/client/ignore.go

@@ -204,9 +204,9 @@ func (c IgnoreUninstalled) BuilderRegistryDialer(ctx context.Context) (func(ctx
 	})
 }
 
-func (c IgnoreUninstalled) CredentialCreate(ctx context.Context, serverAddress, username, password string) (*apiv1.Credential, error) {
+func (c IgnoreUninstalled) CredentialCreate(ctx context.Context, serverAddress, username, password string, skipChecks bool) (*apiv1.Credential, error) {
 	return promptInstall(ctx, func() (*apiv1.Credential, error) {
-		return c.client.CredentialCreate(ctx, serverAddress, username, password)
+		return c.client.CredentialCreate(ctx, serverAddress, username, password, skipChecks)
 	})
 }
 
@@ -218,8 +218,8 @@ func (c IgnoreUninstalled) CredentialGet(ctx context.Context, serverAddress stri
 	return c.client.CredentialGet(ctx, serverAddress)
 }
 
-func (c IgnoreUninstalled) CredentialUpdate(ctx context.Context, serverAddress, username, password string) (*apiv1.Credential, error) {
-	return c.client.CredentialUpdate(ctx, serverAddress, username, password)
+func (c IgnoreUninstalled) CredentialUpdate(ctx context.Context, serverAddress, username, password string, skipChecks bool) (*apiv1.Credential, error) {
+	return c.client.CredentialUpdate(ctx, serverAddress, username, password, skipChecks)
 }
 
 func (c IgnoreUninstalled) CredentialDelete(ctx context.Context, serverAddress string) (*apiv1.Credential, error) {

pkg/install/install.go

@@ -56,7 +56,7 @@ var (
 type Mode string
 
 type Options struct {
-	Checks             *bool
+	SkipChecks         bool
 	OutputFormat       string
 	APIServerReplicas  *int
 	ControllerReplicas *int
@@ -167,7 +167,7 @@ func Install(ctx context.Context, image string, opts *Options) error {
 	}
 
 	checkOpts := CheckOptions{RuntimeImage: image}
-	if opts.Checks == nil || *opts.Checks {
+	if !opts.SkipChecks {
 		s := opts.Progress.New("Running Pre-install Checks")
 		checkResults := PreInstallChecks(ctx, checkOpts)
 		if IsFailed(checkResults) {
@@ -233,16 +233,20 @@ func Install(ctx context.Context, image string, opts *Options) error {
 		return err
 	}
 
-	s = opts.Progress.New("Running Post-install Checks")
-	checkResults := PostInstallChecks(ctx, checkOpts)
-	if IsFailed(checkResults) {
-		msg := "Post-install checks failed. Use `acorn check` to debug or `acorn install --checks=false` to skip"
-		for _, result := range checkResults {
-			if !result.Passed {
-				msg += fmt.Sprintf("\n%s: %s", result.Name, result.Message)
+	if !opts.SkipChecks {
+		s = opts.Progress.New("Running Post-install Checks")
+		checkResults := PostInstallChecks(ctx, checkOpts)
+		if IsFailed(checkResults) {
+			msg := "Post-install checks failed. Use `acorn check` to debug or `acorn install --checks=false` to skip"
+			for _, result := range checkResults {
+				if !result.Passed {
+					msg += fmt.Sprintf("\n%s: %s", result.Name, result.Message)
+				}
 			}
+			s.SuccessWithWarning(msg)
+		} else {
+			s.Success()
 		}
-		s.SuccessWithWarning(msg)
 	} else {
 		s.Success()
 	}

pkg/server/registry/credentials/strategy.go

@@ -10,6 +10,7 @@ import (
 	"github.com/acorn-io/mink/pkg/strategy/translation"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/registry/rest"
 	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -37,3 +38,16 @@ func (s *Strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	cred := obj.(*apiv1.Credential)
 	cred.ServerAddress = normalizeDockerIO(cred.ServerAddress)
 }
+func (s *Strategy) Validate(ctx context.Context, obj runtime.Object) (result field.ErrorList) {
+	params := obj.(*apiv1.Credential)
+	if !params.SkipChecks {
+		if err := s.credentialValidate(ctx, params.Username, *params.Password, params.ServerAddress); err != nil {
+			result = append(result, field.Forbidden(field.NewPath("username/password"), err.Error()))
+		}
+	}
+	return result
+}
+func (s *Strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) (result field.ErrorList) {
+	params := obj.(*apiv1.Credential)
+	return s.Validate(ctx, params)
+}

pkg/server/registry/credentials/strategy_validate.go

@@ -0,0 +1,28 @@
+package credentials
+
+import (
+	"context"
+	"github.com/google/go-containerregistry/pkg/authn"
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
+	"net/http"
+)
+
+// credentialValidate takes a username, password and serverAddress string to validate
+// whether their combination is valid and will succeed login for pushes/pulls.
+func (s *Strategy) credentialValidate(ctx context.Context, username, password, serverAddress string) error {
+	// Build a registry struct for the host
+	reg, err := name.NewRegistry(serverAddress)
+	if err != nil {
+		return err
+	}
+
+	// Build a new transport for the registry which validates authentication
+	auth := &authn.Basic{Username: username, Password: password}
+	_, err = transport.NewWithContext(ctx, reg, auth, http.DefaultTransport, nil)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}