fix: Skip landing page registration on NewLandingPage error

[carterpewpew]

What this PR does / why we need it:

When web.NewLandingPage() returns an error, the handler value is nil and the API is telling us not to use it. The previous code logged the error but still called mux.Handle("/", landingPage), registering a nil *web.LandingPageHandler. Because a nil pointer stored in an http.Handler interface is non-nil in Go, the default mux accepts it silently. The first GET / then panics when ServeHTTP dereferences the nil receiver (e.g. reading routePrefix).

This change:

• Adds a newLandingPage function parameter to buildTelemetryServer() and buildMetricsServer() so the landing page factory is injected per call. Production code passes web.NewLandingPage; tests can inject a failing factory without mutable global state or data races.
• Returns the mux after logging, without registering the / handler, when the factory returns an error. The rest of the server (metrics, health, pprof, etc.) continues to work and the process does not crash on the root path if landing page construction fails.
• Adds focused tests for both the happy path (landing page registered, / returns 200) and the error path (landing page skipped, / returns 404, other routes like /metrics and /healthz still work). All tests are safe to run with t.Parallel() and pass go test -race.

How does this change affect the cardinality of KSM: does not change cardinality

Which issue(s) this PR fixes: N/A

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: carterpewpew
Once this PR has been reviewed and has the lgtm label, please assign dgrisonnet for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

This issue is currently awaiting triage.

If kube-state-metrics contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

Welcome @carterpewpew!

It looks like this is your first PR to kubernetes/kube-state-metrics 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kube-state-metrics has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[Copilot]

Pull request overview

This PR prevents kube-state-metrics from registering a nil landing page handler when web.NewLandingPage() returns an error, avoiding a runtime panic on requests to / while keeping other endpoints functional.

Changes:

• Return early (after logging) from buildTelemetryServer() and buildMetricsServer() when landing page creation fails, skipping mux.Handle("/", landingPage).
• Add tests for the landing page success path and a regression-style test illustrating the nil-handler panic scenario.

Reviewed changes

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

File	Description
pkg/app/server.go	Returns the mux without registering / when landing page construction fails.
pkg/app/server_test.go	Adds landing page tests and a regression-style panic test for a nil landing page handler.

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

pkg/app/server_test.go:1063

• Same issue as above: overriding the global newLandingPage can run concurrently with other t.Parallel() tests in this package and lead to data races/flakes. Use synchronization to guard overrides (or refactor to inject the landing page factory per server build) so tests remain safe under parallel execution and go test -race.

func TestBuildMetricsServerLandingPageError(t *testing.T) {
	original := newLandingPage
	t.Cleanup(func() { newLandingPage = original })
	newLandingPage = func(_ web.LandingConfig) (*web.LandingPageHandler, error) {
		return nil, fmt.Errorf("injected landing page error")
	}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.