Report Connector Status when host and selector are both missing

[evanwang9x]

Closes #2512

Summary by CodeRabbit

• Bug Fixes

  • Connector status now reflects all relevant validation issues, including missing host/selector configuration and TLS or routing problems.
  • Connectors without a host or selector are now marked unconfigured with a clearer status message.

• Tests

  • Expanded coverage for connector configuration status, including the new missing host/selector case.
  • Improved validation of status messages to ensure users see accurate feedback.

[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: 17a6f539-a520-4e81-bb29-e10938c2790c

📥 Commits

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

📒 Files selected for processing (2)

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

---

📝 Walkthrough

Walkthrough

Site.CheckConnector now validates that a connector's spec defines either host or selector, recording a spec error otherwise, and joins this error with existing TLS and router errors when updating the Configured status. Corresponding test cases and assertions were added/updated to verify this behavior.

Changes

Connector Spec Validation

Layer / File(s)	Summary
Spec validation and error propagation internal/kube/site/site.go	Adds specErr, validates that connector.Spec.Host or connector.Spec.Selector is set, and joins tlsErr, specErr, and routerErr when updating the connector's Configured status.
Test coverage for spec validation internal/kube/site/site_test.go	Adds wantConfigured and wantErrMessage test fields, a new "missing host and selector" test case, and assertions checking the Configured condition status and exact message.

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

Possibly related issues

• Add unit tests for Connector Configured status when TLS secret is missing #2477: Both changes touch internal/kube/site/site.go's CheckConnector/TestSite_CheckConnector and the Configured status logic for connector validation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: reporting connector status when host and selector are both missing.
Linked Issues check	✅ Passed	The changes validate missing spec.host/spec.selector in CheckConnector and set Configured false with a clear message, matching #2512.
Out of Scope Changes check	✅ Passed	The PR stays focused on connector validation and status/test updates, with no clear unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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

[c-kruse]

Looks good! Just a nit pick on the wording.

[c-kruse]

Suggest including "non-empty" as a qualifier here: Connector must define a non-empty spec.host or spec.selector. If anyone ever sees this message, they have already defined one of these fields.

As I'm sure you noticed when testing, the kubernetes API refuses an invalid Connector that does not satisfy a oneOf required selector or host.

$ cat << EOF | kubectl apply -f -
apiVersion: skupper.io/v2alpha1
kind: Connector
metadata:
  name: invalid
spec:
  port: 8888
  routingKey: test

EOF

The Connector "invalid" is invalid:
* <nil>: Invalid value: "": "spec" must validate one and only one schema (oneOf). Found none valid
* spec.selector: Required value

$ cat << EOF | kubectl apply -f -
apiVersion: skupper.io/v2alpha1
kind: Connector
metadata:
  name: invalid
spec:
  port: 8888
  routingKey: test
  host: ""
  selector: ""
EOF
The Connector "invalid" is invalid: <nil>: Invalid value: "": "spec" must validate one and only one schema (oneOf). Found 2 valid alternatives

To "trick" it you need to provide either host or selector with an empty string (really was a miss on our api spec side.) Because of how our go types were set up, we end up missing that distinction and can't easily plainly say "Invalid value for host|selector. Must not be empty".

[evanwang9x]

I see thanks for letting me know, I'll implement that change later today👍.