safcm: forbid syncing groups which depend on "detected" groups

diff --git a/cmd/safcm/config/groups.go b/cmd/safcm/config/groups.go
index 7f7cb3f635c346663808e69e990aac020040b345..5af3be09990115740b0ed7f4e9c941b185b87477 100644 (file)
--- 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 9c0acda892585577510a26d3eab5640694fecfb3..57176079bc0782ebfc0a76fb2f59bc84d0a27391 100644 (file)
--- 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 e8f643154851a5fd91cd6b5647c9da84d73ddb4a..7fa2c936b637c7059e5429b273f310233ee92ed4 100644 (file)
--- 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 31252936e663861f18aa352766ab43479540caae..cf24d59801562f72641e4fc808074d2a5750e03b 100644 (file)
--- 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: <nil> 3 host groups: all group host1.example.org remove",
+                               "host1.example.org: <nil> 3 host groups: all group group3 host1.example.org remove",
                                "host1.example.org: <nil> 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 2e5b3ab95106710d84ff2bdcdbacdb891b22a0a7..80b1589072ace73792c3bd51723b02320d996c61 100644 (file)
--- 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 580afaf2bca37a642f544c8bef10377615fad608..6862e40d97f7528fb8345c9c725672dec5863a1f 100644 (file)
--- 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: