Azure / azure-dev

A developer CLI that reduces the time it takes for you to get started on Azure. The Azure Developer CLI (azd) provides a set of developer-friendly commands that map to key stages in your workflow - code, build, deploy, monitor, repeat.
https://aka.ms/azd
MIT License
412 stars 201 forks source link

When pushing js container apps for Aspire, azd is setting NODE_ENV to development #3410

Closed timheuer closed 8 months ago

timheuer commented 8 months ago

Output from azd version Run azd version and copy and paste the output here: azd version 1.7.0-beta.1-pr.3511435 (commit a1be2bad01f96eaf89b22a00248bb98916de0b41)

Describe the bug While validating the change in the above version, I was using this repo: https://github.com/timheuer/js-aspire to deploy.

To Reproduce

  1. azd init
  2. select ALL services to be exposed
  3. azd up

Expected behavior The manifest is indicating NODE_ENV=production but the resulting ACA instance shows development: image

Confirming on an azd infra synth that the NODE_ENV is marked development there.

davidfowl commented 8 months ago

Confirming on an azd infra synth that the NODE_ENV is marked development there.

It's because we don't yet flow the DOTNET_ENVIRONMENT into the apphost

https://github.com/Azure/azure-dev/issues/3233

timheuer commented 8 months ago

But what's weird is that a valid NODE_ENV environment setting is passed in to the manifest, somehow just ignored...

"angular": {
  "type": "dockerfile.v0",
  "path": "../AspireJavaScript.Angular/Dockerfile",
  "context": "../AspireJavaScript.Angular",
  "env": {
    "NODE_ENV": "production",
    "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": 3000
    }
  }
},
davidfowl commented 8 months ago

But what's weird is that a valid NODE_ENV environment setting is passed in to the manifest, somehow just ignored...

Are you looking at the manifest that you're manually generating or looking at the one that azd generates when you azd commands?

timheuer commented 8 months ago

Looking at the one that is generated...is that not what AZD uses? and if not, just a current 'false' sense of truth? The infra synth definitely emits NODE_ENV: development, which causes me confusion why the value even exists in the manifest to begin with.

davidfowl commented 8 months ago

Looking at the one that is generated...is that not what AZD uses? and if not, just a current 'false' sense of truth? The infra synth definitely emits NODE_ENV: development, which causes me confusion why the value even exists in the manifest to begin with.

This is what you should believe 😄, not your manifest (which probably doesn't use the launch profile in the same way AZD is using it right now).

ellismg commented 8 months ago

This should be fixed with #3425. @timheuer, I pulled your repository down, ran azd init to create my own environment and then did the following:

azd infra synth

Since DOTNET_ENVIRONMENT is not defined the value defaults to Production (per https://learn.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-8.0#environments). The generated manifests correctly specify a NODE_ENV of production:

% yq e '.properties.template.containers[0].env' AspireJavaScript.Angular/manifests/containerApp.tmpl.yaml
- name: AZURE_CLIENT_ID
  value: {? {.Env.MANAGED_IDENTITY_CLIENT_ID: ''}: ''}
- name: NODE_ENV
  value: production
- name: PORT
  value: "3000"
- name: services__weatherapi__0
  value: http://weatherapi.internal.{{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_DEFAULT_DOMAIN }}
- name: services__weatherapi__1
  value: https://weatherapi.internal.{{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_DEFAULT_DOMAIN }}

I can then set DOTNET_ENVIRONMENT to Development (either explicitly via azd env set DOTNET_ENVIRONMENT Development or in my shell via export or whatever), and re-generate the infrastructure. Now, since DOTNET_ENVIRONMENT is set to Development, this code:

https://github.com/dotnet/aspire/blob/7590f58289d99644c6da4da414f506b16cf26ce9/src/Aspire.Hosting/Node/NodeExtensions.cs#L58-L60

Causes us to write development as the NODE_ENV:

% azd env set DOTNET_ENVIRONMENT Development
% azd infra synth --force

WARNING: Feature 'infraSynth' is in alpha stage.
To learn more about alpha features and their support, visit https://aka.ms/azd-feature-stages.

% yq e '.properties.template.containers[0].env' AspireJavaScript.Angular/manifests/containerApp.tmpl.yaml
- name: AZURE_CLIENT_ID
  value: {? {.Env.MANAGED_IDENTITY_CLIENT_ID: ''}: ''}
- name: NODE_ENV
  value: development
- name: PORT
  value: "3000"
- name: services__weatherapi__0
  value: http://weatherapi.internal.{{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_DEFAULT_DOMAIN }}
- name: services__weatherapi__1
  value: https://weatherapi.internal.{{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_DEFAULT_DOMAIN }}

And, of course, you can explicitly set DOTNET_ENVIRONMENT to Production and in that case you NODE_ENV will then change back to production.

% azd env set DOTNET_ENVIRONMENT Production 
% azd infra synth --force

WARNING: Feature 'infraSynth' is in alpha stage.
To learn more about alpha features and their support, visit https://aka.ms/azd-feature-stages.

% yq e '.properties.template.containers[0].env' AspireJavaScript.Angular/manifests/containerApp.tmpl.yaml
- name: AZURE_CLIENT_ID
  value: {? {.Env.MANAGED_IDENTITY_CLIENT_ID: ''}: ''}
- name: NODE_ENV
  value: production
- name: PORT
  value: "3000"
- name: services__weatherapi__0
  value: http://weatherapi.internal.{{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_DEFAULT_DOMAIN }}
- name: services__weatherapi__1
  value: https://weatherapi.internal.{{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_DEFAULT_DOMAIN }}
ellismg commented 8 months ago

Fixed by #3425