Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Arvindthiru committed Jun 15, 2023
1 parent f06af33 commit 0f8d5be
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 25 deletions.
File renamed without changes.
2 changes: 1 addition & 1 deletion pkg/webhook/validation/crdvalidation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package validation
import "k8s.io/utils/strings/slices"

var (
validObjectGroups = []string{"networking.fleet.azure.com", "fleet.azure.com", "multicluster.x-k8s.io"}
validObjectGroups = []string{"networking.fleet.azure.com", "fleet.azure.com", "multicluster.x-k8s.io", "placement.karavel.io"}
)

// CheckCRDGroup checks to see if the input CRD group is a fleet CRD group.
Expand Down
37 changes: 13 additions & 24 deletions test/e2e/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,7 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() {
},
},
}
err := HubCluster.KubeClient.Create(ctx, &cr)
Expect(err).Should(Succeed())
Expect(HubCluster.KubeClient.Create(ctx, &cr)).Should(Succeed())

By("create cluster role binding for test-user to modify CRD")
crb := rbacv1.ClusterRoleBinding{
Expand All @@ -511,8 +510,7 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() {
Name: testUserClusterRole,
},
}
err = HubCluster.KubeClient.Create(ctx, &crb)
Expect(err).Should(Succeed())
Expect(HubCluster.KubeClient.Create(ctx, &crb)).Should(Succeed())
})

AfterEach(func() {
Expand All @@ -522,44 +520,40 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() {
Name: testUserClusterRoleBinding,
},
}
err := HubCluster.KubeClient.Delete(ctx, &crb)
Expect(err).Should(Succeed())
Expect(HubCluster.KubeClient.Delete(ctx, &crb)).Should(Succeed())

By("remove cluster role")
cr := rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Name: testUserClusterRole,
},
}
err = HubCluster.KubeClient.Delete(ctx, &cr)
Expect(err).Should(Succeed())
Expect(HubCluster.KubeClient.Delete(ctx, &cr)).Should(Succeed())
})

Context("CRD validation webhook", func() {
It("should deny CREATE operation on Fleet CRD for user not in system:masters group", func() {
var crd v1.CustomResourceDefinition
err := utils.GetObjectFromManifest("./charts/hub-agent/templates/crds/fleet.azure.com_clusterresourceplacements.yaml", &crd)
Expect(err).Should(Succeed())
Expect(utils.GetObjectFromManifest("./charts/hub-agent/templates/crds/fleet.azure.com_clusterresourceplacements.yaml", &crd)).Should(Succeed())

By("expecting denial of operation CREATE of CRD")
err = HubCluster.ImpersonateKubeClient.Create(ctx, &crd)
err := HubCluster.ImpersonateKubeClient.Create(ctx, &crd)
var statusErr *k8sErrors.StatusError
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create CRD call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{})))
Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(crdStatusErrFormat, testUser, testGroups, crd.Name)))
})

It("should deny UPDATE operation on Fleet CRD for user not in system:masters group", func() {
var crd v1.CustomResourceDefinition
err := HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: "memberclusters.fleet.azure.com"}, &crd)
Expect(err).Should(Succeed())
Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: "memberclusters.fleet.azure.com"}, &crd)).Should(Succeed())

By("update labels in CRD")
labels := crd.GetLabels()
labels[testKey] = testValue
crd.SetLabels(labels)

By("expecting denial of operation UPDATE of CRD")
err = HubCluster.ImpersonateKubeClient.Update(ctx, &crd)
err := HubCluster.ImpersonateKubeClient.Update(ctx, &crd)
var statusErr *k8sErrors.StatusError
Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Update CRD call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{})))
Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(crdStatusErrFormat, testUser, testGroups, crd.Name)))
Expand All @@ -580,8 +574,7 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() {

It("should allow UPDATE operation on Fleet CRDs if user in system:masters group", func() {
var crd v1.CustomResourceDefinition
err := HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: "memberclusters.fleet.azure.com"}, &crd)
Expect(err).Should(Succeed())
Expect(HubCluster.KubeClient.Get(ctx, types.NamespacedName{Name: "memberclusters.fleet.azure.com"}, &crd)).Should(Succeed())

By("update labels in CRD")
labels := crd.GetLabels()
Expand All @@ -590,8 +583,7 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() {

By("expecting denial of operation UPDATE of CRD")
// The user associated with KubeClient is kubernetes-admin in groups: [system:masters, system:authenticated]
err = HubCluster.KubeClient.Update(ctx, &crd)
Expect(err).To(Succeed())
Expect(HubCluster.KubeClient.Update(ctx, &crd)).To(Succeed())

By("remove new label added for test")
labels = crd.GetLabels()
Expand All @@ -601,16 +593,13 @@ var _ = Describe("Fleet's CRD Resource Handler webhook tests", func() {

It("should allow CREATE operation on Other CRDs", func() {
var crd v1.CustomResourceDefinition
err := utils.GetObjectFromManifest("./test/integration/manifests/resources/test_clonesets_crd.yaml", &crd)
Expect(err).Should(Succeed())
Expect(utils.GetObjectFromManifest("./test/integration/manifests/resources/test_clonesets_crd.yaml", &crd)).Should(Succeed())

By("expecting error to be nil")
err = HubCluster.KubeClient.Create(ctx, &crd)
Expect(err).To(Succeed())
Expect(HubCluster.KubeClient.Create(ctx, &crd)).To(Succeed())

By("delete clone set CRD")
err = HubCluster.KubeClient.Delete(ctx, &crd)
Expect(err).To(Succeed())
Expect(HubCluster.KubeClient.Delete(ctx, &crd)).To(Succeed())
})
})
})

0 comments on commit 0f8d5be

Please sign in to comment.