defang --provider aws shows improved error if unauthenticated (#808)

* updated tests for NewByocClient

* renamed to NewByocProvider

* more minor revisions to variable names

* edited os to t getenv in integration teest

* edited to use AccountInfo rather than region

* made error-wrapping for ErrMissingAwsCreds

* fixes missing struct field error

Author: commit111

src/pkg/cli/client/byoc/aws/byoc.go

@@ -62,12 +62,29 @@ type ByocAws struct {
 
 var _ client.Provider = (*ByocAws)(nil)
 
-func NewByocProvider(ctx context.Context, tenantId types.TenantID) *ByocAws {
+type ErrMissingAwsCreds struct {
+	err error
+}
+
+func (e ErrMissingAwsCreds) Error() string {
+	return "AWS credentials must be set (https://docs.defang.io/docs/providers/aws/#getting-started)"
+}
+
+func (e ErrMissingAwsCreds) Unwrap() error {
+	return e.err
+}
+
+func NewByocProvider(ctx context.Context, tenantId types.TenantID) (*ByocAws, error) {
 	b := &ByocAws{
 		driver: cfn.New(byoc.CdTaskPrefix, aws.Region("")), // default region
 	}
 	b.ByocBaseClient = byoc.NewByocBaseClient(ctx, tenantId, b)
-	return b
+
+	_, err := b.AccountInfo(ctx)
+	if err != nil {
+		return b, ErrMissingAwsCreds{err: err}
+	}
+	return b, nil
 }
 
 func (b *ByocAws) setUpCD(ctx context.Context, projectName string) (string, error) {

src/pkg/cli/client/byoc/aws/byoc_integration_test.go

@@ -4,6 +4,7 @@ package aws
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"testing"
 
@@ -15,7 +16,14 @@ import (
 var ctx = context.Background()
 
 func TestDeploy(t *testing.T) {
-	b := NewByocProvider(ctx, client.GrpcClient{}, "ten ant") // no domain
+	b, err := NewByocProvider(ctx, client.GrpcClient{}, "ten ant") // no domain
+	if err != nil {
+		var credErr ErrMissingAwsCreds
+		if errors.As(err, &credErr) {
+			t.Skip("skipping test; not authenticated")
+		}
+		t.Fatalf("unexpected error: %v", err)
+	}
 	b.ProjectName = "byoc_integration_test"
 
 	t.Run("multiple ingress without domain", func(t *testing.T) {
@@ -41,17 +49,24 @@ func TestDeploy(t *testing.T) {
 }
 
 func TestTail(t *testing.T) {
-	b := NewByocProvider(ctx, client.GrpcClient{}, "TestTail")
+	b, err := NewByocProvider(ctx, client.GrpcClient{}, "TestTail")
+	if err != nil {
+		var credErr ErrMissingAwsCreds
+		if errors.As(err, &credErr) {
+			t.Skip("skipping test; not authenticated")
+		}
+		t.Fatalf("unexpected error: %v", err)
+	}
 	b.ProjectName = "byoc_integration_test"
 	b.ProjectDomain = "example.com" // avoid rpc call
 
 	ss, err := b.Follow(context.Background(), &defangv1.TailRequest{})
 	if err != nil {
 		// the only acceptable error is "unauthorized"
-		if connect.CodeOf(err) != connect.CodeUnauthenticated {
-			t.Fatal(err)
+		if connect.CodeOf(err) == connect.CodeUnauthenticated {
+			t.Skip("skipping test; not authorized")
 		}
-		t.Skip("skipping test; not authorized")
+		t.Fatalf("unexpected error: %v", err)
 	}
 	defer ss.Close()
 
@@ -69,7 +84,14 @@ func TestTail(t *testing.T) {
 }
 
 func TestGetServices(t *testing.T) {
-	b := NewByocProvider(ctx, client.GrpcClient{}, "TestGetServices")
+	b, err := NewByocProvider(ctx, client.GrpcClient{}, "TestGetServices")
+	if err != nil {
+		var credErr ErrMissingAwsCreds
+		if errors.As(err, &credErr) {
+			t.Skip("skipping test; not authenticated")
+		}
+		t.Fatalf("unexpected error: %v", err)
+	}
 	b.ProjectName = "byoc_integration_test"
 
 	services, err := b.GetServices(context.Background())
@@ -78,7 +100,7 @@ func TestGetServices(t *testing.T) {
 			t.Skip("skipping test; not authorized")
 		}
 		// the only acceptable error is "unauthorized"
-		t.Fatal(err)
+		t.Fatalf("unexpected error: %v", err)
 	}
 
 	if len(services.Services) != 0 {
@@ -89,7 +111,14 @@ func TestGetServices(t *testing.T) {
 func TestPutSecret(t *testing.T) {
 	const secretName = "hello"
 
-	b := NewByocProvider(ctx, client.GrpcClient{}, "TestPutSecret")
+	b, err := NewByocProvider(ctx, client.GrpcClient{}, "TestPutSecret")
+	if err != nil {
+		var credErr ErrMissingAwsCreds
+		if errors.As(err, &credErr) {
+			t.Skip("skipping test; not authenticated")
+		}
+		t.Fatalf("unexpected error: %v", err)
+	}
 	b.ProjectName = "byoc_integration_test"
 
 	t.Run("delete non-existent", func(t *testing.T) {
@@ -141,7 +170,14 @@ func TestPutSecret(t *testing.T) {
 }
 
 func TestListSecrets(t *testing.T) {
-	b := NewByocProvider(ctx, client.GrpcClient{}, "TestListSecrets")
+	b, err := NewByocProvider(ctx, client.GrpcClient{}, "TestListSecrets")
+	if err != nil {
+		var credErr ErrMissingAwsCreds
+		if errors.As(err, &credErr) {
+			t.Skip("skipping test; not authenticated")
+		}
+		t.Fatalf("unexpected error: %v", err)
+	}
 	b.ProjectName = "byoc_integration_test2" // ensure we don't accidentally see the secrets from the other test
 
 	t.Run("list", func(t *testing.T) {

src/pkg/cli/client/byoc/aws/byoc_test.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"embed"
 	"encoding/json"
+	"errors"
 	"io"
 	"path"
 	"strings"
@@ -14,7 +15,9 @@ import (
 
 	"github.com/DefangLabs/defang/src/pkg/cli/client/byoc"
 	"github.com/DefangLabs/defang/src/pkg/cli/compose"
+	"github.com/DefangLabs/defang/src/pkg/clouds/aws"
 	"github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs"
+	"github.com/DefangLabs/defang/src/pkg/clouds/aws/ecs/cfn"
 	"github.com/DefangLabs/defang/src/pkg/types"
 	defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
 	composeTypes "github.com/compose-spec/compose-go/v2/types"
@@ -44,11 +47,17 @@ func TestDomainMultipleProjectSupport(t *testing.T) {
 		{"Project1", "tenant1", "web", port80, "web--80.project1.example.com", "web.project1.example.com", "web.project1.internal"},
 		{"Tenant2", "tenant1", "web", port80, "web--80.tenant2.example.com", "web.tenant2.example.com", "web.tenant2.internal"},
 		{"tenant1", "tenAnt1", "web", port80, "web--80.example.com", "web.example.com", "web.tenant1.internal"},
+		{"tenant1", "tenant1", "web", port80, "web--80.example.com", "web.example.com", "web.tenant1.internal"},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.ProjectName+","+string(tt.TenantID), func(t *testing.T) {
-			b := NewByocProvider(context.Background(), tt.TenantID)
+			//like calling NewByocProvider(), but without needing real AccountInfo data
+			b := &ByocAws{
+				driver: cfn.New(byoc.CdTaskPrefix, aws.Region("")), // default region
+			}
+			b.ByocBaseClient = byoc.NewByocBaseClient(context.Background(), tt.TenantID, b)
+
 			const delegateDomain = "example.com"
 
 			endpoint := b.getEndpoint(tt.Fqn, tt.ProjectName, delegateDomain, tt.Port)
@@ -161,9 +170,28 @@ func TestSubscribe(t *testing.T) {
 	}
 }
 
+func TestNewByocProvider(t *testing.T) {
+	t.Run("no aws credentials", func(t *testing.T) {
+		_, err := NewByocProvider(context.Background(), "tenant1")
+		if err != nil {
+			var credErr ErrMissingAwsCreds
+			if !errors.As(err, &credErr) {
+				t.Fatalf("NewByocProvider() failed: %v", err)
+			}
+		} else {
+			t.Fatal("NewByocProvider() failed: expected MissingAwsCreds error but didn't get one")
+		}
+	})
+}
+
 func TestGetCDImageTag(t *testing.T) {
 	ctx := context.Background()
-	b := NewByocProvider(ctx, "tenant1")
+
+	//like calling NewByocProvider(), but without needing real AccountInfo data
+	b := &ByocAws{
+		driver: cfn.New(byoc.CdTaskPrefix, aws.Region("")), // default region
+	}
+	b.ByocBaseClient = byoc.NewByocBaseClient(context.Background(), "tenant1", b)
 
 	t.Run("no project should use latest", func(t *testing.T) {
 		const expected = byoc.CdLatestImageTag

src/pkg/cli/connect.go

@@ -60,15 +60,18 @@ func NewProvider(ctx context.Context, providerID client.ProviderID, grpcClient c
 	switch providerID {
 	case client.ProviderAWS:
 		term.Info("Using AWS provider")
-		awsProvider := aws.NewByocProvider(ctx, grpcClient.TenantID)
+		awsProvider, err := aws.NewByocProvider(ctx, grpcClient.TenantID)
+		if err != nil {
+			term.Fatal(err)
+		}
 		return awsProvider
 	case client.ProviderDO:
 		term.Info("Using DigitalOcean provider")
-		byocProvider, err := do.NewByocProvider(ctx, grpcClient.TenantID)
+		doProvider, err := do.NewByocProvider(ctx, grpcClient.TenantID)
 		if err != nil {
 			term.Fatal(err)
 		}
-		return byocProvider
+		return doProvider
 	default:
 		term.Info("Using Defang Playground; consider using BYOC (https://s.defang.io/byoc)")
 		return &client.PlaygroundProvider{GrpcClient: grpcClient}

src/pkg/clouds/do/appPlatform/setup.go

@@ -234,7 +234,7 @@ func waitForActiveDeployment(ctx context.Context, apps godo.AppsService, appID s
 func NewClient(ctx context.Context) (*godo.Client, error) {
 	accessToken := os.Getenv("DIGITALOCEAN_TOKEN")
 	if accessToken == "" {
-		return nil, errors.New("DIGITALOCEAN_TOKEN must be set")
+		return nil, errors.New("DIGITALOCEAN_TOKEN must be set (https://docs.defang.io/docs/providers/digitalocean#getting-started)")
 	}
 	tokenSource := &oauth2.Token{AccessToken: accessToken}
 	httpClient := oauth2.NewClient(ctx, oauth2.StaticTokenSource(tokenSource))