Closed ycao56 closed 2 years ago
This PR only generates policyset. Placement generation will be a separate PR.
/hold for review
Thanks! Looks great so far! I still need to look at the code itself, but I had some comments about the configuration (using camel case and the name of the
generatePlacement
field) that I thought might be helpful to have in an isolated discussion before I start looking at other things in depth.
Updated. Please take another look.
It looks like this will work well with my sample policy sets. Thanks
@ycao56 this is what I was thinking. It reduces the time complexity, reduces the amount of custom code, and is easier to read in my opinion.
diff --git a/internal/plugin.go b/internal/plugin.go
index 01b2ad4..f308938 100644
--- a/internal/plugin.go
+++ b/internal/plugin.go
@@ -272,6 +272,26 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
p.PolicyDefaults.Standards = defaults.Standards
}
+ // Generate temporary sets to later merge the policy sets declared in p.Policies[*] and p.PolicySets
+ plcsetToPlc := make(map[string]map[string]bool)
+ plcToPlcset := make(map[string]map[string]bool)
+
+ for _, plcset := range p.PolicySets {
+ if plcsetToPlc[plcset.Name] == nil {
+ plcsetToPlc[plcset.Name] = make(map[string]bool)
+ }
+
+ for _, plc := range plcset.Policies {
+ plcsetToPlc[plcset.Name][plc] = true
+
+ if plcToPlcset[plc] == nil {
+ plcToPlcset[plc] = make(map[string]bool)
+ }
+
+ plcToPlcset[plc][plcset.Name] = true
+ }
+ }
+
for i := range p.Policies {
policy := &p.Policies[i]
if policy.Categories == nil {
@@ -368,44 +388,47 @@ func (p *Plugin) applyDefaults(unmarshaledConfig map[string]interface{}) {
}
}
- // Merge policySets defined in p.policies and p.policyDefaults with p.PolicySets
for _, plcsetInPlc := range policy.PolicySets {
- if len(p.PolicySets) == 0 {
+ if _, ok := plcsetToPlc[plcsetInPlc]; !ok {
newPlcset := types.PolicySetConfig{
Name: plcsetInPlc,
Policies: []string{policy.Name},
}
- p.PolicySets = appendPolicySetIfUnique(p.PolicySets, newPlcset)
- continue
+ p.PolicySets = append(p.PolicySets, newPlcset)
}
- for i, plcsetName := range p.PolicySets {
- if plcsetInPlc == plcsetName.Name {
- // policyset already defined, add policy to policyset
- p.PolicySets[i].Policies = appendIfUnique(p.PolicySets[i].Policies, policy.Name)
- } else {
- // policyset not defined, generate a new policyset and add policy to it
- newPlcset := types.PolicySetConfig{
- Name: plcsetInPlc,
- Policies: []string{policy.Name},
- }
- p.PolicySets = appendPolicySetIfUnique(p.PolicySets, newPlcset)
- }
+
+ if plcToPlcset[policy.Name] == nil {
+ plcToPlcset[policy.Name] = make(map[string]bool)
}
+
+ plcToPlcset[policy.Name][plcsetInPlc] = true
+
+ if plcsetToPlc[plcsetInPlc] == nil {
+ plcsetToPlc[plcsetInPlc] = make(map[string]bool)
+ }
+
+ plcsetToPlc[plcsetInPlc][policy.Name] = true
+ }
+
+ policy.PolicySets = make([]string, 0, len(plcToPlcset[policy.Name]))
+
+ for plcset := range plcToPlcset[policy.Name] {
+ policy.PolicySets = append(policy.PolicySets, plcset)
}
}
- // loop through p.PolicySets and add policyset to p.policies[*].policySets if missing
- // this makes sure p.policies[*].policySets and p.PolicySets are in sync
+ // Sync up the declared policy sets in p.Policies[*]
for i := range p.PolicySets {
- for _, plc := range p.PolicySets[i].Policies {
- for j, plcDef := range p.Policies {
- if plcDef.Name == plc {
- // policyset is missing from p.policies[*].policySets, add it
- p.Policies[j].PolicySets = appendIfUnique(plcDef.PolicySets, p.PolicySets[i].Name)
- }
- }
+ plcset := &p.PolicySets[i]
+ plcset.Policies = make([]string, 0, len(plcsetToPlc[plcset.Name]))
+
+ for plc := range plcsetToPlc[plcset.Name] {
+ plcset.Policies = append(plcset.Policies, plc)
}
+
+ // Sort alphabetically to make it deterministic
+ sort.Strings(plcset.Policies)
}
}
diff --git a/internal/utils.go b/internal/utils.go
index b2d424c..d6b0214 100644
--- a/internal/utils.go
+++ b/internal/utils.go
@@ -313,40 +313,3 @@ func verifyManifestPath(baseDirectory string, manifestPath string) error {
return nil
}
-
-// appendIfUnique appends a string element list only it is unique.
-func appendIfUnique(a []string, b ...string) (c []string) {
- c = a
- for i := range b {
- unique := true
- for j := range a {
- if a[j] == b[i] {
- unique = false
-
- break
- }
- }
- if unique {
- c = append(c, b[i])
- }
- }
-
- return c
-}
-
-// appendPolicySetIfUnique appends a policyset only it is unique.
-func appendPolicySetIfUnique(a []types.PolicySetConfig, b types.PolicySetConfig) []types.PolicySetConfig {
- unique := true
- for _, item := range a {
- if item.Name == b.Name {
- unique = false
-
- break
- }
- }
- if unique {
- return append(a, b)
- }
-
- return a
-}
diff --git a/internal/utils_test.go b/internal/utils_test.go
index 20dc52e..61b76ec 100644
--- a/internal/utils_test.go
+++ b/internal/utils_test.go
@@ -781,68 +781,3 @@ func TestVerifyManifestPath(t *testing.T) {
)
}
}
-
-func TestAppendIfUnique(t *testing.T) {
- t.Parallel()
- a := []string{"aaa", "bbb", "ccc"}
- b := "ccc"
- expected := []string{"aaa", "bbb", "ccc"}
- assertReflectEqual(t, appendIfUnique(a, b), expected)
- b = "aaa"
- assertReflectEqual(t, appendIfUnique(a, b), expected)
- b = "ddd"
- expected = []string{"aaa", "bbb", "ccc", "ddd"}
- assertReflectEqual(t, appendIfUnique(a, b), expected)
-}
-
-func TestAppendPolicysetIfUnique(t *testing.T) {
- t.Parallel()
- a := []types.PolicySetConfig{
- {
- Name: "policyset1",
- },
- {
- Name: "policyset2",
- },
- {
- Name: "policyset3",
- },
- }
- b := types.PolicySetConfig{
- Name: "policyset3",
- }
- expected := []types.PolicySetConfig{
- {
- Name: "policyset1",
- },
- {
- Name: "policyset2",
- },
- {
- Name: "policyset3",
- },
- }
- assertReflectEqual(t, appendPolicySetIfUnique(a, b), expected)
- b = types.PolicySetConfig{
- Name: "policyset1",
- }
- assertReflectEqual(t, appendPolicySetIfUnique(a, b), expected)
- b = types.PolicySetConfig{
- Name: "policyset4",
- }
- expected = []types.PolicySetConfig{
- {
- Name: "policyset1",
- },
- {
- Name: "policyset2",
- },
- {
- Name: "policyset3",
- },
- {
- Name: "policyset4",
- },
- }
- assertReflectEqual(t, appendPolicySetIfUnique(a, b), expected)
-}
@mprahl Gotcha. Thanks for the help! Updated.
@mprahl All issues have been addressed.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: mprahl, ycao56
The full list of commands accepted by this bot can be found here.
The pull request process is described here
/unhold
https://github.com/stolostron/backlog/issues/19412 Implement logic to generate policysets. There are three ways to generate
Signed-off-by: Yu Cao ycao@redhat.com