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

fix persist with default value params in aspire #4524

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="StreamJsonRpc" Version="2.17.8" />
<ItemGroup>
<PackageReference Include="StreamJsonRpc" Version="2.17.8" />
</ItemGroup>

</Project>
54 changes: 42 additions & 12 deletions cli/azd/pkg/apphost/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,15 +321,19 @@ func BicepTemplate(name string, manifest *Manifest, options AppHostOptions) (*me

// order to be deterministic when writing bicep
genParametersKeys := slices.Sorted(maps.Keys(generator.bicepContext.InputParameters))
metadataType := azure.AzdMetadataTypeGenerate
for _, key := range genParametersKeys {
parameter := generator.bicepContext.InputParameters[key]
parameterMetadata := ""
var parameterDefaultValue *string
if parameter.Default != nil {
// main.bicep template handles *string for default.Value. If the value is nil, it will be ignored.
// if not nil, like empty string or any other string, it is used as `= '<value>'`
parameterDefaultValue = parameter.Default.Value
if parameter.Default.Generate != nil {
if parameter.Default.Value != nil {
parameterDefaultValue = parameter.Default.Value
metadataType = azure.AzdMetadataTypeNeedForDeploy
parameterMetadata = "{}"
} else if parameter.Default.Generate != nil { // Note: .Value and .Generate are mutually exclusive
pMetadata, err := inputMetadata(*parameter.Default.Generate)
if err != nil {
return nil, fmt.Errorf("generating input metadata for %s: %w", key, err)
Expand All @@ -343,7 +347,7 @@ func BicepTemplate(name string, manifest *Manifest, options AppHostOptions) (*me
parameters = append(parameters, autoGenInput{
genInput: input,
MetadataConfig: parameterMetadata,
MetadataType: azure.AzdMetadataTypeGenerate})
MetadataType: metadataType})
if slices.Contains(generator.bicepContext.mappedParameters, strings.ReplaceAll(key, "-", "_")) {
mapToResourceParams = append(mapToResourceParams, input)
}
Expand Down Expand Up @@ -565,19 +569,45 @@ func (b *infraGenerator) extractOutputs(resource *Resource) error {
}
}
}
for _, value := range resource.Env {
outputs, err := evaluateForOutputs(value)
if err != nil {
return err

feedFrom := func(from map[string]string, to *genBicepTemplateContext) error {
for _, value := range from {
outputs, err := evaluateForOutputs(value)
if err != nil {
return err
}
for key, output := range outputs {
if strings.Contains(output.Value, ".outputs.") {
to.OutputParameters[key] = output
} else {
to.OutputSecretParameters[key] = output
}
}

}
for key, output := range outputs {
if strings.Contains(output.Value, ".outputs.") {
b.bicepContext.OutputParameters[key] = output
} else {
b.bicepContext.OutputSecretParameters[key] = output
return nil
}
err := feedFrom(resource.Env, &b.bicepContext)
if err != nil {
return err
}

if resource.Deployment != nil {
// Taking only the string values from the deployment parameters. There could be other types like int or object there.
// Only string type could be referencing outputs.
deploymentParams := map[string]string{}
for k, v := range resource.Deployment.Params {
stringValue, castOk := v.(string)
if !castOk {
continue
}
deploymentParams[k] = stringValue
}

err = feedFrom(deploymentParams, &b.bicepContext)
if err != nil {
return err
}
}
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,17 @@ param noVolume_pas_sw_ord string
})
@secure()
param noVolume_password string
@metadata({azd: {
type: 'needForDeploy'
config: {}
}
})
param param_with_empty_value string = ''
@metadata({azd: {
type: 'needForDeploy'
config: {}
}
})
param param_with_value string = 'default value for param'

var tags = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ using './cache.module.bicep'
param cache_volumes_0_storage = '{{ .Env.SERVICE_CACHE_VOLUME_AZURECONTAINERAPPSAPPHOST8F235654EDCACHEDATA_NAME }}'
param complex = 'tcp:cache/foo'
param host = 'cache'
param inputFromBicepModuleOutput = '{{ .Env.STORAGE_THISISANOUTPUT }}'
param outputs_azure_container_apps_environment_id = '{{ .Env.AZURE_CONTAINER_APPS_ENVIRONMENT_ID }}'
param outputs_azure_container_registry_managed_identity_id = '{{ .Env.AZURE_CONTAINER_REGISTRY_MANAGED_IDENTITY_ID }}'
param outputs_managed_identity_client_id = '{{ .Env.MANAGED_IDENTITY_CLIENT_ID }}'
param protocol = 'tcp'
param secretInputFromBicepModuleOutput = '{{ .Env.SERVICE_BINDING_KV1CA9A66_ENDPOINT }}secrets/thisIsSecretOutput'

Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,6 @@ output SERVICE_CACHE_VOLUME_AZURECONTAINERAPPSAPPHOST8F235654EDCACHEDATA_NAME st
output SERVICE_BINDING_KV294FC75C_ENDPOINT string = resources.outputs.SERVICE_BINDING_KV294FC75C_ENDPOINT
output SERVICE_BINDING_KV294FC75C_NAME string = resources.outputs.SERVICE_BINDING_KV294FC75C_NAME
output STORAGE_BLOBENDPOINT string = storage.outputs.blobEndpoint
output STORAGE_THISISANOUTPUT string = storage.outputs.thisIsAnOutput
output AZURE_VOLUMES_STORAGE_ACCOUNT string = resources.outputs.AZURE_VOLUMES_STORAGE_ACCOUNT

4 changes: 3 additions & 1 deletion cli/azd/pkg/apphost/testdata/aspire-projectv1.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
"outputs_azure_container_apps_environment_id": "{.outputs.AZURE_CONTAINER_APPS_ENVIRONMENT_ID}",
"host": "{cache.bindings.tcp.host}",
"protocol": "{cache.bindings.tcp.protocol}",
"complex": "{cache.bindings.tcp.protocol}:{cache.bindings.tcp.host}/foo"
"complex": "{cache.bindings.tcp.protocol}:{cache.bindings.tcp.host}/foo",
"inputFromBicepModuleOutput": "{storage.outputs.thisIsAnOutput}",
"secretInputFromBicepModuleOutput": "{storage.secretOutputs.thisIsSecretOutput}"
}
},
"args": [
Expand Down
1 change: 1 addition & 0 deletions cli/azd/pkg/azure/arm_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ type AzdMetadataType string
const AzdMetadataTypeLocation AzdMetadataType = "location"
const AzdMetadataTypeGenerate AzdMetadataType = "generate"
const AzdMetadataTypeGenerateOrManual AzdMetadataType = "generateOrManual"
const AzdMetadataTypeNeedForDeploy AzdMetadataType = "needForDeploy"

type AzdMetadata struct {
Type *AzdMetadataType `json:"type,omitempty"`
Expand Down
18 changes: 17 additions & 1 deletion cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -1826,15 +1826,32 @@ func (p *BicepProvider) ensureParameters(
for _, key := range sortedKeys {
param := template.Parameters[key]
parameterType := p.mapBicepTypeToInterfaceType(param.Type)
azdMetadata, hasMetadata := param.AzdMetadata()

// If a value is explicitly configured via a parameters file, use it.
// unless the parameter value inference is nil/empty
if v, has := parameters[key]; has {
paramValue := armParameterFileValue(parameterType, v.Value, param.DefaultValue)

if paramValue != nil {
needForDeployParameter := hasMetadata &&
azdMetadata.Type != nil &&
*azdMetadata.Type == azure.AzdMetadataTypeNeedForDeploy
if needForDeployParameter && paramValue == "" && param.DefaultValue != nil {
// Parameters with needForDeploy metadata don't support overriding with empty values when a default
// value is present. If the value is empty, we'll use the default value instead.
defValue, castOk := param.DefaultValue.(string)
if castOk {
paramValue = defValue
}
}
configuredParameters[key] = azure.ArmParameterValue{
Value: paramValue,
}
if needForDeployParameter {
mustSetParamAsConfig(key, paramValue, p.env.Config, param.Secure())
configModified = true
}
continue
}
}
Expand Down Expand Up @@ -1863,7 +1880,6 @@ func (p *BicepProvider) ensureParameters(

// If the parameter is tagged with {type: "generate"}, skip prompting.
// We generate it once, then save to config for next attempts.`.
azdMetadata, hasMetadata := param.AzdMetadata()
if hasMetadata && parameterType == provisioning.ParameterTypeString && azdMetadata.Type != nil &&
*azdMetadata.Type == azure.AzdMetadataTypeGenerate {

Expand Down
40 changes: 0 additions & 40 deletions cli/azd/test/functional/aspire_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@
package cli_test

import (
"context"
"fmt"
"os"
"path/filepath"
"strconv"
"strings"
"sync"
"testing"
"time"

Expand All @@ -32,44 +29,8 @@ import (
// (slow, > 10 mins) go test -run ^Test_CLI_Aspire_Deploy -timeout 30m - Full deployment acceptance tests.
// (all) go test -run ^Test_CLI_Aspire -timeout 30m - Runs all tests.

var dotnetWorkloadInstallOnce sync.Once

func restoreDotnetWorkload(t *testing.T) {
dotnetWorkloadInstallOnce.Do(func() {
dir := t.TempDir()
err := copySample(dir, "aspire-full")
require.NoError(t, err, "failed expanding sample")

ctx := context.Background()
appHostProject := filepath.Join(dir, "AspireAzdTests.AppHost")

wr := logWriter{initialTime: time.Now(), t: t, prefix: "restore: "}
commandRunner := exec.NewCommandRunner(nil)
cmd := "dotnet"
args := []string{"workload", "restore", "--skip-sign-check"}

// On platforms where the system requires `sudo` to install workloads (e.g. macOS and Linux when using system wide
// installations), you can configure sudo to allow passwordless execution of the `dotnet` command by adding something
// like the following to /etc/sudoers:
//
// matell ALL=(ALL) NOPASSWD: /usr/local/share/dotnet/dotnet
//
// and then set AZD_TEST_DOTNET_WORKLOAD_USE_SUDO=1 when running the tests, and we'll run `dotnet workload restore`
// via sudo.
if v, err := strconv.ParseBool(os.Getenv("AZD_TEST_DOTNET_WORKLOAD_USE_SUDO")); err == nil && v {
args = append([]string{cmd}, args...)
cmd = "sudo"
}

runArgs := newRunArgs(cmd, args...).WithCwd(appHostProject).WithStdOut(&wr)
_, err = commandRunner.Run(ctx, runArgs)
require.NoError(t, err)
})
}

// Test_CLI_Aspire_DetectGen tests the detection and generation of an Aspire project.
func Test_CLI_Aspire_DetectGen(t *testing.T) {
restoreDotnetWorkload(t)

sn := snapshot.NewDefaultConfig().WithOptions(cupaloy.SnapshotFileExtension(""))
snRoot := filepath.Join("testdata", "snaps", "aspire-full")
Expand Down Expand Up @@ -236,7 +197,6 @@ func Test_CLI_Aspire_DetectGen(t *testing.T) {

// Test_CLI_Aspire_Deploy tests the full deployment of an Aspire project.
func Test_CLI_Aspire_Deploy(t *testing.T) {
restoreDotnetWorkload(t)

t.Parallel()
ctx, cancel := newTestContext(t)
Expand Down
Loading
Loading