Skip to content

Commit

Permalink
Support args on project.v0 and dockerfile.v0 resources
Browse files Browse the repository at this point in the history
Fixes #3678
  • Loading branch information
ellismg committed Apr 16, 2024
1 parent 6835b4d commit 3be0a9a
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 37 deletions.
80 changes: 58 additions & 22 deletions cli/azd/pkg/apphost/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ func Dockerfiles(manifest *Manifest) map[string]genDockerfile {
Env: comp.Env,
Bindings: comp.Bindings,
BuildArgs: comp.BuildArgs,
Args: comp.Args,
}
}
}
Expand Down Expand Up @@ -447,7 +448,7 @@ func (b *infraGenerator) LoadManifest(m *Manifest) error {
case "azure.appinsights.v0":
b.addAppInsights(name)
case "project.v0":
b.addProject(name, *comp.Path, comp.Env, comp.Bindings)
b.addProject(name, *comp.Path, comp.Env, comp.Bindings, comp.Args)
case "container.v0":
b.addContainer(name, *comp.Image, comp.Env, comp.Bindings, comp.Inputs, comp.Volumes)
case "dapr.v0":
Expand All @@ -461,7 +462,7 @@ func (b *infraGenerator) LoadManifest(m *Manifest) error {
return err
}
case "dockerfile.v0":
b.addDockerfile(name, *comp.Path, *comp.Context, comp.Env, comp.Bindings, comp.BuildArgs)
b.addDockerfile(name, *comp.Path, *comp.Context, comp.Env, comp.Bindings, comp.BuildArgs, comp.Args)
case "redis.v0":
b.addContainerAppService(name, RedisContainerAppService)
case "azure.keyvault.v0":
Expand Down Expand Up @@ -751,7 +752,7 @@ func (b *infraGenerator) addSqlDatabase(sqlAccount, dbName string) {
}

func (b *infraGenerator) addProject(
name string, path string, env map[string]string, bindings custommaps.WithOrder[Binding],
name string, path string, env map[string]string, bindings custommaps.WithOrder[Binding], args *[]string,
) {
b.requireCluster()
b.requireContainerRegistry()
Expand All @@ -760,6 +761,7 @@ func (b *infraGenerator) addProject(
Path: path,
Env: env,
Bindings: bindings,
Args: args,
}
}

Expand Down Expand Up @@ -928,6 +930,7 @@ func (b *infraGenerator) addDaprStateStoreComponent(name string) {
func (b *infraGenerator) addDockerfile(
name string, path string, context string, env map[string]string,
bindings custommaps.WithOrder[Binding], buildArgs map[string]string,
args *[]string,
) {
b.requireCluster()
b.requireContainerRegistry()
Expand All @@ -938,6 +941,7 @@ func (b *infraGenerator) addDockerfile(
Env: env,
Bindings: bindings,
BuildArgs: buildArgs,
Args: args,
}
}

Expand Down Expand Up @@ -1021,6 +1025,10 @@ func (b *infraGenerator) Compile() error {
return fmt.Errorf("configuring environment for resource %s: %w", resourceName, err)
}

if err := b.buildArgsBlock(docker.Args, &projectTemplateCtx); err != nil {
return err
}

b.containerAppTemplateContexts[resourceName] = projectTemplateCtx
}

Expand Down Expand Up @@ -1064,6 +1072,10 @@ func (b *infraGenerator) Compile() error {
return err
}

if err := b.buildArgsBlock(project.Args, &projectTemplateCtx); err != nil {
return err
}

b.containerAppTemplateContexts[resourceName] = projectTemplateCtx
}

Expand Down Expand Up @@ -1346,6 +1358,48 @@ func (b infraGenerator) evalBindingRef(v string, emitType inputEmitType) (string
}
}

// asYamlString converts a string to the YAML representation of the string, ensuring that it is quoted and escaped as needed.
func asYamlString(s string) (string, error) {
// We want to ensure that we render these values in the YAML as strings. If `res` was the string "true"
// (without the quotes), we would naturally create a value directive in yaml that looks like this:
//
// - name: OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES
// value: true
//
// And YAML rules would treat the above as the value being a boolean instead of a string, which the container
// app service expects.
//
// YAML marshalling the string value will give us something like `"true"` (with the quotes, and any escaping
// that needs to be done), which is what we want here.
// Do not use JSON marshall as it would escape the quotes within the string, breaking the meaning of the value.
// yaml marshall will use 'some text "quoted" more text' as a valid yaml string.
yamlString, err := yaml.Marshal(s)
if err != nil {
return "", fmt.Errorf("marshalling env value: %w", err)
}

// remove the trailing newline. yaml marshall will add a newline at the end of the string, as the new line is
// expected at the end of the yaml document. But we are getting a single value with valid yaml here, so we don't
// need the newline
return string(yamlString[0 : len(yamlString)-1]), nil
}

