Skip to content

[HOTFIX] Add IAM Role / Instance Profile auth mode to AWS Bedrock adapter#1944

Merged
jaseemjaskp merged 4 commits intov0.163.2-hotfixfrom
fix/bedrock-iam-role-auth-v0.163.2
May 6, 2026
Merged

[HOTFIX] Add IAM Role / Instance Profile auth mode to AWS Bedrock adapter#1944
jaseemjaskp merged 4 commits intov0.163.2-hotfixfrom
fix/bedrock-iam-role-auth-v0.163.2

Conversation

@pk-zipstack
Copy link
Copy Markdown
Contributor

What

Adds an Authentication Type selector to the AWS Bedrock LLM and embedding adapter forms with two options:

  • Access Keys (default): existing flow — provide aws_access_key_id and aws_secret_access_key
  • IAM Role / Instance Profile (on-prem AWS only): no fields; relies on boto3's default credential chain

When the IAM Role mode is selected, the form-only auth_type field is stripped before LiteLLM validation, and any empty access keys are dropped from the validated kwargs so boto3 falls through to its default credential chain (IRSA on EKS, task role on ECS, instance profile on EC2, AWS Profile, env vars).

Why

Customers running Unstract on AWS infrastructure (specifically EKS with IRSA) cannot — and don't want to — paste long-lived AWS access keys into the adapter form. The pod's service account already has a federated identity that boto3 can use to call Bedrock; the adapter form just needs to accept blank credentials so the ambient identity takes over.

The S3/MinIO connector at unstract/connectors/src/unstract/connectors/filesystems/minio/minio.py already implements this idiom for S3:

if key and secret:
    creds["key"] = key
    creds["secret"] = secret

This PR applies the same pattern to the Bedrock adapter, with the addition of an explicit auth_type selector to make the user's choice clear in the UI.

How

unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/bedrock.json

Adds an auth_type enum with two options (access_keys default, iam_role) and a dependencies/oneOf block that:

  • Renders aws_access_key_id + aws_secret_access_key (both required) when auth_type == access_keys
  • Renders nothing extra when auth_type == iam_role

The selector's description explicitly notes the IRSA / instance-profile mode is for AWS-hosted Unstract deployments only.

unstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/bedrock.json

Same restructure. Drops aws_access_key_id and aws_secret_access_key from top-level required. Adds aws_profile_name for parity with the LLM form.

unstract/sdk1/src/unstract/sdk1/adapters/base1.py

In both AWSBedrockLLMParameters.validate() and AWSBedrockEmbeddingParameters.validate():

  • Strip the form-only auth_type field from validation metadata before Pydantic dump
  • Strip empty aws_access_key_id / aws_secret_access_key from the validated kwargs so boto3's default chain takes over (mirrors the S3/MinIO connector idiom)

AWSBedrockEmbeddingParameters fields gain explicit None defaults so the access-key fields are truly optional in either mode (parity with AWSBedrockLLMParameters which already had this from earlier work).

Can this PR break any existing features?

No. Backward-compatible across every scenario:

Scenario Behavior
Existing adapter with access keys filled in (no auth_type field stored) auth_type defaults to access_keys; keys forwarded to boto3 unchanged
New adapter, Access Keys mode Keys forwarded to boto3
New adapter, IAM Role mode No keys submitted; empty values stripped → boto3 default credential chain takes over
Existing adapter with aws_profile_name set Forwarded unchanged (not part of auth_type)
Bedrock enable_thinking / model_id (Application Inference Profile) Unchanged — these continue to work in both auth modes

End-to-end verified against real AWS Bedrock during development:

  • aws_role_name=arn:...:role/BedrockInvokeRole with bootstrap creds in env: STS:AssumeRole succeeds, Bedrock InvokeModel succeeds with temp creds (token usage returned).
  • auth_type=iam_role with no keys + ambient creds in env: boto3 default chain picks up the env vars; Bedrock call signs and routes correctly.

Relevant Docs

pk-zipstack and others added 2 commits May 6, 2026 01:35
Match the S3/MinIO connector pattern: when AWS access keys are left blank
on the Bedrock LLM and embedding adapter forms, drop them from the kwargs
dict so boto3's default credential chain handles authentication. This
unlocks IAM role / instance profile / IRSA / AWS Profile scenarios on
hosts that already have ambient AWS credentials (e.g. EKS workers with
IRSA, EC2 with an instance profile).

- llm1/static/bedrock.json: clarify access-key descriptions to mention
  IRSA and instance profile (already non-required at v0.163.2 base).
