From bb4678f5af2712233ab454fc88ba54d7a2f84b78 Mon Sep 17 00:00:00 2001 From: Daniel Muehlbachler-Pietrzykowski Date: Fri, 4 Oct 2024 10:16:16 +0200 Subject: [PATCH] feat: expose healthz endpoint on different port; collect status from adguard; fix #131 BREAKING CHANGE: introduce new address and port configuration for healthz endpoint --- README.md | 14 +++-- .../init/configuration/configuration.go | 2 + cmd/webhook/init/dnsprovider/dnsprovider.go | 2 +- .../init/dnsprovider/dnsprovider_test.go | 2 +- cmd/webhook/init/server/server.go | 24 ++++++- cmd/webhook/init/server/server_test.go | 62 ++++++++++++++++++- cmd/webhook/main.go | 6 +- go.mod | 4 +- go.sum | 8 +-- internal/adguard/client.go | 28 ++++++--- internal/adguard/provider.go | 22 +++++-- internal/adguard/provider_test.go | 10 ++- pkg/webhook/webhook.go | 28 ++++----- 13 files changed, 164 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 833628c..d4e1ddf 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,12 @@ However, rules **not matching** above format, for example, `|domain.to.block`, * > **If** an **upgrade path** between version is **listed here**, please make sure to **follow** those paths **without skipping a version**! > Otherwise, the correct behaviour cannot be guaranteed, resulting in possible inconsistencies or errors. +### v7 to v8 + +`v8` introduces the `HEALTHZ_ADDRESS` (default: `0.0.0.0`) and `HEALTHZ_PORT` (default: `8080`) environment variable to introduce compatibility with the official Helm chart. + +Attention: if you are using a customized Helm chart, make sure to adjust the probes accordingly. + ### v5 to v6 `v6` introduces the `ADGUARD_SET_IMPORTANT_FLAG` environment variable to set the `important` flag for AdGuard rules. This is enabled by default. @@ -120,16 +126,18 @@ sidecars: ports: - containerPort: 8888 name: http + - containerPort: 8080 + name: healthz livenessProbe: httpGet: path: /healthz - port: http + port: healthz initialDelaySeconds: 10 timeoutSeconds: 5 readinessProbe: httpGet: path: /healthz - port: http + port: healthz initialDelaySeconds: 10 timeoutSeconds: 5 env: @@ -150,8 +158,6 @@ sidecars: secretKeyRef: name: adguard-configuration key: password - - name: SERVER_HOST - value: "0.0.0.0" - name: DRY_RUN value: "false" EOF diff --git a/cmd/webhook/init/configuration/configuration.go b/cmd/webhook/init/configuration/configuration.go index fd5a17d..8c85f17 100644 --- a/cmd/webhook/init/configuration/configuration.go +++ b/cmd/webhook/init/configuration/configuration.go @@ -11,6 +11,8 @@ import ( type Config struct { ServerHost string `env:"SERVER_HOST" envDefault:"localhost"` ServerPort int `env:"SERVER_PORT" envDefault:"8888"` + HealthzHost string `env:"HEALTHZ_HOST" envDefault:"0.0.0.0"` + HealthzPort int `env:"HEALTHZ_PORT" envDefault:"8080"` ServerReadTimeout time.Duration `env:"SERVER_READ_TIMEOUT"` ServerWriteTimeout time.Duration `env:"SERVER_WRITE_TIMEOUT"` DomainFilter []string `env:"DOMAIN_FILTER" envDefault:""` diff --git a/cmd/webhook/init/dnsprovider/dnsprovider.go b/cmd/webhook/init/dnsprovider/dnsprovider.go index 13882c7..7447998 100644 --- a/cmd/webhook/init/dnsprovider/dnsprovider.go +++ b/cmd/webhook/init/dnsprovider/dnsprovider.go @@ -16,7 +16,7 @@ import ( type AdguardProviderFactory func(baseProvider *provider.BaseProvider, adguardConfig *adguard.Configuration) provider.Provider -func Init(config configuration.Config) (provider.Provider, error) { +func Init(config configuration.Config) (adguard.Provider, error) { var domainFilter endpoint.DomainFilter createMsg := "creating adguard provider with " diff --git a/cmd/webhook/init/dnsprovider/dnsprovider_test.go b/cmd/webhook/init/dnsprovider/dnsprovider_test.go index 0ccf91f..94d560c 100644 --- a/cmd/webhook/init/dnsprovider/dnsprovider_test.go +++ b/cmd/webhook/init/dnsprovider/dnsprovider_test.go @@ -82,7 +82,7 @@ func TestInit(t *testing.T) { return } - assert.Equal(t, tc.expectedFlags, dnsProvider.(*adguard.Provider).Configuration.DNSEntryFlags()) + assert.Equal(t, tc.expectedFlags, dnsProvider.(*adguard.AGProvider).Configuration.DNSEntryFlags()) assert.NoErrorf(t, err, "error creating provider") assert.NotNil(t, dnsProvider) diff --git a/cmd/webhook/init/server/server.go b/cmd/webhook/init/server/server.go index 941d372..3a64085 100644 --- a/cmd/webhook/init/server/server.go +++ b/cmd/webhook/init/server/server.go @@ -26,7 +26,6 @@ import ( func Init(config configuration.Config, p *webhook.Webhook) *http.Server { r := chi.NewRouter() - r.Use(webhook.Health) r.Get("/", p.Negotiate) r.Get("/records", p.Records) r.Post("/records", p.ApplyChanges) @@ -42,6 +41,24 @@ func Init(config configuration.Config, p *webhook.Webhook) *http.Server { return srv } +// Init server initialization function +// The server will respond to the following endpoints: +// - /healthz (GET): health check endpoint +func InitHealthz(config configuration.Config, p *webhook.Webhook) *http.Server { + r := chi.NewRouter() + + r.Get("/healthz", p.Health) + + srv := createHTTPServer(fmt.Sprintf("%s:%d", config.HealthzHost, config.HealthzPort), r, config.ServerReadTimeout, config.ServerWriteTimeout) + go func() { + log.Infof("starting healthz on addr: '%s' ", srv.Addr) + if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Errorf("can't serve healthz on addr: '%s', error: %v", srv.Addr, err) + } + }() + return srv +} + func createHTTPServer(addr string, hand http.Handler, readTimeout, writeTimeout time.Duration) *http.Server { return &http.Server{ ReadTimeout: readTimeout, @@ -52,7 +69,7 @@ func createHTTPServer(addr string, hand http.Handler, readTimeout, writeTimeout } // ShutdownGracefully gracefully shutdown the http server -func ShutdownGracefully(srv *http.Server) { +func ShutdownGracefully(srv *http.Server, healthz *http.Server) { sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT) sig := <-sigCh @@ -62,5 +79,8 @@ func ShutdownGracefully(srv *http.Server) { if err := srv.Shutdown(ctx); err != nil { log.Errorf("error shutting down server: %v", err) } + if err := healthz.Shutdown(ctx); err != nil { + log.Errorf("error shutting down healthz: %v", err) + } cancel() } diff --git a/cmd/webhook/init/server/server_test.go b/cmd/webhook/init/server/server_test.go index 1dbfdf8..debd3d0 100644 --- a/cmd/webhook/init/server/server_test.go +++ b/cmd/webhook/init/server/server_test.go @@ -40,8 +40,10 @@ var mockProvider *MockProvider func TestMain(m *testing.M) { mockProvider = &MockProvider{} - srv := Init(configuration.Init(), webhook.New(mockProvider)) - go ShutdownGracefully(srv) + hook := webhook.New(mockProvider) + srv := Init(configuration.Init(), hook) + healthz := InitHealthz(configuration.Init(), hook) + go ShutdownGracefully(srv, healthz) time.Sleep(300 * time.Millisecond) @@ -51,6 +53,20 @@ func TestMain(m *testing.M) { } } +func TestHealth(t *testing.T) { + testCases := []testCase{ + { + name: "health ok", + method: http.MethodGet, + path: "/healthz", + body: "", + expectedStatusCode: http.StatusOK, + }, + } + + executeHealthzTestCases(t, testCases) +} + func TestRecords(t *testing.T) { testCases := []testCase{ { @@ -517,11 +533,53 @@ func executeTestCases(t *testing.T, testCases []testCase) { } } +func executeHealthzTestCases(t *testing.T, testCases []testCase) { + log.SetLevel(log.DebugLevel) + + for i, tc := range testCases { + t.Run(fmt.Sprintf("%d. %s", i+1, tc.name), func(t *testing.T) { + mockProvider.testCase = tc + mockProvider.t = t + + var bodyReader io.Reader + request, err := http.NewRequest(tc.method, "http://localhost:8080"+tc.path, bodyReader) + if err != nil { + t.Error(err) + } + + response, err := http.DefaultClient.Do(request) + if err != nil { + t.Error(err) + } + + if response.StatusCode != tc.expectedStatusCode { + t.Errorf("expected status code %d, got %d", tc.expectedStatusCode, response.StatusCode) + } + + if tc.expectedBody != "" { + body, err := io.ReadAll(response.Body) + if err != nil { + t.Error(err) + } + _ = response.Body.Close() + actualTrimmedBody := strings.TrimSpace(string(body)) + if actualTrimmedBody != tc.expectedBody { + t.Errorf("expected body '%s', got '%s'", tc.expectedBody, actualTrimmedBody) + } + } + }) + } +} + type MockProvider struct { t *testing.T testCase testCase } +func (d *MockProvider) Health(_ context.Context) bool { + return true +} + func (d *MockProvider) Records(_ context.Context) ([]*endpoint.Endpoint, error) { return d.testCase.returnRecords, d.testCase.hasError } diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index d1a0b3b..c7a0e4b 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -33,6 +33,8 @@ func main() { log.Fatalf("failed to initialize provider: %v", err) } - srv := server.Init(config, webhook.New(provider)) - server.ShutdownGracefully(srv) + hook := webhook.New(provider) + srv := server.Init(config, hook) + healthz := server.InitHealthz(config, hook) + server.ShutdownGracefully(srv, healthz) } diff --git a/go.mod b/go.mod index 964d59b..a9b5db9 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.20.4 // indirect github.com/prometheus/client_model v0.6.1 // indirect - github.com/prometheus/common v0.59.1 // indirect + github.com/prometheus/common v0.60.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/x448/float16 v0.8.4 // indirect golang.org/x/net v0.29.0 // indirect @@ -44,7 +44,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apimachinery v0.31.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect - k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3 // indirect + k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect ) diff --git a/go.sum b/go.sum index 1957e68..aa0295c 100644 --- a/go.sum +++ b/go.sum @@ -53,8 +53,8 @@ github.com/prometheus/client_golang v1.20.4 h1:Tgh3Yr67PaOv/uTqloMsCEdeuFTatm5zI github.com/prometheus/client_golang v1.20.4/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE= github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E= github.com/prometheus/client_model v0.6.1/go.mod h1:OrxVMOVHjw3lKMa8+x6HeMGkHMQyHDk9E3jmP2AmGiY= -github.com/prometheus/common v0.59.1 h1:LXb1quJHWm1P6wq/U824uxYi4Sg0oGvNeUm1z5dJoX0= -github.com/prometheus/common v0.59.1/go.mod h1:GpWM7dewqmVYcd7SmRaiWVe9SSqjf0UrwnYnpEZNuT0= +github.com/prometheus/common v0.60.0 h1:+V9PAREWNvJMAuJ1x1BaWl9dewMW4YrHZQbx0sJNllA= +github.com/prometheus/common v0.60.0/go.mod h1:h0LYf1R1deLSKtD4Vdg8gy4RuOvENW2J/h19V5NADQw= github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc= github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= @@ -125,8 +125,8 @@ k8s.io/apimachinery v0.31.1 h1:mhcUBbj7KUjaVhyXILglcVjuS4nYXiwC+KKFBgIVy7U= k8s.io/apimachinery v0.31.1/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= -k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3 h1:b2FmK8YH+QEwq/Sy2uAEhmqL5nPfGYbJOcaqjeYYZoA= -k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 h1:MDF6h2H/h4tbzmtIKTuctcwZmY0tY9mD9fNT47QO6HI= +k8s.io/utils v0.0.0-20240921022957-49e7df575cb6/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= sigs.k8s.io/external-dns v0.15.0 h1:4NCSLHONsTmJXD8KReb4hubSz9Cx4goCHz3Dl+pGR+Q= sigs.k8s.io/external-dns v0.15.0/go.mod h1:QdocdJu3mk9l4u80fu992lZEKqKd1130h17yNisIC78= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= diff --git a/internal/adguard/client.go b/internal/adguard/client.go index bf09721..dc35bd7 100644 --- a/internal/adguard/client.go +++ b/internal/adguard/client.go @@ -16,6 +16,7 @@ import ( type Client interface { GetFilteringRules(ctx context.Context) ([]string, error) SetFilteringRules(ctx context.Context, rules []string) error + Status(ctx context.Context) (bool, error) } // hhtpClient type used to implement Client with an HTTP client @@ -24,6 +25,12 @@ type httpClient struct { config *Configuration } +// adguardStatus is the response of retrieving the AdGuard status +type adguardStatus struct { + Version string `json:"version"` + Running bool `json:"running"` +} + // getFilteringRules is the response of retrieving filtering rules type getFilteringRules struct { UserRules []string `json:"user_rules"` @@ -43,8 +50,8 @@ func newAdguardClient(config *Configuration) (*httpClient, error) { } // check validity of the configuration - err := c.status(context.Background()) - if err != nil { + s, err := c.Status(context.Background()) + if err != nil || !s { return nil, err } @@ -76,19 +83,26 @@ func (c *httpClient) doRequest(ctx context.Context, method, path string, body io return resp, nil } -func (c *httpClient) status(ctx context.Context) error { +func (c *httpClient) Status(ctx context.Context) (bool, error) { if c.config.DryRun { log.Info("would check adguard configuration") - return nil + return true, nil } r, err := c.doRequest(ctx, http.MethodGet, "status", nil) if err != nil { - return err + return false, err } - _ = r.Body.Close() + defer r.Body.Close() - return nil + var resp adguardStatus + err = json.NewDecoder(r.Body).Decode(&resp) + if err != nil { + return false, err + } + log.Debugf("retrieved status: %+v", resp) + + return resp.Running, nil } // Retrieves all existing filtering rules from Adguard diff --git a/internal/adguard/provider.go b/internal/adguard/provider.go index e2963e1..a01290e 100644 --- a/internal/adguard/provider.go +++ b/internal/adguard/provider.go @@ -14,8 +14,14 @@ import ( "sigs.k8s.io/external-dns/provider" ) +// Provider interface for interfacing with Adguard +type Provider interface { + provider.Provider + Health(ctx context.Context) bool +} + // Provider type for interfacing with Adguard -type Provider struct { +type AGProvider struct { provider.BaseProvider Configuration *Configuration @@ -29,7 +35,7 @@ var ( ) // NewAdguardProvider initializes a new provider -func NewAdguardProvider(domainFilter endpoint.DomainFilter, config *Configuration) (provider.Provider, error) { +func NewAdguardProvider(domainFilter endpoint.DomainFilter, config *Configuration) (Provider, error) { log.Debugf("using adguard at %s", config.URL) // URL adjustment according to the specification @@ -45,7 +51,7 @@ func NewAdguardProvider(domainFilter endpoint.DomainFilter, config *Configuratio return nil, fmt.Errorf("failed to create the adguard client: %w", err) } - p := &Provider{ + p := &AGProvider{ Configuration: config, client: c, domainFilter: domainFilter, @@ -54,8 +60,14 @@ func NewAdguardProvider(domainFilter endpoint.DomainFilter, config *Configuratio return p, nil } +// Health checks if AdGuard is available +func (p *AGProvider) Health(ctx context.Context) bool { + s, _ := p.client.Status(ctx) + return s +} + // ApplyChanges syncs the desired state with Adguard -func (p *Provider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { +func (p *AGProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { log.Debugf("received changes: %+v", changes) or, err := p.client.GetFilteringRules(ctx) @@ -139,7 +151,7 @@ func (p *Provider) ApplyChanges(ctx context.Context, changes *plan.Changes) erro } // Records reads all endpoints from Adguard -func (p *Provider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) { +func (p *AGProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) { resp, err := p.client.GetFilteringRules(ctx) if err != nil { return nil, err diff --git a/internal/adguard/provider_test.go b/internal/adguard/provider_test.go index 4a8d7cb..f462f43 100644 --- a/internal/adguard/provider_test.go +++ b/internal/adguard/provider_test.go @@ -25,7 +25,7 @@ type testCase struct { } var mockHTTPClient *MockHTTPClient -var testProvider *Provider +var testProvider Provider func TestNewAdguardProvider(t *testing.T) { log.SetLevel(log.DebugLevel) @@ -425,7 +425,7 @@ func TestRecords(t *testing.T) { testCase: tc, t: t, } - testProvider = &Provider{ + testProvider = &AGProvider{ client: mockHTTPClient, domainFilter: tc.domainFilter, } @@ -850,7 +850,7 @@ func TestApplyChanges(t *testing.T) { testCase: tc, t: t, } - testProvider = &Provider{ + testProvider = &AGProvider{ Configuration: &Configuration{}, client: mockHTTPClient, domainFilter: tc.domainFilter, @@ -879,3 +879,7 @@ func (d *MockHTTPClient) SetFilteringRules(_ context.Context, rules []string) er require.ElementsMatch(d.t, d.testCase.rules, rules) return nil } + +func (d *MockHTTPClient) Status(_ context.Context) (bool, error) { + return true, nil +} diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 51bea00..b0a0478 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -5,11 +5,11 @@ import ( "fmt" "net/http" + "github.com/muhlba91/external-dns-provider-adguard/internal/adguard" log "github.com/sirupsen/logrus" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" - "sigs.k8s.io/external-dns/provider" ) const ( @@ -17,7 +17,6 @@ const ( contentTypePlaintext = "text/plain" acceptHeader = "Accept" varyHeader = "Vary" - healthPath = "/healthz" logFieldRequestPath = "requestPath" logFieldRequestMethod = "requestMethod" logFieldError = "error" @@ -25,26 +24,15 @@ const ( // Webhook for external dns provider type Webhook struct { - provider provider.Provider + provider adguard.Provider } // New creates a new instance of the Webhook -func New(provider provider.Provider) *Webhook { +func New(provider adguard.Provider) *Webhook { p := Webhook{provider: provider} return &p } -// Health handles the health request -func Health(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == healthPath { - w.WriteHeader(http.StatusOK) - return - } - next.ServeHTTP(w, r) - }) -} - func (p *Webhook) contentTypeHeaderCheck(w http.ResponseWriter, r *http.Request) error { return p.headerCheck(true, w, r) } @@ -101,6 +89,16 @@ func (p *Webhook) headerCheck(isContentType bool, w http.ResponseWriter, r *http return nil } +// Health handles the health request +func (p *Webhook) Health(w http.ResponseWriter, r *http.Request) { + requestLog(r).Debug("requesting healthz") + if !p.provider.Health(r.Context()) { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + w.WriteHeader(http.StatusOK) +} + // Records handles the get request for records func (p *Webhook) Records(w http.ResponseWriter, r *http.Request) { if err := p.acceptHeaderCheck(w, r); err != nil {