fix: allow AWS default credential chain for S3 storage

[davideugui1]

Description

Removes the hard requirement for an explicit S3 access key ID and secret access key in createS3ClientConfig() (cdn-server/src/utils.ts and controlplane/src/core/util.ts), so that deployments without static credentials configured can fall back to the AWS SDK's default credential provider chain — most notably IRSA (IAM Roles for Service Accounts) on EKS, but also plain environment variables and EC2/ECS instance metadata.

Closes #2159
Related to #2772 - Fix 1 scope
Related to #2769, which was closed

Why

Several users run the router/controlplane/cdn-server on EKS with IRSA and don't want to manage long-lived static S3 credentials. Because createS3ClientConfig() always set an explicit credentials object (and threw if access key/secret were blank) the AWS SDK never got the chance to use its default provider chain, even when the pod's service account already had a valid IAM role attached.

What changed

• cdn-server/src/utils.ts / controlplane/src/core/util.ts: createS3ClientConfig() now builds the config object and only attaches credentials when both accessKeyId and secretAccessKey are non-empty. When neither is provided, the field is omitted entirely, letting the AWS SDK's fromNodeProviderChain resolve credentials on its own (IRSA, env vars, instance metadata, etc). The existing check that region is required is unchanged.
• controlplane/test/utils.s3storage.test.ts: added coverage for the new behavior — omitting credentials when username/password are empty strings, omitting credentials when they're absent entirely from both the URL and opts, and confirming the region-required check still throws even without explicit credentials.

Summary by CodeRabbit

• Bug Fixes
  • S3 storage configuration now works in environments without explicitly provided credentials by relying on available runtime credentials.
  • Missing or incomplete access credentials no longer prevent S3 configuration from being created.
  • Missing S3 regions continue to produce a clear validation error.
  • S3 configuration behavior is now consistent across supported services.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b6b9a55-7d74-4c3e-9cf3-4f2487a80547

📥 Commits

Reviewing files that changed from the base of the PR and between 2f50ab8 and 36da5e8.

📒 Files selected for processing (3)

• cdn-server/src/utils.ts
• controlplane/src/core/util.ts
• controlplane/test/utils.s3storage.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• cdn-server/src/utils.ts
• controlplane/test/utils.s3storage.test.ts
• controlplane/src/core/util.ts

---

Walkthrough

Changes

S3 credential resolution

Layer / File(s)	Summary
S3 configuration credential fallback cdn-server/src/utils.ts, controlplane/src/core/util.ts	S3 configuration requires a region, applies static credentials only when both values are provided, and otherwise omits explicit credentials.
Credential fallback validation controlplane/test/utils.s3storage.test.ts	Tests cover empty credentials, absent credentials, and missing-region validation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• #2772: The changes implement the default AWS credential-chain fallback by omitting explicit S3 credentials in the CDN server and control plane.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The controlplane change is not explicitly requested by [#2159], which is limited to CDN server S3 authentication.	Confirm that the controlplane change is intentionally in scope, or move it to a separate linked issue or pull request.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: enabling AWS default credentials for S3 storage.
Linked Issues check	✅ Passed	The changes satisfy [#2159] by making credentials optional, preserving region validation, and enabling the AWS default credential chain.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

controlplane/test/utils.s3storage.test.ts (1)

157-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the complete optional-credentials contract.

The tests cover both credentials being absent, but not the case where only one credential is supplied. Add username-only and password-only cases asserting that credentials remains omitted, plus equivalent regression coverage for the separate CDN implementation.

• controlplane/test/utils.s3storage.test.ts#L157-L204: add partial-credential cases.
• cdn-server/src/utils.ts#L31-L43: add CDN-side tests for omitted credentials and continued region validation.

🤖 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 `@controlplane/test/utils.s3storage.test.ts` around lines 157 - 204, Add
username-only and password-only test cases in
controlplane/test/utils.s3storage.test.ts within the createS3ClientConfig
credential coverage, asserting credentials is omitted; in
cdn-server/src/utils.ts, add equivalent tests for the CDN implementation
covering omitted credentials and continued missing-region validation. Preserve
the existing behavior and assertions for fully absent credentials.

🤖 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.

Nitpick comments:
In `@controlplane/test/utils.s3storage.test.ts`:
- Around line 157-204: Add username-only and password-only test cases in
controlplane/test/utils.s3storage.test.ts within the createS3ClientConfig
credential coverage, asserting credentials is omitted; in
cdn-server/src/utils.ts, add equivalent tests for the CDN implementation
covering omitted credentials and continued missing-region validation. Preserve
the existing behavior and assertions for fully absent credentials.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 15c29707-d9ce-44e0-ba5a-a3ab93d88a60

📥 Commits

Reviewing files that changed from the base of the PR and between 92df9e1 and 63e75e1.

📒 Files selected for processing (3)

• cdn-server/src/utils.ts
• controlplane/src/core/util.ts
• controlplane/test/utils.s3storage.test.ts

[davideugui1]

The Controlplane CI / build_test failure is unrelated to this change. It's a pre-existing CI issue that affects PRs from forks. The workflow's redis/postgres service containers reference secrets.DOCKER_USERNAME/DOCKER_PASSWORD for Docker Hub auth, but GitHub Actions doesn't pass secrets to pull_request runs originating from a fork, so those resolve to '' and fail workflow-template validation (Unexpected value ''). CDN CI passes on the same PR since it has no such service-container credentials block. Happy to help if there's a preferred workaround, but this looks like it needs a maintainer-side fix to the workflow itself.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.