Skip to content

Commit 3b33cb9

Browse files
author
Yu Cao
authored
Ensure policyset name is unique (#36)
Fix the issue that policyset name could be duplicated. Add test cases for policyset config validation. Signed-off-by: Yu Cao <[email protected]>
1 parent b0d41ad commit 3b33cb9

File tree

3 files changed

+228
-14
lines changed

3 files changed

+228
-14
lines changed

‎internal/plugin.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ func (p *Plugin) assertValidConfig() error {
482482
return errors.New("policies is empty but it must be set")
483483
}
484484

485-
seen := map[string]bool{}
485+
seenPlc := map[string]bool{}
486486
plCount := struct {
487487
plc int
488488
plr int
@@ -495,12 +495,12 @@ func (p *Plugin) assertValidConfig() error {
495495
)
496496
}
497497

498-
if seen[policy.Name] {
498+
if seenPlc[policy.Name] {
499499
return fmt.Errorf(
500500
"each policy must have a unique name set, but found a duplicate name: %s", policy.Name,
501501
)
502502
}
503-
seen[policy.Name] = true
503+
seenPlc[policy.Name] = true
504504

505505
if len(p.PolicyDefaults.Namespace+"."+policy.Name) > maxObjectNameLength {
506506
return fmt.Errorf("the policy namespace and name cannot be more than 63 characters: %s.%s",
@@ -587,8 +587,8 @@ func (p *Plugin) assertValidConfig() error {
587587
}
588588
}
589589

590+
seenPlcset := map[string]bool{}
590591
for i := range p.PolicySets {
591-
seen := map[string]bool{}
592592
plcset := &p.PolicySets[i]
593593

594594
if plcset.Name == "" {
@@ -597,12 +597,12 @@ func (p *Plugin) assertValidConfig() error {
597597
)
598598
}
599599

600-
if seen[plcset.Name] {
600+
if seenPlcset[plcset.Name] {
601601
return fmt.Errorf(
602602
"each policySet must have a unique name set, but found a duplicate name: %s", plcset.Name,
603603
)
604604
}
605-
seen[plcset.Name] = true
605+
seenPlcset[plcset.Name] = true
606606

607607
// Validate policy Placement settings
608608
if plcset.Placement.PlacementRulePath != "" && plcset.Placement.PlacementPath != "" {
@@ -660,7 +660,7 @@ func (p *Plugin) assertValidConfig() error {
660660
// Validate only one type of placement kind is in use
661661
if plCount.plc != 0 && plCount.plr != 0 {
662662
return fmt.Errorf(
663-
"may not use a mix of Placement and PlacementRule for policies; found %d Placement and %d PlacementRule",
663+
"may not use a mix of Placement and PlacementRule for policies and policysets; found %d Placement and %d PlacementRule",
664664
plCount.plc, plCount.plr,
665665
)
666666
}

‎internal/plugin_config_test.go‎

Lines changed: 214 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"io/ioutil"
77
"path"
8+
"path/filepath"
89
"testing"
910

1011
"github.com/stolostron/policy-generator-plugin/internal/types"
@@ -582,7 +583,7 @@ policies:
582583
}
583584

584585
expected := "may not use a mix of Placement and PlacementRule for " +
585-
"policies; found 1 Placement and 1 PlacementRule"
586+
"policies and policysets; found 1 Placement and 1 PlacementRule"
586587
assertEqual(t, err.Error(), expected)
587588
}
588589

@@ -766,3 +767,215 @@ policies:
766767
expected := fmt.Sprintf("could not read the placement rule path %s", plrPath)
767768
assertEqual(t, err.Error(), expected)
768769
}
770+
771+
func TestPolicySetConfig(t *testing.T) {
772+
t.Parallel()
773+
tmpDir := t.TempDir()
774+
createConfigMap(t, tmpDir, "configmap.yaml")
775+
776+
testCases := []testCase{
777+
{
778+
name: "policySet must have a name set",
779+
setupFunc: func(p *Plugin) {
780+
p.PolicySets = []types.PolicySetConfig{
781+
{
782+
Placement: types.PlacementConfig{
783+
Name: "policyset-placement",
784+
ClusterSelectors: map[string]string{"my": "app"},
785+
},
786+
},
787+
}
788+
},
789+
expectedErrMsg: "each policySet must have a name set, but did not find a name at policySet array index 0",
790+
},
791+
{
792+
name: "policySet must be unique",
793+
setupFunc: func(p *Plugin) {
794+
p.PolicySets = []types.PolicySetConfig{
795+
{
796+
Name: "my-policyset",
797+
},
798+
{
799+
Name: "my-policyset",
800+
},
801+
}
802+
},
803+
expectedErrMsg: "each policySet must have a unique name set, but found a duplicate name: my-policyset",
804+
},
805+
{
806+
name: "policySet must provide only one of placementRulePath or placementPath",
807+
setupFunc: func(p *Plugin) {
808+
p.PolicySets = []types.PolicySetConfig{
809+
{
810+
Name: "my-policyset",
811+
Placement: types.PlacementConfig{
812+
PlacementPath: "../config/plc.yaml",
813+
PlacementRulePath: "../config/plr.yaml",
814+
},
815+
},
816+
}
817+
},
818+
expectedErrMsg: "policySet my-policyset must provide only one of placementRulePath or placementPath",
819+
},
820+
{
821+
name: "policySet must provide only one of labelSelector or clusterselectors",
822+
setupFunc: func(p *Plugin) {
823+
p.PolicySets = []types.PolicySetConfig{
824+
{
825+
Name: "my-policyset",
826+
Placement: types.PlacementConfig{
827+
LabelSelector: map[string]string{"cloud": "red hat"},
828+
ClusterSelectors: map[string]string{"cloud": "red hat"},
829+
},
830+
},
831+
}
832+
},
833+
expectedErrMsg: "policySet my-policyset must provide only one of placement.labelSelector or placement.clusterselectors",
834+
},
835+
{
836+
name: "policySet may not specify a cluster selector and placement path together",
837+
setupFunc: func(p *Plugin) {
838+
p.PolicySets = []types.PolicySetConfig{
839+
{
840+
Name: "my-policyset",
841+
Placement: types.PlacementConfig{
842+
PlacementPath: "../config/plc.yaml",
843+
ClusterSelectors: map[string]string{"cloud": "red hat"},
844+
},
845+
},
846+
}
847+
},
848+
expectedErrMsg: "policySet my-policyset may not specify a placement selector and placement path together",
849+
},
850+
{
851+
name: "policySet may not specify a label selector and placement path together",
852+
setupFunc: func(p *Plugin) {
853+
p.PolicySets = []types.PolicySetConfig{
854+
{
855+
Name: "my-policyset",
856+
Placement: types.PlacementConfig{
857+
PlacementPath: "../config/plc.yaml",
858+
LabelSelector: map[string]string{"cloud": "red hat"},
859+
},
860+
},
861+
}
862+
},
863+
expectedErrMsg: "policySet my-policyset may not specify a placement selector and placement path together",
864+
},
865+
{
866+
name: "policySet may not specify a cluster selector and placementrule path together",
867+
setupFunc: func(p *Plugin) {
868+
p.PolicySets = []types.PolicySetConfig{
869+
{
870+
Name: "my-policyset",
871+
Placement: types.PlacementConfig{
872+
PlacementRulePath: "../config/plc.yaml",
873+
ClusterSelectors: map[string]string{"cloud": "red hat"},
874+
},
875+
},
876+
}
877+
},
878+
expectedErrMsg: "policySet my-policyset may not specify a placement selector and placement path together",
879+
},
880+
{
881+
name: "policySet may not specify a label selector and placementrule path together",
882+
setupFunc: func(p *Plugin) {
883+
p.PolicySets = []types.PolicySetConfig{
884+
{
885+
Name: "my-policyset",
886+
Placement: types.PlacementConfig{
887+
PlacementRulePath: "../config/plc.yaml",
888+
LabelSelector: map[string]string{"cloud": "red hat"},
889+
},
890+
},
891+
}
892+
},
893+
expectedErrMsg: "policySet my-policyset may not specify a placement selector and placement path together",
894+
},
895+
{
896+
name: "policySet placementrule path not resolvable",
897+
setupFunc: func(p *Plugin) {
898+
p.PolicySets = []types.PolicySetConfig{
899+
{
900+
Name: "my-policyset",
901+
Placement: types.PlacementConfig{
902+
PlacementRulePath: "../config/plc.yaml",
903+
},
904+
},
905+
}
906+
},
907+
expectedErrMsg: "could not read the placement rule path ../config/plc.yaml",
908+
},
909+
{
910+
name: "policySet placement path not resolvable",
911+
setupFunc: func(p *Plugin) {
912+
p.PolicySets = []types.PolicySetConfig{
913+
{
914+
Name: "my-policyset",
915+
Placement: types.PlacementConfig{
916+
PlacementPath: "../config/plc.yaml",
917+
},
918+
},
919+
}
920+
},
921+
expectedErrMsg: "could not read the placement path ../config/plc.yaml",
922+
},
923+
{
924+
name: "Placement and PlacementRule can't be mixed",
925+
setupFunc: func(p *Plugin) {
926+
p.Policies[0].Placement = types.PlacementConfig{
927+
LabelSelector: map[string]string{"cloud": "red hat"},
928+
}
929+
p.PolicySets = []types.PolicySetConfig{
930+
{
931+
Name: "my-policyset",
932+
Placement: types.PlacementConfig{
933+
ClusterSelectors: map[string]string{"cloud": "red hat"},
934+
},
935+
},
936+
}
937+
},
938+
expectedErrMsg: "may not use a mix of Placement and PlacementRule for policies and policysets; found 1 Placement and 1 PlacementRule",
939+
},
940+
}
941+
942+
for _, tc := range testCases {
943+
tc := tc // capture range variable
944+
t.Run(tc.name, func(t *testing.T) {
945+
t.Parallel()
946+
p := Plugin{}
947+
var err error
948+
p.baseDirectory, err = filepath.EvalSymlinks(tmpDir)
949+
if err != nil {
950+
t.Fatal(err.Error())
951+
}
952+
p.PlacementBindingDefaults.Name = "my-placement-binding"
953+
p.PolicyDefaults.Placement.Name = "my-placement-rule"
954+
p.PolicyDefaults.Namespace = "my-policies"
955+
policyConf1 := types.PolicyConfig{
956+
Name: "policy-app-config",
957+
Manifests: []types.Manifest{
958+
{
959+
Path: path.Join(tmpDir, "configmap.yaml"),
960+
},
961+
},
962+
}
963+
policyConf2 := types.PolicyConfig{
964+
Name: "policy-app-config2",
965+
Manifests: []types.Manifest{
966+
{
967+
Path: path.Join(tmpDir, "configmap.yaml"),
968+
},
969+
},
970+
}
971+
p.Policies = append(p.Policies, policyConf1, policyConf2)
972+
tc.setupFunc(&p)
973+
p.applyDefaults(map[string]interface{}{})
974+
err = p.assertValidConfig()
975+
if err == nil {
976+
t.Fatal("Expected an error but did not get one")
977+
}
978+
assertEqual(t, err.Error(), tc.expectedErrMsg)
979+
})
980+
}
981+
}

‎internal/plugin_test.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ import (
1414

1515
type testCase struct {
1616
name string
17-
setupFunc func(p *Plugin, policyConf types.PolicyConfig, policyConf2 types.PolicyConfig)
17+
setupFunc func(p *Plugin)
1818
expectedPolicySetConfigInPolicy [][]string
1919
expectedPolicySetConfigs []types.PolicySetConfig
20+
expectedErrMsg string
2021
}
2122

2223
func TestGenerate(t *testing.T) {
@@ -1170,7 +1171,7 @@ func TestGeneratePolicySets(t *testing.T) {
11701171
testCases := []testCase{
11711172
{
11721173
name: "Use p.PolicyDefaults.PolicySets only",
1173-
setupFunc: func(p *Plugin, policyConf types.PolicyConfig, policyConf2 types.PolicyConfig) {
1174+
setupFunc: func(p *Plugin) {
11741175
// PolicyDefaults.PolicySets should be applied to both policies
11751176
p.PolicyDefaults.PolicySets = []string{"policyset-default"}
11761177
},
@@ -1190,7 +1191,7 @@ func TestGeneratePolicySets(t *testing.T) {
11901191
},
11911192
{
11921193
name: "Use p.Policies[0].PolicySets to override with a different policy set",
1193-
setupFunc: func(p *Plugin, policyConf types.PolicyConfig, policyConf2 types.PolicyConfig) {
1194+
setupFunc: func(p *Plugin) {
11941195
// p.PolicyDefaults.PolicySets should be overridden by p.Policies[0].PolicySets
11951196
p.PolicyDefaults.PolicySets = []string{"policyset-default"}
11961197
p.Policies[0] = types.PolicyConfig{
@@ -1224,7 +1225,7 @@ func TestGeneratePolicySets(t *testing.T) {
12241225
},
12251226
{
12261227
name: "Use p.Policies[0].PolicySets to override with an empty policyset",
1227-
setupFunc: func(p *Plugin, policyConf types.PolicyConfig, policyConf2 types.PolicyConfig) {
1228+
setupFunc: func(p *Plugin) {
12281229
// p.PolicyDefaults.PolicySets should be overridden by p.Policies[0].PolicySets
12291230
p.PolicyDefaults.PolicySets = []string{"policyset-default"}
12301231
p.Policies[0] = types.PolicyConfig{
@@ -1252,7 +1253,7 @@ func TestGeneratePolicySets(t *testing.T) {
12521253
},
12531254
{
12541255
name: "Use p.Policies[0].PolicySets and p.PolicySets, should merge",
1255-
setupFunc: func(p *Plugin, policyConf types.PolicyConfig, policyConf2 types.PolicyConfig) {
1256+
setupFunc: func(p *Plugin) {
12561257
// p.Policies[0].PolicySets and p.PolicySets should merge
12571258
p.PolicySets = []types.PolicySetConfig{
12581259
{
@@ -1311,7 +1312,7 @@ func TestGeneratePolicySets(t *testing.T) {
13111312
},
13121313
}
13131314
p.Policies = append(p.Policies, policyConf, policyConf2)
1314-
tc.setupFunc(&p, policyConf, policyConf2)
1315+
tc.setupFunc(&p)
13151316
p.applyDefaults(map[string]interface{}{})
13161317
assertReflectEqual(t, p.Policies[0].PolicySets, tc.expectedPolicySetConfigInPolicy[0])
13171318
assertReflectEqual(t, p.Policies[1].PolicySets, tc.expectedPolicySetConfigInPolicy[1])

0 commit comments

Comments
 (0)