func (b *infraGenerator) buildArgsBlock(args *[]string, manifestCtx *genContainerAppManifestTemplateContext) error {
if args == nil {
return nil
}

for _, arg := range *args {
yamlString, err := asYamlString(arg)
if err != nil {
return fmt.Errorf("marshalling arg value: %w", err)
}
manifestCtx.Args = append(manifestCtx.Args, yamlString)
}

return nil
}

// buildEnvBlock creates the environment map in the template context. It does this by copying the values from the given map,
// evaluating any binding expressions that are present. It writes the result of the evaluation after calling json.Marshal
// so the values may be emitted into YAML as is without worrying about escaping.
Expand All @@ -1356,29 +1410,11 @@ func (b *infraGenerator) buildEnvBlock(env map[string]string, manifestCtx *genCo
return fmt.Errorf("evaluating value for %s: %w", k, err)
}

// We want to ensure that we render these values in the YAML as strings. If `res` was the string "true"
// (without the quotes), we would naturally create a value directive in yaml that looks like this:
//
// - name: OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES
// value: true
//
// And YAML rules would treat the above as the value being a boolean instead of a string, which the container
// app service expects.
//
// YAML marshalling the string value will give us something like `"true"` (with the quotes, and any escaping
// that needs to be done), which is what we want here.
// Do not use JSON marshall as it would escape the quotes within the string, breaking the meaning of the value.
// yaml marshall will use 'some text "quoted" more text' as a valid yaml string.
yamlString, err := yaml.Marshal(res)
resolvedValue, err := asYamlString(res)
if err != nil {
return fmt.Errorf("marshalling env value: %w", err)
}

// remove the trailing newline. yaml marshall will add a newline at the end of the string, as the new line is
// expected at the end of the yaml document. But we are getting a single value with valid yaml here, so we don't
// need the newline
resolvedValue := string(yamlString[0 : len(yamlString)-1])

// connectionString detection, either of:
// a) explicit connection string key for env, like "ConnectionStrings__resource": "XXXXX"
// b) a connection string field references in the value, like "FOO": "{resource.connectionString}"
Expand Down
22 changes: 22 additions & 0 deletions cli/azd/pkg/apphost/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ var aspireDockerManifest []byte
//go:embed testdata/aspire-storage.json
var aspireStorageManifest []byte

//go:embed testdata/aspire-args.json
var aspireArgsManifest []byte

//go:embed testdata/aspire-bicep.json
var aspireBicepManifest []byte

Expand Down Expand Up @@ -212,6 +215,25 @@ func TestAspireDockerGeneration(t *testing.T) {
require.NoError(t, err)
}

func TestAspireArgsGeneration(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Skipping due to EOL issues on Windows with the baselines")
}

ctx := context.Background()
mockCtx := mocks.NewMockContext(ctx)
mockPublishManifest(mockCtx, aspireArgsManifest, nil)
mockCli := dotnet.NewDotNetCli(mockCtx.CommandRunner)

m, err := ManifestFromAppHost(ctx, filepath.Join("testdata", "AspireArgs.AppHost.csproj"), mockCli, "")
require.NoError(t, err)

manifest, err := ContainerAppManifestTemplateForProject(m, "apiservice")
require.NoError(t, err)

snapshot.SnapshotT(t, manifest)
}

