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

fix: az login error #227

Merged
merged 1 commit into from
Mar 19, 2024
Merged

fix: az login error #227

merged 1 commit into from
Mar 19, 2024

Conversation

nowjean
Copy link
Contributor

@nowjean nowjean commented Jan 30, 2024

Current 'az login' settings get the error.

This PR resolves the 'az login' error for workload Identity.
I have tested these changes in my AKS.

refer to Azure/azure-cli#26858

closes #224

@joshuafernandes
Copy link
Contributor

Hi @nowjean

Thankyou for this. Could you elaborate on this a bit more please?

The identity should still work and this change would break anyone using the native identity? Could we make this an option instead please?

If using federated identity use the method above, else use the normal identity login?

Cheers

@nowjean
Copy link
Contributor Author

nowjean commented Mar 13, 2024

Hi @joshuafernandes.

Thank for your review. By the way, this PR fix to az login error. We have fixed az login with workload Identity through this PR: #203

Current helm chart uses workload Identity not add-pod-identity which is deprecated. (Please refer to this link: Azure/aad-pod-identity#1349).

So, az login --identity --debug will be occur a login error. And az login --federated-token "$(cat $AZURE_FEDERATED_TOKEN_FILE)" --service-principal -u $AZURE_CLIENT_ID -t $AZURE_TENANT_ID can fix this error.

In above reason, I think we don't need to distribute az login. Please review this PR again. Thanks:)

Copy link
Contributor

@joshuafernandes joshuafernandes left a comment

Choose a reason for hiding this comment

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

LGTM

@joshuafernandes joshuafernandes merged commit 7cb2554 into Consensys:master Mar 19, 2024
1 check passed
@joshuafernandes
Copy link
Contributor

Hi @joshuafernandes.

Thank for your review. By the way, this PR fix to az login error. We have fixed az login with workload Identity through this PR: #203

Current helm chart uses workload Identity not add-pod-identity which is deprecated. (Please refer to this link: Azure/aad-pod-identity#1349).

So, az login --identity --debug will be occur a login error. And az login --federated-token "$(cat $AZURE_FEDERATED_TOKEN_FILE)" --service-principal -u $AZURE_CLIENT_ID -t $AZURE_TENANT_ID can fix this error.

In above reason, I think we don't need to distribute az login. Please review this PR again. Thanks:)

Thanks for the clarification @nowjean :)

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.

az login error
2 participants