From 37ccae4e923a7e688e79a135b0e49a746e9b2a06 Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Wed, 21 Aug 2024 20:23:58 +0300 Subject: [PATCH] all: imp code --- internal/dnsforward/dnsforward.go | 2 +- internal/dnsforward/ipset.go | 54 ++++++++++++--------- internal/ipset/ipset.go | 14 +++--- internal/ipset/ipset_linux.go | 25 ++++++---- internal/ipset/ipset_linux_internal_test.go | 6 +-- internal/ipset/ipset_others.go | 4 +- 6 files changed, 59 insertions(+), 46 deletions(-) diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index 997925f2..30d7e731 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -616,7 +616,7 @@ func (s *Server) prepareInternalDNS() (err error) { } ipsetLogger := s.logger.With(slogutil.KeyPrefix, "ipset") - s.ipset, err = newIpsetHandler(ipsetLogger, ipsetList) + s.ipset, err = newIpsetHandler(context.TODO(), ipsetLogger, ipsetList) if err != nil { // Don't wrap the error, because it's informative enough as is. return err diff --git a/internal/dnsforward/ipset.go b/internal/dnsforward/ipset.go index f3c414a5..fecb27e6 100644 --- a/internal/dnsforward/ipset.go +++ b/internal/dnsforward/ipset.go @@ -22,14 +22,19 @@ type ipsetHandler struct { // newIpsetHandler returns a new initialized [ipsetHandler]. It is not safe for // concurrent use. c is always non-nil for [Server.Close]. -func newIpsetHandler(logger *slog.Logger, ipsetList []string) (c *ipsetHandler, err error) { - c = &ipsetHandler{ +func newIpsetHandler( + ctx context.Context, + logger *slog.Logger, + ipsetList []string, +) (h *ipsetHandler, err error) { + h = &ipsetHandler{ logger: logger, } - c.ipsetMgr, err = ipset.NewManager(&ipset.Config{ - Logger: logger, - IpsetList: ipsetList, - }) + conf := &ipset.Config{ + Logger: logger, + Lines: ipsetList, + } + h.ipsetMgr, err = ipset.NewManager(ctx, conf) if errors.Is(err, os.ErrInvalid) || errors.Is(err, os.ErrPermission) || errors.Is(err, errors.ErrUnsupported) { @@ -41,26 +46,27 @@ func newIpsetHandler(logger *slog.Logger, ipsetList []string) (c *ipsetHandler, // // TODO(a.garipov): The Snap problem can probably be solved if we add // the netlink-connector interface plug. - logger.Warn("cannot initialize", slogutil.KeyError, err) + logger.WarnContext(ctx, "cannot initialize", slogutil.KeyError, err) - return c, nil + return h, nil } else if err != nil { - return c, fmt.Errorf("initializing ipset: %w", err) + return h, fmt.Errorf("initializing ipset: %w", err) } - return c, nil + return h, nil } // close closes the Linux Netfilter connections. -func (c *ipsetHandler) close() (err error) { - if c.ipsetMgr != nil { - return c.ipsetMgr.Close() +func (h *ipsetHandler) close() (err error) { + if h.ipsetMgr != nil { + return h.ipsetMgr.Close() } return nil } -func (c *ipsetHandler) dctxIsfilled(dctx *dnsContext) (ok bool) { +// dctxIsFilled returns true if dctx has enough information to process. +func dctxIsFilled(dctx *dnsContext) (ok bool) { return dctx != nil && dctx.responseFromUpstream && dctx.proxyCtx != nil && @@ -71,8 +77,8 @@ func (c *ipsetHandler) dctxIsfilled(dctx *dnsContext) (ok bool) { // skipIpsetProcessing returns true when the ipset processing can be skipped for // this request. -func (c *ipsetHandler) skipIpsetProcessing(dctx *dnsContext) (ok bool) { - if c == nil || c.ipsetMgr == nil || !c.dctxIsfilled(dctx) { +func (h *ipsetHandler) skipIpsetProcessing(dctx *dnsContext) (ok bool) { + if h == nil || h.ipsetMgr == nil || !dctxIsFilled(dctx) { return true } @@ -114,13 +120,13 @@ func ipsFromAnswer(ans []dns.RR) (ip4s, ip6s []net.IP) { } // process adds the resolved IP addresses to the domain's ipsets, if any. -func (c *ipsetHandler) process(dctx *dnsContext) (rc resultCode) { - c.logger.Debug("started processing") - defer c.logger.Debug("finished processing") - +func (h *ipsetHandler) process(dctx *dnsContext) (rc resultCode) { + // TODO(s.chzhen): Use passed context. ctx := context.TODO() + h.logger.DebugContext(ctx, "started processing") + defer h.logger.DebugContext(ctx, "finished processing") - if c.skipIpsetProcessing(dctx) { + if h.skipIpsetProcessing(dctx) { return resultCodeSuccess } @@ -130,15 +136,15 @@ func (c *ipsetHandler) process(dctx *dnsContext) (rc resultCode) { host = strings.ToLower(host) ip4s, ip6s := ipsFromAnswer(dctx.proxyCtx.Res.Answer) - n, err := c.ipsetMgr.Add(ctx, host, ip4s, ip6s) + n, err := h.ipsetMgr.Add(ctx, host, ip4s, ip6s) if err != nil { // Consider ipset errors non-critical to the request. - c.logger.ErrorContext(ctx, "adding host ips", slogutil.KeyError, err) + h.logger.ErrorContext(ctx, "adding host ips", slogutil.KeyError, err) return resultCodeSuccess } - c.logger.DebugContext(ctx, "added new ipset entries", "num", n) + h.logger.DebugContext(ctx, "added new ipset entries", "num", n) return resultCodeSuccess } diff --git a/internal/ipset/ipset.go b/internal/ipset/ipset.go index 5adfc3a6..839497df 100644 --- a/internal/ipset/ipset.go +++ b/internal/ipset/ipset.go @@ -22,23 +22,23 @@ type Config struct { // not be nil. Logger *slog.Logger - // IpsetList is the ipset configuration with the following syntax: + // Lines is the ipset configuration with the following syntax: // // DOMAIN[,DOMAIN].../IPSET_NAME[,IPSET_NAME]... // - // IpsetList must not contain any blank lines or comments. - IpsetList []string + // Lines must not contain any blank lines or comments. + Lines []string } // NewManager returns a new ipset manager. IPv4 addresses are added to an ipset // with an ipv4 family; IPv6 addresses, to an ipv6 ipset. ipset must exist. // -// If conf.IpsetList is empty, mgr and err are nil. The error's chain contains +// If conf.Lines is empty, mgr and err are nil. The error's chain contains // [errors.ErrUnsupported] if current OS is not supported. -func NewManager(conf *Config) (mgr Manager, err error) { - if len(conf.IpsetList) == 0 { +func NewManager(ctx context.Context, conf *Config) (mgr Manager, err error) { + if len(conf.Lines) == 0 { return nil, nil } - return newManager(conf) + return newManager(ctx, conf) } diff --git a/internal/ipset/ipset_linux.go b/internal/ipset/ipset_linux.go index a8e992b8..1d15428e 100644 --- a/internal/ipset/ipset_linux.go +++ b/internal/ipset/ipset_linux.go @@ -36,8 +36,8 @@ import ( // resolved IP addresses. // newManager returns a new Linux ipset manager. -func newManager(conf *Config) (set Manager, err error) { - return newManagerWithDialer(conf, defaultDial) +func newManager(ctx context.Context, conf *Config) (set Manager, err error) { + return newManagerWithDialer(ctx, conf, defaultDial) } // defaultDial is the default netfilter dialing function. @@ -258,7 +258,7 @@ func parseIpsetConfigLine(confStr string) (hosts, ipsetNames []string, err error // parseIpsetConfig parses the ipset configuration and stores ipsets. It // returns an error if the configuration can't be used. -func (m *manager) parseIpsetConfig(ipsetConf []string) (err error) { +func (m *manager) parseIpsetConfig(ctx context.Context, ipsetConf []string) (err error) { // The family doesn't seem to matter when we use a header query, so query // only the IPv4 one. // @@ -282,7 +282,7 @@ func (m *manager) parseIpsetConfig(ipsetConf []string) (err error) { } var ipsets []props - ipsets, err = m.ipsets(ipsetNames, currentlyKnown) + ipsets, err = m.ipsets(ctx, ipsetNames, currentlyKnown) if err != nil { return fmt.Errorf("getting ipsets from config line at idx %d: %w", i, err) } @@ -332,7 +332,11 @@ func (m *manager) ipsetProps(name string) (p props, err error) { // ipsets returns ipset properties of currently known ipsets. It also makes an // additional ipset header data query if needed. -func (m *manager) ipsets(names []string, currentlyKnown map[string]props) (sets []props, err error) { +func (m *manager) ipsets( + ctx context.Context, + names []string, + currentlyKnown map[string]props, +) (sets []props, err error) { for _, n := range names { p, ok := currentlyKnown[n] if !ok { @@ -340,7 +344,8 @@ func (m *manager) ipsets(names []string, currentlyKnown map[string]props) (sets } if p.family != netfilter.ProtoIPv4 && p.family != netfilter.ProtoIPv6 { - m.logger.Debug( + m.logger.DebugContext( + ctx, "got unexpected ipset family while getting set properties", "set_name", p.name, "set_type", p.typeName, @@ -362,7 +367,7 @@ func (m *manager) ipsets(names []string, currentlyKnown map[string]props) (sets // newManagerWithDialer returns a new Linux ipset manager using the provided // dialer. -func newManagerWithDialer(conf *Config, dial dialer) (mgr Manager, err error) { +func newManagerWithDialer(ctx context.Context, conf *Config, dial dialer) (mgr Manager, err error) { defer func() { err = errors.Annotate(err, "ipset: %w") }() m := &manager{ @@ -383,7 +388,7 @@ func newManagerWithDialer(conf *Config, dial dialer) (mgr Manager, err error) { if errors.Is(err, unix.EPROTONOSUPPORT) { // The implementation doesn't support this protocol version. Just // issue a warning. - m.logger.Warn("dialing netfilter", slogutil.KeyError, err) + m.logger.WarnContext(ctx, "dialing netfilter", slogutil.KeyError, err) return nil, nil } @@ -391,12 +396,12 @@ func newManagerWithDialer(conf *Config, dial dialer) (mgr Manager, err error) { return nil, fmt.Errorf("dialing netfilter: %w", err) } - err = m.parseIpsetConfig(conf.IpsetList) + err = m.parseIpsetConfig(ctx, conf.Lines) if err != nil { return nil, fmt.Errorf("getting ipsets: %w", err) } - m.logger.Debug("initialized") + m.logger.DebugContext(ctx, "initialized") return m, nil } diff --git a/internal/ipset/ipset_linux_internal_test.go b/internal/ipset/ipset_linux_internal_test.go index f3805e85..632f0d0f 100644 --- a/internal/ipset/ipset_linux_internal_test.go +++ b/internal/ipset/ipset_linux_internal_test.go @@ -96,10 +96,10 @@ func TestManager_Add(t *testing.T) { } conf := &Config{ - Logger: slogutil.NewDiscardLogger(), - IpsetList: ipsetList, + Logger: slogutil.NewDiscardLogger(), + Lines: ipsetList, } - m, err := newManagerWithDialer(conf, fakeDial) + m, err := newManagerWithDialer(testutil.ContextWithTimeout(t, testTimeout), conf, fakeDial) require.NoError(t, err) ip4 := net.IP{1, 2, 3, 4} diff --git a/internal/ipset/ipset_others.go b/internal/ipset/ipset_others.go index 577fd319..29038b9d 100644 --- a/internal/ipset/ipset_others.go +++ b/internal/ipset/ipset_others.go @@ -3,9 +3,11 @@ package ipset import ( + "context" + "github.com/AdguardTeam/AdGuardHome/internal/aghos" ) -func newManager(_ *Config) (mgr Manager, err error) { +func newManager(_ context.Context, _ *Config) (mgr Manager, err error) { return nil, aghos.Unsupported("ipset") }