diff --git a/errors/errors.go b/errors/errors.go index 07a4e875..398c17cb 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -212,4 +212,7 @@ var ( ErrCertificateWatcherAlreadyRunning = errors.New("certificate watcher is already running") ErrInvalidEndSessionEndpoint = errors.New("end_session_endpoint must be an absolute http(s) URL") ErrPolicyConditionNotCompiled = errors.New("policy condition not compiled") + ErrDisallowedMetricsPath = errors.New("provided metrics path is disallowed") + ErrInvalidMetricsPathPrefix = errors.New("metrics path must start with /") + ErrInvalidMetricsPath = errors.New("invalid metrics path") ) diff --git a/pkg/cli/server/root.go b/pkg/cli/server/root.go index f31f1ada..53f089de 100644 --- a/pkg/cli/server/root.go +++ b/pkg/cli/server/root.go @@ -536,6 +536,32 @@ func validateRemoteSessionStoreConfig(cfg *config.Config, logger zlog.Logger) er return nil } +func validateMetricsConfig(cfg *extconf.ExtensionConfig) error { + metricsCfg := cfg.GetMetricsPrometheusConfig() + if metricsCfg == nil { + return nil + } + + cleanedPath := path.Clean(metricsCfg.Path) + // The path when cleaned should be exactly the same as in config + // to avoid invalid paths from being used for metrics. + if metricsCfg.Path != cleanedPath { + return zerr.ErrInvalidMetricsPath + } + + if !strings.HasPrefix(metricsCfg.Path, "/") { + return zerr.ErrInvalidMetricsPathPrefix + } + + disallowedMetricsPaths := []string{"/", "/v2"} + + if slices.Contains(disallowedMetricsPaths, metricsCfg.Path) { + return zerr.ErrDisallowedMetricsPath + } + + return nil +} + func validateExtensionsConfig(cfg *config.Config, logger zlog.Logger) error { extensionsConfig := cfg.CopyExtensionsConfig() if extensionsConfig != nil && extensionsConfig.Mgmt != nil { @@ -547,6 +573,15 @@ func validateExtensionsConfig(cfg *config.Config, logger zlog.Logger) error { "are now configurable in the HTTP settings.") } + if extensionsConfig != nil { + if metricsValErr := validateMetricsConfig(extensionsConfig); metricsValErr != nil { + joinedErr := errors.Join(zerr.ErrBadConfig, metricsValErr) + logger.Error().Err(joinedErr).Msg("invalid metrics config") + + return joinedErr + } + } + if extensionsConfig.IsUIEnabled() { // it would make sense to also check for mgmt and user prefs to be enabled, // but those are both enabled by having the search and ui extensions enabled diff --git a/pkg/cli/server/root_test.go b/pkg/cli/server/root_test.go index a4d4439d..f1310b06 100644 --- a/pkg/cli/server/root_test.go +++ b/pkg/cli/server/root_test.go @@ -3446,3 +3446,216 @@ func TestBearerASMConfigValidation(t *testing.T) { }) }) } + +func TestMetricsConfigurationValidation(t *testing.T) { + Convey("Test metrics config", t, func() { + Convey("Allow no metrics config", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": {} + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldBeNil) + }) + + Convey("Allow empty metrics config", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": {} + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldBeNil) + }) + + Convey("Allow only metrics enabled", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldBeNil) + }) + }) + + Convey("Test metrics path validation", t, func() { + Convey("Reject / as metrics path", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "/" + } + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrBadConfig) + So(err, ShouldWrap, zerr.ErrDisallowedMetricsPath) + }) + + Convey("Reject /v2 as metrics path", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "/v2" + } + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrBadConfig) + So(err, ShouldWrap, zerr.ErrDisallowedMetricsPath) + }) + + Convey("Reject /v2/ as metrics path", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "/v2/" + } + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrBadConfig) + So(err, ShouldWrap, zerr.ErrInvalidMetricsPath) + }) + + Convey("Reject /abcd/.. as metrics path", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "/abcd/.." + } + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrBadConfig) + So(err, ShouldWrap, zerr.ErrInvalidMetricsPath) + }) + + Convey("Reject abcd as metrics path", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "abcd" + } + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrBadConfig) + So(err, ShouldWrap, zerr.ErrInvalidMetricsPathPrefix) + }) + + Convey("Reject blank metrics path", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "" + } + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrBadConfig) + So(err, ShouldWrap, zerr.ErrInvalidMetricsPath) + }) + + Convey("Allow valid metrics path", func() { + content := `{ + "storage": {"rootDirectory": "/tmp/zot"}, + "http": { + "address": "127.0.0.1", "port": "8080" + }, + "extensions": { + "metrics": { + "enable": true, + "prometheus": { + "path": "/abcd" + } + } + } + }` + cfg := config.New() + tmpfile := MakeTempFileWithContent(t, "zot-test.json", content) + err := cli.LoadConfiguration(cfg, tmpfile) + So(err, ShouldBeNil) + }) + }) +}