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
393 stars 187 forks source link

Prompting for parameters with aspire/azd #3597

Open mip1983 opened 5 months ago

mip1983 commented 5 months ago

azd version 1.7.0 (commit 49d6adc2efb178083f61822e6b4715258560803d)

Describe the issue

Having chatted about this on the aspire discussion area, @davidfowl asked me to raise this as an issue:

https://github.com/dotnet/aspire/discussions/2848#discussioncomment-8915919

I'm using preview 4 and trying to add a sql connection to an existing database.

I've added

var sqlConnection = builder.AddConnectionString("sql", "SQL_CONNECTION");

And then on my project:

var web = builder.AddProject<Projects.AspireTest>("aspire")
    .WithReference(sqlConnection)

This is fine running locally, reading the connection string from the AppHost secrets.

However, if you do an azd deploy, or run azd in your Azure DevOps pipeline, it asks for this connection string as a parameter. e.g. this in the prompt after 'azd deploy':

? Enter a value for the 'sql' infrastructure secured parameter:

If I provide this parameter, I think it essentially adds it to the main.paramaters.json, and then it's not so intuitive in terms of how to get this working as an environment variable so it can be built in a promptless CI/CI pipeline, the process doesn't tell you the name of this variable.

It is documented, but it would be good if during this prompt it could give you the option to save this parameter as an environment variable (and let you know the name of said variable) rather than it ending up in the json file.

Another potential option that might be nice to have is if aspire takes the 'environmentVariableName' param from AddConnectionString and outputs it to the manifest so that azd can use the same name for the environment variable input param (so it's named consistently and in a controllable/predictable way).

To Reproduce

Within an aspire project, declare a connection to an existing DB with the environmentVariableName param e.g.

var sqlConnection = builder.AddConnectionString("sql", "SQL_CONNECTION");

Expected behavior

I (wrongly) expected the project to require the environment variable name I'd given as input to populate the connection string value, wasn't clear how it worked until pointed at the docs

Environment Information on your environment:

ellismg commented 5 months ago

If I provide this parameter, I think it essentially adds it to the main.paramaters.json, and then it's not so intuitive in terms of how to get this working as an environment variable so it can be built in a promptless CI/CI pipeline, the process doesn't tell you the name of this variable.

Your intuition is more or less correct here (in practice the value is saved into a different file, specific to the environment), but then end to end here isn't great yet.

It is documented, but it would be good if during this prompt it could give you the option to save this parameter as an environment variable (and let you know the name of said variable) rather than it ending up in the json file.

What you can do today is the following - in the infra\main.parameters.json you could do something like this:

    "sql": {
      "value": "${SQL_CONNECTION_STRING}"
    },

That would cause azd read the value of SQL_CONNECTION_STRING from an environment variable (you can set this in your pipeline configuration) and assign that to the parameter.

We need to think about this whole end to end a little more and make this less painful, but perhaps this information helps unblock your specific problem today.

mip1983 commented 5 months ago

Hmm, I'm trying this and still running into problems in my CI/CI pipeline. The message I get is:

ERROR: failed deploying service 'ecodriver-analytics-api': failing invoking action 'deploy', failed executing template file: template: containerApp.tmpl.yaml:31:19: executing "containerApp.tmpl.yaml" at <securedParameter "ecoDriverDb">: error calling securedParameter: **parameter ecoDriverDb not found**

Which is confusing as the parameter is in my 'main.parameters.json':

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentParameters.json#",
  "contentVersion": "1.0.0.0",
  "parameters": {
    "ecoDriverDb": {
      "value": "${AZURE_ECO_DRIVER_DB}"
    },
    "environmentName": {
      "value": "${AZURE_ENV_NAME}"
    },
    "location": {
      "value": "${AZURE_LOCATION}"
    }
  }
}

That environment variable is set up in the variables group with the right name 'AZURE_ECO_DRIVER_DB' there in Azure Devops and is referenced in the build step:

- task: AzureCLI@2
  displayName: 'Deploy application to staging'
  inputs:
    azureSubscription: 'azDevServicePrincipalConnection'
    scriptType: 'bash'
    scriptLocation: 'inlineScript'
    inlineScript: 'azd deploy --no-prompt'
  env:
    AZURE_SUBSCRIPTION_ID: $(AZURE_SUBSCRIPTION_ID)
    AZURE_ENV_NAME: $(AZURE_ENV_NAME)
    AZURE_LOCATION: $(AZURE_LOCATION)
    AZURE_ECO_DRIVER_DB: $(AZURE_ECO_DRIVER_DB)

