Skip to content

Commit a139325

Browse files
nreischnreisch
authored
Change SemanticEqual to include annotations for constraint caching (#595) (#606)
* Change SemanticEqual to include annotations for constraint^Caching * nit test name * fix lint --------- (cherry picked from commit 1e1e292) Signed-off-by: Noah Reisch <noahreisch4@gmail.com>
1 parent bd8b9f6 commit a139325

File tree

5 files changed

+639
-9
lines changed

5 files changed

+639
-9
lines changed

‎constraint/pkg/client/client_internal_test.go‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,197 @@ func TestMultiDriverAddTemplate(t *testing.T) {
798798
})
799799
}
800800

801+
func Test_AddConstraint_SemanticEqualWithLabelsAndAnnotations(t *testing.T) {
802+
templateA := cts.New(cts.OptTargets(
803+
cts.TargetCustomEngines(
804+
"h1",
805+
cts.Code("driverA", (&schema.Source{RejectWith: "REJECTING"}).ToUnstructured()),
806+
),
807+
))
808+
809+
driverA := fake.New("driverA")
810+
client, err := NewClient(
811+
Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}),
812+
Driver(driverA),
813+
EnforcementPoints("test"),
814+
)
815+
if err != nil {
816+
t.Fatal(err)
817+
}
818+
819+
ctx, cancel := context.WithCancel(context.Background())
820+
defer cancel()
821+
822+
// Add template
823+
if _, err := client.AddTemplate(ctx, templateA.DeepCopy()); err != nil {
824+
t.Fatal(err)
825+
}
826+
827+
t.Run("Add constraint with metadata", func(t *testing.T) {
828+
// Create constraint with specific labels and annotations
829+
constraint := cts.MakeConstraint(t, "Fakes", "test-constraint")
830+
constraint.SetLabels(map[string]string{"env": "prod", "team": "security"})
831+
constraint.SetAnnotations(map[string]string{"audit": "enabled", "export": "true"})
832+
833+
resp, err := client.AddConstraint(ctx, constraint.DeepCopy())
834+
if err != nil {
835+
t.Fatal(err)
836+
}
837+
838+
if !resp.Handled["h1"] {
839+
t.Errorf("Expected constraint to be handled by 'h1' handler, got: %v", resp.Handled)
840+
}
841+
842+
// Verify constraint was added to driver
843+
storedConstraints := driverA.GetConstraintsForTemplate(templateA)
844+
if len(storedConstraints) != 1 {
845+
t.Errorf("Expected 1 constraint, got %d", len(storedConstraints))
846+
}
847+
if _, exists := storedConstraints["test-constraint"]; !exists {
848+
t.Errorf("Expected constraint 'test-constraint' to be stored")
849+
}
850+
})
851+
852+
t.Run("Add identical constraint - should be no-op", func(t *testing.T) {
853+
// Create identical constraint
854+
identicalConstraint := cts.MakeConstraint(t, "Fakes", "test-constraint")
855+
identicalConstraint.SetLabels(map[string]string{"env": "prod", "team": "security"})
856+
identicalConstraint.SetAnnotations(map[string]string{"audit": "enabled", "export": "true"})
857+
858+
// Track initial state
859+
initialConstraints := driverA.GetConstraintsForTemplate(templateA)
860+
initialCount := len(initialConstraints)
861+
862+
resp, err := client.AddConstraint(ctx, identicalConstraint.DeepCopy())
863+
if err != nil {
864+
t.Fatal(err)
865+
}
866+
867+
if !resp.Handled["h1"] {
868+
t.Errorf("Expected constraint to be handled by 'h1' handler, got: %v", resp.Handled)
869+
}
870+
871+
// Verify no duplicate was created
872+
finalConstraints := driverA.GetConstraintsForTemplate(templateA)
873+
if len(finalConstraints) != initialCount {
874+
t.Errorf("Expected constraint count to remain %d, got %d", initialCount, len(finalConstraints))
875+
}
876+
})
877+
878+
t.Run("Add constraint with different labels - should update", func(t *testing.T) {
879+
// Create constraint with different labels
880+
updatedConstraint := cts.MakeConstraint(t, "Fakes", "test-constraint")
881+
updatedConstraint.SetLabels(map[string]string{"env": "staging", "team": "security"}) // Changed env
882+
updatedConstraint.SetAnnotations(map[string]string{"audit": "enabled", "export": "true"})
883+
884+
resp, err := client.AddConstraint(ctx, updatedConstraint.DeepCopy())
885+
if err != nil {
886+
t.Fatal(err)
887+
}
888+
889+
if !resp.Handled["h1"] {
890+
t.Errorf("Expected constraint to be handled by 'h1' handler, got: %v", resp.Handled)
891+
}
892+
893+
// Verify constraint was updated in driver
894+
storedConstraints := driverA.GetConstraintsForTemplate(templateA)
895+
if len(storedConstraints) != 1 {
896+
t.Errorf("Expected 1 constraint, got %d", len(storedConstraints))
897+
}
898+
899+
stored := storedConstraints["test-constraint"]
900+
if stored == nil {
901+
t.Fatal("Expected constraint 'test-constraint' to be stored")
902+
}
903+
904+
// Compare labels and annotations using DeepEqual
905+
if !reflect.DeepEqual(stored.GetLabels(), updatedConstraint.GetLabels()) {
906+
t.Errorf("Labels mismatch: got %v, want %v", stored.GetLabels(), updatedConstraint.GetLabels())
907+
}
908+
909+
if !reflect.DeepEqual(stored.GetAnnotations(), updatedConstraint.GetAnnotations()) {
910+
t.Errorf("Annotations mismatch: got %v, want %v", stored.GetAnnotations(), updatedConstraint.GetAnnotations())
911+
}
912+
})
913+
914+
t.Run("Add constraint with different annotations - should update", func(t *testing.T) {
915+
// Create constraint with different annotations
916+
updatedConstraint := cts.MakeConstraint(t, "Fakes", "test-constraint")
917+
updatedConstraint.SetLabels(map[string]string{"env": "staging", "team": "security"})
918+
updatedConstraint.SetAnnotations(map[string]string{"audit": "disabled", "export": "false"}) // Changed annotations
919+
920+
resp, err := client.AddConstraint(ctx, updatedConstraint.DeepCopy())
921+
if err != nil {
922+
t.Fatal(err)
923+
}
924+
925+
if !resp.Handled["h1"] {
926+
t.Errorf("Expected constraint to be handled by 'h1' handler, got: %v", resp.Handled)
927+
}
928+
929+
// Verify constraint was updated in driver
930+
storedConstraints := driverA.GetConstraintsForTemplate(templateA)
931+
stored := storedConstraints["test-constraint"]
932+
if stored == nil {
933+
t.Fatal("Expected constraint 'test-constraint' to be stored")
934+
}
935+
// Compare labels and annotations using DeepEqual
936+
if !reflect.DeepEqual(stored.GetLabels(), updatedConstraint.GetLabels()) {
937+
t.Errorf("Labels mismatch: got %v, want %v", stored.GetLabels(), updatedConstraint.GetLabels())
938+
}
939+
940+
if !reflect.DeepEqual(stored.GetAnnotations(), updatedConstraint.GetAnnotations()) {
941+
t.Errorf("Annotations mismatch: got %v, want %v", stored.GetAnnotations(), updatedConstraint.GetAnnotations())
942+
}
943+
})
944+
945+
t.Run("Add constraint with different spec - should update", func(t *testing.T) {
946+
// Create constraint with different spec but same metadata
947+
updatedConstraint := cts.MakeConstraint(t, "Fakes", "test-constraint")
948+
updatedConstraint.SetLabels(map[string]string{"env": "staging", "team": "security"})
949+
updatedConstraint.SetAnnotations(map[string]string{"audit": "disabled", "export": "false"})
950+
err := unstructured.SetNestedField(updatedConstraint.Object, "warn", "spec", "enforcementAction")
951+
if err != nil {
952+
t.Fatal(err)
953+
}
954+
955+
resp, err := client.AddConstraint(ctx, updatedConstraint.DeepCopy())
956+
if err != nil {
957+
t.Fatal(err)
958+
}
959+
960+
if !resp.Handled["h1"] {
961+
t.Errorf("Expected constraint to be handled by 'h1' handler, got: %v", resp.Handled)
962+
}
963+
964+
// Verify constraint was updated
965+
storedConstraints := driverA.GetConstraintsForTemplate(templateA)
966+
if len(storedConstraints) != 1 {
967+
t.Errorf("Expected 1 constraint, got %d", len(storedConstraints))
968+
}
969+
970+
stored := storedConstraints["test-constraint"]
971+
if stored == nil {
972+
t.Fatal("Expected constraint 'test-constraint' to be stored")
973+
}
974+
975+
// Compare the stored constraint directly with the updated constraint using DeepEqual
976+
if !reflect.DeepEqual(stored.GetLabels(), updatedConstraint.GetLabels()) {
977+
t.Errorf("Labels mismatch: got %v, want %v", stored.GetLabels(), updatedConstraint.GetLabels())
978+
}
979+
980+
if !reflect.DeepEqual(stored.GetAnnotations(), updatedConstraint.GetAnnotations()) {
981+
t.Errorf("Annotations mismatch: got %v, want %v", stored.GetAnnotations(), updatedConstraint.GetAnnotations())
982+
}
983+
984+
storedSpec, _, _ := unstructured.NestedMap(stored.Object, "spec")
985+
updatedSpec, _, _ := unstructured.NestedMap(updatedConstraint.Object, "spec")
986+
if !reflect.DeepEqual(storedSpec, updatedSpec) {
987+
t.Errorf("Spec mismatch: got %v, want %v", storedSpec, updatedSpec)
988+
}
989+
})
990+
}
991+
801992
func TestMultiDriverRemoveTemplate(t *testing.T) {
802993
templateA := cts.New(cts.OptTargets(
803994
cts.TargetCustomEngines(

‎constraint/pkg/client/client_test.go‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package client_test
33
import (
44
"context"
55
"errors"
6+
"reflect"
67
"strings"
78
"testing"
89

@@ -1048,6 +1049,185 @@ func TestClient_AddConstraint(t *testing.T) {
10481049
}
10491050
}
10501051

1052+
func TestClient_AddConstraint_SemanticEqualWithLabelsAndAnnotations(t *testing.T) {
1053+
template := cts.New(cts.OptName("semantictest"), cts.OptCRDNames("SemanticTest"))
1054+
1055+
tcs := []struct {
1056+
name string
1057+
initialLabels map[string]string
1058+
initialAnnotations map[string]string
1059+
updateLabels map[string]string
1060+
updateAnnotations map[string]string
1061+
updateEnforcementAction string
1062+
wantHandled map[string]bool
1063+
wantError error
1064+
}{
1065+
{
1066+
name: "Add identical constraint - should be no-op",
1067+
initialLabels: map[string]string{"env": "prod", "team": "security"},
1068+
initialAnnotations: map[string]string{"audit": "enabled", "export": "true"},
1069+
updateLabels: map[string]string{"env": "prod", "team": "security"},
1070+
updateAnnotations: map[string]string{"audit": "enabled", "export": "true"},
1071+
wantHandled: map[string]bool{handlertest.TargetName: true},
1072+
wantError: nil,
1073+
},
1074+
{
1075+
name: "Add constraint with different labels - should update",
1076+
initialLabels: map[string]string{"env": "prod", "team": "security"},
1077+
initialAnnotations: map[string]string{"audit": "enabled", "export": "true"},
1078+
updateLabels: map[string]string{"env": "staging", "team": "security"},
1079+
updateAnnotations: map[string]string{"audit": "enabled", "export": "true"},
1080+
wantHandled: map[string]bool{handlertest.TargetName: true},
1081+
wantError: nil,
1082+
},
1083+
{
1084+
name: "Add constraint with different annotations - should update",
1085+
initialLabels: map[string]string{"env": "staging", "team": "security"},
1086+
initialAnnotations: map[string]string{"audit": "enabled", "export": "true"},
1087+
updateLabels: map[string]string{"env": "staging", "team": "security"},
1088+
updateAnnotations: map[string]string{"audit": "disabled", "export": "false"},
1089+
wantHandled: map[string]bool{handlertest.TargetName: true},
1090+
wantError: nil,
1091+
},
1092+
{
1093+
name: "Add constraint with different spec - should update",
1094+
initialLabels: map[string]string{"env": "staging", "team": "security"},
1095+
initialAnnotations: map[string]string{"audit": "disabled", "export": "false"},
1096+
updateLabels: map[string]string{"env": "staging", "team": "security"},
1097+
updateAnnotations: map[string]string{"audit": "disabled", "export": "false"},
1098+
updateEnforcementAction: "warn",
1099+
wantHandled: map[string]bool{handlertest.TargetName: true},
1100+
wantError: nil,
1101+
},
1102+
}
1103+
1104+
for _, tc := range tcs {
1105+
t.Run(tc.name, func(t *testing.T) {
1106+
d, err := rego.New()
1107+
if err != nil {
1108+
t.Fatal(err)
1109+
}
1110+
1111+
h := &handlertest.Handler{}
1112+
c, err := client.NewClient(client.Targets(h), client.Driver(d), client.EnforcementPoints("test"))
1113+
if err != nil {
1114+
t.Fatal(err)
1115+
}
1116+
1117+
ctx := context.Background()
1118+
1119+
// Add template
1120+
if _, err := c.AddTemplate(ctx, template); err != nil {
1121+
t.Fatal(err)
1122+
}
1123+
1124+
// Add initial constraint
1125+
initialConstraint := cts.MakeConstraint(t, "SemanticTest", "test-constraint")
1126+
initialConstraint.SetLabels(tc.initialLabels)
1127+
initialConstraint.SetAnnotations(tc.initialAnnotations)
1128+
1129+
resp1, err := c.AddConstraint(ctx, initialConstraint)
1130+
if !errors.Is(err, tc.wantError) {
1131+
t.Fatalf("got AddConstraint() initial error = %v, want %v", err, tc.wantError)
1132+
}
1133+
1134+
if tc.wantError != nil {
1135+
return
1136+
}
1137+
1138+
if diff := cmp.Diff(tc.wantHandled, resp1.Handled, cmpopts.EquateEmpty()); diff != "" {
1139+
t.Errorf("initial AddConstraint() handled mismatch: %s", diff)
1140+
}
1141+
1142+
// Verify initial constraint exists with correct metadata
1143+
stored1, err := c.GetConstraint(initialConstraint)
1144+
if err != nil {
1145+
t.Fatal(err)
1146+
}
1147+
1148+
if !reflect.DeepEqual(stored1.GetLabels(), tc.initialLabels) {
1149+
t.Errorf("Initial labels mismatch: got %v, want %v", stored1.GetLabels(), tc.initialLabels)
1150+
}
1151+
1152+
// Update constraint (or re-add identical)
1153+
updateConstraint := cts.MakeConstraint(t, "SemanticTest", "test-constraint")
1154+
updateConstraint.SetLabels(tc.updateLabels)
1155+
updateConstraint.SetAnnotations(tc.updateAnnotations)
1156+
1157+
// Set enforcement action if specified
1158+
if tc.updateEnforcementAction != "" {
1159+
err := unstructured.SetNestedField(updateConstraint.Object, tc.updateEnforcementAction, "spec", "enforcementAction")
1160+
if err != nil {
1161+
t.Fatal(err)
1162+
}
1163+
}
1164+
1165+
resp2, err := c.AddConstraint(ctx, updateConstraint)
1166+
if err != nil {
1167+
t.Fatal(err)
1168+
}
1169+
1170+
if diff := cmp.Diff(tc.wantHandled, resp2.Handled, cmpopts.EquateEmpty()); diff != "" {
1171+
t.Errorf("update AddConstraint() handled mismatch: %s", diff)
1172+
}
1173+
1174+
// Verify final constraint has expected metadata
1175+
got, err := c.GetConstraint(updateConstraint)
1176+
if err != nil {
1177+
t.Fatal(err)
1178+
}
1179+
1180+
// Build expected constraint for comparison
1181+
want := cts.MakeConstraint(t, "SemanticTest", "test-constraint")
1182+
want.SetLabels(tc.updateLabels)
1183+
want.SetAnnotations(tc.updateAnnotations)
1184+
if tc.updateEnforcementAction != "" {
1185+
err := unstructured.SetNestedField(want.Object, tc.updateEnforcementAction, "spec", "enforcementAction")
1186+
if err != nil {
1187+
t.Fatal(err)
1188+
}
1189+
}
1190+
1191+
// Compare the stored constraint with the expected constraint using DeepEqual
1192+
if !reflect.DeepEqual(got.GetLabels(), want.GetLabels()) {
1193+
t.Errorf("Labels mismatch: got %v, want %v", got.GetLabels(), want.GetLabels())
1194+
}
1195+
1196+
if !reflect.DeepEqual(got.GetAnnotations(), want.GetAnnotations()) {
1197+
t.Errorf("Annotations mismatch: got %v, want %v", got.GetAnnotations(), want.GetAnnotations())
1198+
}
1199+
1200+
// Compare the spec portion if enforcement action was set
1201+
if tc.updateEnforcementAction != "" {
1202+
gotSpec, found, err := unstructured.NestedMap(got.Object, "spec")
1203+
if err != nil {
1204+
t.Fatal(err)
1205+
}
1206+
if !found {
1207+
t.Fatal("spec not found in stored constraint")
1208+
}
1209+
1210+
wantSpec, found, err := unstructured.NestedMap(want.Object, "spec")
1211+
if err != nil {
1212+
t.Fatal(err)
1213+
}
1214+
if !found {
1215+
t.Fatal("spec not found in expected constraint")
1216+
}
1217+
1218+
if !reflect.DeepEqual(gotSpec, wantSpec) {
1219+
t.Errorf("Spec mismatch: got %v, want %v", gotSpec, wantSpec)
1220+
}
1221+
}
1222+
1223+
// Clean up
1224+
if _, err := c.RemoveConstraint(ctx, updateConstraint); err != nil {
1225+
t.Fatal(err)
1226+
}
1227+
})
1228+
}
1229+
}
1230+
10511231
// TestClient_AddConstraint_withDefaultParams makes sure that we can inject
10521232
// OpenAPIV3 Default parameters into a constraint that does not define its parameters
10531233
// but its template defines defaults for the parameters in the spec's schema.

0 commit comments

Comments
 (0)