From 0920bb99fe187b97fb176b7eaa50918dc4e4d83a Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Tue, 19 Dec 2023 21:21:17 +0300 Subject: [PATCH 1/7] Pull request 2109: AG-20945-rule-list-filter Squashed commit of the following: commit 2da8c1754f349a9b7f8b629de8f0c892b9bae4dc Merge: 5cea6a6a2 4fc6bf504 Author: Ainar Garipov Date: Tue Dec 19 21:14:07 2023 +0300 Merge branch 'master' into AG-20945-rule-list-filter commit 5cea6a6a2bed88f645828ab5b4e7de09f9bf91ec Author: Ainar Garipov Date: Tue Dec 19 17:53:21 2023 +0300 filtering/rulelist: imp docs, tests commit f01434b37a3f0070d71eb0ae72ad8eb2f4922147 Author: Ainar Garipov Date: Thu Dec 14 19:17:02 2023 +0300 filtering/rulelist: imp names commit fe2bf68e6b99673b216b5c4ba867a5f4ed788d22 Author: Ainar Garipov Date: Thu Dec 14 19:07:53 2023 +0300 all: go mod tidy commit c7081d3486a78e8402dc8fe0223111a6fccdd19f Author: Ainar Garipov Date: Thu Dec 14 19:03:33 2023 +0300 filtering/rulelist: add filter --- go.mod | 3 +- go.sum | 6 +- internal/filtering/http.go | 16 +- internal/filtering/rulelist/filter.go | 338 +++++++++++++++++++ internal/filtering/rulelist/filter_test.go | 107 ++++++ internal/filtering/rulelist/parser_test.go | 8 +- internal/filtering/rulelist/rulelist.go | 48 ++- internal/filtering/rulelist/rulelist_test.go | 22 +- 8 files changed, 531 insertions(+), 17 deletions(-) create mode 100644 internal/filtering/rulelist/filter.go create mode 100644 internal/filtering/rulelist/filter_test.go diff --git a/go.mod b/go.mod index 990ef3090bb..3c4ac5a1eba 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/NYTimes/gziphandler v1.1.1 github.com/ameshkov/dnscrypt/v2 v2.2.7 github.com/bluele/gcache v0.0.2 + github.com/c2h5oh/datasize v0.0.0-20220606134207-859f65c6625b github.com/digineo/go-ipset/v2 v2.2.1 github.com/dimfeld/httptreemux/v5 v5.5.0 github.com/fsnotify/fsnotify v1.7.0 @@ -16,7 +17,7 @@ require ( github.com/google/go-cmp v0.6.0 github.com/google/gopacket v1.1.19 github.com/google/renameio/v2 v2.0.0 - github.com/google/uuid v1.4.0 + github.com/google/uuid v1.5.0 github.com/insomniacslk/dhcp v0.0.0-20231206064809-8c70d406f6d2 github.com/josharian/native v1.1.1-0.20230202152459-5c7d0dd6ab86 github.com/kardianos/service v1.2.2 diff --git a/go.sum b/go.sum index d33b4e08f13..0c53e35e4b7 100644 --- a/go.sum +++ b/go.sum @@ -18,6 +18,8 @@ github.com/beefsack/go-rate v0.0.0-20220214233405-116f4ca011a0 h1:0b2vaepXIfMsG+ github.com/beefsack/go-rate v0.0.0-20220214233405-116f4ca011a0/go.mod h1:6YNgTHLutezwnBvyneBbwvB8C82y3dcoOj5EQJIdGXA= github.com/bluele/gcache v0.0.2 h1:WcbfdXICg7G/DGBh1PFfcirkWOQV+v077yF1pSy3DGw= github.com/bluele/gcache v0.0.2/go.mod h1:m15KV+ECjptwSPxKhOhQoAFQVtUFjTVkc3H8o0t/fp0= +github.com/c2h5oh/datasize v0.0.0-20220606134207-859f65c6625b h1:6+ZFm0flnudZzdSE0JxlhR2hKnGPcNB35BjQf4RYQDY= +github.com/c2h5oh/datasize v0.0.0-20220606134207-859f65c6625b/go.mod h1:S/7n9copUssQ56c7aAgHqftWO4LTf4xY6CGWt8Bc+3M= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -46,8 +48,8 @@ github.com/google/pprof v0.0.0-20231205033806-a5a03c77bf08/go.mod h1:czg5+yv1E0Z github.com/google/renameio/v2 v2.0.0 h1:UifI23ZTGY8Tt29JbYFiuyIU3eX+RNFtUwefq9qAhxg= github.com/google/renameio/v2 v2.0.0/go.mod h1:BtmJXm5YlszgC+TD4HOEEUFgkJP3nLxehU6hfe7jRt4= github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/google/uuid v1.4.0 h1:MtMxsa51/r9yyhkyLsVeVt0B+BGQZzpQiTQ4eHZ8bc4= -github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.5.0 h1:1p67kYwdtXjb0gL0BPiP1Av9wiZPo5A8z2cWkTZ+eyU= +github.com/google/uuid v1.5.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hugelgupf/socketpair v0.0.0-20190730060125-05d35a94e714 h1:/jC7qQFrv8CrSJVmaolDVOxTfS9kc36uB6H40kdbQq8= github.com/insomniacslk/dhcp v0.0.0-20231206064809-8c70d406f6d2 h1:9K06NfxkBh25x56yVhWWlKFE8YpicaSfHwoV8SFbueA= github.com/insomniacslk/dhcp v0.0.0-20231206064809-8c70d406f6d2/go.mod h1:3A9PQ1cunSDF/1rbTq99Ts4pVnycWg+vlPkfeD2NLFI= diff --git a/internal/filtering/http.go b/internal/filtering/http.go index ca6b8cf9487..362ef0e4aa4 100644 --- a/internal/filtering/http.go +++ b/internal/filtering/http.go @@ -24,23 +24,25 @@ func validateFilterURL(urlStr string) (err error) { if filepath.IsAbs(urlStr) { _, err = os.Stat(urlStr) - if err != nil { - // Don't wrap the error since it's informative enough as is. - return err - } - return nil + // Don't wrap the error since it's informative enough as is. + return err } u, err := url.ParseRequestURI(urlStr) if err != nil { // Don't wrap the error since it's informative enough as is. return err - } else if s := u.Scheme; s != aghhttp.SchemeHTTP && s != aghhttp.SchemeHTTPS { + } + + if s := u.Scheme; s != aghhttp.SchemeHTTP && s != aghhttp.SchemeHTTPS { return &url.Error{ Op: "Check scheme", URL: urlStr, - Err: fmt.Errorf("only %v allowed", []string{aghhttp.SchemeHTTP, aghhttp.SchemeHTTPS}), + Err: fmt.Errorf("only %v allowed", []string{ + aghhttp.SchemeHTTP, + aghhttp.SchemeHTTPS, + }), } } diff --git a/internal/filtering/rulelist/filter.go b/internal/filtering/rulelist/filter.go new file mode 100644 index 00000000000..278eef5c0f1 --- /dev/null +++ b/internal/filtering/rulelist/filter.go @@ -0,0 +1,338 @@ +package rulelist + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "net/url" + "os" + "path/filepath" + "time" + + "github.com/AdguardTeam/AdGuardHome/internal/aghrenameio" + "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/ioutil" + "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/urlfilter/filterlist" + "github.com/c2h5oh/datasize" +) + +// Filter contains information about a single rule-list filter. +// +// TODO(a.garipov): Use. +type Filter struct { + // url is the URL of this rule list. Supported schemes are: + // - http + // - https + // - file + url *url.URL + + // ruleList is the last successfully compiled [filterlist.RuleList]. + ruleList filterlist.RuleList + + // updated is the time of the last successful update. + updated time.Time + + // name is the human-readable name of this rule-list filter. + name string + + // uid is the unique ID of this rule-list filter. + uid UID + + // urlFilterID is used for working with package urlfilter. + urlFilterID URLFilterID + + // rulesCount contains the number of rules in this rule-list filter. + rulesCount int + + // checksum is a CRC32 hash used to quickly check if the rules within a list + // file have changed. + checksum uint32 + + // enabled, if true, means that this rule-list filter is used for filtering. + // + // TODO(a.garipov): Take into account. + enabled bool +} + +// FilterConfig contains the configuration for a [Filter]. +type FilterConfig struct { + // URL is the URL of this rule-list filter. Supported schemes are: + // - http + // - https + // - file + URL *url.URL + + // Name is the human-readable name of this rule-list filter. If not set, it + // is either taken from the rule-list data or generated synthetically from + // the UID. + Name string + + // UID is the unique ID of this rule-list filter. + UID UID + + // URLFilterID is used for working with package urlfilter. + URLFilterID URLFilterID + + // Enabled, if true, means that this rule-list filter is used for filtering. + Enabled bool +} + +// NewFilter creates a new rule-list filter. The filter is not refreshed, so a +// refresh should be performed before use. +func NewFilter(c *FilterConfig) (f *Filter, err error) { + if c.URL == nil { + return nil, errors.Error("no url") + } + + switch s := c.URL.Scheme; s { + case "http", "https", "file": + // Go on. + default: + return nil, fmt.Errorf("bad url scheme: %q", s) + } + + return &Filter{ + url: c.URL, + name: c.Name, + uid: c.UID, + urlFilterID: c.URLFilterID, + enabled: c.Enabled, + }, nil +} + +// Refresh updates the data in the rule-list filter. parseBuf is the initial +// buffer used to parse information from the data. cli and maxSize are only +// used when f is a URL-based list. +func (f *Filter) Refresh( + ctx context.Context, + parseBuf []byte, + cli *http.Client, + cacheDir string, + maxSize datasize.ByteSize, +) (parseRes *ParseResult, err error) { + cachePath := filepath.Join(cacheDir, f.uid.String()+".txt") + + switch s := f.url.Scheme; s { + case "http", "https": + parseRes, err = f.setFromHTTP(ctx, parseBuf, cli, cachePath, maxSize.Bytes()) + case "file": + parseRes, err = f.setFromFile(parseBuf, f.url.Path, cachePath) + default: + // Since the URL has been prevalidated in New, consider this a + // programmer error. + panic(fmt.Errorf("bad url scheme: %q", s)) + } + if err != nil { + // Don't wrap the error, because it's informative enough as is. + return nil, err + } + + if f.checksum != parseRes.Checksum { + f.checksum = parseRes.Checksum + f.rulesCount = parseRes.RulesCount + f.setName(parseRes.Title) + f.updated = time.Now() + } + + return parseRes, nil +} + +// setFromHTTP sets the rule-list filter's data from its URL. It also caches +// the data into a file. +func (f *Filter) setFromHTTP( + ctx context.Context, + parseBuf []byte, + cli *http.Client, + cachePath string, + maxSize uint64, +) (parseRes *ParseResult, err error) { + defer func() { err = errors.Annotate(err, "setting from http: %w") }() + + text, parseRes, err := f.readFromHTTP(ctx, parseBuf, cli, cachePath, maxSize) + if err != nil { + // Don't wrap the error, because it's informative enough as is. + return nil, err + } + + // TODO(a.garipov): Add filterlist.BytesRuleList. + f.ruleList = &filterlist.StringRuleList{ + ID: f.urlFilterID, + RulesText: text, + IgnoreCosmetic: true, + } + + return parseRes, nil +} + +// readFromHTTP reads the data from the rule-list filter's URL into the cache +// file as well as returns it as a string. The data is filtered through a +// parser and so is free from comments, unnecessary whitespace, etc. +func (f *Filter) readFromHTTP( + ctx context.Context, + parseBuf []byte, + cli *http.Client, + cachePath string, + maxSize uint64, +) (text string, parseRes *ParseResult, err error) { + urlStr := f.url.String() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, urlStr, nil) + if err != nil { + return "", nil, fmt.Errorf("making request for http url %q: %w", urlStr, err) + } + + resp, err := cli.Do(req) + if err != nil { + return "", nil, fmt.Errorf("requesting from http url: %w", err) + } + defer func() { err = errors.WithDeferred(err, resp.Body.Close()) }() + + // TODO(a.garipov): Use [agdhttp.CheckStatus] when it's moved to golibs. + if resp.StatusCode != http.StatusOK { + return "", nil, fmt.Errorf("got status code %d, want %d", resp.StatusCode, http.StatusOK) + } + + fltFile, err := aghrenameio.NewPendingFile(cachePath, 0o644) + if err != nil { + return "", nil, fmt.Errorf("creating temp file: %w", err) + } + defer func() { err = aghrenameio.WithDeferredCleanup(err, fltFile) }() + + buf := &bytes.Buffer{} + mw := io.MultiWriter(buf, fltFile) + + parser := NewParser() + httpBody := ioutil.LimitReader(resp.Body, maxSize) + parseRes, err = parser.Parse(mw, httpBody, parseBuf) + if err != nil { + return "", nil, fmt.Errorf("parsing response from http url %q: %w", urlStr, err) + } + + return buf.String(), parseRes, nil +} + +// setName sets the title using either the already-present name, the given title +// from the rule-list data, or a synthetic name. +func (f *Filter) setName(title string) { + if f.name != "" { + return + } + + if title != "" { + f.name = title + + return + } + + f.name = fmt.Sprintf("List %s", f.uid) +} + +// setFromFile sets the rule-list filter's data from a file path. It also +// caches the data into a file. +// +// TODO(a.garipov): Retest on Windows once rule-list updater is committed. See +// if calling Close is necessary here. +func (f *Filter) setFromFile( + parseBuf []byte, + filePath string, + cachePath string, +) (parseRes *ParseResult, err error) { + defer func() { err = errors.Annotate(err, "setting from file: %w") }() + + parseRes, err = parseIntoCache(parseBuf, filePath, cachePath) + if err != nil { + // Don't wrap the error, because it's informative enough as is. + return nil, err + } + + err = f.Close() + if err != nil { + return nil, fmt.Errorf("closing old rule list: %w", err) + } + + rl, err := filterlist.NewFileRuleList(f.urlFilterID, cachePath, true) + if err != nil { + return nil, fmt.Errorf("opening new rule list: %w", err) + } + + f.ruleList = rl + + return parseRes, nil +} + +// parseIntoCache copies the relevant the data from filePath into cachePath +// while also parsing it. +func parseIntoCache( + parseBuf []byte, + filePath string, + cachePath string, +) (parseRes *ParseResult, err error) { + tmpFile, err := aghrenameio.NewPendingFile(cachePath, 0o644) + if err != nil { + return nil, fmt.Errorf("creating temp file: %w", err) + } + defer func() { err = aghrenameio.WithDeferredCleanup(err, tmpFile) }() + + // #nosec G304 -- Assume that cachePath is always cacheDir joined with a + // uid using [filepath.Join]. + f, err := os.Open(filePath) + if err != nil { + return nil, fmt.Errorf("opening src file: %w", err) + } + defer func() { err = errors.WithDeferred(err, f.Close()) }() + + parser := NewParser() + parseRes, err = parser.Parse(tmpFile, f, parseBuf) + if err != nil { + return nil, fmt.Errorf("copying src file: %w", err) + } + + return parseRes, nil +} + +// Close closes the underlying rule list. +func (f *Filter) Close() (err error) { + if f.ruleList == nil { + return nil + } + + return f.ruleList.Close() +} + +// filterUpdate represents a single ongoing rule-list filter update. +// +//lint:ignore U1000 TODO(a.garipov): Use. +type filterUpdate struct { + httpCli *http.Client + cacheDir string + name string + parseBuf []byte + maxSize datasize.ByteSize +} + +// process runs an update of a single rule-list. +func (u *filterUpdate) process(ctx context.Context, f *Filter) (err error) { + prevChecksum := f.checksum + parseRes, err := f.Refresh(ctx, u.parseBuf, u.httpCli, u.cacheDir, u.maxSize) + if err != nil { + return fmt.Errorf("updating %s: %w", f.uid, err) + } + + if prevChecksum == parseRes.Checksum { + log.Info("filtering: filter %q: filter %q: no change", u.name, f.uid) + + return nil + } + + log.Info( + "filtering: updated filter %q: filter %q: %d bytes, %d rules", + u.name, + f.uid, + parseRes.BytesWritten, + parseRes.RulesCount, + ) + + return nil +} diff --git a/internal/filtering/rulelist/filter_test.go b/internal/filtering/rulelist/filter_test.go new file mode 100644 index 00000000000..93cd6e9c378 --- /dev/null +++ b/internal/filtering/rulelist/filter_test.go @@ -0,0 +1,107 @@ +package rulelist_test + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "testing" + + "github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist" + "github.com/AdguardTeam/golibs/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFilter_Refresh(t *testing.T) { + cacheDir := t.TempDir() + uid := rulelist.MustNewUID() + + initialFile := filepath.Join(cacheDir, "initial.txt") + initialData := []byte( + testRuleTextTitle + + testRuleTextBlocked, + ) + writeErr := os.WriteFile(initialFile, initialData, 0o644) + require.NoError(t, writeErr) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + pt := testutil.PanicT{} + + _, err := io.WriteString(w, testRuleTextTitle+testRuleTextBlocked) + require.NoError(pt, err) + })) + + srvURL, urlErr := url.Parse(srv.URL) + require.NoError(t, urlErr) + + testCases := []struct { + url *url.URL + name string + wantNewErrMsg string + }{{ + url: nil, + name: "nil_url", + wantNewErrMsg: "no url", + }, { + url: &url.URL{ + Scheme: "ftp", + }, + name: "bad_scheme", + wantNewErrMsg: `bad url scheme: "ftp"`, + }, { + name: "file", + url: &url.URL{ + Scheme: "file", + Path: initialFile, + }, + wantNewErrMsg: "", + }, { + name: "http", + url: srvURL, + wantNewErrMsg: "", + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f, err := rulelist.NewFilter(&rulelist.FilterConfig{ + URL: tc.url, + Name: tc.name, + UID: uid, + URLFilterID: testURLFilterID, + Enabled: true, + }) + if tc.wantNewErrMsg != "" { + assert.EqualError(t, err, tc.wantNewErrMsg) + + return + } + + testutil.CleanupAndRequireSuccess(t, f.Close) + + require.NotNil(t, f) + + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) + t.Cleanup(cancel) + + buf := make([]byte, rulelist.DefaultRuleBufSize) + cli := &http.Client{ + Timeout: testTimeout, + } + + res, err := f.Refresh(ctx, buf, cli, cacheDir, rulelist.DefaultMaxRuleListSize) + require.NoError(t, err) + + assert.Equal(t, testTitle, res.Title) + assert.Equal(t, len(testRuleTextBlocked), res.BytesWritten) + assert.Equal(t, 1, res.RulesCount) + + // Check that the cached file exists. + _, err = os.Stat(filepath.Join(cacheDir, uid.String()+".txt")) + require.NoError(t, err) + }) + } +} diff --git a/internal/filtering/rulelist/parser_test.go b/internal/filtering/rulelist/parser_test.go index 5554458d825..45a8e46552c 100644 --- a/internal/filtering/rulelist/parser_test.go +++ b/internal/filtering/rulelist/parser_test.go @@ -69,12 +69,12 @@ func TestParser_Parse(t *testing.T) { wantWritten: len(testRuleTextBlocked) + len(testRuleTextHTML), }, { name: "title", - in: "! Title: Test Title \n" + + in: testRuleTextTitle + "! Title: Bad, Ignored Title\n" + testRuleTextBlocked, wantDst: testRuleTextBlocked, wantErrMsg: "", - wantTitle: "Test Title", + wantTitle: testTitle, wantRulesNum: 1, wantWritten: len(testRuleTextBlocked), }, { @@ -87,14 +87,14 @@ func TestParser_Parse(t *testing.T) { wantWritten: len(testRuleTextCosmetic), }, { name: "bad_char", - in: "! Title: Test Title \n" + + in: testRuleTextTitle + testRuleTextBlocked + ">>>\x7F<<<", wantDst: testRuleTextBlocked, wantErrMsg: "line 3: " + "character 4: " + "likely binary character '\\x7f'", - wantTitle: "Test Title", + wantTitle: testTitle, wantRulesNum: 1, wantWritten: len(testRuleTextBlocked), }, { diff --git a/internal/filtering/rulelist/rulelist.go b/internal/filtering/rulelist/rulelist.go index 464650a1f20..e0fd61b48e6 100644 --- a/internal/filtering/rulelist/rulelist.go +++ b/internal/filtering/rulelist/rulelist.go @@ -1,9 +1,55 @@ // Package rulelist contains the implementation of the standard rule-list // filter that wraps an urlfilter filtering-engine. // -// TODO(a.garipov): Expand. +// TODO(a.garipov): Add a new update worker. package rulelist +import ( + "fmt" + + "github.com/c2h5oh/datasize" + "github.com/google/uuid" +) + // DefaultRuleBufSize is the default length of a buffer used to read a line with // a filtering rule, in bytes. +// +// TODO(a.garipov): Consider using [datasize.ByteSize]. It is currently only +// used as an int. const DefaultRuleBufSize = 1024 + +// DefaultMaxRuleListSize is the default maximum filtering-rule list size. +const DefaultMaxRuleListSize = 64 * datasize.MB + +// URLFilterID is a semantic type-alias for IDs used for working with package +// urlfilter. +type URLFilterID = int + +// UID is the type for the unique IDs of filtering-rule lists. +type UID uuid.UUID + +// NewUID returns a new filtering-rule list UID. Any error returned is an error +// from the cryptographic randomness reader. +func NewUID() (uid UID, err error) { + uuidv7, err := uuid.NewV7() + + return UID(uuidv7), err +} + +// MustNewUID is a wrapper around [NewUID] that panics if there is an error. +func MustNewUID() (uid UID) { + uid, err := NewUID() + if err != nil { + panic(fmt.Errorf("unexpected uuidv7 error: %w", err)) + } + + return uid +} + +// type check +var _ fmt.Stringer = UID{} + +// String implements the [fmt.Stringer] interface for UID. +func (id UID) String() (s string) { + return uuid.UUID(id).String() +} diff --git a/internal/filtering/rulelist/rulelist_test.go b/internal/filtering/rulelist/rulelist_test.go index aec6f33bafe..dc79d503fe4 100644 --- a/internal/filtering/rulelist/rulelist_test.go +++ b/internal/filtering/rulelist/rulelist_test.go @@ -1,16 +1,34 @@ package rulelist_test -import "time" +import ( + "testing" + "time" + + "github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist" + "github.com/AdguardTeam/golibs/testutil" +) + +func TestMain(m *testing.M) { + testutil.DiscardLogOutput(m) +} // testTimeout is the common timeout for tests. const testTimeout = 1 * time.Second -// Common texts for tests. +// testURLFilterID is the common [rulelist.URLFilterID] for tests. +const testURLFilterID rulelist.URLFilterID = 1 + +// testTitle is the common title for tests. +const testTitle = "Test Title" + +// Common rule texts for tests. const ( testRuleTextBadTab = "||bad-tab-and-comment.example^\t# A comment.\n" testRuleTextBlocked = "||blocked.example^\n" + testRuleTextBlocked2 = "||blocked-2.example^\n" testRuleTextEtcHostsTab = "0.0.0.0 tab..example^\t# A comment.\n" testRuleTextHTML = "\n" + testRuleTextTitle = "! Title: " + testTitle + " \n" // testRuleTextCosmetic is a cosmetic rule with a zero-width non-joiner. // From 8d1e1e8bec72e6c05d2fc928ba4874762c40bf95 Mon Sep 17 00:00:00 2001 From: Marc Brugger Date: Wed, 20 Dec 2023 07:52:39 +0100 Subject: [PATCH 2/7] correct schema type for client upstreams_cache_size Signed-off-by: bakito --- openapi/openapi.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 5ab3fa528c1..781959ff771 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -2709,7 +2709,7 @@ changed. This behaviour can be changed in the future versions. - 'type': 'boolean' + 'type': 'integer' 'ClientAuto': 'type': 'object' 'description': 'Auto-Client information' From 97a048b238926cad8fed19bcff8c2dae2f9262ff Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Wed, 20 Dec 2023 18:15:20 +0300 Subject: [PATCH 3/7] openapi: upd chlog --- openapi/CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index b71ee56c8a2..669662000d3 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -4,6 +4,13 @@ ## v0.108.0: API changes +## v0.107.44: API changes + +### Type correction in `Client` + +* Field `upstreams_cache_size` of object `Client` now correctly has type + `integer` instead of the previous incorrect type `boolean`. + ## v0.107.42: API changes ### The new field `"serve_plain_dns"` in `TlsConfig` @@ -152,7 +159,7 @@ has the following format: * The new field `"top_upstreams_responses"` in `GET /control/stats` method shows the total number of responses from each upstream. -* The new field `"top_upstrems_avg_time"` in `GET /control/stats` method shows +* The new field `"top_upstreams_avg_time"` in `GET /control/stats` method shows the average processing time in seconds of requests from each upstream. ## v0.107.30: API changes From 38b3ec19cc6fed5fb2f9f5bff65f48aad8dd9677 Mon Sep 17 00:00:00 2001 From: Ildar Kamalov Date: Wed, 20 Dec 2023 18:17:05 +0300 Subject: [PATCH 4/7] Pull request: fix edit static lease Updates #6534 Squashed commit of the following: commit 1ca6cdc37a865ff0beab2d1f4fb0d44528bd4df3 Author: Ildar Kamalov Date: Wed Dec 20 15:41:27 2023 +0300 ADG-7889 fix edit static lease --- CHANGELOG.md | 2 ++ client/src/components/Settings/Dhcp/Leases.js | 7 +++++-- client/src/helpers/constants.js | 1 + client/src/reducers/dhcp.js | 3 ++- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65f60b6c8e3..6067c7ad56e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,12 +33,14 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Fixed +- Pre-filling the Edit static lease window with data ([#6534]). - Names defined in the `/etc/hosts` for a single address family wrongly considered undefined for another family ([#6541]). - Omitted CNAME records in safe search results, which can cause YouTube to not work on iOS ([#6352]). [#6352]: https://github.com/AdguardTeam/AdGuardHome/issues/6352 +[#6534]: https://github.com/AdguardTeam/AdGuardHome/issues/6534 [#6541]: https://github.com/AdguardTeam/AdGuardHome/issues/6541 [#6545]: https://github.com/AdguardTeam/AdGuardHome/issues/6545 diff --git a/client/src/components/Settings/Dhcp/Leases.js b/client/src/components/Settings/Dhcp/Leases.js index 9b245dbe284..3b267b3b4be 100644 --- a/client/src/components/Settings/Dhcp/Leases.js +++ b/client/src/components/Settings/Dhcp/Leases.js @@ -3,7 +3,7 @@ import { connect } from 'react-redux'; import PropTypes from 'prop-types'; import ReactTable from 'react-table'; import { Trans, withTranslation } from 'react-i18next'; -import { LEASES_TABLE_DEFAULT_PAGE_SIZE } from '../../../helpers/constants'; +import { LEASES_TABLE_DEFAULT_PAGE_SIZE, MODAL_TYPE } from '../../../helpers/constants'; import { sortIp } from '../../../helpers/helpers'; import { toggleLeaseModal } from '../../../actions'; @@ -18,7 +18,10 @@ class Leases extends Component { convertToStatic = (data) => () => { const { dispatch } = this.props; - dispatch(toggleLeaseModal(data)); + dispatch(toggleLeaseModal({ + type: MODAL_TYPE.ADD_LEASE, + config: data, + })); } makeStatic = ({ row }) => { diff --git a/client/src/helpers/constants.js b/client/src/helpers/constants.js index 0b345f4df3d..89f53cae349 100644 --- a/client/src/helpers/constants.js +++ b/client/src/helpers/constants.js @@ -181,6 +181,7 @@ export const MODAL_TYPE = { ADD_REWRITE: 'ADD_REWRITE', EDIT_REWRITE: 'EDIT_REWRITE', EDIT_LEASE: 'EDIT_LEASE', + ADD_LEASE: 'ADD_LEASE', }; export const CLIENT_ID = { diff --git a/client/src/reducers/dhcp.js b/client/src/reducers/dhcp.js index 877451a524d..47768b509eb 100644 --- a/client/src/reducers/dhcp.js +++ b/client/src/reducers/dhcp.js @@ -128,7 +128,8 @@ const dhcp = handleActions( const newState = { ...state, isModalOpen: !state.isModalOpen, - leaseModalConfig: payload, + modalType: payload?.type || '', + leaseModalConfig: payload?.config, }; return newState; }, From 4bc5c346a7efd805745392be602d8997df1a34de Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Wed, 20 Dec 2023 19:16:22 +0300 Subject: [PATCH 5/7] Pull request 2115: ADG-7924-stats-interval Squashed commit of the following: commit 2c7ee92b82087c7dfcb9f6955cc062e1c32d07f8 Merge: 67313ec7d 469857922 Author: Stanislav Chzhen Date: Wed Dec 20 19:04:44 2023 +0300 Merge branch 'master' into ADG-7924-stats-interval commit 67313ec7de745b4b1fd6201e127b7237dfe1bc30 Author: Stanislav Chzhen Date: Tue Dec 19 20:02:31 2023 +0300 all: imp docs commit f073dc46f18438cbc7d7249c8d623f6fa0e1403c Author: Stanislav Chzhen Date: Tue Dec 19 18:33:51 2023 +0300 upd: chlog commit 109dbb146ad589ee6d3f2b1b8bf4e12b31ccee61 Author: Stanislav Chzhen Date: Tue Dec 19 17:23:58 2023 +0300 stats: interval --- CHANGELOG.md | 1 + internal/stats/unit.go | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6067c7ad56e..1e18591055b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Fixed +- Statistics for 7 days displayed as 168 hours on the dashboard. - Pre-filling the Edit static lease window with data ([#6534]). - Names defined in the `/etc/hosts` for a single address family wrongly considered undefined for another family ([#6541]). diff --git a/internal/stats/unit.go b/internal/stats/unit.go index 84c6770b9c3..478d9f66d89 100644 --- a/internal/stats/unit.go +++ b/internal/stats/unit.go @@ -484,7 +484,7 @@ func (s *StatsCtx) fillCollectedStats(data *StatsResp, units []*unitDB, curID ui data.TimeUnits = timeUnitsHours daysCount := size / 24 - if daysCount > 7 { + if daysCount >= 7 { size = daysCount data.TimeUnits = timeUnitsDays } @@ -510,6 +510,10 @@ func (s *StatsCtx) fillCollectedStats(data *StatsResp, units []*unitDB, curID ui // fillCollectedStatsDaily fills data with collected daily statistics. units // must contain data for the count of days. +// +// TODO(s.chzhen): Improve collection of statistics for frontend. Dashboard +// cards should contain statistics for the whole interval without rounding to +// days. func (s *StatsCtx) fillCollectedStatsDaily( data *StatsResp, units []*unitDB, From abf20c6dea2f8f66cc1f8461660b2cff74784ee3 Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Mon, 25 Dec 2023 13:48:44 +0300 Subject: [PATCH 6/7] Pull request 2116: fix-nil-deref Squashed commit of the following: commit bf6cfdb4e2315dc6826daa4c83aef5e961b86ddc Merge: 3c532f508 4bc5c346a Author: Stanislav Chzhen Date: Mon Dec 25 13:35:45 2023 +0300 Merge branch 'master' into fix-nil-deref commit 3c532f50876f3d04c63e1377b1143f2436fd37f2 Author: Stanislav Chzhen Date: Wed Dec 20 13:59:29 2023 +0300 home: fix nil deref --- internal/home/clientshttp.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/home/clientshttp.go b/internal/home/clientshttp.go index 20d6080f1fb..0bbdea9ce77 100644 --- a/internal/home/clientshttp.go +++ b/internal/home/clientshttp.go @@ -378,6 +378,8 @@ func (clients *clientsContainer) handleUpdateClient(w http.ResponseWriter, r *ht if !ok { aghhttp.Error(r, w, http.StatusBadRequest, "client not found") + + return } c, err := clients.jsonToClient(dj.Data, prev) From 1511fabeecf24067b1bb36af943826d91712d502 Mon Sep 17 00:00:00 2001 From: Dimitry Kolyshev Date: Mon, 25 Dec 2023 14:16:48 +0300 Subject: [PATCH 7/7] Pull request: AG-28771 conf upstream mode Squashed commit of the following: commit afb5a0d8a499bccf7761baea40910f39c92b8a20 Merge: 09ac43c85 abf20c6de Author: Dimitry Kolyshev Date: Mon Dec 25 12:55:45 2023 +0200 Merge remote-tracking branch 'origin/master' into conf-ups-mode commit 09ac43c859ef8cbd3bb0488d1a945589cd59ca19 Author: Dimitry Kolyshev Date: Fri Dec 22 14:36:07 2023 +0200 openapi: imp docs commit d0fbd4349e4bddde73c6e92f75854acfc481ac0d Author: Dimitry Kolyshev Date: Fri Dec 22 11:47:10 2023 +0200 all: changelog commit 105f9c50738733b0736a768fb9ee09d2e7fbf42e Merge: 62a2cf12d 4bc5c346a Author: Dimitry Kolyshev Date: Fri Dec 22 11:27:21 2023 +0200 Merge remote-tracking branch 'origin/master' into conf-ups-mode # Conflicts: # openapi/CHANGELOG.md commit 62a2cf12df694611888e840a5041a9c517cdfddb Author: Dimitry Kolyshev Date: Fri Dec 22 10:52:59 2023 +0200 openapi: imp docs commit 87956c49240da44b216489920feff69996e3502b Author: Dimitry Kolyshev Date: Thu Dec 21 12:08:07 2023 +0200 dnsforward: imp code commit bf74d67ad112735d557be3d8fac75964cd99e375 Author: Dimitry Kolyshev Date: Wed Dec 20 15:46:38 2023 +0200 dnsforward: imp code commit 3a98dee88809a25118a14a1f07eeecbfccb14cd9 Author: Dimitry Kolyshev Date: Wed Dec 20 15:41:06 2023 +0200 dnsforward: imp code commit 1499da1fa0319ac3ad914171e807446f2c4d2fdb Author: Dimitry Kolyshev Date: Wed Dec 20 13:36:28 2023 +0200 dnsforward: imp code commit 228c61a5a0f73cc13655cef8bdaa1995b3f7fced Author: Dimitry Kolyshev Date: Wed Dec 20 13:06:11 2023 +0200 dnsforward: imp code commit 069ee22c6d904db4e983135ce87a9fe8d12b7e9a Author: Dimitry Kolyshev Date: Tue Dec 19 12:39:25 2023 +0200 dnsforward: imp code commit 90919f99a975862dcb07ac82fb740e4404e48bae Author: Dimitry Kolyshev Date: Tue Dec 19 12:10:43 2023 +0200 confmigrate: fix commit a8c329950423b59098d1f2b16d1da7100dd54f8d Author: Dimitry Kolyshev Date: Tue Dec 19 12:08:05 2023 +0200 dnsforward: imp code commit 58b53ccd97d353fab0df29f13425b5e341c8fdeb Author: Dimitry Kolyshev Date: Mon Dec 18 15:10:01 2023 +0200 all: conf upstream mode --- CHANGELOG.md | 25 ++++++ internal/configmigrate/configmigrate.go | 2 +- .../configmigrate/migrations_internal_test.go | 82 +++++++++++++++++++ internal/configmigrate/migrator.go | 1 + internal/configmigrate/v28.go | 47 +++++++++++ internal/dnsforward/config.go | 28 ++++--- internal/dnsforward/dns64_test.go | 1 + internal/dnsforward/dnsforward.go | 10 +-- internal/dnsforward/dnsforward_test.go | 29 ++++++- internal/dnsforward/dnsrewrite_test.go | 1 + internal/dnsforward/filter_test.go | 1 + internal/dnsforward/http.go | 73 +++++++++++++---- internal/dnsforward/http_test.go | 7 +- internal/dnsforward/process_internal_test.go | 4 + internal/dnsforward/svcbmsg_test.go | 1 + internal/dnsforward/upstreams.go | 16 ++-- internal/home/config.go | 2 +- openapi/CHANGELOG.md | 7 ++ openapi/openapi.yaml | 11 ++- 19 files changed, 297 insertions(+), 51 deletions(-) create mode 100644 internal/configmigrate/v28.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e18591055b..e5c5ec9ae68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,31 @@ NOTE: Add new changes BELOW THIS COMMENT. - Ability to disable plain-DNS serving via UI if an encrypted protocol is already used ([#1660]). +### Changed + +- The field `"upstream_mode"` in `POST /control/dns_config` and + `GET /control/dns_info` HTTP APIs now accepts `load_balance` value. Check + `openapi/CHANGELOG.md` for more details. + +#### Configuration changes + +- The properties `dns.'all_servers` and `dns.fastest_addr` were removed, their + values migrated to newly added field `dns.upstream_mode` that describes the + logic through which upstreams will be used. + + ```yaml + # BEFORE: + 'dns': + # … + 'all_servers': true + 'fastest_addr': true + + # AFTER: + 'dns': + # … + 'upstream_mode': 'parallel' + ``` + ### Fixed - Statistics for 7 days displayed as 168 hours on the dashboard. diff --git a/internal/configmigrate/configmigrate.go b/internal/configmigrate/configmigrate.go index 05198cc54bc..6e8845e0986 100644 --- a/internal/configmigrate/configmigrate.go +++ b/internal/configmigrate/configmigrate.go @@ -2,4 +2,4 @@ package configmigrate // LastSchemaVersion is the most recent schema version. -const LastSchemaVersion uint = 27 +const LastSchemaVersion uint = 28 diff --git a/internal/configmigrate/migrations_internal_test.go b/internal/configmigrate/migrations_internal_test.go index 4531ea687e5..5349102fea6 100644 --- a/internal/configmigrate/migrations_internal_test.go +++ b/internal/configmigrate/migrations_internal_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/golibs/testutil" "github.com/AdguardTeam/golibs/timeutil" @@ -1646,3 +1647,84 @@ func TestUpgradeSchema26to27(t *testing.T) { }) } } + +func TestUpgradeSchema27to28(t *testing.T) { + const newSchemaVer = 28 + + testCases := []struct { + in yobj + want yobj + name string + }{{ + name: "empty", + in: yobj{}, + want: yobj{ + "schema_version": newSchemaVer, + }, + }, { + name: "load_balance", + in: yobj{ + "dns": yobj{ + "all_servers": false, + "fastest_addr": false, + }, + }, + want: yobj{ + "dns": yobj{ + "upstream_mode": dnsforward.UpstreamModeLoadBalance, + }, + "schema_version": newSchemaVer, + }, + }, { + name: "parallel", + in: yobj{ + "dns": yobj{ + "all_servers": true, + "fastest_addr": false, + }, + }, + want: yobj{ + "dns": yobj{ + "upstream_mode": dnsforward.UpstreamModeParallel, + }, + "schema_version": newSchemaVer, + }, + }, { + name: "parallel_fastest", + in: yobj{ + "dns": yobj{ + "all_servers": true, + "fastest_addr": true, + }, + }, + want: yobj{ + "dns": yobj{ + "upstream_mode": dnsforward.UpstreamModeParallel, + }, + "schema_version": newSchemaVer, + }, + }, { + name: "load_balance", + in: yobj{ + "dns": yobj{ + "all_servers": false, + "fastest_addr": true, + }, + }, + want: yobj{ + "dns": yobj{ + "upstream_mode": dnsforward.UpstreamModeFastestAddr, + }, + "schema_version": newSchemaVer, + }, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := migrateTo28(tc.in) + require.NoError(t, err) + + assert.Equal(t, tc.want, tc.in) + }) + } +} diff --git a/internal/configmigrate/migrator.go b/internal/configmigrate/migrator.go index f0cab769b5c..ebdf6ba7430 100644 --- a/internal/configmigrate/migrator.go +++ b/internal/configmigrate/migrator.go @@ -119,6 +119,7 @@ func (m *Migrator) upgradeConfigSchema(current, target uint, diskConf yobj) (err 24: migrateTo25, 25: migrateTo26, 26: migrateTo27, + 27: migrateTo28, } for i, migrate := range upgrades[current:target] { diff --git a/internal/configmigrate/v28.go b/internal/configmigrate/v28.go new file mode 100644 index 00000000000..c32f9f3fb5f --- /dev/null +++ b/internal/configmigrate/v28.go @@ -0,0 +1,47 @@ +package configmigrate + +import ( + "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" +) + +// migrateTo28 performs the following changes: +// +// # BEFORE: +// 'dns': +// 'all_servers': true +// 'fastest_addr': true +// # … +// # … +// +// # AFTER: +// 'dns': +// 'upstream_mode': 'parallel' +// # … +// # … +func migrateTo28(diskConf yobj) (err error) { + diskConf["schema_version"] = 28 + + dns, ok, err := fieldVal[yobj](diskConf, "dns") + if !ok { + return err + } + + allServers, _, _ := fieldVal[bool](dns, "all_servers") + fastestAddr, _, _ := fieldVal[bool](dns, "fastest_addr") + + var upstreamModeType dnsforward.UpstreamMode + if allServers { + upstreamModeType = dnsforward.UpstreamModeParallel + } else if fastestAddr { + upstreamModeType = dnsforward.UpstreamModeFastestAddr + } else { + upstreamModeType = dnsforward.UpstreamModeLoadBalance + } + + dns["upstream_mode"] = upstreamModeType + + delete(dns, "all_servers") + delete(dns, "fastest_addr") + + return nil +} diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go index ec20096b951..e37e93761b1 100644 --- a/internal/dnsforward/config.go +++ b/internal/dnsforward/config.go @@ -89,12 +89,8 @@ type Config struct { // servers are not responding. FallbackDNS []string `yaml:"fallback_dns"` - // AllServers, if true, parallel queries to all configured upstream servers - // are enabled. - AllServers bool `yaml:"all_servers"` - - // FastestAddr, if true, use Fastest Address algorithm. - FastestAddr bool `yaml:"fastest_addr"` + // UpstreamMode determines the logic through which upstreams will be used. + UpstreamMode UpstreamMode `yaml:"upstream_mode"` // FastestTimeout replaces the default timeout for dialing IP addresses // when FastestAddr is true. @@ -294,6 +290,16 @@ type ServerConfig struct { ServePlainDNS bool } +// UpstreamMode is a enumeration of upstream mode representations. See +// [proxy.UpstreamModeType]. +type UpstreamMode string + +const ( + UpstreamModeLoadBalance UpstreamMode = "load_balance" + UpstreamModeParallel UpstreamMode = "parallel" + UpstreamModeFastestAddr UpstreamMode = "fastest_addr" +) + // newProxyConfig creates and validates configuration for the main proxy. func (s *Server) newProxyConfig() (conf *proxy.Config, err error) { srvConf := s.conf @@ -328,12 +334,10 @@ func (s *Server) newProxyConfig() (conf *proxy.Config, err error) { conf.CacheSizeBytes = int(srvConf.CacheSize) } - setProxyUpstreamMode( - conf, - srvConf.AllServers, - srvConf.FastestAddr, - srvConf.FastestTimeout.Duration, - ) + err = setProxyUpstreamMode(conf, srvConf.UpstreamMode, srvConf.FastestTimeout.Duration) + if err != nil { + return nil, fmt.Errorf("upstream mode: %w", err) + } conf.BogusNXDomain, err = parseBogusNXDOMAIN(srvConf.BogusNXDomain) if err != nil { diff --git a/internal/dnsforward/dns64_test.go b/internal/dnsforward/dns64_test.go index 55c08db7faa..ad89098cfa7 100644 --- a/internal/dnsforward/dns64_test.go +++ b/internal/dnsforward/dns64_test.go @@ -290,6 +290,7 @@ func TestServer_HandleDNSRequest_dns64(t *testing.T) { TCPListenAddrs: []*net.TCPAddr{{}}, UseDNS64: true, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index cdd2c2404fb..65ed57666da 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -703,12 +703,10 @@ func (s *Server) prepareInternalProxy() (err error) { MaxGoroutines: int(s.conf.MaxGoroutines), } - setProxyUpstreamMode( - conf, - srvConf.AllServers, - srvConf.FastestAddr, - srvConf.FastestTimeout.Duration, - ) + err = setProxyUpstreamMode(conf, srvConf.UpstreamMode, srvConf.FastestTimeout.Duration) + if err != nil { + return fmt.Errorf("invalid upstream mode: %w", err) + } // TODO(a.garipov): Make a proper constructor for proxy.Proxy. p := &proxy.Proxy{ diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index bdd3308d080..479b5528fcf 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -177,6 +177,7 @@ func createTestTLS(t *testing.T, tlsConf TLSConfig) (s *Server, certPem []byte) UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -305,6 +306,7 @@ func TestServer(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -344,6 +346,7 @@ func TestServer_timeout(t *testing.T) { srvConf := &ServerConfig{ UpstreamTimeout: testTimeout, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -362,6 +365,7 @@ func TestServer_timeout(t *testing.T) { s, err := NewServer(DNSCreateParams{DNSFilter: createTestDNSFilter(t)}) require.NoError(t, err) + s.conf.Config.UpstreamMode = UpstreamModeLoadBalance s.conf.Config.EDNSClientSubnet = &EDNSClientSubnet{ Enabled: false, } @@ -379,6 +383,7 @@ func TestServer_Prepare_fallbacks(t *testing.T) { "#tls://1.1.1.1", "8.8.8.8", }, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -401,6 +406,7 @@ func TestServerWithProtectionDisabled(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -478,7 +484,8 @@ func TestServerRace(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ - UpstreamDNS: []string{"8.8.8.8:53", "8.8.4.4:53"}, + UpstreamMode: UpstreamModeLoadBalance, + UpstreamDNS: []string{"8.8.8.8:53", "8.8.4.4:53"}, }, ConfigModified: func() {}, ServePlainDNS: true, @@ -531,6 +538,7 @@ func TestSafeSearch(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -614,6 +622,7 @@ func TestInvalidRequest(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -643,6 +652,7 @@ func TestBlockedRequest(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -678,7 +688,8 @@ func TestServerCustomClientUpstream(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ - CacheSize: defaultCacheSize, + CacheSize: defaultCacheSize, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -756,6 +767,7 @@ func TestBlockCNAMEProtectionEnabled(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -789,6 +801,7 @@ func TestBlockCNAME(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -864,6 +877,7 @@ func TestClientRulesForCNAMEMatching(t *testing.T) { FilterHandler: func(_ netip.Addr, _ string, settings *filtering.Settings) { settings.FilteringEnabled = false }, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -909,6 +923,7 @@ func TestNullBlockedRequest(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -974,7 +989,8 @@ func TestBlockedCustomIP(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ - UpstreamDNS: []string{"8.8.8.8:53", "8.8.4.4:53"}, + UpstreamDNS: []string{"8.8.8.8:53", "8.8.4.4:53"}, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -1027,6 +1043,7 @@ func TestBlockedByHosts(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -1078,6 +1095,7 @@ func TestBlockedBySafeBrowsing(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -1136,7 +1154,8 @@ func TestRewrite(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ - UpstreamDNS: []string{"8.8.8.8:53"}, + UpstreamDNS: []string{"8.8.8.8:53"}, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, @@ -1265,6 +1284,7 @@ func TestPTRResponseFromDHCPLeases(t *testing.T) { s.conf.TCPListenAddrs = []*net.TCPAddr{{}} s.conf.UpstreamDNS = []string{"127.0.0.1:53"} s.conf.Config.EDNSClientSubnet = &EDNSClientSubnet{Enabled: false} + s.conf.Config.UpstreamMode = UpstreamModeLoadBalance err = s.Prepare(&s.conf) require.NoError(t, err) @@ -1347,6 +1367,7 @@ func TestPTRResponseFromHosts(t *testing.T) { s.conf.TCPListenAddrs = []*net.TCPAddr{{}} s.conf.UpstreamDNS = []string{"127.0.0.1:53"} s.conf.Config.EDNSClientSubnet = &EDNSClientSubnet{Enabled: false} + s.conf.Config.UpstreamMode = UpstreamModeLoadBalance err = s.Prepare(&s.conf) require.NoError(t, err) diff --git a/internal/dnsforward/dnsrewrite_test.go b/internal/dnsforward/dnsrewrite_test.go index 1022388f5cc..5204c2f2c1d 100644 --- a/internal/dnsforward/dnsrewrite_test.go +++ b/internal/dnsforward/dnsrewrite_test.go @@ -38,6 +38,7 @@ func TestServer_FilterDNSRewrite(t *testing.T) { BlockingMode: filtering.BlockingModeDefault, }, ServerConfig{ Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, diff --git a/internal/dnsforward/filter_test.go b/internal/dnsforward/filter_test.go index 6559b308464..9e172a32705 100644 --- a/internal/dnsforward/filter_test.go +++ b/internal/dnsforward/filter_test.go @@ -31,6 +31,7 @@ func TestHandleDNSRequest_handleDNSRequest(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{ Enabled: false, }, diff --git a/internal/dnsforward/http.go b/internal/dnsforward/http.go index ac82ea76f9f..9c1eb89f280 100644 --- a/internal/dnsforward/http.go +++ b/internal/dnsforward/http.go @@ -70,7 +70,7 @@ type jsonDNSConfig struct { DisableIPv6 *bool `json:"disable_ipv6"` // UpstreamMode defines the way DNS requests are constructed. - UpstreamMode *string `json:"upstream_mode"` + UpstreamMode *jsonUpstreamMode `json:"upstream_mode"` // BlockedResponseTTL is the TTL for blocked responses. BlockedResponseTTL *uint32 `json:"blocked_response_ttl"` @@ -114,6 +114,21 @@ type jsonDNSConfig struct { DefaultLocalPTRUpstreams []string `json:"default_local_ptr_upstreams,omitempty"` } +// jsonUpstreamMode is a enumeration of upstream modes. +type jsonUpstreamMode string + +const ( + // jsonUpstreamModeEmpty is the default value on frontend, it is used as + // jsonUpstreamModeLoadBalance mode. + // + // Deprecated: Use jsonUpstreamModeLoadBalance instead. + jsonUpstreamModeEmpty jsonUpstreamMode = "" + + jsonUpstreamModeLoadBalance jsonUpstreamMode = "load_balance" + jsonUpstreamModeParallel jsonUpstreamMode = "parallel" + jsonUpstreamModeFastestAddr jsonUpstreamMode = "fastest_addr" +) + func (s *Server) getDNSConfig() (c *jsonDNSConfig) { protectionEnabled, protectionDisabledUntil := s.UpdatedProtectionStatus() @@ -145,11 +160,16 @@ func (s *Server) getDNSConfig() (c *jsonDNSConfig) { usePrivateRDNS := s.conf.UsePrivateRDNS localPTRUpstreams := stringutil.CloneSliceOrEmpty(s.conf.LocalPTRResolvers) - var upstreamMode string - if s.conf.FastestAddr { - upstreamMode = "fastest_addr" - } else if s.conf.AllServers { - upstreamMode = "parallel" + var upstreamMode jsonUpstreamMode + switch s.conf.UpstreamMode { + case UpstreamModeLoadBalance: + // TODO(d.kolyshev): Support jsonUpstreamModeLoadBalance on frontend instead + // of jsonUpstreamModeEmpty. + upstreamMode = jsonUpstreamModeEmpty + case UpstreamModeParallel: + upstreamMode = jsonUpstreamModeParallel + case UpstreamModeFastestAddr: + upstreamMode = jsonUpstreamModeFastestAddr } defPTRUps, err := s.defaultLocalPTRUpstreams() @@ -222,18 +242,22 @@ func (req *jsonDNSConfig) checkBlockingMode() (err error) { return validateBlockingMode(*req.BlockingMode, req.BlockingIPv4, req.BlockingIPv6) } -// checkUpstreamsMode returns an error if the upstream mode is invalid. -func (req *jsonDNSConfig) checkUpstreamsMode() (err error) { +// checkUpstreamMode returns an error if the upstream mode is invalid. +func (req *jsonDNSConfig) checkUpstreamMode() (err error) { if req.UpstreamMode == nil { return nil } - mode := *req.UpstreamMode - if ok := slices.Contains([]string{"", "fastest_addr", "parallel"}, mode); !ok { - return fmt.Errorf("upstream_mode: incorrect value %q", mode) + switch um := *req.UpstreamMode; um { + case + jsonUpstreamModeEmpty, + jsonUpstreamModeLoadBalance, + jsonUpstreamModeParallel, + jsonUpstreamModeFastestAddr: + return nil + default: + return fmt.Errorf("upstream_mode: incorrect value %q", um) } - - return nil } // checkBootstrap returns an error if any bootstrap address is invalid. @@ -297,7 +321,7 @@ func (req *jsonDNSConfig) validate(privateNets netutil.SubnetSet) (err error) { return err } - err = req.checkUpstreamsMode() + err = req.checkUpstreamMode() if err != nil { // Don't wrap the error since it's informative enough as is. return err @@ -446,8 +470,9 @@ func (s *Server) setConfig(dc *jsonDNSConfig) (shouldRestart bool) { } if dc.UpstreamMode != nil { - s.conf.AllServers = *dc.UpstreamMode == "parallel" - s.conf.FastestAddr = *dc.UpstreamMode == "fastest_addr" + s.conf.UpstreamMode = mustParseUpstreamMode(*dc.UpstreamMode) + } else { + s.conf.UpstreamMode = UpstreamModeLoadBalance } if dc.EDNSCSUseCustom != nil && *dc.EDNSCSUseCustom { @@ -460,6 +485,22 @@ func (s *Server) setConfig(dc *jsonDNSConfig) (shouldRestart bool) { return s.setConfigRestartable(dc) } +// mustParseUpstreamMode returns an upstream mode parsed from jsonUpstreamMode. +// Panics in case of invalid value. +func mustParseUpstreamMode(mode jsonUpstreamMode) (um UpstreamMode) { + switch mode { + case jsonUpstreamModeEmpty, jsonUpstreamModeLoadBalance: + return UpstreamModeLoadBalance + case jsonUpstreamModeParallel: + return UpstreamModeParallel + case jsonUpstreamModeFastestAddr: + return UpstreamModeFastestAddr + default: + // Should never happen, since the value should be validated. + panic(fmt.Errorf("unexpected upstream mode: %q", mode)) + } +} + // setIfNotNil sets the value pointed at by currentPtr to the value pointed at // by newPtr if newPtr is not nil. currentPtr must not be nil. func setIfNotNil[T any](currentPtr, newPtr *T) (hasSet bool) { diff --git a/internal/dnsforward/http_test.go b/internal/dnsforward/http_test.go index 4b6987bb84a..64b27833e26 100644 --- a/internal/dnsforward/http_test.go +++ b/internal/dnsforward/http_test.go @@ -77,6 +77,7 @@ func TestDNSForwardHTTP_handleGetConfig(t *testing.T) { FallbackDNS: []string{"9.9.9.10"}, RatelimitSubnetLenIPv4: 24, RatelimitSubnetLenIPv6: 56, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ConfigModified: func() {}, @@ -103,7 +104,7 @@ func TestDNSForwardHTTP_handleGetConfig(t *testing.T) { }, { conf: func() ServerConfig { conf := defaultConf - conf.FastestAddr = true + conf.UpstreamMode = UpstreamModeFastestAddr return conf }, @@ -111,7 +112,7 @@ func TestDNSForwardHTTP_handleGetConfig(t *testing.T) { }, { conf: func() ServerConfig { conf := defaultConf - conf.AllServers = true + conf.UpstreamMode = UpstreamModeParallel return conf }, @@ -157,6 +158,7 @@ func TestDNSForwardHTTP_handleSetConfig(t *testing.T) { UpstreamDNS: []string{"8.8.8.8:53", "8.8.4.4:53"}, RatelimitSubnetLenIPv4: 24, RatelimitSubnetLenIPv6: 56, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ConfigModified: func() {}, @@ -523,6 +525,7 @@ func TestServer_HandleTestUpstreamDNS(t *testing.T) { TCPListenAddrs: []*net.TCPAddr{{}}, UpstreamTimeout: upsTimeout, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, diff --git a/internal/dnsforward/process_internal_test.go b/internal/dnsforward/process_internal_test.go index 18b04b3f67d..bec9c98ec5f 100644 --- a/internal/dnsforward/process_internal_test.go +++ b/internal/dnsforward/process_internal_test.go @@ -79,6 +79,7 @@ func TestServer_ProcessInitial(t *testing.T) { c := ServerConfig{ Config: Config{ AAAADisabled: tc.aaaaDisabled, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -179,6 +180,7 @@ func TestServer_ProcessFilteringAfterResponse(t *testing.T) { c := ServerConfig{ Config: Config{ AAAADisabled: tc.aaaaDisabled, + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -694,6 +696,7 @@ func TestServer_ProcessRestrictLocal(t *testing.T) { // TODO(s.chzhen): Add tests where EDNSClientSubnet.Enabled is true. // Improve Config declaration for tests. Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, @@ -770,6 +773,7 @@ func TestServer_ProcessLocalPTR_usingResolvers(t *testing.T) { UDPListenAddrs: []*net.UDPAddr{{}}, TCPListenAddrs: []*net.TCPAddr{{}}, Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, diff --git a/internal/dnsforward/svcbmsg_test.go b/internal/dnsforward/svcbmsg_test.go index 58275ef4f81..2c2b7b0bac2 100644 --- a/internal/dnsforward/svcbmsg_test.go +++ b/internal/dnsforward/svcbmsg_test.go @@ -17,6 +17,7 @@ func TestGenAnswerHTTPS_andSVCB(t *testing.T) { BlockingMode: filtering.BlockingModeDefault, }, ServerConfig{ Config: Config{ + UpstreamMode: UpstreamModeLoadBalance, EDNSClientSubnet: &EDNSClientSubnet{Enabled: false}, }, ServePlainDNS: true, diff --git a/internal/dnsforward/upstreams.go b/internal/dnsforward/upstreams.go index 3f877ac745c..c9fc6834225 100644 --- a/internal/dnsforward/upstreams.go +++ b/internal/dnsforward/upstreams.go @@ -136,18 +136,22 @@ func UpstreamHTTPVersions(http3 bool) (v []upstream.HTTPVersion) { // based on provided parameters. func setProxyUpstreamMode( conf *proxy.Config, - allServers bool, - fastestAddr bool, + upstreamMode UpstreamMode, fastestTimeout time.Duration, -) { - if allServers { +) (err error) { + switch upstreamMode { + case UpstreamModeParallel: conf.UpstreamMode = proxy.UModeParallel - } else if fastestAddr { + case UpstreamModeFastestAddr: conf.UpstreamMode = proxy.UModeFastestAddr conf.FastestPingTimeout = fastestTimeout - } else { + case UpstreamModeLoadBalance: conf.UpstreamMode = proxy.UModeLoadBalance + default: + return fmt.Errorf("unexpected value %q", upstreamMode) } + + return nil } // createBootstrap returns a bootstrap resolver based on the configuration of s. diff --git a/internal/home/config.go b/internal/home/config.go index 5ddcf2abeae..231e6521712 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -315,7 +315,7 @@ var config = &configuration{ RatelimitSubnetLenIPv4: 24, RatelimitSubnetLenIPv6: 56, RefuseAny: true, - AllServers: false, + UpstreamMode: dnsforward.UpstreamModeLoadBalance, HandleDDR: true, FastestTimeout: timeutil.Duration{ Duration: fastip.DefaultPingWaitTimeout, diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index 669662000d3..de87f6fae96 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -6,6 +6,13 @@ ## v0.107.44: API changes +### The field `"upstream_mode"` in `DNSConfig` + +* The field `"upstream_mode"` in `POST /control/dns_config` and + `GET /control/dns_info` now accepts `load_balance` value. Note that, the usage + of an empty string or field absence is considered to as deprecated and is not + recommended. Use `load_balance` instead. + ### Type correction in `Client` * Field `upstreams_cache_size` of object `Client` now correctly has type diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 781959ff771..300311c16a1 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1524,10 +1524,15 @@ 'cache_optimistic': 'type': 'boolean' 'upstream_mode': + 'type': 'string' 'enum': - - '' - - 'parallel' - - 'fastest_addr' + - const: '' + deprecated: true + description: Use `load_balance` instead. + - const: 'fastest_addr' + - const: 'load_balance' + - const: 'parallel' + 'description': Upstream modes enumeration. 'use_private_ptr_resolvers': 'type': 'boolean' 'resolve_clients':