Show link-access SecuredAccess failures instead of only logging them

[evanwang9x]

Closes #2513

Summary by CodeRabbit

• Bug Fixes
  • Site reconciliation now stops and reports errors when secured access initialization fails, instead of silently continuing.
  • Partial secured-access setup failures are now collected and returned as a single combined error for clearer visibility.
  • If secured access cannot be established during startup, the site remains unconfigured and reflects the error state.
• Tests
  • Added unit tests covering secured-access error aggregation and initialization failure behavior.

[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: dca3f6ba-29bd-442c-bd93-9e55055cc6dd

📥 Commits

Reviewing files that changed from the base of the PR and between 73fed56 and 2b044eb.

📒 Files selected for processing (2)

• internal/kube/site/site.go
• internal/kube/site/site_test.go

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

• internal/kube/site/site.go
• internal/kube/site/site_test.go

---

📝 Walkthrough

Walkthrough

checkSecuredAccess now aggregates Ensure() failures and returns a combined error, while reconcile now propagates that error into site status handling. New tests cover failure reporting, access mapping updates, and initialization error handling.

Changes

SecuredAccess error propagation

Layer / File(s)	Summary
Aggregate and return SecuredAccess errors internal/kube/site/site.go	checkSecuredAccess now collects Ensure failures, updates accessMapping only on success, returns a combined error, and reconcile forwards that error instead of ignoring it.
Unit tests for error propagation internal/kube/site/site_test.go	New tests verify error return on Ensure failure, empty accessMapping on failure, and an unconfigured site after initialization-time failure.

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

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: surfacing SecuredAccess failures instead of only logging them.
Linked Issues check	✅ Passed	The changes surface checkSecuredAccess failures through reconcile so updateConfigured(err) can reflect the error in Site status, matching #2513.
Out of Scope Changes check	✅ Passed	The code and tests are tightly scoped to SecuredAccess failure handling and status reporting, with no unrelated changes evident.

---

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

[Copilot]

Pull request overview

This PR aims to ensure link-access SecuredAccess ensure failures are surfaced via Site configured status (instead of only being logged), addressing issue #2513 so a Site can’t appear healthy while link access is broken.

Changes:

• In Site.reconcile() initialization, attempts to reflect checkSecuredAccess() failures in configured status.
• Updates checkSecuredAccess() to aggregate SecuredAccessFactory.Ensure(...) failures and return an error.
• Adds unit tests asserting checkSecuredAccess() returns errors and that configured status can be set to not-configured on failure.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
internal/kube/site/site.go	Propagates SecuredAccess ensure failures upward (now returning an error from checkSecuredAccess) and attempts to reflect initialization failures in site status.
internal/kube/site/site_test.go	Adds tests for checkSecuredAccess() error behavior and configured-status reporting on failure.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[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)

internal/kube/site/site.go (1)

227-233: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Return the checkSecuredAccess() error from reconcile()

Reconcile() already applies updateConfigured(err) to the returned error, so this branch should return the checkSecuredAccess() failure instead of swallowing it; otherwise the failure is lost and the later success update can overwrite the status.

🧹 Nitpick comments (1)

internal/kube/site/site.go (1)

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

Use errors.Join for the aggregated SecuredAccess errors. This repo targets Go 1.25.0, so the current comma-joined string can be replaced with a joined error to preserve errors.Is/errors.As support.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d701051d-beb5-41f0-8ffe-544337df8093

📥 Commits

Reviewing files that changed from the base of the PR and between e0dd9ce and 73fed56.

📒 Files selected for processing (2)

• internal/kube/site/site.go
• internal/kube/site/site_test.go