fix(aws): make CIRoleArn output ARN

[lionello]

Description

CIRoleARN CloudFormation output was not an ARN.

• After merge, we can use defang cloudformation to create the template YAML to put in our public S3 bucket (MVP repo, ecs folder)

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

  • Updated ECS setup and template tests to use the revised setup flow and shared test data.

• Refactor

  • CloudFormation upserts now accept and pass deployment parameters; stack creation path refined.
  • Standardized task-definition output naming and added CI role ARN into ECS config and outputs.
  • OIDC/JWK parsing expanded to include RSA parameters and certificate representations; thumbprint extraction now prefers explicit thumbprints.

• Chore

  • Minor test comment tweak.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

SetUp builds and passes CloudFormation parameters to a parameterized upsertStackAndWait; CloudFormation outputs include a CI role ARN and TaskDef ARN key rename; OIDC JWK struct exposes RSA fields and DER cert bytes and thumbprint extraction now prefers x5t; minor test/comment adjustments.

Changes

Cohort / File(s)	Summary
AWS ECS CloudFormation Setup & Runtime 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 assembles parameters (VPC, RetainBucket, EnablePullThroughCache, DockerHub creds) and calls upsertStackAndWait with variadic cfnTypes.Parameter; upsertStackAndWait signature updated to accept parameters and will create stack when UpdateStack reports non-existent stack. Added CIRoleARN field to AwsEcs.
CloudFormation Outputs & Template 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 output key OutputsTaskDefArn → OutputsTaskDefARN. CI role output now uses Fn::GetAtt [CIRole, Arn]. Template metadata (ParameterGroups/Labels) added; tests/testdata updated accordingly.
OIDC / JWK Handling src/pkg/clouds/aws/ecs/cfn/oidc.go	Jwk struct extended with Kid, N, E, and X5c changed to [][]byte (DER bytes). FetchThumbprints now prefers x5t (base64url decoded → hex); previous X5c-derived thumbprint path left as placeholder.
CLI Compose Tests (minor) src/pkg/cli/compose/context_test.go	Single-line comment addition in Test_getRemoteBuildContext; no behavioral change.

Sequence Diagram(s)

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

    SetUp->>Upserter: Build parameters (VPC, RetainBucket, flags, creds)\nCall upsertStackAndWait(template, parameters...)
    Upserter->>CFN: UpdateStack (with parameters)
    CFN-->>Upserter: APIError (stack not found)
    alt Stack Missing
        Upserter->>CFN: CreateStack (with parameters)
        CFN-->>Upserter: CreateStack success
    else Stack Exists
        CFN-->>Upserter: UpdateStack success
    end
    Upserter->>CFN: DescribeStacks (wait)
    CFN-->>Upserter: Stack outputs
    Upserter->>Mapper: Provide outputs
    Mapper-->>Upserter: Populated OutputsTaskDefARN, OutputsCIRoleARN
    Upserter-->>SetUp: Return success / populated config

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: update OIDC provider parameters and conditions #1775: Overlapping changes to OIDC/JWK handling and introduction of OutputsCIRoleARN in templates and code.

Suggested reviewers

• jordanstephens
• edwardrf

Poem

🐰 Hop, hop — parameters set to fly,
Stacks awaken under cloud-lit sky,
JWKs store modulus, expo, and cert,
Thumbprints favor x5t, tidy and pert,
CI ARNs gleam — the rabbit gives a sigh.

🚥 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
Title check	✅ Passed	The title 'fix(aws): make CIRoleArn output ARN' directly reflects the main change in the pull request—fixing the CIRoleARN CloudFormation output to be a proper ARN instead of a reference.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches

• 📝 Generate docstrings

---

_{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.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/pkg/clouds/aws/ecs/cfn/template.go`:
- Around line 709-717: TemplateRevision needs to be incremented to reflect the
template changes (new Outputs and metadata) so OutputsTemplateVersion is not
stale; locate the constant or variable named TemplateRevision in this file and
bump its numeric value (e.g., from 3 to 4), then ensure any related exported
value OutputsTemplateVersion is derived from that constant so downstream
consumers pick up the new revision; verify no other places rely on the old
number and update tests or comments referencing the previous revision.