- embedding1/static/bedrock.json: drop aws_access_key_id and
  aws_secret_access_key from top-level required; same description fix;
  expose aws_profile_name for parity with the LLM form.
- base1.py: AWSBedrockLLMParameters and AWSBedrockEmbeddingParameters
  now strip empty access-key values from the validated kwargs before
  returning, so empty strings don't override boto3's default chain.
  AWSBedrockEmbeddingParameters fields gain explicit None defaults
  and an aws_profile_name field.

Backward-compatible: existing adapters with access keys filled in
continue to work unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add an explicit `auth_type` selector with two options, making the auth
choice clear to users:

- "Access Keys" (default): existing flow, keys required
- "IAM Role / Instance Profile (on-prem AWS only)": no fields; relies on
  boto3's default credential chain (IRSA on EKS, task role on ECS,
  instance profile on EC2). Description on the selector explicitly notes
  this option is only for AWS-hosted Unstract deployments.

The form-only auth_type field is stripped before LiteLLM validation in
both AWSBedrockLLMParameters.validate() and AWSBedrockEmbeddingParameters.
validate(). Empty access keys continue to be stripped so boto3 falls
through to the default chain even when the access_keys arm is selected
without values (matches the S3/MinIO connector pattern).

Backward-compatible: legacy adapters without auth_type behave as
"Access Keys" mode (the default), and existing keys are forwarded
unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 846b5f8b-0a78-4d81-bc8a-93e7bb3e5825

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/bedrock-iam-role-auth-v0.163.2

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 5, 2026

Greptile Summary

This PR adds an Authentication Type selector (access_keys / iam_role) to the AWS Bedrock LLM and embedding adapter forms, allowing customers running Unstract on AWS infrastructure (EKS/IRSA, ECS task roles, EC2 instance profiles) to authenticate via boto3's default credential chain instead of long-lived access keys. It also addresses a previously identified bug where switching a saved adapter from Access Keys to IAM Role mode would silently leak stale stored credentials.

  • base1.py: Extracts a shared _resolve_bedrock_aws_credentials helper that unconditionally drops access keys in iam_role mode, enforces non-blank values in access_keys mode, and preserves lenient legacy behaviour when auth_type is absent. Both AWSBedrockLLMParameters.validate() and AWSBedrockEmbeddingParameters.validate() are updated symmetrically.
  • bedrock.json (LLM + Embedding): Introduces the auth_type enum and a dependencies/oneOf block that conditionally renders credential fields only when access_keys is selected.
  • test_bedrock_adapter.py: New test file covering all auth matrix permutations for both adapters, including the stale-key regression, whitespace-only keys, unknown auth_type, and backwards-compatible legacy configs.

Confidence Score: 5/5

This PR is safe to merge — the auth logic is backward-compatible, the previously identified stale-key bug is correctly resolved, and the test matrix covers all four auth scenarios for both adapters.

The credential-stripping logic is applied unconditionally in iam_role mode regardless of what is stored, closing the stale-key leakage path. The access_keys mode enforces non-blank values rather than silently falling through to ambient credentials, and legacy adapters without auth_type continue to behave exactly as before. The JSON schema changes use a standard dependencies/oneOf pattern with a safe default (access_keys), so no existing stored configuration will be affected. Test coverage is thorough across both adapters.

No files require special attention — all four changed files implement the same well-reasoned pattern consistently.

Important Files Changed

Filename Overview
unstract/sdk1/src/unstract/sdk1/adapters/base1.py Adds shared _resolve_bedrock_aws_credentials helper; correctly unconditionally drops keys in iam_role mode, enforces non-blank in access_keys mode, and preserves legacy leniency — fixes the previously flagged stale-key bug.
unstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/bedrock.json Removes access key fields from top-level required, adds auth_type enum with dependencies/oneOf block that conditionally renders credentials; structure mirrors the LLM schema correctly.
unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/bedrock.json Moves access key fields into dependencies/oneOf under auth_type; auth_type defaults to access_keys preserving existing form behaviour for all existing adapter configurations.
unstract/sdk1/tests/test_bedrock_adapter.py Comprehensive new test file covering all auth matrix permutations (iam_role, access_keys, legacy) for both LLM and embedding adapters, including whitespace-key rejection and stale-key regression.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[adapter_metadata submitted] --> B{auth_type present?}
    B -- No / null --> C[Legacy mode: strip blank/None keys only]
    B -- access_keys --> D{aws_access_key_id & aws_secret_access_key non-blank?}
    B -- iam_role --> E[Unconditionally drop aws_access_key_id & aws_secret_access_key]
    B -- other --> F[Raise ValueError: Unknown auth_type]
    D -- Yes --> G[Forward keys to LiteLLM]
    D -- No --> H[Raise ValueError: key is required]
    C --> I[boto3 default credential chain takes over for missing keys]
    E --> I
    G --> J[LiteLLM / boto3 Authenticate with explicit keys]
    I --> K[boto3: IRSA / task role / instance profile / env vars / profile]
