From 78ce16332a827e2eced41257a7a5328f1d396cf1 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Mon, 29 Jan 2024 17:14:11 -0800 Subject: [PATCH 01/10] WIP: Adds support for referencing existing images --- cli/azd/pkg/project/container_helper.go | 12 ++++++++ cli/azd/pkg/project/framework_service.go | 15 +++++++--- .../pkg/project/framework_service_docker.go | 30 ++++++++++++------- cli/azd/pkg/project/service_manager.go | 6 ++++ 4 files changed, 49 insertions(+), 14 deletions(-) diff --git a/cli/azd/pkg/project/container_helper.go b/cli/azd/pkg/project/container_helper.go index da6561599f5..7dd447f381c 100644 --- a/cli/azd/pkg/project/container_helper.go +++ b/cli/azd/pkg/project/container_helper.go @@ -162,6 +162,18 @@ func (ch *ContainerHelper) Deploy( localImageTag = packageDetails.ImageTag } + // If we don't have a registry specified and the service does not reference a project path + // then we are referencing a public/pre-existing image and don't have anything to tag or push + if loginServer == "" && serviceConfig.RelativePath != "" { + task.SetResult(&ServiceDeployResult{ + Package: packageOutput, + Details: &dockerDeployResult{ + RemoteImageTag: localImageTag, + }, + }) + return + } + if localImageTag == "" { task.SetError(errors.New("failed retrieving package result details")) return diff --git a/cli/azd/pkg/project/framework_service.go b/cli/azd/pkg/project/framework_service.go index c4c31fd926d..064dd4e80c2 100644 --- a/cli/azd/pkg/project/framework_service.go +++ b/cli/azd/pkg/project/framework_service.go @@ -15,6 +15,7 @@ import ( type ServiceLanguageKind string const ( + ServiceLanguageNone ServiceLanguageKind = "" ServiceLanguageDotNet ServiceLanguageKind = "dotnet" ServiceLanguageCsharp ServiceLanguageKind = "csharp" ServiceLanguageFsharp ServiceLanguageKind = "fsharp" @@ -25,18 +26,24 @@ const ( ServiceLanguageDocker ServiceLanguageKind = "docker" ) -func parseServiceLanguage(kind ServiceLanguageKind) (ServiceLanguageKind, error) { - if string(kind) == "" { - return ServiceLanguageKind(""), fmt.Errorf("language property must not be empty") +var ( + NoFrameworkRequirements = FrameworkRequirements{ + Package: FrameworkPackageRequirements{ + RequireRestore: false, + RequireBuild: false, + }, } +) +func parseServiceLanguage(kind ServiceLanguageKind) (ServiceLanguageKind, error) { // aliases if string(kind) == "py" { return ServiceLanguagePython, nil } switch kind { - case ServiceLanguageDotNet, + case ServiceLanguageNone, + ServiceLanguageDotNet, ServiceLanguageCsharp, ServiceLanguageFsharp, ServiceLanguageJavaScript, diff --git a/cli/azd/pkg/project/framework_service_docker.go b/cli/azd/pkg/project/framework_service_docker.go index d22bd66a373..742074ca1aa 100644 --- a/cli/azd/pkg/project/framework_service_docker.go +++ b/cli/azd/pkg/project/framework_service_docker.go @@ -31,6 +31,8 @@ import ( "go.opentelemetry.io/otel/trace" ) +var DefaultDockerProjectOptions DockerProjectOptions = DockerProjectOptions{} + type DockerProjectOptions struct { Path string `yaml:"path,omitempty" json:"path,omitempty"` Context string `yaml:"context,omitempty" json:"context,omitempty"` @@ -167,6 +169,12 @@ func (p *dockerProject) Build( func(task *async.TaskContextWithProgress[*ServiceBuildResult, ServiceProgress]) { dockerOptions := getDockerOptionsWithDefaults(serviceConfig.Docker) + // No framework has been set, return empty build result + if _, ok := p.framework.(*noOpProject); ok { + task.SetResult(&ServiceBuildResult{}) + return + } + buildArgs := []string{} for _, arg := range dockerOptions.BuildArgs { buildArgs = append(buildArgs, exec.RedactSensitiveData(arg)) @@ -257,10 +265,10 @@ func (p *dockerProject) Package( ) *async.TaskWithProgress[*ServicePackageResult, ServiceProgress] { return async.RunTaskWithProgress( func(task *async.TaskContextWithProgress[*ServicePackageResult, ServiceProgress]) { - imageId := buildOutput.BuildOutputPath - if imageId == "" { - task.SetError(errors.New("missing container image id from build output")) - return + var imageId string + + if buildOutput != nil { + imageId = buildOutput.BuildOutputPath } localTag, err := p.containerHelper.LocalImageTag(ctx, serviceConfig) @@ -269,12 +277,14 @@ func (p *dockerProject) Package( return } - // Tag image. - log.Printf("tagging image %s as %s", imageId, localTag) - task.SetProgress(NewServiceProgress("Tagging Docker image")) - if err := p.docker.Tag(ctx, serviceConfig.Path(), imageId, localTag); err != nil { - task.SetError(fmt.Errorf("tagging image: %w", err)) - return + if imageId != "" { + // Tag image. + log.Printf("tagging image %s as %s", imageId, localTag) + task.SetProgress(NewServiceProgress("Tagging Docker image")) + if err := p.docker.Tag(ctx, serviceConfig.Path(), imageId, localTag); err != nil { + task.SetError(fmt.Errorf("tagging image: %w", err)) + return + } } task.SetResult(&ServicePackageResult{ diff --git a/cli/azd/pkg/project/service_manager.go b/cli/azd/pkg/project/service_manager.go index 2049e853b89..9e76afb1ba1 100644 --- a/cli/azd/pkg/project/service_manager.go +++ b/cli/azd/pkg/project/service_manager.go @@ -9,6 +9,7 @@ import ( "log" "os" "path/filepath" + "reflect" "strings" "github.com/azure/azure-dev/cli/azd/pkg/alpha" @@ -551,6 +552,11 @@ func (sm *serviceManager) GetServiceTarget(ctx context.Context, serviceConfig *S func (sm *serviceManager) GetFrameworkService(ctx context.Context, serviceConfig *ServiceConfig) (FrameworkService, error) { var frameworkService FrameworkService + if serviceConfig.Language == ServiceLanguageNone && + !reflect.DeepEqual(serviceConfig.Docker, DefaultDockerProjectOptions) { + serviceConfig.Language = ServiceLanguageDocker + } + if err := sm.serviceLocator.ResolveNamed(string(serviceConfig.Language), &frameworkService); err != nil { panic(fmt.Errorf( "failed to resolve language '%s' for service '%s', %w", From 70e0090826b299e562a7b89590d34c86dc52319d Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Wed, 31 Jan 2024 13:13:21 -0800 Subject: [PATCH 02/10] Adds unit tests to support public image references --- cli/azd/pkg/project/container_helper.go | 82 ++++--- cli/azd/pkg/project/container_helper_test.go | 243 ++++++++++++++++++- cli/azd/pkg/project/service_manager_test.go | 50 +++- 3 files changed, 336 insertions(+), 39 deletions(-) diff --git a/cli/azd/pkg/project/container_helper.go b/cli/azd/pkg/project/container_helper.go index 7dd447f381c..92a047d6a89 100644 --- a/cli/azd/pkg/project/container_helper.go +++ b/cli/azd/pkg/project/container_helper.go @@ -60,7 +60,11 @@ func (ch *ContainerHelper) RegistryName(ctx context.Context, serviceConfig *Serv registryName = yamlRegistryName } - if registryName == "" { + // If the service provides its own code artifacts then the expectation is that an images needs to be built and + // pushed to a container registry. + // If the service does not provide its own code artifacts then the expectation is a registry is optional and + // an image can be referenced independently. + if serviceConfig.RelativePath != "" && registryName == "" { return "", fmt.Errorf( //nolint:lll "could not determine container registry endpoint, ensure 'registry' has been set in the docker options or '%s' environment variable has been set", @@ -81,6 +85,11 @@ func (ch *ContainerHelper) RemoteImageTag( return "", err } + // If no registry/login server is defined return the original tag + if loginServer == "" { + return localImageTag, nil + } + return fmt.Sprintf( "%s/%s", loginServer, @@ -122,7 +131,14 @@ func (ch *ContainerHelper) Login( return "", err } - return loginServer, ch.containerRegistryService.Login(ctx, targetResource.SubscriptionId(), loginServer) + // Only perform automatic login for ACR + // Other registries require manual login via external 'docker login' command + hostParts := strings.Split(loginServer, ".") + if len(hostParts) == 1 || strings.Contains(loginServer, "azurecr.io") { + return loginServer, ch.containerRegistryService.Login(ctx, targetResource.SubscriptionId(), loginServer) + } + + return loginServer, nil } func (ch *ContainerHelper) Credentials( @@ -150,7 +166,7 @@ func (ch *ContainerHelper) Deploy( return async.RunTaskWithProgress( func(task *async.TaskContextWithProgress[*ServiceDeployResult, ServiceProgress]) { // Get ACR Login Server - loginServer, err := ch.RegistryName(ctx, serviceConfig) + registryName, err := ch.RegistryName(ctx, serviceConfig) if err != nil { task.SetError(err) return @@ -164,7 +180,7 @@ func (ch *ContainerHelper) Deploy( // If we don't have a registry specified and the service does not reference a project path // then we are referencing a public/pre-existing image and don't have anything to tag or push - if loginServer == "" && serviceConfig.RelativePath != "" { + if registryName == "" && serviceConfig.RelativePath != "" { task.SetResult(&ServiceDeployResult{ Package: packageOutput, Details: &dockerDeployResult{ @@ -179,34 +195,42 @@ func (ch *ContainerHelper) Deploy( return } - // Tag image - // Get remote tag from the container helper then call docker cli tag command - remoteTag, err := ch.RemoteImageTag(ctx, serviceConfig, localImageTag) - if err != nil { - task.SetError(fmt.Errorf("getting remote image tag: %w", err)) - return - } + // Default to the local image tag + remoteTag := localImageTag - task.SetProgress(NewServiceProgress("Tagging container image")) - if err := ch.docker.Tag(ctx, serviceConfig.Path(), localImageTag, remoteTag); err != nil { - task.SetError(err) - return - } + // If a registry has not been defined then there is no need to tag or push any images + if registryName != "" { + // Tag image + // Get remote tag from the container helper then call docker cli tag command + tag, err := ch.RemoteImageTag(ctx, serviceConfig, localImageTag) + if err != nil { + task.SetError(fmt.Errorf("getting remote image tag: %w", err)) + return + } - log.Printf("logging into container registry '%s'\n", loginServer) - task.SetProgress(NewServiceProgress("Logging into container registry")) - err = ch.containerRegistryService.Login(ctx, targetResource.SubscriptionId(), loginServer) - if err != nil { - task.SetError(err) - return - } + remoteTag = tag - // Push image. - log.Printf("pushing %s to registry", remoteTag) - task.SetProgress(NewServiceProgress("Pushing container image")) - if err := ch.docker.Push(ctx, serviceConfig.Path(), remoteTag); err != nil { - task.SetError(err) - return + task.SetProgress(NewServiceProgress("Tagging container image")) + if err := ch.docker.Tag(ctx, serviceConfig.Path(), localImageTag, remoteTag); err != nil { + task.SetError(err) + return + } + + log.Printf("logging into container registry '%s'\n", registryName) + task.SetProgress(NewServiceProgress("Logging into container registry")) + err = ch.containerRegistryService.Login(ctx, targetResource.SubscriptionId(), registryName) + if err != nil { + task.SetError(err) + return + } + + // Push image. + log.Printf("pushing %s to registry", remoteTag) + task.SetProgress(NewServiceProgress("Pushing container image")) + if err := ch.docker.Push(ctx, serviceConfig.Path(), remoteTag); err != nil { + task.SetError(err) + return + } } if writeImageToEnv { diff --git a/cli/azd/pkg/project/container_helper_test.go b/cli/azd/pkg/project/container_helper_test.go index 7af3a072eb8..02f6d170584 100644 --- a/cli/azd/pkg/project/container_helper_test.go +++ b/cli/azd/pkg/project/container_helper_test.go @@ -3,13 +3,19 @@ package project import ( "context" "fmt" + "strings" "testing" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerregistry/armcontainerregistry" "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/exec" + "github.com/azure/azure-dev/cli/azd/pkg/tools/azcli" + "github.com/azure/azure-dev/cli/azd/pkg/tools/docker" "github.com/azure/azure-dev/cli/azd/test/mocks" "github.com/azure/azure-dev/cli/azd/test/mocks/mockenv" "github.com/benbjohnson/clock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -88,7 +94,7 @@ func Test_ContainerHelper_RemoteImageTag_NoContainer_Registry(t *testing.T) { require.Empty(t, imageTag) } -func Test_Resolve_RegistryName(t *testing.T) { +func Test_ContainerHelper_Resolve_RegistryName(t *testing.T) { t.Run("Default EnvVar", func(t *testing.T) { mockContext := mocks.NewMockContext(context.Background()) env := environment.NewWithValues("dev", map[string]string{ @@ -142,3 +148,238 @@ func Test_Resolve_RegistryName(t *testing.T) { require.Empty(t, registryName) }) } + +func Test_ContainerHelper_Deploy(t *testing.T) { + t.Run("Registry and private image", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + mockResults := setupDockerMocks(mockContext) + env := environment.NewWithValues("dev", map[string]string{ + environment.ContainerRegistryEndpointEnvVarName: "contoso.azurecr.io", + }) + dockerCli := docker.NewDocker(mockContext.CommandRunner) + envManager := &mockenv.MockEnvManager{} + envManager.On("Save", *mockContext.Context, env).Return(nil) + + mockContainerRegistryService := &mockContainerRegistryService{} + setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) + + containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) + serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) + + packageOutput := &ServicePackageResult{ + Details: &dockerPackageResult{ + ImageHash: "1234567890", + ImageTag: "my-app:azd-deploy-1234567890", + }, + } + + targetResource := environment.NewTargetResource( + "SUBSCRIPTION_ID", + "RESOURCE_GROUP", + "CONTAINER_APP", + "Microsoft.App/containerApps", + ) + + deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, packageOutput, targetResource, true) + logProgress(deployTask) + deployResult, err := deployTask.Await() + + require.NoError(t, err) + require.NotNil(t, deployResult) + + dockerDeployDetails, _ := deployResult.Details.(*dockerDeployResult) + require.Equal(t, "contoso.azurecr.io/my-app:azd-deploy-1234567890", dockerDeployDetails.RemoteImageTag) + + _, dockerTagCalled := mockResults["docker-tag"] + _, dockerPushCalled := mockResults["docker-push"] + + require.True(t, dockerTagCalled) + require.True(t, dockerPushCalled) + mockContainerRegistryService.AssertCalled(t, "Login", *mockContext.Context, "SUBSCRIPTION_ID", "contoso.azurecr.io") + }) + + t.Run("Registry and public image", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + mockResults := setupDockerMocks(mockContext) + env := environment.NewWithValues("dev", map[string]string{ + environment.ContainerRegistryEndpointEnvVarName: "contoso.azurecr.io", + }) + dockerCli := docker.NewDocker(mockContext.CommandRunner) + envManager := &mockenv.MockEnvManager{} + envManager.On("Save", *mockContext.Context, env).Return(nil) + + mockContainerRegistryService := &mockContainerRegistryService{} + setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) + + containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) + serviceConfig := createTestServiceConfig("", ContainerAppTarget, ServiceLanguageDocker) + + packageOutput := &ServicePackageResult{ + Details: &dockerPackageResult{ + ImageHash: "", + ImageTag: "nginx", + }, + } + + targetResource := environment.NewTargetResource( + "SUBSCRIPTION_ID", + "RESOURCE_GROUP", + "CONTAINER_APP", + "Microsoft.App/containerApps", + ) + + deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, packageOutput, targetResource, true) + logProgress(deployTask) + deployResult, err := deployTask.Await() + + require.NoError(t, err) + require.NotNil(t, deployResult) + + dockerDeployDetails, _ := deployResult.Details.(*dockerDeployResult) + require.Equal(t, "contoso.azurecr.io/nginx", dockerDeployDetails.RemoteImageTag) + + _, dockerTagCalled := mockResults["docker-tag"] + _, dockerPushCalled := mockResults["docker-push"] + + require.True(t, dockerTagCalled) + require.True(t, dockerPushCalled) + mockContainerRegistryService.AssertCalled(t, "Login", *mockContext.Context, "SUBSCRIPTION_ID", "contoso.azurecr.io") + }) + + t.Run("No registry and public image", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + mockResults := setupDockerMocks(mockContext) + env := environment.NewWithValues("dev", map[string]string{}) + dockerCli := docker.NewDocker(mockContext.CommandRunner) + envManager := &mockenv.MockEnvManager{} + envManager.On("Save", *mockContext.Context, env).Return(nil) + + mockContainerRegistryService := &mockContainerRegistryService{} + setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) + + containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) + serviceConfig := createTestServiceConfig("", ContainerAppTarget, ServiceLanguageDocker) + + packageOutput := &ServicePackageResult{ + Details: &dockerPackageResult{ + ImageHash: "", + ImageTag: "nginx", + }, + } + + targetResource := environment.NewTargetResource( + "SUBSCRIPTION_ID", + "RESOURCE_GROUP", + "CONTAINER_APP", + "Microsoft.App/containerApps", + ) + + deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, packageOutput, targetResource, true) + logProgress(deployTask) + deployResult, err := deployTask.Await() + + require.NoError(t, err) + require.NotNil(t, deployResult) + + dockerDeployDetails, _ := deployResult.Details.(*dockerDeployResult) + require.Equal(t, "nginx", dockerDeployDetails.RemoteImageTag) + + _, dockerTagCalled := mockResults["docker-tag"] + _, dockerPushCalled := mockResults["docker-push"] + + require.False(t, dockerTagCalled) + require.False(t, dockerPushCalled) + mockContainerRegistryService.AssertNotCalled(t, "Login") + }) + + t.Run("Code without registry", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + env := environment.NewWithValues("dev", map[string]string{}) + dockerCli := docker.NewDocker(mockContext.CommandRunner) + envManager := &mockenv.MockEnvManager{} + envManager.On("Save", *mockContext.Context, env).Return(nil) + + mockContainerRegistryService := &mockContainerRegistryService{} + setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) + + containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) + serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) + + targetResource := environment.NewTargetResource( + "SUBSCRIPTION_ID", + "RESOURCE_GROUP", + "CONTAINER_APP", + "Microsoft.App/containerApps", + ) + + deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, nil, targetResource, true) + logProgress(deployTask) + deployResult, err := deployTask.Await() + + // Expected to fail when no registry is specified + require.Error(t, err) + require.Nil(t, deployResult) + }) +} + +func setupContainerRegistryMocks(mockContext *mocks.MockContext, mockContainerRegistryService *mock.Mock) { + mockContainerRegistryService.On( + "Login", + *mockContext.Context, + mock.AnythingOfType("string"), + mock.AnythingOfType("string")). + Return(nil) +} + +func setupDockerMocks(mockContext *mocks.MockContext) map[string]exec.RunArgs { + mockResults := map[string]exec.RunArgs{} + + mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool { + return strings.Contains(command, "docker tag") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + mockResults["docker-tag"] = args + return exec.NewRunResult(0, "", ""), nil + }) + + mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool { + return strings.Contains(command, "docker push") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + mockResults["docker-push"] = args + return exec.NewRunResult(0, "", ""), nil + }) + + mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool { + return strings.Contains(command, "docker login") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + mockResults["docker-login"] = args + return exec.NewRunResult(0, "", ""), nil + }) + + return mockResults +} + +type mockContainerRegistryService struct { + mock.Mock +} + +func (m *mockContainerRegistryService) Login(ctx context.Context, subscriptionId string, loginServer string) error { + args := m.Called(ctx, subscriptionId, loginServer) + return args.Error(0) +} + +func (m *mockContainerRegistryService) Credentials( + ctx context.Context, + subscriptionId string, + loginServer string, +) (*azcli.DockerCredentials, error) { + args := m.Called(ctx, subscriptionId, loginServer) + return args.Get(0).(*azcli.DockerCredentials), args.Error(1) +} + +func (m *mockContainerRegistryService) GetContainerRegistries( + ctx context.Context, + subscriptionId string, +) ([]*armcontainerregistry.Registry, error) { + args := m.Called(ctx, subscriptionId) + return args.Get(0).([]*armcontainerregistry.Registry), args.Error(1) +} diff --git a/cli/azd/pkg/project/service_manager_test.go b/cli/azd/pkg/project/service_manager_test.go index 7f346d121ab..4c37a12a13f 100644 --- a/cli/azd/pkg/project/service_manager_test.go +++ b/cli/azd/pkg/project/service_manager_test.go @@ -220,16 +220,48 @@ func Test_ServiceManager_Deploy(t *testing.T) { } func Test_ServiceManager_GetFrameworkService(t *testing.T) { - mockContext := mocks.NewMockContext(context.Background()) - setupMocksForServiceManager(mockContext) - env := environment.New("test") - sm := createServiceManager(mockContext, env, ServiceOperationCache{}) - serviceConfig := createTestServiceConfig("./src/api", ServiceTargetFake, ServiceLanguageFake) + t.Run("Standard", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + setupMocksForServiceManager(mockContext) + env := environment.New("test") + sm := createServiceManager(mockContext, env, ServiceOperationCache{}) + serviceConfig := createTestServiceConfig("./src/api", ServiceTargetFake, ServiceLanguageFake) + + framework, err := sm.GetFrameworkService(*mockContext.Context, serviceConfig) + require.NoError(t, err) + require.NotNil(t, framework) + require.IsType(t, new(fakeFramework), framework) + }) - framework, err := sm.GetFrameworkService(*mockContext.Context, serviceConfig) - require.NoError(t, err) - require.NotNil(t, framework) - require.IsType(t, new(fakeFramework), framework) + t.Run("No project path and has docker tag", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + mockContext.Container.MustRegisterNamedTransient("docker", newFakeFramework) + + setupMocksForServiceManager(mockContext) + env := environment.New("test") + sm := createServiceManager(mockContext, env, ServiceOperationCache{}) + serviceConfig := createTestServiceConfig("", ServiceTargetFake, ServiceLanguageNone) + serviceConfig.Docker.Tag = NewExpandableString("nginx") + + framework, err := sm.GetFrameworkService(*mockContext.Context, serviceConfig) + require.NoError(t, err) + require.NotNil(t, framework) + require.IsType(t, new(fakeFramework), framework) + }) + + t.Run("No project path or docker tag", func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + mockContext.Container.MustRegisterNamedTransient("docker", newFakeFramework) + + setupMocksForServiceManager(mockContext) + env := environment.New("test") + sm := createServiceManager(mockContext, env, ServiceOperationCache{}) + serviceConfig := createTestServiceConfig("", ServiceTargetFake, ServiceLanguageNone) + + require.Panics(t, func() { + _, _ = sm.GetFrameworkService(*mockContext.Context, serviceConfig) + }) + }) } func Test_ServiceManager_GetServiceTarget(t *testing.T) { From 6e1290720c5e7a38389d1179f7714f28957a1e7a Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Wed, 31 Jan 2024 17:00:07 -0800 Subject: [PATCH 03/10] Display N/A when image id is not available --- cli/azd/pkg/project/container_helper.go | 97 +++++++++--- cli/azd/pkg/project/container_helper_test.go | 16 +- .../pkg/project/framework_service_docker.go | 10 +- cli/azd/pkg/tools/docker/container_image.go | 86 +++++++++++ .../pkg/tools/docker/container_image_test.go | 139 ++++++++++++++++++ cli/azd/pkg/tools/docker/docker.go | 10 ++ 6 files changed, 333 insertions(+), 25 deletions(-) create mode 100644 cli/azd/pkg/tools/docker/container_image.go create mode 100644 cli/azd/pkg/tools/docker/container_image_test.go diff --git a/cli/azd/pkg/project/container_helper.go b/cli/azd/pkg/project/container_helper.go index 92a047d6a89..2a8510ebf15 100644 --- a/cli/azd/pkg/project/container_helper.go +++ b/cli/azd/pkg/project/container_helper.go @@ -39,7 +39,7 @@ func NewContainerHelper( } } -// RegistryName returns the name of the container registry to use for the current environment from the following: +// RegistryName returns the name of the destination container registry to use for the current environment from the following: // 1. AZURE_CONTAINER_REGISTRY_ENDPOINT environment variable // 2. docker.registry from the service configuration func (ch *ContainerHelper) RegistryName(ctx context.Context, serviceConfig *ServiceConfig) (string, error) { @@ -75,44 +75,82 @@ func (ch *ContainerHelper) RegistryName(ctx context.Context, serviceConfig *Serv return registryName, nil } +// ConfiguredImage returns the configured image from the service configuration +// or a default image name generated from the service name and environment name. +func (ch *ContainerHelper) ConfiguredImage( + ctx context.Context, + serviceConfig *ServiceConfig, +) (*docker.ContainerImage, error) { + // Parse the image from azure.yaml configuration when available + configuredImage, err := serviceConfig.Docker.Image.Envsubst(ch.env.Getenv) + if err != nil { + return nil, fmt.Errorf("failed parsing 'image' from docker configuration, %w", err) + } + + parsedImage, err := docker.ParseContainerImage(configuredImage) + if err == nil { + if parsedImage.Tag == "" { + configuredTag, err := serviceConfig.Docker.Tag.Envsubst(ch.env.Getenv) + if err != nil { + return nil, fmt.Errorf("failed parsing 'tag' from docker configuration, %w", err) + } + + parsedImage.Tag = configuredTag + } + + return parsedImage, nil + } + + configuredRegistry, err := ch.RegistryName(ctx, serviceConfig) + if err != nil { + return nil, err + } + + // Otherwise return a image name generated from default strategy + defaultImage := &docker.ContainerImage{ + Registry: configuredRegistry, + Repository: fmt.Sprintf("%s/%s-%s", + strings.ToLower(serviceConfig.Project.Name), + strings.ToLower(serviceConfig.Name), + strings.ToLower(ch.env.Name()), + ), + Tag: fmt.Sprintf("azd-deploy-%d", + ch.clock.Now().Unix(), + ), + } + + return defaultImage, nil +} + +// RemoteImageTag returns the remote image tag for the service configuration. func (ch *ContainerHelper) RemoteImageTag( ctx context.Context, serviceConfig *ServiceConfig, localImageTag string, ) (string, error) { - loginServer, err := ch.RegistryName(ctx, serviceConfig) + registryName, err := ch.RegistryName(ctx, serviceConfig) if err != nil { return "", err } - // If no registry/login server is defined return the original tag - if loginServer == "" { - return localImageTag, nil + containerImage, err := docker.ParseContainerImage(localImageTag) + if err != nil { + return "", err } - return fmt.Sprintf( - "%s/%s", - loginServer, - localImageTag, - ), nil + containerImage.Registry = registryName + + return containerImage.Remote(), nil } +// LocalImageTag returns the local image tag for the service configuration. func (ch *ContainerHelper) LocalImageTag(ctx context.Context, serviceConfig *ServiceConfig) (string, error) { - configuredTag, err := serviceConfig.Docker.Tag.Envsubst(ch.env.Getenv) + configuredImage, err := ch.ConfiguredImage(ctx, serviceConfig) if err != nil { return "", err } - if configuredTag != "" { - return configuredTag, nil - } - - return fmt.Sprintf("%s/%s-%s:azd-deploy-%d", - strings.ToLower(serviceConfig.Project.Name), - strings.ToLower(serviceConfig.Name), - strings.ToLower(ch.env.Name()), - ch.clock.Now().Unix(), - ), nil + return configuredImage.Local(), nil } func (ch *ContainerHelper) RequiredExternalTools(context.Context) []tools.ExternalTool { @@ -200,6 +238,23 @@ func (ch *ContainerHelper) Deploy( // If a registry has not been defined then there is no need to tag or push any images if registryName != "" { + // When the project does not contain source and we are using an external image we first need to pull the image + // before we're able to push it to a remote registry + if serviceConfig.RelativePath == "" { + sourceImage, err := ch.ConfiguredImage(ctx, serviceConfig) + if err != nil { + task.SetError(fmt.Errorf("getting configured image: %w", err)) + return + } + + task.SetProgress(NewServiceProgress("Pulling container image")) + err = ch.docker.Pull(ctx, sourceImage.Remote()) + if err != nil { + task.SetError(fmt.Errorf("pulling image: %w", err)) + return + } + } + // Tag image // Get remote tag from the container helper then call docker cli tag command tag, err := ch.RemoteImageTag(ctx, serviceConfig, localImageTag) diff --git a/cli/azd/pkg/project/container_helper_test.go b/cli/azd/pkg/project/container_helper_test.go index 02f6d170584..45711498c9a 100644 --- a/cli/azd/pkg/project/container_helper_test.go +++ b/cli/azd/pkg/project/container_helper_test.go @@ -48,9 +48,10 @@ func Test_ContainerHelper_LocalImageTag(t *testing.T) { { "ImageTagSpecified", DockerProjectOptions{ - Tag: NewExpandableString("contoso/contoso-image:latest"), + Image: NewExpandableString("contoso/contoso-image:latest"), }, - "contoso/contoso-image:latest"}, + "contoso/contoso-image:latest", + }, } for _, tt := range tests { @@ -239,9 +240,11 @@ func Test_ContainerHelper_Deploy(t *testing.T) { require.Equal(t, "contoso.azurecr.io/nginx", dockerDeployDetails.RemoteImageTag) _, dockerTagCalled := mockResults["docker-tag"] + _, dockerPullCalled := mockResults["docker-pull"] _, dockerPushCalled := mockResults["docker-push"] require.True(t, dockerTagCalled) + require.True(t, dockerPullCalled) require.True(t, dockerPushCalled) mockContainerRegistryService.AssertCalled(t, "Login", *mockContext.Context, "SUBSCRIPTION_ID", "contoso.azurecr.io") }) @@ -285,9 +288,11 @@ func Test_ContainerHelper_Deploy(t *testing.T) { require.Equal(t, "nginx", dockerDeployDetails.RemoteImageTag) _, dockerTagCalled := mockResults["docker-tag"] + _, dockerPullCalled := mockResults["docker-pull"] _, dockerPushCalled := mockResults["docker-push"] require.False(t, dockerTagCalled) + require.False(t, dockerPullCalled) require.False(t, dockerPushCalled) mockContainerRegistryService.AssertNotCalled(t, "Login") }) @@ -348,6 +353,13 @@ func setupDockerMocks(mockContext *mocks.MockContext) map[string]exec.RunArgs { return exec.NewRunResult(0, "", ""), nil }) + mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool { + return strings.Contains(command, "docker pull") + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + mockResults["docker-pull"] = args + return exec.NewRunResult(0, "", ""), nil + }) + mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool { return strings.Contains(command, "docker login") }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { diff --git a/cli/azd/pkg/project/framework_service_docker.go b/cli/azd/pkg/project/framework_service_docker.go index 742074ca1aa..ca96759327d 100644 --- a/cli/azd/pkg/project/framework_service_docker.go +++ b/cli/azd/pkg/project/framework_service_docker.go @@ -38,8 +38,9 @@ type DockerProjectOptions struct { Context string `yaml:"context,omitempty" json:"context,omitempty"` Platform string `yaml:"platform,omitempty" json:"platform,omitempty"` Target string `yaml:"target,omitempty" json:"target,omitempty"` - Tag ExpandableString `yaml:"tag,omitempty" json:"tag,omitempty"` Registry ExpandableString `yaml:"registry,omitempty" json:"registry,omitempty"` + Image ExpandableString `yaml:"image,omitempty" json:"image,omitempty"` + Tag ExpandableString `yaml:"tag,omitempty" json:"tag,omitempty"` BuildArgs []string `yaml:"buildArgs,omitempty" json:"buildArgs,omitempty"` } @@ -67,8 +68,13 @@ type dockerPackageResult struct { } func (dpr *dockerPackageResult) ToString(currentIndentation string) string { + imageHash := dpr.ImageHash + if imageHash == "" { + imageHash = "N/A" + } + lines := []string{ - fmt.Sprintf("%s- Image Hash: %s", currentIndentation, output.WithLinkFormat(dpr.ImageHash)), + fmt.Sprintf("%s- Image Hash: %s", currentIndentation, output.WithLinkFormat(imageHash)), fmt.Sprintf("%s- Image Tag: %s", currentIndentation, output.WithLinkFormat(dpr.ImageTag)), } diff --git a/cli/azd/pkg/tools/docker/container_image.go b/cli/azd/pkg/tools/docker/container_image.go new file mode 100644 index 00000000000..9170309e5f7 --- /dev/null +++ b/cli/azd/pkg/tools/docker/container_image.go @@ -0,0 +1,86 @@ +package docker + +import ( + "errors" + "strings" +) + +// ContainerImage represents a container image and its components +type ContainerImage struct { + // The registry name + Registry string + // The repository name or path + Repository string + // The tag + Tag string +} + +// Local returns the local image name without registry +func (ci *ContainerImage) Local() string { + builder := strings.Builder{} + + if ci.Repository != "" { + builder.WriteString(ci.Repository) + } + + if ci.Tag != "" { + builder.WriteString(":") + builder.WriteString(ci.Tag) + } + + return builder.String() +} + +// Remote returns the remote image name with registry when specified +func (ci *ContainerImage) Remote() string { + builder := strings.Builder{} + if ci.Registry != "" { + builder.WriteString(ci.Registry) + builder.WriteString("/") + } + + if ci.Repository != "" { + builder.WriteString(ci.Repository) + } + + if ci.Tag != "" { + builder.WriteString(":") + builder.WriteString(ci.Tag) + } + + return builder.String() +} + +func ParseContainerImage(image string) (*ContainerImage, error) { + // Check if the imageURL is empty + if image == "" { + return nil, errors.New("empty image URL provided") + } + + containerImage := &ContainerImage{} + + // Detect tags + tagParts := strings.Split(image, ":") + if len(tagParts) > 2 { + return containerImage, errors.New("invalid tag format") + } + + if len(tagParts) == 2 { + containerImage.Tag = tagParts[1] + image = tagParts[0] + } + + // Split the imageURL by "/" + parts := strings.Split(image, "/") + + // Check if the parts contain a registry (parts[0] contains ".") + if strings.Contains(parts[0], ".") { + containerImage.Registry = parts[0] + parts = parts[1:] + } + + // Set the repository as the remaining parts joined by "/" + containerImage.Repository = strings.Join(parts, "/") + + return containerImage, nil +} diff --git a/cli/azd/pkg/tools/docker/container_image_test.go b/cli/azd/pkg/tools/docker/container_image_test.go new file mode 100644 index 00000000000..da94fb26681 --- /dev/null +++ b/cli/azd/pkg/tools/docker/container_image_test.go @@ -0,0 +1,139 @@ +package docker + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_ParseContainerImage(t *testing.T) { + tests := []struct { + name string + input string + expected ContainerImage + }{ + { + name: "image with registry and tag", + input: "registry.example.com/my-image:1.0", + expected: ContainerImage{ + Registry: "registry.example.com", + Repository: "my-image", + Tag: "1.0", + }, + }, + { + name: "image with registry, no tag", + input: "registry.example.com/my-image", + expected: ContainerImage{ + Registry: "registry.example.com", + Repository: "my-image", + Tag: "", + }, + }, + { + name: "image with tag and no registry", + input: "my-image:1.0", + expected: ContainerImage{ + Registry: "", // no registry + Repository: "my-image", + Tag: "1.0", + }, + }, + { + name: "image with no registry or tag", + input: "my-image", + expected: ContainerImage{ + Registry: "", // no registry + Repository: "my-image", + Tag: "", + }, + }, + { + name: "image with multi-part repository", + input: "registry.example.com/my-image/foo/bar:1.0", + expected: ContainerImage{ + Registry: "registry.example.com", + Repository: "my-image/foo/bar", + Tag: "1.0", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual, err := ParseContainerImage(tt.input) + require.NoError(t, err) + require.Equal(t, tt.expected, *actual) + }) + } +} + +func Test_ContainerImage_Local_And_Remote(t *testing.T) { + tests := []struct { + name string + input ContainerImage + expectedLocal string + expectedRemote string + }{ + { + name: "image with registry and tag", + input: ContainerImage{ + Registry: "registry.example.com", + Repository: "my-image", + Tag: "1.0", + }, + expectedRemote: "registry.example.com/my-image:1.0", + expectedLocal: "my-image:1.0", + }, + { + name: "image with registry and no tag", + input: ContainerImage{ + Registry: "registry.example.com", + Repository: "my-image", + Tag: "", + }, + expectedRemote: "registry.example.com/my-image", + expectedLocal: "my-image", + }, + { + name: "image with tag and no registry", + input: ContainerImage{ + Registry: "", // no registry + Repository: "my-image", + Tag: "1.0", + }, + expectedRemote: "my-image:1.0", + expectedLocal: "my-image:1.0", + }, + { + name: "image with no registry or tag", + input: ContainerImage{ + Registry: "", // no registry + Repository: "my-image", + Tag: "", + }, + expectedRemote: "my-image", + expectedLocal: "my-image", + }, + { + name: "image with multi-part repository", + input: ContainerImage{ + Registry: "registry.example.com", + Repository: "my-image/foo/bar", + Tag: "1.0", + }, + expectedRemote: "registry.example.com/my-image/foo/bar:1.0", + expectedLocal: "my-image/foo/bar:1.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualRemote := tt.input.Remote() + require.Equal(t, tt.expectedRemote, actualRemote) + + actualLocal := tt.input.Local() + require.Equal(t, tt.expectedLocal, actualLocal) + }) + } +} diff --git a/cli/azd/pkg/tools/docker/docker.go b/cli/azd/pkg/tools/docker/docker.go index b3e0b6026cb..18b2e2777b3 100644 --- a/cli/azd/pkg/tools/docker/docker.go +++ b/cli/azd/pkg/tools/docker/docker.go @@ -34,6 +34,7 @@ type Docker interface { ) (string, error) Tag(ctx context.Context, cwd string, imageName string, tag string) error Push(ctx context.Context, cwd string, tag string) error + Pull(ctx context.Context, imageName string) error Inspect(ctx context.Context, imageName string, format string) (string, error) } @@ -154,6 +155,15 @@ func (d *docker) Push(ctx context.Context, cwd string, tag string) error { return nil } +func (d *docker) Pull(ctx context.Context, imageName string) error { + _, err := d.executeCommand(ctx, "", "pull", imageName) + if err != nil { + return fmt.Errorf("pulling image: %w", err) + } + + return nil +} + func (d *docker) Inspect(ctx context.Context, imageName string, format string) (string, error) { out, err := d.executeCommand(ctx, "", "image", "inspect", "--format", format, imageName) if err != nil { From 9bd3db2d9469cb1f498b65fd202ccccfd4b77820 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Wed, 31 Jan 2024 17:21:43 -0800 Subject: [PATCH 04/10] Fixes docker project unit tests --- cli/azd/pkg/project/framework_service_docker_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cli/azd/pkg/project/framework_service_docker_test.go b/cli/azd/pkg/project/framework_service_docker_test.go index 5c81846afb7..0a66a7db073 100644 --- a/cli/azd/pkg/project/framework_service_docker_test.go +++ b/cli/azd/pkg/project/framework_service_docker_test.go @@ -261,12 +261,16 @@ func Test_DockerProject_Build(t *testing.T) { env := environment.New("test") dockerCli := docker.NewDocker(mockContext.CommandRunner) serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) + temp := t.TempDir() + + serviceConfig.Docker.Registry = NewExpandableString("contoso.azurecr.io") serviceConfig.Project.Path = temp serviceConfig.RelativePath = "" err := os.WriteFile(filepath.Join(temp, "Dockerfile"), []byte("FROM node:14"), 0600) require.NoError(t, err) + npmProject := NewNpmProject(npm.NewNpmCli(mockContext.CommandRunner), env) dockerProject := NewDockerProject( env, dockerCli, @@ -274,6 +278,8 @@ func Test_DockerProject_Build(t *testing.T) { mockinput.NewMockConsole(), mockContext.AlphaFeaturesManager, mockContext.CommandRunner) + dockerProject.SetSource(npmProject) + buildTask := dockerProject.Build(*mockContext.Context, serviceConfig, nil) logProgress(buildTask) @@ -319,7 +325,9 @@ func Test_DockerProject_Package(t *testing.T) { env := environment.NewWithValues("test", map[string]string{}) dockerCli := docker.NewDocker(mockContext.CommandRunner) serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) + serviceConfig.Docker.Registry = NewExpandableString("contoso.azurecr.io") + npmProject := NewNpmProject(npm.NewNpmCli(mockContext.CommandRunner), env) dockerProject := NewDockerProject( env, dockerCli, @@ -327,6 +335,8 @@ func Test_DockerProject_Package(t *testing.T) { mockinput.NewMockConsole(), mockContext.AlphaFeaturesManager, mockContext.CommandRunner) + dockerProject.SetSource(npmProject) + packageTask := dockerProject.Package( *mockContext.Context, serviceConfig, From 139dbc7a813d8863cd684b99326961228d26942e Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Wed, 31 Jan 2024 18:10:50 -0800 Subject: [PATCH 05/10] Remove unneeded code --- cli/azd/pkg/project/framework_service.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/cli/azd/pkg/project/framework_service.go b/cli/azd/pkg/project/framework_service.go index 064dd4e80c2..05c871553ad 100644 --- a/cli/azd/pkg/project/framework_service.go +++ b/cli/azd/pkg/project/framework_service.go @@ -26,15 +26,6 @@ const ( ServiceLanguageDocker ServiceLanguageKind = "docker" ) -var ( - NoFrameworkRequirements = FrameworkRequirements{ - Package: FrameworkPackageRequirements{ - RequireRestore: false, - RequireBuild: false, - }, - } -) - func parseServiceLanguage(kind ServiceLanguageKind) (ServiceLanguageKind, error) { // aliases if string(kind) == "py" { From 228657374e3dca1a0f7b128d48dc2694c62b70ae Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Mon, 5 Feb 2024 09:52:43 -0800 Subject: [PATCH 06/10] Updates azure.yaml schema for updates --- schemas/alpha/azure.yaml.json | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/schemas/alpha/azure.yaml.json b/schemas/alpha/azure.yaml.json index dd25b867b40..a780365567e 100644 --- a/schemas/alpha/azure.yaml.json +++ b/schemas/alpha/azure.yaml.json @@ -66,8 +66,7 @@ "type": "object", "additionalProperties": false, "required": [ - "project", - "language" + "host" ], "properties": { "resourceName": { @@ -81,10 +80,9 @@ }, "host": { "type": "string", - "title": "Type of Azure resource used for service implementation", - "description": "If omitted, App Service will be assumed.", + "title": "Required. The type of Azure resource used for service implementation", + "description": "The Azure service that will be used as the target for deployment operations for the service.", "enum": [ - "", "appservice", "containerapp", "function", @@ -176,6 +174,10 @@ } }, "then": { + "required": [ + "project", + "language" + ], "properties": { "docker": false } @@ -541,16 +543,21 @@ "title": "The platform target", "default": "amd64" }, - "tag": { - "type": "string", - "title": "The tag that will be applied to the built container image.", - "description": "If omitted, a unique tag will be generated based on the format: {appName}/{serviceName}-{environmentName}:azd-deploy-{unix time (seconds)}. Supports environment variable substitution. For example, to generate unique tags for a given release: myapp/myimage:${DOCKER_IMAGE_TAG}" - }, "registry": { "type": "string", - "title": "The container registry to push the image to.", + "title": "Optional. The container registry to push the image to.", "description": "If omitted, will default to value of AZURE_CONTAINER_REGISTRY_ENDPOINT environment variable. Supports environment variable substitution." }, + "image": { + "type": "string", + "title": "Optional. The name of the container image to use for the service.", + "description": "If omitted, will default to the '{appName}/{serviceName}-{environmentName}'. Supports environment variable substitution." + }, + "tag": { + "type": "string", + "title": "The tag that will be applied to the built container image.", + "description": "If omitted, will default to 'azd-deploy-{unix time (seconds)}'. Supports environment variable substitution. For example, to generate unique tags for a given release: myapp/myimage:${DOCKER_IMAGE_TAG}" + }, "buildArgs": { "type": "array", "title": "Optional. Build arguments to pass to the docker build command", From 87687452d5f4b38f3347766c562e9d61a58d0438 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Tue, 6 Feb 2024 17:06:30 -0800 Subject: [PATCH 07/10] Refactors usage of source image with containerapp --- cli/azd/pkg/project/container_helper.go | 107 +++--- cli/azd/pkg/project/container_helper_test.go | 336 +++++++++--------- .../pkg/project/framework_service_docker.go | 85 +++-- .../project/framework_service_docker_test.go | 182 +++++++--- cli/azd/pkg/project/service_config.go | 4 +- cli/azd/pkg/project/service_manager.go | 4 +- cli/azd/pkg/project/service_manager_test.go | 2 +- cli/azd/pkg/project/service_models_test.go | 4 +- .../pkg/project/service_target_aks_test.go | 4 +- .../service_target_containerapp_test.go | 4 +- cli/azd/pkg/tools/docker/container_image.go | 33 +- .../pkg/tools/docker/container_image_test.go | 50 ++- schemas/alpha/azure.yaml.json | 59 ++- 13 files changed, 552 insertions(+), 322 deletions(-) diff --git a/cli/azd/pkg/project/container_helper.go b/cli/azd/pkg/project/container_helper.go index 2a8510ebf15..6f19d06e560 100644 --- a/cli/azd/pkg/project/container_helper.go +++ b/cli/azd/pkg/project/container_helper.go @@ -75,9 +75,9 @@ func (ch *ContainerHelper) RegistryName(ctx context.Context, serviceConfig *Serv return registryName, nil } -// ConfiguredImage returns the configured image from the service configuration +// GeneratedImage returns the configured image from the service configuration // or a default image name generated from the service name and environment name. -func (ch *ContainerHelper) ConfiguredImage( +func (ch *ContainerHelper) GeneratedImage( ctx context.Context, serviceConfig *ServiceConfig, ) (*docker.ContainerImage, error) { @@ -87,39 +87,46 @@ func (ch *ContainerHelper) ConfiguredImage( return nil, fmt.Errorf("failed parsing 'image' from docker configuration, %w", err) } + // Set default image name if not configured + if configuredImage == "" { + configuredImage = fmt.Sprintf("%s/%s-%s", + strings.ToLower(serviceConfig.Project.Name), + strings.ToLower(serviceConfig.Name), + strings.ToLower(ch.env.Name()), + ) + } + parsedImage, err := docker.ParseContainerImage(configuredImage) - if err == nil { - if parsedImage.Tag == "" { - configuredTag, err := serviceConfig.Docker.Tag.Envsubst(ch.env.Getenv) - if err != nil { - return nil, fmt.Errorf("failed parsing 'tag' from docker configuration, %w", err) - } + if err != nil { + return nil, fmt.Errorf("failed parsing configured image, %w", err) + } - parsedImage.Tag = configuredTag + if parsedImage.Tag == "" { + configuredTag, err := serviceConfig.Docker.Tag.Envsubst(ch.env.Getenv) + if err != nil { + return nil, fmt.Errorf("failed parsing 'tag' from docker configuration, %w", err) } - return parsedImage, nil - } + // Set default tag if not configured + if configuredTag == "" { + configuredTag = fmt.Sprintf("azd-deploy-%d", + ch.clock.Now().Unix(), + ) + } - configuredRegistry, err := ch.RegistryName(ctx, serviceConfig) - if err != nil { - return nil, err + parsedImage.Tag = configuredTag } - // Otherwise return a image name generated from default strategy - defaultImage := &docker.ContainerImage{ - Registry: configuredRegistry, - Repository: fmt.Sprintf("%s/%s-%s", - strings.ToLower(serviceConfig.Project.Name), - strings.ToLower(serviceConfig.Name), - strings.ToLower(ch.env.Name()), - ), - Tag: fmt.Sprintf("azd-deploy-%d", - ch.clock.Now().Unix(), - ), + // Set default registry if not configured + if parsedImage.Registry == "" { + // This can fail if called before provisioning the registry + configuredRegistry, err := ch.RegistryName(ctx, serviceConfig) + if err == nil { + parsedImage.Registry = configuredRegistry + } } - return defaultImage, nil + return parsedImage, nil } // RemoteImageTag returns the remote image tag for the service configuration. @@ -145,7 +152,7 @@ func (ch *ContainerHelper) RemoteImageTag( // LocalImageTag returns the local image tag for the service configuration. func (ch *ContainerHelper) LocalImageTag(ctx context.Context, serviceConfig *ServiceConfig) (string, error) { - configuredImage, err := ch.ConfiguredImage(ctx, serviceConfig) + configuredImage, err := ch.GeneratedImage(ctx, serviceConfig) if err != nil { return "", err } @@ -210,45 +217,43 @@ func (ch *ContainerHelper) Deploy( return } - localImageTag := packageOutput.PackagePath - packageDetails, ok := packageOutput.Details.(*dockerPackageResult) - if ok && packageDetails != nil { - localImageTag = packageDetails.ImageTag + var sourceImage string + targetImage := packageOutput.PackagePath + + packageDetails := packageOutput.Details.(*dockerPackageResult) + if packageDetails != nil { + sourceImage = packageDetails.SourceImage + targetImage = packageDetails.TargetImage } // If we don't have a registry specified and the service does not reference a project path // then we are referencing a public/pre-existing image and don't have anything to tag or push - if registryName == "" && serviceConfig.RelativePath != "" { + if registryName == "" && serviceConfig.RelativePath == "" && sourceImage != "" { task.SetResult(&ServiceDeployResult{ Package: packageOutput, Details: &dockerDeployResult{ - RemoteImageTag: localImageTag, + RemoteImageTag: sourceImage, }, }) return } - if localImageTag == "" { + if targetImage == "" { task.SetError(errors.New("failed retrieving package result details")) return } // Default to the local image tag - remoteTag := localImageTag + remoteImage := targetImage // If a registry has not been defined then there is no need to tag or push any images if registryName != "" { // When the project does not contain source and we are using an external image we first need to pull the image // before we're able to push it to a remote registry - if serviceConfig.RelativePath == "" { - sourceImage, err := ch.ConfiguredImage(ctx, serviceConfig) - if err != nil { - task.SetError(fmt.Errorf("getting configured image: %w", err)) - return - } - + // In most cases this pull will have already been part of the package step + if packageDetails != nil && serviceConfig.RelativePath == "" { task.SetProgress(NewServiceProgress("Pulling container image")) - err = ch.docker.Pull(ctx, sourceImage.Remote()) + err = ch.docker.Pull(ctx, sourceImage) if err != nil { task.SetError(fmt.Errorf("pulling image: %w", err)) return @@ -256,17 +261,17 @@ func (ch *ContainerHelper) Deploy( } // Tag image - // Get remote tag from the container helper then call docker cli tag command - tag, err := ch.RemoteImageTag(ctx, serviceConfig, localImageTag) + // Get remote remoteImageWithTag from the container helper then call docker cli remoteImageWithTag command + remoteImageWithTag, err := ch.RemoteImageTag(ctx, serviceConfig, targetImage) if err != nil { task.SetError(fmt.Errorf("getting remote image tag: %w", err)) return } - remoteTag = tag + remoteImage = remoteImageWithTag task.SetProgress(NewServiceProgress("Tagging container image")) - if err := ch.docker.Tag(ctx, serviceConfig.Path(), localImageTag, remoteTag); err != nil { + if err := ch.docker.Tag(ctx, serviceConfig.Path(), targetImage, remoteImage); err != nil { task.SetError(err) return } @@ -280,9 +285,9 @@ func (ch *ContainerHelper) Deploy( } // Push image. - log.Printf("pushing %s to registry", remoteTag) + log.Printf("pushing %s to registry", remoteImage) task.SetProgress(NewServiceProgress("Pushing container image")) - if err := ch.docker.Push(ctx, serviceConfig.Path(), remoteTag); err != nil { + if err := ch.docker.Push(ctx, serviceConfig.Path(), remoteImage); err != nil { task.SetError(err) return } @@ -291,7 +296,7 @@ func (ch *ContainerHelper) Deploy( if writeImageToEnv { // Save the name of the image we pushed into the environment with a well known key. log.Printf("writing image name to environment") - ch.env.SetServiceProperty(serviceConfig.Name, "IMAGE_NAME", remoteTag) + ch.env.SetServiceProperty(serviceConfig.Name, "IMAGE_NAME", remoteImage) if err := ch.envManager.Save(ctx, ch.env); err != nil { task.SetError(fmt.Errorf("saving image name to environment: %w", err)) @@ -302,7 +307,7 @@ func (ch *ContainerHelper) Deploy( task.SetResult(&ServiceDeployResult{ Package: packageOutput, Details: &dockerDeployResult{ - RemoteImageTag: remoteTag, + RemoteImageTag: remoteImage, }, }) }) diff --git a/cli/azd/pkg/project/container_helper_test.go b/cli/azd/pkg/project/container_helper_test.go index 45711498c9a..cd422e25a0d 100644 --- a/cli/azd/pkg/project/container_helper_test.go +++ b/cli/azd/pkg/project/container_helper_test.go @@ -2,6 +2,7 @@ package project import ( "context" + "errors" "fmt" "strings" "testing" @@ -151,180 +152,162 @@ func Test_ContainerHelper_Resolve_RegistryName(t *testing.T) { } func Test_ContainerHelper_Deploy(t *testing.T) { - t.Run("Registry and private image", func(t *testing.T) { - mockContext := mocks.NewMockContext(context.Background()) - mockResults := setupDockerMocks(mockContext) - env := environment.NewWithValues("dev", map[string]string{ - environment.ContainerRegistryEndpointEnvVarName: "contoso.azurecr.io", - }) - dockerCli := docker.NewDocker(mockContext.CommandRunner) - envManager := &mockenv.MockEnvManager{} - envManager.On("Save", *mockContext.Context, env).Return(nil) - - mockContainerRegistryService := &mockContainerRegistryService{} - setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) - - containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) - serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) - - packageOutput := &ServicePackageResult{ - Details: &dockerPackageResult{ - ImageHash: "1234567890", - ImageTag: "my-app:azd-deploy-1234567890", + tests := []struct { + name string + registry ExpandableString + image string + project string + packagePath string + dockerDetails *dockerPackageResult + expectedRemoteImage string + expectDockerPullCalled bool + expectDockerTagCalled bool + expectDockerPushCalled bool + expectError bool + }{ + { + name: "Source code and registry", + project: "./src/api", + registry: NewExpandableString("contoso.azurecr.io"), + dockerDetails: &dockerPackageResult{ + ImageHash: "IMAGE_ID", + SourceImage: "", + TargetImage: "my-project/my-service:azd-deploy-0", }, - } - - targetResource := environment.NewTargetResource( - "SUBSCRIPTION_ID", - "RESOURCE_GROUP", - "CONTAINER_APP", - "Microsoft.App/containerApps", - ) - - deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, packageOutput, targetResource, true) - logProgress(deployTask) - deployResult, err := deployTask.Await() - - require.NoError(t, err) - require.NotNil(t, deployResult) - - dockerDeployDetails, _ := deployResult.Details.(*dockerDeployResult) - require.Equal(t, "contoso.azurecr.io/my-app:azd-deploy-1234567890", dockerDeployDetails.RemoteImageTag) - - _, dockerTagCalled := mockResults["docker-tag"] - _, dockerPushCalled := mockResults["docker-push"] - - require.True(t, dockerTagCalled) - require.True(t, dockerPushCalled) - mockContainerRegistryService.AssertCalled(t, "Login", *mockContext.Context, "SUBSCRIPTION_ID", "contoso.azurecr.io") - }) - - t.Run("Registry and public image", func(t *testing.T) { - mockContext := mocks.NewMockContext(context.Background()) - mockResults := setupDockerMocks(mockContext) - env := environment.NewWithValues("dev", map[string]string{ - environment.ContainerRegistryEndpointEnvVarName: "contoso.azurecr.io", - }) - dockerCli := docker.NewDocker(mockContext.CommandRunner) - envManager := &mockenv.MockEnvManager{} - envManager.On("Save", *mockContext.Context, env).Return(nil) - - mockContainerRegistryService := &mockContainerRegistryService{} - setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) - - containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) - serviceConfig := createTestServiceConfig("", ContainerAppTarget, ServiceLanguageDocker) - - packageOutput := &ServicePackageResult{ - Details: &dockerPackageResult{ - ImageHash: "", - ImageTag: "nginx", + expectDockerPullCalled: false, + expectDockerTagCalled: true, + expectDockerPushCalled: true, + expectedRemoteImage: "contoso.azurecr.io/my-project/my-service:azd-deploy-0", + expectError: false, + }, + { + name: "Source code and no registry", + project: "./src/api", + dockerDetails: &dockerPackageResult{ + ImageHash: "IMAGE_ID", + SourceImage: "", + TargetImage: "my-project/my-service:azd-deploy-0", }, - } - - targetResource := environment.NewTargetResource( - "SUBSCRIPTION_ID", - "RESOURCE_GROUP", - "CONTAINER_APP", - "Microsoft.App/containerApps", - ) - - deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, packageOutput, targetResource, true) - logProgress(deployTask) - deployResult, err := deployTask.Await() - - require.NoError(t, err) - require.NotNil(t, deployResult) - - dockerDeployDetails, _ := deployResult.Details.(*dockerDeployResult) - require.Equal(t, "contoso.azurecr.io/nginx", dockerDeployDetails.RemoteImageTag) - - _, dockerTagCalled := mockResults["docker-tag"] - _, dockerPullCalled := mockResults["docker-pull"] - _, dockerPushCalled := mockResults["docker-push"] - - require.True(t, dockerTagCalled) - require.True(t, dockerPullCalled) - require.True(t, dockerPushCalled) - mockContainerRegistryService.AssertCalled(t, "Login", *mockContext.Context, "SUBSCRIPTION_ID", "contoso.azurecr.io") - }) - - t.Run("No registry and public image", func(t *testing.T) { - mockContext := mocks.NewMockContext(context.Background()) - mockResults := setupDockerMocks(mockContext) - env := environment.NewWithValues("dev", map[string]string{}) - dockerCli := docker.NewDocker(mockContext.CommandRunner) - envManager := &mockenv.MockEnvManager{} - envManager.On("Save", *mockContext.Context, env).Return(nil) - - mockContainerRegistryService := &mockContainerRegistryService{} - setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) - - containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) - serviceConfig := createTestServiceConfig("", ContainerAppTarget, ServiceLanguageDocker) - - packageOutput := &ServicePackageResult{ - Details: &dockerPackageResult{ - ImageHash: "", - ImageTag: "nginx", + expectError: true, + expectDockerPullCalled: false, + expectDockerTagCalled: false, + expectDockerPushCalled: false, + }, + { + name: "Source code with existing package path", + project: "./src/api", + registry: NewExpandableString("contoso.azurecr.io"), + packagePath: "my-project/my-service:azd-deploy-0", + expectedRemoteImage: "contoso.azurecr.io/my-project/my-service:azd-deploy-0", + expectDockerPullCalled: false, + expectDockerTagCalled: true, + expectDockerPushCalled: true, + expectError: false, + }, + { + name: "Source image and registry", + image: "nginx", + registry: NewExpandableString("contoso.azurecr.io"), + dockerDetails: &dockerPackageResult{ + ImageHash: "", + SourceImage: "nginx", + TargetImage: "my-project/nginx:azd-deploy-0", }, - } - - targetResource := environment.NewTargetResource( - "SUBSCRIPTION_ID", - "RESOURCE_GROUP", - "CONTAINER_APP", - "Microsoft.App/containerApps", - ) - - deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, packageOutput, targetResource, true) - logProgress(deployTask) - deployResult, err := deployTask.Await() - - require.NoError(t, err) - require.NotNil(t, deployResult) - - dockerDeployDetails, _ := deployResult.Details.(*dockerDeployResult) - require.Equal(t, "nginx", dockerDeployDetails.RemoteImageTag) - - _, dockerTagCalled := mockResults["docker-tag"] - _, dockerPullCalled := mockResults["docker-pull"] - _, dockerPushCalled := mockResults["docker-push"] - - require.False(t, dockerTagCalled) - require.False(t, dockerPullCalled) - require.False(t, dockerPushCalled) - mockContainerRegistryService.AssertNotCalled(t, "Login") - }) - - t.Run("Code without registry", func(t *testing.T) { - mockContext := mocks.NewMockContext(context.Background()) - env := environment.NewWithValues("dev", map[string]string{}) - dockerCli := docker.NewDocker(mockContext.CommandRunner) - envManager := &mockenv.MockEnvManager{} - envManager.On("Save", *mockContext.Context, env).Return(nil) - - mockContainerRegistryService := &mockContainerRegistryService{} - setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) - - containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) - serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) - - targetResource := environment.NewTargetResource( - "SUBSCRIPTION_ID", - "RESOURCE_GROUP", - "CONTAINER_APP", - "Microsoft.App/containerApps", - ) + expectDockerPullCalled: true, + expectDockerTagCalled: true, + expectDockerPushCalled: true, + expectedRemoteImage: "contoso.azurecr.io/my-project/nginx:azd-deploy-0", + expectError: false, + }, + { + name: "Source image and no registry", + image: "nginx", + dockerDetails: &dockerPackageResult{ + ImageHash: "", + SourceImage: "nginx", + TargetImage: "my-project/nginx:azd-deploy-0", + }, + expectDockerPullCalled: false, + expectDockerTagCalled: false, + expectDockerPushCalled: false, + expectedRemoteImage: "nginx", + expectError: false, + }, + { + name: "Source image with existing package path and registry", + registry: NewExpandableString("contoso.azurecr.io"), + packagePath: "my-project/my-service:azd-deploy-0", + expectedRemoteImage: "contoso.azurecr.io/my-project/my-service:azd-deploy-0", + expectDockerPullCalled: false, + expectDockerTagCalled: true, + expectDockerPushCalled: true, + expectError: false, + }, + { + name: "Missing target image", + dockerDetails: &dockerPackageResult{}, + expectError: true, + expectDockerPullCalled: false, + expectDockerTagCalled: false, + expectDockerPushCalled: false, + }, + } - deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, nil, targetResource, true) - logProgress(deployTask) - deployResult, err := deployTask.Await() + targetResource := environment.NewTargetResource( + "SUBSCRIPTION_ID", + "RESOURCE_GROUP", + "CONTAINER_APP", + "Microsoft.App/containerApps", + ) - // Expected to fail when no registry is specified - require.Error(t, err) - require.Nil(t, deployResult) - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + mockResults := setupDockerMocks(mockContext) + env := environment.NewWithValues("dev", map[string]string{}) + dockerCli := docker.NewDocker(mockContext.CommandRunner) + envManager := &mockenv.MockEnvManager{} + envManager.On("Save", *mockContext.Context, env).Return(nil) + + mockContainerRegistryService := &mockContainerRegistryService{} + setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) + + containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), mockContainerRegistryService, dockerCli) + serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) + + serviceConfig.Image = tt.image + serviceConfig.RelativePath = tt.project + serviceConfig.Docker.Registry = tt.registry + + packageOutput := &ServicePackageResult{ + Details: tt.dockerDetails, + PackagePath: tt.packagePath, + } + + deployTask := containerHelper.Deploy(*mockContext.Context, serviceConfig, packageOutput, targetResource, true) + logProgress(deployTask) + deployResult, err := deployTask.Await() + + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Same(t, packageOutput, deployResult.Package) + + dockerDeployResult, ok := deployResult.Details.(*dockerDeployResult) + require.True(t, ok) + require.Equal(t, tt.expectedRemoteImage, dockerDeployResult.RemoteImageTag) + } + + _, dockerPullCalled := mockResults["docker-pull"] + _, dockerTagCalled := mockResults["docker-tag"] + _, dockerPushCalled := mockResults["docker-push"] + + require.Equal(t, tt.expectDockerPullCalled, dockerPullCalled) + require.Equal(t, tt.expectDockerTagCalled, dockerTagCalled) + require.Equal(t, tt.expectDockerPushCalled, dockerPushCalled) + }) + } } func setupContainerRegistryMocks(mockContext *mocks.MockContext, mockContainerRegistryService *mock.Mock) { @@ -343,6 +326,11 @@ func setupDockerMocks(mockContext *mocks.MockContext) map[string]exec.RunArgs { return strings.Contains(command, "docker tag") }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { mockResults["docker-tag"] = args + + if args.Args[1] == "" || args.Args[2] == "" { + return exec.NewRunResult(1, "", ""), errors.New("no image or tag") + } + return exec.NewRunResult(0, "", ""), nil }) @@ -350,6 +338,11 @@ func setupDockerMocks(mockContext *mocks.MockContext) map[string]exec.RunArgs { return strings.Contains(command, "docker push") }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { mockResults["docker-push"] = args + + if args.Args[1] == "" { + return exec.NewRunResult(1, "", ""), errors.New("no image") + } + return exec.NewRunResult(0, "", ""), nil }) @@ -357,6 +350,11 @@ func setupDockerMocks(mockContext *mocks.MockContext) map[string]exec.RunArgs { return strings.Contains(command, "docker pull") }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { mockResults["docker-pull"] = args + + if args.Args[1] == "" { + return exec.NewRunResult(1, "", ""), errors.New("no image") + } + return exec.NewRunResult(0, "", ""), nil }) diff --git a/cli/azd/pkg/project/framework_service_docker.go b/cli/azd/pkg/project/framework_service_docker.go index ca96759327d..695c3985862 100644 --- a/cli/azd/pkg/project/framework_service_docker.go +++ b/cli/azd/pkg/project/framework_service_docker.go @@ -31,8 +31,6 @@ import ( "go.opentelemetry.io/otel/trace" ) -var DefaultDockerProjectOptions DockerProjectOptions = DockerProjectOptions{} - type DockerProjectOptions struct { Path string `yaml:"path,omitempty" json:"path,omitempty"` Context string `yaml:"context,omitempty" json:"context,omitempty"` @@ -63,22 +61,39 @@ func (dbr *dockerBuildResult) MarshalJSON() ([]byte, error) { } type dockerPackageResult struct { + // The image hash that is generated from a docker build ImageHash string `json:"imageHash"` - ImageTag string `json:"imageTag"` + // The external source image specified when not building from source + SourceImage string `json:"sourceImage"` + // The target image with tag that is used for publishing and deployment when targeting a container registry + TargetImage string `json:"targetImage"` } func (dpr *dockerPackageResult) ToString(currentIndentation string) string { - imageHash := dpr.ImageHash - if imageHash == "" { - imageHash = "N/A" + builder := strings.Builder{} + if dpr.ImageHash != "" { + builder.WriteString(fmt.Sprintf("%s- Image Hash: %s\n", currentIndentation, output.WithLinkFormat(dpr.ImageHash))) } - lines := []string{ - fmt.Sprintf("%s- Image Hash: %s", currentIndentation, output.WithLinkFormat(imageHash)), - fmt.Sprintf("%s- Image Tag: %s", currentIndentation, output.WithLinkFormat(dpr.ImageTag)), + if dpr.SourceImage != "" { + builder.WriteString( + fmt.Sprintf("%s- Source Image: %s\n", + currentIndentation, + output.WithLinkFormat(dpr.SourceImage), + ), + ) } - return strings.Join(lines, "\n") + if dpr.TargetImage != "" { + builder.WriteString( + fmt.Sprintf("%s- Target Image: %s\n", + currentIndentation, + output.WithLinkFormat(dpr.TargetImage), + ), + ) + } + + return builder.String() } func (dpr *dockerPackageResult) MarshalJSON() ([]byte, error) { @@ -277,29 +292,51 @@ func (p *dockerProject) Package( imageId = buildOutput.BuildOutputPath } - localTag, err := p.containerHelper.LocalImageTag(ctx, serviceConfig) + packageDetails := &dockerPackageResult{ + ImageHash: imageId, + } + + // If we don't have an image ID from a docker build then an external source image is being used + if imageId == "" { + sourceImage, err := docker.ParseContainerImage(serviceConfig.Image) + if err != nil { + task.SetError(fmt.Errorf("parsing source container image: %w", err)) + return + } + + remoteImageUrl := sourceImage.Remote() + + task.SetProgress(NewServiceProgress("Pulling container source image")) + if err := p.docker.Pull(ctx, remoteImageUrl); err != nil { + task.SetError(fmt.Errorf("pulling source container image: %w", err)) + return + } + + imageId = remoteImageUrl + packageDetails.SourceImage = remoteImageUrl + } + + // Generate a local tag from the 'docker' configuration section of the service + imageWithTag, err := p.containerHelper.LocalImageTag(ctx, serviceConfig) if err != nil { task.SetError(fmt.Errorf("generating local image tag: %w", err)) return } - if imageId != "" { - // Tag image. - log.Printf("tagging image %s as %s", imageId, localTag) - task.SetProgress(NewServiceProgress("Tagging Docker image")) - if err := p.docker.Tag(ctx, serviceConfig.Path(), imageId, localTag); err != nil { - task.SetError(fmt.Errorf("tagging image: %w", err)) - return - } + // Tag image. + log.Printf("tagging image %s as %s", imageId, imageWithTag) + task.SetProgress(NewServiceProgress("Tagging container image")) + if err := p.docker.Tag(ctx, serviceConfig.Path(), imageId, imageWithTag); err != nil { + task.SetError(fmt.Errorf("tagging image: %w", err)) + return } + packageDetails.TargetImage = imageWithTag + task.SetResult(&ServicePackageResult{ Build: buildOutput, - PackagePath: localTag, - Details: &dockerPackageResult{ - ImageHash: imageId, - ImageTag: localTag, - }, + PackagePath: packageDetails.SourceImage, + Details: packageDetails, }) }, ) diff --git a/cli/azd/pkg/project/framework_service_docker_test.go b/cli/azd/pkg/project/framework_service_docker_test.go index 0a66a7db073..f23b1908fa2 100644 --- a/cli/azd/pkg/project/framework_service_docker_test.go +++ b/cli/azd/pkg/project/framework_service_docker_test.go @@ -308,59 +308,137 @@ func Test_DockerProject_Build(t *testing.T) { } func Test_DockerProject_Package(t *testing.T) { - var runArgs exec.RunArgs - - mockContext := mocks.NewMockContext(context.Background()) - envManager := &mockenv.MockEnvManager{} - - mockContext.CommandRunner. - When(func(args exec.RunArgs, command string) bool { - return strings.Contains(command, "docker tag") - }). - RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { - runArgs = args - return exec.NewRunResult(0, "IMAGE_ID", ""), nil - }) - - env := environment.NewWithValues("test", map[string]string{}) - dockerCli := docker.NewDocker(mockContext.CommandRunner) - serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) - serviceConfig.Docker.Registry = NewExpandableString("contoso.azurecr.io") - - npmProject := NewNpmProject(npm.NewNpmCli(mockContext.CommandRunner), env) - dockerProject := NewDockerProject( - env, - dockerCli, - NewContainerHelper(env, envManager, clock.NewMock(), nil, dockerCli), - mockinput.NewMockConsole(), - mockContext.AlphaFeaturesManager, - mockContext.CommandRunner) - dockerProject.SetSource(npmProject) - - packageTask := dockerProject.Package( - *mockContext.Context, - serviceConfig, - &ServiceBuildResult{ - BuildOutputPath: "IMAGE_ID", + tests := []struct { + name string + image string + project string + docker DockerProjectOptions + expectedPackageResult dockerPackageResult + expectDockerPullCalled bool + expectDockerTagCalled bool + }{ + { + name: "source with defaults", + project: "./src/api", + expectedPackageResult: dockerPackageResult{ + ImageHash: "IMAGE_ID", + SourceImage: "", + TargetImage: "test-app/api-test:azd-deploy-0", + }, + expectDockerPullCalled: false, + expectDockerTagCalled: true, }, - ) - logProgress(packageTask) - - result, err := packageTask.Await() - require.NoError(t, err) - require.NotNil(t, result) - require.IsType(t, new(dockerPackageResult), result.Details) - - packageResult, ok := result.Details.(*dockerPackageResult) - require.Equal(t, "test-app/api-test:azd-deploy-0", result.PackagePath) + { + name: "source with custom docker options", + project: "./src/api", + docker: DockerProjectOptions{ + Image: NewExpandableString("foo/bar"), + Tag: NewExpandableString("latest"), + }, + expectedPackageResult: dockerPackageResult{ + ImageHash: "IMAGE_ID", + SourceImage: "", + TargetImage: "foo/bar:latest", + }, + expectDockerPullCalled: false, + expectDockerTagCalled: true, + }, + { + name: "image with defaults", + image: "nginx:latest", + expectedPackageResult: dockerPackageResult{ + ImageHash: "", + SourceImage: "nginx:latest", + TargetImage: "test-app/api-test:azd-deploy-0", + }, + expectDockerPullCalled: true, + expectDockerTagCalled: true, + }, + { + name: "image with custom docker options", + image: "nginx:latest", + docker: DockerProjectOptions{ + Image: NewExpandableString("foo/bar"), + Tag: NewExpandableString("latest"), + }, + expectedPackageResult: dockerPackageResult{ + ImageHash: "", + SourceImage: "nginx:latest", + TargetImage: "foo/bar:latest", + }, + expectDockerPullCalled: true, + expectDockerTagCalled: true, + }, + { + name: "fully qualified image with custom docker options", + image: "docker.io/repository/iamge:latest", + docker: DockerProjectOptions{ + Image: NewExpandableString("myapp-service"), + Tag: NewExpandableString("latest"), + }, + expectedPackageResult: dockerPackageResult{ + ImageHash: "", + SourceImage: "docker.io/repository/iamge:latest", + TargetImage: "myapp-service:latest", + }, + expectDockerPullCalled: true, + expectDockerTagCalled: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + mockResults := setupDockerMocks(mockContext) + envManager := &mockenv.MockEnvManager{} + + env := environment.NewWithValues("test", map[string]string{}) + dockerCli := docker.NewDocker(mockContext.CommandRunner) + serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) + + dockerProject := NewDockerProject( + env, + dockerCli, + NewContainerHelper(env, envManager, clock.NewMock(), nil, dockerCli), + mockinput.NewMockConsole(), + mockContext.AlphaFeaturesManager, + mockContext.CommandRunner) + + // Set the custom test options + serviceConfig.Docker = tt.docker + serviceConfig.RelativePath = tt.project + serviceConfig.Image = tt.image + + if serviceConfig.RelativePath != "" { + npmProject := NewNpmProject(npm.NewNpmCli(mockContext.CommandRunner), env) + dockerProject.SetSource(npmProject) + } + + buildOutputPath := "" + if serviceConfig.Image == "" && serviceConfig.RelativePath != "" { + buildOutputPath = "IMAGE_ID" + } + + packageTask := dockerProject.Package( + *mockContext.Context, + serviceConfig, + &ServiceBuildResult{ + BuildOutputPath: buildOutputPath, + }, + ) + logProgress(packageTask) + + result, err := packageTask.Await() + require.NoError(t, err) + dockerDetails, ok := result.Details.(*dockerPackageResult) + require.True(t, ok) + require.Equal(t, tt.expectedPackageResult, *dockerDetails) - require.True(t, ok) - require.Equal(t, "test-app/api-test:azd-deploy-0", packageResult.ImageTag) + _, dockerPullCalled := mockResults["docker-pull"] + _, dockerTagCalled := mockResults["docker-tag"] - require.Equal(t, "docker", runArgs.Cmd) - require.Equal(t, serviceConfig.RelativePath, runArgs.Cwd) - require.Equal(t, - []string{"tag", "IMAGE_ID", "test-app/api-test:azd-deploy-0"}, - runArgs.Args, - ) + require.Equal(t, tt.expectDockerPullCalled, dockerPullCalled) + require.Equal(t, tt.expectDockerTagCalled, dockerTagCalled) + }) + } } diff --git a/cli/azd/pkg/project/service_config.go b/cli/azd/pkg/project/service_config.go index 7ec19fdf49d..2b8ab6ae9c8 100644 --- a/cli/azd/pkg/project/service_config.go +++ b/cli/azd/pkg/project/service_config.go @@ -23,7 +23,9 @@ type ServiceConfig struct { Language ServiceLanguageKind `yaml:"language"` // The output path for build artifacts OutputPath string `yaml:"dist,omitempty"` - // The optional docker options + // The source image to use for container based applications + Image string `yaml:"image,omitempty"` + // The optional docker options for configuring the output image Docker DockerProjectOptions `yaml:"docker,omitempty"` // The optional K8S / AKS options K8s AksOptions `yaml:"k8s,omitempty"` diff --git a/cli/azd/pkg/project/service_manager.go b/cli/azd/pkg/project/service_manager.go index 9e76afb1ba1..4120936aba4 100644 --- a/cli/azd/pkg/project/service_manager.go +++ b/cli/azd/pkg/project/service_manager.go @@ -9,7 +9,6 @@ import ( "log" "os" "path/filepath" - "reflect" "strings" "github.com/azure/azure-dev/cli/azd/pkg/alpha" @@ -552,8 +551,7 @@ func (sm *serviceManager) GetServiceTarget(ctx context.Context, serviceConfig *S func (sm *serviceManager) GetFrameworkService(ctx context.Context, serviceConfig *ServiceConfig) (FrameworkService, error) { var frameworkService FrameworkService - if serviceConfig.Language == ServiceLanguageNone && - !reflect.DeepEqual(serviceConfig.Docker, DefaultDockerProjectOptions) { + if serviceConfig.Language == ServiceLanguageNone && serviceConfig.Image != "" { serviceConfig.Language = ServiceLanguageDocker } diff --git a/cli/azd/pkg/project/service_manager_test.go b/cli/azd/pkg/project/service_manager_test.go index 4c37a12a13f..ecbafc90742 100644 --- a/cli/azd/pkg/project/service_manager_test.go +++ b/cli/azd/pkg/project/service_manager_test.go @@ -241,7 +241,7 @@ func Test_ServiceManager_GetFrameworkService(t *testing.T) { env := environment.New("test") sm := createServiceManager(mockContext, env, ServiceOperationCache{}) serviceConfig := createTestServiceConfig("", ServiceTargetFake, ServiceLanguageNone) - serviceConfig.Docker.Tag = NewExpandableString("nginx") + serviceConfig.Image = "nginx" framework, err := sm.GetFrameworkService(*mockContext.Context, serviceConfig) require.NoError(t, err) diff --git a/cli/azd/pkg/project/service_models_test.go b/cli/azd/pkg/project/service_models_test.go index 563f6cbee7a..11966c6d22d 100644 --- a/cli/azd/pkg/project/service_models_test.go +++ b/cli/azd/pkg/project/service_models_test.go @@ -26,8 +26,8 @@ func Test_ServiceResults_Json_Marshal(t *testing.T) { }, PackagePath: "package/path/project.zip", Details: &dockerPackageResult{ - ImageHash: "image-hash", - ImageTag: "image-tag", + ImageHash: "image-hash", + SourceImage: "image-tag", }, }, } diff --git a/cli/azd/pkg/project/service_target_aks_test.go b/cli/azd/pkg/project/service_target_aks_test.go index 7ec387fab8a..feded94a772 100644 --- a/cli/azd/pkg/project/service_target_aks_test.go +++ b/cli/azd/pkg/project/service_target_aks_test.go @@ -91,8 +91,8 @@ func Test_Package_Deploy_HappyPath(t *testing.T) { &ServicePackageResult{ PackagePath: "test-app/api-test:azd-deploy-0", Details: &dockerPackageResult{ - ImageHash: "IMAGE_HASH", - ImageTag: "test-app/api-test:azd-deploy-0", + ImageHash: "IMAGE_HASH", + SourceImage: "test-app/api-test:azd-deploy-0", }, }, ) diff --git a/cli/azd/pkg/project/service_target_containerapp_test.go b/cli/azd/pkg/project/service_target_containerapp_test.go index 17c7b032029..b8f901760d7 100644 --- a/cli/azd/pkg/project/service_target_containerapp_test.go +++ b/cli/azd/pkg/project/service_target_containerapp_test.go @@ -89,8 +89,8 @@ func Test_ContainerApp_Deploy(t *testing.T) { &ServicePackageResult{ PackagePath: "test-app/api-test:azd-deploy-0", Details: &dockerPackageResult{ - ImageHash: "IMAGE_HASH", - ImageTag: "test-app/api-test:azd-deploy-0", + ImageHash: "IMAGE_HASH", + SourceImage: "test-app/api-test:azd-deploy-0", }, }, ) diff --git a/cli/azd/pkg/tools/docker/container_image.go b/cli/azd/pkg/tools/docker/container_image.go index 9170309e5f7..fb7f969383e 100644 --- a/cli/azd/pkg/tools/docker/container_image.go +++ b/cli/azd/pkg/tools/docker/container_image.go @@ -8,11 +8,11 @@ import ( // ContainerImage represents a container image and its components type ContainerImage struct { // The registry name - Registry string + Registry string `json:"registry,omitempty"` // The repository name or path - Repository string + Repository string `json:"repository,omitempty"` // The tag - Tag string + Tag string `json:"tag,omitempty"` } // Local returns the local image name without registry @@ -58,29 +58,38 @@ func ParseContainerImage(image string) (*ContainerImage, error) { } containerImage := &ContainerImage{} + imageWithTag := image + slashParts := strings.Split(imageWithTag, "/") // Detect tags - tagParts := strings.Split(image, ":") + if len(slashParts) > 1 { + imageWithTag = slashParts[len(slashParts)-1] + } + + tagParts := strings.Split(imageWithTag, ":") if len(tagParts) > 2 { - return containerImage, errors.New("invalid tag format") + return nil, errors.New("invalid tag format") } if len(tagParts) == 2 { containerImage.Tag = tagParts[1] - image = tagParts[0] } - // Split the imageURL by "/" - parts := strings.Split(image, "/") + allParts := slashParts[:len(slashParts)-1] + allParts = append(allParts, tagParts[0]) // Check if the parts contain a registry (parts[0] contains ".") - if strings.Contains(parts[0], ".") { - containerImage.Registry = parts[0] - parts = parts[1:] + if strings.Contains(allParts[0], ".") { + containerImage.Registry = allParts[0] + allParts = allParts[1:] } // Set the repository as the remaining parts joined by "/" - containerImage.Repository = strings.Join(parts, "/") + containerImage.Repository = strings.Join(allParts, "/") + + if containerImage.Repository == "" { + return nil, errors.New("empty repository") + } return containerImage, nil } diff --git a/cli/azd/pkg/tools/docker/container_image_test.go b/cli/azd/pkg/tools/docker/container_image_test.go index da94fb26681..6831330f95d 100644 --- a/cli/azd/pkg/tools/docker/container_image_test.go +++ b/cli/azd/pkg/tools/docker/container_image_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_ParseContainerImage(t *testing.T) { +func Test_ParseContainerImage_Success(t *testing.T) { tests := []struct { name string input string @@ -22,7 +22,7 @@ func Test_ParseContainerImage(t *testing.T) { }, }, { - name: "image with registry, no tag", + name: "image with registry and no tag", input: "registry.example.com/my-image", expected: ContainerImage{ Registry: "registry.example.com", @@ -57,6 +57,15 @@ func Test_ParseContainerImage(t *testing.T) { Tag: "1.0", }, }, + { + name: "image with host and port", + input: "registry.example.com:5000/my-image:1.0", + expected: ContainerImage{ + Registry: "registry.example.com:5000", + Repository: "my-image", + Tag: "1.0", + }, + }, } for _, tt := range tests { @@ -68,6 +77,43 @@ func Test_ParseContainerImage(t *testing.T) { } } +func Test_ParseContainerImage_Invalid(t *testing.T) { + tests := []struct { + name string + input string + }{ + { + name: "empty image", + input: "", + }, + { + name: "image with only tag", + input: ":1.0", + }, + { + name: "image with multiple tags", + input: "my-image:1.0:latest", + }, + { + name: "image with only registry", + input: "registry.example.com", + }, + { + name: "image with only registry and tag", + input: "registry.example.com:1.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual, err := ParseContainerImage(tt.input) + require.Error(t, err) + require.Nil(t, actual) + }) + } + +} + func Test_ContainerImage_Local_And_Remote(t *testing.T) { tests := []struct { name string diff --git a/schemas/alpha/azure.yaml.json b/schemas/alpha/azure.yaml.json index a780365567e..6b3dc4ba9ca 100644 --- a/schemas/alpha/azure.yaml.json +++ b/schemas/alpha/azure.yaml.json @@ -78,6 +78,11 @@ "type": "string", "title": "Path to the service source code directory" }, + "image": { + "type": "string", + "title": "Optional. The source image to be used for the container image instead of building from source.", + "description": "If omitted, container image will be built from source specified in the 'project' property. Setting both 'project' and 'image' is invalid." + }, "host": { "type": "string", "title": "Required. The type of Azure resource used for service implementation", @@ -160,6 +165,58 @@ } }, "allOf": [ + { + "if": { + "properties": { + "host": { + "const": "containerapp" + } + } + }, + "then": { + "anyOf": [ + { + "required": [ + "image" + ], + "properties": { + "language": false + }, + "not": { + "required": [ + "project" + ] + } + }, + { + "required": [ + "project" + ], + "not": { + "required": [ + "image" + ] + } + } + ] + } + }, + { + "if": { + "not": { + "properties": { + "host": { + "const": "containerapp" + } + } + } + }, + "then": { + "properties": { + "image": false + } + } + }, { "if": { "not": { @@ -550,7 +607,7 @@ }, "image": { "type": "string", - "title": "Optional. The name of the container image to use for the service.", + "title": "Optional. The name that will be applied to the built container image.", "description": "If omitted, will default to the '{appName}/{serviceName}-{environmentName}'. Supports environment variable substitution." }, "tag": { From 681b27f3599c5684209d3a60d6bc58630fb473fe Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Tue, 6 Feb 2024 17:17:35 -0800 Subject: [PATCH 08/10] Fixes failing unit tests --- cli/azd/pkg/project/service_models_test.go | 2 +- cli/azd/pkg/project/service_target_aks_test.go | 2 +- cli/azd/pkg/project/service_target_containerapp_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/azd/pkg/project/service_models_test.go b/cli/azd/pkg/project/service_models_test.go index 11966c6d22d..9f4b02f47dd 100644 --- a/cli/azd/pkg/project/service_models_test.go +++ b/cli/azd/pkg/project/service_models_test.go @@ -27,7 +27,7 @@ func Test_ServiceResults_Json_Marshal(t *testing.T) { PackagePath: "package/path/project.zip", Details: &dockerPackageResult{ ImageHash: "image-hash", - SourceImage: "image-tag", + TargetImage: "image-tag", }, }, } diff --git a/cli/azd/pkg/project/service_target_aks_test.go b/cli/azd/pkg/project/service_target_aks_test.go index feded94a772..0a0405fccf9 100644 --- a/cli/azd/pkg/project/service_target_aks_test.go +++ b/cli/azd/pkg/project/service_target_aks_test.go @@ -92,7 +92,7 @@ func Test_Package_Deploy_HappyPath(t *testing.T) { PackagePath: "test-app/api-test:azd-deploy-0", Details: &dockerPackageResult{ ImageHash: "IMAGE_HASH", - SourceImage: "test-app/api-test:azd-deploy-0", + TargetImage: "test-app/api-test:azd-deploy-0", }, }, ) diff --git a/cli/azd/pkg/project/service_target_containerapp_test.go b/cli/azd/pkg/project/service_target_containerapp_test.go index b8f901760d7..294c2c1925a 100644 --- a/cli/azd/pkg/project/service_target_containerapp_test.go +++ b/cli/azd/pkg/project/service_target_containerapp_test.go @@ -90,7 +90,7 @@ func Test_ContainerApp_Deploy(t *testing.T) { PackagePath: "test-app/api-test:azd-deploy-0", Details: &dockerPackageResult{ ImageHash: "IMAGE_HASH", - SourceImage: "test-app/api-test:azd-deploy-0", + TargetImage: "test-app/api-test:azd-deploy-0", }, }, ) From a78d15490bc5fe9759412f73be312622d3e89d49 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Wed, 7 Feb 2024 15:08:10 -0800 Subject: [PATCH 09/10] Adds addidtional unit tests for image name/tag generation --- cli/azd/pkg/project/container_helper.go | 4 +- cli/azd/pkg/project/container_helper_test.go | 216 ++++++++++++++++--- 2 files changed, 195 insertions(+), 25 deletions(-) diff --git a/cli/azd/pkg/project/container_helper.go b/cli/azd/pkg/project/container_helper.go index 6f19d06e560..a6b01da8710 100644 --- a/cli/azd/pkg/project/container_helper.go +++ b/cli/azd/pkg/project/container_helper.go @@ -145,7 +145,9 @@ func (ch *ContainerHelper) RemoteImageTag( return "", err } - containerImage.Registry = registryName + if registryName != "" { + containerImage.Registry = registryName + } return containerImage.Remote(), nil } diff --git a/cli/azd/pkg/project/container_helper_test.go b/cli/azd/pkg/project/container_helper_test.go index cd422e25a0d..f74da540f0f 100644 --- a/cli/azd/pkg/project/container_helper_test.go +++ b/cli/azd/pkg/project/container_helper_test.go @@ -35,8 +35,6 @@ func Test_ContainerHelper_LocalImageTag(t *testing.T) { } defaultImageName := fmt.Sprintf("%s/%s-%s", projectName, serviceName, envName) - envManager := &mockenv.MockEnvManager{} - tests := []struct { name string dockerConfig DockerProjectOptions @@ -58,7 +56,7 @@ func Test_ContainerHelper_LocalImageTag(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { env := environment.NewWithValues("dev", map[string]string{}) - containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), nil, nil) + containerHelper := NewContainerHelper(env, nil, clock.NewMock(), nil, nil) serviceConfig.Docker = tt.dockerConfig tag, err := containerHelper.LocalImageTag(*mockContext.Context, serviceConfig) @@ -69,31 +67,62 @@ func Test_ContainerHelper_LocalImageTag(t *testing.T) { } func Test_ContainerHelper_RemoteImageTag(t *testing.T) { - mockContext := mocks.NewMockContext(context.Background()) - env := environment.NewWithValues("dev", map[string]string{ - environment.ContainerRegistryEndpointEnvVarName: "contoso.azurecr.io", - }) - envManager := &mockenv.MockEnvManager{} - containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), nil, nil) - serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) - localTag, err := containerHelper.LocalImageTag(*mockContext.Context, serviceConfig) - require.NoError(t, err) - remoteTag, err := containerHelper.RemoteImageTag(*mockContext.Context, serviceConfig, localTag) - require.NoError(t, err) - require.Equal(t, "contoso.azurecr.io/test-app/api-dev:azd-deploy-0", remoteTag) -} + tests := []struct { + name string + project string + localImageTag string + registry ExpandableString + expectedRemoteTag string + expectError bool + }{ + { + name: "with registry", + project: "./src/api", + registry: NewExpandableString("contoso.azurecr.io"), + localImageTag: "test-app/api-dev:azd-deploy-0", + expectedRemoteTag: "contoso.azurecr.io/test-app/api-dev:azd-deploy-0", + }, + { + name: "with no registry and custom image", + project: "", + localImageTag: "docker.io/org/my-custom-image:latest", + expectedRemoteTag: "docker.io/org/my-custom-image:latest", + }, + { + name: "registry with fully qualified custom image", + project: "", + registry: NewExpandableString("contoso.azurecr.io"), + localImageTag: "docker.io/org/my-custom-image:latest", + expectedRemoteTag: "contoso.azurecr.io/org/my-custom-image:latest", + }, + { + name: "missing registry with local project", + project: "./src/api", + localImageTag: "test-app/api-dev:azd-deploy-0", + expectError: true, + }, + } -func Test_ContainerHelper_RemoteImageTag_NoContainer_Registry(t *testing.T) { mockContext := mocks.NewMockContext(context.Background()) + env := environment.NewWithValues("dev", map[string]string{}) + containerHelper := NewContainerHelper(env, nil, clock.NewMock(), nil, nil) - env := environment.New("test") - serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) - envManager := &mockenv.MockEnvManager{} - containerHelper := NewContainerHelper(env, envManager, clock.NewMock(), nil, nil) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + serviceConfig := createTestServiceConfig(tt.project, ContainerAppTarget, ServiceLanguageTypeScript) + serviceConfig.Docker.Registry = tt.registry - imageTag, err := containerHelper.RemoteImageTag(*mockContext.Context, serviceConfig, "local_tag") - require.Error(t, err) - require.Empty(t, imageTag) + remoteTag, err := containerHelper.RemoteImageTag(*mockContext.Context, serviceConfig, tt.localImageTag) + + if tt.expectError { + require.Error(t, err) + require.Empty(t, remoteTag) + } else { + require.NoError(t, err) + require.Equal(t, tt.expectedRemoteTag, remoteTag) + } + }) + } } func Test_ContainerHelper_Resolve_RegistryName(t *testing.T) { @@ -310,6 +339,145 @@ func Test_ContainerHelper_Deploy(t *testing.T) { } } +func Test_ContainerHelper_ConfiguredImage(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + env := environment.NewWithValues("dev", map[string]string{}) + containerHelper := NewContainerHelper(env, nil, clock.NewMock(), nil, nil) + + tests := []struct { + name string + projectName string + serviceName string + sourceImage string + env map[string]string + registry ExpandableString + image ExpandableString + tag ExpandableString + expectedImage docker.ContainerImage + expectError bool + expectedErrorMessage string + }{ + { + name: "with defaults", + expectedImage: docker.ContainerImage{ + Registry: "", + Repository: "test-app/api-dev", + Tag: "azd-deploy-0", + }, + }, + { + name: "with custom tag", + tag: NewExpandableString("custom-tag"), + expectedImage: docker.ContainerImage{ + Registry: "", + Repository: "test-app/api-dev", + Tag: "custom-tag", + }, + }, + { + name: "with custom iamge", + image: NewExpandableString("custom-image"), + expectedImage: docker.ContainerImage{ + Registry: "", + Repository: "custom-image", + Tag: "azd-deploy-0", + }, + }, + { + name: "with custom iamge and tag", + image: NewExpandableString("custom-image"), + tag: NewExpandableString("custom-tag"), + expectedImage: docker.ContainerImage{ + Registry: "", + Repository: "custom-image", + Tag: "custom-tag", + }, + }, + { + name: "with registry", + registry: NewExpandableString("contoso.azurecr.io"), + expectedImage: docker.ContainerImage{ + Registry: "contoso.azurecr.io", + Repository: "test-app/api-dev", + Tag: "azd-deploy-0", + }, + }, + { + name: "with registry, custom image and tag", + registry: NewExpandableString("contoso.azurecr.io"), + image: NewExpandableString("custom-image"), + tag: NewExpandableString("custom-tag"), + expectedImage: docker.ContainerImage{ + Registry: "contoso.azurecr.io", + Repository: "custom-image", + Tag: "custom-tag", + }, + }, + { + name: "with expandable overrides", + env: map[string]string{ + "MY_CUSTOM_REGISTRY": "custom.azurecr.io", + "MY_CUSTOM_IMAGE": "custom-image", + "MY_CUSTOM_TAG": "custom-tag", + }, + registry: NewExpandableString("${MY_CUSTOM_REGISTRY}"), + image: NewExpandableString("${MY_CUSTOM_IMAGE}"), + tag: NewExpandableString("${MY_CUSTOM_TAG}"), + expectedImage: docker.ContainerImage{ + Registry: "custom.azurecr.io", + Repository: "custom-image", + Tag: "custom-tag", + }, + }, + { + name: "invalid image name", + image: NewExpandableString("${MISSING_CLOSING_BRACE"), + expectError: true, + expectedErrorMessage: "missing closing brace", + }, + { + name: "invalid tag", + image: NewExpandableString("repo/::latest"), + expectError: true, + expectedErrorMessage: "invalid tag", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) + if tt.projectName != "" { + serviceConfig.Project.Name = tt.projectName + } + if tt.serviceName != "" { + serviceConfig.Name = tt.serviceName + } + serviceConfig.Image = tt.sourceImage + serviceConfig.Docker.Registry = tt.registry + serviceConfig.Docker.Image = tt.image + serviceConfig.Docker.Tag = tt.tag + + for k, v := range tt.env { + env.DotenvSet(k, v) + } + + image, err := containerHelper.GeneratedImage(*mockContext.Context, serviceConfig) + + if tt.expectError { + require.Error(t, err) + require.Nil(t, image) + if tt.expectedErrorMessage != "" { + require.Contains(t, err.Error(), tt.expectedErrorMessage) + } + } else { + require.NoError(t, err) + require.NotNil(t, image) + require.Equal(t, tt.expectedImage, *image) + } + }) + } +} + func setupContainerRegistryMocks(mockContext *mocks.MockContext, mockContainerRegistryService *mock.Mock) { mockContainerRegistryService.On( "Login", From b5a76e1ab1c25fb357e85a7467b4e6848ebb2974 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Wed, 7 Feb 2024 15:27:55 -0800 Subject: [PATCH 10/10] Addresses PR feedback --- cli/azd/pkg/project/project.go | 7 +++++++ cli/azd/pkg/project/service_manager.go | 1 + 2 files changed, 8 insertions(+) diff --git a/cli/azd/pkg/project/project.go b/cli/azd/pkg/project/project.go index e3595acbcb4..854a8ec5419 100644 --- a/cli/azd/pkg/project/project.go +++ b/cli/azd/pkg/project/project.go @@ -102,6 +102,13 @@ func Parse(ctx context.Context, yamlContent string) (*ProjectConfig, error) { if err != nil { return nil, fmt.Errorf("parsing service %s: %w", svc.Name, err) } + + // TODO: Move parsing/validation requirements for service targets into their respective components. + // When working within container based applications users may be using external/pre-built images instead of source + // In this case it is valid to have not specified a language but would be required to specify a source image + if svc.Host == ContainerAppTarget && svc.Language == ServiceLanguageNone && svc.Image == "" { + return nil, fmt.Errorf("parsing service %s: must specify language or image", svc.Name) + } } return &projectConfig, nil diff --git a/cli/azd/pkg/project/service_manager.go b/cli/azd/pkg/project/service_manager.go index 4120936aba4..7495fc5f58a 100644 --- a/cli/azd/pkg/project/service_manager.go +++ b/cli/azd/pkg/project/service_manager.go @@ -551,6 +551,7 @@ func (sm *serviceManager) GetServiceTarget(ctx context.Context, serviceConfig *S func (sm *serviceManager) GetFrameworkService(ctx context.Context, serviceConfig *ServiceConfig) (FrameworkService, error) { var frameworkService FrameworkService + // Publishing from an existing image currently follows the same lifecycle as a docker project if serviceConfig.Language == ServiceLanguageNone && serviceConfig.Image != "" { serviceConfig.Language = ServiceLanguageDocker }