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

emulate az with azd - for terraform only #3971

Closed
wants to merge 14 commits into from

Conversation

vhvb1989
Copy link
Member

@vhvb1989 vhvb1989 commented Jun 3, 2024

The emulation strategy

When azd invokes terraform cli, an az cli is emulated during the execution of terraform cli automatically. This is how it works:

  • azd automatically sets 2 environment variables:
    • AZURE_AZ_EMULATOR=true
    • PATH=~/.azd/bin//az -> overrides the path for the terraform cli to execute az cli from it

Implementation notes:

  • azd makes a copy of azd binary into ~/.azd/bin/<unique-forlder>/az. The copy is deleted at the end of the command execution.

This strategy has been successfully tested for terraform.

Terraform local dev with azd

The steps for getting started with azd+terraform are:

before this PR:

  • install azd
  • azd auth login
  • azd init (terraform template or existing code)
  • install az
  • az login
  • az account set --subscription xxx-xxx-xx (select the same subscription you will use with azd)
  • azd up

After this PR:

  • install azd
  • azd auth login
  • azd init (terraform template or existing code)
  • azd up

Copy link
Contributor

@wbreza wbreza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel like the correct direction to create an emulation strategy to solve the authentication challenge with Azure and Terraform.

If the azure provider still requires the az CLI, then we should work with both the Azure team and Terraform to come up with a better strategy.

@vhvb1989
Copy link
Member Author

vhvb1989 commented Jun 3, 2024

This doesn't feel like the correct direction to create an emulation strategy to solve the authentication challenge with Azure and Terraform.

If the azure provider still requires the az CLI, then we should work with both the Azure team and Terraform to come up with a better strategy.

Yeah. My first approach was to make the azurerm provider to support using azd to auth. I created a PR for it but it was rejected by Hashicorp (see: hashicorp/terraform-provider-azurerm#22827 (comment)). They won't take changes to the auth and they are not using DefaultAzureCredentials :).

The other approach is to make azd to expose an MSI when invoking tools like terraform. But, that's way more complicated and the trick there would be to make terraform to think it is running in an Azure Environment. So, azd would be emulating Managed Identity, instead of the az. :)

Copy link
Contributor

@weikanglim weikanglim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we're still looking in this direction based on today's discussion, but I'll leave feedback I had recorded prior to the discussion.

cli/azd/pkg/exec/command_runner.go Outdated Show resolved Hide resolved
@@ -194,68 +243,7 @@ func (r *commandRunner) RunList(ctx context.Context, commands []string, args Run
if err != nil {
return NewRunResult(-1, "", ""), err
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to bring this up for discussion. I'm not entirely sure if the refactor for RunList and Run to share runImpl is 100% safe or in the right direction (do we have the full utility we want longer term?), but I could be easily convince with just another look from @ellismg's eyes on .

The notable behavioral changes I looked at by code diffing:

  1. RunList will now log something incorrectly like .venv/bin/activate python instead of .venv/bin/activate && python.
  2. RunList now honors Interactive flag / the attachment to Stdin reader. (I think this is an improvement?)

The main thing that I was previously hesitant about refactoring is that RunList will always take advantage of shell expansion, and I'd prefer the "nominal behavior" to not leverage the shell, but maybe this is wishful thinking since we have to do it for Windows anyways...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cli/azd/cmd/auth_token.go Outdated Show resolved Hide resolved

// creates a copy of azd binary and renames it to az and returns the path to it
func emulateAzFromPath() (string, error) {
path, err := exec.LookPath("azd")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be using os.Executable()? The azd in PATH is not necessarily the one being executed. The process executing may not be in PATH at all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. Not sure if it helps or not when running with debug.
Limiting to using PATH is at least a way to require azd to be installed and fail on non-standard cases. azd installation scripts will all set azd in the PATH

cli/azd/pkg/exec/az_emulator.go Outdated Show resolved Hide resolved
"github.com/spf13/pflag"
)

func azCliEmulateAccountCommands(root *actions.ActionDescriptor) *actions.ActionDescriptor {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned it today during our meeting, but it'd be nice to have a true emulation where we shim the few commands we need, and the rest are passthrough. I think this would maybe require thinking about changing how commands are registered in emulator mode.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating the passthrough-proxy is a little complicated due to authentication and current state of az.
Before investing time on building that, I would rather wait to see if we ever need to do passthrough for any tool. Right now, the passthrough is not required for the current scenario.

Also, the az emulator is just a temporary solution; to be removed by oneAuth eventually. So, I don't think it should have lot of investment.

cli/azd/pkg/exec/runargs.go Outdated Show resolved Hide resolved
@vhvb1989 vhvb1989 changed the title Adding support for commandRunner for merging system environment and to emulate az with azd emulate az with azd - for terraform only Jun 3, 2024
@ellismg
Copy link
Member

ellismg commented Jun 6, 2024

Based on internal team discussion - I think we are going to explore using a local MSI endpoint, similar to what is outlined in #3979 and then set the relevant terraform environment variables to tell the AzureRM provider to use MSI and our own endpoint instead of putting a dummy az on the path. I think we are all just a bit too nervous about hijacking az in this way, and so plugging in via MSI will have fewer moving parts.

@vhvb1989
Copy link
Member Author

Instead of emulating az, we will pivot and explore using MSI endpoint for terraform as the solution here

@vhvb1989 vhvb1989 closed this Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants