-
Notifications
You must be signed in to change notification settings - Fork 216
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
Conversation
…o emulate az with azd
There was a problem hiding this 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.
Yeah. My first approach was to make the 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. :) |
There was a problem hiding this 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
@@ -194,68 +243,7 @@ func (r *commandRunner) RunList(ctx context.Context, commands []string, args Run | |||
if err != nil { | |||
return NewRunResult(-1, "", ""), err | |||
} | |||
|
There was a problem hiding this comment.
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:
RunList
will now log something incorrectly like.venv/bin/activate python
instead of.venv/bin/activate && python
.RunList
now honorsInteractive
flag / the attachment toStdin
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cli/azd/pkg/exec/az_emulator.go
Outdated
|
||
// 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") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
"github.com/spf13/pflag" | ||
) | ||
|
||
func azCliEmulateAccountCommands(root *actions.ActionDescriptor) *actions.ActionDescriptor { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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 |
Instead of emulating az, we will pivot and explore using MSI endpoint for terraform as the solution here |
The emulation strategy
When azd invokes
terraform cli
, anaz cli
is emulated during the execution of terraform cli automatically. This is how it works:az cli
from itImplementation notes:
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:
After this PR: