Improve AWS IRSA credential handling with role chaining and regional STS by willdavsmith · Pull Request #11570 · radius-project/radius

willdavsmith · 2026-04-07T17:02:28Z

Overview

This PR improves the robustness of the AWS IRSA (IAM Roles for Service Accounts) credential acquisition path. Split from #11538 to separate credential improvements from the bug fix in #11569.

Changes

Role chaining for IRSA credentials (`pkg/ucp/aws/ucpcredentialprovider.go`)

The IRSA credential path now performs a two-step credential retrieval:

AssumeRoleWithWebIdentity — obtains initial credentials using the OIDC token
AssumeRole on the same role — converts web identity federation credentials into standard IAM session credentials

This avoids web identity session restrictions that can cause issues when downstream AWS services (e.g., CloudControl/CloudFormation) attempt further role assumptions.

Regional STS endpoint support

pkg/ucp/frontend/aws/routes.go: Reads AWS_REGION/AWS_DEFAULT_REGION env vars for STS configuration
deploy/Chart/templates/ucp/deployment.yaml: Injects AWS_REGION env var into the UCP container
deploy/Chart/values.yaml: Adds global.aws.region Helm value
.github/workflows/functional-test-cloud.yaml: Passes AWS_REGION to Helm install

Code quality improvements

Use ctx instead of context.TODO() for config.LoadDefaultConfig calls
Apply c.options.Duration to both web identity and AssumeRole sessions
Use logger.Error for error paths instead of logger.Info
Use idiomatic int32(duration / time.Second) for duration conversion

IAM Role Requirement

The AWS IAM role trust policy must allow sts:AssumeRole from cloudformation.amazonaws.com and self-assumption for the role chaining to work:

{
    "Effect": "Allow",
    "Principal": {
        "Service": "cloudformation.amazonaws.com"
    },
    "Action": "sts:AssumeRole"
}

This trust policy statement is already configured on the functional test IAM role.

Contributor checklist

Please verify that the PR meets the following requirements, where applicable:

An overview of proposed schema changes is included in a linked GitHub issue.
- Yes
- Not applicable
A design document PR is created in the design-notes repository, if new APIs are being introduced.
- Yes
- Not applicable
The design document has been reviewed and approved by Radius maintainers/approvers.
- Yes
- Not applicable
A PR for the samples repository is created, if existing samples are affected by the changes in this PR.
- Yes
- Not applicable
A PR for the documentation repository is created, if the changes in this PR affect the documentation or any user facing updates are made.
- Yes
- Not applicable
A PR for the recipes repository is created, if existing recipes are affected by the changes in this PR.
- Yes
- Not applicable

Copilot

Pull request overview

Improves AWS IRSA credential acquisition in the UCP by introducing role chaining (web identity → standard AssumeRole session) and enabling optional regional STS endpoint selection, with supporting Helm/workflow wiring.

Changes:

Add IRSA role chaining in UCPCredentialProvider and propagate STS endpoint region configuration.
Read AWS_REGION / AWS_DEFAULT_REGION in the AWS frontend module and pass through to the credential provider.
Add Helm value + deployment env var injection for AWS_REGION, and pass the value in the cloud functional test workflow.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/ucp/frontend/aws/routes.go	Reads region env vars and passes STS region to the UCP AWS credential provider.
pkg/ucp/aws/ucpcredentialprovider.go	Implements IRSA role chaining and adds STS endpoint region option.
pkg/ucp/aws/ucpcredentialprovider_test.go	Updates constructor usage and validates default/explicit STS region option behavior.
deploy/Chart/values.yaml	Adds `global.aws.region` Helm value.
deploy/Chart/templates/ucp/deployment.yaml	Injects `AWS_REGION` env var into the UCP deployment when configured.
.github/workflows/functional-test-cloud.yaml	Passes `global.aws.region` into Helm install; updates pinned action SHAs.

Copilot · 2026-04-07T17:07:30Z

