mirror of
https://github.com/moby/moby.git
synced 2026-01-11 18:51:37 +00:00
daemon: validate args in network.New*Filter
Filter-term validation does not belong in the API module. Clients should not be making any assumptions about which terms the daemon understands. Users should not need to upgrade their clients to use filter terms introduced in a newer daemon. Move the network filter validation from the api module into the daemon. Split network.NewFilter into network.NewListFilter and network.NewPruneFilter constructors which validate the filter terms, enforcing the invariant that any network.Filter is a well-formed filter for networks. The network route handlers have been leveraging a hidden 'idOrName' filter term that is not listed in the set of accepted filters and therefore not accepted in API client requests. And it's a good thing that it was never part of the API: it is completely broken and not fit for purpose! When a filter contains an idOrName term, the term values are ignored and instead the filter tests whether either the 'id' or 'name' terms match the Name of the network. Unless the filter contains both 'id' and 'name' terms, the match will evaluate to true for all networks! None of the daemon-internal users of 'idOrName' set either of those terms, therefore it has the same effect as if the filter did not contain the 'idOrName' term in the first place. Filtering networks by id-or-name is a quirky thing that the daemon needs to do to uphold its end of the Engine API contract, not something that would be of use to clients. Fixing up the idOrName filter would necessitate adding it to the list of accepted terms so the filter passes validaton, which would have the side effect of also making the filter available to API clients. Instead, add an exported field to the Filter struct so that daemon code can opt into the internal-only behaviour of having the 'id' term match on either the network Name or ID. Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit is contained in:
@@ -9,11 +9,33 @@ import (
|
||||
"github.com/pkg/errors"
|
||||
)
|
||||
|
||||
var (
|
||||
acceptedFilters = map[string]bool{
|
||||
"dangling": true,
|
||||
"driver": true,
|
||||
"id": true,
|
||||
"label": true,
|
||||
"name": true,
|
||||
"scope": true,
|
||||
"type": true,
|
||||
}
|
||||
|
||||
pruneFilters = map[string]bool{
|
||||
"label": true,
|
||||
"label!": true,
|
||||
"until": true,
|
||||
}
|
||||
)
|
||||
|
||||
type Filter struct {
|
||||
args filters.Args
|
||||
|
||||
filterByUse, danglingOnly bool
|
||||
until time.Time
|
||||
|
||||
// IDAlsoMatchesName makes the "id" filter term also match against
|
||||
// network names.
|
||||
IDAlsoMatchesName bool
|
||||
}
|
||||
|
||||
type FilterNetwork interface {
|
||||
@@ -27,11 +49,34 @@ type FilterNetwork interface {
|
||||
HasServiceAttachments() bool
|
||||
}
|
||||
|
||||
// NewFilter returns a network filter that filters by the provided args.
|
||||
// NewFilter returns a network list filter that filters by the provided args.
|
||||
//
|
||||
// An [errdefs.InvalidParameter] error is returned if the filter args are not
|
||||
// well-formed.
|
||||
func NewFilter(args filters.Args) (Filter, error) {
|
||||
if err := args.Validate(acceptedFilters); err != nil {
|
||||
return Filter{}, err
|
||||
}
|
||||
return newFilter(args)
|
||||
}
|
||||
|
||||
// NewPruneFilter returns a network prune filter that filters by the provided args.
|
||||
//
|
||||
// The filter matches dangling networks which also match args.
|
||||
func NewPruneFilter(args filters.Args) (Filter, error) {
|
||||
if err := args.Validate(pruneFilters); err != nil {
|
||||
return Filter{}, err
|
||||
}
|
||||
f, err := newFilter(args)
|
||||
if err != nil {
|
||||
return Filter{}, err
|
||||
}
|
||||
f.filterByUse = true
|
||||
f.danglingOnly = true
|
||||
return f, nil
|
||||
}
|
||||
|
||||
func newFilter(args filters.Args) (Filter, error) {
|
||||
var filterByUse, danglingOnly bool
|
||||
if values := args.Get("dangling"); len(values) > 0 {
|
||||
if len(values) > 1 {
|
||||
@@ -89,7 +134,8 @@ func (f Filter) Matches(nw FilterNetwork) bool {
|
||||
return false
|
||||
}
|
||||
if f.args.Contains("id") &&
|
||||
!f.args.Match("id", nw.ID()) {
|
||||
!f.args.Match("id", nw.ID()) &&
|
||||
(!f.IDAlsoMatchesName || !f.args.Match("id", nw.Name())) {
|
||||
return false
|
||||
}
|
||||
if f.args.Contains("label") &&
|
||||
@@ -104,11 +150,6 @@ func (f Filter) Matches(nw FilterNetwork) bool {
|
||||
!f.args.ExactMatch("scope", nw.Scope()) {
|
||||
return false
|
||||
}
|
||||
if f.args.Contains("idOrName") &&
|
||||
!f.args.Match("name", nw.Name()) &&
|
||||
!f.args.Match("id", nw.Name()) {
|
||||
return false
|
||||
}
|
||||
if f.filterByUse &&
|
||||
!matchesUse(f.danglingOnly, nw) {
|
||||
return false
|
||||
|
||||
@@ -129,8 +129,10 @@ func TestFilter(t *testing.T) {
|
||||
}
|
||||
|
||||
testCases := []struct {
|
||||
subtest string
|
||||
filter filters.Args
|
||||
subtest string
|
||||
filter filters.Args
|
||||
pruneFilter bool
|
||||
idAlsoMatchesName bool
|
||||
|
||||
err string
|
||||
results []string
|
||||
@@ -234,56 +236,127 @@ func TestFilter(t *testing.T) {
|
||||
subtest: "label=foo=baz",
|
||||
filter: filters.NewArgs(filters.Arg("label", "foo=baz")),
|
||||
},
|
||||
{
|
||||
subtest: "label!=foo",
|
||||
filter: filters.NewArgs(filters.Arg("label!", "foo")),
|
||||
err: "invalid filter 'label!'",
|
||||
},
|
||||
{
|
||||
subtest: "until=1h",
|
||||
filter: filters.NewArgs(filters.Arg("until", "1h")),
|
||||
err: "invalid filter 'until'",
|
||||
},
|
||||
{
|
||||
subtest: "name=with",
|
||||
filter: filters.NewArgs(filters.Arg("name", "with")),
|
||||
results: []string{"networkwithcontainer", "networkwithservice"},
|
||||
},
|
||||
{
|
||||
subtest: "id=with",
|
||||
subtest: "id=with/IDAlsoMatchesName=False",
|
||||
filter: filters.NewArgs(filters.Arg("id", "with")),
|
||||
},
|
||||
{
|
||||
subtest: "id=jbexjvgu",
|
||||
subtest: "id=with/IDAlsoMatchesName=True",
|
||||
filter: filters.NewArgs(filters.Arg("id", "with")),
|
||||
idAlsoMatchesName: true,
|
||||
results: []string{"networkwithcontainer", "networkwithservice"},
|
||||
},
|
||||
{
|
||||
subtest: "id=jbexjvgu/IDAlsoMatchesName=False",
|
||||
filter: filters.NewArgs(filters.Arg("id", "argjbex")),
|
||||
results: []string{"networkwithcontainer", "networkwithservice"},
|
||||
},
|
||||
{
|
||||
subtest: "id=my",
|
||||
subtest: "id=jbexjvgu/IDAlsoMatchesName=True",
|
||||
filter: filters.NewArgs(filters.Arg("id", "argjbex")),
|
||||
idAlsoMatchesName: true,
|
||||
results: []string{"networkwithcontainer", "networkwithservice"},
|
||||
},
|
||||
{
|
||||
subtest: "id=my/IDAlsoMatchesName=False",
|
||||
filter: filters.NewArgs(filters.Arg("id", "my")),
|
||||
results: []string{"idoverlap"},
|
||||
},
|
||||
{
|
||||
subtest: "label!=foo",
|
||||
filter: filters.NewArgs(filters.Arg("label!", "foo")),
|
||||
results: []string{network.NetworkHost, network.NetworkBridge, network.NetworkNone, "myoverlay", "mykvnet", "networkwithcontainer", "networkwithservice", "idoverlap"},
|
||||
subtest: "id=my/IDAlsoMatchesName=True",
|
||||
filter: filters.NewArgs(filters.Arg("id", "my")),
|
||||
idAlsoMatchesName: true,
|
||||
results: []string{"myoverlay", "mydrivernet", "mykvnet", "idoverlap"},
|
||||
},
|
||||
{
|
||||
subtest: "until=2025-01-01",
|
||||
filter: filters.NewArgs(filters.Arg("until", "2025-01-01")),
|
||||
results: []string{"myoverlay", "mydrivernet", "mykvnet"},
|
||||
subtest: "Prune/empty",
|
||||
filter: filters.NewArgs(),
|
||||
pruneFilter: true,
|
||||
// Prune filters have an implicit dangling=true
|
||||
results: []string{"myoverlay", "mydrivernet", "mykvnet", "idoverlap"},
|
||||
},
|
||||
{
|
||||
subtest: "until=2024-12-01T01:00:00",
|
||||
filter: filters.NewArgs(filters.Arg("until", "2024-12-01T01:00:00")),
|
||||
results: []string{"myoverlay"},
|
||||
subtest: "Prune/label=foo",
|
||||
filter: filters.NewArgs(filters.Arg("label", "foo")),
|
||||
pruneFilter: true,
|
||||
results: []string{"mydrivernet"},
|
||||
},
|
||||
{
|
||||
subtest: "MultipleTerms=until",
|
||||
filter: filters.NewArgs(filters.Arg("until", "2024-12-01T01:00:00"), filters.Arg("until", "2h")),
|
||||
err: "more than one until filter specified",
|
||||
subtest: "Prune/label=foo=bar",
|
||||
filter: filters.NewArgs(filters.Arg("label", "foo=bar")),
|
||||
pruneFilter: true,
|
||||
results: []string{"mydrivernet"},
|
||||
},
|
||||
{
|
||||
subtest: "Prune/label=foo=baz",
|
||||
filter: filters.NewArgs(filters.Arg("label", "foo=baz")),
|
||||
pruneFilter: true,
|
||||
},
|
||||
{
|
||||
subtest: "Prune/label!=foo",
|
||||
filter: filters.NewArgs(filters.Arg("label!", "foo")),
|
||||
pruneFilter: true,
|
||||
results: []string{"myoverlay", "mykvnet", "idoverlap"},
|
||||
},
|
||||
{
|
||||
subtest: "Prune/until=2025-01-01",
|
||||
filter: filters.NewArgs(filters.Arg("until", "2025-01-01")),
|
||||
pruneFilter: true,
|
||||
results: []string{"myoverlay", "mydrivernet", "mykvnet"},
|
||||
},
|
||||
{
|
||||
subtest: "Prune/until=2024-12-01T01:00:00",
|
||||
filter: filters.NewArgs(filters.Arg("until", "2024-12-01T01:00:00")),
|
||||
pruneFilter: true,
|
||||
results: []string{"myoverlay"},
|
||||
},
|
||||
{
|
||||
subtest: "Prune/MultipleTerms=until",
|
||||
filter: filters.NewArgs(filters.Arg("until", "2024-12-01T01:00:00"), filters.Arg("until", "2h")),
|
||||
pruneFilter: true,
|
||||
err: "more than one until filter specified",
|
||||
},
|
||||
{
|
||||
subtest: "Prune/id=invalid",
|
||||
filter: filters.NewArgs(filters.Arg("id", "invalid")),
|
||||
pruneFilter: true,
|
||||
err: "invalid filter 'id'",
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range testCases {
|
||||
t.Run(testCase.subtest, func(t *testing.T) {
|
||||
flt, err := NewFilter(testCase.filter)
|
||||
var (
|
||||
flt Filter
|
||||
err error
|
||||
)
|
||||
if testCase.pruneFilter {
|
||||
flt, err = NewPruneFilter(testCase.filter)
|
||||
} else {
|
||||
flt, err = NewFilter(testCase.filter)
|
||||
}
|
||||
if testCase.err != "" {
|
||||
assert.ErrorContains(t, err, testCase.err)
|
||||
return
|
||||
}
|
||||
assert.NilError(t, err)
|
||||
|
||||
flt.IDAlsoMatchesName = testCase.idAlsoMatchesName
|
||||
got := map[string]bool{}
|
||||
for _, nw := range networks {
|
||||
if flt.Matches(nw) {
|
||||
|
||||
Reference in New Issue
Block a user