diff --git a/web/serve_flags_test.go b/web/serve_flags_test.go new file mode 100644 index 00000000..c9a85cc2 --- /dev/null +++ b/web/serve_flags_test.go @@ -0,0 +1,73 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package web + +import ( + "errors" + "net" + "net/http" + "testing" +) + +func testListener(t *testing.T) net.Listener { + t.Helper() + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("Unable to listen: %v", err) + } + t.Cleanup(func() { l.Close() }) + return l +} + +// TestServeRejectsIncompleteFlagConfig checks that Serve and ServeMultiple +// report an incomplete FlagConfig instead of dereferencing a nil field. They +// are exported for callers that create their own listeners, and those callers +// build a FlagConfig by hand rather than getting one from kingpinflag. +func TestServeRejectsIncompleteFlagConfig(t *testing.T) { + for _, tc := range []struct { + name string + flags *FlagConfig + }{ + {name: "nil FlagConfig", flags: nil}, + {name: "no WebConfigFile", flags: &FlagConfig{}}, + {name: "no WebConfigFile with listen addresses", flags: &FlagConfig{ + WebListenAddresses: &[]string{"localhost:0"}, + WebSystemdSocket: new(false), + }}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Run("Serve", func(t *testing.T) { + err := Serve(testListener(t), &http.Server{}, tc.flags, testlogger) + if !errors.Is(err, ErrMissingFlag) { + t.Errorf("Serve() = %v, expected %v", err, ErrMissingFlag) + } + }) + t.Run("ServeMultiple", func(t *testing.T) { + err := ServeMultiple([]net.Listener{testListener(t)}, &http.Server{}, tc.flags, testlogger) + if !errors.Is(err, ErrMissingFlag) { + t.Errorf("ServeMultiple() = %v, expected %v", err, ErrMissingFlag) + } + }) + }) + } +} + +// TestServeDoesNotRequireListenAddresses checks that Serve accepts a +// FlagConfig without listener fields. The caller passes the listener in, so +// requiring them would reject a legitimate configuration. +func TestServeDoesNotRequireListenAddresses(t *testing.T) { + if err := (&FlagConfig{WebConfigFile: new("")}).checkWebConfigFlag(); err != nil { + t.Errorf("checkWebConfigFlag() = %v, expected nil", err) + } +} diff --git a/web/tls_config.go b/web/tls_config.go index 022a839e..cd0e6def 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -81,14 +81,21 @@ type FlagConfig struct { WebConfigFile *string } +// checkWebConfigFlag validates that the flag configuration contains the web +// config field. Serve and ServeMultiple only need this much, because their +// caller supplies the listeners. +func (c *FlagConfig) checkWebConfigFlag() error { + if c == nil || c.WebConfigFile == nil { + return ErrMissingFlag + } + return nil +} + // checkFlags validates that the flag configuration contains the required // listener and web config fields needed by the web package. func (c *FlagConfig) checkFlags() error { - if c == nil { - return ErrMissingFlag - } - if c.WebConfigFile == nil { - return ErrMissingFlag + if err := c.checkWebConfigFlag(); err != nil { + return err } // Listen addresses are only optional when systemd socket activation is // actually enabled. Checking that WebSystemdSocket is non-nil is not @@ -413,6 +420,9 @@ func Serve(l net.Listener, server *http.Server, flags *FlagConfig, logger *slog. } func configureServer(server *http.Server, flags *FlagConfig, logger *slog.Logger) (*HTTPConfig, error) { + if err := flags.checkWebConfigFlag(); err != nil { + return nil, err + } setErrorLog(server, logger) tlsConfigPath := *flags.WebConfigFile if tlsConfigPath == "" {