.github/workflows/functional-test-cloud.yaml


      - name: Setup Go
-        uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0
+        uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0


This pins actions/setup-go to v6.3.0, while other workflows in this repo are pinned to v6.4.0 (e.g., .github/workflows/build.yaml). Unless there’s a specific regression being worked around, keep the version/SHA consistent across workflows and prefer the newer pinned revision to reduce maintenance and security risk.

Suggested change

uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0

uses: actions/setup-go@v6.4.0 # v6.4.0

These version mismatches were from the branch being based on a stale point in main's history. Rebased on latest main and reset the workflow file — the only diff now is the --set global.aws.region line we added. The SHAs need to be locked to specific commits as we do throughout the rest of the repo, so taking main's current pinned versions is the right approach.

Copilot · 2026-04-07T17:07:30Z

.github/workflows/functional-test-cloud.yaml

          permission-checks: write

-      - uses: LouisBrunner/checks-action@937cbbcde3259005b50746dc91cde29098aac2ff # v3.1.0
+      - uses: LouisBrunner/checks-action@dfcbcf801bff1ea7f1414824fc28f2cd697b35da # v3.0.0


This changes LouisBrunner/checks-action from v3.1.0 to v3.0.0. Please avoid downgrading action versions unless necessary; either keep it at the newer pinned SHA used previously or add a short comment explaining the compatibility reason for the downgrade.

Suggested change

- uses: LouisBrunner/checks-action@dfcbcf801bff1ea7f1414824fc28f2cd697b35da # v3.0.0

- uses: LouisBrunner/checks-action@v3.1.0

Same as above — resolved by rebasing on main. These stale SHAs are no longer in the diff.

Copilot · 2026-04-07T17:07:30Z

.github/workflows/functional-test-cloud.yaml

          permission-checks: write
          permission-pull-requests: write
          permission-contents: read



This changes LouisBrunner/checks-action from v3.1.0 to v3.0.0. Please avoid downgrading action versions unless necessary; either keep it at the newer pinned SHA used previously or add a short comment explaining the compatibility reason for the downgrade.

Suggested change

# Intentionally pinned to v3.0.0 for workflow compatibility; do not upgrade back to v3.1.0 until that issue is resolved.

Resolved by rebase — no longer in the diff.

Copilot · 2026-04-07T17:07:31Z

.github/workflows/functional-test-cloud.yaml

          echo "test_status=$TEST_STATUS" >> $GITHUB_OUTPUT

-      - uses: LouisBrunner/checks-action@937cbbcde3259005b50746dc91cde29098aac2ff # v3.1.0
+      - uses: LouisBrunner/checks-action@dfcbcf801bff1ea7f1414824fc28f2cd697b35da # v3.0.0


This changes LouisBrunner/checks-action from v3.1.0 to v3.0.0. Please avoid downgrading action versions unless necessary; either keep it at the newer pinned SHA used previously or add a short comment explaining the compatibility reason for the downgrade.

Suggested change

- uses: LouisBrunner/checks-action@dfcbcf801bff1ea7f1414824fc28f2cd697b35da # v3.0.0

- uses: LouisBrunner/checks-action@v3.1.0

Resolved by rebase — no longer in the diff.

github-actions · 2026-04-07T17:18:13Z

Unit Tests

2 files ±0 415 suites ±0 6m 42s ⏱️ -7s
4 872 tests ±0 4 870 ✅ ±0 2 💤 ±0 0 ❌ ±0
5 774 runs ±0 5 772 ✅ ±0 2 💤 ±0 0 ❌ ±0

Results for commit 478a568. ± Comparison against base commit ec0db63.

♻️ This comment has been updated with latest results.

codecov · 2026-04-07T17:18:53Z

Codecov Report

❌ Patch coverage is 12.00000% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.34%. Comparing base (ec0db63) to head (478a568).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/ucp/aws/ucpcredentialprovider.go	13.63%	38 Missing ⚠️
pkg/ucp/frontend/aws/routes.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11570      +/-   ##
==========================================
- Coverage   51.38%   51.34%   -0.04%     
==========================================
  Files         699      699              
  Lines       44114    44148      +34     
