Pull request 1883: 951-blocked-services-client-schedule

Updates #951. Squashed commit of the following: commit 94e4766 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Jun 27 17:21:41 2023 +0300 chlog: upd docs commit b4022c3 Merge: cfa24ff e7e6384 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Jun 27 16:33:20 2023 +0300 Merge branch 'master' into 951-blocked-services-client-schedule commit cfa24ff Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Jun 27 15:04:10 2023 +0300 chlog: imp docs commit dad2759 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Jun 26 17:38:08 2023 +0300 home: imp err msg commit 7d9ba98 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Jun 26 16:58:00 2023 +0300 all: add tests commit 8e952fc Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Jun 23 16:36:10 2023 +0300 schedule: add todo commit 723573a Merge: 2151ab2 e54fc9b Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Jun 23 11:40:03 2023 +0300 Merge branch 'master' into 951-blocked-services-client-schedule commit 2151ab2 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Jun 23 11:37:49 2023 +0300 all: add tests commit 81ab341 Merge: aa7ae41 66345e8 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Thu Jun 22 17:59:01 2023 +0300 Merge branch 'master' into 951-blocked-services-client-schedule commit aa7ae41 Merge: 304389a 06d465b Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Jun 21 17:10:11 2023 +0300 Merge branch 'master' into 951-blocked-services-client-schedule commit 304389a Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Wed Jun 21 17:05:31 2023 +0300 home: imp err msg commit 29cfc7a Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Jun 20 20:42:59 2023 +0300 all: imp err handling commit 8543868 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Jun 20 18:21:50 2023 +0300 all: upd chlog commit c5b614d Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Tue Jun 20 14:37:47 2023 +0300 all: add blocked services schedule
AdguardTeam · Jun 27, 2023 · d881813 · d881813
1 parent e7e6384
commit d881813
Show file tree

Hide file tree

Showing 14 changed files with 418 additions and 71 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,18 +27,52 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 - The new command-line flag `--web-addr` is the address to serve the web UI on,
   in the host:port format.