In the yaml file for the project it appears as:

      - name: connectionstrings--ecodriverdb
        value: '{{ securedParameter "ecoDriverDb" }}'

What am I missing, why is it not picking up the environment variable for this parameter?

vhvb1989 commented 4 months ago

@mip1983, the parameter mapping works only for the bicep template. It doesn't work for service yaml templates. For service's yaml template (in .Net Aspire world), secured parameters are persisted within the azd's environment config at .azure/env-name/config.json and never within the .azure/env-name/.env.

The expression: '{{ securedParameter "ecoDriverDb" }}' makes azd to look for the ecoDriverDb within the env's config.json.

Please try this:

  env:
    AZURE_SUBSCRIPTION_ID: $(AZURE_SUBSCRIPTION_ID)
    AZURE_ENV_NAME: $(AZURE_ENV_NAME)
    AZURE_LOCATION: $(AZURE_LOCATION)
    AZD_INITIAL_ENVIRONMENT_CONFIG: $(AZD_INITIAL_ENVIRONMENT_CONFIG)
mip1983 commented 4 months ago

I'm a bit lost there, I don't use azd pipeline config. I've set the build pipeline manually in Azure DevOps. I then push in variables for different environments using the 'Library' feature. I don't think I really want environment specific config on my local machine, particularly production values. e.g.

- ${{ if or(eq(parameters.provisionStage, true), eq(parameters.releaseStaging, true)) }}:
  - stage: 'DevEnvRelease'
    displayName: 'Development environment release'   
    jobs:
    - deployment: 'DevDeployment'
      displayName: 'Provision and/or Deploy dev'
      **variables:
        - group: 'AnalyticsDev'**
      environment: dev      
      strategy:
        runOnce:
          deploy:
            steps:
              - checkout: self

              - task: setup-azd@0 
                displayName: 'Setup AZD'

etc...

Is there a way to get azd to pick up this value that I'm trying to push in from the library?

vhvb1989 commented 4 months ago

@mip1983 , yes, if you are doing all manual, you will need to:

broline commented 4 months ago

Im having this same issue and tried following your advice with no luck. same issue on the deployment.

@vhvb1989

When you say

Push a variable AZD_INITIAL_ENVIRONMENT_CONFIG for each one of your environments.

What exactly do you mean by this?

Im assuming you mean to add an env var (AZD_INITIAL_ENVIRONMENT_CONFIG) to each github environment here

image

and then set the value to the json of the config.json

I tried adding AZD_INITIAL_ENVIRONMENT_CONFIG: $(AZD_INITIAL_ENVIRONMENT_CONFIG) to the env in my github actions workflow and then running azd pipeline config but it didnt automatically add that env var to github

vhvb1989 commented 4 months ago

What exactly do you mean by this?

I was referring to the Azure DevOps - library's variable group. Based on the commented user case:

I don't use azd pipeline config. I've set the build pipeline manually in Azure DevOps. I then push in variables for different environments using the 'Library' feature.

Within each variable group, there should be a variable AZD_INITIAL_ENVIRONMENT_CONFIG with the content of the infrastructure params. Then, the variable should be included in the pipeline definition (yaml)

broline commented 4 months ago

I got this to work in github

Added AZD_INITIAL_ENVIRONMENT_CONFIG env var for my 'dev' envrionment with the json from the config.json of that environment from my local machine. Full azure-dev.yml looks like this

on:
  workflow_dispatch:
  push:
    # Run when commits are pushed to mainline branch (main or master)
    # Set this to the mainline branch you are using
    branches:
      - dev

permissions:
  id-token: write
  contents: read

