From 077793fb37ef58a4ef09c7e69d6285e7ae86c5bb Mon Sep 17 00:00:00 2001 From: Bowen Zhu Date: Thu, 7 Jul 2022 10:12:32 +0800 Subject: [PATCH] Add V1beta1CSRAPICompatibility feature gate for registration-controller (#259) * Add V1beta1CSRAPICompatibility feature gate for registration-controller Signed-off-by: Promacanthus * Add csrInfo and update isSpokeClusterClientCertRenewal Signed-off-by: Promacanthus * Rerun go mod vendor Signed-off-by: Promacanthus * Register V1beta1CSRAPICompatibility and refacotr authorize function Signed-off-by: Promacanthus * Update api dependency Signed-off-by: Promacanthus --- CONTRIBUTING.md | 21 ++- go.mod | 2 +- go.sum | 4 +- pkg/helpers/helpers.go | 14 ++ pkg/helpers/testing/assertion.go | 25 +++ pkg/helpers/testing/testinghelpers.go | 52 ++++++ pkg/hub/csr/controller.go | 88 +++++++--- pkg/hub/csr/controller_beta.go | 96 +++++++++++ pkg/hub/csr/controller_beta_test.go | 157 ++++++++++++++++++ pkg/hub/csr/controller_test.go | 2 +- pkg/hub/manager.go | 34 +++- vendor/modules.txt | 2 +- .../api/cluster/v1beta1/helpers.go | 67 ++++++++ .../api/feature/feature.go | 8 +- .../api/work/v1/types.go | 3 + 15 files changed, 533 insertions(+), 42 deletions(-) create mode 100644 pkg/hub/csr/controller_beta.go create mode 100644 pkg/hub/csr/controller_beta_test.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 00dd4da8b..220053734 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,12 +3,12 @@ **Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* - [Contributing guidelines](#contributing-guidelines) - - [Contributions](#contributions) - - [Certificate of Origin](#certificate-of-origin) - - [Contributing A Patch](#contributing-a-patch) - - [Issue and Pull Request Management](#issue-and-pull-request-management) - - [Pre-check before submitting a PR](#pre-check-before-submitting-a-pr) - - [Build images](#build-images) + - [Contributions](#contributions) + - [Certificate of Origin](#certificate-of-origin) + - [Contributing A Patch](#contributing-a-patch) + - [Issue and Pull Request Management](#issue-and-pull-request-management) + - [Pre-check before submitting a PR](#pre-check-before-submitting-a-pr) + - [Build images](#build-images) @@ -45,19 +45,26 @@ Repo maintainers can assign you an issue or pull request by leaving a After your PR is ready to commit, please run following commands to check your code. -- verify your code +- verify your code + ```shell make verify ``` + - run the unit test + ```shell make test ``` + - run the integration test + ```shell make test-integration ``` + - run the end-to-end test, prepare a kind cluster and run the following command + ```shell export KUBECONFIG={the kubeconfig of your kind cluster that will run the e2e test} diff --git a/go.mod b/go.mod index d7a22fcba..6644b5a50 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( k8s.io/klog/v2 v2.60.1 k8s.io/kube-aggregator v0.23.5 k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 - open-cluster-management.io/api v0.7.1-0.20220609033924-5cc58e815c1a + open-cluster-management.io/api v0.7.1-0.20220706083507-8e98feac6b08 sigs.k8s.io/controller-runtime v0.11.1 ) diff --git a/go.sum b/go.sum index 86b4ac1b2..c13bf2500 100644 --- a/go.sum +++ b/go.sum @@ -1231,8 +1231,8 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk= modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k= modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs= modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I= -open-cluster-management.io/api v0.7.1-0.20220609033924-5cc58e815c1a h1:WasoJMZTgGkQLWfpSU3qtP885oa+o/Uqo2qtGWvSleY= -open-cluster-management.io/api v0.7.1-0.20220609033924-5cc58e815c1a/go.mod h1:+OEARSAl2jIhuLItUcS30UgLA3khmA9ihygLVxzEn+U= +open-cluster-management.io/api v0.7.1-0.20220706083507-8e98feac6b08 h1:c15u9p4c4UWHCGS8vcPE2T/DaUvGWDiqITHaGZZbb+w= +open-cluster-management.io/api v0.7.1-0.20220706083507-8e98feac6b08/go.mod h1:+OEARSAl2jIhuLItUcS30UgLA3khmA9ihygLVxzEn+U= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 83bffbef7..7a6b64fe8 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -21,6 +21,7 @@ import ( errorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers" certificatesv1 "k8s.io/api/certificates/v1" + certificatesv1beta1 "k8s.io/api/certificates/v1beta1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -203,6 +204,19 @@ func IsCSRInTerminalState(status *certificatesv1.CertificateSigningRequestStatus return false } +// Isv1beta1CSRInTerminalState checks whether a CSR is in terminal state for v1beta1 version. +func Isv1beta1CSRInTerminalState(status *certificatesv1beta1.CertificateSigningRequestStatus) bool { + for _, c := range status.Conditions { + if c.Type == certificatesv1beta1.CertificateApproved { + return true + } + if c.Type == certificatesv1beta1.CertificateDenied { + return true + } + } + return false +} + // IsValidHTTPSURL validate whether a URL is https URL func IsValidHTTPSURL(serverURL string) bool { if serverURL == "" { diff --git a/pkg/helpers/testing/assertion.go b/pkg/helpers/testing/assertion.go index 12be17bb3..34607e01b 100644 --- a/pkg/helpers/testing/assertion.go +++ b/pkg/helpers/testing/assertion.go @@ -12,6 +12,7 @@ import ( authorizationv1 "k8s.io/api/authorization/v1" certv1 "k8s.io/api/certificates/v1" + certv1beta1 "k8s.io/api/certificates/v1beta1" coordinationv1 "k8s.io/api/coordination/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -193,6 +194,30 @@ func AssertCSRCondition( } } +// AssertV1beta1CSRCondition asserts the actual csr conditions has the expected condition +func AssertV1beta1CSRCondition( + t *testing.T, + actualConditions []certv1beta1.CertificateSigningRequestCondition, + expectedCondition certv1beta1.CertificateSigningRequestCondition) { + var cond *certv1beta1.CertificateSigningRequestCondition + for i := range actualConditions { + condition := actualConditions[i] + if condition.Type == expectedCondition.Type { + cond = &condition + break + } + } + if cond == nil { + t.Errorf("expected condition %s but got: %s", expectedCondition.Type, cond.Type) + } + if cond.Reason != expectedCondition.Reason { + t.Errorf("expected reason %s but got: %s", expectedCondition.Reason, cond.Reason) + } + if cond.Message != expectedCondition.Message { + t.Errorf("expected message %s but got: %s", expectedCondition.Message, cond.Message) + } +} + // AssertLeaseUpdated asserts the lease obj is updated func AssertLeaseUpdated(t *testing.T, lease, lastLease *coordinationv1.Lease) { if lease == nil || lastLease == nil { diff --git a/pkg/helpers/testing/testinghelpers.go b/pkg/helpers/testing/testinghelpers.go index 6d2a37da0..79473ff14 100644 --- a/pkg/helpers/testing/testinghelpers.go +++ b/pkg/helpers/testing/testinghelpers.go @@ -22,6 +22,7 @@ import ( "github.com/openshift/library-go/pkg/operator/events/eventstesting" certv1 "k8s.io/api/certificates/v1" + certv1beta1 "k8s.io/api/certificates/v1beta1" coordv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -331,6 +332,39 @@ func NewCSR(holder CSRHolder) *certv1.CertificateSigningRequest { } } +func NewV1beta1CSR(holder CSRHolder) *certv1beta1.CertificateSigningRequest { + insecureRand := rand.New(rand.NewSource(0)) + pk, err := ecdsa.GenerateKey(elliptic.P256(), insecureRand) + if err != nil { + panic(err) + } + csrb, err := x509.CreateCertificateRequest(insecureRand, &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: holder.CN, + Organization: holder.Orgs, + }, + DNSNames: []string{}, + EmailAddresses: []string{}, + IPAddresses: []net.IP{}, + }, pk) + if err != nil { + panic(err) + } + return &certv1beta1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: holder.Name, + GenerateName: "csr-", + Labels: holder.Labels, + }, + Spec: certv1beta1.CertificateSigningRequestSpec{ + Username: holder.Username, + Usages: []certv1beta1.KeyUsage{}, + SignerName: &holder.SignerName, + Request: pem.EncodeToMemory(&pem.Block{Type: holder.ReqBlockType, Bytes: csrb}), + }, + } +} + func NewDeniedCSR(holder CSRHolder) *certv1.CertificateSigningRequest { csr := NewCSR(holder) csr.Status.Conditions = append(csr.Status.Conditions, certv1.CertificateSigningRequestCondition{ @@ -340,6 +374,15 @@ func NewDeniedCSR(holder CSRHolder) *certv1.CertificateSigningRequest { return csr } +func NewDeniedV1beta1CSR(holder CSRHolder) *certv1beta1.CertificateSigningRequest { + csr := NewV1beta1CSR(holder) + csr.Status.Conditions = append(csr.Status.Conditions, certv1beta1.CertificateSigningRequestCondition{ + Type: certv1beta1.CertificateDenied, + Status: corev1.ConditionTrue, + }) + return csr +} + func NewApprovedCSR(holder CSRHolder) *certv1.CertificateSigningRequest { csr := NewCSR(holder) csr.Status.Conditions = append(csr.Status.Conditions, certv1.CertificateSigningRequestCondition{ @@ -349,6 +392,15 @@ func NewApprovedCSR(holder CSRHolder) *certv1.CertificateSigningRequest { return csr } +func NewApprovedV1beta1CSR(holder CSRHolder) *certv1beta1.CertificateSigningRequest { + csr := NewV1beta1CSR(holder) + csr.Status.Conditions = append(csr.Status.Conditions, certv1beta1.CertificateSigningRequestCondition{ + Type: certv1beta1.CertificateApproved, + Status: corev1.ConditionTrue, + }) + return csr +} + func NewKubeconfig(key, cert []byte) []byte { var clientKey, clientCertificate string var clientKeyData, clientCertificateData []byte diff --git a/pkg/hub/csr/controller.go b/pkg/hub/csr/controller.go index 41ac04ae1..201c33019 100644 --- a/pkg/hub/csr/controller.go +++ b/pkg/hub/csr/controller.go @@ -11,6 +11,7 @@ import ( "github.com/openshift/library-go/pkg/operator/events" authorizationv1 "k8s.io/api/authorization/v1" certificatesv1 "k8s.io/api/certificates/v1" + certificatesv1beta1 "k8s.io/api/certificates/v1beta1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -21,7 +22,6 @@ import ( "k8s.io/client-go/kubernetes" certificateslisters "k8s.io/client-go/listers/certificates/v1" "k8s.io/klog/v2" - "open-cluster-management.io/registration/pkg/helpers" "open-cluster-management.io/registration/pkg/hub/user" ) @@ -70,15 +70,16 @@ func (c *csrApprovingController) sync(ctx context.Context, syncCtx factory.SyncC return nil } + csrInfo := newCSRInfo(csr) // Check whether current csr is a renewal spoker cluster csr. - isRenewal := isSpokeClusterClientCertRenewal(csr) + isRenewal := isSpokeClusterClientCertRenewal(csrInfo) if !isRenewal { klog.V(4).Infof("CSR %q was not recognized", csr.Name) return nil } // Authorize whether the current spoke agent has been authorized to renew its csr. - allowed, err := c.authorize(ctx, csr) + allowed, err := authorize(ctx, c.kubeClient, csrInfo) if err != nil { return err } @@ -105,18 +106,13 @@ func (c *csrApprovingController) sync(ctx context.Context, syncCtx factory.SyncC // Using SubjectAccessReview API to check whether a spoke agent has been authorized to renew its csr, // a spoke agent is authorized after its spoke cluster is accepted by hub cluster admin. -func (c *csrApprovingController) authorize(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) (bool, error) { - extra := make(map[string]authorizationv1.ExtraValue) - for k, v := range csr.Spec.Extra { - extra[k] = authorizationv1.ExtraValue(v) - } - +func authorize(ctx context.Context, kubeClient kubernetes.Interface, csr csrInfo) (bool, error) { sar := &authorizationv1.SubjectAccessReview{ Spec: authorizationv1.SubjectAccessReviewSpec{ - User: csr.Spec.Username, - UID: csr.Spec.UID, - Groups: csr.Spec.Groups, - Extra: extra, + User: csr.username, + UID: csr.uid, + Groups: csr.groups, + Extra: csr.extra, ResourceAttributes: &authorizationv1.ResourceAttributes{ Group: "register.open-cluster-management.io", Resource: "managedclusters", @@ -125,7 +121,8 @@ func (c *csrApprovingController) authorize(ctx context.Context, csr *certificate }, }, } - sar, err := c.kubeClient.AuthorizationV1().SubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{}) + + sar, err := kubeClient.AuthorizationV1().SubjectAccessReviews().Create(ctx, sar, metav1.CreateOptions{}) if err != nil { return false, err } @@ -136,25 +133,25 @@ func (c *csrApprovingController) authorize(ctx context.Context, csr *certificate // 1. if the signer name in csr request is valid. // 2. if organization field and commonName field in csr request is valid. // 3. if user name in csr is the same as commonName field in csr request. -func isSpokeClusterClientCertRenewal(csr *certificatesv1.CertificateSigningRequest) bool { - spokeClusterName, existed := csr.Labels[spokeClusterNameLabel] +func isSpokeClusterClientCertRenewal(csr csrInfo) bool { + spokeClusterName, existed := csr.labels[spokeClusterNameLabel] if !existed { return false } - if csr.Spec.SignerName != certificatesv1.KubeAPIServerClientSignerName { + if csr.signerName != certificatesv1.KubeAPIServerClientSignerName { return false } - block, _ := pem.Decode(csr.Spec.Request) + block, _ := pem.Decode(csr.request) if block == nil || block.Type != "CERTIFICATE REQUEST" { - klog.V(4).Infof("csr %q was not recognized: PEM block type is not CERTIFICATE REQUEST", csr.Name) + klog.V(4).Infof("csr %q was not recognized: PEM block type is not CERTIFICATE REQUEST", csr.name) return false } x509cr, err := x509.ParseCertificateRequest(block.Bytes) if err != nil { - klog.V(4).Infof("csr %q was not recognized: %v", csr.Name, err) + klog.V(4).Infof("csr %q was not recognized: %v", csr.name, err) return false } @@ -175,5 +172,54 @@ func isSpokeClusterClientCertRenewal(csr *certificatesv1.CertificateSigningReque return false } - return csr.Spec.Username == x509cr.Subject.CommonName + return csr.username == x509cr.Subject.CommonName +} + +type csrInfo struct { + name string + labels map[string]string + signerName string + username string + uid string + groups []string + extra map[string]authorizationv1.ExtraValue + request []byte +} + +// newCSRInfo creates csrInfo from CertificateSigningRequest by api version(v1/v1beta1). +func newCSRInfo(csr any) csrInfo { + extra := make(map[string]authorizationv1.ExtraValue) + switch v := csr.(type) { + case *certificatesv1.CertificateSigningRequest: + for k, v := range v.Spec.Extra { + extra[k] = authorizationv1.ExtraValue(v) + } + return csrInfo{ + name: v.Name, + labels: v.Labels, + signerName: v.Spec.SignerName, + username: v.Spec.Username, + uid: v.Spec.UID, + groups: v.Spec.Groups, + extra: extra, + request: v.Spec.Request, + } + case *certificatesv1beta1.CertificateSigningRequest: + for k, v := range v.Spec.Extra { + extra[k] = authorizationv1.ExtraValue(v) + } + return csrInfo{ + name: v.Name, + labels: v.Labels, + signerName: *v.Spec.SignerName, + username: v.Spec.Username, + uid: v.Spec.UID, + groups: v.Spec.Groups, + extra: extra, + request: v.Spec.Request, + } + default: + klog.Errorf("Unsupported type %T", v) + return csrInfo{} + } } diff --git a/pkg/hub/csr/controller_beta.go b/pkg/hub/csr/controller_beta.go new file mode 100644 index 000000000..87d55cf22 --- /dev/null +++ b/pkg/hub/csr/controller_beta.go @@ -0,0 +1,96 @@ +package csr + +import ( + "context" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes" + "k8s.io/klog/v2" + "open-cluster-management.io/registration/pkg/helpers" + + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + certificatesv1beta1 "k8s.io/api/certificates/v1beta1" + corev1 "k8s.io/api/core/v1" + certificatesv1beta1informers "k8s.io/client-go/informers/certificates/v1beta1" + certificatesv1beta1lister "k8s.io/client-go/listers/certificates/v1beta1" +) + +// v1beta1CSRApprovingController auto approve the renewal CertificateSigningRequests for an accepted spoke cluster on the hub. +type v1beta1CSRApprovingController struct { + kubeClient kubernetes.Interface + csrLister certificatesv1beta1lister.CertificateSigningRequestLister + eventRecorder events.Recorder +} + +func NewV1beta1CSRApprovingController( + kubeClient kubernetes.Interface, + v1beta1CSRInformer certificatesv1beta1informers.CertificateSigningRequestInformer, + recorder events.Recorder) factory.Controller { + + c := &v1beta1CSRApprovingController{ + kubeClient: kubeClient, + csrLister: v1beta1CSRInformer.Lister(), + eventRecorder: recorder.WithComponentSuffix("csr-approving-controller"), + } + + return factory.New().WithInformersQueueKeyFunc(func(obj runtime.Object) string { + accessor, _ := meta.Accessor(obj) + return accessor.GetName() + }, v1beta1CSRInformer.Informer()). + WithSync(c.sync). + ToController("V1Beta1CSRApprovingController", recorder) +} + +func (c *v1beta1CSRApprovingController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + csrName := syncCtx.QueueKey() + klog.V(4).Infof("Reconciling CertificateSigningRequests %q", csrName) + csr, err := c.csrLister.Get(csrName) + if errors.IsNotFound(err) { + return nil + } + if err != nil { + return err + } + + csr = csr.DeepCopy() + // Current csr is in terminal state, do nothing. + if helpers.Isv1beta1CSRInTerminalState(&csr.Status) { + return nil + } + + csrInfo := newCSRInfo(csr) + // Check whether current csr is a renewal spoke cluster csr. + isRenewal := isSpokeClusterClientCertRenewal(csrInfo) + if !isRenewal { + klog.V(4).Infof("CSR %q was not recognized", csr.Name) + return nil + } + + allowed, err := authorize(ctx, c.kubeClient, csrInfo) + if err != nil { + return err + } + if !allowed { + //TODO find a way to avoid looking at this CSR again. + klog.V(4).Infof("Managed cluster csr %q cannont be auto approved due to subject access review was not approved", csr.Name) + return nil + } + + // Auto approve the spoke cluster csr + csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1beta1.CertificateSigningRequestCondition{ + Type: certificatesv1beta1.CertificateApproved, + Status: corev1.ConditionTrue, + Reason: "AutoApprovedByHubCSRApprovingController", + Message: "Auto approving Managed cluster agent certificate after SubjectAccessReview.", + }) + _, err = c.kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, csr, metav1.UpdateOptions{}) + if err != nil { + return err + } + c.eventRecorder.Eventf("ManagedClusterCSRAutoApproved", "spoke cluster csr %q is auto approved by hub csr controller", csr.Name) + return nil +} diff --git a/pkg/hub/csr/controller_beta_test.go b/pkg/hub/csr/controller_beta_test.go new file mode 100644 index 000000000..36aa03e4e --- /dev/null +++ b/pkg/hub/csr/controller_beta_test.go @@ -0,0 +1,157 @@ +package csr + +import ( + "context" + "testing" + "time" + + "github.com/openshift/library-go/pkg/operator/events/eventstesting" + authorizationv1 "k8s.io/api/authorization/v1" + certificatesv1beta1 "k8s.io/api/certificates/v1beta1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/informers" + kubefake "k8s.io/client-go/kubernetes/fake" + clienttesting "k8s.io/client-go/testing" + testinghelpers "open-cluster-management.io/registration/pkg/helpers/testing" + "open-cluster-management.io/registration/pkg/hub/user" +) + +var validV1beta1CSR = testinghelpers.CSRHolder{ + Name: "testcsr", + Labels: map[string]string{"open-cluster-management.io/cluster-name": "managedcluster1"}, + SignerName: certificatesv1beta1.KubeAPIServerClientSignerName, + CN: user.SubjectPrefix + "managedcluster1:spokeagent1", + Orgs: []string{user.SubjectPrefix + "managedcluster1", user.ManagedClustersGroup}, + Username: user.SubjectPrefix + "managedcluster1:spokeagent1", + ReqBlockType: "CERTIFICATE REQUEST", +} + +func Test_v1beta1CSRApprovingController_sync(t *testing.T) { + tests := []struct { + name string + startingCSRs []runtime.Object + autoApprovingAllowed bool + wantErr bool + validateActions func(t *testing.T, actions []clienttesting.Action) + }{ + { + name: "sync a deleted csr", + startingCSRs: []runtime.Object{}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertNoActions(t, actions) + }, + }, + { + name: "sync a denied csr", + startingCSRs: []runtime.Object{testinghelpers.NewDeniedV1beta1CSR(validV1beta1CSR)}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertNoActions(t, actions) + }, + }, + { + name: "sync an approved csr", + startingCSRs: []runtime.Object{testinghelpers.NewApprovedV1beta1CSR(validV1beta1CSR)}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertNoActions(t, actions) + }, + }, + { + name: "sync an invalid csr", + startingCSRs: []runtime.Object{testinghelpers.NewV1beta1CSR(testinghelpers.CSRHolder{ + Name: validV1beta1CSR.Name, + Labels: validV1beta1CSR.Labels, + SignerName: validV1beta1CSR.SignerName, + CN: "system:open-cluster-management:managedcluster1:invalidagent", + Orgs: validV1beta1CSR.Orgs, + Username: validV1beta1CSR.Username, + ReqBlockType: validV1beta1CSR.ReqBlockType, + })}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertNoActions(t, actions) + }, + }, + { + name: "deny an auto approving csr", + startingCSRs: []runtime.Object{testinghelpers.NewV1beta1CSR(validV1beta1CSR)}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertActions(t, actions, "create") + testinghelpers.AssertSubjectAccessReviewObj(t, actions[0].(clienttesting.CreateActionImpl).Object) + }, + }, + { + name: "allow an auto approving csr", + startingCSRs: []runtime.Object{testinghelpers.NewV1beta1CSR(validV1beta1CSR)}, + autoApprovingAllowed: true, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + expectedCondition := certificatesv1beta1.CertificateSigningRequestCondition{ + Type: certificatesv1beta1.CertificateApproved, + Status: corev1.ConditionTrue, + Reason: "AutoApprovedByHubCSRApprovingController", + Message: "Auto approving Managed cluster agent certificate after SubjectAccessReview.", + } + testinghelpers.AssertActions(t, actions, "create", "update") + actual := actions[1].(clienttesting.UpdateActionImpl).Object + testinghelpers.AssertV1beta1CSRCondition(t, actual.(*certificatesv1beta1.CertificateSigningRequest).Status.Conditions, expectedCondition) + }, + }, + { + name: "allow an auto approving csr w/o ManagedClusterGroup for backward-compatibility", + startingCSRs: []runtime.Object{testinghelpers.NewV1beta1CSR(testinghelpers.CSRHolder{ + Name: validV1beta1CSR.Name, + Labels: validV1beta1CSR.Labels, + SignerName: validV1beta1CSR.SignerName, + CN: validV1beta1CSR.CN, + Orgs: sets.NewString(validV1beta1CSR.Orgs...).Delete(user.ManagedClustersGroup).List(), + Username: validV1beta1CSR.Username, + ReqBlockType: validV1beta1CSR.ReqBlockType, + })}, + autoApprovingAllowed: true, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + expectedCondition := certificatesv1beta1.CertificateSigningRequestCondition{ + Type: certificatesv1beta1.CertificateApproved, + Status: corev1.ConditionTrue, + Reason: "AutoApprovedByHubCSRApprovingController", + Message: "Auto approving Managed cluster agent certificate after SubjectAccessReview.", + } + testinghelpers.AssertActions(t, actions, "create", "update") + actual := actions[1].(clienttesting.UpdateActionImpl).Object + testinghelpers.AssertV1beta1CSRCondition(t, actual.(*certificatesv1beta1.CertificateSigningRequest).Status.Conditions, expectedCondition) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + kubeClient := kubefake.NewSimpleClientset(tt.startingCSRs...) + kubeClient.PrependReactor( + "create", + "subjectaccessreviews", + func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, + &authorizationv1.SubjectAccessReview{ + Status: authorizationv1.SubjectAccessReviewStatus{Allowed: tt.autoApprovingAllowed}, + }, nil + }, + ) + + informerFactory := informers.NewSharedInformerFactory(kubeClient, 3*time.Minute) + csrStore := informerFactory.Certificates().V1beta1().CertificateSigningRequests().Informer().GetStore() + for _, csr := range tt.startingCSRs { + if err := csrStore.Add(csr); err != nil { + t.Fatal(err) + } + } + + ctrl := &v1beta1CSRApprovingController{ + kubeClient, + informerFactory.Certificates().V1beta1().CertificateSigningRequests().Lister(), + eventstesting.NewTestingEventRecorder(t), + } + if err := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, validV1beta1CSR.Name)); (err != nil) != tt.wantErr { + t.Errorf("v1beta1CSRApprovingController.sync() error = %v, wantErr %v", err, tt.wantErr) + } + tt.validateActions(t, kubeClient.Actions()) + }) + } +} diff --git a/pkg/hub/csr/controller_test.go b/pkg/hub/csr/controller_test.go index c95468b41..144e9cafe 100644 --- a/pkg/hub/csr/controller_test.go +++ b/pkg/hub/csr/controller_test.go @@ -252,7 +252,7 @@ func TestIsSpokeClusterClientCertRenewal(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - isRenewal := isSpokeClusterClientCertRenewal(testinghelpers.NewCSR(c.csr)) + isRenewal := isSpokeClusterClientCertRenewal(newCSRInfo(testinghelpers.NewCSR(c.csr))) if isRenewal != c.isRenewal { t.Errorf("expected %t, but failed", c.isRenewal) } diff --git a/pkg/hub/manager.go b/pkg/hub/manager.go index 6db4c8939..12a0341fa 100644 --- a/pkg/hub/manager.go +++ b/pkg/hub/manager.go @@ -2,10 +2,12 @@ package hub import ( "context" - ocmfeature "open-cluster-management.io/api/feature" "time" + ocmfeature "open-cluster-management.io/api/feature" + "open-cluster-management.io/registration/pkg/features" + "open-cluster-management.io/registration/pkg/helpers" "open-cluster-management.io/registration/pkg/hub/managedclustersetbinding" "open-cluster-management.io/registration/pkg/hub/taint" @@ -25,10 +27,12 @@ import ( "github.com/openshift/library-go/pkg/controller/controllercmd" "github.com/openshift/library-go/pkg/controller/factory" + "github.com/pkg/errors" kubeinformers "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/klog/v2" ) var ResyncInterval = 5 * time.Minute @@ -82,11 +86,29 @@ func RunControllerManager(ctx context.Context, controllerContext *controllercmd. controllerContext.EventRecorder, ) - csrController := csr.NewCSRApprovingController( - kubeClient, - kubeInfomers.Certificates().V1().CertificateSigningRequests(), - controllerContext.EventRecorder, - ) + var csrController factory.Controller + if features.DefaultHubMutableFeatureGate.Enabled(ocmfeature.V1beta1CSRAPICompatibility) { + v1CSRSupported, v1beta1CSRSupported, err := helpers.IsCSRSupported(kubeClient) + if err != nil { + return errors.Wrapf(err, "failed CSR api discovery") + } + + if !v1CSRSupported && v1beta1CSRSupported { + csrController = csr.NewV1beta1CSRApprovingController( + kubeClient, + kubeInfomers.Certificates().V1beta1().CertificateSigningRequests(), + controllerContext.EventRecorder, + ) + klog.Info("Using v1beta1 CSR api to manage spoke client certificate") + } + } + if csrController == nil { + csrController = csr.NewCSRApprovingController( + kubeClient, + kubeInfomers.Certificates().V1().CertificateSigningRequests(), + controllerContext.EventRecorder, + ) + } leaseController := lease.NewClusterLeaseController( kubeClient, diff --git a/vendor/modules.txt b/vendor/modules.txt index d970731dd..e2e568e05 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1193,7 +1193,7 @@ k8s.io/utils/net k8s.io/utils/path k8s.io/utils/pointer k8s.io/utils/trace -# open-cluster-management.io/api v0.7.1-0.20220609033924-5cc58e815c1a +# open-cluster-management.io/api v0.7.1-0.20220706083507-8e98feac6b08 ## explicit; go 1.18 open-cluster-management.io/api/addon/v1alpha1 open-cluster-management.io/api/client/addon/clientset/versioned diff --git a/vendor/open-cluster-management.io/api/cluster/v1beta1/helpers.go b/vendor/open-cluster-management.io/api/cluster/v1beta1/helpers.go index bb85b54fd..e4284e172 100644 --- a/vendor/open-cluster-management.io/api/cluster/v1beta1/helpers.go +++ b/vendor/open-cluster-management.io/api/cluster/v1beta1/helpers.go @@ -2,10 +2,12 @@ package v1beta1 import ( "fmt" + "sync" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/sets" v1 "open-cluster-management.io/api/cluster/v1" ) @@ -108,3 +110,68 @@ func GetBoundManagedClusterSetBindings(namespace string, return boundBindings, nil } + +type PlacementDecisionGetter interface { + List(selector labels.Selector, namespace string) (ret []*PlacementDecision, err error) +} + +// +k8s:deepcopy-gen=false +type PlacementDecisionClustersTracker struct { + placement *Placement + placementDecisionGetter PlacementDecisionGetter + existingScheduledClusters sets.String + lock sync.RWMutex +} + +func NewPlacementDecisionClustersTracker(placement *Placement, pdl PlacementDecisionGetter, existingScheduledClusters sets.String) *PlacementDecisionClustersTracker { + pdct := &PlacementDecisionClustersTracker{ + placement: placement, + placementDecisionGetter: pdl, + existingScheduledClusters: existingScheduledClusters, + } + return pdct +} + +// Get() update the tracker's decisionClusters and return the added and deleted cluster names. +func (pdct *PlacementDecisionClustersTracker) Get() (sets.String, sets.String, error) { + pdct.lock.Lock() + defer pdct.lock.Unlock() + + if pdct.placement == nil || pdct.placementDecisionGetter == nil { + return nil, nil, nil + } + + // Get the generated PlacementDecisions + decisionSelector := labels.SelectorFromSet(labels.Set{ + PlacementLabel: pdct.placement.Name, + }) + decisions, err := pdct.placementDecisionGetter.List(decisionSelector, pdct.placement.Namespace) + if err != nil { + return nil, nil, fmt.Errorf("failed to list PlacementDecisions: %w", err) + } + + // Get the decision cluster names + newScheduledClusters := sets.NewString() + for _, d := range decisions { + for _, sd := range d.Status.Decisions { + newScheduledClusters.Insert(sd.ClusterName) + } + } + + // Compare the difference + added := newScheduledClusters.Difference(pdct.existingScheduledClusters) + deleted := pdct.existingScheduledClusters.Difference(newScheduledClusters) + + // Update the existing decision cluster names + pdct.existingScheduledClusters = newScheduledClusters + + return added, deleted, nil +} + +// Existing() returns the tracker's existing decision cluster names. +func (pdct *PlacementDecisionClustersTracker) Existing() sets.String { + pdct.lock.RLock() + defer pdct.lock.RUnlock() + + return pdct.existingScheduledClusters +} diff --git a/vendor/open-cluster-management.io/api/feature/feature.go b/vendor/open-cluster-management.io/api/feature/feature.go index 9975797a5..4d7fc9d9c 100644 --- a/vendor/open-cluster-management.io/api/feature/feature.go +++ b/vendor/open-cluster-management.io/api/feature/feature.go @@ -27,9 +27,10 @@ const ( // registration and maintains the status of managed cluster addons through watching their leases. AddonManagement featuregate.Feature = "AddonManagement" - // DefaultClusterSet will make registration hub controller to maintain a default cluster set. All clusters - // without clusterset label will be automatically added into the default cluster set by adding a label + // DefaultClusterSet will make registration hub controller to maintain a default clusterset and a global clusterset. + // All clusters without clusterset label will be automatically added into the default clusterset by adding a label // "cluster.open-cluster-management.io/clusterset=default" to the clusters. + // All clusters will be included to the global clusterset DefaultClusterSet featuregate.Feature = "DefaultClusterSet" // V1beta1CSRAPICompatibility will make the spoke registration agent to issue CSR requests @@ -54,5 +55,6 @@ var DefaultSpokeRegistrationFeatureGates = map[featuregate.Feature]featuregate.F // feature keys for registration hub controller. To add a new feature, define a key for it above and // add it here. var DefaultHubRegistrationFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ - DefaultClusterSet: {Default: false, PreRelease: featuregate.Alpha}, + DefaultClusterSet: {Default: false, PreRelease: featuregate.Alpha}, + V1beta1CSRAPICompatibility: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/vendor/open-cluster-management.io/api/work/v1/types.go b/vendor/open-cluster-management.io/api/work/v1/types.go index 9216d8bbd..2777a157a 100644 --- a/vendor/open-cluster-management.io/api/work/v1/types.go +++ b/vendor/open-cluster-management.io/api/work/v1/types.go @@ -198,6 +198,9 @@ type ServerSideApplyConfig struct { FieldManager string `json:"fieldManager,omitempty"` } +// DefaultFieldManager is the default field manager of the manifestwork when the field manager is not set. +const DefaultFieldManager = "work-agent" + type FeedbackRule struct { // Type defines the option of how status can be returned. // It can be jsonPaths or wellKnownStatus.