Fix: deny accept request when namespace is terminating. (#310)

Signed-off-by: xuezhaojun <zxue@redhat.com>
This commit is contained in:
xuezhaojun
2023-04-09 20:39:36 +08:00
committed by GitHub
parent 2238edce7d
commit fc792f3816
4 changed files with 265 additions and 10 deletions

View File

@@ -7,6 +7,9 @@ rules:
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["namespaces"]
verbs: ["get"]
# Allow managedcluster admission to create subjectaccessreviews
- apiGroups: ["authorization.k8s.io"]
resources: ["subjectaccessreviews"]

View File

@@ -19,6 +19,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
corev1 "k8s.io/api/core/v1"
)
var _ webhook.CustomValidator = &ManagedClusterWebhook{}
@@ -33,17 +35,22 @@ func (r *ManagedClusterWebhook) ValidateCreate(ctx context.Context, obj runtime.
if err != nil {
return apierrors.NewBadRequest(err.Error())
}
//Validate if Spec.ManagedClusterClientConfigs is Valid HTTPS URL
err = r.validateManagedClusterObj(*managedCluster)
if err != nil {
return err
}
// the HubAcceptsClient field is changed, we need to check the request user whether
// has been allowed to change the HubAcceptsClient field with SubjectAccessReview api
// the HubAcceptsClient field is changed, we need to:
// 1. check whether cluster namespace is terminating.
// 2. check the request user whether has been allowed to change the HubAcceptsClient field with
// SubjectAccessReview api.
if managedCluster.Spec.HubAcceptsClient {
err := r.allowUpdateAcceptField(managedCluster.Name, req.UserInfo)
if err != nil {
if err := r.validateAcceptByClusterNamespace(managedCluster.Name); err != nil {
return err
}
if err := r.allowUpdateAcceptField(managedCluster.Name, req.UserInfo); err != nil {
return err
}
}
@@ -78,12 +85,16 @@ func (r *ManagedClusterWebhook) ValidateUpdate(ctx context.Context, oldObj, newO
return err
}
// the HubAcceptsClient field is changed, we need to check the request user whether
// has been allowed to change the HubAcceptsClient field with SubjectAccessReview api
// the HubAcceptsClient field is changed, we need to:
// 1. check whether cluster namespace is terminating.
// 2. check the request user whether has been allowed to change the HubAcceptsClient field with
// SubjectAccessReview api.
if managedCluster.Spec.HubAcceptsClient != oldManagedCluster.Spec.HubAcceptsClient {
if managedCluster.Spec.HubAcceptsClient {
err := r.allowUpdateAcceptField(managedCluster.Name, req.UserInfo)
if err != nil {
if err := r.validateAcceptByClusterNamespace(managedCluster.Name); err != nil {
return err
}
if err := r.allowUpdateAcceptField(managedCluster.Name, req.UserInfo); err != nil {
return err
}
}
@@ -173,6 +184,29 @@ func (r *ManagedClusterWebhook) allowUpdateAcceptField(clusterName string, userI
return nil
}
// validateClusterNamespace checks the cluster namespace, if the namespace is terminating, reject the accept request.
func (r *ManagedClusterWebhook) validateAcceptByClusterNamespace(clusterName string) error {
clusterNamespace, err := r.kubeClient.CoreV1().Namespaces().Get(context.TODO(), clusterName, metav1.GetOptions{})
if apierrors.IsNotFound(err) {
return nil
}
if err != nil {
return apierrors.NewForbidden(
v1.Resource("managedclusters/accept"),
clusterName,
err,
)
}
if clusterNamespace.Status.Phase == corev1.NamespaceTerminating {
return apierrors.NewForbidden(
v1.Resource("managedclusters/accept"),
clusterName,
fmt.Errorf("cluster namespace %q is terminating", clusterName),
)
}
return nil
}
// allowSetClusterSetLabel checks whether a request user has been authorized to set clusterset label
func (r *ManagedClusterWebhook) allowSetClusterSetLabel(userInfo authenticationv1.UserInfo, originalClusterSet, newClusterSet string) error {
if originalClusterSet == newClusterSet {

View File

@@ -15,12 +15,15 @@ import (
clienttesting "k8s.io/client-go/testing"
v1 "open-cluster-management.io/api/cluster/v1"
"open-cluster-management.io/api/cluster/v1beta1"
corev1 "k8s.io/api/core/v1"
)
func TestValidateCreate(t *testing.T) {
cases := []struct {
name string
cluster *v1.ManagedCluster
preObjs []runtime.Object
expectedError bool
allowUpdateAcceptField bool
allowClusterset bool
@@ -61,6 +64,52 @@ func TestValidateCreate(t *testing.T) {
},
},
},
{
name: "validate cluster namespace, namespace is active",
expectedError: false,
allowUpdateAcceptField: true,
cluster: &v1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Spec: v1.ManagedClusterSpec{
HubAcceptsClient: true,
},
},
preObjs: []runtime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Status: corev1.NamespaceStatus{
Phase: corev1.NamespaceActive,
},
},
},
},
{
name: "validate cluster namespace, namespace is terminating",
expectedError: true,
allowUpdateAcceptField: true,
cluster: &v1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Spec: v1.ManagedClusterSpec{
HubAcceptsClient: true,
},
},
preObjs: []runtime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Status: corev1.NamespaceStatus{
Phase: corev1.NamespaceTerminating,
},
},
},
},
{
name: "validate setting clusterset label",
expectedError: false,
@@ -136,7 +185,7 @@ func TestValidateCreate(t *testing.T) {
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
kubeClient := kubefake.NewSimpleClientset()
kubeClient := kubefake.NewSimpleClientset(c.preObjs...)
kubeClient.PrependReactor(
"create",
"subjectaccessreviews",
@@ -194,6 +243,7 @@ func TestValidateUpdate(t *testing.T) {
name string
cluster *v1.ManagedCluster
oldCluster *v1.ManagedCluster
preObjs []runtime.Object
expectedError bool
allowUpdateAcceptField bool
allowClusterset bool
@@ -244,6 +294,66 @@ func TestValidateUpdate(t *testing.T) {
},
},
},
{
name: "validate update ManagedCluster when namespace is terminating",
expectedError: true,
allowUpdateAcceptField: true,
cluster: &v1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Spec: v1.ManagedClusterSpec{
HubAcceptsClient: true,
},
},
oldCluster: &v1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Spec: v1.ManagedClusterSpec{
HubAcceptsClient: false,
},
},
preObjs: []runtime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Status: corev1.NamespaceStatus{
Phase: corev1.NamespaceTerminating,
},
}},
},
{
name: "validate update ManagedCluster when namespace is active",
expectedError: false,
allowUpdateAcceptField: true,
cluster: &v1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Spec: v1.ManagedClusterSpec{
HubAcceptsClient: true,
},
},
oldCluster: &v1.ManagedCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Spec: v1.ManagedClusterSpec{
HubAcceptsClient: false,
},
},
preObjs: []runtime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "set-1",
},
Status: corev1.NamespaceStatus{
Phase: corev1.NamespaceActive,
},
}},
},
{
name: "validate updating an accepted ManagedCluster with permission",
expectedError: false,
@@ -443,7 +553,7 @@ func TestValidateUpdate(t *testing.T) {
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
kubeClient := kubefake.NewSimpleClientset()
kubeClient := kubefake.NewSimpleClientset(c.preObjs...)
kubeClient.PrependReactor(
"create",
"subjectaccessreviews",

View File

@@ -239,6 +239,62 @@ var _ = ginkgo.Describe("Admission webhook", func() {
gomega.Expect(cleanupClusterClient(saNamespace, sa)).ToNot(gomega.HaveOccurred())
})
ginkgo.It("Should forbid the request when creating a managed cluster with a termaniting namespace", func() {
var err error
sa := fmt.Sprintf("webhook-sa-%s", rand.String(6))
clusterName := fmt.Sprintf("webhook-spoke-%s", rand.String(6))
authorizedClient, err := buildClusterClient(saNamespace, sa, []rbacv1.PolicyRule{
{
APIGroups: []string{"cluster.open-cluster-management.io"},
Resources: []string{"managedclusters"},
Verbs: []string{"create", "get", "update"},
},
{
APIGroups: []string{"cluster.open-cluster-management.io"},
Resources: []string{"managedclustersets/join"},
Verbs: []string{"create"},
},
{
APIGroups: []string{"register.open-cluster-management.io"},
Resources: []string{"managedclusters/accept"},
ResourceNames: []string{clusterName},
Verbs: []string{"update"},
},
}, nil)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
// create a namespace, add a finilizer to it, and delete it
_, err = hubClient.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: clusterName,
Finalizers: []string{
"open-cluster-mangement.io/finalizer",
},
},
}, metav1.CreateOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
// delete the namespace
err = hubClient.CoreV1().Namespaces().Delete(context.TODO(), clusterName, metav1.DeleteOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
// create a managed cluster, should be denied
managedCluster := newManagedCluster(clusterName, true, validURL)
_, err = authorizedClient.ClusterV1().ManagedClusters().Create(context.TODO(), managedCluster, metav1.CreateOptions{})
gomega.Expect(errors.IsForbidden(err)).Should(gomega.BeTrue())
// remove the finalizer to truely delete the namespace
ns, err := hubClient.CoreV1().Namespaces().Get(context.TODO(), clusterName, metav1.GetOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
ns.Finalizers = []string{}
_, err = hubClient.CoreV1().Namespaces().Update(context.TODO(), ns, metav1.UpdateOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
gomega.Expect(cleanupClusterClient(saNamespace, sa)).ToNot(gomega.HaveOccurred())
})
ginkgo.It("Should accept the request when creating an accepted managed cluster by authorized user", func() {
sa := fmt.Sprintf("webhook-sa-%s", rand.String(6))
clusterName := fmt.Sprintf("webhook-spoke-%s", rand.String(6))
@@ -505,6 +561,58 @@ var _ = ginkgo.Describe("Admission webhook", func() {
gomega.Expect(cleanupClusterClient(saNamespace, sa)).ToNot(gomega.HaveOccurred())
})
ginkgo.It("Should forbid the request when updating a managed cluster with a terminating namespace", func() {
sa := fmt.Sprintf("webhook-sa-%s", rand.String(6))
var err error
authorizedClient, err := buildClusterClient(saNamespace, sa, []rbacv1.PolicyRule{
{
APIGroups: []string{"cluster.open-cluster-management.io"},
Resources: []string{"managedclusters"},
Verbs: []string{"create", "get", "update"},
},
{
APIGroups: []string{"cluster.open-cluster-management.io"},
Resources: []string{"managedclustersets/join"},
Verbs: []string{"create"},
},
{
APIGroups: []string{"register.open-cluster-management.io"},
Resources: []string{"managedclusters/accept"},
ResourceNames: []string{clusterName},
Verbs: []string{"update"},
},
}, nil)
gomega.Expect(err).ToNot(gomega.HaveOccurred())
// create a namespace, add a finilizer to it, and delete it
_, err = hubClient.CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: clusterName,
Finalizers: []string{
"open-cluster-mangement.io/finalizer",
},
},
}, metav1.CreateOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
// delete the namespace
err = hubClient.CoreV1().Namespaces().Delete(context.TODO(), clusterName, metav1.DeleteOptions{})
gomega.Expect(err).ToNot(gomega.HaveOccurred())
// update the HubAcceptsClient field to true
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
managedCluster, err := authorizedClient.ClusterV1().ManagedClusters().Get(context.TODO(), clusterName, metav1.GetOptions{})
gomega.Expect(err).NotTo(gomega.HaveOccurred())
managedCluster.Spec.HubAcceptsClient = true
_, err = authorizedClient.ClusterV1().ManagedClusters().Update(context.TODO(), managedCluster, metav1.UpdateOptions{})
return err
})
gomega.Expect(errors.IsForbidden(err)).To(gomega.BeTrue())
gomega.Expect(cleanupClusterClient(saNamespace, sa)).ToNot(gomega.HaveOccurred())
})
ginkgo.It("Should accept the request when updating the clusterset of a managed cluster by authorized user", func() {
clusterSetName := fmt.Sprintf("webhook-spoke-%s", rand.String(6))
ginkgo.By(fmt.Sprintf("create a managed cluster set %q", clusterSetName))