Skip to content

Commit 6166175

Browse files
author
Nont
committed
Preserve ObjectMeta.ResourceVersion
Signed-off-by: Nont <nont@duck.com>
1 parent 3a20471 commit 6166175

File tree

2 files changed

+174
-20
lines changed

2 files changed

+174
-20
lines changed

‎pkg/webhook/managedresource/validatingadmissionpolicy.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ func getValidatingAdmissionPolicy(isHub bool) *admv1.ValidatingAdmissionPolicy {
2525
}
2626

2727
func mutateValidatingAdmissionPolicy(vap *admv1.ValidatingAdmissionPolicy, isHub bool) {
28-
vap.TypeMeta = metav1.TypeMeta{
29-
APIVersion: "admissionregistration.k8s.io/v1",
30-
Kind: "ValidatingAdmissionPolicy",
31-
}
32-
vap.ObjectMeta.Labels = map[string]string{
33-
"fleet.azure.com/managed-by": "arm",
28+
ometa := metav1.ObjectMeta{
29+
Labels: map[string]string{
30+
"fleet.azure.com/managed-by": "arm",
31+
},
32+
ResourceVersion: vap.ResourceVersion,
3433
}
34+
vap.ObjectMeta = ometa
3535
vap.Spec = admv1.ValidatingAdmissionPolicySpec{
3636
MatchConstraints: &admv1.MatchResources{
3737
ObjectSelector: &metav1.LabelSelector{
@@ -108,13 +108,13 @@ func getValidatingAdmissionPolicyBinding() *admv1.ValidatingAdmissionPolicyBindi
108108
}
109109

110110
func mutateValidatingAdmissionPolicyBinding(vapb *admv1.ValidatingAdmissionPolicyBinding) {
111-
vapb.TypeMeta = metav1.TypeMeta{
112-
APIVersion: "admissionregistration.k8s.io/v1",
113-
Kind: "ValidatingAdmissionPolicyBinding",
114-
}
115-
vapb.ObjectMeta.Labels = map[string]string{
116-
"fleet.azure.com/managed-by": "arm",
111+
ometa := metav1.ObjectMeta{
112+
Labels: map[string]string{
113+
"fleet.azure.com/managed-by": "arm",
114+
},
115+
ResourceVersion: vapb.ResourceVersion,
117116
}
117+
vapb.ObjectMeta = ometa
118118
vapb.Spec = admv1.ValidatingAdmissionPolicyBindingSpec{
119119
PolicyName: resourceName,
120120
ValidationActions: []admv1.ValidationAction{

‎pkg/webhook/managedresource/validatingadmissionpolicy_test.go‎

Lines changed: 162 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ package managedresource
88
import (
99
"testing"
1010

11-
"github.com/stretchr/testify/assert"
11+
"github.com/google/go-cmp/cmp"
1212
admv1 "k8s.io/api/admissionregistration/v1"
13+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1314
)
1415

1516
func TestGetValidatingAdmissionPolicy(t *testing.T) {
@@ -19,8 +20,11 @@ func TestGetValidatingAdmissionPolicy(t *testing.T) {
1920
t.Parallel()
2021

2122
vap := getValidatingAdmissionPolicy(false)
22-
assert.NotNil(t, vap)
23-
assert.NotContains(t, vap.Spec.MatchConstraints.ResourceRules, admv1.NamedRuleWithOperations{
23+
if vap == nil {
24+
t.Errorf("getValidatingAdmissionPolicy(false) = nil, want non-nil")
25+
}
26+
27+
unwantedRule := admv1.NamedRuleWithOperations{
2428
RuleWithOperations: admv1.RuleWithOperations{
2529
Rule: admv1.Rule{
2630
APIGroups: []string{"placement.kubernetes-fleet.io"},
@@ -29,15 +33,24 @@ func TestGetValidatingAdmissionPolicy(t *testing.T) {
2933
},
3034
Operations: []admv1.OperationType{admv1.Create, admv1.Update, admv1.Delete},
3135
},
32-
})
36+
}
37+
38+
for _, rule := range vap.Spec.MatchConstraints.ResourceRules {
39+
if diff := cmp.Diff(unwantedRule, rule); diff == "" {
40+
t.Errorf("getValidatingAdmissionPolicy(false) contains unwanted rule %+v", unwantedRule)
41+
}
42+
}
3343
})
3444

3545
t.Run("hub", func(t *testing.T) {
3646
t.Parallel()
3747

3848
vap := getValidatingAdmissionPolicy(true)
39-
assert.NotNil(t, vap)
40-
assert.Contains(t, vap.Spec.MatchConstraints.ResourceRules, admv1.NamedRuleWithOperations{
49+
if vap == nil {
50+
t.Errorf("getValidatingAdmissionPolicy(true) = nil, want non-nil")
51+
}
52+
53+
wantedRule := admv1.NamedRuleWithOperations{
4154
RuleWithOperations: admv1.RuleWithOperations{
4255
Rule: admv1.Rule{
4356
APIGroups: []string{"placement.kubernetes-fleet.io"},
@@ -46,13 +59,154 @@ func TestGetValidatingAdmissionPolicy(t *testing.T) {
4659
},
4760
Operations: []admv1.OperationType{admv1.Create, admv1.Update, admv1.Delete},
4861
},
49-
})
62+
}
63+
64+
found := false
65+
for _, rule := range vap.Spec.MatchConstraints.ResourceRules {
66+
if diff := cmp.Diff(wantedRule, rule); diff == "" {
67+
found = true
68+
break
69+
}
70+
}
71+
if !found {
72+
t.Errorf("getValidatingAdmissionPolicy(true) missing expected rule %+v", wantedRule)
73+
}
5074
})
5175
}
5276

77+
func TestMutateValidatingAdmissionPolicy(t *testing.T) {
78+
t.Parallel()
79+
80+
tests := []struct {
81+
name string
82+
isHub bool
83+
resourceVersion string
84+
initialLabels map[string]string
85+
}{
86+
{
87+
name: "preserves ResourceVersion and updates labels for member cluster",
88+
isHub: false,
89+
resourceVersion: "12345",
90+
initialLabels: map[string]string{"existing": "label"},
91+
},
92+
{
93+
name: "preserves ResourceVersion and updates labels for hub cluster",
94+
isHub: true,
95+
resourceVersion: "67890",
96+
initialLabels: map[string]string{"old": "value"},
97+
},
98+
{
99+
name: "preserves empty ResourceVersion and sets labels",
100+
isHub: false,
101+
resourceVersion: "",
102+
initialLabels: nil,
103+
},
104+
{
105+
name: "overwrites existing managed label while preserving ResourceVersion",
106+
isHub: false,
107+
resourceVersion: "54321",
108+
initialLabels: map[string]string{"fleet.azure.com/managed-by": "old-value", "other": "label"},
109+
},
110+
}
111+
112+
for _, tt := range tests {
113+
t.Run(tt.name, func(t *testing.T) {
114+
t.Parallel()
115+
116+
vap := &admv1.ValidatingAdmissionPolicy{
117+
ObjectMeta: metav1.ObjectMeta{
118+
Name: "test-policy",
119+
ResourceVersion: tt.resourceVersion,
120+
Labels: tt.initialLabels,
121+
},
122+
}
123+
124+
mutateValidatingAdmissionPolicy(vap, tt.isHub)
125+
126+
if vap.ResourceVersion != tt.resourceVersion {
127+
t.Errorf("mutateValidatingAdmissionPolicy() ResourceVersion = %v, want %v", vap.ResourceVersion, tt.resourceVersion)
128+
}
129+
130+
wantManagedByLabel := "arm"
131+
if got := vap.Labels["fleet.azure.com/managed-by"]; got != wantManagedByLabel {
132+
t.Errorf("mutateValidatingAdmissionPolicy() managed-by label = %v, want %v", got, wantManagedByLabel)
133+
}
134+
135+
// Verify that only the managed-by label exists (other labels are not preserved)
136+
wantLabels := map[string]string{
137+
"fleet.azure.com/managed-by": "arm",
138+
}
139+
if diff := cmp.Diff(wantLabels, vap.Labels); diff != "" {
140+
t.Errorf("mutateValidatingAdmissionPolicy() labels mismatch (-want +got):\n%s", diff)
141+
}
142+
})
143+
}
144+
}
145+
53146
func TestGetValidatingAdmissionPolicyBinding(t *testing.T) {
54147
t.Parallel()
55148

56149
vap := getValidatingAdmissionPolicyBinding()
57-
assert.NotNil(t, vap)
150+
if vap == nil {
151+
t.Errorf("getValidatingAdmissionPolicyBinding() = nil, want non-nil")
152+
}
153+
}
154+
155+
func TestMutateValidatingAdmissionPolicyBinding(t *testing.T) {
156+
t.Parallel()
157+
158+
tests := []struct {
159+
name string
160+
resourceVersion string
161+
initialLabels map[string]string
162+
}{
163+
{
164+
name: "preserves ResourceVersion and updates labels",
165+
resourceVersion: "12345",
166+
initialLabels: map[string]string{"existing": "label"},
167+
},
168+
{
169+
name: "preserves empty ResourceVersion and sets labels",
170+
resourceVersion: "",
171+
initialLabels: nil,
172+
},
173+
{
174+
name: "overwrites existing managed label while preserving ResourceVersion",
175+
resourceVersion: "67890",
176+
initialLabels: map[string]string{"fleet.azure.com/managed-by": "old-value", "other": "label"},
177+
},
178+
}
179+
180+
for _, tt := range tests {
181+
t.Run(tt.name, func(t *testing.T) {
182+
t.Parallel()
183+
184+
vapb := &admv1.ValidatingAdmissionPolicyBinding{
185+
ObjectMeta: metav1.ObjectMeta{
186+
Name: "test-binding",
187+
ResourceVersion: tt.resourceVersion,
188+
Labels: tt.initialLabels,
189+
},
190+
}
191+
192+
mutateValidatingAdmissionPolicyBinding(vapb)
193+
194+
if vapb.ResourceVersion != tt.resourceVersion {
195+
t.Errorf("mutateValidatingAdmissionPolicyBinding() ResourceVersion = %v, want %v", vapb.ResourceVersion, tt.resourceVersion)
196+
}
197+
198+
wantManagedByLabel := "arm"
199+
if got := vapb.Labels["fleet.azure.com/managed-by"]; got != wantManagedByLabel {
200+
t.Errorf("mutateValidatingAdmissionPolicyBinding() managed-by label = %v, want %v", got, wantManagedByLabel)
201+
}
202+
203+
// Verify that only the managed-by label exists (other labels are not preserved)
204+
wantLabels := map[string]string{
205+
"fleet.azure.com/managed-by": "arm",
206+
}
207+
if diff := cmp.Diff(wantLabels, vapb.Labels); diff != "" {
208+
t.Errorf("mutateValidatingAdmissionPolicyBinding() labels mismatch (-want +got):\n%s", diff)
209+
}
210+
})
211+
}
58212
}

0 commit comments

Comments
 (0)