Loading

Reviews (2): Last reviewed commit: "[pre-commit.ci] auto fixes from pre-comm..." | Re-trigger Greptile

Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py Outdated
@pk-zipstack pk-zipstack requested a review from jaseemjaskp May 5, 2026 20:19
Copy link
Copy Markdown
Contributor

@jaseemjaskp jaseemjaskp left a comment

Choose a reason for hiding this comment

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

Comprehensive PR review via PR Review Toolkit (Comment Analyzer, PR Test Analyzer, Silent Failure Hunter, Type Design Analyzer, Code Reviewer, Code Simplifier). The greptile P1 on base1.py:548 is acknowledged and not duplicated; the findings below extend it (embedding-side parity) and surface separate concerns.

Priority distribution: 4 P0 (silent-failure / correctness), 3 P1 (structural / coverage), 4 P2 (style / docs).

Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py Outdated
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py Outdated
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py Outdated
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py Outdated
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/base1.py
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/bedrock.json Outdated
Comment thread unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/bedrock.json
pk-zipstack and others added 2 commits May 6, 2026 11:27
Fixes the P0/P1 issues raised by greptile-apps and jaseemjaskp on
PR #1944.

Behaviour fixes:
- Stale-key leak in IAM Role mode: switching an existing adapter from
  Access Keys to IAM Role would carry truthy stored access keys through
  the strip-empty-only loop, so boto3 silently authenticated with the
  old long-lived credentials instead of falling through to the host's
  IRSA / instance-profile identity. Both LLM and embedding paths were
  affected.
- Silent acceptance of unknown auth_type: a typo (e.g. "access_key") or
  a malformed payload from a non-UI client passed through the dict
  comprehension untouched, with no enum guard.
- Cross-field validation gap: explicit Access Keys mode with blank or
  whitespace-only values silently fell through to the default
  credential chain instead of surfacing the misconfiguration.

Implementation:
- Add a module-level _resolve_bedrock_aws_credentials helper used by
  both AWSBedrockLLMParameters.validate() and AWSBedrock
  EmbeddingParameters.validate(), so the auth-type contract is
  expressed once.
  - Validates auth_type against an allowlist (None | "access_keys" |
    "iam_role"); raises ValueError on anything else.
  - iam_role: unconditionally drops aws_access_key_id and
    aws_secret_access_key.
  - access_keys (explicit): requires non-blank values; raises ValueError
    if either is empty or whitespace-only.
  - Legacy (auth_type absent): retains the lenient strip behaviour so
    pre-PR adapter configurations continue to deserialise unchanged.
- Restore aws_region_name as required (no `= None` default) on
  AWSBedrockEmbeddingParameters; only credentials may legitimately be
  absent.
- Drop the orphan aws_profile_name field from
  embedding1/static/bedrock.json: it was added for parity with the LLM
  form but lives outside the auth_type oneOf and contradicts the
  selector's "no further input" semantics. The LLM form already had
  aws_profile_name pre-PR and is left alone for backwards compatibility.

Tests:
- New tests/test_bedrock_adapter.py covers 15 cases across LLM and
  embedding adapters: legacy-no-auth-type, explicit access_keys with
  valid/blank/whitespace keys, iam_role with stale/no keys, unknown
  auth_type rejection, cross-field validation, and preservation of
  unrelated params (model_id, aws_profile_name, region, thinking).

Skipped (P2 nice-to-have):
- Comment-scope clarification, MinIO reference rewording,
  validate-mutates-caller'\''s-dict, and the LLM form description nit
  about aws_profile_name visibility. These don'\''t change behaviour
  and can be addressed in a follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 6, 2026

@jaseemjaskp jaseemjaskp self-requested a review May 6, 2026 06:16
@jaseemjaskp jaseemjaskp merged commit c34f240 into v0.163.2-hotfix May 6, 2026
5 checks passed
@jaseemjaskp jaseemjaskp deleted the fix/bedrock-iam-role-auth-v0.163.2 branch May 6, 2026 06:30
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.

2 participants