Pull request 2244: AG-27492-client-storage-usage

Squashed commit of the following:

commit 46956d0f5a36fbcd2324125bcc146bdb57cb3f7e
Merge: 85ccad786 3993f4c47
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Jul 1 16:11:59 2024 +0300

    Merge branch 'master' into AG-27492-client-storage-usage

commit 85ccad7862a35cbebc935776ac057f6bb6cfc75f
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Jun 28 18:05:37 2024 +0300

    all: imp docs

commit e7043efcda32635b4260344f11b5a61b5120a9d2
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Jun 27 20:10:26 2024 +0300

    all: client storage usage
This commit is contained in:
Stanislav Chzhen
2024-07-01 17:34:47 +03:00
parent 3993f4c476
commit 9d1c45fd94
11 changed files with 255 additions and 312 deletions

View File

@@ -19,7 +19,6 @@ import (
"github.com/AdguardTeam/AdGuardHome/internal/whois"
"github.com/AdguardTeam/dnsproxy/proxy"
"github.com/AdguardTeam/dnsproxy/upstream"
"github.com/AdguardTeam/golibs/container"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/hostsfile"
"github.com/AdguardTeam/golibs/log"
@@ -45,14 +44,12 @@ type DHCP interface {
// clientsContainer is the storage of all runtime and persistent clients.
type clientsContainer struct {
// clientIndex stores information about persistent clients.
clientIndex *client.Index
// storage stores information about persistent clients.
storage *client.Storage
// runtimeIndex stores information about runtime clients.
runtimeIndex *client.RuntimeIndex
allTags *container.MapSet[string]
// dhcp is the DHCP service implementation.
dhcp DHCP
@@ -104,15 +101,15 @@ func (clients *clientsContainer) Init(
filteringConf *filtering.Config,
) (err error) {
// TODO(s.chzhen): Refactor it.
if clients.clientIndex != nil {
if clients.storage != nil {
return errors.Error("clients container already initialized")
}
clients.runtimeIndex = client.NewRuntimeIndex()
clients.clientIndex = client.NewIndex()
clients.allTags = container.NewMapSet(clientTags...)
clients.storage = client.NewStorage(&client.Config{
AllowedTags: clientTags,
})
// TODO(e.burkov): Use [dhcpsvc] implementation when it's ready.
clients.dhcp = dhcpServer
@@ -217,7 +214,6 @@ type clientObject struct {
// toPersistent returns an initialized persistent client if there are no errors.
func (o *clientObject) toPersistent(
filteringConf *filtering.Config,
allTags *container.MapSet[string],
) (cli *client.Persistent, err error) {
cli = &client.Persistent{
Name: o.Name,
@@ -274,7 +270,7 @@ func (o *clientObject) toPersistent(
cli.BlockedServices = o.BlockedServices.Clone()
cli.SetTags(o.Tags, allTags)
cli.Tags = slices.Clone(o.Tags)
return cli, nil
}
@@ -287,22 +283,14 @@ func (clients *clientsContainer) addFromConfig(
) (err error) {
for i, o := range objects {
var cli *client.Persistent
cli, err = o.toPersistent(filteringConf, clients.allTags)
cli, err = o.toPersistent(filteringConf)
if err != nil {
return fmt.Errorf("clients: init persistent client at index %d: %w", i, err)
}
// TODO(s.chzhen): Consider moving to the client index constructor.
err = clients.clientIndex.ClashesUID(cli)
err = clients.storage.Add(cli)
if err != nil {
return fmt.Errorf("adding client %s at index %d: %w", cli.Name, i, err)
}
err = clients.add(cli)
if err != nil {
// TODO(s.chzhen): Return an error instead of logging if more
// stringent requirements are implemented.
log.Error("clients: adding client %s at index %d: %s", cli.Name, i, err)
return fmt.Errorf("adding client %q at index %d: %w", cli.Name, i, err)
}
}
@@ -315,8 +303,8 @@ func (clients *clientsContainer) forConfig() (objs []*clientObject) {
clients.lock.Lock()
defer clients.lock.Unlock()
objs = make([]*clientObject, 0, clients.clientIndex.Size())
clients.clientIndex.RangeByName(func(cli *client.Persistent) (cont bool) {
objs = make([]*clientObject, 0, clients.storage.Size())
clients.storage.RangeByName(func(cli *client.Persistent) (cont bool) {
objs = append(objs, &clientObject{
Name: cli.Name,
@@ -360,7 +348,7 @@ func (clients *clientsContainer) periodicUpdate() {
// clientSource checks if client with this IP address already exists and returns
// the source which updated it last. It returns [client.SourceNone] if the
// client doesn't exist.
// client doesn't exist. Note that it is only used in tests.
func (clients *clientsContainer) clientSource(ip netip.Addr) (src client.Source) {
clients.lock.Lock()
defer clients.lock.Unlock()
@@ -419,12 +407,8 @@ func (clients *clientsContainer) clientOrArtificial(
}
}()
cli, ok := clients.find(id)
if !ok {
cli = clients.clientIndex.FindByIPWithoutZone(ip)
}
if cli != nil {
cli, ok := clients.storage.FindLoose(ip, id)
if ok {
return &querylog.Client{
Name: cli.Name,
IgnoreQueryLog: cli.IgnoreQueryLog,
@@ -456,7 +440,7 @@ func (clients *clientsContainer) find(id string) (c *client.Persistent, ok bool)
return nil, false
}
return c.ShallowClone(), true
return c, true
}
// shouldCountClient is a wrapper around [clientsContainer.find] to make it a
@@ -530,7 +514,7 @@ func (clients *clientsContainer) UpstreamConfigByID(
// findLocked searches for a client by its ID. clients.lock is expected to be
// locked.
func (clients *clientsContainer) findLocked(id string) (c *client.Persistent, ok bool) {
c, ok = clients.clientIndex.Find(id)
c, ok = clients.storage.Find(id)
if ok {
return c, true
}
@@ -552,7 +536,7 @@ func (clients *clientsContainer) findDHCP(ip netip.Addr) (c *client.Persistent,
return nil, false
}
return clients.clientIndex.FindByMAC(foundMAC)
return clients.storage.FindByMAC(foundMAC)
}
// runtimeClient returns a runtime client from internal index. Note that it
@@ -586,114 +570,6 @@ func (clients *clientsContainer) findRuntimeClient(ip netip.Addr) (rc *client.Ru
return rc
}
// check validates the client. It also sorts the client tags.
func (clients *clientsContainer) check(c *client.Persistent) (err error) {
switch {
case c == nil:
return errors.Error("client is nil")
case c.Name == "":
return errors.Error("invalid name")
case c.IDsLen() == 0:
return errors.Error("id required")
default:
// Go on.
}
for _, t := range c.Tags {
if !clients.allTags.Has(t) {
return fmt.Errorf("invalid tag: %q", t)
}
}
// TODO(s.chzhen): Move to the constructor.
slices.Sort(c.Tags)
_, err = proxy.ParseUpstreamsConfig(c.Upstreams, &upstream.Options{})
if err != nil {
return fmt.Errorf("invalid upstream servers: %w", err)
}
return nil
}
// add adds a persistent client or returns an error.
func (clients *clientsContainer) add(c *client.Persistent) (err error) {
err = clients.check(c)
if err != nil {
// Don't wrap the error since it's informative enough as is.
return err
}
clients.lock.Lock()
defer clients.lock.Unlock()
err = clients.clientIndex.Clashes(c)
if err != nil {
// Don't wrap the error since it's informative enough as is.
return err
}
clients.addLocked(c)
log.Debug("clients: added %q: ID:%q [%d]", c.Name, c.IDs(), clients.clientIndex.Size())
return nil
}
// addLocked c to the indexes. clients.lock is expected to be locked.
func (clients *clientsContainer) addLocked(c *client.Persistent) {
clients.clientIndex.Add(c)
}
// remove removes a client. ok is false if there is no such client.
func (clients *clientsContainer) remove(name string) (ok bool) {
clients.lock.Lock()
defer clients.lock.Unlock()
c, ok := clients.clientIndex.FindByName(name)
if !ok {
return false
}
clients.removeLocked(c)
return true
}
// removeLocked removes c from the indexes. clients.lock is expected to be
// locked.
func (clients *clientsContainer) removeLocked(c *client.Persistent) {
if err := c.CloseUpstreams(); err != nil {
log.Error("client container: removing client %s: %s", c.Name, err)
}
// Update the ID index.
clients.clientIndex.Delete(c)
}
// update updates a client by its name.
func (clients *clientsContainer) update(prev, c *client.Persistent) (err error) {
err = clients.check(c)
if err != nil {
// Don't wrap the error since it's informative enough as is.
return err
}
clients.lock.Lock()
defer clients.lock.Unlock()
err = clients.clientIndex.Clashes(c)
if err != nil {
// Don't wrap the error since it's informative enough as is.
return err
}
clients.removeLocked(prev)
clients.addLocked(c)
return nil
}
// setWHOISInfo sets the WHOIS information for a client. clients.lock is
// expected to be locked.
func (clients *clientsContainer) setWHOISInfo(ip netip.Addr, wi *whois.Info) {
@@ -855,5 +731,5 @@ func (clients *clientsContainer) addFromSystemARP() {
// close gracefully closes all the client-specific upstream configurations of
// the persistent clients.
func (clients *clientsContainer) close() (err error) {
return clients.clientIndex.CloseUpstreams()
return clients.storage.CloseUpstreams()
}

View File

@@ -72,7 +72,7 @@ func TestClients(t *testing.T) {
IPs: []netip.Addr{cli1IP, cliIPv6},
}
err := clients.add(c)
err := clients.storage.Add(c)
require.NoError(t, err)
c = &client.Persistent{
@@ -81,7 +81,7 @@ func TestClients(t *testing.T) {
IPs: []netip.Addr{cli2IP},
}
err = clients.add(c)
err = clients.storage.Add(c)
require.NoError(t, err)
c, ok := clients.find(cli1)
@@ -107,7 +107,7 @@ func TestClients(t *testing.T) {
})
t.Run("add_fail_name", func(t *testing.T) {
err := clients.add(&client.Persistent{
err := clients.storage.Add(&client.Persistent{
Name: "client1",
UID: client.MustNewUID(),
IPs: []netip.Addr{netip.MustParseAddr("1.2.3.5")},
@@ -116,7 +116,7 @@ func TestClients(t *testing.T) {
})
t.Run("add_fail_ip", func(t *testing.T) {
err := clients.add(&client.Persistent{
err := clients.storage.Add(&client.Persistent{
Name: "client3",
UID: client.MustNewUID(),
})
@@ -124,7 +124,7 @@ func TestClients(t *testing.T) {
})
t.Run("update_fail_ip", func(t *testing.T) {
err := clients.update(&client.Persistent{Name: "client1"}, &client.Persistent{
err := clients.storage.Update("client1", &client.Persistent{
Name: "client1",
UID: client.MustNewUID(),
})
@@ -139,11 +139,11 @@ func TestClients(t *testing.T) {
cliNewIP = netip.MustParseAddr(cliNew)
)
prev, ok := clients.clientIndex.FindByName("client1")
prev, ok := clients.storage.FindByName("client1")
require.True(t, ok)
require.NotNil(t, prev)
err := clients.update(prev, &client.Persistent{
err := clients.storage.Update("client1", &client.Persistent{
Name: "client1",
UID: prev.UID,
IPs: []netip.Addr{cliNewIP},
@@ -155,11 +155,11 @@ func TestClients(t *testing.T) {
assert.Equal(t, clients.clientSource(cliNewIP), client.SourcePersistent)
prev, ok = clients.clientIndex.FindByName("client1")
prev, ok = clients.storage.FindByName("client1")
require.True(t, ok)
require.NotNil(t, prev)
err = clients.update(prev, &client.Persistent{
err = clients.storage.Update("client1", &client.Persistent{
Name: "client1-renamed",
UID: prev.UID,
IPs: []netip.Addr{cliNewIP},
@@ -173,7 +173,7 @@ func TestClients(t *testing.T) {
assert.Equal(t, "client1-renamed", c.Name)
assert.True(t, c.UseOwnSettings)
nilCli, ok := clients.clientIndex.FindByName("client1")
nilCli, ok := clients.storage.FindByName("client1")
require.False(t, ok)
assert.Nil(t, nilCli)
@@ -184,7 +184,7 @@ func TestClients(t *testing.T) {
})
t.Run("del_success", func(t *testing.T) {
ok := clients.remove("client1-renamed")
ok := clients.storage.RemoveByName("client1-renamed")
require.True(t, ok)
_, ok = clients.find("1.1.1.2")
@@ -192,7 +192,7 @@ func TestClients(t *testing.T) {
})
t.Run("del_fail", func(t *testing.T) {
ok := clients.remove("client3")
ok := clients.storage.RemoveByName("client3")
assert.False(t, ok)
})
@@ -261,7 +261,7 @@ func TestClientsWHOIS(t *testing.T) {
t.Run("can't_set_manually-added", func(t *testing.T) {
ip := netip.MustParseAddr("1.1.1.2")
err := clients.add(&client.Persistent{
err := clients.storage.Add(&client.Persistent{
Name: "client1",
UID: client.MustNewUID(),
IPs: []netip.Addr{netip.MustParseAddr("1.1.1.2")},
@@ -272,7 +272,7 @@ func TestClientsWHOIS(t *testing.T) {
rc := clients.runtimeIndex.Client(ip)
require.Nil(t, rc)
assert.True(t, clients.remove("client1"))
assert.True(t, clients.storage.RemoveByName("client1"))
})
}
@@ -283,7 +283,7 @@ func TestClientsAddExisting(t *testing.T) {
ip := netip.MustParseAddr("1.1.1.1")
// Add a client.
err := clients.add(&client.Persistent{
err := clients.storage.Add(&client.Persistent{
Name: "client1",
UID: client.MustNewUID(),
IPs: []netip.Addr{ip, netip.MustParseAddr("1:2:3::4")},
@@ -333,7 +333,7 @@ func TestClientsAddExisting(t *testing.T) {
require.NoError(t, err)
// Add a new client with the same IP as for a client with MAC.
err = clients.add(&client.Persistent{
err = clients.storage.Add(&client.Persistent{
Name: "client2",
UID: client.MustNewUID(),
IPs: []netip.Addr{ip},
@@ -341,7 +341,7 @@ func TestClientsAddExisting(t *testing.T) {
require.NoError(t, err)
// Add a new client with the IP from the first client's IP range.
err = clients.add(&client.Persistent{
err = clients.storage.Add(&client.Persistent{
Name: "client3",
UID: client.MustNewUID(),
IPs: []netip.Addr{netip.MustParseAddr("2.2.2.2")},
@@ -354,7 +354,7 @@ func TestClientsCustomUpstream(t *testing.T) {
clients := newClientsContainer(t)
// Add client with upstreams.
err := clients.add(&client.Persistent{
err := clients.storage.Add(&client.Persistent{
Name: "client1",
UID: client.MustNewUID(),
IPs: []netip.Addr{netip.MustParseAddr("1.1.1.1"), netip.MustParseAddr("1:2:3::4")},

View File

@@ -96,7 +96,7 @@ func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http
clients.lock.Lock()
defer clients.lock.Unlock()
clients.clientIndex.Range(func(c *client.Persistent) (cont bool) {
clients.storage.RangeByName(func(c *client.Persistent) (cont bool) {
cj := clientToJSON(c)
data.Clients = append(data.Clients, cj)
@@ -336,7 +336,7 @@ func (clients *clientsContainer) handleAddClient(w http.ResponseWriter, r *http.
return
}
err = clients.add(c)
err = clients.storage.Add(c)
if err != nil {
aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
@@ -364,7 +364,7 @@ func (clients *clientsContainer) handleDelClient(w http.ResponseWriter, r *http.
return
}
if !clients.remove(cj.Name) {
if !clients.storage.RemoveByName(cj.Name) {
aghhttp.Error(r, w, http.StatusBadRequest, "Client not found")
return
@@ -399,30 +399,14 @@ func (clients *clientsContainer) handleUpdateClient(w http.ResponseWriter, r *ht
return
}
var prev *client.Persistent
var ok bool
func() {
clients.lock.Lock()
defer clients.lock.Unlock()
prev, ok = clients.clientIndex.FindByName(dj.Name)
}()
if !ok {
aghhttp.Error(r, w, http.StatusBadRequest, "client not found")
return
}
c, err := clients.jsonToClient(dj.Data, prev)
c, err := clients.jsonToClient(dj.Data, nil)
if err != nil {
aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
return
}
err = clients.update(prev, c)
err = clients.storage.Update(dj.Name, c)
if err != nil {
aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)

View File

@@ -198,11 +198,11 @@ func TestClientsContainer_HandleDelClient(t *testing.T) {
clients := newClientsContainer(t)
clientOne := newPersistentClientWithIDs(t, "client1", []string{testClientIP1})
err := clients.add(clientOne)
err := clients.storage.Add(clientOne)
require.NoError(t, err)
clientTwo := newPersistentClientWithIDs(t, "client2", []string{testClientIP2})
err = clients.add(clientTwo)
err = clients.storage.Add(clientTwo)
require.NoError(t, err)
assertPersistentClients(t, clients, []*client.Persistent{clientOne, clientTwo})
@@ -260,7 +260,7 @@ func TestClientsContainer_HandleUpdateClient(t *testing.T) {
clients := newClientsContainer(t)
clientOne := newPersistentClientWithIDs(t, "client1", []string{testClientIP1})
err := clients.add(clientOne)
err := clients.storage.Add(clientOne)
require.NoError(t, err)
assertPersistentClients(t, clients, []*client.Persistent{clientOne})
@@ -342,11 +342,11 @@ func TestClientsContainer_HandleFindClient(t *testing.T) {
}
clientOne := newPersistentClientWithIDs(t, "client1", []string{testClientIP1})
err := clients.add(clientOne)
err := clients.storage.Add(clientOne)
require.NoError(t, err)
clientTwo := newPersistentClientWithIDs(t, "client2", []string{testClientIP2})
err = clients.add(clientTwo)
err = clients.storage.Add(clientTwo)
require.NoError(t, err)
assertPersistentClients(t, clients, []*client.Persistent{clientOne, clientTwo})

View File

@@ -13,17 +13,21 @@ import (
var testIPv4 = netip.AddrFrom4([4]byte{1, 2, 3, 4})
// newIDIndex is a helper function that returns a client index filled with
// persistent clients from the m. It also generates a UID for each client.
func newIDIndex(m []*client.Persistent) (ci *client.Index) {
ci = client.NewIndex()
// newStorage is a helper function that returns a client storage filled with
// persistent clients. It also generates a UID for each client.
func newStorage(tb testing.TB, clients []*client.Persistent) (s *client.Storage) {
tb.Helper()
for _, c := range m {
c.UID = client.MustNewUID()
ci.Add(c)
s = client.NewStorage(&client.Config{
AllowedTags: nil,
})
for _, p := range clients {
p.UID = client.MustNewUID()
require.NoError(tb, s.Add(p))
}
return ci
return s
}
func TestApplyAdditionalFiltering(t *testing.T) {
@@ -36,7 +40,8 @@ func TestApplyAdditionalFiltering(t *testing.T) {
}, nil)
require.NoError(t, err)
Context.clients.clientIndex = newIDIndex([]*client.Persistent{{
Context.clients.storage = newStorage(t, []*client.Persistent{{
Name: "default",
ClientIDs: []string{"default"},
UseOwnSettings: false,
SafeSearchConf: filtering.SafeSearchConfig{Enabled: false},
@@ -44,6 +49,7 @@ func TestApplyAdditionalFiltering(t *testing.T) {
SafeBrowsingEnabled: false,
ParentalEnabled: false,
}, {
Name: "custom_filtering",
ClientIDs: []string{"custom_filtering"},
UseOwnSettings: true,
SafeSearchConf: filtering.SafeSearchConfig{Enabled: true},
@@ -51,6 +57,7 @@ func TestApplyAdditionalFiltering(t *testing.T) {
SafeBrowsingEnabled: true,
ParentalEnabled: true,
}, {
Name: "partial_custom_filtering",
ClientIDs: []string{"partial_custom_filtering"},
UseOwnSettings: true,
SafeSearchConf: filtering.SafeSearchConfig{Enabled: true},
@@ -121,16 +128,19 @@ func TestApplyAdditionalFiltering_blockedServices(t *testing.T) {
}, nil)
require.NoError(t, err)
Context.clients.clientIndex = newIDIndex([]*client.Persistent{{
Context.clients.storage = newStorage(t, []*client.Persistent{{
Name: "default",
ClientIDs: []string{"default"},
UseOwnBlockedServices: false,
}, {
Name: "no_services",
ClientIDs: []string{"no_services"},
BlockedServices: &filtering.BlockedServices{
Schedule: schedule.EmptyWeekly(),
},
UseOwnBlockedServices: true,
}, {
Name: "services",
ClientIDs: []string{"services"},
BlockedServices: &filtering.BlockedServices{
Schedule: schedule.EmptyWeekly(),
@@ -138,6 +148,7 @@ func TestApplyAdditionalFiltering_blockedServices(t *testing.T) {
},
UseOwnBlockedServices: true,
}, {
Name: "invalid_services",
ClientIDs: []string{"invalid_services"},
BlockedServices: &filtering.BlockedServices{
Schedule: schedule.EmptyWeekly(),
@@ -145,6 +156,7 @@ func TestApplyAdditionalFiltering_blockedServices(t *testing.T) {
},
UseOwnBlockedServices: true,
}, {
Name: "allow_all",
ClientIDs: []string{"allow_all"},
BlockedServices: &filtering.BlockedServices{
Schedule: schedule.FullWeekly(),