jobs:
  build:
    runs-on: ubuntu-latest
    environment: dev
    env:
      AZURE_CLIENT_ID: ${{ vars.AZURE_CLIENT_ID }}
      AZURE_TENANT_ID: ${{ vars.AZURE_TENANT_ID }}
      AZURE_SUBSCRIPTION_ID: ${{ vars.AZURE_SUBSCRIPTION_ID }}
      AZURE_CREDENTIALS: ${{ secrets.AZURE_CREDENTIALS }}
      AZURE_LOCATION: ${{ vars.AZURE_LOCATION }}
      AZD_INITIAL_ENVIRONMENT_CONFIG: ${{ vars. AZD_INITIAL_ENVIRONMENT_CONFIG }}
    steps:
      - name: Checkout
        uses: actions/checkout@v4

      - name: Install azd
        uses: Azure/setup-azd@v0.1.0

      - name: Install .NET Aspire workload
        run: dotnet workload install aspire

      - name: Log in with Azure (Federated Credentials)
        if: ${{ env.AZURE_CLIENT_ID != '' }}
        run: |
          azd auth login `
            --client-id "$Env:AZURE_CLIENT_ID" `
            --federated-credential-provider "github" `
            --tenant-id "$Env:AZURE_TENANT_ID"
        shell: pwsh

      - name: Log in with Azure (Client Credentials)
        if: ${{ env.AZURE_CREDENTIALS != '' }}
        run: |
          $info = $Env:AZURE_CREDENTIALS | ConvertFrom-Json -AsHashtable;
          Write-Host "::add-mask::$($info.clientSecret)"

          azd auth login `
            --client-id "$($info.clientId)" `
            --client-secret "$($info.clientSecret)" `
            --tenant-id "$($info.tenantId)"
        shell: pwsh

      - name: Provision Infrastructure
        run: azd provision -e dev --no-prompt

      - name: Deploy Application
        run: azd deploy -e dev --no-prompt

However, one interesting gotcha was that when i do azd pipeline config the client id that it stores in gitbub variables was wrong so i just had to update that in github with the correct one

Looks like it was using the value from AZURE_PIPELINE_CLIENT_ID in the .env file instead of MANAGED_IDENTITY_CLIENT_ID... despite AZD_PIPELINE_PROVIDER being set to 'github' Not sure why they are different though.

mip1983 commented 4 months ago

I've not tried out adding 'AZD_INITIAL_ENVIRONMENT_CONFIG' to my library just yet, I imagine adding a multi-line json string to the library would work, just looks a little messy in what is otherwise key/value pairs.

What I've done at the moment is use 'azd infra synth'. I've left the connection string as an input in main.parameters and main.bicep. I just then added the connection string as an output variable at the end of the main.bicep file. I've then updated the tml.yaml file, replacing the parameter with something along these lines:

      - name: connectionstrings--sql
        value: '{{ .Env.SQL_CONNECTIONSTRING }}'    <-- Name of the output variable from main.bicep

Seen as this is working for now, and is nice and consistent with how all the other connection strings are set up, I might leave as is and hang on a little longer to see if a more user-friendly way of doing this emerges.

Maybe anything that's an input in main.parameters.json should be available as an output like I've done. Then it can be used easily by CI/CD and you don't have this separate and not so intuitive home for variables in the form of json against 'AZD_INITIAL_ENVIRONMENT_CONFIG'. If other connection strings are ending up in the .env file, this separate secure parameter concept seems a bit redundant and confusing.

Bpflugrad commented 4 months ago

@mip1983 Is this still working after AZD 1.9.0? I tried implementing your solution by editing the azd infra synth but my *.tmpl.yaml files are being ignored by azd deploy.

vhvb1989 commented 4 months ago

@mip1983 , ideally, you should not set connections strings as output params and/or write them to .env files. Your workaround of updating the tml.yaml works, but you are relaxing security.

Consider using AZD_INITIAL_ENVIRONMENT_CONFIG. See this: https://github.com/Azure/azure-dev/issues/3850#issuecomment-2103915345

mip1983 commented 4 months ago

@mip1983 , ideally, you should not set connections strings as output params and/or write them to .env files. Your workaround of updating the tml.yaml works, but you are relaxing security.

Consider using AZD_INITIAL_ENVIRONMENT_CONFIG. See this: #3850 (comment)

I don't feel this is well addressed with this 'AZD_INITIAL_ENVIRONMENT_CONFIG' param. It doesn't suit the key-value nature of an Azure DevOps variable library, it's not intuitive or well documented (correct me if I've missed something there) and I think it's left a few users scratching heads from what I can tell. It's overly complex to simply add a connection string.

You've got all the other config including connection strings for blob storage and app insights etc. being put into .env files when using azd locally. Why is this bit of config not in the same place, and why is the json file secure and the env file not? It's not a file in source control, and I'm only using it locally during the setup phase to test things. It's not something that will get checked in and used as a real store for the wider dev team. The dev team will have this in secrets.json during local dev, and it'll be secured in Azure DevOps for deployment.

