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

Azure Provider: Authenticating using the Azure CLI #2300

Closed

Conversation

v-mavick
Copy link
Contributor

First editorial pass. Feedback greatly welcome!
@tombuildsstuff: Should the references to terraform and az on line 22 instead be Terraform and Azure?

@tombuildsstuff
Copy link
Contributor

hey @v-mavick

Thanks for this PR :)

Should the references to terraform and az on line 22 instead be Terraform and Azure?

Nope - taking a look at this these are a reference to the binaries terraform / az - so I think these are fine as-is (that said, we could probably rephrase this sentence without the reference? 🤔)

Thanks!

@v-mavick
Copy link
Contributor Author

v-mavick commented Nov 13, 2018 via email

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @v-mavick

Thanks for this PR :)

On the whole this is looking pretty good - if we can fix up the minor issues (and sort out the indentation) this should otherwise be good to merge 👍

Thanks!


When `terraform` and `az` are run on hosts / containers with different timezones, the variable $TZ should be set on the host.
When `terraform` and `az` are run on hosts/containers with different timezones, the variable $TZ should be set on the host.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like line 20 / 22 are saying the same thing; so perhaps we could combine them? 🤔

What do you think of:

When the timezones in which az and terraform differ, such as when these binaries are run from within Docker (which defaults to UTC) and the system timezone (which may not) - Terraform may incorrectly detect these access tokens as expired. It's possible to work around this by ensuring the Environment Variable $TZ is configured the same on both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified as suggested.

website/docs/authenticating_via_azure_cli.html.markdown Outdated Show resolved Hide resolved
```
```shell
$ az login
```
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this indentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed indentation.

```
```shell
To sign in, use a web browser to open the page https://aka.ms/devicelogin and enter the code XXXXXXXX to authenticate.
```
Copy link
Contributor

Choose a reason for hiding this comment

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

this behaviours changed slightly since then, so it could be worth updating the output from this block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this something I need to address in this document?


Also, if you have been authenticating with a service principal and you switch to Azure CLI, you must null out the ARM_* environment variables. Failure to do so causes errors to be thrown.
Also, if you have been authenticating with a service principal and you switch to Azure CLI, you must null out the ARM_* environment variables. Failure to do so causes errors to be thrown.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this indentation? this'll mean it's not rendered correctly, unfortunately

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indentation removed.


This will prompt you to open a web browser, as shown below:
1. You are prompted to open a web browser:
Copy link
Contributor

Choose a reason for hiding this comment

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

does this want to be 2.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It should be corrected in the next commit.


Once logged in - it's possible to list the Subscriptions associated with the account via:
1. Once logged in, it's possible to list the Subscriptions associated with the account via:
Copy link
Contributor

Choose a reason for hiding this comment

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

does this want to be 3.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I "hard-coded" the numbers, which should all display correctly now.

@v-mavick
Copy link
Contributor Author

v-mavick commented Nov 13, 2018 via email

Co-Authored-By: v-mavick <33636473+v-mavick@users.noreply.github.com>
@tombuildsstuff
Copy link
Contributor

hey @v-mavick

Thanks for pushing those changes - there's some changes coming to Azure CLI authentication in the form of #2387 such that this document changes somewhat. Since #2387 changes the document slightly (insofar as we no longer parse the Azure CLI access tokens) I've incorporated the changes made in this document into that PR - I hope you don't mind!

Thanks!

tombuildsstuff added a commit that referenced this pull request Nov 26, 2018
@v-mavick
Copy link
Contributor Author

v-mavick commented Nov 26, 2018 via email

@ghost ghost removed the waiting-response label Nov 26, 2018
tombuildsstuff added a commit that referenced this pull request Nov 27, 2018
* dependencies: upgrading to 0.2.0 of github.com/hashicorp/go-azure-helpers

* authentication: switching to use Azure CLI Token authentication

* documenting that only az 2.0 is supported

* Incorporating the changes from #2300
@ghost
Copy link

ghost commented Mar 5, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants