Skip to content

Commit 3473060

Browse files
author
Ryan Zhang
committed
add conflict tests
1 parent c3bb485 commit 3473060

File tree

2 files changed

+268
-10
lines changed

2 files changed

+268
-10
lines changed

‎pkg/controllers/workgenerator/controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be
456456
}
457457
}
458458
if len(simpleManifests) == 0 {
459-
klog.V(2).InfoS("the snapshot contains enveloped resource only", "snapshot", klog.KObj(snapshot))
459+
klog.V(2).InfoS("the snapshot contains no resource to apply either because of override or enveloped resources", "snapshot", klog.KObj(snapshot))
460460
}
461461
// generate a work object for the manifests even if there is nothing to place
462462
// to allow CRP to collect the status of the placement

‎pkg/controllers/workgenerator/override_test.go‎

Lines changed: 267 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
521521
},
522522
},
523523
{
524-
name: "selected by clusterResourceOverride",
524+
name: "selected by clusterResourceOverride but only one rule matched the cluster",
525525
clusterRole: rbacv1.ClusterRole{
526526
TypeMeta: clusterRoleType,
527527
ObjectMeta: metav1.ObjectMeta{
@@ -553,6 +553,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
553553
Policy: &placementv1alpha1.OverridePolicy{
554554
OverrideRules: []placementv1alpha1.OverrideRule{
555555
{
556+
// matching rule
556557
ClusterSelector: &placementv1beta1.ClusterSelector{
557558
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
558559
{
@@ -573,6 +574,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
573574
},
574575
},
575576
{
577+
// non matching rule
576578
ClusterSelector: &placementv1beta1.ClusterSelector{
577579
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
578580
{
@@ -611,12 +613,19 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
611613
},
612614
},
613615
{
614-
name: "invalid json patch of clusterResourceOverride",
616+
name: "selected by clusterResourceOverride with two rules that don't conflict",
615617
clusterRole: rbacv1.ClusterRole{
616618
TypeMeta: clusterRoleType,
617619
ObjectMeta: metav1.ObjectMeta{
618620
Name: "clusterrole-name",
619621
},
622+
Rules: []rbacv1.PolicyRule{
623+
{
624+
APIGroups: []string{"authorization.k8s.io"},
625+
Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"},
626+
Verbs: []string{"create"},
627+
},
628+
},
620629
},
621630
cluster: clusterv1beta1.MemberCluster{
622631
ObjectMeta: metav1.ObjectMeta{
@@ -654,8 +663,8 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
654663
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
655664
{
656665
Operator: placementv1alpha1.JSONPatchOverrideOpAdd,
657-
Path: "/metadata/labels/new-label",
658-
Value: apiextensionsv1.JSON{Raw: []byte(`"new-value"`)},
666+
Path: "/rules/0/verbs/1",
667+
Value: apiextensionsv1.JSON{Raw: []byte(`"read"`)},
659668
},
660669
},
661670
},
@@ -665,17 +674,266 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
665674
{
666675
LabelSelector: &metav1.LabelSelector{
667676
MatchLabels: map[string]string{
668-
"key2": "value1",
677+
"key2": "value2",
669678
},
670679
},
671680
},
672681
},
673682
},
674683
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
675684
{
676-
Operator: placementv1alpha1.JSONPatchOverrideOpReplace,
685+
Operator: placementv1alpha1.JSONPatchOverrideOpRemove,
686+
Path: "/rules/0/verbs/0",
687+
},
688+
},
689+
},
690+
},
691+
},
692+
},
693+
},
694+
},
695+
},
696+
},
697+
wantClusterRole: rbacv1.ClusterRole{
698+
TypeMeta: clusterRoleType,
699+
ObjectMeta: metav1.ObjectMeta{
700+
Name: "clusterrole-name",
701+
},
702+
Rules: []rbacv1.PolicyRule{
703+
{
704+
APIGroups: []string{"authorization.k8s.io"},
705+
Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"},
706+
Verbs: []string{"read"},
707+
},
708+
},
709+
},
710+
},
711+
{
712+
name: "selected by clusterResourceOverride with two rules that conflict but still a valid patch",
713+
clusterRole: rbacv1.ClusterRole{
714+
TypeMeta: clusterRoleType,
715+
ObjectMeta: metav1.ObjectMeta{
716+
Name: "clusterrole-name",
717+
},
718+
Rules: []rbacv1.PolicyRule{
719+
{
720+
APIGroups: []string{"authorization.k8s.io"},
721+
Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"},
722+
Verbs: []string{"create"},
723+
},
724+
},
725+
},
726+
cluster: clusterv1beta1.MemberCluster{
727+
ObjectMeta: metav1.ObjectMeta{
728+
Name: "cluster-1",
729+
Labels: map[string]string{
730+
"key1": "value1",
731+
"key2": "value2",
732+
},
733+
},
734+
},
735+
croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{
736+
{
737+
Group: "rbac.authorization.k8s.io",
738+
Version: "v1",
739+
Kind: "ClusterRole",
740+
Name: "clusterrole-name",
741+
}: {
742+
{
743+
Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{
744+
OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{
745+
Policy: &placementv1alpha1.OverridePolicy{
746+
OverrideRules: []placementv1alpha1.OverrideRule{
747+
{
748+
ClusterSelector: &placementv1beta1.ClusterSelector{
749+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
750+
{
751+
LabelSelector: &metav1.LabelSelector{
752+
MatchLabels: map[string]string{
753+
"key1": "value1",
754+
},
755+
},
756+
},
757+
},
758+
},
759+
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
760+
{
761+
Operator: placementv1alpha1.JSONPatchOverrideOpAdd,
762+
Path: "/rules/0/verbs/1",
763+
Value: apiextensionsv1.JSON{Raw: []byte(`"read"`)},
764+
},
765+
},
766+
},
767+
{
768+
ClusterSelector: &placementv1beta1.ClusterSelector{
769+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
770+
{
771+
LabelSelector: &metav1.LabelSelector{
772+
MatchLabels: map[string]string{
773+
"key2": "value2",
774+
},
775+
},
776+
},
777+
},
778+
},
779+
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
780+
{
781+
Operator: placementv1alpha1.JSONPatchOverrideOpRemove,
782+
Path: "/rules/0/verbs/1",
783+
},
784+
},
785+
},
786+
},
787+
},
788+
},
789+
},
790+
},
791+
},
792+
},
793+
wantClusterRole: rbacv1.ClusterRole{
794+
TypeMeta: clusterRoleType,
795+
ObjectMeta: metav1.ObjectMeta{
796+
Name: "clusterrole-name",
797+
},
798+
Rules: []rbacv1.PolicyRule{
799+
{
800+
APIGroups: []string{"authorization.k8s.io"},
801+
Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"},
802+
Verbs: []string{"create"},
803+
},
804+
},
805+
},
806+
},
807+
{
808+
name: "selected by clusterResourceOverride with two rules that conflict and result in error",
809+
clusterRole: rbacv1.ClusterRole{
810+
TypeMeta: clusterRoleType,
811+
ObjectMeta: metav1.ObjectMeta{
812+
Name: "clusterrole-name",
813+
},
814+
Rules: []rbacv1.PolicyRule{
815+
{
816+
APIGroups: []string{"authorization.k8s.io"},
817+
Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"},
818+
Verbs: []string{"create"},
819+
},
820+
},
821+
},
822+
cluster: clusterv1beta1.MemberCluster{
823+
ObjectMeta: metav1.ObjectMeta{
824+
Name: "cluster-1",
825+
Labels: map[string]string{
826+
"key1": "value1",
827+
"key2": "value2",
828+
},
829+
},
830+
},
831+
croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{
832+
{
833+
Group: "rbac.authorization.k8s.io",
834+
Version: "v1",
835+
Kind: "ClusterRole",
836+
Name: "clusterrole-name",
837+
}: {
838+
{
839+
Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{
840+
OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{
841+
Policy: &placementv1alpha1.OverridePolicy{
842+
OverrideRules: []placementv1alpha1.OverrideRule{
843+
{
844+
ClusterSelector: &placementv1beta1.ClusterSelector{
845+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
846+
{
847+
LabelSelector: &metav1.LabelSelector{
848+
MatchLabels: map[string]string{
849+
"key1": "value1",
850+
},
851+
},
852+
},
853+
},
854+
},
855+
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
856+
{
857+
Operator: placementv1alpha1.JSONPatchOverrideOpRemove,
858+
Path: "/rules/0/verbs",
859+
},
860+
},
861+
},
862+
{
863+
ClusterSelector: &placementv1beta1.ClusterSelector{
864+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
865+
{
866+
LabelSelector: &metav1.LabelSelector{
867+
MatchLabels: map[string]string{
868+
"key2": "value2",
869+
},
870+
},
871+
},
872+
},
873+
},
874+
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
875+
{
876+
Operator: placementv1alpha1.JSONPatchOverrideOpAdd,
877+
Path: "/rules/0/verbs/1",
878+
Value: apiextensionsv1.JSON{Raw: []byte(`"read"`)},
879+
},
880+
},
881+
},
882+
},
883+
},
884+
},
885+
},
886+
},
887+
},
888+
},
889+
wantErr: controller.ErrUserError,
890+
},
891+
{
892+
name: "invalid json patch of clusterResourceOverride",
893+
clusterRole: rbacv1.ClusterRole{
894+
TypeMeta: clusterRoleType,
895+
ObjectMeta: metav1.ObjectMeta{
896+
Name: "clusterrole-name",
897+
},
898+
},
899+
cluster: clusterv1beta1.MemberCluster{
900+
ObjectMeta: metav1.ObjectMeta{
901+
Name: "cluster-1",
902+
Labels: map[string]string{
903+
"key1": "value1",
904+
"key2": "value2",
905+
},
906+
},
907+
},
908+
croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{
909+
{
910+
Group: "rbac.authorization.k8s.io",
911+
Version: "v1",
912+
Kind: "ClusterRole",
913+
Name: "clusterrole-name",
914+
}: {
915+
{
916+
Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{
917+
OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{
918+
Policy: &placementv1alpha1.OverridePolicy{
919+
OverrideRules: []placementv1alpha1.OverrideRule{
920+
{
921+
ClusterSelector: &placementv1beta1.ClusterSelector{
922+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
923+
{
924+
LabelSelector: &metav1.LabelSelector{
925+
MatchLabels: map[string]string{
926+
"key1": "value1",
927+
},
928+
},
929+
},
930+
},
931+
},
932+
JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{
933+
{
934+
Operator: placementv1alpha1.JSONPatchOverrideOpAdd,
677935
Path: "/metadata/labels/new-label",
678-
Value: apiextensionsv1.JSON{Raw: []byte(`"new-value1"`)},
936+
Value: apiextensionsv1.JSON{Raw: []byte(`"new-value"`)},
679937
},
680938
},
681939
},
@@ -789,7 +1047,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) {
7891047

7901048
var clusterRole rbacv1.ClusterRole
7911049
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &clusterRole); err != nil {
792-
t.Fatalf("Failed to convert the result to deployment: %v, want nil", err)
1050+
t.Fatalf("Failed to convert the result to clusterole: %v, want nil", err)
7931051
}
7941052

7951053
if diff := cmp.Diff(tc.wantClusterRole, clusterRole); diff != "" {
@@ -808,7 +1066,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) {
8081066
Kind: "Deployment",
8091067
}: true,
8101068
},
811-
IsClusterScopedResource: true,
1069+
IsClusterScopedResource: false,
8121070
}
8131071
deploymentType := metav1.TypeMeta{
8141072
APIVersion: utils.DeploymentGVK.GroupVersion().String(),

0 commit comments

Comments
 (0)