fix(aws): make CIRoleArn output ARN

[lionello]

Description

CIRoleARN CloudFormation output was not an ARN.

Checklist

• I have performed a self-review of my code
• I have added appropriate tests
• I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

• Tests

  • Enhanced ECS configuration setup test coverage

• Refactor

  • Improved CloudFormation parameter handling for ECS deployment
  • Updated internal field naming conventions for consistency

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes extend AWS ECS CloudFormation stack management to support parameterized updates, add CI role ARN output handling, and update the OIDC provider JWK structure with additional RSA cryptographic fields while refining thumbprint extraction logic.

Changes

Cohort / File(s)	Summary
AWS ECS CloudFormation Setup and Configuration src/pkg/clouds/aws/ecs/cfn/setup.go, src/pkg/clouds/aws/ecs/cfn/setup_test.go, src/pkg/clouds/aws/ecs/common.go	SetUp now builds CloudFormation parameters (VPC, RetainBucket, EnablePullThroughCache, Docker Hub credentials) and passes them to upsertStackAndWait. Updated upsertStackAndWait to accept variadic parameters and handle non-existent stacks via APIError detection. fillWithOutputs extracts new OutputsCIRoleARN output. Added CIRoleARN field to AwsEcs struct.
AWS ECS CloudFormation Output Definitions src/pkg/clouds/aws/ecs/cfn/outputs.go, src/pkg/clouds/aws/ecs/cfn/template.go, src/pkg/clouds/aws/ecs/cfn/template_test.go, src/pkg/clouds/aws/ecs/cfn/testdata/template.yaml	Renamed OutputsTaskDefArn to OutputsTaskDefARN across output definitions. Changed CI role ARN CloudFormation output from Ref to GetAtt("Arn") to fetch the ARN attribute directly. Updated test template generation to use package-level testContainers slice.
AWS OIDC Provider src/pkg/clouds/aws/ecs/cfn/oidc.go	Extended Jwk struct with Kid, N, E fields (supporting RSA parameters) and changed X5c from []string to [][]byte for DER-encoded certificates. Modified FetchThumbprints to prefer X5t with base64url decoding; X5c-based thumbprint computation is now de-emphasized with a placeholder comment.
CLI Compose Tests src/pkg/cli/compose/context_test.go	Minor comment addition to Test_getRemoteBuildContext suggesting inspection of tmpDir files by path modification.

Sequence Diagram(s)

sequenceDiagram
    participant SetUp
    participant upsertStackAndWait
    participant CFN as CloudFormation API
    participant fillWithOutputs

    SetUp->>SetUp: Build parameters<br/>(VPC, RetainBucket, etc.)
    SetUp->>upsertStackAndWait: Call with parameters
    
    upsertStackAndWait->>CFN: Attempt UpdateStack
    CFN-->>upsertStackAndWait: APIError (stack not found)
    
    alt Stack Exists
        upsertStackAndWait->>CFN: UpdateStack succeeds
    else Stack Missing
        upsertStackAndWait->>CFN: CreateStack
        CFN-->>upsertStackAndWait: CreateStack success
    end
    
    upsertStackAndWait->>CFN: DescribeStacks (wait for completion)
    CFN-->>upsertStackAndWait: Stack outputs
    
    upsertStackAndWait->>fillWithOutputs: Extract outputs
    fillWithOutputs->>fillWithOutputs: Map OutputsTaskDefARN<br/>and OutputsCIRoleARN
    fillWithOutputs-->>upsertStackAndWait: Populated config
    upsertStackAndWait-->>SetUp: Return success

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: update OIDC provider parameters and conditions #1775: Modifies OIDC provider handling and OutputsCIRoleARN output definitions at the code and template level.
• Fix CD refresh/list/down #1804: Updates the AWS ECS CloudFormation setup workflow in setup.go with related parameter handling.

Suggested reviewers

• jordanstephens
• raphaeltm
• edwardrf

Poem

🐰 Hop, hop! Parameters now flow,
CloudFormation stacks learn to grow,
OIDC thumbprints extracted with care,
CI roles ARNed in the crisp morning air! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: changing the CIRoleARN output to return an ARN value instead of a reference.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter"
level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/pkg/clouds/aws/ecs/cfn/setup.go (1)

184-200: Allow Docker Hub credentials to be cleared after being set.

The current implementation uses os.Getenv, which treats unset and explicitly empty environment variables identically. This prevents clearing credentials because omitted parameters fall back to UsePreviousValue in CloudFormation updates. Switch to os.LookupEnv to distinguish between unset (preserve previous) and explicitly empty (clear parameter):

🔧 Proposed fix

- if dockerHubUsername := os.Getenv("DOCKERHUB_USERNAME"); dockerHubUsername != "" {
+ if dockerHubUsername, ok := os.LookupEnv("DOCKERHUB_USERNAME"); ok {
    parameters = append(parameters, cfnTypes.Parameter{
      ParameterKey:   ptr.String(ParamsDockerHubUsername),
      ParameterValue: ptr.String(dockerHubUsername),
    })
  }
- if dockerHubToken := os.Getenv("DOCKERHUB_ACCESS_TOKEN"); dockerHubToken != "" {
+ if dockerHubToken, ok := os.LookupEnv("DOCKERHUB_ACCESS_TOKEN"); ok {
    parameters = append(parameters, cfnTypes.Parameter{
      ParameterKey:   ptr.String(ParamsDockerHubAccessToken),
      ParameterValue: ptr.String(dockerHubToken),
    })
  }

🤖 Fix all issues with AI agents

In `@src/pkg/clouds/aws/ecs/cfn/oidc.go`:
- Line 19: The X5c field currently typed as [][]byte will fail to unmarshal JWK
x5c entries (they are JSON strings containing standard base64), so change the
X5c field on the OIDC JWK struct to []string (or []json.RawMessage) and when you
need the DER bytes decode them with base64.StdEncoding.DecodeString (e.g. in the
codepath that inspects key.X5c). Also add the encoding/base64 import and handle
decode errors where you reference X5c so usages of X5c (the commented else-if
branch or any function that reads key.X5c) work correctly.