Skip to content

Commit a097bd7

Browse files
zyjjayopenshift-merge-bot[bot]
zyjjay
authored and
openshift-merge-bot[bot]
committed
Allow override across different placement types
This change will allow placements to override placementrule defaults and vice versa. ref: https://issues.redhat.com/browse/ACM-10152 Signed-off-by: Jason Zhang <[email protected]>
1 parent 1a25c98 commit a097bd7

File tree

3 files changed

+211
-23
lines changed

3 files changed

+211
-23
lines changed

‎internal/plugin.go‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -834,28 +834,32 @@ func applyDefaultPlacementFields(placement *types.PlacementConfig, defaultPlacem
834834
len(defaultPlacement.ClusterSelector) != 0 ||
835835
defaultPlacement.PlacementRulePath != "" ||
836836
defaultPlacement.PlacementRuleName != ""
837+
policyPlcUnset := len(placement.LabelSelector) == 0 &&
838+
placement.PlacementPath == "" &&
839+
placement.PlacementName == ""
840+
policyPlrUnset := len(placement.ClusterSelectors) == 0 &&
841+
len(placement.ClusterSelector) == 0 &&
842+
placement.PlacementRulePath == "" &&
843+
placement.PlacementRuleName == ""
837844

838845
// If both cluster label selectors and placement path/name aren't set, then use the defaults with a
839846
// priority on placement path followed by placement name.
840-
if len(placement.LabelSelector) == 0 &&
841-
placement.PlacementPath == "" &&
842-
placement.PlacementName == "" &&
843-
plcDefaultSet {
844-
if defaultPlacement.PlacementPath != "" {
847+
if policyPlcUnset && plcDefaultSet {
848+
if !policyPlrUnset {
849+
return
850+
} else if defaultPlacement.PlacementPath != "" {
845851
placement.PlacementPath = defaultPlacement.PlacementPath
846852
} else if defaultPlacement.PlacementName != "" {
847853
placement.PlacementName = defaultPlacement.PlacementName
848854
} else if len(defaultPlacement.LabelSelector) > 0 {
849855
placement.LabelSelector = defaultPlacement.LabelSelector
850856
}
851-
} else if len(placement.ClusterSelectors) == 0 &&
857+
} else if policyPlrUnset && plrDefaultSet {
852858
// Else if both cluster selectors and placement rule path/name aren't set, then use the defaults with a
853859
// priority on placement rule path followed by placement rule name.
854-
len(placement.ClusterSelector) == 0 &&
855-
placement.PlacementRulePath == "" &&
856-
placement.PlacementRuleName == "" &&
857-
plrDefaultSet {
858-
if defaultPlacement.PlacementRulePath != "" {
860+
if !policyPlcUnset {
861+
return
862+
} else if defaultPlacement.PlacementRulePath != "" {
859863
placement.PlacementRulePath = defaultPlacement.PlacementRulePath
860864
} else if defaultPlacement.PlacementRuleName != "" {
861865
placement.PlacementRuleName = defaultPlacement.PlacementRuleName

‎internal/plugin_config_test.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -788,13 +788,9 @@ policies:
788788
p := Plugin{}
789789

790790
err = p.Config([]byte(config), tmpDir)
791-
if err == nil {
792-
t.Fatal("Expected an error but did not get one")
791+
if err != nil {
792+
t.Fatal(err.Error())
793793
}
794-
795-
expected := "policy policy-app-config must specify only one of " +
796-
"placement selector, placement path, or placement name"
797-
assertEqual(t, err.Error(), expected)
798794
}
799795

800796
func TestConfigMultipleDefaultAndPolicyPlacementNames(t *testing.T) {
@@ -824,13 +820,9 @@ policies:
824820
p := Plugin{}
825821

826822
err := p.Config([]byte(config), tmpDir)
827-
if err == nil {
828-
t.Fatal("Expected an error but did not get one")
823+
if err != nil {
824+
t.Fatal(err.Error())
829825
}
830-
831-
expected := "policy policy-app-config must specify only one of " +
832-
"placement selector, placement path, or placement name"
833-
assertEqual(t, err.Error(), expected)
834826
}
835827

836828
func TestConfigPlacementInvalidMixture(t *testing.T) {

‎internal/plugin_test.go‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,198 @@ subjects:
574574
assertEqual(t, string(output), expected)
575575
}
576576

577+
func TestGeneratePolicyOverrideDefaultPlacement(t *testing.T) {
578+
t.Parallel()
579+
tmpDir := t.TempDir()
580+
createConfigMap(t, tmpDir, "configmap.yaml")
581+
582+
p := Plugin{}
583+
var err error
584+
585+
p.baseDirectory, err = filepath.EvalSymlinks(tmpDir)
586+
if err != nil {
587+
t.Fatal(err.Error())
588+
}
589+
590+
p.PolicyDefaults.Placement.PlacementName = "my-placement"
591+
p.PolicyDefaults.Namespace = "my-policies"
592+
PolicyConf := types.PolicyConfig{
593+
Name: "policy-app-config",
594+
Manifests: []types.Manifest{
595+
{
596+
Path: path.Join(tmpDir, "configmap.yaml"),
597+
},
598+
},
599+
PolicyOptions: types.PolicyOptions{
600+
Placement: types.PlacementConfig{
601+
PlacementRuleName: "my-placement-rule",
602+
},
603+
},
604+
}
605+
p.Policies = append(p.Policies, PolicyConf)
606+
p.applyDefaults(map[string]interface{}{})
607+
608+
assertEqual(t, p.Policies[0].ConsolidateManifests, true)
609+
610+
if err := p.assertValidConfig(); err != nil {
611+
t.Fatal(err.Error())
612+
}
613+
614+
expected := `
615+
---
616+
apiVersion: policy.open-cluster-management.io/v1
617+
kind: Policy
618+
metadata:
619+
annotations:
620+
policy.open-cluster-management.io/categories: CM Configuration Management
621+
policy.open-cluster-management.io/controls: CM-2 Baseline Configuration
622+
policy.open-cluster-management.io/description: ""
623+
policy.open-cluster-management.io/standards: NIST SP 800-53
624+
name: policy-app-config
625+
namespace: my-policies
626+
spec:
627+
disabled: false
628+
policy-templates:
629+
- objectDefinition:
630+
apiVersion: policy.open-cluster-management.io/v1
631+
kind: ConfigurationPolicy
632+
metadata:
633+
name: policy-app-config
634+
spec:
635+
object-templates:
636+
- complianceType: musthave
637+
objectDefinition:
638+
apiVersion: v1
639+
data:
640+
game.properties: enemies=potato
641+
kind: ConfigMap
642+
metadata:
643+
name: my-configmap
644+
remediationAction: inform
645+
severity: low
646+
remediationAction: inform
647+
---
648+
apiVersion: policy.open-cluster-management.io/v1
649+
kind: PlacementBinding
650+
metadata:
651+
name: binding-policy-app-config
652+
namespace: my-policies
653+
placementRef:
654+
apiGroup: apps.open-cluster-management.io
655+
kind: PlacementRule
656+
name: my-placement-rule
657+
subjects:
658+
- apiGroup: policy.open-cluster-management.io
659+
kind: Policy
660+
name: policy-app-config
661+
`
662+
663+
expected = strings.TrimPrefix(expected, "\n")
664+
665+
output, err := p.Generate()
666+
if err != nil {
667+
t.Fatal(err.Error())
668+
}
669+
670+
assertEqual(t, string(output), expected)
671+
}
672+
673+
func TestGeneratePolicyOverrideDefaultPlacementRule(t *testing.T) {
674+
t.Parallel()
675+
tmpDir := t.TempDir()
676+
createConfigMap(t, tmpDir, "configmap.yaml")
677+
678+
p := Plugin{}
679+
var err error
680+
681+
p.baseDirectory, err = filepath.EvalSymlinks(tmpDir)
682+
if err != nil {
683+
t.Fatal(err.Error())
684+
}
685+
686+
p.PolicyDefaults.Placement.PlacementRuleName = "my-placement-rule"
687+
p.PolicyDefaults.Namespace = "my-policies"
688+
PolicyConf := types.PolicyConfig{
689+
Name: "policy-app-config",
690+
Manifests: []types.Manifest{
691+
{
692+
Path: path.Join(tmpDir, "configmap.yaml"),
693+
},
694+
},
695+
PolicyOptions: types.PolicyOptions{
696+
Placement: types.PlacementConfig{
697+
PlacementName: "my-placement",
698+
},
699+
},
700+
}
701+
p.Policies = append(p.Policies, PolicyConf)
702+
p.applyDefaults(map[string]interface{}{})
703+
704+
assertEqual(t, p.Policies[0].ConsolidateManifests, true)
705+
706+
if err := p.assertValidConfig(); err != nil {
707+
t.Fatal(err.Error())
708+
}
709+
710+
expected := `
711+
---
712+
apiVersion: policy.open-cluster-management.io/v1
713+
kind: Policy
714+
metadata:
715+
annotations:
716+
policy.open-cluster-management.io/categories: CM Configuration Management
717+
policy.open-cluster-management.io/controls: CM-2 Baseline Configuration
718+
policy.open-cluster-management.io/description: ""
719+
policy.open-cluster-management.io/standards: NIST SP 800-53
720+
name: policy-app-config
721+
namespace: my-policies
722+
spec:
723+
disabled: false
724+
policy-templates:
725+
- objectDefinition:
726+
apiVersion: policy.open-cluster-management.io/v1
727+
kind: ConfigurationPolicy
728+
metadata:
729+
name: policy-app-config
730+
spec:
731+
object-templates:
732+
- complianceType: musthave
733+
objectDefinition:
734+
apiVersion: v1
735+
data:
736+
game.properties: enemies=potato
737+
kind: ConfigMap
738+
metadata:
739+
name: my-configmap
740+
remediationAction: inform
741+
severity: low
742+
remediationAction: inform
743+
---
744+
apiVersion: policy.open-cluster-management.io/v1
745+
kind: PlacementBinding
746+
metadata:
747+
name: binding-policy-app-config
748+
namespace: my-policies
749+
placementRef:
750+
apiGroup: cluster.open-cluster-management.io
751+
kind: Placement
752+
name: my-placement
753+
subjects:
754+
- apiGroup: policy.open-cluster-management.io
755+
kind: Policy
756+
name: policy-app-config
757+
`
758+
759+
expected = strings.TrimPrefix(expected, "\n")
760+
761+
output, err := p.Generate()
762+
if err != nil {
763+
t.Fatal(err.Error())
764+
}
765+
766+
assertEqual(t, string(output), expected)
767+
}
768+
577769
func TestGeneratePolicyExistingPlacementName(t *testing.T) {
578770
t.Parallel()
579771
tmpDir := t.TempDir()

0 commit comments

Comments
 (0)