From 673c079821d8605ff128bde8411658f7b142a832 Mon Sep 17 00:00:00 2001 From: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> Date: Tue, 12 Mar 2024 10:02:52 +0100 Subject: [PATCH] refactor: return http loader cfg by value --- pkg/config/http.go | 50 ++++++++++++++++++++--------------------- pkg/config/http_test.go | 18 +++++++-------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/pkg/config/http.go b/pkg/config/http.go index a43b4592..aeaffbd8 100644 --- a/pkg/config/http.go +++ b/pkg/config/http.go @@ -20,6 +20,7 @@ package config import ( "context" + "errors" "fmt" "io" "net/http" @@ -52,37 +53,36 @@ func NewHttpLoader(cfg *Config, cRuntime chan<- runtime.Config) *HttpLoader { // Run gets the runtime configuration from the remote file of the configured http endpoint. // The config will be loaded periodically defined by the loader interval configuration. // If the interval is 0, the configuration is only fetched once and the loader is disabled. -func (hl *HttpLoader) Run(ctx context.Context) error { +func (h *HttpLoader) Run(ctx context.Context) error { ctx, cancel := logger.NewContextWithLogger(ctx) defer cancel() log := logger.FromContext(ctx) - var runtimeCfg *runtime.Config + var cfg runtime.Config getConfigRetry := helper.Retry(func(ctx context.Context) (err error) { - runtimeCfg, err = hl.getRuntimeConfig(ctx) + cfg, err = h.getRuntimeConfig(ctx) return err - }, hl.cfg.Http.RetryCfg) + }, h.cfg.Http.RetryCfg) // Get the runtime configuration once on startup err := getConfigRetry(ctx) if err != nil { log.Warn("Could not get remote runtime configuration", "error", err) err = fmt.Errorf("could not get remote runtime configuration: %w", err) - } else { // The else block is necessary to avoid sending a nil pointer - hl.cRuntime <- *runtimeCfg } + h.cRuntime <- cfg - if hl.cfg.Interval == 0 { + if h.cfg.Interval == 0 { log.Info("HTTP Loader disabled") return err } - tick := time.NewTicker(hl.cfg.Interval) + tick := time.NewTicker(h.cfg.Interval) defer tick.Stop() for { select { - case <-hl.done: + case <-h.done: log.Info("HTTP Loader terminated") return nil case <-ctx.Done(): @@ -90,24 +90,24 @@ func (hl *HttpLoader) Run(ctx context.Context) error { case <-tick.C: if err := getConfigRetry(ctx); err != nil { log.Warn("Could not get remote runtime configuration", "error", err) - tick.Reset(hl.cfg.Interval) + tick.Reset(h.cfg.Interval) continue } log.Info("Successfully got remote runtime configuration") - hl.cRuntime <- *runtimeCfg - tick.Reset(hl.cfg.Interval) + h.cRuntime <- cfg + tick.Reset(h.cfg.Interval) } } } // GetRuntimeConfig gets the remote runtime configuration -func (hl *HttpLoader) getRuntimeConfig(ctx context.Context) (*runtime.Config, error) { +func (hl *HttpLoader) getRuntimeConfig(ctx context.Context) (cfg runtime.Config, err error) { log := logger.FromContext(ctx).With("url", hl.cfg.Http.Url) req, err := http.NewRequestWithContext(ctx, http.MethodGet, hl.cfg.Http.Url, http.NoBody) if err != nil { log.Error("Could not create http GET request", "error", err.Error()) - return nil, err + return cfg, err } if hl.cfg.Http.Token != "" { req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", hl.cfg.Http.Token)) @@ -116,34 +116,34 @@ func (hl *HttpLoader) getRuntimeConfig(ctx context.Context) (*runtime.Config, er res, err := hl.client.Do(req) //nolint:bodyclose if err != nil { log.Error("Http get request failed", "error", err.Error()) - return nil, err + return cfg, err } defer func(Body io.ReadCloser) { - err = Body.Close() - if err != nil { - log.Error("Failed to close response body", "error", err.Error()) + cErr := Body.Close() + if cErr != nil { + log.Error("Failed to close response body", "error", cErr) + err = errors.Join(cErr, err) } }(res.Body) if res.StatusCode != http.StatusOK { log.Error("Http get request failed", "status", res.Status) - return nil, fmt.Errorf("request failed, status is %s", res.Status) + return cfg, fmt.Errorf("request failed, status is %s", res.Status) } - body, err := io.ReadAll(res.Body) + b, err := io.ReadAll(res.Body) if err != nil { log.Error("Could not read response body", "error", err.Error()) - return nil, err + return cfg, err } log.Debug("Successfully got response") - runtimeCfg := &runtime.Config{} - if err := yaml.Unmarshal(body, &runtimeCfg); err != nil { + if err := yaml.Unmarshal(b, &cfg); err != nil { log.Error("Could not unmarshal response", "error", err.Error()) - return nil, err + return cfg, err } - return runtimeCfg, nil + return cfg, nil } // Shutdown stops the loader diff --git a/pkg/config/http_test.go b/pkg/config/http_test.go index 2eca538c..a8e93b90 100644 --- a/pkg/config/http_test.go +++ b/pkg/config/http_test.go @@ -50,7 +50,7 @@ func TestHttpLoader_GetRuntimeConfig(t *testing.T) { name string cfg *Config httpResponder httpResponder - want *runtime.Config + want runtime.Config wantErr bool }{ { @@ -65,7 +65,7 @@ func TestHttpLoader_GetRuntimeConfig(t *testing.T) { statusCode: 200, response: httpmock.File("test/data/config.yaml").String(), }, - want: &runtime.Config{ + want: runtime.Config{ Health: &health.Config{ Targets: []string{"http://localhost:8080/health"}, Interval: 1 * time.Second, @@ -88,7 +88,7 @@ func TestHttpLoader_GetRuntimeConfig(t *testing.T) { statusCode: 200, response: httpmock.File("test/data/config.yaml").String(), }, - want: &runtime.Config{ + want: runtime.Config{ Health: &health.Config{ Targets: []string{"http://localhost:8080/health"}, Interval: 1 * time.Second, @@ -161,27 +161,27 @@ func TestHttpLoader_Run(t *testing.T) { tests := []struct { name string interval time.Duration - response *runtime.Config + response runtime.Config code int wantErr bool }{ { name: "non-200 response", interval: 500 * time.Millisecond, - response: nil, + response: runtime.Config{}, code: http.StatusInternalServerError, wantErr: false, }, { name: "empty checks' configuration", interval: 500 * time.Millisecond, - response: &runtime.Config{}, + response: runtime.Config{}, code: http.StatusOK, }, { name: "config with health check", interval: 500 * time.Millisecond, - response: &runtime.Config{ + response: runtime.Config{ Health: &health.Config{ Targets: []string{"http://localhost:8080/health"}, Interval: 1 * time.Second, @@ -192,7 +192,7 @@ func TestHttpLoader_Run(t *testing.T) { { name: "continuous loading disabled", interval: 0, - response: &runtime.Config{ + response: runtime.Config{ Health: &health.Config{ Targets: []string{"http://localhost:8080/health"}, Interval: 1 * time.Second, @@ -348,7 +348,7 @@ func TestHttpLoader_Run_config_not_sent_to_channel_500(t *testing.T) { httpmock.Activate() defer httpmock.DeactivateAndReset() - resp, err := httpmock.NewJsonResponder(500, nil) + resp, err := httpmock.NewJsonResponder(http.StatusInternalServerError, nil) if err != nil { t.Fatalf("Failed creating json responder: %v", err) }