Consensys / quorum-kubernetes

Helm charts for Hyperledger Besu and GoQuorum
Apache License 2.0
119 stars 128 forks source link

fix: az login error #227

Closed nowjean closed 7 months ago

nowjean commented 9 months ago

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 https://github.com/Azure/azure-cli/issues/26858

closes #224

joshuafernandes commented 7 months ago

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 commented 7 months ago

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: https://github.com/Consensys/quorum-kubernetes/pull/203

Current helm chart uses workload Identity not add-pod-identity which is deprecated. (Please refer to this link: https://github.com/Azure/aad-pod-identity/issues/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:)

joshuafernandes commented 7 months ago

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 :)