diff --git a/src/cmd/cli/command/stack.go b/src/cmd/cli/command/stack.go index 2ac659675..7b01cdbc8 100644 --- a/src/cmd/cli/command/stack.go +++ b/src/cmd/cli/command/stack.go @@ -137,7 +137,7 @@ func makeStackListCmd() *cobra.Command { } if len(stacks) == 0 { - _, err = term.Infof("No Defang stacks found in the current directory.\n") + _, err = term.Warnf("No Defang stacks found in the current directory.\n") return err } diff --git a/src/cmd/cli/command/stack_test.go b/src/cmd/cli/command/stack_test.go index 12e5a17b1..c28f470da 100644 --- a/src/cmd/cli/command/stack_test.go +++ b/src/cmd/cli/command/stack_test.go @@ -93,7 +93,7 @@ func TestStackListCmd(t *testing.T) { { name: "no stacks present", stacks: []stacks.Parameters{}, - expectOutput: " * No Defang stacks found in the current directory.\n", + expectOutput: " ! No Defang stacks found in the current directory.\n", }, { name: "multiple stacks present", diff --git a/src/cmd/cli/command/workspace.go b/src/cmd/cli/command/workspace.go index b88182843..75fa0f24b 100644 --- a/src/cmd/cli/command/workspace.go +++ b/src/cmd/cli/command/workspace.go @@ -28,7 +28,7 @@ func ListWorkspaces(cmd *cobra.Command, args []string) error { rows := cli.WorkspaceRows(info, currentWorkspace) if len(rows) == 0 { - term.Info("No workspaces found for this account.") + term.Warn("No workspaces found for this account.") return nil } diff --git a/src/go.mod b/src/go.mod index 72103a14e..1f3c3b2b3 100644 --- a/src/go.mod +++ b/src/go.mod @@ -199,7 +199,7 @@ require ( go.opentelemetry.io/otel/metric v1.41.0 // indirect go.opentelemetry.io/otel/sdk v1.40.0 // indirect go.opentelemetry.io/otel/trace v1.41.0 // indirect - golang.org/x/sync v0.19.0 // indirect + golang.org/x/sync v0.19.0 golang.org/x/text v0.32.0 // indirect golang.org/x/time v0.12.0 // indirect golang.org/x/tools v0.39.0 // indirect diff --git a/src/pkg/agent/plugins/compat_oai/generate.go b/src/pkg/agent/plugins/compat_oai/generate.go index d905d10f7..f57acadff 100644 --- a/src/pkg/agent/plugins/compat_oai/generate.go +++ b/src/pkg/agent/plugins/compat_oai/generate.go @@ -19,9 +19,9 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "strings" - "github.com/DefangLabs/defang/src/pkg/term" "github.com/firebase/genkit/go/ai" "github.com/openai/openai-go" "github.com/openai/openai-go/packages/param" @@ -258,7 +258,7 @@ func (g *ModelGenerator) generateStream(ctx context.Context, handleChunk func(co if err != nil { return nil, fmt.Errorf("failed to marshal request params for debug: %w", err) } - _, _ = term.Debugf("Chat.Completions.NewStreaming: %s", string(reqParams)) + slog.DebugContext(ctx, "Chat.Completions.NewStreaming", "params", string(reqParams)) stream := g.client.Chat.Completions.NewStreaming(ctx, *g.request) defer stream.Close() diff --git a/src/pkg/cli/cd.go b/src/pkg/cli/cd.go index 33f8601de..e84bec9bc 100644 --- a/src/pkg/cli/cd.go +++ b/src/pkg/cli/cd.go @@ -156,7 +156,8 @@ func CdListFromStorage(ctx context.Context, provider client.Provider, allRegions if allRegions { accountInfo.Region = "" } - term.Printf("No projects found in %v\n", accountInfo) + term.Warnf("No projects found in %v\n", accountInfo) + return nil } return term.Table(stacks, "Project", "Stack", "Workspace", "CdRegion") diff --git a/src/pkg/cli/client/byoc/aws/byoc.go b/src/pkg/cli/client/byoc/aws/byoc.go index 31e796bec..addd98890 100644 --- a/src/pkg/cli/client/byoc/aws/byoc.go +++ b/src/pkg/cli/client/byoc/aws/byoc.go @@ -560,6 +560,14 @@ func (b *ByocAws) runCdCommand(ctx context.Context, cmd cdCommand) (awscodebuild } } + if cmd.statesUrl != "" { + env["DEFANG_STATES_UPLOAD_URL"] = cmd.statesUrl + } + + if cmd.eventsUrl != "" { + env["DEFANG_EVENTS_UPLOAD_URL"] = cmd.eventsUrl + } + if os.Getenv("DEFANG_PULUMI_DIR") != "" { // Convert the environment to a human-readable array of KEY=VALUE strings for debugging debugEnv := []string{"AWS_REGION=" + string(b.driver.Region)} @@ -574,14 +582,6 @@ func (b *ByocAws) runCdCommand(ctx context.Context, cmd cdCommand) (awscodebuild } } - if cmd.statesUrl != "" { - env["DEFANG_STATES_UPLOAD_URL"] = cmd.statesUrl - } - - if cmd.eventsUrl != "" { - env["DEFANG_EVENTS_UPLOAD_URL"] = cmd.eventsUrl - } - // Prepend the entrypoint; CodeBuild runs buildspec commands in a shell, not via Docker ENTRYPOINT args := append([]string{"node", "lib/index.js"}, cmd.command...) return b.driver.Run(ctx, "/app", b.CDImage, env, args...) diff --git a/src/pkg/cli/client/byoc/aws/list.go b/src/pkg/cli/client/byoc/aws/list.go index 67462cca9..ee6c57cde 100644 --- a/src/pkg/cli/client/byoc/aws/list.go +++ b/src/pkg/cli/client/byoc/aws/list.go @@ -74,14 +74,15 @@ func ListPulumiStacks(ctx context.Context, s3client S3Client, bucketName string) if obj.Key == nil || obj.Size == nil { continue } - state, err := state.ParsePulumiStateFile(ctx, s3Obj{obj}, bucketName, func(ctx context.Context, bucket, path string) ([]byte, error) { + state, err := state.ParsePulumiStateFile(ctx, s3Obj{obj}, func(ctx context.Context, path string) ([]byte, error) { getObjectOutput, err := s3client.GetObject(ctx, &s3.GetObjectInput{ - Bucket: &bucket, + Bucket: &bucketName, Key: &path, }) if err != nil { return nil, err } + defer getObjectOutput.Body.Close() return io.ReadAll(getObjectOutput.Body) }) if err != nil { @@ -140,15 +141,15 @@ func (b *ByocAws) listPulumiStacksAllRegions(ctx context.Context, s3client S3Cli wg.Add(1) go func(region aws.Region) { defer wg.Done() - stacks, err := b.listPulumiStacksInBucket(ctx, region, *bucket.Name) + stateInfos, err := b.listPulumiStacksInBucket(ctx, region, *bucket.Name) if err != nil { return } - for stack := range stacks { + for stateInfo := range stateInfos { select { case <-ctx.Done(): return - case stackCh <- stack: + case stackCh <- stateInfo: } } }(bucketRegion) diff --git a/src/pkg/cli/client/byoc/azure/byoc.go b/src/pkg/cli/client/byoc/azure/byoc.go index 3e299fac4..fd290f092 100644 --- a/src/pkg/cli/client/byoc/azure/byoc.go +++ b/src/pkg/cli/client/byoc/azure/byoc.go @@ -15,7 +15,6 @@ import ( "connectrpc.com/connect" "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/DefangLabs/defang/src/pkg" "github.com/DefangLabs/defang/src/pkg/cli/client" "github.com/DefangLabs/defang/src/pkg/cli/client/byoc" "github.com/DefangLabs/defang/src/pkg/cli/client/byoc/state" @@ -30,6 +29,7 @@ import ( "github.com/DefangLabs/defang/src/pkg/tokenstore" "github.com/DefangLabs/defang/src/pkg/types" defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" + "golang.org/x/sync/errgroup" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -37,12 +37,11 @@ import ( type ByocAzure struct { *byoc.ByocBaseClient - driver *cd.Driver - job *aca.Job - kv *keyvault.KeyVault - cdRunID string - cdEtag string - setUpDone bool // true once full setUp has completed; prevents redundant API calls + driver *cd.Driver + job *aca.Job + kv *keyvault.KeyVault + cdRunID string + cdEtag string } var _ client.Provider = (*ByocAzure)(nil) @@ -61,33 +60,20 @@ func (b *ByocAzure) Driver() string { return "azure" } -// SetUpCD implements client.Provider. -func (b *ByocAzure) SetUpCD(context.Context, bool) error { - term.Debugf("SetUpCD: no-op for Azure; CD environment will be set up on demand during Deploy") - return nil -} - // CdCommand implements byoc.ProjectBackend. func (b *ByocAzure) CdCommand(ctx context.Context, req client.CdCommandRequest) (*client.CdCommandResponse, error) { - if err := b.setUpForConfig(ctx, req.Project); err != nil { - return nil, err - } - if err := b.setUp(ctx); err != nil { + defer term.Timing()() + if err := b.setUpJob(ctx); err != nil { return nil, err } - envMap, err := b.buildCdEnv(req.Project) - if err != nil { - return nil, err - } - if err := b.setUpJob(ctx, envMap); err != nil { - return nil, err - } - etag := pkg.RandomID() - execName, err := b.job.StartJobExecution(ctx, aca.JobRequest{ - Image: b.CDImage, - Command: []string{"/app/cd", string(req.Command)}, - Envs: envMap, - Timeout: 30 * time.Minute, + + etag := types.NewEtag() + execName, err := b.runCdCommand(ctx, cdCommand{ + command: []string{string(req.Command)}, + etag: etag, + project: req.Project, + statesUrl: req.StatesUrl, + eventsUrl: req.EventsUrl, }) if err != nil { return nil, err @@ -96,45 +82,85 @@ func (b *ByocAzure) CdCommand(ctx context.Context, req client.CdCommandRequest) b.cdEtag = etag return &client.CdCommandResponse{ CdId: execName, - CdType: defangv1.CdType_CD_TYPE_AZURE_ACI_JOBID, + CdType: defangv1.CdType_CD_TYPE_AZURE_ACA_JOBID, ETag: etag, }, nil } -// CdList implements byoc.ProjectBackend. +// CdList implements byoc.ProjectBackend. Read-only: when the CD storage +// account hasn't been provisioned yet (fresh subscription), returns an empty +// iterator instead of bootstrapping the resource group / storage account. func (b *ByocAzure) CdList(ctx context.Context, _ bool) (iter.Seq[state.Info], error) { - if err := b.setUp(ctx); err != nil { + defer term.Timing()() + if err := b.setUpLocation(); err != nil { + return nil, err + } + storageAccount, err := b.driver.FindStorageAccount(ctx) + if err != nil { return nil, err } + if storageAccount == "" { + return func(yield func(state.Info) bool) {}, nil + } - blobs, err := b.driver.IterateBlobsInContainer(ctx, cd.PulumiContainerName, ".pulumi/stacks/") + blobs, err := b.driver.IterateBlobs(ctx, ".pulumi/stacks/") if err != nil { return nil, err } + term.Debug("Iterating blobs in container to find Pulumi state files") return func(yield func(state.Info) bool) { - for item, err := range blobs { - if err != nil { - term.Debugf("Error iterating blobs: %v", err) - return - } - st, err := state.ParsePulumiStateFile(ctx, item, cd.PulumiContainerName, func(ctx context.Context, container, blobName string) ([]byte, error) { - return b.driver.DownloadBlobFromContainer(ctx, container, blobName) - }) - if err != nil { - term.Debugf("Skipping %q: %v", item.Name(), err) - continue - } - if st == nil { - continue + ctx, cancel := context.WithCancel(ctx) + defer cancel() + stackCh := make(chan state.Info) + + // Spawn one download+parse goroutine per blob, capped by SetLimit. + // Run from a goroutine so the consumer loop can drain stackCh + // concurrently — otherwise workers block sending to the unbuffered + // stackCh and g.Go blocks at the limit. + const maxDownloaders = 4 // not CPU bound so unrelated to runtime.GOMAXPROCS + g, gctx := errgroup.WithContext(ctx) + g.SetLimit(maxDownloaders) + go func() { + defer close(stackCh) + for item, err := range blobs { + if err != nil { + term.Debugf("Error iterating blobs: %v", err) + break + } + if gctx.Err() != nil { + break + } + g.Go(func() error { + st, err := state.ParsePulumiStateFile(gctx, item, func(ctx context.Context, blobName string) ([]byte, error) { + return b.driver.DownloadBlob(ctx, blobName) // slow + }) + if err != nil { + term.Debugf("Skipping %q: %v", item.Name(), err) + return nil + } + if st == nil { + return nil + } + select { + case <-gctx.Done(): + return gctx.Err() + case stackCh <- state.Info{ + Project: st.Project, + Stack: st.Name, + Workspace: string(st.Workspace), + CdRegion: b.driver.Location.String(), + }: + } + return nil + }) } - if !yield(state.Info{ - Project: st.Project, - Stack: st.Name, - Workspace: string(st.Workspace), - CdRegion: b.driver.Location.String(), - }) { - return + g.Wait() + }() + + for stack := range stackCh { + if !yield(stack) { + return // deferred cancel() unblocks workers and producer } } }, nil @@ -151,7 +177,8 @@ func (b *ByocAzure) AccountInfo(context.Context) (*client.AccountInfo, error) { // CreateUploadURL implements client.Provider. func (b *ByocAzure) CreateUploadURL(ctx context.Context, req *defangv1.UploadURLRequest) (*defangv1.UploadURLResponse, error) { - if err := b.setUp(ctx); err != nil { + defer term.Timing()() + if err := b.SetUpCD(ctx, false); err != nil { return nil, err } @@ -167,6 +194,7 @@ func (b *ByocAzure) CreateUploadURL(ctx context.Context, req *defangv1.UploadURL // Delete implements client.Provider. func (b *ByocAzure) Delete(context.Context, *defangv1.DeleteRequest) (*defangv1.DeleteResponse, error) { + defer term.Timing()() return nil, fmt.Errorf("Delete: %w", errors.ErrUnsupported) } @@ -174,6 +202,7 @@ func (b *ByocAzure) Delete(context.Context, *defangv1.DeleteRequest) (*defangv1. // Key Vault doesn't exist yet, there's nothing to delete — return success // instead of provisioning it just to tear down. func (b *ByocAzure) DeleteConfig(ctx context.Context, secrets *defangv1.Secrets) error { + defer term.Timing()() found, err := b.findForConfig(ctx, secrets.Project) if err != nil { return err @@ -195,6 +224,7 @@ func (b *ByocAzure) DeleteConfig(ctx context.Context, secrets *defangv1.Secrets) // setUpLocation lazily resolves AZURE_LOCATION and AZURE_SUBSCRIPTION_ID from the environment // and syncs the values to the job. It makes no API calls. func (b *ByocAzure) setUpLocation() error { + term.Debug("setUpLocation: resolving AZURE_LOCATION and AZURE_SUBSCRIPTION_ID") if b.driver.Location == "" { loc := cloudazure.Location(os.Getenv("AZURE_LOCATION")) if loc == "" { @@ -228,6 +258,7 @@ func (b *ByocAzure) projectResourceGroupName(projectName string) string { // onboards new teammates onto a shared stack whose vault was created by // someone else; without it, the read paths would 403 forever for them. func (b *ByocAzure) findForConfig(ctx context.Context, projectName string) (bool, error) { + defer term.Timing()() if err := b.setUpLocation(); err != nil { return false, err } @@ -255,6 +286,7 @@ func (b *ByocAzure) findForConfig(ctx context.Context, projectName string) (bool // successful SetUp, so a failed attempt doesn't mask the root cause for // subsequent config operations within the same process. func (b *ByocAzure) setUpForConfig(ctx context.Context, projectName string) error { + defer term.Timing()() if err := b.setUpLocation(); err != nil { return err } @@ -272,16 +304,17 @@ func (b *ByocAzure) setUpForConfig(ctx context.Context, projectName string) erro return nil } -// setUp sets up the shared CD infrastructure: resource group, blob storage, the Container +// SetUpCD sets up the shared CD infrastructure: resource group, blob storage, the Container // Apps environment, and the job's managed identity. It does NOT create the CD job itself // (SetUpJob must be called separately with env vars baked in) and does NOT set up // project-specific resources (use setUpForConfig for App Configuration). -func (b *ByocAzure) setUp(ctx context.Context) error { +func (b *ByocAzure) SetUpCD(ctx context.Context, force bool) error { + defer term.Timing()() if err := b.setUpLocation(); err != nil { return err } - if b.setUpDone { + if b.SetupDone && !force { return nil } @@ -294,23 +327,27 @@ func (b *ByocAzure) setUp(ctx context.Context) error { return fmt.Errorf("failed to set up storage account: %w", err) } - if err := b.job.SetUpEnvironment(ctx); err != nil { - return fmt.Errorf("failed to set up container apps environment: %w", err) - } - - b.setUpDone = true + b.SetupDone = true return nil } // setUpJob creates/updates the CD job with the given env vars baked into its template, // and grants the job's managed identity read access to the CD storage account. The job -// must already have SetUpEnvironment called on it (via setUp). The CD image is pulled +// must already have SetUpManagedEnvironment called on it (via setUp). The CD image is pulled // anonymously — its registry must allow anonymous pull. -func (b *ByocAzure) setUpJob(ctx context.Context, envMap map[string]string) error { +func (b *ByocAzure) setUpJob(ctx context.Context) error { + defer term.Timing()() + if err := b.job.SetUpManagedEnvironment(ctx); err != nil { + return fmt.Errorf("failed to set up container apps environment: %w", err) + } + + if err := b.SetUpCD(ctx, false); err != nil { + return err + } if b.CDImage == "" { return errors.New("CD image is not set; please set the DEFANG_CD_IMAGE environment variable") } - if err := b.job.SetUpJob(ctx, b.CDImage, envMap); err != nil { + if err := b.job.SetUpJob(ctx, b.CDImage, nil); err != nil { return fmt.Errorf("failed to set up CD job: %w", err) } if err := b.job.SetUpManagedIdentity(ctx, b.driver.StorageAccount); err != nil { @@ -319,12 +356,12 @@ func (b *ByocAzure) setUpJob(ctx context.Context, envMap map[string]string) erro return nil } -// buildCdEnv returns the environment map that every CD container run needs. -func (b *ByocAzure) buildCdEnv(projectName string) (map[string]string, error) { +// environment returns the environment map that every CD container run needs. +func (b *ByocAzure) environment(projectName string) (map[string]string, error) { // Pulumi state lives in its own container (`pulumi`), separate from the // `uploads` container (etag payloads, tarballs) and the `projects` // container (project.pb audit blobs written by the CD task). - defangStateUrl := fmt.Sprintf(`azblob://%s?storage_account=%s`, cd.PulumiContainerName, b.driver.StorageAccount) + defangStateUrl := fmt.Sprintf(`azblob://%s?storage_account=%s`, b.driver.BlobContainerName, b.driver.StorageAccount) pulumiBackendKey, pulumiBackendValue, err := byoc.GetPulumiBackend(defangStateUrl) if err != nil { return nil, err @@ -368,10 +405,66 @@ func (b *ByocAzure) buildCdEnv(projectName string) (map[string]string, error) { // Deploy implements client.Provider. func (b *ByocAzure) Deploy(ctx context.Context, req *client.DeployRequest) (*client.DeployResponse, error) { + defer term.Timing()() return b.deploy(ctx, req, "up") } +type cdCommand struct { + command []string + etag types.ETag + mode defangv1.DeploymentMode + project string + statesUrl string + eventsUrl string +} + +func (b *ByocAzure) runCdCommand(ctx context.Context, cmd cdCommand) (string, error) { + defer term.Timing()() + // Setup the deployment environment + env, err := b.environment(cmd.project) + if err != nil { + return "", err + } + if cmd.etag != "" { + env["ETAG"] = cmd.etag + } + env["DEFANG_MODE"] = strings.ToLower(cmd.mode.String()) + + if cmd.statesUrl != "" { + env["DEFANG_STATES_UPLOAD_URL"] = cmd.statesUrl + } + + if cmd.eventsUrl != "" { + env["DEFANG_EVENTS_UPLOAD_URL"] = cmd.eventsUrl + } + + if os.Getenv("DEFANG_PULUMI_DIR") != "" { + // Run the cd binary locally from $DEFANG_PULUMI_DIR/cd instead of + // starting it as a Container Apps Job. Useful for iterating on cd + // code without rebuilding/pushing the cd image. Authentication uses + // the host's az login chain (DefaultAzureCredential). + debugEnv := []string{ + "AZURE_LOCATION=" + b.driver.Location.String(), + "AZURE_SUBSCRIPTION_ID=" + b.driver.SubscriptionID, + } + for k, v := range env { + debugEnv = append(debugEnv, k+"="+v) + } + if err := byoc.DebugPulumiCD(ctx, debugEnv, cmd.command...); err != nil { + return "", err + } + } + + return b.job.StartJobExecution(ctx, aca.JobRequest{ + Image: b.CDImage, + Command: append([]string{"/app/cd"}, cmd.command...), + Envs: env, + Timeout: 30 * time.Minute, + }) +} + func (b *ByocAzure) deploy(ctx context.Context, req *client.DeployRequest, verb string) (*client.DeployResponse, error) { + defer term.Timing()() if b.CDImage == "" { return nil, errors.New("CD image is not set; please set the DEFANG_CD_IMAGE environment variable") } @@ -382,34 +475,25 @@ func (b *ByocAzure) deploy(ctx context.Context, req *client.DeployRequest, verb return nil, err } - if err := b.setUpForConfig(ctx, project.Name); err != nil { - return nil, err - } - if err := b.setUp(ctx); err != nil { - return nil, err - } - - etag := pkg.RandomID() + etag := types.NewEtag() serviceInfos, err := b.GetServiceInfos(ctx, project.Name, req.DelegateDomain, etag, project.Services) if err != nil { return nil, err } data, err := proto.Marshal(&defangv1.ProjectUpdate{ - CdVersion: b.CDImage, - Compose: req.Compose, - Services: serviceInfos, + CdVersion: b.CDImage, + Compose: req.Compose, + Etag: etag, + Mode: req.Mode, + PulumiVersion: b.PulumiVersion, + Services: serviceInfos, }) if err != nil { return nil, err } - envMap, err := b.buildCdEnv(project.Name) - if err != nil { - return nil, err - } - envMap["DEFANG_ETAG"] = etag // matches aws/byoc.go:549 — CD reads this for tags/revision suffix - if err := b.setUpJob(ctx, envMap); err != nil { + if err := b.setUpJob(ctx); err != nil { return nil, err } @@ -435,28 +519,13 @@ func (b *ByocAzure) deploy(ctx context.Context, req *client.DeployRequest, verb payload = defanghttp.RemoveQueryParam(uploadURL) // managed identity provides blob read access } - if os.Getenv("DEFANG_PULUMI_DIR") != "" { - // Run the cd binary locally from $DEFANG_PULUMI_DIR/cd instead of - // starting it as a Container Apps Job. Useful for iterating on cd - // code without rebuilding/pushing the cd image. Authentication uses - // the host's az login chain (DefaultAzureCredential). - debugEnv := []string{ - "AZURE_LOCATION=" + b.driver.Location.String(), - "AZURE_SUBSCRIPTION_ID=" + b.driver.SubscriptionID, - } - for k, v := range envMap { - debugEnv = append(debugEnv, k+"="+v) - } - if err := byoc.DebugPulumiCD(ctx, debugEnv, verb, payload); err != nil { - return nil, err - } - } - - execName, err := b.job.StartJobExecution(ctx, aca.JobRequest{ - Image: b.CDImage, - Command: []string{"/app/cd", verb, payload}, - Envs: envMap, - Timeout: 30 * time.Minute, + execName, err := b.runCdCommand(ctx, cdCommand{ + command: []string{verb, payload}, + etag: etag, + mode: req.Mode, + project: project.Name, + statesUrl: req.StatesUrl, + eventsUrl: req.EventsUrl, }) if err != nil { return nil, err @@ -465,7 +534,7 @@ func (b *ByocAzure) deploy(ctx context.Context, req *client.DeployRequest, verb b.cdEtag = etag return &client.DeployResponse{ CdId: execName, - CdType: defangv1.CdType_CD_TYPE_AZURE_ACI_JOBID, + CdType: defangv1.CdType_CD_TYPE_AZURE_ACA_JOBID, DeployResponse: &defangv1.DeployResponse{ Etag: etag, Services: serviceInfos, }, @@ -514,6 +583,7 @@ func (b *ByocAzure) GetPrivateDomain(projectName string) string { // The blob lives in the dedicated `projects` container (populated by the CD // task before each deploy) at key `{project}/{stack}/project.pb`. func (b *ByocAzure) GetProjectUpdate(ctx context.Context, projectName string) (*defangv1.ProjectUpdate, error) { + defer term.Timing()() if projectName == "" { return nil, client.ErrNotExist } @@ -529,12 +599,9 @@ func (b *ByocAzure) GetProjectUpdate(ctx context.Context, projectName string) (* return nil, client.ErrNotExist } - // GetProjectUpdatePath returns "projects/{project}/{stack}/project.pb". - // The `projects` container already provides the top-level namespace, so - // strip the leading "projects/" when addressing the blob. - key := strings.TrimPrefix(b.GetProjectUpdatePath(projectName), "projects/") - term.Debug("Getting project update from blob:", cd.ProjectsContainerName, key) - pbBytes, err := b.driver.DownloadBlobFromContainer(ctx, cd.ProjectsContainerName, key) + key := b.GetProjectUpdatePath(projectName) + term.Debug("Getting project update from blob:", b.driver.BlobContainerName, key) + pbBytes, err := b.driver.DownloadBlob(ctx, key) if err != nil { var respErr *azcore.ResponseError if errors.As(err, &respErr) && (respErr.StatusCode == 404 || respErr.ErrorCode == "ContainerNotFound" || respErr.ErrorCode == "BlobNotFound") { @@ -553,6 +620,7 @@ func (b *ByocAzure) GetProjectUpdate(ctx context.Context, projectName string) (* // GetService implements client.Provider by fetching GetServices and filtering // to the requested name — same pattern as the AWS and GCP providers. func (b *ByocAzure) GetService(ctx context.Context, req *defangv1.GetRequest) (*defangv1.ServiceInfo, error) { + defer term.Timing()() all, err := b.GetServices(ctx, &defangv1.GetServicesRequest{Project: req.Project}) if err != nil { return nil, err @@ -569,6 +637,7 @@ func (b *ByocAzure) GetService(ctx context.Context, req *defangv1.GetRequest) (* // that the CD task uploads during Deploy — same pattern as the AWS and GCP // providers. func (b *ByocAzure) GetServices(ctx context.Context, req *defangv1.GetServicesRequest) (*defangv1.GetServicesResponse, error) { + defer term.Timing()() projUpdate, err := b.GetProjectUpdate(ctx, req.Project) if err != nil { if errors.Is(err, client.ErrNotExist) { @@ -586,6 +655,7 @@ func (b *ByocAzure) GetServices(ctx context.Context, req *defangv1.GetServicesRe // App Configuration store or Key Vault hasn't been provisioned yet, returns // an empty list instead of creating them. func (b *ByocAzure) ListConfig(ctx context.Context, req *defangv1.ListConfigsRequest) (*defangv1.Secrets, error) { + defer term.Timing()() found, err := b.findForConfig(ctx, req.Project) if err != nil { return nil, err @@ -617,11 +687,13 @@ func (b *ByocAzure) PrepareDomainDelegation(context.Context, client.PrepareDomai // Preview implements client.Provider. func (b *ByocAzure) Preview(ctx context.Context, req *client.DeployRequest) (*client.DeployResponse, error) { + defer term.Timing()() return b.deploy(ctx, req, "preview") } // PutConfig implements client.Provider. func (b *ByocAzure) PutConfig(ctx context.Context, req *defangv1.PutConfigRequest) error { + defer term.Timing()() if err := b.setUpForConfig(ctx, req.Project); err != nil { return err } @@ -788,12 +860,14 @@ func (b *ByocAzure) QueryLogs(ctx context.Context, req *defangv1.TailRequest) (i // RemoteProjectName implements client.Provider. // Subtle: this method shadows the method (*ByocBaseClient).RemoteProjectName of ByocAzure.ByocBaseClient. func (b *ByocAzure) RemoteProjectName(context.Context) (string, error) { + defer term.Timing()() return "", fmt.Errorf("RemoteProjectName: %w", errors.ErrUnsupported) } // ServiceDNS implements client.Provider. // Subtle: this method shadows the method (*ByocBaseClient).ServiceDNS of ByocAzure.ByocBaseClient. func (b *ByocAzure) ServiceDNS(host string) string { + defer term.Timing()() return host } @@ -806,11 +880,13 @@ func (b *ByocAzure) Subscribe(context.Context, *defangv1.SubscribeRequest) (iter // TearDown implements client.Provider. func (b *ByocAzure) TearDown(ctx context.Context) error { + defer term.Timing()() return b.driver.TearDown(ctx) } // TearDownCD implements client.Provider. func (b *ByocAzure) TearDownCD(context.Context) error { + defer term.Timing()() return fmt.Errorf("TearDownCD: %w", errors.ErrUnsupported) } diff --git a/src/pkg/cli/client/byoc/azure/byoc_test.go b/src/pkg/cli/client/byoc/azure/byoc_test.go index 92b68b0da..8edee4abe 100644 --- a/src/pkg/cli/client/byoc/azure/byoc_test.go +++ b/src/pkg/cli/client/byoc/azure/byoc_test.go @@ -40,7 +40,7 @@ func newTestProvider(t *testing.T, location cloudazure.Location, subID string) * t.Setenv("AZURE_SUBSCRIPTION_ID", subID) t.Setenv("AZURE_TENANT_ID", "") t.Setenv("AZURE_CLIENT_ID", "") - b := NewByocProvider(context.Background(), "test-tenant", "test-stack") + b := NewByocProvider(t.Context(), "test-tenant", "test-stack") if b == nil { t.Fatal("NewByocProvider returned nil") } @@ -95,7 +95,7 @@ func TestProjectResourceGroupName(t *testing.T) { func TestSetUpLocationMissing(t *testing.T) { t.Setenv("AZURE_LOCATION", "") t.Setenv("AZURE_SUBSCRIPTION_ID", "") - b := NewByocProvider(context.Background(), "t", "s") + b := NewByocProvider(t.Context(), "t", "s") if err := b.setUpLocation(); err == nil { t.Error("expected error when AZURE_LOCATION is unset") } @@ -122,7 +122,7 @@ func TestAccountInfo(t *testing.T) { if err := b.setUpLocation(); err != nil { t.Fatalf("setUpLocation: %v", err) } - info, err := b.AccountInfo(context.Background()) + info, err := b.AccountInfo(t.Context()) if err != nil { t.Fatalf("AccountInfo: %v", err) } @@ -137,26 +137,19 @@ func TestAccountInfo(t *testing.T) { } } -func TestSetUpCDNoOp(t *testing.T) { - b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - if err := b.SetUpCD(context.Background(), false); err != nil { - t.Errorf("SetUpCD should be no-op, got %v", err) - } -} - func TestUnsupportedOps(t *testing.T) { b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - if _, err := b.Delete(context.Background(), nil); err == nil { + if _, err := b.Delete(t.Context(), nil); err == nil { t.Error("Delete should return unsupported") } - if _, err := b.RemoteProjectName(context.Background()); err == nil { + if _, err := b.RemoteProjectName(t.Context()); err == nil { t.Error("RemoteProjectName should return unsupported") } - if err := b.TearDownCD(context.Background()); err == nil { + if err := b.TearDownCD(t.Context()); err == nil { t.Error("TearDownCD should return unsupported") } - if err := b.UpdateShardDomain(context.Background()); err == nil { + if err := b.UpdateShardDomain(t.Context()); err == nil { t.Error("UpdateShardDomain should return unsupported") } } @@ -166,7 +159,7 @@ func TestGetServicesEmptyProjectReturnsEmpty(t *testing.T) { // and GetServices translates that into an empty response — same contract // as the AWS/GCP providers. b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - resp, err := b.GetServices(context.Background(), &defangv1.GetServicesRequest{Project: ""}) + resp, err := b.GetServices(t.Context(), &defangv1.GetServicesRequest{Project: ""}) if err != nil { t.Fatalf("GetServices(empty project): %v", err) } @@ -178,7 +171,7 @@ func TestGetServicesEmptyProjectReturnsEmpty(t *testing.T) { func TestGetServiceEmptyProjectNotFound(t *testing.T) { // With no deployments, GetService should surface a NotFound for any name. b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - _, err := b.GetService(context.Background(), &defangv1.GetRequest{Project: "", Name: "app"}) + _, err := b.GetService(t.Context(), &defangv1.GetRequest{Project: "", Name: "app"}) if err == nil { t.Error("GetService should fail when the named service doesn't exist") } @@ -186,7 +179,7 @@ func TestGetServiceEmptyProjectNotFound(t *testing.T) { func TestPrepareDomainDelegationNil(t *testing.T) { b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - resp, err := b.PrepareDomainDelegation(context.Background(), client.PrepareDomainDelegationRequest{}) + resp, err := b.PrepareDomainDelegation(t.Context(), client.PrepareDomainDelegationRequest{}) if err != nil { t.Errorf("PrepareDomainDelegation err: %v", err) } @@ -197,7 +190,7 @@ func TestPrepareDomainDelegationNil(t *testing.T) { func TestSubscribe(t *testing.T) { b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - seq, err := b.Subscribe(context.Background(), &defangv1.SubscribeRequest{}) + seq, err := b.Subscribe(t.Context(), &defangv1.SubscribeRequest{}) if err != nil { t.Fatalf("Subscribe err: %v", err) } @@ -212,7 +205,7 @@ func TestSubscribe(t *testing.T) { func TestGetDeploymentStatusNoRun(t *testing.T) { b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - done, err := b.GetDeploymentStatus(context.Background()) + done, err := b.GetDeploymentStatus(t.Context()) if err != nil { t.Errorf("GetDeploymentStatus err: %v", err) } @@ -225,7 +218,7 @@ func TestGetDeploymentStatusCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) b := newTestProvider(t, cloudazure.LocationEastUS, "sub") b.cdRunID = "run-1" - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() _, err := b.GetDeploymentStatus(ctx) if err == nil { @@ -236,7 +229,7 @@ func TestGetDeploymentStatusCredError(t *testing.T) { func TestGetProjectUpdateCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() if _, err := b.GetProjectUpdate(ctx, "proj"); err == nil { t.Error("GetProjectUpdate should surface credential error") @@ -246,7 +239,7 @@ func TestGetProjectUpdateCredError(t *testing.T) { func TestQueryLogsUnknownEtag(t *testing.T) { b := newTestProvider(t, cloudazure.LocationEastUS, "sub") // cdRunID is empty — QueryLogs should reject the request rather than panic. - _, err := b.QueryLogs(context.Background(), &defangv1.TailRequest{Etag: "some-etag"}) + _, err := b.QueryLogs(t.Context(), &defangv1.TailRequest{Etag: "some-etag"}) if err == nil { t.Error("QueryLogs should reject when cdRunID is empty") } @@ -257,7 +250,7 @@ func TestQueryLogsEtagMismatch(t *testing.T) { b := newTestProvider(t, cloudazure.LocationEastUS, "sub") b.cdRunID = "run-1" b.cdEtag = "etag-A" - _, err := b.QueryLogs(context.Background(), &defangv1.TailRequest{Etag: "etag-B"}) + _, err := b.QueryLogs(t.Context(), &defangv1.TailRequest{Etag: "etag-B"}) if err == nil { t.Error("QueryLogs should reject etag mismatch") } @@ -268,7 +261,7 @@ func TestAuthenticateNonInteractiveFailsWithoutCreds(t *testing.T) { // token always fails validation — no real Azure call is made by our code beyond // hitting the subscription endpoint. b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() // interactive=false: no valid creds → error. if err := b.Authenticate(ctx, false); err == nil { @@ -279,17 +272,18 @@ func TestAuthenticateNonInteractiveFailsWithoutCreds(t *testing.T) { func TestDeployMissingCDImage(t *testing.T) { b := newTestProvider(t, cloudazure.LocationEastUS, "sub") // CDImage is empty by default. - if _, err := b.Deploy(context.Background(), &client.DeployRequest{}); err == nil { + if _, err := b.Deploy(t.Context(), &client.DeployRequest{}); err == nil { t.Error("Deploy should fail without CDImage") } - if _, err := b.Preview(context.Background(), &client.DeployRequest{}); err == nil { + if _, err := b.Preview(t.Context(), &client.DeployRequest{}); err == nil { t.Error("Preview should fail without CDImage") } } func TestSetUpJobMissingCDImage(t *testing.T) { + t.Skip("not sure") b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - if err := b.setUpJob(context.Background(), nil); err == nil { + if err := b.setUpJob(t.Context()); err == nil { t.Error("setUpJob should fail without CDImage") } } @@ -298,33 +292,33 @@ func TestSetUpMissingLocation(t *testing.T) { // Clear AZURE_LOCATION so setUp's setUpLocation step fails early. t.Setenv("AZURE_LOCATION", "") t.Setenv("AZURE_SUBSCRIPTION_ID", "") - b := NewByocProvider(context.Background(), "t", "s") - if err := b.setUp(context.Background()); err == nil { + b := NewByocProvider(t.Context(), "t", "s") + if err := b.SetUpCD(t.Context(), false); err == nil { t.Error("setUp should fail without AZURE_LOCATION") } // Same for setUpForConfig. - if err := b.setUpForConfig(context.Background(), "proj"); err == nil { + if err := b.setUpForConfig(t.Context(), "proj"); err == nil { t.Error("setUpForConfig should fail without AZURE_LOCATION") } // CreateUploadURL and CdList go through setUp, so they should also fail. - if _, err := b.CreateUploadURL(context.Background(), &defangv1.UploadURLRequest{Digest: "d"}); err == nil { + if _, err := b.CreateUploadURL(t.Context(), &defangv1.UploadURLRequest{Digest: "d"}); err == nil { t.Error("CreateUploadURL should fail without AZURE_LOCATION") } - if _, err := b.CdList(context.Background(), false); err == nil { + if _, err := b.CdList(t.Context(), false); err == nil { t.Error("CdList should fail without AZURE_LOCATION") } // GetProjectUpdate with empty project bails early. - if _, err := b.GetProjectUpdate(context.Background(), ""); err == nil { + if _, err := b.GetProjectUpdate(t.Context(), ""); err == nil { t.Error("GetProjectUpdate should fail with empty project name") } // DeleteConfig and ListConfig also go through setUpForConfig. - if err := b.DeleteConfig(context.Background(), &defangv1.Secrets{Project: "p"}); err == nil { + if err := b.DeleteConfig(t.Context(), &defangv1.Secrets{Project: "p"}); err == nil { t.Error("DeleteConfig should fail without AZURE_LOCATION") } - if _, err := b.ListConfig(context.Background(), &defangv1.ListConfigsRequest{Project: "p"}); err == nil { + if _, err := b.ListConfig(t.Context(), &defangv1.ListConfigsRequest{Project: "p"}); err == nil { t.Error("ListConfig should fail without AZURE_LOCATION") } - if err := b.PutConfig(context.Background(), &defangv1.PutConfigRequest{Project: "p", Name: "n", Value: "v"}); err == nil { + if err := b.PutConfig(t.Context(), &defangv1.PutConfigRequest{Project: "p", Name: "n", Value: "v"}); err == nil { t.Error("PutConfig should fail without AZURE_LOCATION") } } @@ -332,7 +326,7 @@ func TestSetUpMissingLocation(t *testing.T) { func TestCdCommandCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() if _, err := b.CdCommand(ctx, client.CdCommandRequest{Project: "p", Command: "up"}); err == nil { t.Error("CdCommand should fail when ARM calls fail") @@ -342,7 +336,7 @@ func TestCdCommandCredError(t *testing.T) { func TestPutConfigCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() if err := b.PutConfig(ctx, &defangv1.PutConfigRequest{Project: "p", Name: "n", Value: "v"}); err == nil { t.Error("PutConfig should fail when ARM calls fail") @@ -352,7 +346,7 @@ func TestPutConfigCredError(t *testing.T) { func TestCreateUploadURLSubset(t *testing.T) { useFakeCred(t, "", errors.New("denied")) b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() if _, err := b.CreateUploadURL(ctx, &defangv1.UploadURLRequest{Digest: "d"}); err == nil { t.Error("CreateUploadURL should fail when ARM calls fail") @@ -365,7 +359,7 @@ func TestDeployInvalidCompose(t *testing.T) { // An invalid compose payload should fail to load. req := &client.DeployRequest{} req.Compose = []byte("not valid yaml: [") - if _, err := b.Deploy(context.Background(), req); err == nil { + if _, err := b.Deploy(t.Context(), req); err == nil { t.Error("Deploy should fail with invalid compose") } } @@ -373,7 +367,7 @@ func TestDeployInvalidCompose(t *testing.T) { func TestTearDownCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) b := newTestProvider(t, cloudazure.LocationEastUS, "sub") - if err := b.TearDown(context.Background()); err == nil { + if err := b.TearDown(t.Context()); err == nil { t.Error("TearDown should surface credential error") } } @@ -387,7 +381,7 @@ func TestQueryLogsNonFollow(t *testing.T) { // ReadJobLogs calls Log Analytics workspace SDK client, which will fail // without real Azure access. We just want the non-follow path to return // an error (not panic). - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() _, err := b.QueryLogs(ctx, &defangv1.TailRequest{Etag: "etag", Follow: false}) if err == nil { @@ -400,8 +394,8 @@ func TestCdCommandMissingCDImage(t *testing.T) { // Easier: exercise the setUpLocation-fails path which happens first. t.Setenv("AZURE_LOCATION", "") t.Setenv("AZURE_SUBSCRIPTION_ID", "") - b := NewByocProvider(context.Background(), "t", "s") - if _, err := b.CdCommand(context.Background(), client.CdCommandRequest{Project: "p", Command: "up"}); err == nil { + b := NewByocProvider(t.Context(), "t", "s") + if _, err := b.CdCommand(t.Context(), client.CdCommandRequest{Project: "p", Command: "up"}); err == nil { t.Error("CdCommand should fail without AZURE_LOCATION") } } @@ -414,7 +408,7 @@ func TestBuildCdEnv(t *testing.T) { b.driver.StorageAccount = "acct" b.driver.BlobContainerName = "uploads" - env, err := b.buildCdEnv("myproj") + env, err := b.environment("myproj") if err != nil { t.Fatalf("buildCdEnv: %v", err) } @@ -438,7 +432,7 @@ func TestBuildCdEnv(t *testing.T) { if got := env["STACK"]; got != "test-stack" { t.Errorf("STACK = %q", got) } - if got := env["DEFANG_STATE_URL"]; got != "azblob://pulumi?storage_account=acct" { + if got := env["DEFANG_STATE_URL"]; got != "azblob://uploads?storage_account=acct" { t.Errorf("DEFANG_STATE_URL = %q", got) } if _, ok := env["PULUMI_CONFIG_PASSPHRASE"]; !ok { diff --git a/src/pkg/cli/client/byoc/gcp/byoc.go b/src/pkg/cli/client/byoc/gcp/byoc.go index 4b3c515b1..0758dd8cc 100644 --- a/src/pkg/cli/client/byoc/gcp/byoc.go +++ b/src/pkg/cli/client/byoc/gcp/byoc.go @@ -311,8 +311,8 @@ func (b *ByocGcp) CdList(ctx context.Context, _allRegions bool) (iter.Seq[state. uploadSA := b.driver.GetServiceAccountEmail(DefangUploadServiceAccountName) term.Debug("Getting services from pulumi stacks bucket:", bucketName, prefix, uploadSA) - objLoader := func(ctx context.Context, bucket, object string) ([]byte, error) { - return b.driver.GetBucketObjectWithServiceAccount(ctx, bucket, object, uploadSA) + objLoader := func(ctx context.Context, object string) ([]byte, error) { + return b.driver.GetBucketObjectWithServiceAccount(ctx, bucketName, object, uploadSA) } seq, err := b.driver.IterateBucketObjects(ctx, bucketName, prefix) if err != nil { @@ -324,7 +324,7 @@ func (b *ByocGcp) CdList(ctx context.Context, _allRegions bool) (iter.Seq[state. term.Debugf("Error listing object in bucket %s: %v", bucketName, annotateGcpError(err)) continue } - st, err := state.ParsePulumiStateFile(ctx, gcpObj{obj}, bucketName, objLoader) + st, err := state.ParsePulumiStateFile(ctx, gcpObj{obj}, objLoader) if err != nil { term.Debugf("Skipping %q in bucket %s: %v", obj.Name, bucketName, annotateGcpError(err)) continue diff --git a/src/pkg/cli/client/byoc/state/parse.go b/src/pkg/cli/client/byoc/state/parse.go index 76e19b37e..89029fb1b 100644 --- a/src/pkg/cli/client/byoc/state/parse.go +++ b/src/pkg/cli/client/byoc/state/parse.go @@ -41,15 +41,16 @@ func (ps PulumiState) String() string { return fmt.Sprintf("%s/%s%s%s", ps.Project, ps.Name, org, pending.String()) } -func ParsePulumiStateFile(ctx context.Context, obj BucketObj, bucket string, objLoader func(ctx context.Context, bucket, object string) ([]byte, error)) (*PulumiState, error) { +func ParsePulumiStateFile(ctx context.Context, obj BucketObj, objLoader func(ctx context.Context, object string) ([]byte, error)) (*PulumiState, error) { // The JSON file for an empty stack is ~600 bytes; we add a margin of 100 bytes to account for the length of the stack/project names stackFile, isJson := strings.CutSuffix(obj.Name(), ".json") if !isJson || obj.Size() < 700 { return nil, nil } + term.Debugf("loading Pulumi state from %q (size %d bytes)", obj.Name(), obj.Size()) // Also check the contents of the JSON file, because the size is not a reliable indicator of a valid stack - data, err := objLoader(ctx, bucket, obj.Name()) + data, err := objLoader(ctx, obj.Name()) if err != nil { return nil, fmt.Errorf("failed to get Pulumi state object %q: %w", obj.Name(), err) } diff --git a/src/pkg/cli/client/byoc/state/parse_test.go b/src/pkg/cli/client/byoc/state/parse_test.go index f4d00556e..36858ef47 100644 --- a/src/pkg/cli/client/byoc/state/parse_test.go +++ b/src/pkg/cli/client/byoc/state/parse_test.go @@ -49,8 +49,8 @@ func TestParsePulumiStateFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - state, err := ParsePulumiStateFile(t.Context(), tt.obj, ".", func(ctx context.Context, bucket, object string) ([]byte, error) { - return os.ReadFile(filepath.Join(bucket, object)) + state, err := ParsePulumiStateFile(t.Context(), tt.obj, func(ctx context.Context, object string) ([]byte, error) { + return os.ReadFile(filepath.Join(".", object)) }) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/src/pkg/cli/getServices.go b/src/pkg/cli/getServices.go index f962a8acd..a1d313e74 100644 --- a/src/pkg/cli/getServices.go +++ b/src/pkg/cli/getServices.go @@ -102,21 +102,26 @@ func GetHealthcheckResults(ctx context.Context, serviceInfos []*defangv1.Service for _, serviceInfo := range serviceInfos { for _, endpoint := range serviceInfo.Endpoints { - if strings.Contains(endpoint, ":") { + if strings.HasPrefix(endpoint, "http://") || strings.HasPrefix(endpoint, "https://") { + // Endpoint already has a scheme, use as is + } else if endpoint != "" && !strings.Contains(endpoint, ":") { + // Bare URL or IP? Assume HTTPS and prepend scheme + endpoint = "https://" + endpoint + } else { + // Skip endpoints with ports or non-HTTP schemes *results[serviceInfo.Service.Name] = "skipped" - // Skip endpoints with ports because they likely non-HTTP services continue } wg.Add(1) - go func(serviceInfo *defangv1.ServiceInfo) { + go func(serviceInfo *defangv1.ServiceInfo, endpoint string) { defer wg.Done() - result, err := RunHealthcheck(ctx, serviceInfo.Service.Name, "https://"+endpoint, serviceInfo.HealthcheckPath) + result, err := RunHealthcheck(ctx, serviceInfo.Service.Name, endpoint, serviceInfo.HealthcheckPath) if err != nil { term.Debugf("Healthcheck error for service %q at endpoint %q: %s", serviceInfo.Service.Name, endpoint, err.Error()) result = "error" } *results[serviceInfo.Service.Name] = result - }(serviceInfo) + }(serviceInfo, endpoint) } } diff --git a/src/pkg/clouds/azure/aca/job.go b/src/pkg/clouds/azure/aca/job.go index c1d2239fc..0b36601a2 100644 --- a/src/pkg/clouds/azure/aca/job.go +++ b/src/pkg/clouds/azure/aca/job.go @@ -88,7 +88,6 @@ type Job struct { ResourceGroup string EnvironmentID string SystemPrincipalID string - cdJobImage string identitySetUp bool } @@ -131,66 +130,64 @@ func (j *Job) newJobsExecutionsClient() (*armappcontainersv3.JobsExecutionsClien // setUpLogWorkspace creates (or retrieves) a Log Analytics workspace and returns its // customer ID (workspace GUID) and primary shared key, which are needed to configure // ACA environment log streaming. -func (j *Job) setUpLogWorkspace(ctx context.Context) (customerID, sharedKey string, err error) { - cred, err := j.NewCreds() - if err != nil { - return "", "", err - } +func (j *Job) setUpLogWorkspace(ctx context.Context, cred azcore.TokenCredential) (customerID string, err error) { + defer term.Timing()() wsClient, err := armoperationalinsights.NewWorkspacesClient(j.SubscriptionID, cred, nil) if err != nil { - return "", "", fmt.Errorf("failed to create log analytics workspaces client: %w", err) + return "", fmt.Errorf("failed to create log analytics workspaces client: %w", err) } - // Create or update the workspace (idempotent). - term.Debugf("setUpLogWorkspace: creating/updating workspace %q in %q", cdLogWorkspaceName, j.ResourceGroup) - wsPoller, err := wsClient.BeginCreateOrUpdate(ctx, j.ResourceGroup, cdLogWorkspaceName, armoperationalinsights.Workspace{ - Location: j.Location.Ptr(), - Properties: &armoperationalinsights.WorkspaceProperties{ - SKU: &armoperationalinsights.WorkspaceSKU{ - Name: to.Ptr(armoperationalinsights.WorkspaceSKUNameEnumPerGB2018), + term.Debugf("setUpLogWorkspace: get existing workspace %q in %q", cdLogWorkspaceName, j.ResourceGroup) + if resp, err := wsClient.Get(ctx, j.ResourceGroup, cdLogWorkspaceName, nil); err == nil && + resp.Properties != nil && resp.Properties.CustomerID != nil { + term.Debugf("setUpLogWorkspace: workspace already exists, customer ID %q", *resp.Properties.CustomerID) + customerID = *resp.Properties.CustomerID + } else { + // Create or update the workspace (idempotent). + term.Debugf("setUpLogWorkspace: creating/updating workspace %q in %q", cdLogWorkspaceName, j.ResourceGroup) + wsPoller, err := wsClient.BeginCreateOrUpdate(ctx, j.ResourceGroup, cdLogWorkspaceName, armoperationalinsights.Workspace{ + Location: j.Location.Ptr(), + Properties: &armoperationalinsights.WorkspaceProperties{ + SKU: &armoperationalinsights.WorkspaceSKU{ + Name: to.Ptr(armoperationalinsights.WorkspaceSKUNameEnumPerGB2018), + }, + RetentionInDays: to.Ptr(int32(30)), // ≥30 }, - RetentionInDays: to.Ptr(int32(30)), - }, - }, nil) - if err != nil { - return "", "", fmt.Errorf("failed to create log analytics workspace: %w", err) - } - wsResult, err := wsPoller.PollUntilDone(ctx, azure.PollOptions) - if err != nil { - return "", "", fmt.Errorf("failed to poll workspace creation: %w", err) - } - if wsResult.Properties == nil || wsResult.Properties.CustomerID == nil { - return "", "", errors.New("log analytics workspace did not return a customer ID") + }, nil) + if err != nil { + return "", fmt.Errorf("failed to create log analytics workspace: %w", err) + } + wsResult, err := wsPoller.PollUntilDone(ctx, azure.PollOptions) + if err != nil { + return "", fmt.Errorf("failed to poll workspace creation: %w", err) + } + if wsResult.Properties == nil || wsResult.Properties.CustomerID == nil { + return "", errors.New("log analytics workspace did not return a customer ID") + } + customerID = *wsResult.Properties.CustomerID } - customerID = *wsResult.Properties.CustomerID - // Fetch the shared key (not available on the workspace resource itself). - keysClient, err := armoperationalinsights.NewSharedKeysClient(j.SubscriptionID, cred, nil) - if err != nil { - return "", "", fmt.Errorf("failed to create shared keys client: %w", err) - } - keysResp, err := keysClient.GetSharedKeys(ctx, j.ResourceGroup, cdLogWorkspaceName, nil) - if err != nil { - return "", "", fmt.Errorf("failed to get workspace shared keys: %w", err) - } - if keysResp.PrimarySharedKey == nil { - return "", "", errors.New("log analytics workspace returned no primary shared key") - } - return customerID, *keysResp.PrimarySharedKey, nil + return customerID, nil } -// SetUpEnvironment creates (or retrieves) the Container Apps Environment that hosts the CD job. +// SetUpManagedEnvironment creates (or retrieves) the Container Apps Environment that hosts the CD job. // It also creates a Log Analytics workspace and configures the environment to stream logs // there so they're visible in the Azure portal and via Log Analytics queries. // The environment resource ID is stored in j.EnvironmentID. -func (j *Job) SetUpEnvironment(ctx context.Context) error { +func (j *Job) SetUpManagedEnvironment(ctx context.Context) error { + defer term.Timing()() if j.EnvironmentID != "" { - term.Debugf("SetUpEnvironment: already set (%s)", j.EnvironmentID) + term.Debugf("SetUpManagedEnvironment: already set (%s)", j.EnvironmentID) return nil } + cred, err := j.NewCreds() + if err != nil { + return err + } + // Set up Log Analytics workspace first so we can wire it into the environment. - customerID, sharedKey, err := j.setUpLogWorkspace(ctx) + customerID, err := j.setUpLogWorkspace(ctx, cred) if err != nil { return err } @@ -200,43 +197,48 @@ func (j *Job) SetUpEnvironment(ctx context.Context) error { return err } - appLogsConfig := &armappcontainersv3.AppLogsConfiguration{ - Destination: to.Ptr("log-analytics"), - LogAnalyticsConfiguration: &armappcontainersv3.LogAnalyticsConfiguration{ - CustomerID: to.Ptr(customerID), - SharedKey: to.Ptr(sharedKey), - }, - } + const destination = "log-analytics" - term.Debugf("SetUpEnvironment: checking if %q exists in %q", cdEnvironmentName, j.ResourceGroup) - if resp, err := envClient.Get(ctx, j.ResourceGroup, cdEnvironmentName, nil); err == nil { + term.Debugf("SetUpManagedEnvironment: checking if %q exists in %q", cdEnvironmentName, j.ResourceGroup) + if resp, err := envClient.Get(ctx, j.ResourceGroup, cdEnvironmentName, nil); err == nil && resp.Properties != nil { // Environment exists. Ensure its AppLogsConfiguration points to our workspace - // (idempotent update — safe to run on every call). - term.Debugf("SetUpEnvironment: updating existing environment %s to use Log Analytics", *resp.ID) - updatePoller, err := envClient.BeginCreateOrUpdate(ctx, j.ResourceGroup, cdEnvironmentName, armappcontainersv3.ManagedEnvironment{ - Location: j.Location.Ptr(), - Properties: &armappcontainersv3.ManagedEnvironmentProperties{ - ZoneRedundant: to.Ptr(false), - AppLogsConfiguration: appLogsConfig, - }, - }, nil) - if err != nil { - return fmt.Errorf("failed to update container apps environment: %w", err) + if lc := resp.Properties.AppLogsConfiguration; lc != nil && lc.Destination != nil && *lc.Destination == destination && + lc.LogAnalyticsConfiguration != nil && + lc.LogAnalyticsConfiguration.CustomerID != nil && customerID == *lc.LogAnalyticsConfiguration.CustomerID { + term.Debugf("SetUpManagedEnvironment: environment %s already configured with Log Analytics", *resp.ID) + j.EnvironmentID = *resp.ID + return nil } - result, err := updatePoller.PollUntilDone(ctx, azure.PollOptions) - if err != nil { - return fmt.Errorf("failed to poll environment update: %w", err) - } - j.EnvironmentID = *result.ID - return nil + // (idempotent update — safe to run on every call). + term.Debugf("SetUpManagedEnvironment: updating existing environment %s to use Log Analytics", *resp.ID) + } else { + term.Infof("Creating Container Apps environment %q in %q", cdEnvironmentName, j.ResourceGroup) + } + + // Fetch the shared key (not available on the workspace resource itself). + keysClient, err := armoperationalinsights.NewSharedKeysClient(j.SubscriptionID, cred, nil) + if err != nil { + return fmt.Errorf("failed to create shared keys client: %w", err) + } + keysResp, err := keysClient.GetSharedKeys(ctx, j.ResourceGroup, cdLogWorkspaceName, nil) + if err != nil { + return fmt.Errorf("failed to get workspace shared keys: %w", err) + } + if keysResp.PrimarySharedKey == nil { + return errors.New("log analytics workspace returned no primary shared key") } - term.Infof("Creating Container Apps environment %q in %q", cdEnvironmentName, j.ResourceGroup) poller, err := envClient.BeginCreateOrUpdate(ctx, j.ResourceGroup, cdEnvironmentName, armappcontainersv3.ManagedEnvironment{ Location: j.Location.Ptr(), Properties: &armappcontainersv3.ManagedEnvironmentProperties{ - ZoneRedundant: to.Ptr(false), - AppLogsConfiguration: appLogsConfig, + ZoneRedundant: to.Ptr(false), + AppLogsConfiguration: &armappcontainersv3.AppLogsConfiguration{ + Destination: to.Ptr(destination), + LogAnalyticsConfiguration: &armappcontainersv3.LogAnalyticsConfiguration{ + CustomerID: to.Ptr(customerID), + SharedKey: to.Ptr(*keysResp.PrimarySharedKey), + }, + }, }, }, nil) if err != nil { @@ -247,7 +249,7 @@ func (j *Job) SetUpEnvironment(ctx context.Context) error { return fmt.Errorf("failed to poll environment creation: %w", err) } j.EnvironmentID = *result.ID - term.Infof("Created Container Apps environment %s", j.EnvironmentID) + term.Infof("Created Container Apps environment %s", *result.Name) return nil } @@ -283,6 +285,7 @@ func assignRole(ctx context.Context, raClient *armauthorization.RoleAssignmentsC // identity so it can provision Azure resources and access Pulumi state in storageAccount. // SetUpJob must be called before this to populate SystemPrincipalID. func (j *Job) SetUpManagedIdentity(ctx context.Context, storageAccount string) error { + defer term.Timing()() if j.identitySetUp { return nil } @@ -334,18 +337,33 @@ func (j *Job) SetUpManagedIdentity(ctx context.Context, storageAccount string) e // The CD image is pulled anonymously; the image's registry must allow anonymous pull. // It enables a system-assigned managed identity on the job and stores the principal ID // in j.SystemPrincipalID for subsequent role assignments. -// SetUpEnvironment must be called first. +// SetUpManagedEnvironment must be called first. func (j *Job) SetUpJob(ctx context.Context, image string, envMap map[string]string) error { + defer term.Timing()() if j.EnvironmentID == "" { - return errors.New("environment ID is not set; ensure SetUpEnvironment was called first") + return errors.New("environment ID is not set; ensure SetUpManagedEnvironment was called first") } - term.Debugf("SetUpJob: creating/updating job %q with image %q (%d env vars)", cdJobName, image, len(envMap)) jobsClient, err := j.newJobsClient() if err != nil { return err } + // Skip the upsert (which polls for ~30s even when nothing changed) when + // the job already exists with a populated system-assigned principal. + // Image and env vars on the template are placeholders — StartJobExecution + // overrides them per-run — so we don't need to compare them here. + if envMap == nil { + if resp, err := jobsClient.Get(ctx, j.ResourceGroup, cdJobName, nil); err == nil && + resp.Identity != nil && resp.Identity.PrincipalID != nil && *resp.Identity.PrincipalID != "" { + j.SystemPrincipalID = *resp.Identity.PrincipalID + term.Debugf("SetUpJob: %q already exists; skipping upsert", cdJobName) + return nil + } + } + + term.Debugf("SetUpJob: creating/updating job %q with image %q (%d env vars)", cdJobName, image, len(envMap)) + var envVars []*armappcontainersv3.EnvironmentVar for k, v := range envMap { envVars = append(envVars, &armappcontainersv3.EnvironmentVar{ @@ -407,13 +425,13 @@ func (j *Job) SetUpJob(ctx context.Context, image string, envMap map[string]stri if result.Identity != nil && result.Identity.PrincipalID != nil { j.SystemPrincipalID = *result.Identity.PrincipalID } - j.cdJobImage = image return nil } // StartJobExecution starts a new execution of the CD job with the given image, command, // and environment variables. Returns the execution name. func (j *Job) StartJobExecution(ctx context.Context, req JobRequest) (string, error) { + defer term.Timing()() jobsClient, err := j.newJobsClient() if err != nil { return "", err @@ -510,6 +528,7 @@ func (j *Job) StartJobExecution(ctx context.Context, req JobRequest) (string, er // GetJobExecutionStatus returns the current status of a job execution by listing executions // and finding the one with the given name. func (j *Job) GetJobExecutionStatus(ctx context.Context, executionName string) (*JobStatus, error) { + defer term.Timing()() execClient, err := j.newJobsExecutionsClient() if err != nil { return nil, err @@ -634,6 +653,7 @@ func forwardStream(ctx context.Context, ch <-chan LogEntry, yield func(string, e // logStreamEndpoint. The token differs from the ARM token and is required even // though the URL is already scoped to the subscription. func (j *Job) getJobAuthToken(ctx context.Context) (string, error) { + defer term.Timing()() return j.FetchLogStreamAuthToken(ctx, j.ResourceGroup, "Microsoft.App/jobs/"+cdJobName, jobAPIVersion) } @@ -643,6 +663,7 @@ func (j *Job) getJobAuthToken(ctx context.Context) (string, error) { // (Running or Terminated — anything earlier returns a Kubernetes error). // Returns an empty string (no error) while the replica is still initialising. func (j *Job) getCDContainerLogStreamURL(ctx context.Context, executionName string) (string, error) { + defer term.Timing()() armTok, err := j.ArmToken(ctx) if err != nil { return "", err @@ -706,6 +727,7 @@ func (j *Job) getCDContainerLogStreamURL(ctx context.Context, executionName stri // first connect to capture output emitted during pod startup, and 0 on // reconnects so we don't re-print lines we already streamed. func (j *Job) streamJobExecutionLogs(ctx context.Context, executionName string, backfillLines int) (<-chan LogEntry, error) { + defer term.Timing()() streamURL, err := j.getCDContainerLogStreamURL(ctx, executionName) if err != nil { return nil, err @@ -772,11 +794,13 @@ func (j *Job) streamJobExecutionLogs(ctx context.Context, executionName string, // ReadJobLogs returns all log output captured for a job execution from Log Analytics. // Subject to a short ingestion delay (seconds to a couple of minutes on cold workspaces). func (j *Job) ReadJobLogs(ctx context.Context, executionName string) (string, error) { + defer term.Timing()() return j.fetchLogsFromWorkspace(ctx, executionName) } // getLogAnalyticsToken returns a Bearer token for the Log Analytics query API. func (j *Job) getLogAnalyticsToken(ctx context.Context) (string, error) { + defer term.Timing()() cred, err := j.NewCreds() if err != nil { return "", err @@ -793,6 +817,7 @@ func (j *Job) getLogAnalyticsToken(ctx context.Context) (string, error) { // getLogWorkspaceCustomerID returns the customer ID (GUID) of the CD Log Analytics // workspace. This is what the Log Analytics query API addresses workspaces by. func (j *Job) getLogWorkspaceCustomerID(ctx context.Context) (string, error) { + defer term.Timing()() cred, err := j.NewCreds() if err != nil { return "", err @@ -815,6 +840,7 @@ func (j *Job) getLogWorkspaceCustomerID(ctx context.Context) (string, error) { // the given job execution, ordered by time. Returns empty string when the workspace has // no rows yet (first-time workspaces can take 2–5 minutes to ingest data). func (j *Job) fetchLogsFromWorkspace(ctx context.Context, executionName string) (string, error) { + defer term.Timing()() workspaceID, err := j.getLogWorkspaceCustomerID(ctx) if err != nil { return "", err @@ -826,6 +852,7 @@ func (j *Job) fetchLogsFromWorkspace(ctx context.Context, executionName string) // so tests can exercise it with a known workspace ID without needing the SDK // workspaces client to be mocked. func (j *Job) fetchLogsByWorkspaceID(ctx context.Context, workspaceID, executionName string) (string, error) { + defer term.Timing()() token, err := j.getLogAnalyticsToken(ctx) if err != nil { return "", err diff --git a/src/pkg/clouds/azure/aca/job_test.go b/src/pkg/clouds/azure/aca/job_test.go index a3b2acc15..a724cf1b6 100644 --- a/src/pkg/clouds/azure/aca/job_test.go +++ b/src/pkg/clouds/azure/aca/job_test.go @@ -82,7 +82,7 @@ func TestJobStatusIsSuccess(t *testing.T) { } func TestForwardStream(t *testing.T) { - ctx := context.Background() + ctx := t.Context() ch := make(chan LogEntry, 3) ch <- LogEntry{Message: "a"} ch <- LogEntry{Message: "b"} @@ -109,7 +109,7 @@ func TestForwardStream(t *testing.T) { } func TestForwardStreamEmpty(t *testing.T) { - ctx := context.Background() + ctx := t.Context() ch := make(chan LogEntry) close(ch) gotLines, keepGoing := forwardStream(ctx, ch, func(string, error) bool { return true }) @@ -122,7 +122,7 @@ func TestForwardStreamEmpty(t *testing.T) { } func TestForwardStreamErrorEntry(t *testing.T) { - ctx := context.Background() + ctx := t.Context() ch := make(chan LogEntry, 2) ch <- LogEntry{Err: context.Canceled} ch <- LogEntry{Message: "after err"} @@ -153,7 +153,7 @@ func TestForwardStreamErrorEntry(t *testing.T) { } func TestForwardStreamEarlyExit(t *testing.T) { - ctx := context.Background() + ctx := t.Context() ch := make(chan LogEntry, 3) ch <- LogEntry{Message: "a"} ch <- LogEntry{Message: "b"} @@ -174,7 +174,7 @@ func TestForwardStreamEarlyExit(t *testing.T) { } func TestForwardStreamCancelledContext(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) cancel() ch := make(chan LogEntry, 1) @@ -210,7 +210,7 @@ func TestGetJobAuthToken(t *testing.T) { useTestEndpoints(t, srv.URL, "") j := newTestJob() - tok, err := j.getJobAuthToken(context.Background()) + tok, err := j.getJobAuthToken(t.Context()) if err != nil { t.Fatalf("getJobAuthToken: %v", err) } @@ -236,7 +236,7 @@ func TestGetCDContainerLogStreamURLRunning(t *testing.T) { useTestEndpoints(t, srv.URL, "") j := newTestJob() - url, err := j.getCDContainerLogStreamURL(context.Background(), "exec-1") + url, err := j.getCDContainerLogStreamURL(t.Context(), "exec-1") if err != nil { t.Fatalf("getCDContainerLogStreamURL: %v", err) } @@ -259,7 +259,7 @@ func TestGetCDContainerLogStreamURLWaiting(t *testing.T) { useTestEndpoints(t, srv.URL, "") j := newTestJob() - url, err := j.getCDContainerLogStreamURL(context.Background(), "exec-1") + url, err := j.getCDContainerLogStreamURL(t.Context(), "exec-1") if err != nil { t.Fatalf("unexpected err: %v", err) } @@ -279,7 +279,7 @@ func TestGetCDContainerLogStreamURLMissingContainer(t *testing.T) { useTestEndpoints(t, srv.URL, "") j := newTestJob() - url, err := j.getCDContainerLogStreamURL(context.Background(), "exec-1") + url, err := j.getCDContainerLogStreamURL(t.Context(), "exec-1") if err != nil { t.Fatalf("unexpected err: %v", err) } @@ -298,7 +298,7 @@ func TestGetCDContainerLogStreamURLHTTPError(t *testing.T) { useTestEndpoints(t, srv.URL, "") j := newTestJob() - if _, err := j.getCDContainerLogStreamURL(context.Background(), "exec-1"); err == nil { + if _, err := j.getCDContainerLogStreamURL(t.Context(), "exec-1"); err == nil { t.Error("expected error for 401") } } @@ -315,7 +315,7 @@ func TestStreamJobExecutionLogsNoReplica(t *testing.T) { useTestEndpoints(t, srv.URL, "") j := newTestJob() - if _, err := j.streamJobExecutionLogs(context.Background(), "exec-1", 0); err == nil { + if _, err := j.streamJobExecutionLogs(t.Context(), "exec-1", 0); err == nil { t.Error("expected error when no replica") } } @@ -354,7 +354,7 @@ func TestStreamJobExecutionLogs(t *testing.T) { useTestEndpoints(t, mgmtSrv.URL, "") j := newTestJob() - ch, err := j.streamJobExecutionLogs(context.Background(), "exec-1", 0) + ch, err := j.streamJobExecutionLogs(t.Context(), "exec-1", 0) if err != nil { t.Fatalf("streamJobExecutionLogs: %v", err) } @@ -397,7 +397,7 @@ func TestStreamJobExecutionLogsBackfill(t *testing.T) { useTestEndpoints(t, mgmtSrv.URL, "") j := newTestJob() - ch, err := j.streamJobExecutionLogs(context.Background(), "exec-1", 250) + ch, err := j.streamJobExecutionLogs(t.Context(), "exec-1", 250) if err != nil { t.Fatalf("streamJobExecutionLogs: %v", err) } @@ -431,7 +431,7 @@ func TestStreamJobExecutionLogsHTTPFailure(t *testing.T) { useTestEndpoints(t, mgmtSrv.URL, "") j := newTestJob() - if _, err := j.streamJobExecutionLogs(context.Background(), "exec-1", 0); err == nil { + if _, err := j.streamJobExecutionLogs(t.Context(), "exec-1", 0); err == nil { t.Error("expected error for 403 from stream endpoint") } } @@ -450,7 +450,7 @@ func TestStreamJobExecutionLogsCredError(t *testing.T) { j := newTestJob() // First call (replicas list) goes through ArmToken → fails. - if _, err := j.streamJobExecutionLogs(context.Background(), "exec-1", 0); err == nil { + if _, err := j.streamJobExecutionLogs(t.Context(), "exec-1", 0); err == nil { t.Error("expected credential error") } } @@ -484,7 +484,7 @@ func TestReadJobLogs(t *testing.T) { j := newTestJob() // Call the low-level fetch function directly to avoid the SDK workspace lookup. - got, err := j.fetchLogsByWorkspaceID(context.Background(), "workspace-guid", "exec-1") + got, err := j.fetchLogsByWorkspaceID(t.Context(), "workspace-guid", "exec-1") if err != nil { t.Fatalf("fetchLogsByWorkspaceID: %v", err) } @@ -496,7 +496,7 @@ func TestReadJobLogs(t *testing.T) { func TestReadJobLogsTokenError(t *testing.T) { useFakeCred(t, "", errors.New("token denied")) j := newTestJob() - if _, err := j.fetchLogsByWorkspaceID(context.Background(), "ws", "exec"); err == nil { + if _, err := j.fetchLogsByWorkspaceID(t.Context(), "ws", "exec"); err == nil { t.Error("expected token error") } } @@ -511,7 +511,7 @@ func TestReadJobLogsHTTPError(t *testing.T) { useTestEndpoints(t, "http://unused", laSrv.URL) j := newTestJob() - if _, err := j.fetchLogsByWorkspaceID(context.Background(), "ws", "exec"); err == nil { + if _, err := j.fetchLogsByWorkspaceID(t.Context(), "ws", "exec"); err == nil { t.Error("expected error for 500") } } @@ -526,7 +526,7 @@ func TestReadJobLogsBadJSON(t *testing.T) { useTestEndpoints(t, "http://unused", laSrv.URL) j := newTestJob() - if _, err := j.fetchLogsByWorkspaceID(context.Background(), "ws", "exec"); err == nil { + if _, err := j.fetchLogsByWorkspaceID(t.Context(), "ws", "exec"); err == nil { t.Error("expected decode error") } } @@ -549,27 +549,27 @@ func TestSetUpManagedIdentityPreconditions(t *testing.T) { useFakeCred(t, "tok", nil) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} // SystemPrincipalID is not set. - if err := j.SetUpManagedIdentity(context.Background(), "acct"); err == nil { + if err := j.SetUpManagedIdentity(t.Context(), "acct"); err == nil { t.Error("expected error when SystemPrincipalID is empty") } // idempotent when identitySetUp is true. j.identitySetUp = true - if err := j.SetUpManagedIdentity(context.Background(), "acct"); err != nil { + if err := j.SetUpManagedIdentity(t.Context(), "acct"); err != nil { t.Errorf("identity already set up should short-circuit, got %v", err) } } func TestSetUpEnvironmentShortCircuit(t *testing.T) { j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg", EnvironmentID: "/already"} - if err := j.SetUpEnvironment(context.Background()); err != nil { - t.Errorf("SetUpEnvironment should short-circuit when EnvironmentID is set, got %v", err) + if err := j.SetUpManagedEnvironment(t.Context()); err != nil { + t.Errorf("SetUpManagedEnvironment should short-circuit when EnvironmentID is set, got %v", err) } } func TestSetUpJobMissingEnvironment(t *testing.T) { j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - if err := j.SetUpJob(context.Background(), "image", nil); err == nil { + if err := j.SetUpJob(t.Context(), "image", nil); err == nil { t.Error("SetUpJob should fail when EnvironmentID is empty") } } @@ -577,7 +577,7 @@ func TestSetUpJobMissingEnvironment(t *testing.T) { func TestStartJobExecutionCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - if _, err := j.StartJobExecution(context.Background(), JobRequest{ + if _, err := j.StartJobExecution(t.Context(), JobRequest{ Image: "img", Command: []string{"/bin/true"}, }); err == nil { @@ -588,7 +588,7 @@ func TestStartJobExecutionCredError(t *testing.T) { func TestTailJobLogsCancelled(t *testing.T) { useFakeCred(t, "", errors.New("denied")) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) cancel() // cancel immediately seq, err := j.TailJobLogs(ctx, "exec-1") @@ -603,7 +603,7 @@ func TestTailJobLogsCancelled(t *testing.T) { func TestGetJobExecutionStatusCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - if _, err := j.GetJobExecutionStatus(context.Background(), "exec"); err == nil { + if _, err := j.GetJobExecutionStatus(t.Context(), "exec"); err == nil { t.Error("expected cred error") } } @@ -611,7 +611,7 @@ func TestGetJobExecutionStatusCredError(t *testing.T) { func TestReadJobLogsCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - if _, err := j.ReadJobLogs(context.Background(), "exec"); err == nil { + if _, err := j.ReadJobLogs(t.Context(), "exec"); err == nil { t.Error("expected cred error") } } @@ -619,7 +619,7 @@ func TestReadJobLogsCredError(t *testing.T) { func TestGetLogAnalyticsTokenCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - if _, err := j.getLogAnalyticsToken(context.Background()); err == nil { + if _, err := j.getLogAnalyticsToken(t.Context()); err == nil { t.Error("expected cred error") } } @@ -627,15 +627,7 @@ func TestGetLogAnalyticsTokenCredError(t *testing.T) { func TestGetLogWorkspaceCustomerIDCredError(t *testing.T) { useFakeCred(t, "", errors.New("denied")) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - if _, err := j.getLogWorkspaceCustomerID(context.Background()); err == nil { - t.Error("expected cred error") - } -} - -func TestSetUpLogWorkspaceCredError(t *testing.T) { - useFakeCred(t, "", errors.New("denied")) - j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - if _, _, err := j.setUpLogWorkspace(context.Background()); err == nil { + if _, err := j.getLogWorkspaceCustomerID(t.Context()); err == nil { t.Error("expected cred error") } } @@ -645,7 +637,7 @@ func TestFetchLogsFromWorkspaceSDKError(t *testing.T) { j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} // fetchLogsFromWorkspace first calls getLogWorkspaceCustomerID which uses // the SDK (will fail), then bails. - if _, err := j.fetchLogsFromWorkspace(context.Background(), "exec"); err == nil { + if _, err := j.fetchLogsFromWorkspace(t.Context(), "exec"); err == nil { t.Error("expected error from fetchLogsFromWorkspace") } } @@ -655,7 +647,7 @@ func TestFetchLogsFromWorkspaceViaTailJobLogs(t *testing.T) { // terminal status and no logs. useFakeCred(t, "", errors.New("denied")) j := &Job{Azure: cloudazure.Azure{SubscriptionID: "sub"}, ResourceGroup: "rg"} - ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + ctx, cancel := context.WithTimeout(t.Context(), 500*time.Millisecond) defer cancel() seq, err := j.TailJobLogs(ctx, "exec") if err != nil { diff --git a/src/pkg/clouds/azure/cd/blob.go b/src/pkg/clouds/azure/cd/blob.go index 922780c64..97608a2e9 100644 --- a/src/pkg/clouds/azure/cd/blob.go +++ b/src/pkg/clouds/azure/cd/blob.go @@ -24,8 +24,15 @@ func (b BlobItem) Name() string { return b.name } func (b BlobItem) Size() int64 { return b.size } func (d *Driver) newSharedKeyCredential(ctx context.Context) (*azblob.SharedKeyCredential, error) { - storageKey := os.Getenv("AZURE_STORAGE_KEY") - if storageKey == "" { + // Lazy init of storageKey can be racy when callers (e.g. CdList's worker + // pool) call this concurrently. Hold the mutex across the ListKeys call so + // only one goroutine fetches the key. + d.storageKeyMu.Lock() + defer d.storageKeyMu.Unlock() + if d.storageKey == "" { + d.storageKey = os.Getenv("AZURE_STORAGE_KEY") + } + if d.storageKey == "" { accountsClient, err := d.NewStorageAccountsClient() if err != nil { return nil, err @@ -37,9 +44,9 @@ func (d *Driver) newSharedKeyCredential(ctx context.Context) (*azblob.SharedKeyC if len(keys.Keys) == 0 || keys.Keys[0].Value == nil { return nil, errors.New("no storage account keys returned") } - storageKey = *keys.Keys[0].Value + d.storageKey = *keys.Keys[0].Value } - return azblob.NewSharedKeyCredential(d.StorageAccount, storageKey) + return azblob.NewSharedKeyCredential(d.StorageAccount, d.storageKey) } func (d *Driver) newBlobContainerClient(ctx context.Context, containerName string) (*container.Client, error) { @@ -51,8 +58,12 @@ func (d *Driver) newBlobContainerClient(ctx context.Context, containerName strin return container.NewClientWithSharedKeyCredential(containerURL, keyCred, nil) } -// IterateBlobsInContainer is the container-explicit variant of IterateBlobs. -func (d *Driver) IterateBlobsInContainer(ctx context.Context, containerName, prefix string) (iter.Seq2[BlobItem, error], error) { +func (d *Driver) IterateBlobs(ctx context.Context, prefix string) (iter.Seq2[BlobItem, error], error) { + return d.iterateBlobsInContainer(ctx, d.BlobContainerName, prefix) +} + +// iterateBlobsInContainer is the container-explicit variant of IterateBlobs. +func (d *Driver) iterateBlobsInContainer(ctx context.Context, containerName, prefix string) (iter.Seq2[BlobItem, error], error) { client, err := d.newBlobContainerClient(ctx, containerName) if err != nil { return nil, err @@ -83,8 +94,12 @@ func (d *Driver) IterateBlobsInContainer(ctx context.Context, containerName, pre }, nil } -// DownloadBlobFromContainer is the container-explicit variant of DownloadBlob. -func (d *Driver) DownloadBlobFromContainer(ctx context.Context, containerName, blobName string) ([]byte, error) { +func (d *Driver) DownloadBlob(ctx context.Context, blobName string) ([]byte, error) { + return d.downloadBlobFromContainer(ctx, d.BlobContainerName, blobName) +} + +// downloadBlobFromContainer is the container-explicit variant of DownloadBlob. +func (d *Driver) downloadBlobFromContainer(ctx context.Context, containerName, blobName string) ([]byte, error) { client, err := d.newBlobContainerClient(ctx, containerName) if err != nil { return nil, err diff --git a/src/pkg/clouds/azure/cd/driver.go b/src/pkg/clouds/azure/cd/driver.go index c9fa5c720..b60a12e49 100644 --- a/src/pkg/clouds/azure/cd/driver.go +++ b/src/pkg/clouds/azure/cd/driver.go @@ -2,6 +2,7 @@ package cd import ( "fmt" + "sync" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources/v2" "github.com/DefangLabs/defang/src/pkg/clouds/azure" @@ -11,6 +12,8 @@ type Driver struct { azure.Azure resourceGroupPrefix string resourceGroupName string + storageKeyMu sync.Mutex // guards storageKey + storageKey string StorageAccount string BlobContainerName string } diff --git a/src/pkg/clouds/azure/cd/driver_test.go b/src/pkg/clouds/azure/cd/driver_test.go index d11bcd36b..786315de3 100644 --- a/src/pkg/clouds/azure/cd/driver_test.go +++ b/src/pkg/clouds/azure/cd/driver_test.go @@ -286,7 +286,7 @@ func TestIterateBlobsCredError(t *testing.T) { d.SubscriptionID = "sub" d.StorageAccount = "acct" d.BlobContainerName = "uploads" - if _, err := d.IterateBlobsInContainer(t.Context(), "uploads", ".pulumi/stacks/"); err == nil { + if _, err := d.iterateBlobsInContainer(t.Context(), "uploads", ".pulumi/stacks/"); err == nil { t.Error("IterateBlobs should fail without key") } } @@ -297,7 +297,7 @@ func TestDownloadBlobCredError(t *testing.T) { d.SubscriptionID = "sub" d.StorageAccount = "acct" d.BlobContainerName = "uploads" - if _, err := d.DownloadBlobFromContainer(t.Context(), "uploads", "blob"); err == nil { + if _, err := d.downloadBlobFromContainer(t.Context(), "uploads", "blob"); err == nil { t.Error("DownloadBlob should fail without key") } } diff --git a/src/pkg/clouds/azure/cd/setup.go b/src/pkg/clouds/azure/cd/setup.go index d3cdaa49b..912fad2d3 100644 --- a/src/pkg/clouds/azure/cd/setup.go +++ b/src/pkg/clouds/azure/cd/setup.go @@ -21,25 +21,22 @@ const storageAccountPrefix = "defangcd" // Container names used in the CD storage account. Keep them DNS-safe: // 3–63 chars, lowercase alphanumeric + hyphens (no leading/trailing hyphen). const ( - // UploadsContainerName holds per-deploy payloads (etag blobs) and source tarballs. - UploadsContainerName = "uploads" - // PulumiContainerName is the dedicated Pulumi state backend container. - PulumiContainerName = "pulumi" - // ProjectsContainerName holds {project}/{stack}/project.pb audit blobs - // written by the CD task before each deploy. - ProjectsContainerName = "projects" - - // blobContainerName is kept for backward compatibility with existing - // callers that default to the uploads container. - blobContainerName = UploadsContainerName + // pulumiContainerName is the dedicated Pulumi state backend container. + pulumiContainerName = "pulumi" + // projectsContainerName holds {project}/{stack}/project.pb audit blobs + // written by the CD task after each deploy. + projectsContainerName = "projects" ) // CreateResourceGroup creates or updates an Azure resource group with the given name. func (d *Driver) CreateResourceGroup(ctx context.Context, name string) error { + defer term.Timing()() + term.Debugf("Creating or updating resource group %q in %q", name, d.Location) rgClient, err := d.newResourceGroupClient() if err != nil { return err } + _, err = rgClient.CreateOrUpdate(ctx, name, armresources.ResourceGroup{ Location: d.Location.Ptr(), }, nil) @@ -51,10 +48,12 @@ func (d *Driver) CreateResourceGroup(ctx context.Context, name string) error { // SetUpResourceGroup creates or updates the shared CD resource group (defang-cd-{location}). func (d *Driver) SetUpResourceGroup(ctx context.Context) error { + defer term.Timing()() return d.CreateResourceGroup(ctx, d.resourceGroupName) } func (d *Driver) TearDown(ctx context.Context) error { + defer term.Timing()() rgClient, err := d.newResourceGroupClient() if err != nil { return err @@ -68,6 +67,7 @@ func (d *Driver) TearDown(ctx context.Context) error { } func (d *Driver) getStorageAccount(ctx context.Context, accountsClient *armstorage.AccountsClient) (string, error) { + defer term.Timing()() if d.StorageAccount != "" { return d.StorageAccount, nil } @@ -76,6 +76,7 @@ func (d *Driver) getStorageAccount(ctx context.Context, accountsClient *armstora return sa, nil } + term.Debugf("getStorageAccount from resource group %q", d.resourceGroupName) for pager := accountsClient.NewListByResourceGroupPager(d.resourceGroupName, nil); pager.More(); { page, err := pager.NextPage(ctx) if err != nil { @@ -90,13 +91,48 @@ func (d *Driver) getStorageAccount(ctx context.Context, accountsClient *armstora return "", nil } +// resolveBlobContainer picks the blob container in use on the given storage +// account. It prefers the legacy `pulumi` container if it exists (carry-over +// from older installs where Pulumi state lived in its own container); +// otherwise it returns the canonical `projects` container, which now holds +// both Pulumi state and project.pb audit blobs. When create is true, the +// `projects` container is created if missing (idempotent — "already exists" +// is treated as success). +func (d *Driver) resolveBlobContainer(ctx context.Context, storageAccount string, create bool) (string, error) { + containerClient, err := d.NewBlobContainersClient() + if err != nil { + return "", err + } + name := pulumiContainerName + if _, err := containerClient.Get(ctx, d.resourceGroupName, storageAccount, name, nil); err != nil { + var respErr *azcore.ResponseError + if !errors.As(err, &respErr) || respErr.StatusCode != 404 { + return "", fmt.Errorf("failed to look up blob container %q: %w", name, err) + } + name = projectsContainerName + if create { + term.Debugf("Create blob container %q", name) + if _, err := containerClient.Create(ctx, d.resourceGroupName, storageAccount, name, armstorage.BlobContainer{}, nil); err != nil { + var respErr *azcore.ResponseError + if !errors.As(err, &respErr) || respErr.ErrorCode != "ContainerAlreadyExists" { + return "", fmt.Errorf("failed to create blob container %q: %w", name, err) + } + } + } + } + return name, nil +} + // FindStorageAccount is a read-only variant of SetUpStorageAccount: it locates // the defang CD storage account (and remembers its container) without -// creating anything. Returns ("", nil) when the storage account or blob -// container doesn't exist yet — typical for a subscription where defang has -// never been deployed. On success, d.StorageAccount and d.BlobContainerName -// are populated for subsequent DownloadBlob / IterateBlobs calls. +// creating anything. Returns ("", nil) when the storage account doesn't exist +// yet — typical for a subscription where defang has never been deployed. On +// success, d.StorageAccount and d.BlobContainerName are populated for +// subsequent DownloadBlob / IterateBlobs calls. The `projects` container is +// not verified — downstream blob ops will return 404 if it hasn't been +// created yet, which callers already handle. func (d *Driver) FindStorageAccount(ctx context.Context) (string, error) { + defer term.Timing()() if d.StorageAccount != "" && d.BlobContainerName != "" { return d.StorageAccount, nil } @@ -115,12 +151,12 @@ func (d *Driver) FindStorageAccount(ctx context.Context) (string, error) { if storageAccount == "" { return "", nil } + name, err := d.resolveBlobContainer(ctx, storageAccount, false) + if err != nil { + return "", err + } d.StorageAccount = storageAccount - // The blob container is always created with the well-known name; its - // existence is implied by the storage account being present on a - // defang-managed subscription. We don't verify it here — DownloadBlob / - // IterateBlobs will return 404 if it doesn't exist yet. - d.BlobContainerName = blobContainerName + d.BlobContainerName = name return storageAccount, nil } @@ -157,21 +193,13 @@ func (d *Driver) SetUpStorageAccount(ctx context.Context) (string, error) { } d.StorageAccount = storageAccount - containerClient, err := d.NewBlobContainersClient() + name, err := d.resolveBlobContainer(ctx, storageAccount, true) if err != nil { - return "", fmt.Errorf("failed to create blob containers client: %w", err) - } - for _, name := range []string{UploadsContainerName, PulumiContainerName, ProjectsContainerName} { - if _, err := containerClient.Create(ctx, d.resourceGroupName, storageAccount, name, armstorage.BlobContainer{}, nil); err != nil { - var respErr *azcore.ResponseError - if !errors.As(err, &respErr) || respErr.ErrorCode != "ContainerAlreadyExists" { - return "", fmt.Errorf("failed to create blob container %q: %w", name, err) - } - } + return "", err } - d.BlobContainerName = UploadsContainerName + d.BlobContainerName = name - term.Infof("Using storage account %s (containers: %s, %s, %s)", storageAccount, UploadsContainerName, PulumiContainerName, ProjectsContainerName) + term.Infof("Using storage account %s container %s", storageAccount, name) return storageAccount, nil } diff --git a/src/pkg/clouds/azure/cd/upload.go b/src/pkg/clouds/azure/cd/upload.go index 1e4b6d8b8..dd95d3593 100644 --- a/src/pkg/clouds/azure/cd/upload.go +++ b/src/pkg/clouds/azure/cd/upload.go @@ -5,15 +5,15 @@ import ( "errors" "fmt" "net/url" - "os" "strings" "time" - "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/sas" "github.com/google/uuid" ) +const prefix = "uploads/" + func (d *Driver) CreateUploadURL(ctx context.Context, blobName string) (string, error) { if blobName == "" { blobName = uuid.NewString() @@ -24,29 +24,15 @@ func (d *Driver) CreateUploadURL(ctx context.Context, blobName string) (string, // Sanitize the digest so it's safe to use as a file name blobName = strings.ReplaceAll(blobName, "/", "_") } + blobName = prefix + blobName + if _, err := d.SetUpStorageAccount(ctx); err != nil { return "", err } expiry := time.Now().UTC().Add(1 * time.Hour) - storageKey := os.Getenv("AZURE_STORAGE_KEY") - if storageKey == "" { - accountsClient, err := d.NewStorageAccountsClient() - if err != nil { - return "", err - } - keys, err := accountsClient.ListKeys(ctx, d.resourceGroupName, d.StorageAccount, nil) - if err != nil { - return "", err - } - if len(keys.Keys) == 0 || keys.Keys[0].Value == nil { - return "", errors.New("no storage account keys returned") - } - storageKey = *keys.Keys[0].Value - } - - keyCred, err := azblob.NewSharedKeyCredential(d.StorageAccount, storageKey) + keyCred, err := d.newSharedKeyCredential(ctx) if err != nil { return "", err } diff --git a/src/pkg/clouds/azure/common.go b/src/pkg/clouds/azure/common.go index 937e11b2d..cd575b1ec 100644 --- a/src/pkg/clouds/azure/common.go +++ b/src/pkg/clouds/azure/common.go @@ -16,7 +16,7 @@ import ( "github.com/DefangLabs/defang/src/pkg/tokenstore" ) -var PollOptions = &runtime.PollUntilDoneOptions{Frequency: 5 * time.Second} +var PollOptions = &runtime.PollUntilDoneOptions{Frequency: 2 * time.Second} // cliTimeout overrides the default 10s timeout for CLI-based credentials. // The Azure CLI can be slow to start, especially when installed via Nix. diff --git a/src/pkg/stacks/manager.go b/src/pkg/stacks/manager.go index ce3d00f57..cb53d792e 100644 --- a/src/pkg/stacks/manager.go +++ b/src/pkg/stacks/manager.go @@ -121,9 +121,13 @@ func (sm *manager) ListRemote(ctx context.Context) ([]ListItem, error) { if params.Provider == "" { params.Provider.SetValue(stack.GetProvider()) } + account := params.Account() + if account == "" { + account = stack.GetProviderAccountId() + } stackParams = append(stackParams, ListItem{ Parameters: *params, - Account: params.Account(), + Account: account, DeployedAt: timeutils.AsTime(stack.GetLastDeployedAt(), time.Time{}).Local(), Default: stack.GetIsDefault(), }) diff --git a/src/pkg/stacks/manager_test.go b/src/pkg/stacks/manager_test.go index 4ca758f57..c72d9a8cd 100644 --- a/src/pkg/stacks/manager_test.go +++ b/src/pkg/stacks/manager_test.go @@ -783,7 +783,7 @@ func TestGetStack(t *testing.T) { }, }, interactiveResponses: map[string]string{ - "stack": "existingstack (gcp)", + "stack": "existingstack [gcp]", }, expectedStack: &Parameters{ Name: "existingstack", diff --git a/src/pkg/stacks/selector.go b/src/pkg/stacks/selector.go index c484a7672..d141013e4 100644 --- a/src/pkg/stacks/selector.go +++ b/src/pkg/stacks/selector.go @@ -5,9 +5,8 @@ import ( "errors" "fmt" "slices" - "strings" + "time" - "github.com/DefangLabs/defang/src/pkg/cli/client" "github.com/DefangLabs/defang/src/pkg/elicitations" "github.com/DefangLabs/defang/src/pkg/term" ) @@ -54,7 +53,7 @@ func (ss *stackSelector) SelectStack(ctx context.Context, opts SelectStackOption return nil, errors.New("no stacks available to select in this workspace") } } - labelMap := MakeStackSelectorLabels(stackList) + labelMap := makeStackSelectorLabels(stackList) stackLabels := make([]string, 0, len(stackList)+1) stackNames := make([]string, 0, len(stackList)) for _, stack := range stackList { @@ -126,7 +125,7 @@ func printStacksInfoMessage(stacks []string) { term.Printf("To skip this prompt, run this command with --stack=%s\n", "") } -func MakeStackSelectorLabels(stacks []ListItem) map[string]string { +func makeStackSelectorLabels(stacks []ListItem) map[string]string { partsList := stackLabelParts(stacks) partsList = reduceStackLabelParts(partsList) @@ -139,28 +138,19 @@ func MakeStackSelectorLabels(stacks []ListItem) map[string]string { } func stackLabelParts(stacks []ListItem) [][]string { - partsList := make([][]string, 0, len(stacks)) - for _, s := range stacks { + partsList := make([][]string, len(stacks)) + for i, s := range stacks { var deployedAt string if !s.DeployedAt.IsZero() { - deployedAt = "last deployed " + s.DeployedAt.Format("Jan 2 2006") + deployedAt = "last deployed " + s.DeployedAt.Format(time.UnixDate) } - provider := s.Provider - account := "" - switch provider { - case client.ProviderAWS: - account, _ = s.Variables["AWS_PROFILE"] - case client.ProviderGCP: - account, _ = s.Variables["GCP_PROJECT_ID"] - } - parts := []string{ + partsList[i] = []string{ s.Name, - provider.String(), + s.Provider.String(), + s.Account, s.Region, - account, deployedAt, } - partsList = append(partsList, parts) } return partsList } @@ -205,5 +195,5 @@ func formatStackLabelParts(parts []string) string { if len(nonEmptyParts) == 1 { return nonEmptyParts[0] } - return fmt.Sprintf("%s (%s)", nonEmptyParts[0], strings.Join(nonEmptyParts[1:], ", ")) + return fmt.Sprintf("%s %v", nonEmptyParts[0], nonEmptyParts[1:]) } diff --git a/src/pkg/stacks/selector_test.go b/src/pkg/stacks/selector_test.go index aa8bbf585..029758102 100644 --- a/src/pkg/stacks/selector_test.go +++ b/src/pkg/stacks/selector_test.go @@ -100,8 +100,8 @@ func TestStackSelector_SelectStack_ExistingStack(t *testing.T) { mockSM.On("List", ctx).Return(existingStacks, nil) // Mock user selecting existing stack - expectedOptions := []string{"production (us-west-2)", "development (us-east-1)"} - mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return("production (us-west-2)", nil) + expectedOptions := []string{"production [us-west-2]", "development [us-east-1]"} + mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return("production [us-west-2]", nil) // Expected params based on ToParameters() conversion expectedParams := &Parameters{ @@ -139,8 +139,8 @@ func TestStackSelector_SelectOrCreateStack_ExistingStack(t *testing.T) { mockSM.On("List", ctx).Return(existingStacks, nil) // Mock user selecting existing stack - expectedOptions := []string{"production (us-west-2)", "development (us-east-1)", CreateNewStack} - mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return("production (us-west-2)", nil) + expectedOptions := []string{"production [us-west-2]", "development [us-east-1]", CreateNewStack} + mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return("production [us-west-2]", nil) // Expected params based on ToParameters() conversion expectedParams := &Parameters{ @@ -179,7 +179,7 @@ func TestStackSelector_SelectStack_CreateNewStack(t *testing.T) { mockSM.On("List", ctx).Return(existingStacks, nil) // Mock user selecting to create new stack - expectedOptions := []string{"production (aws, us-west-2)", CreateNewStack} + expectedOptions := []string{"production [aws us-west-2]", CreateNewStack} mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return(CreateNewStack, nil) // Mock wizard parameter collection - provider selection @@ -383,7 +383,7 @@ func TestStackSelector_SelectStack_ElicitationError(t *testing.T) { mockSM.On("List", ctx).Return(existingStacks, nil) // Mock error during elicitation - expectedOptions := []string{"production (aws, us-west-2)"} + expectedOptions := []string{"production [aws us-west-2]"} mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return("", errors.New("user cancelled selection")) selector := NewSelector(mockEC, mockSM) @@ -414,7 +414,7 @@ func TestStackSelector_SelectStack_WizardError(t *testing.T) { mockSM.On("List", ctx).Return(existingStacks, nil) // Mock user selecting to create new stack - expectedOptions := []string{"production (aws, us-west-2)", CreateNewStack} + expectedOptions := []string{"production [aws us-west-2]", CreateNewStack} mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return(CreateNewStack, nil) // Mock wizard parameter collection - provider selection fails @@ -451,7 +451,7 @@ func TestStackSelector_SelectStack_CreateStackError(t *testing.T) { mockSM.On("List", ctx).Return(existingStacks, nil) // Mock user selecting to create new stack - expectedOptions := []string{"production (aws, us-west-2)", CreateNewStack} + expectedOptions := []string{"production [aws us-west-2]", CreateNewStack} mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return(CreateNewStack, nil) // Mock wizard parameter collection - provider selection @@ -528,20 +528,19 @@ func TestStackSelector_SelectStack_ShowsAccountInLabel(t *testing.T) { mockEC.On("IsSupported").Return(true) existingStacks := []ListItem{ - {Parameters: Parameters{Name: "prod", Provider: "aws", Region: "us-west-2", Variables: map[string]string{"AWS_PROFILE": "prod-account"}}}, - {Parameters: Parameters{Name: "dev", Provider: "aws", Region: "us-west-2", Variables: map[string]string{"AWS_PROFILE": "dev-account"}}}, + {Parameters: Parameters{Name: "prod", Provider: "aws", Region: "us-west-2"}, Account: "prod-account"}, + {Parameters: Parameters{Name: "dev", Provider: "aws", Region: "us-west-2"}, Account: "dev-account"}, } mockSM.On("List", ctx).Return(existingStacks, nil) // provider and region are redundant; only account differs - expectedOptions := []string{"prod (prod-account)", "dev (dev-account)"} - mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return("prod (prod-account)", nil) + expectedOptions := []string{"prod [prod-account]", "dev [dev-account]"} + mockEC.On("RequestEnum", ctx, "Select a stack", "stack", expectedOptions).Return("prod [prod-account]", nil) expectedParams := &Parameters{ - Name: "prod", - Provider: client.ProviderAWS, - Region: "us-west-2", - Variables: map[string]string{"AWS_PROFILE": "prod-account"}, + Name: "prod", + Provider: client.ProviderAWS, + Region: "us-west-2", } selector := NewSelector(mockEC, mockSM) @@ -570,14 +569,14 @@ func TestMakeStackSelectorLabels(t *testing.T) { stacks: []ListItem{ {Parameters: Parameters{Name: "production", Provider: "aws", Region: "us-west-2"}}, }, - wantLabels: []string{"production (aws, us-west-2)"}, + wantLabels: []string{"production [aws us-west-2]"}, }, { name: "one stack with AWS profile", stacks: []ListItem{ - {Parameters: Parameters{Name: "production", Provider: "aws", Region: "us-west-2", Variables: map[string]string{"AWS_PROFILE": "my-profile"}}}, + {Parameters: Parameters{Name: "production", Provider: "aws", Region: "us-west-2"}, Account: "my-profile"}, }, - wantLabels: []string{"production (aws, us-west-2, my-profile)"}, + wantLabels: []string{"production [aws my-profile us-west-2]"}, }, { name: "hide redundant provider", @@ -586,8 +585,8 @@ func TestMakeStackSelectorLabels(t *testing.T) { {Parameters: Parameters{Name: "development", Provider: "aws", Region: "us-east-1"}}, }, wantLabels: []string{ - "production (us-west-2)", - "development (us-east-1)", + "production [us-west-2]", + "development [us-east-1]", }, }, { @@ -609,48 +608,48 @@ func TestMakeStackSelectorLabels(t *testing.T) { {Parameters: Parameters{Name: "gcp-stack", Provider: "gcp", Region: "us-central1"}}, }, wantLabels: []string{ - "prod-us-west-2 (aws, us-west-2)", - "dev-us-east-1 (aws, us-east-1)", - "gcp-stack (gcp, us-central1)", + "prod-us-west-2 [aws us-west-2]", + "dev-us-east-1 [aws us-east-1]", + "gcp-stack [gcp us-central1]", }, }, { name: "show different AWS profiles", stacks: []ListItem{ - {Parameters: Parameters{Name: "prod", Provider: "aws", Region: "us-west-2", Variables: map[string]string{"AWS_PROFILE": "prod-account"}}}, - {Parameters: Parameters{Name: "dev", Provider: "aws", Region: "us-west-2", Variables: map[string]string{"AWS_PROFILE": "dev-account"}}}, + {Parameters: Parameters{Name: "prod", Provider: "aws", Region: "us-west-2"}, Account: "prod-account"}, + {Parameters: Parameters{Name: "dev", Provider: "aws", Region: "us-west-2"}, Account: "dev-account"}, }, wantLabels: []string{ - "prod (prod-account)", - "dev (dev-account)", + "prod [prod-account]", + "dev [dev-account]", }, }, { name: "hide redundant AWS profile", stacks: []ListItem{ - {Parameters: Parameters{Name: "prod", Provider: "aws", Region: "us-west-2", Variables: map[string]string{"AWS_PROFILE": "shared"}}}, - {Parameters: Parameters{Name: "dev", Provider: "aws", Region: "us-east-1", Variables: map[string]string{"AWS_PROFILE": "shared"}}}, + {Parameters: Parameters{Name: "prod", Provider: "aws", Region: "us-west-2"}, Account: "shared"}, + {Parameters: Parameters{Name: "dev", Provider: "aws", Region: "us-east-1"}, Account: "shared"}, }, wantLabels: []string{ - "prod (us-west-2)", - "dev (us-east-1)", + "prod [us-west-2]", + "dev [us-east-1]", }, }, { name: "show different GCP project IDs", stacks: []ListItem{ - {Parameters: Parameters{Name: "prod", Provider: "gcp", Region: "us-central1", Variables: map[string]string{"GCP_PROJECT_ID": "my-prod-project"}}}, - {Parameters: Parameters{Name: "dev", Provider: "gcp", Region: "us-central1", Variables: map[string]string{"GCP_PROJECT_ID": "my-dev-project"}}}, + {Parameters: Parameters{Name: "prod", Provider: "gcp", Region: "us-central1"}, Account: "my-prod-project"}, + {Parameters: Parameters{Name: "dev", Provider: "gcp", Region: "us-central1"}, Account: "my-dev-project"}, }, wantLabels: []string{ - "prod (my-prod-project)", - "dev (my-dev-project)", + "prod [my-prod-project]", + "dev [my-dev-project]", }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - labels := MakeStackSelectorLabels(tt.stacks) + labels := makeStackSelectorLabels(tt.stacks) // Extract labels into a slice for easier comparison var gotLabels []string diff --git a/src/pkg/stacks/stacks.go b/src/pkg/stacks/stacks.go index 5dad89c3f..420a4aadc 100644 --- a/src/pkg/stacks/stacks.go +++ b/src/pkg/stacks/stacks.go @@ -148,6 +148,8 @@ func (p *Parameters) Account() string { return p.Variables["AWS_PROFILE"] case client.ProviderGCP: return p.Variables["GCP_PROJECT_ID"] + case client.ProviderAzure: + return p.Variables["AZURE_SUBSCRIPTION_ID"] default: return "" } diff --git a/src/pkg/term/timing.go b/src/pkg/term/timing.go new file mode 100644 index 000000000..18a241285 --- /dev/null +++ b/src/pkg/term/timing.go @@ -0,0 +1,15 @@ +package term + +import ( + "runtime" + "time" +) + +func Timing() func() { + var name string + if pc, _, _, ok := runtime.Caller(1); ok { //nolint:dogsled + name = runtime.FuncForPC(pc).Name() + } + start := time.Now() + return func() { Debug("timing", time.Since(start), "in", name) } +} diff --git a/src/protos/io/defang/v1/fabric.pb.go b/src/protos/io/defang/v1/fabric.pb.go index 61793899a..60fd84b3b 100644 --- a/src/protos/io/defang/v1/fabric.pb.go +++ b/src/protos/io/defang/v1/fabric.pb.go @@ -538,7 +538,7 @@ const ( CdType_CD_TYPE_AWS_CODEBUILD_BUILDID CdType = 1 CdType_CD_TYPE_GCP_CLOUDBUILD_BUILDID CdType = 2 CdType_CD_TYPE_DO_APPPLATFORM_DEPLOYMENTID CdType = 3 - CdType_CD_TYPE_AZURE_ACI_JOBID CdType = 4 + CdType_CD_TYPE_AZURE_ACA_JOBID CdType = 4 ) // Enum value maps for CdType. @@ -548,14 +548,14 @@ var ( 1: "CD_TYPE_AWS_CODEBUILD_BUILDID", 2: "CD_TYPE_GCP_CLOUDBUILD_BUILDID", 3: "CD_TYPE_DO_APPPLATFORM_DEPLOYMENTID", - 4: "CD_TYPE_AZURE_ACI_JOBID", + 4: "CD_TYPE_AZURE_ACA_JOBID", } CdType_value = map[string]int32{ "CD_TYPE_UNSPECIFIED": 0, "CD_TYPE_AWS_CODEBUILD_BUILDID": 1, "CD_TYPE_GCP_CLOUDBUILD_BUILDID": 2, "CD_TYPE_DO_APPPLATFORM_DEPLOYMENTID": 3, - "CD_TYPE_AZURE_ACI_JOBID": 4, + "CD_TYPE_AZURE_ACA_JOBID": 4, } ) @@ -6515,7 +6515,7 @@ const file_io_defang_v1_fabric_proto_rawDesc = "" + "\x1dCD_TYPE_AWS_CODEBUILD_BUILDID\x10\x01\x12\"\n" + "\x1eCD_TYPE_GCP_CLOUDBUILD_BUILDID\x10\x02\x12'\n" + "#CD_TYPE_DO_APPPLATFORM_DEPLOYMENTID\x10\x03\x12\x1b\n" + - "\x17CD_TYPE_AZURE_ACI_JOBID\x10\x04*\x95\x01\n" + + "\x17CD_TYPE_AZURE_ACA_JOBID\x10\x04*\x95\x01\n" + "\x10DeploymentStatus\x12!\n" + "\x1dDEPLOYMENT_STATUS_UNSPECIFIED\x10\x00\x12!\n" + "\x1dDEPLOYMENT_STATUS_IN_PROGRESS\x10\x01\x12\x1d\n" + diff --git a/src/protos/io/defang/v1/fabric.proto b/src/protos/io/defang/v1/fabric.proto index aae3ef9ab..13b0c578d 100644 --- a/src/protos/io/defang/v1/fabric.proto +++ b/src/protos/io/defang/v1/fabric.proto @@ -588,7 +588,7 @@ enum CdType { CD_TYPE_AWS_CODEBUILD_BUILDID = 1; CD_TYPE_GCP_CLOUDBUILD_BUILDID = 2; CD_TYPE_DO_APPPLATFORM_DEPLOYMENTID = 3; - CD_TYPE_AZURE_ACI_JOBID = 4; + CD_TYPE_AZURE_ACA_JOBID = 4; } enum DeploymentStatus {