func TestAspireContainerGeneration(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Skipping due to EOL issues on Windows with the baselines")
Expand Down
3 changes: 3 additions & 0 deletions cli/azd/pkg/apphost/generate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,13 @@ type genDockerfile struct {
Env map[string]string
Bindings custommaps.WithOrder[Binding]
BuildArgs map[string]string
Args *[]string
}

type genProject struct {
Path string
Env map[string]string
Args *[]string
Bindings custommaps.WithOrder[Binding]
}

Expand Down Expand Up @@ -152,6 +154,7 @@ type genContainerAppManifestTemplateContext struct {
Env map[string]string
Secrets map[string]string
KeyVaultSecrets map[string]string
Args []string
Dapr *genContainerAppManifestTemplateContextDapr
}

Expand Down
3 changes: 3 additions & 0 deletions cli/azd/pkg/apphost/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type Resource struct {
// BuildArgs is present on a dockerfile.v0 resource and is the --build-arg for building the docker image.
BuildArgs map[string]string `json:"buildArgs,omitempty"`

// Args is optionally present on project.v0 and dockerfile.v0 resources and are the arguments to pass to the container.
Args *[]string `json:"args,omitempty"`

// Parent is present on a resource which is a child of another. It is the name of the parent resource. For example, a
// postgres.database.v0 is a child of a postgres.server.v0, and so it would have a parent of which is the name of
// the server resource.
Expand Down
48 changes: 48 additions & 0 deletions cli/azd/pkg/apphost/testdata/TestAspireArgsGeneration.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
api-version: 2024-02-02-preview
location: {{ .Env.AZURE_LOCATION }}
identity:
type: UserAssigned
userAssignedIdentities:
? "{{ .Env.AZURE_CONTAINER_REGISTRY_MANAGED_IDENTITY_ID }}"
: {}
properties:
environmentId: {{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_ID }}
configuration:
activeRevisionsMode: single
runtime:
dotnet:
autoConfigureDataProtection: true
ingress:
external: false
targetPort: {{ targetPortOrDefault 8080 }}
transport: http
allowInsecure: true
registries:
- server: {{ .Env.AZURE_CONTAINER_REGISTRY_ENDPOINT }}
identity: {{ .Env.AZURE_CONTAINER_REGISTRY_MANAGED_IDENTITY_ID }}
template:
containers:
- image: {{ .Image }}
name: apiservice
args:
- --supports-args
- "true"
- --port
- "12345"
env:
- name: AZURE_CLIENT_ID
value: {{ .Env.MANAGED_IDENTITY_CLIENT_ID }}
- name: ASPNETCORE_FORWARDEDHEADERS_ENABLED
value: "true"
- name: OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EVENT_LOG_ATTRIBUTES
value: "true"
- name: OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES
value: "true"
- name: OTEL_DOTNET_EXPERIMENTAL_OTLP_RETRY
value: in_memory
scale:
minReplicas: 1
tags:
azd-service-name: apiservice
aspire-resource-name: apiservice

32 changes: 32 additions & 0 deletions cli/azd/pkg/apphost/testdata/aspire-args.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"resources": {
"apiservice": {
"type": "project.v0",
"path": "../AspireArgs.ApiService/AspireArgs.ApiService.csproj",
"args": [
"--supports-args",
"true",
"--port",
"12345"
],
"env": {
"OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES": "true",
"OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EVENT_LOG_ATTRIBUTES": "true",
"OTEL_DOTNET_EXPERIMENTAL_OTLP_RETRY": "in_memory",
"ASPNETCORE_FORWARDEDHEADERS_ENABLED": "true"
},
"bindings": {
"http": {
"scheme": "http",
"protocol": "tcp",
"transport": "http"
},
"https": {
"scheme": "https",
"protocol": "tcp",
"transport": "http"
}
}
}
}
}
36 changes: 21 additions & 15 deletions cli/azd/resources/apphost/templates/containerApp.tmpl.yamlt
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ properties:
{{- if .Ingress.AdditionalPortMappings }}
additionalPortMappings:
{{- range $additionalPort := .Ingress.AdditionalPortMappings }}
- targetPort: {{ $additionalPort.TargetPort }}
external: {{ $additionalPort.External }}
- targetPort: {{ $additionalPort.TargetPort }}
external: {{ $additionalPort.External }}
{{- end}}
{{- end}}
external: {{ .Ingress.External }}
Expand All @@ -51,8 +51,8 @@ properties:
allowInsecure: {{ .Ingress.AllowInsecure }}
{{- end }}
registries:
- server: {{ "{{ .Env.AZURE_CONTAINER_REGISTRY_ENDPOINT }}" }}
identity: {{ "{{ .Env.AZURE_CONTAINER_REGISTRY_MANAGED_IDENTITY_ID }}" }}
- server: {{ "{{ .Env.AZURE_CONTAINER_REGISTRY_ENDPOINT }}" }}
identity: {{ "{{ .Env.AZURE_CONTAINER_REGISTRY_MANAGED_IDENTITY_ID }}" }}
{{- if or (gt (len .Secrets) 0) (gt (len .KeyVaultSecrets) 0) }}
secrets:
{{- end}}
Expand All @@ -67,22 +67,28 @@ properties:
{{- end}}
template:
containers:
- image: {{ "{{ .Image }}" }}
name: {{ .Name }}
env:
- name: AZURE_CLIENT_ID
value: {{ "{{ .Env.MANAGED_IDENTITY_CLIENT_ID }}" }}
- image: {{ "{{ .Image }}" }}
name: {{ .Name }}
{{- if .Args }}
args:
{{- range $arg := .Args}}
- {{$arg}}
{{- end}}
{{- end}}
env:
- name: AZURE_CLIENT_ID
value: {{ "{{ .Env.MANAGED_IDENTITY_CLIENT_ID }}" }}
{{- range $name, $value := .Env}}
- name: {{$name}}
value: {{$value}}
- name: {{$name}}
value: {{$value}}
{{- end}}
{{- range $name, $value := .Secrets}}
- name: {{$name}}
secretRef: {{containerAppSecretName $name}}
- name: {{$name}}
secretRef: {{containerAppSecretName $name}}
{{- end}}
{{- range $name, $value := .KeyVaultSecrets}}
- name: {{$name}}
secretRef: {{containerAppSecretName $name}}
- name: {{$name}}
secretRef: {{containerAppSecretName $name}}
{{- end}}
scale:
minReplicas: 1
Expand Down

0 comments on commit 3be0a9a

Please sign in to comment.