Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azd up doesn't pass --build-arg when calling docker build #3432

Closed
1 task done
IEvangelist opened this issue Feb 23, 2024 · 8 comments · Fixed by #3545
Closed
1 task done

azd up doesn't pass --build-arg when calling docker build #3432

IEvangelist opened this issue Feb 23, 2024 · 8 comments · Fixed by #3545
Assignees
Milestone

Comments

@IEvangelist
Copy link
Member

Output from azd version

Run azd version and copy and paste the output here:

azd version 1.7.0-beta.1-pr.3511727 (commit 3442439a53d9f59437ee3b13070ff9985c07cd91)

Describe the bug

Description of issue you're seeing...

I have a .NET Aspire application with an ASP.NET Core Minimal API, and three SPA framework frontends, Angular, React, and Vue. When I call azd up --debug, I can observe that the docker build command invocations aren't passing any --build-args when I believe that they should.

.NET Aspire generates the following manifest:

{
  "resources": {
    "weatherapi": {
      "type": "project.v0",
      "path": "../AspireJavaScript.MinimalApi/AspireJavaScript.MinimalApi.csproj",
      "env": {
        "OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES": "true",
        "OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EVENT_LOG_ATTRIBUTES": "true"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http"
        },
        "https": {
          "scheme": "https",
          "protocol": "tcp",
          "transport": "http"
        }
      }
    },
    "angular": {
      "type": "dockerfile.v0",
      "path": "../AspireJavaScript.Angular/Dockerfile",
      "context": "../AspireJavaScript.Angular",
      "env": {
        "NODE_ENV": "development",
        "services__weatherapi__0": "{weatherapi.bindings.http.url}",
        "services__weatherapi__1": "{weatherapi.bindings.https.url}",
        "PORT": "{angular.bindings.http.port}"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http",
          "containerPort": 4000
        }
      }
    },
    "react": {
      "type": "dockerfile.v0",
      "path": "../AspireJavaScript.React/Dockerfile",
      "context": "../AspireJavaScript.React",
      "env": {
        "NODE_ENV": "development",
        "services__weatherapi__0": "{weatherapi.bindings.http.url}",
        "services__weatherapi__1": "{weatherapi.bindings.https.url}",
        "PORT": "{react.bindings.http.port}"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http",
          "containerPort": 4001
        }
      }
    },
    "vue": {
      "type": "dockerfile.v0",
      "path": "../AspireJavaScript.Vue/Dockerfile",
      "context": "../AspireJavaScript.Vue",
      "env": {
        "NODE_ENV": "development",
        "services__weatherapi__0": "{weatherapi.bindings.http.url}",
        "services__weatherapi__1": "{weatherapi.bindings.https.url}",
        "PORT": "{vue.bindings.http.port}"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http",
          "containerPort": 4002
        }
      }
    }
  }
}

To Reproduce
Steps to reproduce the behavior...

Create a .NET Aspire app, add a non-.NET app to it that defines a Dockerfile such as a simple SPA app with an nginx image. Note that the manifest defines env variables, I believe that these should be splatted into the --build-arg. Here's an example output from the azd up--debug:

2024/02/23 14:17:17 dotnet.go:67: dotnet version: 8.0.200
2024/02/23 14:17:17 ensure.go:47: Skipping install check for '.NET CLI'. It was previously confirmed.
2024/02/23 14:17:18 command_runner.go:307: Run exec: 'dotnet msbuild D:\source\repos\aspire-samples\samples\AspireWithJavaScript\AspireJavaScript.AppHost\AspireJavaScript.AppHost.csproj --getProperty:IsAspireHost' , exit code: 0
Additional env:
   DOTNET_CLI_WORKLOAD_UPDATE_NOTIFY_DISABLE=<redacted>
-------------------------------------stdout-------------------------------------------
true
2024/02/23 14:17:20 command_runner.go:307: Run exec: 'dotnet run --project AspireJavaScript.AppHost.csproj --publisher manifest --output-path C:\Users\dapine\AppData\Local\Temp\azd-provision4286696080\apphost-manifest.json' , exit code: 0
-------------------------------------stdout-------------------------------------------
Building...
info: Aspire.Hosting.Publishing.ManifestPublisher[0]
      Published manifest to: C:\Users\dapine\AppData\Local\Temp\azd-provision4286696080\apphost-manifest.json
