Azure Credential to provide OIDC token for playbooks - #193
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds an Azure OIDC credential lookup plugin with Azure cloud selection, service-principal or managed-identity initialization, token retrieval, Azure error mapping, package registration, and importability coverage. ChangesAzure OIDC credential plugin
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant azure_oidc_backend
participant AzureClouds
participant _initialize_credential
participant AzureCredential
Caller->>azure_oidc_backend: provide url and cloud_name
azure_oidc_backend->>AzureClouds: resolve matching cloud
AzureClouds-->>azure_oidc_backend: selected cloud
azure_oidc_backend->>_initialize_credential: initialize credential
_initialize_credential-->>azure_oidc_backend: return Azure credential
azure_oidc_backend->>AzureCredential: get_token(url)
AzureCredential-->>azure_oidc_backend: token or Azure exception
azure_oidc_backend-->>Caller: token string or RuntimeError
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/awx_plugins/credentials/azure_token.py`:
- Around line 95-109: The credential initialization in _initialize_ currently
falls back to ManagedIdentityCredential whenever tenant, client, or secret is
missing, which can hide partially configured service-principal settings. Update
_initialize_ to explicitly detect when some but not all of tenant/client/secret
are provided and reject that configuration with an error instead of using
managed identity. Keep the existing ClientSecretCredential path for the fully
populated case, and only return ManagedIdentityCredential when none of the three
values are supplied.
- Around line 73-77: `cloud_name` is currently only exposed in the Azure
credential field definitions and is not propagated into the credential setup, so
the selected cloud never affects authentication. Update the Azure credential
flow by threading the chosen `cloud_name` from the credential constructors into
`_initialize_credential` (or through `azure_oidc_backend`), and use it to select
the correct Azure authority host instead of always defaulting to the public
cloud. Make sure the selection is wired through the relevant credential creation
path so `ClientSecretCredential` uses the matching cloud environment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 83cdb174-872d-423c-a128-9b3a027d7d9f
📒 Files selected for processing (1)
src/awx_plugins/credentials/azure_token.py
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
163-168: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd optional dependencies feature-flag for
azure_token.The new
azure_tokencredential plugin usesazure-identityandmsrestazure, but a corresponding feature-flag dependency group (e.g.,credentials-azure-token) is missing inpyproject.toml. This prevents end-users from easily installing the required dependencies for this new plugin.💻 Proposed fix
Add the following under
[project.optional-dependencies]:credentials-azure-token = [ "awx_plugins.interfaces", "azure-identity", "msrestazure", ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` around lines 163 - 168, Add a credentials-azure-token optional dependency group under [project.optional-dependencies] containing awx_plugins.interfaces, azure-identity, and msrestazure, matching the existing feature-flag dependency group structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/awx_plugins/credentials/azure_token.py`:
- Around line 98-118: Update _initialize_credential to reject partial
service-principal configuration instead of falling back to managed identity,
while preserving ClientSecretCredential for complete tenant, client, and secret
values. Construct ManagedIdentityCredential without the unsupported
cloud_environment argument, and pass client_id only when client is non-empty,
otherwise use None.
- Line 41: Update the default_cloud assignment to retain the
azure_cloud.AZURE_PUBLIC_CLOUD object instead of its .name string, so the
existing default_cloud.name accesses later in the plugin remain valid.
---
Outside diff comments:
In `@pyproject.toml`:
- Around line 163-168: Add a credentials-azure-token optional dependency group
under [project.optional-dependencies] containing awx_plugins.interfaces,
azure-identity, and msrestazure, matching the existing feature-flag dependency
group structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: acdc7ca2-3439-4668-af9f-7e2162c49205
📒 Files selected for processing (3)
pyproject.tomlsrc/awx_plugins/credentials/azure_token.pytests/importable_test.py
Requires Client Secret Credentials or a Host under Managed Identity.
f533152 to
b24b425
Compare
for more information, see https://pre-commit.ci
Requires Client Secret Credentials or a Host under Managed Identity.
Summary by CodeRabbit