Skip to content

Commit 65dd91c

Browse files
author
Nont
committed
Add e2e and fix comments
Signed-off-by: Nont <nont@duck.com>
1 parent 635d5f1 commit 65dd91c

File tree

7 files changed

+112
-20
lines changed

7 files changed

+112
-20
lines changed

‎pkg/webhook/managedresource/managedresource_validating_webhook.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,15 @@ import (
2424
)
2525

2626
const (
27-
managedByArmKey = "managed-by"
28-
managedByArmValue = "arm"
27+
ManagedByArmKey = "managed-by"
28+
ManagedByArmValue = "arm"
2929
deniedResource = "denied admission for managed resource"
3030
resourceDeniedFormat = "the operation on the managed resource type '%s' name '%s' in namespace '%s' is not allowed"
3131
)
3232

3333
// ValidationPath is the webhook service path which admission requests are routed to.
3434
var (
3535
ValidationPath = fmt.Sprintf(utils.ValidationPathFmt, "arm", "managed", "resources")
36-
metaAccessor = meta.NewAccessor()
3736
)
3837

3938
// Add registers the webhook for K8s bulit-in object types.
@@ -81,7 +80,7 @@ func managedByArm(m map[string]string) bool {
8180
if len(m) == 0 {
8281
return false
8382
}
84-
if v, ok := m[managedByArmKey]; ok && v == managedByArmValue {
83+
if v, ok := m[ManagedByArmKey]; ok && v == ManagedByArmValue {
8584
return true
8685
}
8786
return false

‎pkg/webhook/managedresource/managedresource_validating_webhook_test.go‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"testing"
1313

1414
"github.com/google/go-cmp/cmp"
15-
"github.com/stretchr/testify/assert"
1615
admissionv1 "k8s.io/api/admission/v1"
1716
authenticationv1 "k8s.io/api/authentication/v1"
1817
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -60,7 +59,7 @@ func Test_managedResourceValidzaator_Handle(t *testing.T) {
6059
operation: admissionv1.Create,
6160
oldLabels: nil,
6261
oldAnnotations: nil,
63-
newLabels: map[string]string{managedByArmKey: managedByArmValue},
62+
newLabels: map[string]string{ManagedByArmKey: ManagedByArmValue},
6463
newAnnotations: nil,
6564
expectedResp: admission.Denied(fmt.Sprintf(resourceDeniedFormat, metav1.GroupVersionKind{Kind: "TestKind"}, "test-resource", "default")),
6665
},
@@ -70,7 +69,7 @@ func Test_managedResourceValidzaator_Handle(t *testing.T) {
7069
oldLabels: nil,
7170
oldAnnotations: nil,
7271
newLabels: nil,
73-
newAnnotations: map[string]string{managedByArmKey: managedByArmValue},
72+
newAnnotations: map[string]string{ManagedByArmKey: ManagedByArmValue},
7473
expectedResp: admission.Denied(fmt.Sprintf(resourceDeniedFormat, metav1.GroupVersionKind{Kind: "TestKind"}, "test-resource", "default")),
7574
},
7675
{
@@ -86,7 +85,7 @@ func Test_managedResourceValidzaator_Handle(t *testing.T) {
8685
name: "allowed for other operations - managed by arm, but user whitelisted",
8786
username: "fleet1p",
8887
operation: admissionv1.Update,
89-
oldLabels: map[string]string{"managedBy": managedByArmValue},
88+
oldLabels: map[string]string{"managedBy": ManagedByArmValue},
9089
oldAnnotations: nil,
9190
newLabels: nil,
9291
newAnnotations: nil,
@@ -164,12 +163,14 @@ func Test_getLabelsAndAnnotations(t *testing.T) {
164163
for _, tt := range tests {
165164
t.Run(tt.name, func(t *testing.T) {
166165
labels, annotations, err := getLabelsAndAnnotations(tt.obj)
167-
if tt.expectError {
168-
assert.Error(t, err)
169-
} else {
170-
assert.NoError(t, err)
171-
assert.Equal(t, tt.wantLabels, labels)
172-
assert.Equal(t, tt.wantAnnotations, annotations)
166+
if err != nil && !tt.expectError {
167+
t.Fatalf("unexpected error: %v", err)
168+
}
169+
if diff := cmp.Diff(tt.wantLabels, labels); diff != "" {
170+
t.Errorf("labels mismatch (-want +got):\n%s", diff)
171+
}
172+
if diff := cmp.Diff(tt.wantAnnotations, annotations); diff != "" {
173+
t.Errorf("annotations mismatch (-want +got):\n%s", diff)
173174
}
174175
})
175176
}
@@ -198,24 +199,26 @@ func Test_managedByArm(t *testing.T) {
198199
},
199200
{
200201
name: "key present, not managed key",
201-
m: map[string]string{"managingBy": managedByArmValue},
202+
m: map[string]string{"managingBy": ManagedByArmValue},
202203
want: false,
203204
},
204205
{
205206
name: "key present, not managed value",
206-
m: map[string]string{managedByArmKey: "not-arm"},
207+
m: map[string]string{ManagedByArmKey: "not-arm"},
207208
want: false,
208209
},
209210
{
210211
name: "key present, managed key and value",
211-
m: map[string]string{managedByArmKey: managedByArmValue},
212+
m: map[string]string{ManagedByArmKey: ManagedByArmValue},
212213
want: true,
213214
},
214215
}
215216

216217
for _, tt := range tests {
217218
t.Run(tt.name, func(t *testing.T) {
218-
assert.Equal(t, tt.want, managedByArm(tt.m))
219+
if diff := cmp.Diff(tt.want, managedByArm(tt.m)); diff != "" {
220+
t.Errorf("managedByArm result (-want +got):\n%s", diff)
221+
}
219222
})
220223
}
221224
}

‎pkg/webhook/webhook.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ func (w *Config) buildFleetValidatingWebhooks() []admv1.ValidatingWebhook {
429429
networkingv1.SchemeGroupVersion.Version,
430430
},
431431
[]string{
432-
fleetv1alpha1.ClusterResourcePlacementResource,
432+
placementv1beta1.ClusterResourcePlacementResource,
433433
namespaceResourceName,
434434
resourceQuotaResourceName,
435435
networkPolicyResourceName,

‎pkg/webhook/webhook_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestBuildFleetValidatingWebhooks(t *testing.T) {
2525
serviceURL: "test-url",
2626
clientConnectionType: &url,
2727
},
28-
wantLength: 9,
28+
wantLength: 10,
2929
},
3030
}
3131

‎test/e2e/resources_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const (
5050
crpEvictionNameTemplate = "crpe-%d"
5151
updateRunStrategyNameTemplate = "curs-%d"
5252
updateRunNameWithSubIndexTemplate = "cur-%d-%d"
53+
managedNamespaceTemplate = "managedns-%d"
5354

5455
customDeletionBlockerFinalizer = "kubernetes-fleet.io/custom-deletion-blocker-finalizer"
5556
workNamespaceLabelName = "process"

‎test/e2e/utils_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import (
5252
"go.goms.io/fleet/pkg/propertyprovider/azure/trackers"
5353
"go.goms.io/fleet/pkg/utils"
5454
"go.goms.io/fleet/pkg/utils/condition"
55+
"go.goms.io/fleet/pkg/webhook/managedresource"
5556
testv1alpha1 "go.goms.io/fleet/test/apis/v1alpha1"
5657
"go.goms.io/fleet/test/e2e/framework"
5758
)
@@ -708,6 +709,20 @@ func cleanupWorkResources() {
708709
cleanWorkResourcesOnCluster(hubCluster)
709710
}
710711

712+
func createManagedNamespace() corev1.Namespace {
713+
ns := corev1.Namespace{
714+
ObjectMeta: metav1.ObjectMeta{
715+
Name: fmt.Sprintf(managedNamespaceTemplate, GinkgoParallelProcess()),
716+
Labels: map[string]string{
717+
managedresource.ManagedByArmKey: managedresource.ManagedByArmValue,
718+
},
719+
},
720+
}
721+
722+
Expect(hubClient.Create(ctx, &ns)).To(Succeed(), "Failed to create namespace %s", ns.Name)
723+
return ns
724+
}
725+
711726
func cleanWorkResourcesOnCluster(cluster *framework.Cluster) {
712727
ns := appNamespace()
713728
Expect(client.IgnoreNotFound(cluster.KubeClient.Delete(ctx, &ns))).To(Succeed(), "Failed to delete namespace %s", ns.Name)

‎test/e2e/webhook_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import (
3232
clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
3333
placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1"
3434
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
35+
"go.goms.io/fleet/pkg/utils"
36+
"go.goms.io/fleet/pkg/webhook/managedresource"
3537
testutils "go.goms.io/fleet/test/e2e/v1alpha1/utils"
3638
)
3739

@@ -1291,3 +1293,75 @@ var _ = Describe("webhook tests for ResourceOverride UPDATE operations", Ordered
12911293
}, testutils.PollTimeout, testutils.PollInterval).Should(Succeed())
12921294
})
12931295
})
1296+
1297+
var _ = Describe("webhook tests for operations on ARM managed resources", Ordered, func() {
1298+
BeforeAll(func() {
1299+
By("creating a managed namespace")
1300+
createManagedNamespace()
1301+
})
1302+
1303+
AfterAll(func() {
1304+
By("deleting the managed namespace")
1305+
ns := createManagedNamespace()
1306+
Expect(hubClient.Delete(ctx, &ns)).Should(SatisfyAny(Succeed(), utils.NotFoundMatcher{}), "Failed to delete the managed namespace")
1307+
Eventually(
1308+
func() error {
1309+
if err := hubClient.Get(ctx, types.NamespacedName{Name: ns.Name}, &corev1.Namespace{}); !k8sErrors.IsNotFound(err) {
1310+
return fmt.Errorf("The managed namespace still exists or an unexpected error occurred: %w", err)
1311+
}
1312+
return nil
1313+
}, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to remove the managed namespace %s", ns.Name)
1314+
})
1315+
1316+
It("should deny create a managed resource namespace", func() {
1317+
Eventually(func(g Gomega) error {
1318+
createNs := createManagedNamespace()
1319+
createNs.Name = fmt.Sprintf("test-create-managed-ns-%d", GinkgoParallelProcess())
1320+
err := impersonateHubClient.Update(ctx, &createNs)
1321+
if k8sErrors.IsConflict(err) {
1322+
return err
1323+
}
1324+
var statusErr *k8sErrors.StatusError
1325+
g.Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create managed namespace call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{})))
1326+
Expect(statusErr.ErrStatus.Message).Should(MatchRegexp("the operation on the managed resource type * is not allowed"))
1327+
return nil
1328+
}, testutils.PollTimeout, testutils.PollInterval).Should(Succeed())
1329+
})
1330+
1331+
It("should deny update a managed resource namespace", func() {
1332+
Eventually(func(g Gomega) error {
1333+
updateNamespace := createManagedNamespace()
1334+
updateNamespace.Labels["foo"] = "NotManaged"
1335+
err := impersonateHubClient.Update(ctx, &updateNamespace)
1336+
if k8sErrors.IsConflict(err) {
1337+
return err
1338+
}
1339+
var statusErr *k8sErrors.StatusError
1340+
g.Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Update managed namespace call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{})))
1341+
Expect(statusErr.ErrStatus.Message).Should(MatchRegexp("the operation on the managed resource type * is not allowed"))
1342+
return nil
1343+
}, testutils.PollTimeout, testutils.PollInterval).Should(Succeed())
1344+
})
1345+
1346+
It("should deny delete a managed resource namespace", func() {
1347+
Eventually(func(g Gomega) error {
1348+
created := createManagedNamespace()
1349+
err := impersonateHubClient.Delete(ctx, &created)
1350+
if k8sErrors.IsConflict(err) {
1351+
return err
1352+
}
1353+
var statusErr *k8sErrors.StatusError
1354+
g.Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Delete managed namespace call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{})))
1355+
Expect(statusErr.ErrStatus.Message).Should(MatchRegexp("the operation on the managed resource type * is not allowed"))
1356+
return nil
1357+
}, testutils.PollTimeout, testutils.PollInterval).Should(Succeed())
1358+
})
1359+
1360+
It("should allow create an unmanaged resource namespace", func() {
1361+
Eventually(func(g Gomega) error {
1362+
creating := createManagedNamespace()
1363+
delete(creating.Labels, managedresource.ManagedByArmKey)
1364+
return impersonateHubClient.Create(ctx, &creating)
1365+
}, testutils.PollTimeout, testutils.PollInterval).Should(Succeed())
1366+
})
1367+
})

0 commit comments

Comments
 (0)