From 8ba842654cc46cab083a91a31d78899c3ffe9ab3 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Tue, 3 Feb 2026 14:12:18 -0800 Subject: [PATCH 1/3] fix: update client type and refactor login handling for improved error management --- src/cmd/cli/command/commands.go | 5 +++-- src/cmd/cli/command/globals.go | 2 +- src/cmd/cli/command/testdata/no-stack/.defang | 1 - .../child/{.keep => .gitkeep} | 0 .../without-project/{.keep => .gitkeep} | 0 .../without-stack/child/{.keep => .gitkeep} | 0 src/pkg/cli/client/client.go | 7 +++++-- src/pkg/login/login.go | 17 +++++++++-------- src/pkg/setup/setup.go | 5 +++-- 9 files changed, 21 insertions(+), 16 deletions(-) delete mode 100644 src/cmd/cli/command/testdata/no-stack/.defang rename src/cmd/cli/command/testdata/with-project-and-stack/child/{.keep => .gitkeep} (100%) rename src/cmd/cli/command/testdata/without-project/{.keep => .gitkeep} (100%) rename src/cmd/cli/command/testdata/without-stack/child/{.keep => .gitkeep} (100%) diff --git a/src/cmd/cli/command/commands.go b/src/cmd/cli/command/commands.go index 8c619273f..2a481c544 100644 --- a/src/cmd/cli/command/commands.go +++ b/src/cmd/cli/command/commands.go @@ -435,7 +435,7 @@ var RootCmd = &cobra.Command{ if global.NonInteractive { err = global.Client.CheckLoginAndToS(ctx) } else { - err = login.InteractiveRequireLoginAndToS(ctx, global.Client, global.Cluster) + global.Client, err = login.InteractiveRequireLoginAndToS(ctx, global.Client, global.Cluster) } return err @@ -446,7 +446,8 @@ var RootCmd = &cobra.Command{ } ctx := cmd.Context() - err := login.InteractiveRequireLoginAndToS(ctx, global.Client, global.Cluster) + var err error + global.Client, err = login.InteractiveRequireLoginAndToS(ctx, global.Client, global.Cluster) if err != nil { return err } diff --git a/src/cmd/cli/command/globals.go b/src/cmd/cli/command/globals.go index 9945c78d8..a59fe0671 100644 --- a/src/cmd/cli/command/globals.go +++ b/src/cmd/cli/command/globals.go @@ -63,7 +63,7 @@ Note: Ensure the flag name, environment variable name, and struct field name are and follow the established naming conventions. */ type GlobalConfig struct { - Client *client.GrpcClient + Client client.FabricClient Cluster string ColorMode ColorMode Debug bool diff --git a/src/cmd/cli/command/testdata/no-stack/.defang b/src/cmd/cli/command/testdata/no-stack/.defang deleted file mode 100644 index e58b2a657..000000000 --- a/src/cmd/cli/command/testdata/no-stack/.defang +++ /dev/null @@ -1 +0,0 @@ -VALUE=from .defang \ No newline at end of file diff --git a/src/cmd/cli/command/testdata/with-project-and-stack/child/.keep b/src/cmd/cli/command/testdata/with-project-and-stack/child/.gitkeep similarity index 100% rename from src/cmd/cli/command/testdata/with-project-and-stack/child/.keep rename to src/cmd/cli/command/testdata/with-project-and-stack/child/.gitkeep diff --git a/src/cmd/cli/command/testdata/without-project/.keep b/src/cmd/cli/command/testdata/without-project/.gitkeep similarity index 100% rename from src/cmd/cli/command/testdata/without-project/.keep rename to src/cmd/cli/command/testdata/without-project/.gitkeep diff --git a/src/cmd/cli/command/testdata/without-stack/child/.keep b/src/cmd/cli/command/testdata/without-stack/child/.gitkeep similarity index 100% rename from src/cmd/cli/command/testdata/without-stack/child/.keep rename to src/cmd/cli/command/testdata/without-stack/child/.gitkeep diff --git a/src/pkg/cli/client/client.go b/src/pkg/cli/client/client.go index ad02e2f84..4330c9b97 100644 --- a/src/pkg/cli/client/client.go +++ b/src/pkg/cli/client/client.go @@ -19,18 +19,21 @@ type FabricClient interface { Estimate(context.Context, *defangv1.EstimateRequest) (*defangv1.EstimateResponse, error) GenerateCompose(context.Context, *defangv1.GenerateComposeRequest) (*defangv1.GenerateComposeResponse, error) GenerateFiles(context.Context, *defangv1.GenerateFilesRequest) (*defangv1.GenerateFilesResponse, error) - GetFabricClient() defangv1connect.FabricControllerClient + GetDefaultStack(context.Context, *defangv1.GetDefaultStackRequest) (*defangv1.GetStackResponse, error) GetDelegateSubdomainZone(context.Context, *defangv1.GetDelegateSubdomainZoneRequest) (*defangv1.DelegateSubdomainZoneResponse, error) + GetFabricClient() defangv1connect.FabricControllerClient GetPlaygroundProjectDomain(context.Context) (*defangv1.GetPlaygroundProjectDomainResponse, error) - GetDefaultStack(context.Context, *defangv1.GetDefaultStackRequest) (*defangv1.GetStackResponse, error) + GetRequestedTenant() types.TenantNameOrID GetTenantName() types.TenantLabel GetVersions(context.Context) (*defangv1.Version, error) ListDeployments(context.Context, *defangv1.ListDeploymentsRequest) (*defangv1.ListDeploymentsResponse, error) + ListStacks(context.Context, *defangv1.ListStacksRequest) (*defangv1.ListStacksResponse, error) Preview(context.Context, *defangv1.PreviewRequest) (*defangv1.PreviewResponse, error) Publish(context.Context, *defangv1.PublishRequest) error PutDeployment(context.Context, *defangv1.PutDeploymentRequest) error PutStack(context.Context, *defangv1.PutStackRequest) error RevokeToken(context.Context) error + SetOptions(context.Context, *defangv1.SetOptionsRequest) error Token(context.Context, *defangv1.TokenRequest) (*defangv1.TokenResponse, error) Track(string, ...Property) error VerifyDNSSetup(context.Context, *defangv1.VerifyDNSSetupRequest) error diff --git a/src/pkg/login/login.go b/src/pkg/login/login.go index d26b15acf..3a1cc0805 100644 --- a/src/pkg/login/login.go +++ b/src/pkg/login/login.go @@ -112,7 +112,9 @@ func NonInteractiveGitHubLogin(ctx context.Context, fabric client.FabricClient, return err } -func InteractiveRequireLoginAndToS(ctx context.Context, fabric *client.GrpcClient, addr string) error { +// InteractiveRequireLoginAndToS ensures the user is logged in and has agreed to the terms of service. +// If necessary, it will reconnect to the server as the right tenant, returning the updated Fabric client. +func InteractiveRequireLoginAndToS(ctx context.Context, fabric client.FabricClient, addr string) (client.FabricClient, error) { var err error if err = fabric.CheckLoginAndToS(ctx); err != nil { // Login interactively now; only do this for authorization-related errors @@ -123,19 +125,19 @@ func InteractiveRequireLoginAndToS(ctx context.Context, fabric *client.GrpcClien defer func() { track.Cmd(nil, "Login", P("reason", err)) }() if err = InteractiveLogin(ctx, addr); err != nil { - return err + return fabric, err } // Reconnect with the new token if newFabric, err := cli.ConnectWithTenant(ctx, addr, fabric.GetRequestedTenant()); err != nil { - return err + return fabric, err } else { - *fabric = *newFabric + fabric = newFabric track.Tracker = fabric // update global tracker } if err = fabric.CheckLoginAndToS(ctx); err == nil { // recheck (new token = new user) - return nil // success + return fabric, nil // success } } @@ -145,10 +147,9 @@ func InteractiveRequireLoginAndToS(ctx context.Context, fabric *client.GrpcClien defer func() { track.Cmd(nil, "Terms", P("reason", err)) }() if err = InteractiveAgreeToS(ctx, fabric); err != nil { - return err // fatal + return fabric, err // fatal } } } - - return err + return fabric, err } diff --git a/src/pkg/setup/setup.go b/src/pkg/setup/setup.go index 620cb2283..e170aef09 100644 --- a/src/pkg/setup/setup.go +++ b/src/pkg/setup/setup.go @@ -24,7 +24,7 @@ var P = track.P type SetupClient struct { Surveyor *surveyor.DefaultSurveyor ModelID string - Fabric *client.GrpcClient + Fabric client.FabricClient Cluster string } @@ -225,7 +225,8 @@ func beforeGenerate(directory string) { } func (s *SetupClient) MigrateFromHeroku(ctx context.Context) (SetupResult, error) { - err := login.InteractiveRequireLoginAndToS(ctx, s.Fabric, s.Cluster) + var err error + s.Fabric, err = login.InteractiveRequireLoginAndToS(ctx, s.Fabric, s.Cluster) if err != nil { return SetupResult{}, err } From df3acc40ccb7c5325d07d7a2411f27db8e21d446 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Tue, 3 Feb 2026 14:22:55 -0800 Subject: [PATCH 2/3] fix: refactor session handling in compose command and add tests for unauthenticated scenarios --- src/cmd/cli/command/compose.go | 15 ++++++++---- src/cmd/cli/command/compose_test.go | 36 +++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/cmd/cli/command/compose.go b/src/cmd/cli/command/compose.go index d3ebfd1c7..51d6b2126 100644 --- a/src/cmd/cli/command/compose.go +++ b/src/cmd/cli/command/compose.go @@ -531,26 +531,31 @@ func makeComposeConfigCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - session, err := newCommandSessionWithOpts(cmd, commandSessionOpts{ + sessionx, err := newCommandSessionWithOpts(cmd, commandSessionOpts{ CheckAccountInfo: false, }) if err != nil { term.Warn("unable to load stack:", err, "- some information may not be up-to-date") + sessionx = &session.Session{ + Loader: configureLoader(cmd), + Provider: client.NewPlaygroundProvider(global.Client, stacks.DefaultBeta), + Stack: &stacks.Parameters{Name: stacks.DefaultBeta, Provider: client.ProviderDefang}, + } } - _, err = session.Provider.AccountInfo(ctx) + _, err = sessionx.Provider.AccountInfo(ctx) if err != nil { term.Warn("unable to connect to cloud provider:", err, "- some information may not be up-to-date") } - project, loadErr := session.Loader.LoadProject(ctx) + project, loadErr := sessionx.Loader.LoadProject(ctx) if loadErr != nil { if global.NonInteractive { return loadErr } term.Error("Cannot load project:", loadErr) - project, err := session.Loader.CreateProjectForDebug() + project, err := sessionx.Loader.CreateProjectForDebug() if err != nil { term.Warn("Failed to create project for debug:", err) return loadErr @@ -567,7 +572,7 @@ func makeComposeConfigCmd() *cobra.Command { }, loadErr) } - _, _, err = cli.ComposeUp(ctx, global.Client, session.Provider, session.Stack, cli.ComposeUpParams{ + _, _, err = cli.ComposeUp(ctx, global.Client, sessionx.Provider, sessionx.Stack, cli.ComposeUpParams{ Project: project, UploadMode: compose.UploadModeIgnore, Mode: modes.ModeUnspecified, diff --git a/src/cmd/cli/command/compose_test.go b/src/cmd/cli/command/compose_test.go index 5a7a78bc7..b386cf56b 100644 --- a/src/cmd/cli/command/compose_test.go +++ b/src/cmd/cli/command/compose_test.go @@ -2,12 +2,14 @@ package command import ( "bytes" + "context" "os" "testing" "github.com/DefangLabs/defang/src/pkg/cli/client" "github.com/DefangLabs/defang/src/pkg/term" defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" + "github.com/bufbuild/connect-go" ) func TestInitializeTailCmd(t *testing.T) { @@ -43,3 +45,37 @@ func TestPrintPlaygroundPortalServiceURLs(t *testing.T) { t.Errorf("got %q, want %q", got, want) } } + +type unauthedMockFabricClient struct { + client.MockFabricClient +} + +func (c unauthedMockFabricClient) GetDefaultStack(context.Context, *defangv1.GetDefaultStackRequest) (*defangv1.GetStackResponse, error) { + return nil, connect.NewError(connect.CodeUnauthenticated, nil) +} + +func TestComposeConfig(t *testing.T) { + // Test fix for https://github.com/DefangLabs/defang/issues/1894 + global.Client = unauthedMockFabricClient{} + t.Cleanup(func() { + global.Client = nil + }) + + t.Run("Unauth OK", func(t *testing.T) { + t.Chdir("testdata/without-stack") + cmd := makeComposeConfigCmd() + err := cmd.Execute() + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) + + t.Run("Unauth OK - with stack", func(t *testing.T) { + t.Chdir("testdata/with-project-and-stack") + cmd := makeComposeConfigCmd() + err := cmd.Execute() + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) +} From 8ccb90286dde57874bf170e9f88d8f9f922a77e9 Mon Sep 17 00:00:00 2001 From: Lionello Lunesu Date: Tue, 3 Feb 2026 14:23:12 -0800 Subject: [PATCH 3/3] fix: ensure GCP project environment variables are properly unset during tests --- src/pkg/cli/client/byoc/gcp/byoc_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pkg/cli/client/byoc/gcp/byoc_test.go b/src/pkg/cli/client/byoc/gcp/byoc_test.go index 07a266a30..dc916ceaa 100644 --- a/src/pkg/cli/client/byoc/gcp/byoc_test.go +++ b/src/pkg/cli/client/byoc/gcp/byoc_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "errors" "io" + "os" "testing" "time" @@ -258,6 +259,11 @@ func TestGetGcpProjectID(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + for _, envVar := range pkg.GCPProjectEnvVars { + t.Setenv(envVar, "") // this ensures env var is restored after the test + os.Unsetenv(envVar) // after t.Setenv, to really unset it + } + // Set test environment variables for k, v := range tt.envVars { t.Setenv(k, v)