The connection strings for deployment in an Azure DevOps library are secured by permissions within Azure DevOps and can be backed by key vault, so this seems like a reasonable place to want to keep this config securely, having different libraries for different environments.

As such, I really think you should be able to easily pass variables like these connection strings from a library in Azure DevOps into azd in your CI/CI pipeline. It's a bit messy with one connection at the moment, but if you had multiple and you're cramming them all into this one 'AZD_INITIAL_ENVIRONMENT_CONFIG' variable in your library, it's just not very clean and doesn't seem like how it should be organized.

Originally I was trying to pass this variable in my pipeline via the 'env' section, I think this should be supported with it being on the user to secure Azure Devops libraries. If it could accept this input as a secure param, I wouldn't then need the work around outputting it in bicep.

Or better yet, if possible, the task should pick up matching variables from the variable group specified on the pipeline without having to manually pass them in on the task.

/unresolve

mip1983 commented 4 months ago

@mip1983 Is this still working after AZD 1.9.0? I tried implementing your solution by editing the azd infra synth but my *.tmpl.yaml files are being ignored by azd deploy.

Yes, this has stopped working for me since 1.9, I'm going to do some more playing around with it, but it seems like it's not picking up things in the yaml file, including my custom domain binding (#3875).

vhvb1989 commented 4 months ago

You've got all the other config including connection strings for blob storage and app insights etc. being put into .env files

There should be only host names, but not connection strings (including keys).

For any parameter that Aspire-manifest defines as secured() , azd can't just place the value for it in plain text in .env Even if it is not checked-in to the repo, it becomes a vulnerability (what if the repo is not github, or what if the .gitignore gets removed, etc). The security bar must be as high as possible when it comes to parameters set as secured.

In a world where azd would only be meant to run in CI/CD, it would be ok to directly use one secret for each parameter, which, in fact, works for mapping env to infra. But for deploy services, azd needs to know where to get the value from (.env , config, etc). Regardless of where azd is running (CI or local).

mip1983 commented 4 months ago

Ok, so because it could have credentials and is marked as secured() it get's treated with extra caution locally and you don't see the value in .env or config.json, makes sense.

What about allowing the user to explicitly specify as an argument that they want to accept secure params from the environment so this is something you can do deliberately just in a pipeline? i.e.

  - task: AzureCLI@2
    displayName: 'Deploy application to staging'
    inputs:
      azureSubscription: 'mySub'
      scriptType: 'bash'
      scriptLocation: 'inlineScript'
      inlineScript: 'azd deploy --no-prompt --secure-params-from-env'  <-- Extra arg
    env:
      AZURE_SUBSCRIPTION_ID: $(AZURE_SUBSCRIPTION_ID)
      AZURE_ENV_NAME: $(AZURE_ENV_NAME)
      AZURE_LOCATION: $(AZURE_LOCATION)
      AZURE_MY_SECURE_PARAM: $(Library_Variable)  <-- This is the parameter name you see in main.parameters.json, azd should give a clear message in the console output with the variable name if it's not been supplied rather than just panic.
rajeshkamal5050 commented 3 weeks ago

@vhvb1989 @mip1983 can we mark this complete? anything pending here?

mip1983 commented 3 weeks ago

@rajeshkamal5050

Hi there, nothing has changed much on this front as far as I'm aware.

I'd say if the goal of Aspire is to simplify and remove the pain/complexity of building and deploying cloud native apps, then this aspect of supplying a connection string (secured parameter) in a CI/CD pipeline (including one that's not had the yaml generated via azd) is still too difficult to figure and clunky (in my opinion, but I think I've seen a few posts by others on GitHub on it). It isn't really covered in the documentation unless there's more detail elsewhere?

I still have 'AZD_INITIAL_ENVIRONMENT_CONFIG' with some JSON in the value field in Libraries in the Azure DevOps which is really for key/value pairs. I'd like to see a better way and clear documentation, perhaps like I've mentioned in my previous comment.

Mike-E-angelo commented 1 week ago

it's not intuitive or well documented (correct me if I've missed something there)

I can confirm that this issue/thread is the "Best" documentation that I have found for this field and I am still very much unclear how to use it for my own purposes.