From 4473e968425319e6beae558643bb047a6b01c17a Mon Sep 17 00:00:00 2001 From: Simon Ruderich Date: Tue, 13 Apr 2021 08:04:03 +0200 Subject: [PATCH] safcm: forbid syncing groups which depend on "detected" groups --- cmd/safcm/config/groups.go | 39 +++++++ cmd/safcm/config/groups_test.go | 151 +++++++++++++++++++++++++ cmd/safcm/sync.go | 18 ++- cmd/safcm/sync_sync_test.go | 4 +- cmd/safcm/sync_test.go | 27 ++++- cmd/safcm/testdata/project/groups.yaml | 5 + 6 files changed, 237 insertions(+), 7 deletions(-) diff --git a/cmd/safcm/config/groups.go b/cmd/safcm/config/groups.go index 7f7cb3f..5af3be0 100644 --- a/cmd/safcm/config/groups.go +++ b/cmd/safcm/config/groups.go @@ -186,3 +186,42 @@ func ResolveHostGroups(host string, sort.Strings(res) return res, nil } + +// TransitivelyDetectedGroups returns all groups which depend on "detected" +// groups, either directly or by depending on groups which transitively depend +// on "detected" groups. +func TransitivelyDetectedGroups(groups map[string][]string) []string { + work := make(map[string][]string) + for k, v := range groups { + work[k] = v + } + + // Mark all groups which contain "detected" groups as long as new + // (transitive) "detected" groups are found. + detected := make(map[string]bool) + for { + change := false + for group, members := range work { + for _, x := range members { + if !detected[x] && !strings.HasPrefix(x, + GroupDetectedPrefix) { + continue + } + detected[strings.TrimSuffix(group, + GroupRemoveSuffix)] = true + delete(work, group) + change = true + } + } + if !change { + break + } + } + + var res []string + for x := range detected { + res = append(res, x) + } + sort.Strings(res) + return res +} diff --git a/cmd/safcm/config/groups_test.go b/cmd/safcm/config/groups_test.go index 9c0acda..5717607 100644 --- a/cmd/safcm/config/groups_test.go +++ b/cmd/safcm/config/groups_test.go @@ -77,6 +77,12 @@ func TestLoadGroups(t *testing.T) { "group2:remove": { "remove", }, + "group3": { + "host1.example.org", + }, + "group3:remove": { + "host2", + }, "all_except_some": { "all", }, @@ -233,6 +239,7 @@ func TestResolveHostGroups(t *testing.T) { []string{ "all", "group", + "group3", "host1.example.org", "remove", }, @@ -282,6 +289,7 @@ func TestResolveHostGroups(t *testing.T) { []string{ "all", "detected_mips", + "group3", "host1.example.org", "remove", }, @@ -312,3 +320,146 @@ func TestResolveHostGroups(t *testing.T) { }) } } + +func TestTransitivelyDetectedGroups(t *testing.T) { + tests := []struct { + name string + groups map[string][]string + exp []string + }{ + + { + "no detected", + map[string][]string{ + "group-a": { + "a", + "b", + "group-b", + }, + "group-a:remove": { + "d", + }, + "group-b": { + "c", + "d", + }, + }, + nil, + }, + + { + "detected as direct member", + map[string][]string{ + "group-a": { + "a", + "b", + "detected_foo", + }, + "group-b": { + "c", + "d", + }, + }, + []string{ + "group-a", + }, + }, + + { + "detected as direct :remove member", + map[string][]string{ + "group-a": { + "a", + "b", + "group-b", + }, + "group-a:remove": { + "d", + "detected_foo", + }, + "group-b": { + "c", + "d", + }, + }, + []string{ + "group-a", + }, + }, + + { + "detected as transitive member", + map[string][]string{ + "group-a": { + "group-b", + }, + "group-b": { + "group-c", + }, + "group-c": { + "group-d", + "detected_bar", + }, + "group-d": { + "group-e", + }, + "group-e": { + "detected_foo", + }, + "group-f": { + "a", + "b", + }, + }, + []string{ + "group-a", + "group-b", + "group-c", + "group-d", + "group-e", + }, + }, + + { + "detected as transitive :remove member", + map[string][]string{ + "group-a": { + "group-b", + }, + "group-b": { + "group-c", + }, + "group-c": { + "group-d", + }, + "group-d": { + "group-e", + }, + "group-e": { + "all", + }, + "group-e:remove": { + "detected_foo", + }, + "group-f": { + "a", + "b", + }, + }, + []string{ + "group-a", + "group-b", + "group-c", + "group-d", + "group-e", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + res := TransitivelyDetectedGroups(tc.groups) + testutil.AssertEqual(t, "res", res, tc.exp) + }) + } +} diff --git a/cmd/safcm/sync.go b/cmd/safcm/sync.go index e8f6431..7fa2c93 100644 --- a/cmd/safcm/sync.go +++ b/cmd/safcm/sync.go @@ -186,8 +186,24 @@ func MainSync(args []string) error { func hostsToSync(names []string, allHosts *config.Hosts, allGroups map[string][]string) ([]*config.Host, error) { + detectedMap := make(map[string]bool) + for _, x := range config.TransitivelyDetectedGroups(allGroups) { + detectedMap[x] = true + } + + const detectedErr = ` + +Groups depending on "detected" groups cannot be used to select hosts as these +are only available after the hosts were contacted. +` + nameMap := make(map[string]bool) for _, x := range names { + if detectedMap[x] { + return nil, fmt.Errorf( + "group %q depends on \"detected\" groups%s", + x, detectedErr) + } nameMap[x] = true } nameMatched := make(map[string]bool) @@ -203,8 +219,6 @@ func hostsToSync(names []string, allHosts *config.Hosts, nameMatched[host.Name] = true } - // TODO: don't permit groups which contain "detected" groups - // because these are not available yet groups, err := config.ResolveHostGroups(host.Name, allGroups, nil) if err != nil { diff --git a/cmd/safcm/sync_sync_test.go b/cmd/safcm/sync_sync_test.go index 3125293..cf24d59 100644 --- a/cmd/safcm/sync_sync_test.go +++ b/cmd/safcm/sync_sync_test.go @@ -59,6 +59,7 @@ func TestHostSyncReq(t *testing.T) { Groups: []string{ "all", "group", + "group3", "remove", "host1.example.org", }, @@ -141,7 +142,7 @@ func TestHostSyncReq(t *testing.T) { }, }, []string{ - "host1.example.org: 3 host groups: all group host1.example.org remove", + "host1.example.org: 3 host groups: all group group3 host1.example.org remove", "host1.example.org: 3 host group priorities (desc. order): host1.example.org", }, nil, @@ -157,6 +158,7 @@ func TestHostSyncReq(t *testing.T) { Groups: []string{ "all", "group", + "group3", "remove", "host1.example.org", }, diff --git a/cmd/safcm/sync_test.go b/cmd/safcm/sync_test.go index 2e5b3ab..80b1589 100644 --- a/cmd/safcm/sync_test.go +++ b/cmd/safcm/sync_test.go @@ -44,6 +44,12 @@ func TestHostsToSync(t *testing.T) { t.Fatal(err) } + const errMsg = ` + +Groups depending on "detected" groups cannot be used to select hosts as these +are only available after the hosts were contacted. +` + tests := []struct { name string names []string @@ -104,7 +110,7 @@ func TestHostsToSync(t *testing.T) { { "group: single name", - []string{"group"}, + []string{"group3"}, []*config.Host{ allHosts.Map["host1.example.org"], }, @@ -112,7 +118,7 @@ func TestHostsToSync(t *testing.T) { }, { "group: multiple names", - []string{"group", "group2"}, + []string{"group3", "group2"}, []*config.Host{ allHosts.Map["host1.example.org"], allHosts.Map["host2"], @@ -121,7 +127,7 @@ func TestHostsToSync(t *testing.T) { }, { "group: multiple identical names", - []string{"group", "group2", "group"}, + []string{"group3", "group2", "group3"}, []*config.Host{ allHosts.Map["host1.example.org"], allHosts.Map["host2"], @@ -130,7 +136,7 @@ func TestHostsToSync(t *testing.T) { }, { "group: multiple names, including unknown", - []string{"group", "group2", "unknown-group"}, + []string{"group3", "group2", "unknown-group"}, nil, fmt.Errorf("hosts/groups not found: \"unknown-group\""), }, @@ -145,6 +151,19 @@ func TestHostsToSync(t *testing.T) { nil, }, + { + "group: single name (detected)", + []string{"group"}, + nil, + fmt.Errorf(`group "group" depends on "detected" groups` + errMsg), + }, + { + "group: multiple names (detected)", + []string{"group", "group2"}, + nil, + fmt.Errorf(`group "group" depends on "detected" groups` + errMsg), + }, + { "\"all\" and name", []string{"all", "group2"}, diff --git a/cmd/safcm/testdata/project/groups.yaml b/cmd/safcm/testdata/project/groups.yaml index 580afaf..6862e40 100644 --- a/cmd/safcm/testdata/project/groups.yaml +++ b/cmd/safcm/testdata/project/groups.yaml @@ -11,6 +11,11 @@ group2: group2:remove: - remove +group3: + - host1.example.org +group3:remove: + - host2 + all_except_some: - all all_except_some:remove: -- 2.45.2