-- The ability to set inactivity periods for filtering blocked services in the
-  configuration file ([#951]).  The UI changes are coming in the upcoming
-  releases.
+- The ability to set inactivity periods for filtering blocked services, both
+  globally and per client, in the configuration file ([#951]).  The UI changes
+  are coming in the upcoming releases.
 - The ability to edit rewrite rules via `PUT /control/rewrite/update` HTTP API
   and the Web UI ([#1577]).
 
 ### Changed
 
 #### Configuration Changes
 
-In this release, the schema version has changed from 20 to 21.
+In this release, the schema version has changed from 20 to 22.
+
+- Property `clients.persistent.blocked_services`, which in schema versions 21
+  and earlier used to be a list containing ids of blocked services, is now an
+  object containing ids and schedule for blocked services:
+
+  ```yaml
+  # BEFORE:
+  'clients':
+    'persistent':
+      - 'name': 'client-name'
+        'blocked_services':
+        - id_1
+        - id_2
+
+  # AFTER:
+  'clients':
+    'persistent':
+    - 'name': client-name
+      'blocked_services':
+        'ids':
+        - id_1
+        - id_2
+      'schedule':
+        'time_zone': 'Local'
+        'sun':
+          'start': '0s'
+          'end': '24h'
+        'mon':
+          'start': '1h'
+          'end': '23h'
+  ```
 
+  To rollback this change, replace `clients.persistent.blocked_services` object
+  with the list of ids of blocked services and change the `schema_version` back
+  to `21`.
 - Property `dns.blocked_services`, which in schema versions 20 and earlier used
   to be a list containing ids of blocked services, is now an object containing
   ids and schedule for blocked services:

diff --git a/internal/filtering/blocked.go b/internal/filtering/blocked.go
@@ -2,6 +2,7 @@ package filtering
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"time"
 
@@ -55,11 +56,29 @@ type BlockedServices struct {
 	IDs []string `yaml:"ids"`
 }
 
-// BlockedSvcKnown returns true if a blocked service ID is known.
-func BlockedSvcKnown(s string) (ok bool) {
-	_, ok = serviceRules[s]
+// Clone returns a deep copy of blocked services.
+func (s *BlockedServices) Clone() (c *BlockedServices) {
+	if s == nil {
+		return nil
+	}
+
+	return &BlockedServices{
+		Schedule: s.Schedule.Clone(),
+		IDs:      slices.Clone(s.IDs),
+	}
+}
+
+// Validate returns an error if blocked services contain unknown service ID.  s
+// must not be nil.
+func (s *BlockedServices) Validate() (err error) {
+	for _, id := range s.IDs {
+		_, ok := serviceRules[id]
+		if !ok {
+			return fmt.Errorf("unknown blocked-service %q", id)
+		}
+	}
 
-	return ok
+	return nil
 }
 
 // ApplyBlockedServices - set blocked services settings for this DNS request

diff --git a/internal/filtering/filtering.go b/internal/filtering/filtering.go
@@ -988,17 +988,11 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
 	}
 
 	if d.BlockedServices != nil {
-		bsvcs := []string{}
-		for _, s := range d.BlockedServices.IDs {
-			if !BlockedSvcKnown(s) {
-				log.Debug("skipping unknown blocked-service %q", s)
+		err = d.BlockedServices.Validate()
 
-				continue
-			}
-
-			bsvcs = append(bsvcs, s)
+		if err != nil {
+			return nil, fmt.Errorf("filtering: %w", err)
 		}
-		d.BlockedServices.IDs = bsvcs
 	}
 
 	if blockFilters != nil {

diff --git a/internal/home/client.go b/internal/home/client.go
@@ -23,12 +23,14 @@ type Client struct {
 	safeSearchConf filtering.SafeSearchConfig
 	SafeSearch     filtering.SafeSearch
 
+	// BlockedServices is the configuration of blocked services of a client.
+	BlockedServices *filtering.BlockedServices
+
 	Name string
 
-	IDs             []string
-	Tags            []string
-	BlockedServices []string
-	Upstreams       []string
+	IDs       []string
+	Tags      []string
+	Upstreams []string
 
 	UseOwnSettings        bool
 	FilteringEnabled      bool
@@ -44,9 +46,9 @@ type Client struct {
 func (c *Client) ShallowClone() (sh *Client) {
 	clone := *c
 
+	clone.BlockedServices = c.BlockedServices.Clone()
 	clone.IDs = stringutil.CloneSlice(c.IDs)
 	clone.Tags = stringutil.CloneSlice(c.Tags)
-	clone.BlockedServices = stringutil.CloneSlice(c.BlockedServices)
 	clone.Upstreams = stringutil.CloneSlice(c.Upstreams)
 
 	return &clone

diff --git a/internal/home/clients.go b/internal/home/clients.go
@@ -96,7 +96,7 @@ func (clients *clientsContainer) Init(
 	etcHosts *aghnet.HostsContainer,
 	arpdb aghnet.ARPDB,
 	filteringConf *filtering.Config,
-) {
+) (err error) {
 	if clients.list != nil {
 		log.Fatal("clients.list != nil")
 	}
@@ -110,13 +110,17 @@ func (clients *clientsContainer) Init(
 	clients.dhcpServer = dhcpServer
 	clients.etcHosts = etcHosts
 	clients.arpdb = arpdb
-	clients.addFromConfig(objects, filteringConf)
+	err = clients.addFromConfig(objects, filteringConf)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		return err
+	}
 
 	clients.safeSearchCacheSize = filteringConf.SafeSearchCacheSize
 	clients.safeSearchCacheTTL = time.Minute * time.Duration(filteringConf.CacheTime)
 
 	if clients.testing {
-		return
+		return nil
 	}
 
 	if clients.dhcpServer != nil {
@@ -127,6 +131,8 @@ func (clients *clientsContainer) Init(
 	if clients.etcHosts != nil {
 		go clients.handleHostsUpdates()
 	}
+
+	return nil
 }
 
 func (clients *clientsContainer) handleHostsUpdates() {
@@ -166,12 +172,14 @@ func (clients *clientsContainer) reloadARP() {
 type clientObject struct {
 	SafeSearchConf filtering.SafeSearchConfig `yaml:"safe_search"`
 
+	// BlockedServices is the configuration of blocked services of a client.
+	BlockedServices *filtering.BlockedServices `yaml:"blocked_services"`
+
 	Name string `yaml:"name"`
 
-	Tags            []string `yaml:"tags"`
-	IDs             []string `yaml:"ids"`
-	BlockedServices []string `yaml:"blocked_services"`
-	Upstreams       []string `yaml:"upstreams"`
+	IDs       []string `yaml:"ids"`
+	Tags      []string `yaml:"tags"`
+	Upstreams []string `yaml:"upstreams"`
 
 	UseGlobalSettings        bool `yaml:"use_global_settings"`
 	FilteringEnabled         bool `yaml:"filtering_enabled"`
@@ -185,7 +193,10 @@ type clientObject struct {
 
 // addFromConfig initializes the clients container with objects from the
 // configuration file.
-func (clients *clientsContainer) addFromConfig(objects []*clientObject, filteringConf *filtering.Config) {
+func (clients *clientsContainer) addFromConfig(
+	objects []*clientObject,
+	filteringConf *filtering.Config,
+) (err error) {
 	for _, o := range objects {
 		cli := &Client{
 			Name: o.Name,
@@ -206,7 +217,7 @@ func (clients *clientsContainer) addFromConfig(objects []*clientObject, filterin
 		if o.SafeSearchConf.Enabled {
 			o.SafeSearchConf.CustomResolver = safeSearchResolver{}
 
-			err := cli.setSafeSearch(
+			err = cli.setSafeSearch(
 				o.SafeSearchConf,
 				filteringConf.SafeSearchCacheSize,
 				time.Minute*time.Duration(filteringConf.CacheTime),
@@ -218,14 +229,13 @@ func (clients *clientsContainer) addFromConfig(objects []*clientObject, filterin
 			}
 		}
 
-		for _, s := range o.BlockedServices {
-			if filtering.BlockedSvcKnown(s) {
-				cli.BlockedServices = append(cli.BlockedServices, s)
-			} else {
-				log.Info("clients: skipping unknown blocked service %q", s)
-			}
+		err = o.BlockedServices.Validate()
+		if err != nil {
+			return fmt.Errorf("clients: init client blocked services %q: %w", cli.Name, err)
 		}
 
+		cli.BlockedServices = o.BlockedServices.Clone()
+
 		for _, t := range o.Tags {
 			if clients.allTags.Has(t) {
 				cli.Tags = append(cli.Tags, t)
@@ -236,11 +246,13 @@ func (clients *clientsContainer) addFromConfig(objects []*clientObject, filterin
 
 		slices.Sort(cli.Tags)
 
-		_, err := clients.Add(cli)
+		_, err = clients.Add(cli)
 		if err != nil {
 			log.Error("clients: adding clients %s: %s", cli.Name, err)
 		}
 	}
+
+	return nil
 }
 
 // forConfig returns all currently known persistent clients as objects for the
@@ -254,10 +266,11 @@ func (clients *clientsContainer) forConfig() (objs []*clientObject) {
 		o := &clientObject{
 			Name: cli.Name,
 
-			Tags:            stringutil.CloneSlice(cli.Tags),
-			IDs:             stringutil.CloneSlice(cli.IDs),
-			BlockedServices: stringutil.CloneSlice(cli.BlockedServices),
-			Upstreams:       stringutil.CloneSlice(cli.Upstreams),
+			BlockedServices: cli.BlockedServices.Clone(),
+
+			IDs:       stringutil.CloneSlice(cli.IDs),
+			Tags:      stringutil.CloneSlice(cli.Tags),
+			Upstreams: stringutil.CloneSlice(cli.Upstreams),
 
 			UseGlobalSettings:        !cli.UseOwnSettings,
 			FilteringEnabled:         cli.FilteringEnabled,

diff --git a/internal/home/clients_test.go b/internal/home/clients_test.go
@@ -16,18 +16,19 @@ import (
 
 // newClientsContainer is a helper that creates a new clients container for
 // tests.
-func newClientsContainer() (c *clientsContainer) {
+func newClientsContainer(t *testing.T) (c *clientsContainer) {
 	c = &clientsContainer{
 		testing: true,
 	}
 
-	c.Init(nil, nil, nil, nil, &filtering.Config{})
+	err := c.Init(nil, nil, nil, nil, &filtering.Config{})
+	require.NoError(t, err)
 
 	return c
 }
 
 func TestClients(t *testing.T) {
-	clients := newClientsContainer()
+	clients := newClientsContainer(t)
 
 	t.Run("add_success", func(t *testing.T) {
 		var (
@@ -198,7 +199,7 @@ func TestClients(t *testing.T) {
 }
 
 func TestClientsWHOIS(t *testing.T) {
-	clients := newClientsContainer()
+	clients := newClientsContainer(t)
 	whois := &whois.Info{
 		Country: "AU",
 		Orgname: "Example Org",
@@ -244,7 +245,7 @@ func TestClientsWHOIS(t *testing.T) {
 }
 
 func TestClientsAddExisting(t *testing.T) {
-	clients := newClientsContainer()
+	clients := newClientsContainer(t)
 
 	t.Run("simple", func(t *testing.T) {
 		ip := netip.MustParseAddr("1.1.1.1")
@@ -316,7 +317,7 @@ func TestClientsAddExisting(t *testing.T) {
 }
 
 func TestClientsCustomUpstream(t *testing.T) {
-	clients := newClientsContainer()
+	clients := newClientsContainer(t)
 
 	// Add client with upstreams.
 	ok, err := clients.Add(&Client{

diff --git a/internal/home/clientshttp.go b/internal/home/clientshttp.go
@@ -123,10 +123,14 @@ func (clients *clientsContainer) jsonToClient(cj clientJSON, prev *Client) (c *C
 
 		Name: cj.Name,
 
-		IDs:             cj.IDs,
-		Tags:            cj.Tags,
-		BlockedServices: cj.BlockedServices,
-		Upstreams:       cj.Upstreams,
+		BlockedServices: &filtering.BlockedServices{
+			Schedule: prev.BlockedServices.Schedule.Clone(),
+			IDs:      cj.BlockedServices,
+		},
+
+		IDs:       cj.IDs,
+		Tags:      cj.Tags,
+		Upstreams: cj.Upstreams,
 
 		UseOwnSettings:        !cj.UseGlobalSettings,
 		FilteringEnabled:      cj.FilteringEnabled,
@@ -180,7 +184,8 @@ func clientToJSON(c *Client) (cj *clientJSON) {
 		SafeBrowsingEnabled: c.SafeBrowsingEnabled,
 
 		UseGlobalBlockedServices: !c.UseOwnBlockedServices,
-		BlockedServices:          c.BlockedServices,
+
+		BlockedServices: c.BlockedServices.IDs,
 
 		Upstreams: c.Upstreams,
 

diff --git a/internal/home/dns.go b/internal/home/dns.go
@@ -474,9 +474,11 @@ func applyAdditionalFiltering(clientIP net.IP, clientID string, setts *filtering
 	if c.UseOwnBlockedServices {
 		// TODO(e.burkov):  Get rid of this crutch.
 		setts.ServicesRules = nil
-		svcs := c.BlockedServices
-		Context.filters.ApplyBlockedServicesList(setts, svcs)
-		log.Debug("%s: services for client %q set: %s", pref, c.Name, svcs)
+		svcs := c.BlockedServices.IDs
+		if !c.BlockedServices.Schedule.Contains(time.Now()) {
+			Context.filters.ApplyBlockedServicesList(setts, svcs)
+			log.Debug("%s: services for client %q set: %s", pref, c.Name, svcs)
+		}
 	}
 
 	setts.ClientName = c.Name