microsoft / AzureTRE

An accelerator to help organizations build Trusted Research Environments on Azure.
https://microsoft.github.io/AzureTRE
MIT License
185 stars 145 forks source link

make letsencypt fails on storage account key authentication error #4125

Closed jonnyry closed 1 week ago

jonnyry commented 2 weeks ago

(running on current main)

Image

jonnyry commented 2 weeks ago

@marrobi this line is failing when running Lets Encrypt from GitHub Actions:

https://github.com/microsoft/AzureTRE/blob/15fa48bd631dd6aa22d25a75996e8c51d47e284b/core/terraform/outputs.sh#L8-L12

It looks like its caused by the Storage Account Keys change https://github.com/microsoft/AzureTRE/pull/4104

I think the fix is the following (following your similar change to bootstrap.sh -

Add the following to outputs.sh since it looks like its trying to initialise the backend using a storage account key and failing:

      -backend-config="use_azuread_auth=true" \
      -backend-config="use_oidc=true"
marrobi commented 2 weeks ago

@jonnyry Just done some digging...

If -e USE_ENV_VARS_NOT_FILES="true" \ is set in the workflow these lines are needed in the action:

https://github.com/marrobi/AzureTRE/blob/a8c637121dc480a32d7bea2d4c573e1fabea743a/.github/actions/devcontainer_run_command/action.yml#L189-L191

@yuvalyaron I think rather than setting:

    # Configure AzureRM provider and backend to use Azure AD to connect to storage accounts
    export ARM_STORAGE_USE_AZUREAD=true
    export ARM_USE_AZUREAD=true
    export ARM_USE_OIDC=true

in devops/scripts/load_and_validate_env.sh - which is bypassed for CI/CD when no config file exists, the env variables need setting in devops/scripts/check_dependencies.sh or prior to each terraform command being run - so in the wrapper file, etc.

The commands can then need removing from the workflow too. otherwise everyone is going to have to update their custom workflows which is best to avoid.

jonnyry commented 2 weeks ago

@marrobi OK, ignore my PR - it works for my use case, but appreciate I haven't necessarily taken into account all flows through the build system.

marrobi commented 2 weeks ago

Sorry, didn't see the PR. Are you good to close it?

Think we need to revisit this and will look early next week.

yuvalyaron commented 2 weeks ago

@marrobi, it should also work, let's try that: https://github.com/microsoft/AzureTRE/pull/4131

The terraform_wrapper isn't an option because it's not invoked before all Terraform calls.

yuvalyaron commented 2 weeks ago

@jonnyry https://github.com/microsoft/AzureTRE/pull/4131 should fix the issue

jonnyry commented 1 week ago

Hi @yuvalyaron

Just to confirm that has fixed the issue.
Thank you :-)