From 39c28715fd0df9d8f789b508b786416ee5ec95dc Mon Sep 17 00:00:00 2001 From: hujiajing Date: Tue, 28 Jun 2022 23:48:33 +0800 Subject: [PATCH] Auto update ClusterSet in leader cluster Signed-off-by: hujiajing --- .../antrea-multicluster-leader-namespaced.yml | 7 + .../memberclusterannounce_webhook.go | 42 +++-- .../memberclusterannounce_webhook_test.go | 144 ++++++++++++++++-- .../commonarea/remote_common_area.go | 1 + .../member_clusterset_controller.go | 27 ++++ .../memberclusterannounce_controller.go | 56 ++++++- 6 files changed, 246 insertions(+), 31 deletions(-) diff --git a/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml b/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml index 272fb8d1c0f..5c28cec5e29 100644 --- a/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml +++ b/multicluster/build/yamls/antrea-multicluster-leader-namespaced.yml @@ -23,6 +23,13 @@ metadata: name: antrea-mc-controller-role namespace: antrea-multicluster rules: +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - watch + - list - apiGroups: - "" resources: diff --git a/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook.go b/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook.go index de7253b4a7c..4619d2cf5ce 100644 --- a/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook.go +++ b/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook.go @@ -21,6 +21,7 @@ import ( "fmt" "net/http" + v1 "k8s.io/api/core/v1" "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -54,6 +55,23 @@ func (v *memberClusterAnnounceValidator) Handle(ctx context.Context, req admissi return admission.Errored(http.StatusBadRequest, err) } + serviceAccountList := &v1.ServiceAccountList{} + if err := v.Client.List(context.TODO(), serviceAccountList, client.InNamespace(v.namespace)); err != nil { + klog.ErrorS(err, "Error reading ServiceAccount", "Namespace", v.namespace) + return admission.Errored(http.StatusPreconditionFailed, err) + } + + var found bool + for _, sa := range serviceAccountList.Items { + if sa.Name == saName { + found = true + break + } + } + if !found { + return admission.Denied("ServiceAccount of the token does not exist in the leader cluster") + } + // read the ClusterSet info clusterSetList := &multiclusterv1alpha1.ClusterSetList{} if err := v.Client.List(context.TODO(), clusterSetList, client.InNamespace(v.namespace)); err != nil { @@ -67,20 +85,20 @@ func (v *memberClusterAnnounceValidator) Handle(ctx context.Context, req admissi } clusterSet := clusterSetList.Items[0] - if clusterSet.Name == memberClusterAnnounce.ClusterSetID { - for _, member := range clusterSet.Spec.Members { - if member.ClusterID == memberClusterAnnounce.ClusterID { - // validate the ServiceAccount used is correct - if member.ServiceAccount == saName { - return admission.Allowed("") - } else { - return admission.Denied("Member does not have permissions") - } - } - } + if len(clusterSet.Spec.Leaders) != 1 { + return admission.Errored(http.StatusPreconditionFailed, + fmt.Errorf("invalid ClusterSet config in the leader cluster, there must be one leader cluster in the ClusterSet")) } - return admission.Denied("Unknown member") + if clusterSet.Name == memberClusterAnnounce.ClusterSetID && + clusterSet.Spec.Leaders[0].ClusterID == memberClusterAnnounce.LeaderClusterID { + return admission.Allowed("") + } + + if clusterSet.Name != memberClusterAnnounce.ClusterSetID { + return admission.Denied("Unknown ClusterSet ID") + } + return admission.Denied("The leader cluster ID in the MemberClusterAnnounce is not defined in the leader cluster config") } func (v *memberClusterAnnounceValidator) InjectDecoder(d *admission.Decoder) error { diff --git a/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook_test.go b/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook_test.go index 5863bfa23f6..73133394204 100644 --- a/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook_test.go +++ b/multicluster/cmd/multicluster-controller/memberclusterannounce_webhook_test.go @@ -19,23 +19,22 @@ package main import ( "context" j "encoding/json" + "testing" "github.com/stretchr/testify/assert" v1 "k8s.io/api/admission/v1" authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" k8smcsv1alpha1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" mcsv1alpha1 "antrea.io/antrea/multicluster/apis/multicluster/v1alpha1" - - "k8s.io/apimachinery/pkg/runtime" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/klog/v2" - - "testing" ) var mcaWebhookUnderTest *memberClusterAnnounceValidator @@ -65,11 +64,28 @@ func setup() { }, } + existingServiceAccounts := &corev1.ServiceAccountList{ + Items: []corev1.ServiceAccount{ + corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "mcs1", + Name: "east-access-sa", + }, + }, + corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "mcs1", + Name: "west-access-sa", + }, + }, + }, + } + newScheme := runtime.NewScheme() utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) utilruntime.Must(k8smcsv1alpha1.AddToScheme(newScheme)) utilruntime.Must(mcsv1alpha1.AddToScheme(newScheme)) - fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(existingClusterSet).Build() + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(existingClusterSet).WithLists(existingServiceAccounts).Build() mcaWebhookUnderTest = &memberClusterAnnounceValidator{ Client: fakeClient, @@ -132,7 +148,56 @@ func TestWebhookAllow(t *testing.T) { assert.Equal(t, true, response.Allowed) } -func TestWebhookDeniedUnknownMember(t *testing.T) { +func TestWebhookJoinAllow(t *testing.T) { + setup() + + mca := &mcsv1alpha1.MemberClusterAnnounce{ + ObjectMeta: metav1.ObjectMeta{ + Name: "member-announce-from-south", + Namespace: "mcs1", + }, + ClusterID: "south", + ClusterSetID: "clusterset1", + LeaderClusterID: "leader1", + } + b, _ := j.Marshal(mca) + + req := admission.Request{ + AdmissionRequest: v1.AdmissionRequest{ + UID: "07e52e8d-4513-11e9-a716-42010a800270", + Kind: metav1.GroupVersionKind{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Kind: "MemberClusterAnnounce", + }, + Resource: metav1.GroupVersionResource{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Resource: "memberclusterannounces", + }, + Name: "member-announce-from-south", + Namespace: "mcs1", + Operation: v1.Create, + Object: runtime.RawExtension{ + Raw: b, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "system:serviceaccount:mcs1:east-access-sa", + UID: "4842eb60-68e3-4e38-adad-3abfd6117241", + Groups: []string{ + "system:serviceaccounts", + "system:serviceaccounts:mcs1", + "system:authenticated", + }, + }, + }, + } + + response := mcaWebhookUnderTest.Handle(context.Background(), req) + assert.Equal(t, true, response.Allowed) +} + +func TestWebhookDeniedDifferentClusterSet(t *testing.T) { setup() mca := &mcsv1alpha1.MemberClusterAnnounce{ @@ -141,7 +206,7 @@ func TestWebhookDeniedUnknownMember(t *testing.T) { Namespace: "mcs1", }, ClusterID: "north", - ClusterSetID: "clusterset1", + ClusterSetID: "another-clusterset", LeaderClusterID: "leader1", } b, _ := j.Marshal(mca) @@ -179,18 +244,66 @@ func TestWebhookDeniedUnknownMember(t *testing.T) { response := mcaWebhookUnderTest.Handle(context.Background(), req) assert.Equal(t, false, response.Allowed) - assert.Equal(t, metav1.StatusReason("Unknown member"), response.Result.Reason) } -func TestWebhookDeniedNoPermission(t *testing.T) { +func TestWebhookDeniedDifferentLeaderCluster(t *testing.T) { setup() mca := &mcsv1alpha1.MemberClusterAnnounce{ ObjectMeta: metav1.ObjectMeta{ - Name: "member-announce-from-east", + Name: "member-announce-from-north", Namespace: "mcs1", }, - ClusterID: "east", + ClusterID: "north", + ClusterSetID: "clusterset1", + LeaderClusterID: "different-leader", + } + b, _ := j.Marshal(mca) + + req := admission.Request{ + AdmissionRequest: v1.AdmissionRequest{ + UID: "07e52e8d-4513-11e9-a716-42010a800270", + Kind: metav1.GroupVersionKind{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Kind: "MemberClusterAnnounce", + }, + Resource: metav1.GroupVersionResource{ + Group: "multicluster.crd.antrea.io", + Version: "v1alpha1", + Resource: "memberclusterannounces", + }, + Name: "member-announce-from-north", + Namespace: "mcs1", + Operation: v1.Create, + Object: runtime.RawExtension{ + Raw: b, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "system:serviceaccount:mcs1:east-access-sa", + UID: "4842eb60-68e3-4e38-adad-3abfd6117241", + Groups: []string{ + "system:serviceaccounts", + "system:serviceaccounts:mcs1", + "system:authenticated", + }, + }, + }, + } + + response := mcaWebhookUnderTest.Handle(context.Background(), req) + assert.Equal(t, false, response.Allowed) +} + +func TestWebhookDeniedUnknownServiceAccount(t *testing.T) { + setup() + + mca := &mcsv1alpha1.MemberClusterAnnounce{ + ObjectMeta: metav1.ObjectMeta{ + Name: "member-announce-from-south", + Namespace: "mcs1", + }, + ClusterID: "south", ClusterSetID: "clusterset1", LeaderClusterID: "leader1", } @@ -209,14 +322,14 @@ func TestWebhookDeniedNoPermission(t *testing.T) { Version: "v1alpha1", Resource: "memberclusterannounces", }, - Name: "member-announce-from-east", + Name: "member-announce-from-south", Namespace: "mcs1", Operation: v1.Create, Object: runtime.RawExtension{ Raw: b, }, UserInfo: authenticationv1.UserInfo{ - Username: "system:serviceaccount:mcs1:north-access-sa", + Username: "system:serviceaccount:mcs1:unknown-access-sa", UID: "4842eb60-68e3-4e38-adad-3abfd6117241", Groups: []string{ "system:serviceaccounts", @@ -229,5 +342,4 @@ func TestWebhookDeniedNoPermission(t *testing.T) { response := mcaWebhookUnderTest.Handle(context.Background(), req) assert.Equal(t, false, response.Allowed) - assert.Equal(t, metav1.StatusReason("Member does not have permissions"), response.Result.Reason) } diff --git a/multicluster/controllers/multicluster/commonarea/remote_common_area.go b/multicluster/controllers/multicluster/commonarea/remote_common_area.go index 9a072604f20..2755287c1af 100644 --- a/multicluster/controllers/multicluster/commonarea/remote_common_area.go +++ b/multicluster/controllers/multicluster/commonarea/remote_common_area.go @@ -232,6 +232,7 @@ func (r *remoteCommonArea) SendMemberAnnounce() error { localClusterMemberAnnounce.Name = "member-announce-from-" + string(r.remoteCommonAreaManager.GetLocalClusterID()) localClusterMemberAnnounce.Namespace = r.Namespace localClusterMemberAnnounce.ClusterSetID = string(r.ClusterSetID) + localClusterMemberAnnounce.LeaderClusterID = string(r.GetClusterID()) if err := r.Create(context.TODO(), &localClusterMemberAnnounce, &client.CreateOptions{}); err != nil { klog.ErrorS(err, "Error creating MemberClusterAnnounce", "Cluster", r.GetClusterID()) return err diff --git a/multicluster/controllers/multicluster/member_clusterset_controller.go b/multicluster/controllers/multicluster/member_clusterset_controller.go index 262d4370a67..89be1e70066 100644 --- a/multicluster/controllers/multicluster/member_clusterset_controller.go +++ b/multicluster/controllers/multicluster/member_clusterset_controller.go @@ -84,6 +84,9 @@ func (r *MemberClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, err } klog.InfoS("Received ClusterSet delete", "clusterset", req.NamespacedName) + if err := r.deleteMemberAnnounce(); err != nil { + return ctrl.Result{}, err + } stopErr := r.remoteCommonAreaManager.Stop() r.remoteCommonAreaManager = nil r.clusterSetConfig = nil @@ -345,3 +348,27 @@ func (r *MemberClusterSetReconciler) GetRemoteCommonAreaAndLocalID() (commonarea } return nil, "", errors.New("no connected remote common area") } + +func (r *MemberClusterSetReconciler) deleteMemberAnnounce() error { + memberClusterAnnounce := &multiclusterv1alpha1.MemberClusterAnnounce{} + + commonArea, ok := r.remoteCommonAreaManager.GetRemoteCommonAreas()[r.remoteCommonAreaManager.GetElectedLeaderClusterID()] + if !ok { + return fmt.Errorf("no common area for ClusetSet %s", r.clusterSetID) + } + + if err := commonArea.Get(context.TODO(), types.NamespacedName{ + Namespace: commonArea.GetNamespace(), + Name: "member-announce-from-" + string(r.clusterID), + }, memberClusterAnnounce); err != nil { + klog.ErrorS(err, "Failed to get MemberClusterAnnounce in leader cluster", "MemberClusterAnnounce", "member-announce-from-"+string(r.clusterID)) + return err + } + memberClusterAnnounce.Annotations[isDeletedAnnotation] = "true" + if err := commonArea.Update(context.TODO(), memberClusterAnnounce); err != nil { + klog.ErrorS(err, "Failed to update MemberClusterAnnounce", "member-announce-from-"+string(r.clusterID)) + return err + } + + return nil +} diff --git a/multicluster/controllers/multicluster/memberclusterannounce_controller.go b/multicluster/controllers/multicluster/memberclusterannounce_controller.go index cac4c884ce1..7a902c8b5e6 100644 --- a/multicluster/controllers/multicluster/memberclusterannounce_controller.go +++ b/multicluster/controllers/multicluster/memberclusterannounce_controller.go @@ -27,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -45,6 +46,8 @@ var ( TimerInterval = 5 * time.Second ConnectionTimeout = 3 * TimerInterval + + isDeletedAnnotation = "multicluster.antrea.io/is-member-deleted" ) type leaderStatus struct { @@ -138,10 +141,57 @@ func (r *MemberClusterAnnounceReconciler) Reconcile(ctx context.Context, req ctr } } } - // If err != nil, probably ClusterClaims were deleted during the processing of MemberClusterAnnounce. - // Nothing to handle in this case and MemberClusterAnnounce will also be deleted soon. - // TODO: Add ClusterClaim webhook to make sure it cannot be deleted while ClusterSet is present. } + // If err != nil, probably ClusterClaims were deleted during the processing of MemberClusterAnnounce. + // Nothing to handle in this case and MemberClusterAnnounce will also be deleted soon. + // TODO: Add ClusterClaim webhook to make sure it cannot be deleted while ClusterSet is present. + + clusterSetID := memberAnnounce.ClusterSetID + clusterSet := &multiclusterv1alpha1.ClusterSet{} + if err := r.Get(context.TODO(), types.NamespacedName{Namespace: memberAnnounce.Namespace, Name: clusterSetID}, clusterSet); err != nil { + if errors.IsNotFound(err) { + klog.ErrorS(err, "ClusterSet not found in leader cluster", "ClusterSet", clusterSetID) + return ctrl.Result{}, err + } + klog.ErrorS(err, "Failed to get ClusterSet in leader cluster", "ClusterSet", clusterSetID) + return ctrl.Result{}, err + } + if memberAnnounce.Annotations[isDeletedAnnotation] == "true" { + reservedMembers := []multiclusterv1alpha1.MemberCluster{} + + for _, member := range clusterSet.Spec.Members { + if member.ClusterID != memberAnnounce.ClusterID { + reservedMembers = append(reservedMembers, member) + } + } + clusterSet.Spec.Members = reservedMembers + if err := r.Update(context.TODO(), clusterSet); err != nil { + klog.ErrorS(err, "Failed to delete member cluster in ClusterSet", "memberCluster", memberAnnounce.ClusterID, "ClusterSet", clusterSet.Name) + return ctrl.Result{}, err + } + + if err := r.Delete(context.TODO(), memberAnnounce); err != nil { + klog.ErrorS(err, "Failed to delete MemberClusterAnnounce", "MemberClusterAnnounce", memberAnnounce.Name) + return ctrl.Result{}, err + } + } else { + isExist := false + for _, member := range clusterSet.Spec.Members { + if member.ClusterID == memberAnnounce.ClusterID { + isExist = true + break + } + } + + if !isExist { + clusterSet.Spec.Members = append(clusterSet.Spec.Members, multiclusterv1alpha1.MemberCluster{ClusterID: memberAnnounce.ClusterID}) + if err := r.Update(context.TODO(), clusterSet); err != nil { + klog.ErrorS(err, "Failed to add member cluster in ClusterSet", "memberCluster", memberAnnounce.ClusterID, "ClusterSet", clusterSet.Name) + return ctrl.Result{}, err + } + } + } + // Member not found. If this happens, the MemberClusterAnnounce should soon be deleted. // Nothing to do here.