2024/02/23 14:17:20 framework_service_docker.go:208: building image for service angular, cwd: D:\source\repos\aspire-samples\samples\AspireWithJavaScript\AspireJavaScript.Angular, path: D:/source/repos/aspire-samples/samples/AspireWithJavaScript/AspireJavaScript.Angular/Dockerfile, context: D:/source/repos/aspire-samples/samples/AspireWithJavaScript/AspireJavaScript.Angular, buildArgs: [])
2024/02/23 14:17:23 command_runner.go:307: Run exec: 'docker build -f D:/source/repos/aspire-samples/samples/AspireWithJavaScript/AspireJavaScript.Angular/Dockerfile --platform linux/amd64 -t aspirewithjavascript-angular D:/source/repos/aspire-samples/samples/AspireWithJavaScript/AspireJavaScript.Angular --iidfile C:\Users\dapine\AppData\Local\Temp\azd-docker-build4282026611\imgId' , exit code: 0

From this output you can see the framework_service_docker.go claims that buildArgs is an empty array [], thus there aren't any --build-arg switches.

Expected behavior
A clear and concise description of what you expected to happen.

I expected that for each env defined in the .NET Aspire manifest to have a corresponding --build-arg passed when the docker build command is called.

docker build -f ./AspireJavaScript.Angular/Dockerfile --platform linux/amd64 -t aspirewithjavascript-angular ./AspireJavaScript.Angular --iidfile .\AppData\Local\Temp\azd-docker-build4282026611\imgId --build-arg NODE_ENV=development --build-arg services__weatherapi__0=http://some-resolved-url:port --build-arg services__weatherapi__1=https://some-resolved-url:port --build-arg PORT=resolve-port'

Additional context
Add any other context about the problem here.

See Set build-time variables (--build-arg)
Discovered as part of dotnet/aspire-samples#125

@ellismg
Copy link
Member

ellismg commented Feb 23, 2024

Create a .NET Aspire app, add a non-.NET app to it that defines a Dockerfile such as a simple SPA app with an nginx image. Note that the manifest defines env variables, I believe that these should be splatted into the --build-arg. Here's an example output from the azd up--debug:

Hrm. This is interesting. I'm not certain that splatting these as args is actually the right thing to do? To me the env block is about the environment that the container itself runs in, not something about how the container itself is built. Do you know, @IEvangelist, when running via VS using F5 and DCP, does it actually do this splatting?

I'm slightly concerned about cases where we may not be able to resolve values for the env block when we go to build the container, but could later during publishing. I'm just wondering if things like {weatherapi.bindings.http.url} are in the limit going to be hard to resolve "at build time". Perhaps in practice this won't be an issue because we can delay the docker build late enough (i.e. right before we publish) that it won't be an issue, but it does give me pause.

I do expect you want to do this so you can burn in things like the URL of the weather service into your SPA as you build it (either during the compiler or by writing some config.json file or something that you serve and your SPA pulls to get endpoint information).

