Skip to content

Commit 6e5059e

Browse files
JeffreyCACopilot
andauthored
Pass buildArgs to ACR remote build task (#7997)
* Pass buildArgs to ACR remote build task Docker buildArgs and buildEnv defined in azure.yaml were silently dropped when `remoteBuild: true` was set because runRemoteBuild never forwarded them to the ACR DockerBuildRequest. Resolve buildArgs (with environment variable substitution and parameter resolution) and buildEnv the same way the local Docker build does, then convert each entry into an armcontainerregistry.Argument and attach it to the build request. Shorthand `NAME` entries are looked up from os.Environ, the azd environment, and buildEnv (in increasing precedence). The shared resolution logic is extracted into resolveDockerBuildArgs / resolveDockerParameters so local and remote builds stay in sync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR feedback: fix stale comment, preallocate slice, clarify error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e32b523 commit 6e5059e

2 files changed

Lines changed: 382 additions & 32 deletions

File tree

cli/azd/pkg/project/container_helper.go

Lines changed: 127 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -356,42 +356,13 @@ func (ch *ContainerHelper) Build(
356356
dockerOptions := getDockerOptionsWithDefaults(serviceConfig.Docker)
357357
resolveDockerPaths(serviceConfig, &dockerOptions)
358358

359-
resolveParameters := func(source []string) ([]string, error) {
360-
result := make([]string, len(source))
361-
for i, arg := range source {
362-
evaluatedString, err := apphost.EvalString(arg, func(match string) (string, error) {
363-
path := match
364-
value, has := env.Config.GetString(path)
365-
if !has {
366-
return "", fmt.Errorf("parameter %s not found", path)
367-
}
368-
return value, nil
369-
})
370-
if err != nil {
371-
return nil, err
372-
}
373-
result[i] = evaluatedString
374-
}
375-
return result, nil
376-
}
377-
378-
dockerBuildArgs := []string{}
379-
for _, arg := range dockerOptions.BuildArgs {
380-
buildArgValue, err := arg.Envsubst(env.Getenv)
381-
if err != nil {
382-
return nil, fmt.Errorf("substituting environment variables in build args: %w", err)
383-
}
384-
385-
dockerBuildArgs = append(dockerBuildArgs, buildArgValue)
386-
}
387-
388-
// resolve parameters for build args and secrets
389-
resolvedBuildArgs, err := resolveParameters(dockerBuildArgs)
359+
resolvedBuildArgs, err := resolveDockerBuildArgs(dockerOptions.BuildArgs, env)
390360
if err != nil {
391361
return nil, err
392362
}
393363

394-
resolvedBuildEnv, err := resolveParameters(dockerOptions.BuildEnv)
364+
// resolve parameters for build env
365+
resolvedBuildEnv, err := resolveDockerParameters(dockerOptions.BuildEnv, env)
395366
if err != nil {
396367
return nil, err
397368
}
@@ -525,6 +496,40 @@ func (ch *ContainerHelper) Build(
525496
}, nil
526497
}
527498

499+
func resolveDockerBuildArgs(buildArgs []osutil.ExpandableString, env *environment.Environment) ([]string, error) {
500+
dockerBuildArgs := make([]string, 0, len(buildArgs))
501+
for _, arg := range buildArgs {
502+
buildArgValue, err := arg.Envsubst(env.Getenv)
503+
if err != nil {
504+
return nil, fmt.Errorf("substituting environment variables in build args: %w", err)
505+
}
506+
507+
dockerBuildArgs = append(dockerBuildArgs, buildArgValue)
508+
}
509+
510+
return resolveDockerParameters(dockerBuildArgs, env)
511+
}
512+
513+
func resolveDockerParameters(source []string, env *environment.Environment) ([]string, error) {
514+
result := make([]string, len(source))
515+
for i, arg := range source {
516+
evaluatedString, err := apphost.EvalString(arg, func(match string) (string, error) {
517+
path := match
518+
value, has := env.Config.GetString(path)
519+
if !has {
520+
return "", fmt.Errorf("parameter %s not found", path)
521+
}
522+
return value, nil
523+
})
524+
if err != nil {
525+
return nil, err
526+
}
527+
result[i] = evaluatedString
528+
}
529+
530+
return result, nil
531+
}
532+
528533
func (ch *ContainerHelper) Package(
529534
ctx context.Context,
530535
serviceConfig *ServiceConfig,
@@ -812,6 +817,24 @@ func (ch *ContainerHelper) runRemoteBuild(
812817
return "", fmt.Errorf("remote build only supports the linux/amd64 platform")
813818
}
814819

820+
resolvedBuildArgs, err := resolveDockerBuildArgs(dockerOptions.BuildArgs, env)
821+
if err != nil {
822+
return "", err
823+
}
824+
825+
resolvedBuildEnv, err := resolveDockerParameters(dockerOptions.BuildEnv, env)
826+
if err != nil {
827+
return "", err
828+
}
829+
830+
acrBuildArgs, err := dockerBuildArgsToAcrArguments(
831+
resolvedBuildArgs,
832+
dockerBuildArgEnvResolver(env, resolvedBuildEnv),
833+
)
834+
if err != nil {
835+
return "", err
836+
}
837+
815838
progress.SetProgress(NewServiceProgress("Packing remote build context"))
816839

817840
contextPath, dockerPath, err := containerregistry.PackRemoteBuildSource(ctx, dockerOptions.Context, dockerOptions.Path)
@@ -872,6 +895,9 @@ func (ch *ContainerHelper) runRemoteBuild(
872895
Architecture: to.Ptr(armcontainerregistry.ArchitectureAmd64),
873896
},
874897
}
898+
if len(acrBuildArgs) > 0 {
899+
buildRequest.Arguments = acrBuildArgs
900+
}
875901

876902
previewerWriter := ch.console.ShowPreviewer(ctx,
877903
&input.ShowPreviewerOptions{
@@ -889,6 +915,75 @@ func (ch *ContainerHelper) runRemoteBuild(
889915
return imageName, nil
890916
}
891917

918+
func dockerBuildArgsToAcrArguments(
919+
buildArgs []string,
920+
lookupEnv func(string) (string, bool),
921+
) ([]*armcontainerregistry.Argument, error) {
922+
if len(buildArgs) == 0 {
923+
return nil, nil
924+
}
925+
926+
acrArgs := make([]*armcontainerregistry.Argument, 0, len(buildArgs))
927+
for i, arg := range buildArgs {
928+
name, value, hasValue := strings.Cut(arg, "=")
929+
if name == "" {
930+
return nil, fmt.Errorf("docker build arg at index %d has an empty name", i)
931+
}
932+
933+
if !hasValue {
934+
var ok bool
935+
value, ok = lookupEnv(name)
936+
if !ok {
937+
return nil, fmt.Errorf(
938+
"resolving docker build arg %q for remote build: environment variable is not set; "+
939+
"use %s=<value> or set environment variable %s",
940+
name,
941+
name,
942+
name,
943+
)
944+
}
945+
}
946+
947+
acrArgs = append(acrArgs, &armcontainerregistry.Argument{
948+
Name: new(name),
949+
Value: new(value),
950+
IsSecret: new(false),
951+
})
952+
}
953+
954+
return acrArgs, nil
955+
}
956+
957+
func dockerBuildArgEnvResolver(
958+
env *environment.Environment,
959+
buildEnv []string,
960+
) func(string) (string, bool) {
961+
effectiveEnv := map[string]string{}
962+
for _, envVar := range os.Environ() {
963+
key, value, ok := strings.Cut(envVar, "=")
964+
if ok {
965+
effectiveEnv[key] = value
966+
}
967+
}
968+
for _, envVar := range env.Environ() {
969+
key, value, ok := strings.Cut(envVar, "=")
970+
if ok {
971+
effectiveEnv[key] = value
972+
}
973+
}
974+
for _, envVar := range buildEnv {
975+
key, value, ok := strings.Cut(envVar, "=")
976+
if ok {
977+
effectiveEnv[key] = value
978+
}
979+
}
980+
981+
return func(key string) (string, bool) {
982+
value, ok := effectiveEnv[key]
983+
return value, ok
984+
}
985+
}
986+
892987
// runDotnetPublish builds and publishes the container image using `dotnet publish`. It returns the full remote image name.
893988
func (ch *ContainerHelper) runDotnetPublish(
894989
ctx context.Context,

0 commit comments

Comments
 (0)