==========================================
+ Hits        22668    22669       +1     
- Misses      19278    19310      +32     
- Partials     2168     2169       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Add role chaining to the IRSA credential path: after AssumeRoleWithWebIdentity, perform a follow-up AssumeRole on the same role to produce standard IAM session credentials. This avoids web identity session restrictions that can cause issues with downstream AWS services. Also adds support for configuring the STS endpoint region via AWS_REGION environment variable, plumbed through Helm values and the cloud functional test workflow. Additional improvements from review feedback: - Use ctx instead of context.TODO() for config.LoadDefaultConfig calls - Apply credential Duration to both web identity and AssumeRole sessions - Use logger.Error for error paths instead of logger.Info - Use int32 division for duration conversion instead of float-based .Seconds() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Copilot · 2026-04-07T20:55:25Z

pkg/ucp/aws/ucpcredentialprovider.go

+			RoleArn:         &s.IRSACredential.RoleARN,
+			RoleSessionName: aws.String(sessionPrefix + uuid.New().String()),
+		}
+		if c.options.Duration > 0 {


DurationSeconds is derived via int32(c.options.Duration / time.Second), which truncates sub-second durations to 0 and can also produce values outside STS’s accepted range (and for role chaining the max is more restrictive). Validate/clamp the duration (or omit DurationSeconds when it would be invalid) before calling AssumeRole to avoid runtime validation errors.

Suggested change

if c.options.Duration > 0 {

if c.options.Duration >= 15*time.Minute && c.options.Duration <= time.Hour {

Good catch — added validation to only set DurationSeconds when the value falls within the STS-accepted range for role chaining (15 min to 1 hour).

Copilot · 2026-04-07T20:55:25Z

pkg/ucp/aws/ucpcredentialprovider.go

+	// STSEndpointRegion is the AWS region to use for the STS endpoint when retrieving
+	// IRSA credentials. Using a regional STS endpoint (matching the target service region)
+	// avoids token compatibility issues with some AWS services like CloudWatch Logs.
+	// If empty, defaults to "us-east-1" (global endpoint).


The field comment says an empty STSEndpointRegion “defaults to "us-east-1" (global endpoint)”, but the implementation only sets config.WithRegion("us-east-1") and does not explicitly configure the legacy/global STS endpoint. Please reword this to avoid implying global endpoint behavior, or explicitly configure the endpoint if that’s required.

Suggested change

// If empty, defaults to "us-east-1" (global endpoint).

// If empty, defaults to "us-east-1".

Fixed — removed the "(global endpoint)" phrasing.

Copilot · 2026-04-07T20:55:25Z

pkg/ucp/aws/ucpcredentialprovider.go

+		reAssumeCfg, err := config.LoadDefaultConfig(ctx,
+			config.WithRegion(stsRegion),
+			config.WithCredentialsProvider(aws.CredentialsProviderFunc(
+				func(ctx context.Context) (aws.Credentials, error) {
+					return webIdentityCreds, nil
+				},
+			)),
+		)


Retrieve calls config.LoadDefaultConfig twice (for awscfg and reAssumeCfg). This is relatively expensive and will run every time credentials are refreshed. Consider reusing the first loaded config and creating the second STS client via explicit sts.Options (region + credentials) instead of re-loading the full default config.

Fixed — now reuses the existing awscfg and creates the second STS client via sts.NewFromConfig(awscfg, ...) with a credentials override, avoiding the second LoadDefaultConfig call.

Copilot · 2026-04-07T20:55:25Z

pkg/ucp/aws/ucpcredentialprovider.go

+		// Step 1: Get web identity credentials via AssumeRoleWithWebIdentity.
+		webIdentityProvider := stscreds.NewWebIdentityRoleProvider(
+			stsClient,
 			s.IRSACredential.RoleARN,
 			stscreds.IdentityTokenFile(TokenFilePath),
 			func(o *stscreds.WebIdentityRoleOptions) {
-				o.RoleSessionName = sessionPrefix + uuid.New().String()
+				o.RoleSessionName = sessionPrefix + "wi-" + uuid.New().String()
+				if c.options.Duration > 0 {
+					o.Duration = c.options.Duration
+				}
 			},
-		))
+		)

-		value, err = credsCache.Retrieve(ctx)
+		webIdentityCreds, err := webIdentityProvider.Retrieve(ctx)
+		if err != nil {
+			logger.Error(err, "Failed to retrieve web identity credentials")
+			return aws.Credentials{}, err
+		}
+		logger.Info("Successfully retrieved web identity credentials")
+


The new IRSA role-chaining path (AssumeRoleWithWebIdentity -> AssumeRole) isn’t covered by unit tests. Add a test that stubs STS responses to assert both calls happen, and that Duration and STSEndpointRegion propagate into the STS requests (including the session name changes).

Agreed this would be valuable. The current STS client is created directly from the AWS SDK, so testing the IRSA path would require either injecting an STS client factory or using a higher-level interface. Will track this as a follow-up.

@willdavsmith should we track this in a github issue?

- Validate DurationSeconds falls within STS-accepted range (15min-1hr) for role chaining sessions before setting it - Fix STSEndpointRegion comment to not imply global endpoint behavior - Reuse existing AWS config for second STS client instead of calling LoadDefaultConfig twice Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Copilot · 2026-04-07T21:16:23Z

pkg/ucp/aws/ucpcredentialprovider.go

+		logger.Info("Successfully re-assumed role for clean session credentials")
+
+		value = aws.Credentials{
+			AccessKeyID:     *assumeRoleOutput.Credentials.AccessKeyId,
+			SecretAccessKey:  *assumeRoleOutput.Credentials.SecretAccessKey,
+			SessionToken:    *assumeRoleOutput.Credentials.SessionToken,
+			Source:          credentialSource,
+			CanExpire:       true,
+			Expires:         assumeRoleOutput.Credentials.Expiration.UTC(),


assumeRoleOutput.Credentials and its fields (AccessKeyId, SecretAccessKey, SessionToken, Expiration) are pointers in the AWS SDK. This code dereferences them without nil checks, and Expiration.UTC() will not compile if Expiration is *time.Time. Add a defensive nil check for assumeRoleOutput.Credentials and use a safe conversion/dereference for Expiration (and the string fields) before constructing aws.Credentials to avoid panics/compile errors.

Suggested change

logger.Info("Successfully re-assumed role for clean session credentials")

value = aws.Credentials{

AccessKeyID: *assumeRoleOutput.Credentials.AccessKeyId,

SecretAccessKey: *assumeRoleOutput.Credentials.SecretAccessKey,

SessionToken: *assumeRoleOutput.Credentials.SessionToken,

Source: credentialSource,

CanExpire: true,

Expires: assumeRoleOutput.Credentials.Expiration.UTC(),

if assumeRoleOutput.Credentials == nil {

return aws.Credentials{}, fmt.Errorf("assume role returned nil credentials")

}

logger.Info("Successfully re-assumed role for clean session credentials")

credentials := assumeRoleOutput.Credentials

value = aws.Credentials{

AccessKeyID: aws.ToString(credentials.AccessKeyId),

SecretAccessKey: aws.ToString(credentials.SecretAccessKey),

SessionToken: aws.ToString(credentials.SessionToken),

Source: credentialSource,

CanExpire: true,

Expires: aws.ToTime(credentials.Expiration).UTC(),

Good catch — added a nil check for assumeRoleOutput.Credentials and switched to aws.ToString/aws.ToTime for safe pointer dereferencing.

Use aws.ToString and aws.ToTime for safe pointer dereferencing to prevent potential panics if the STS response has nil fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Copilot · 2026-04-07T21:37:17Z

pkg/ucp/aws/ucpcredentialprovider.go

-		value.Expires = time.Now().UTC().Add(c.options.Duration)
+
+		if assumeRoleOutput.Credentials == nil {
+			return aws.Credentials{}, fmt.Errorf("AssumeRole returned nil credentials")


Error strings should be lowercase (Go convention). Consider changing fmt.Errorf("AssumeRole returned nil credentials") to start with a lowercase letter (and keep the rest consistent with other errors in this file).

Suggested change

return aws.Credentials{}, fmt.Errorf("AssumeRole returned nil credentials")

return aws.Credentials{}, fmt.Errorf("assumeRole returned nil credentials")

Copilot AI review requested due to automatic review settings April 7, 2026 17:02

willdavsmith requested review from a team as code owners April 7, 2026 17:02

willdavsmith requested a deployment to external-contributor-approval April 7, 2026 17:02 — with GitHub Actions Waiting

Copilot started reviewing on behalf of willdavsmith April 7, 2026 17:03 View session

willdavsmith mentioned this pull request Apr 7, 2026

Re-enable and fix Test_AWSRedeployWithUpdatedResourceUpdatesResource #11538

Closed

12 tasks

Copilot AI reviewed Apr 7, 2026

View reviewed changes

willdavsmith force-pushed the improve-aws-irsa-credentials branch from 1e198f2 to c7301e3 Compare April 7, 2026 20:50

willdavsmith requested a deployment to external-contributor-approval April 7, 2026 20:50 — with GitHub Actions Waiting

willdavsmith requested a review from Copilot April 7, 2026 20:52

Copilot started reviewing on behalf of willdavsmith April 7, 2026 20:52 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

willdavsmith requested a deployment to external-contributor-approval April 7, 2026 21:11 — with GitHub Actions Waiting

willdavsmith requested a review from Copilot April 7, 2026 21:12

Copilot started reviewing on behalf of willdavsmith April 7, 2026 21:13 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Add nil check for AssumeRole response credentials

478a568

Use aws.ToString and aws.ToTime for safe pointer dereferencing to prevent potential panics if the STS response has nil fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

willdavsmith requested a deployment to external-contributor-approval April 7, 2026 21:30 — with GitHub Actions Waiting

willdavsmith requested a review from Copilot April 7, 2026 21:33

Copilot started reviewing on behalf of willdavsmith April 7, 2026 21:34 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

kachawla approved these changes Apr 7, 2026

View reviewed changes

	uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0
	uses: actions/setup-go@v6.4.0 # v6.4.0

	- uses: LouisBrunner/checks-action@dfcbcf801bff1ea7f1414824fc28f2cd697b35da # v3.0.0
	- uses: LouisBrunner/checks-action@v3.1.0



	# Intentionally pinned to v3.0.0 for workflow compatibility; do not upgrade back to v3.1.0 until that issue is resolved.

	if c.options.Duration > 0 {
	if c.options.Duration >= 15*time.Minute && c.options.Duration <= time.Hour {

	// If empty, defaults to "us-east-1" (global endpoint).
	// If empty, defaults to "us-east-1".

	return aws.Credentials{}, fmt.Errorf("AssumeRole returned nil credentials")
	return aws.Credentials{}, fmt.Errorf("assumeRole returned nil credentials")

Conversation

willdavsmith commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

Role chaining for IRSA credentials (pkg/ucp/aws/ucpcredentialprovider.go)

Regional STS endpoint support

Code quality improvements

IAM Role Requirement

Contributor checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests

Uh oh!

codecov bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

willdavsmith commented Apr 7, 2026 •

edited

Loading

Role chaining for IRSA credentials (`pkg/ucp/aws/ucpcredentialprovider.go`)

github-actions bot commented Apr 7, 2026 •

edited

Loading

codecov bot commented Apr 7, 2026 •

edited

Loading