-
Notifications
You must be signed in to change notification settings - Fork 2.4k
config: Adding support to read configuration from http endpoint #5405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
da7d3ec
decfed0
8ccccaf
ca95692
bb88443
6cfd06d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -248,11 +248,25 @@ func (a *App) setup() error { | |
|
|
||
| stopc := make(chan struct{}) | ||
| var wg sync.WaitGroup | ||
| var loader config.ConfigLoader | ||
| if opts.ConfigHTTPURL != "" { | ||
| loader = config.NewHTTPLoader(opts.ConfigHTTPURL) | ||
| // Sanitize URL for logging to avoid credential leakage | ||
| sanitizedURL := config.SanitizeURL(opts.ConfigHTTPURL) | ||
| logger.Info("Starting Alertmanager in HTTP configuration mode", "source", sanitizedURL) | ||
| } else { | ||
| loader = config.NewFileLoader(opts.ConfigFile) | ||
| logger.Info("Starting Alertmanager in file configuration mode", "source", opts.ConfigFile) | ||
| } | ||
|
|
||
| // Load config once for both event recorder initialization and the | ||
| // first coordinator apply. Subsequent reloads go through | ||
| // configCoordinator.Reload() which reads the file again. | ||
| initialConf, err := config.LoadFile(opts.ConfigFile) | ||
| data, err := loader.Load(context.Background()) | ||
| if err != nil { | ||
| return fmt.Errorf("error loading configuration: %w", err) | ||
| } | ||
| initialConf, err := config.Load(string(data)) | ||
| if err != nil { | ||
| return fmt.Errorf("error loading configuration file: %w", err) | ||
| } | ||
|
|
@@ -457,7 +471,13 @@ func (a *App) setup() error { | |
| }) | ||
|
|
||
| configLogger := logger.With("component", "configuration") | ||
| if opts.ConfigHTTPURL != "" { | ||
| loader = config.NewHTTPLoader(opts.ConfigHTTPURL) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this need to be reconstructed? It seems like we could use the |
||
| } else { | ||
| loader = config.NewFileLoader(opts.ConfigFile) | ||
| } | ||
| configCoordinator := config.NewCoordinator( | ||
| loader, | ||
| opts.ConfigFile, | ||
| reg, | ||
| configLogger, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| // Copyright 2024 Prometheus Team | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make: *** [Makefile.common:146: common-check_license] Error 1 please use the license header without date. eg https://github.com/prometheus/alertmanager/blob/main/config/config.go#L1-L12
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you @TheMeier for suggestions, will do the changes |
||
| // 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 app | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/prometheus/client_golang/prometheus" | ||
| "github.com/prometheus/common/promslog" | ||
| "github.com/prometheus/exporter-toolkit/web" | ||
|
|
||
| "github.com/prometheus/alertmanager/featurecontrol" | ||
| ) | ||
|
|
||
| func TestStartupWithHTTPConfig(t *testing.T) { | ||
| // Start a HTTP server that returns a minimal valid config. | ||
| srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusOK) | ||
| w.Write([]byte("route:\n receiver: test\nreceivers:\n- name: test")) | ||
| })) | ||
| defer srv.Close() | ||
|
|
||
| // Create a temporary data directory. | ||
| dir := t.TempDir() | ||
|
|
||
| // Build minimal options for HTTP source. | ||
| webCfg := web.FlagConfig{} | ||
| webCfg.WebListenAddresses = &[]string{"127.0.0.1:0"} | ||
| webCfgFile := "" | ||
| webCfg.WebConfigFile = &webCfgFile | ||
|
|
||
| ff, err := featurecontrol.NewFlags(promslog.NewNopLogger(), "") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| opts := Options{ | ||
| ConfigHTTPURL: srv.URL, | ||
| DataDir: dir, | ||
| Retention: DefaultRetention, | ||
| MaintenanceInterval: DefaultMaintenanceInterval, | ||
| AlertGCInterval: DefaultAlertGCInterval, | ||
| DispatchMaintenanceInterval: DefaultDispatchMaintenanceInterval, | ||
| WebConfig: &webCfg, | ||
| Logger: promslog.NewNopLogger(), | ||
| Registerer: prometheus.NewRegistry(), | ||
| Flagger: ff, | ||
| } | ||
|
|
||
| // Try to create the app (setup). | ||
| app, err := New(opts) | ||
| if err != nil { | ||
| t.Fatalf("failed to create app with HTTP config: %v", err) | ||
| } | ||
| defer func() { _ = app.Stop(context.Background()) }() | ||
|
|
||
| // Start the app. | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| defer cancel() | ||
| go func() { _ = app.Start() }() | ||
|
|
||
| // Verify it started without error. | ||
| select { | ||
| case <-ctx.Done(): | ||
| t.Fatal("app stopped unexpectedly") | ||
| default: | ||
| } | ||
| } | ||
|
|
||
| func TestStartupWithFileConfig(t *testing.T) { | ||
| // Create a temporary config file. | ||
| dir := t.TempDir() | ||
| configPath := filepath.Join(dir, "alertmanager.yml") | ||
| data := []byte("route:\n receiver: test\nreceivers:\n- name: test") | ||
| if err := os.WriteFile(configPath, data, 0o600); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| webCfg := web.FlagConfig{} | ||
| webCfg.WebListenAddresses = &[]string{"127.0.0.1:0"} | ||
| webCfgFile := "" | ||
| webCfg.WebConfigFile = &webCfgFile | ||
|
|
||
| ff, err := featurecontrol.NewFlags(promslog.NewNopLogger(), "") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| opts := Options{ | ||
| ConfigFile: configPath, | ||
| DataDir: dir, | ||
| Retention: DefaultRetention, | ||
| MaintenanceInterval: DefaultMaintenanceInterval, | ||
| AlertGCInterval: DefaultAlertGCInterval, | ||
| DispatchMaintenanceInterval: DefaultDispatchMaintenanceInterval, | ||
| WebConfig: &webCfg, | ||
| Logger: promslog.NewNopLogger(), | ||
| Registerer: prometheus.NewRegistry(), | ||
| Flagger: ff, | ||
| } | ||
|
|
||
| app, err := New(opts) | ||
| if err != nil { | ||
| t.Fatalf("failed to create app with file config: %v", err) | ||
| } | ||
| defer func() { _ = app.Stop(context.Background()) }() | ||
|
|
||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| defer cancel() | ||
| go func() { _ = app.Start() }() | ||
|
|
||
| select { | ||
| case <-ctx.Done(): | ||
| t.Fatal("app stopped unexpectedly") | ||
| default: | ||
| } | ||
| } | ||
|
|
||
| func TestStartupWithBothSources(t *testing.T) { | ||
| // Create a temporary config file. | ||
| dir := t.TempDir() | ||
| configPath := filepath.Join(dir, "alertmanager.yml") | ||
| data := []byte("route:\n receiver: test\nreceivers:\n- name: test") | ||
| if err := os.WriteFile(configPath, data, 0o600); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| webCfg := web.FlagConfig{} | ||
| webCfg.WebListenAddresses = &[]string{"127.0.0.1:0"} | ||
| webCfgFile := "" | ||
| webCfg.WebConfigFile = &webCfgFile | ||
| ff, _ := featurecontrol.NewFlags(promslog.NewNopLogger(), "") | ||
| opts := Options{ | ||
| ConfigFile: configPath, | ||
| ConfigHTTPURL: "http://example.com/config", | ||
| DataDir: dir, | ||
| Retention: DefaultRetention, | ||
| MaintenanceInterval: DefaultMaintenanceInterval, | ||
| AlertGCInterval: DefaultAlertGCInterval, | ||
| DispatchMaintenanceInterval: DefaultDispatchMaintenanceInterval, | ||
| WebConfig: &webCfg, | ||
| Logger: promslog.NewNopLogger(), | ||
| Registerer: prometheus.NewRegistry(), | ||
| Flagger: ff, | ||
| } | ||
|
|
||
| _, err := New(opts) | ||
| if err == nil { | ||
| t.Fatal("expected error when both config sources are set") | ||
| } | ||
| if err.Error() != "alertmanager/app: Options.ConfigFile and Options.ConfigHTTPURL are mutually exclusive" { | ||
| t.Fatalf("unexpected error message: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestStartupWithNeitherSource(t *testing.T) { | ||
| webCfg := web.FlagConfig{} | ||
| webCfg.WebListenAddresses = &[]string{"127.0.0.1:0"} | ||
| webCfgFile := "" | ||
| webCfg.WebConfigFile = &webCfgFile | ||
| ff, _ := featurecontrol.NewFlags(promslog.NewNopLogger(), "") | ||
| opts := Options{ | ||
| DataDir: t.TempDir(), | ||
| Retention: DefaultRetention, | ||
| MaintenanceInterval: DefaultMaintenanceInterval, | ||
| AlertGCInterval: DefaultAlertGCInterval, | ||
| DispatchMaintenanceInterval: DefaultDispatchMaintenanceInterval, | ||
| WebConfig: &webCfg, | ||
| Logger: promslog.NewNopLogger(), | ||
| Registerer: prometheus.NewRegistry(), | ||
| Flagger: ff, | ||
| } | ||
|
|
||
| _, err := New(opts) | ||
| if err == nil { | ||
| t.Fatal("expected error when no config source is set") | ||
| } | ||
| if err.Error() != "alertmanager/app: exactly one of Options.ConfigFile or Options.ConfigHTTPURL must be set" { | ||
| t.Fatalf("unexpected error message: %v", err) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,7 +48,8 @@ func run() int { | |
| } | ||
|
|
||
| var ( | ||
| configFile = kingpin.Flag("config.file", "Alertmanager configuration file name.").Default("alertmanager.yml").String() | ||
| configFile = kingpin.Flag("config.file", "Alertmanager configuration file name.").String() | ||
| configHTTPURL = kingpin.Flag("config.http-url", "Alertmanager configuration URL (mutually exclusive with --config.file).").String() | ||
| dataDir = kingpin.Flag("storage.path", "Base path for data storage.").Default("data/").String() | ||
| retention = kingpin.Flag("data.retention", "How long to keep data for.").Default("120h").Duration() | ||
| maintenanceInterval = kingpin.Flag("data.maintenance-interval", "Interval between garbage collection and snapshotting to disk of the silences and the notification logs.").Default("15m").Duration() | ||
|
|
@@ -97,6 +98,14 @@ func run() int { | |
| kingpin.CommandLine.GetFlag("help").Short('h') | ||
| kingpin.Parse() | ||
|
|
||
| // Validate exactly one configuration source is provided. | ||
| if *configFile == "" && *configHTTPURL == "" { | ||
| kingpin.Fatalf("Need to configure one of the following --config.file or --config.http-url") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is a behavior regression - there may be production users of Alertmanager who depend on the default value of the |
||
| } | ||
| if *configFile != "" && *configHTTPURL != "" { | ||
| kingpin.Fatalf("Need to configure only one of the following --config.file or --config.http-url") | ||
| } | ||
|
|
||
| logger := promslog.New(&promslogConfig) | ||
|
|
||
| prometheus.MustRegister(versioncollector.NewCollector("alertmanager")) | ||
|
|
@@ -153,6 +162,7 @@ func run() int { | |
|
|
||
| opts := app.Options{ | ||
| ConfigFile: *configFile, | ||
| ConfigHTTPURL: *configHTTPURL, | ||
| DataDir: *dataDir, | ||
| Retention: *retention, | ||
| MaintenanceInterval: *maintenanceInterval, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the repository’s command-block style.
The added fenced block triggers MD040, MD046, and MD014. Use the existing indented-command style and omit the
$prompt.Suggested change
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 49-49: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 49-49: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 50-50: Dollar signs used before commands without showing output
(MD014, commands-show-output)
🤖 Prompt for AI Agents
Source: Linters/SAST tools