Part of me wonders if we should just extend the schema for the dockerfile.v0 resource to include something like buildArgs which is like "env" (the value is an object of build arg names to values, which may include {binding.expression.strings} instead of conflating the two concerns. This would also likely be closer to how we model Dockerfiles outside of Aspire today, where we have an explicit buildArgs member on azure.yaml that allows you to control this. Thoughts on this, @davidfowl or @mitchdenny.

Whatever we do here - I do want to be consistent with however DCP handles this situation...

@IEvangelist
Copy link
Member Author

IEvangelist commented Feb 23, 2024

I'm less concerned about where the --build-arg values come from, I just know that they're missing. 🤓

It is commonplace at build time for Node.js apps to require env vars. If the docker build runs, and the Dockerfile has anything like this at all:

FROM node:20.7 as build

WORKDIR /app

COPY package.json package.json
COPY package-lock.json package-lock.json

RUN npm i

COPY . .

RUN npm run build-prod

FROM nginx:alpine

COPY --from=build /app/default.conf.template /etc/nginx/templates/default.conf.template
COPY --from=build /app/dist/weather/browser /usr/share/nginx/html

EXPOSE 4000 80

CMD ["nginx", "-g", "daemon off;"]

Where it's doing RUN npm i and RUN npm run build, env vars will need to be available. I would have expected that whatever is consuming the .NET Aspire manifest.json, that it would parse that file to understand dependencies, and populate env values based on the order of the dependencies. This might pose an interesting issue with cycles, and having situations where you need to build containers multiple times - unless there's a way to get URLs ahead of time?

@davidfowl
Copy link
Member

Part of me wonders if we should just extend the schema for the dockerfile.v0 resource to include something like buildArgs which is like "env" (the value is an object of build arg names to values, which may include {binding.expression.strings} instead of conflating the two concerns. This would also likely be closer to how we model Dockerfiles outside of Aspire today, where we have an explicit buildArgs member on azure.yaml that allows you to control this. Thoughts on this, @davidfowl or @mitchdenny.

This is the right thing to do. Baking in ENV as build args isn't right and it should be explicitly defined.

@IEvangelist
Copy link
Member Author

I'm good with that, makes sense. On the Aspire side of things, we'll need to expose this functionality then, right?

@IEvangelist
Copy link
Member Author

IEvangelist commented Mar 14, 2024

Hi @ellismg and @rajeshkamal5050, we have a PR in place that will update the resulting manifest for the dockerfile.v0 resource type to add an optional buildArgs node. When available, this node is an object that has key/value pairs (null values are still build args, they just omit the =VALUE semenatics, as docker will pull the value from the current environment variable). Consider the following manifest.json:

{
  "type": "dockerfile.v0",
  "path": "NodeFrontend/Dockerfile",
  "context": "NodeFrontend",
  "buildArgs": {
    "SOME_BUILD_ARG": "Test",
    "ANOTHER_BUILD_ARG": null
  },
  "env": {
    "NODE_ENV": "production"
  }
}

When docker build is called, it would be expected that the following --build-arg values are provided:

docker build --build-arg SOME_BUILD_ARG=Test --build-arg ANOTHER_BUILD_ARG .

Additionally, it's possible for the values to be represented as value expressions, just like we do with the env node now. For example, this is also a valid manifest:

{
  "resources": {
    "weatherapi": {
      "type": "project.v0",
      "path": "../AspireJavaScript.MinimalApi/AspireJavaScript.MinimalApi.csproj",
      "env": {
        "OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES": "true",
        "OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EVENT_LOG_ATTRIBUTES": "true"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http"
        },
        "https": {
          "scheme": "https",
          "protocol": "tcp",
          "transport": "http"
        }
      }
    },
    "angular": {
      "type": "dockerfile.v0",
      "path": "../AspireJavaScript.Angular/Dockerfile",
      "context": "../AspireJavaScript.Angular",
      "buildArgs": {
        "services__weatherapi__0": "{weatherapi.bindings.http.url}",
        "services__weatherapi__1": "{weatherapi.bindings.https.url}"
      },
      "env": {
        "NODE_ENV": "development",
        "services__weatherapi__0": "{weatherapi.bindings.http.url}",
        "services__weatherapi__1": "{weatherapi.bindings.https.url}",
        "PORT": "{angular.bindings.http.port}"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http",
          "containerPort": 4000
        }
      }
    },
    "react": {
      "type": "dockerfile.v0",
      "path": "../AspireJavaScript.React/Dockerfile",
      "context": "../AspireJavaScript.React",
      "env": {
        "NODE_ENV": "development",
        "services__weatherapi__0": "{weatherapi.bindings.http.url}",
        "services__weatherapi__1": "{weatherapi.bindings.https.url}",
        "PORT": "{react.bindings.http.port}"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http",
          "containerPort": 4001
        }
      }
    },
    "vue": {
      "type": "dockerfile.v0",
      "path": "../AspireJavaScript.Vue/Dockerfile",
      "context": "../AspireJavaScript.Vue",
      "env": {
        "NODE_ENV": "development",
        "services__weatherapi__0": "{weatherapi.bindings.http.url}",
        "services__weatherapi__1": "{weatherapi.bindings.https.url}",
        "PORT": "{vue.bindings.http.port}"
      },
      "bindings": {
        "http": {
          "scheme": "http",
          "protocol": "tcp",
          "transport": "http",
          "containerPort": 4002
        }
      }
    }
  }
}

Note

Notice that the angular resource has buildArgs.

Does that make sense? See dotnet/aspire#2851

@ellismg
Copy link
Member

ellismg commented Mar 14, 2024

Thanks, @IEvangelist - This makes sense to me - we'll add support for this new flag so we pass buildArgs when they live in the manifest.

@davidfowl
Copy link
Member

This just got mereged!

@vhvb1989
Copy link
Member

@IEvangelist , with #3545 azd will use the build-args, however, azd is not resolving references like it does with the environment block.
There is some concerns around secrets values, which would be licked to the terminal history. Azd could return an error when a secret is referenced as build-arg, but I would like to move the logic to netAspire, if possible, to make the AppHost fail (even on local dev) referencing secret values.

So, for now, the first step would be to honor build-args verbatim from the manifest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants