Support managed identity for Azure App Service/Azure Container Apps #7086
Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
apc-kamezaki
force-pushed
the
feature/appservice-msi
branch
from
2b12d3a
to
5d09567
Compare
| @@ -55,15 +58,25 @@ func (ap *azureManagedIdentitiesAuthPlugin) NewClient(c Config) (*http.Client, e | |||
| } | |||
|
|
|||
| if ap.Endpoint == "" { | |||
| ap.Endpoint = azureIMDSEndpoint | |||
| identityEndpoint := os.Getenv("IDENTITY_ENDPOINT") | |||
There was a problem hiding this comment.
Are there any docs or links we can add here that point to the ordering used here? I mean if IDENTITY_ENDPOINT is set, does that determine the flow?
There was a problem hiding this comment.
I checked the Azure SDK implementation as a reference.
There was a problem hiding this comment.
@apc-kamezaki the changes looks good. Just a comment for clarification 👇
|
@apc-kamezaki if you could please update your commit message as per the guidelines documented here, we can get this in. Thanks. |
apc-kamezaki
force-pushed
the
feature/appservice-msi
branch
from
5d09567
to
815d1cb
Compare
|
@ashutosh-narkar I updated the commit message. |
|
Do we need to update anything in the docs? |
apc-kamezaki
force-pushed
the
feature/appservice-msi
branch
from
815d1cb
to
6a52076
Compare
|
I'm sorry I forgot to write document. I tried to wrote the docs at this time. |
docs/content/configuration.md
Outdated
| | `services[_].credentials.azure_managed_identity.resource` | `string` | No | App ID URI of the target resource. (default: `https://storage.azure.com/`) | | ||
| | `services[_].credentials.azure_managed_identity.object_id` | `string` | No | Optional object ID of the managed identity you would like the token for. Required, if your VM has multiple user-assigned managed identities. | | ||
| | `services[_].credentials.azure_managed_identity.client_id` | `string` | No | Optional client ID of the managed identity you would like the token for. Required, if your VM has multiple user-assigned managed identities. | | ||
| | `services[_].credentials.azure_managed_identity.mi_res_id` | `string` | No | Optional Azure Resource ID of the managed identity you would like the token for. Required, if your VM has multiple user-assigned managed identities. | | ||
| | `services[_].credentials.azure_managed_identity.use_app_service_msi` | `bool` | No | Set true if you would like to set `x-identity-header` instead of `Metadata` header for token request. Default: set true automatically when you use managed identity on Azure App Servic or Container Apps. | |
There was a problem hiding this comment.
Why do we need to expose this? From the code, it looks like if IDENTITY_ENDPOINT is set, then .UseAppServiceMsi is true.
There was a problem hiding this comment.
I included this value in case we want to explicitly use IMDS and disable auto detection.
I updated the document again at this time.
There was a problem hiding this comment.
Let's remove use_app_service_msi from the docs. If we need it, we can always add it.
There was a problem hiding this comment.
Ok, I'll remove it.
apc-kamezaki
force-pushed
the
feature/appservice-msi
branch
from
6a52076
to
d2486cc
Compare
docs/content/configuration.md
Outdated
|
|
||
| The following is an example of how to use an [Azure storage | ||
| The following is an example of how to use an [Azure storageß |
There was a problem hiding this comment.
Oh sorry. I'll fix it.
There was a problem hiding this comment.
@apc-kamezaki please make the doc update and we can then get this in. Thanks for the contribution!
apc-kamezaki
force-pushed
the
feature/appservice-msi
branch
2 times, most recently
from
08a7fe1
to
abb9c96
Compare
|
Thank you for your review. I've updated the docs. |
…iner Apps IDENTITY_ENDPOINT and IDENTITY_HEADER envirnnment variables are provided on Azure App Service for getting the token. We can detect these variables and switch the endpoint and header value from IMDS. Fixes: open-policy-agent#7085 Signed-off-by: Hitoshi Kamezaki <[email protected]>
ashutosh-narkar
force-pushed
the
feature/appservice-msi
branch
from
abb9c96
to
7405de4
Compare
Why the changes in this PR are needed?
As you can see on the discussion https://github.com/orgs/open-policy-agent/discussions/592 , opa server cannot connect azure blob storage on Azure App Service using managed identity.
It seems that IMDS(Instance MetaData Service) endpoint is not available on Azure App Service/Container Apps. It should use special endpoint for getting token instead.
See :
https://learn.microsoft.com/en-us/azure/app-service/overview-managed-identity?tabs=portal%2Chttp#connect-to-azure-services-in-app-code
https://learn.microsoft.com/en-us/azure/container-apps/managed-identity?tabs=bicep%2Chttp#connect-to-azure-services-in-app-code
What are the changes in this PR?
IDENTITY_ENDPOINT and IDENTITY_HEADER envirnnment variables are defined on Azure App Service for getting the token.
Detect these variables and switch the endpoint and header value from IMDS to IDENTITY_ENDPOINT
Notes to assist PR review:
IMDS endpoint (169.254.169.254) and IDENTITY_ENDPOINT is the almost same request/response format.
But minimum api-version is bit different. It should be 2019-08-01 or later.
Here is the endpoint reference : https://learn.microsoft.com/en-us/azure/app-service/overview-managed-identity?tabs=portal%2Chttp#rest-endpoint-reference
Further comments:
This PR is related to #7085