Repository: openyurtio/openyurt
Pull request #2058 (2024-05-27 10:10:25)

improve csr approver controller by replacing kubernetes client with client.Client
<!--  Thanks for sending a pull request!  Here are some tips for you:
https://github.com/openyurtio/openyurt/blob/master/CONTRIBUTING.md 
-->

#### What type of PR is this?
> Uncomment only one `/kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line:
> /kind bug
> /kind documentation
> /kind enhancement
> /kind good-first-issue
> /kind feature
> /kind question
> /kind design
> /sig ai
> /sig iot
> /sig network
> /sig storage

/kind enhancement

#### What this PR does / why we need it:
in controller-runtime v0.16, client.Client is supported to update csr approval. so remove original kubernetes client in csr approver controller and use the client.Client to replace it.

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #

#### Special notes for your reviewer:
<!--
use this label to assign your reviewer
/assign @your_reviewer
-->

#### Does this PR introduce a user-facing change?
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".

-->
```release-note

```

#### other Note
<!--
If your current PR is still working in process, start the PR title name with [WIP], such as: [WIP] add new crd for yurt-app-manager
If the PR title name begins with [WIP], OpenYurt-bot automatically adds a do-not-merge/work-in-progress label for your pr 
-->


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=openyurtio_openyurt&pullRequest=2058) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=openyurtio_openyurt&pullRequest=2058&resolved=false&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=openyurtio_openyurt&pullRequest=2058&resolutions=WONTFIX)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=openyurtio_openyurt&pullRequest=2058&resolved=false&sinceLeakPeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=openyurtio_openyurt&pullRequest=2058&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=openyurtio_openyurt&pullRequest=2058)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/openyurtio/openyurt/pull/2058?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 56.01%. Comparing base [(`e42545e`)](https://app.codecov.io/gh/openyurtio/openyurt/commit/e42545eaf9f438c577238be776b30196708913de?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio) to head [(`6687587`)](https://app.codecov.io/gh/openyurtio/openyurt/commit/6687587d1af718983fd03b164e27228accc33043?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio).


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2058      +/-   ##
==========================================
+ Coverage   55.53%   56.01%   +0.48%     
==========================================
  Files         186      186              
  Lines       18088    18082       -6     
==========================================
+ Hits        10045    10129      +84     
+ Misses       7013     6920      -93     
- Partials     1030     1033       +3     
```

| [Flag](https://app.codecov.io/gh/openyurtio/openyurt/pull/2058/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/openyurtio/openyurt/pull/2058/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio) | `56.01% <100.00%> (+0.48%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/openyurtio/openyurt/pull/2058?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openyurtio).


Patch:
diff --git a/pkg/yurthub/kubernetes/meta/restmapper.go b/pkg/yurthub/kubernetes/meta/restmapper.go
index 7c417310..291edf42 100644
--- a/pkg/yurthub/kubernetes/meta/restmapper.go
+++ b/pkg/yurthub/kubernetes/meta/restmapper.go
@@ -42,6 +42,10 @@ var (
 	// It is not updatable and is only used for the judgment of scheme resources
 	unsafeSchemeRESTMapper = NewDefaultRESTMapperFromScheme()
 	ErrGVRNotRecognized    = errors.New("GroupVersionResource is not recognized")
+
+	specifiedResources = []string{
+		"gateway",
+	}
 )
 
 // RESTMapperManager is responsible for managing different kind of RESTMapper
@@ -141,7 +145,7 @@ func (rm *RESTMapperManager) dynamicKindFor(gvr schema.GroupVersionResource) (sc
 // Used to delete the mapping relationship between GVR and GVK in dynamicRESTMapper
 func (rm *RESTMapperManager) deleteKind(gvk schema.GroupVersionKind) error {
 	kindName := strings.TrimSuffix(gvk.Kind, "List")
-	plural, singular := meta.UnsafeGuessKindToResource(gvk.GroupVersion().WithKind(kindName))
+	plural, singular := specifiedKindToResource(gvk.GroupVersion().WithKind(kindName))
 	rm.Lock()
 	delete(rm.dynamicRESTMapper, plural)
 	delete(rm.dynamicRESTMapper, singular)
@@ -193,7 +197,7 @@ func (rm *RESTMapperManager) DeleteKindFor(gvr schema.GroupVersionResource) erro
 func (rm *RESTMapperManager) UpdateKind(gvk schema.GroupVersionKind) error {
 	kindName := strings.TrimSuffix(gvk.Kind, "List")
 	gvk = gvk.GroupVersion().WithKind(kindName)
-	plural, singular := meta.UnsafeGuessKindToResource(gvk.GroupVersion().WithKind(kindName))
+	plural, singular := specifiedKindToResource(gvk.GroupVersion().WithKind(kindName))
 	// If it is not a built-in resource and it is not stored in DynamicRESTMapper, add it to DynamicRESTMapper
 	isScheme, t := rm.KindFor(singular)
 	if !isScheme && t.Empty() {
@@ -269,3 +273,23 @@ func IsSchemeResource(gvr schema.GroupVersionResource) bool {
 
 	return false
 }
+
+// specifiedKindToResource converts Kind to a resource name.
+// Broken. This method only "sort of" works when used outside of this package.  It assumes that Kinds and Resources match
+// and they aren't guaranteed to do so.
+func specifiedKindToResource(kind schema.GroupVersionKind) ( /*plural*/ schema.GroupVersionResource /*singular*/, schema.GroupVersionResource) {
+	kindName := kind.Kind
+	if len(kindName) == 0 {
+		return schema.GroupVersionResource{}, schema.GroupVersionResource{}
+	}
+	singularName := strings.ToLower(kindName)
+	singular := kind.GroupVersion().WithResource(singularName)
+
+	for _, skip := range specifiedResources {
+		if strings.HasSuffix(singularName, skip) {
+			return kind.GroupVersion().WithResource(singularName + "s"), singular
+		}
+	}
+
+	return meta.UnsafeGuessKindToResource(kind)
+}
diff --git a/pkg/yurtmanager/controller/csrapprover/csrapprover_controller.go b/pkg/yurtmanager/controller/csrapprover/csrapprover_controller.go
index bf83a9a9..4561e291 100644
--- a/pkg/yurtmanager/controller/csrapprover/csrapprover_controller.go
+++ b/pkg/yurtmanager/controller/csrapprover/csrapprover_controller.go
@@ -27,10 +27,8 @@ import (
 	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authentication/user"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -120,20 +118,13 @@ var _ reconcile.Reconciler = &ReconcileCsrApprover{}
 // ReconcileCsrApprover reconciles a CsrApprover object
 type ReconcileCsrApprover struct {
 	client.Client
-	csrV1Supported    bool
-	csrApproverClient kubernetes.Interface
+	csrV1Supported bool
 }
 
 func NewReconcileCsrApprover(mgr manager.Manager) (*ReconcileCsrApprover, error) {
 	r := &ReconcileCsrApprover{
 		Client: mgr.GetClient(),
 	}
-	client, err := kubernetes.NewForConfig(mgr.GetConfig())
-	if err != nil {
-		klog.Errorf("could not create kube client, %v", err)
-		return nil, err
-	}
-	r.csrApproverClient = client
 
 	mapper := mgr.GetRESTMapper()
 	if gvk, err := mapper.KindFor(csrV1Resource); err != nil {
@@ -211,14 +202,13 @@ func (r *ReconcileCsrApprover) Reconcile(ctx context.Context, request reconcile.
 }
 
 // updateApproval is used for adding approval info into csr resource
-func (r *ReconcileCsrApprover) updateApproval(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) (err error) {
+func (r *ReconcileCsrApprover) updateApproval(ctx context.Context, csr *certificatesv1.CertificateSigningRequest) error {
 	if r.csrV1Supported {
-		_, err = r.csrApproverClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csr.Name, csr, metav1.UpdateOptions{})
+		return r.SubResource("approval").Update(ctx, csr, &client.SubResourceUpdateOptions{})
 	} else {
 		v1beta1Csr := v1Csr2v1beta1Csr(csr)
-		_, err = r.csrApproverClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, v1beta1Csr, metav1.UpdateOptions{})
+		return r.SubResource("approval").Update(ctx, v1beta1Csr, &client.SubResourceUpdateOptions{})
 	}
-	return
 }
 
 // isYurtCSR checks if given csr is an openyurt related csr and


Test patch:
diff --git a/pkg/yurthub/kubernetes/meta/restmapper_test.go b/pkg/yurthub/kubernetes/meta/restmapper_test.go
index 84ddccc7..11f075a1 100644
--- a/pkg/yurthub/kubernetes/meta/restmapper_test.go
+++ b/pkg/yurthub/kubernetes/meta/restmapper_test.go
@@ -237,3 +237,31 @@ func dynamicRESTMapperToString(m map[schema.GroupVersionResource]schema.GroupVer
 	}
 	return resultMapper
 }
+
+func TestKindToResource(t *testing.T) {
+	testCases := []struct {
+		Kind             string
+		Plural, Singular string
+	}{
+		{Kind: "Pod", Plural: "pods", Singular: "pod"},
+
+		{Kind: "Gateway", Plural: "gateways", Singular: "gateway"},
+
+		{Kind: "ReplicationController", Plural: "replicationcontrollers", Singular: "replicationcontroller"},
+
+		// Add "ies" when ending with "y"
+		{Kind: "ImageRepository", Plural: "imagerepositories", Singular: "imagerepository"},
+		// Add "es" when ending with "s"
+		{Kind: "miss", Plural: "misses", Singular: "miss"},
+		// Add "s" otherwise
+		{Kind: "lowercase", Plural: "lowercases", Singular: "lowercase"},
+	}
+	for i, testCase := range testCases {
+		version := schema.GroupVersion{}
+
+		plural, singular := specifiedKindToResource(version.WithKind(testCase.Kind))
+		if singular != version.WithResource(testCase.Singular) || plural != version.WithResource(testCase.Plural) {
+			t.Errorf("%d: unexpected plural and singular: %v %v", i, plural, singular)
+		}
+	}
+}
diff --git a/pkg/yurtmanager/controller/csrapprover/csrapprover_controller_test.go b/pkg/yurtmanager/controller/csrapprover/csrapprover_controller_test.go
index 5fd1d599..5c8e14be 100644
--- a/pkg/yurtmanager/controller/csrapprover/csrapprover_controller_test.go
+++ b/pkg/yurtmanager/controller/csrapprover/csrapprover_controller_test.go
@@ -1,5 +1,5 @@
 /*
-Copyright 2020 The OpenYurt Authors.
+Copyright 2024 The OpenYurt Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@ limitations under the License.
 package csrapprover
 
 import (
+	"context"
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	cryptorand "crypto/rand"
@@ -24,51 +25,157 @@ import (
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"net"
+	"reflect"
 	"testing"
 
 	certificatesv1 "k8s.io/api/certificates/v1"
+	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apiserver/pkg/authentication/user"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/util/cert"
 	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/openyurtio/openyurt/pkg/yurthub/certificate/token"
 	"github.com/openyurtio/openyurt/pkg/yurttunnel/constants"
 )
 
-func TestIsYurtCSR(t *testing.T) {
-	tests := []struct {
-		desc string
-		csr  *certificatesv1.CertificateSigningRequest
-		exp  bool
+func TestReconcile(t *testing.T) {
+	v1beta1SignerName := certificatesv1beta1.KubeAPIServerClientSignerName
+	csrData := newCSRData("system:node:xxx", []string{token.YurtHubCSROrg, user.NodesGroup, "unknown org"}, []string{}, []net.IP{})
+	testcases := map[string]struct {
+		obj            runtime.Object
+		csrV1Supported bool
+		skipRequest    bool
+		expectedObj    runtime.Object
 	}{
-		{
-			desc: "is not certificate request",
-			csr: &certificatesv1.CertificateSigningRequest{
+		"yurthub server related certificate request": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
-					Request: pem.EncodeToMemory(
-						&pem.Block{
-							Type: "PUBLIC KEY",
-						}),
+					SignerName: certificatesv1.KubeletServingSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageServerAuth,
+					},
+					Request: newCSRData("system:node:xxx", []string{user.NodesGroup}, []string{}, []net.IP{net.ParseIP("127.0.0.1")}),
 				},
 			},
-			exp: false,
-		},
-		{
-			desc: "can not parse certificate request",
-			csr: &certificatesv1.CertificateSigningRequest{
+			csrV1Supported: true,
+			skipRequest:    true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
-					Request: pem.EncodeToMemory(
-						&pem.Block{
-							Type:  "CERTIFICATE REQUEST",
-							Bytes: []byte(`MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAlRuRnThUjU8/prwYxbty`),
-						}),
+					SignerName: certificatesv1.KubeletServingSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageServerAuth,
+					},
+					// don't compare CSR data
+					Request: []byte{},
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateApproved,
+							Status:  corev1.ConditionTrue,
+							Reason:  "AutoApproved",
+							Message: "Auto approving openyurt tls server certificate",
+						},
+					},
+				},
+			},
+		},
+		"yurthub node client related certificate request": {
+			obj: &certificatesv1beta1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "node-client-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					SignerName: &v1beta1SignerName,
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageClientAuth,
+					},
+					Request: newCSRData("system:node:xxx", []string{token.YurtHubCSROrg, user.NodesGroup, "openyurt:tenant:xxx"}, []string{}, []net.IP{}),
+				},
+				Status: certificatesv1beta1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1beta1.RequestConditionType("test"),
+							Status:  corev1.ConditionTrue,
+							Reason:  "test",
+							Message: "test",
+						},
+					},
+				},
+			},
+			csrV1Supported: false,
+			skipRequest:    true,
+			expectedObj: &certificatesv1beta1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1beta1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "node-client-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1beta1.CertificateSigningRequestSpec{
+					SignerName: &v1beta1SignerName,
+					Usages: []certificatesv1beta1.KeyUsage{
+						certificatesv1beta1.UsageDigitalSignature,
+						certificatesv1beta1.UsageKeyEncipherment,
+						certificatesv1beta1.UsageClientAuth,
+					},
+					// don't compare CSR data
+					Request: []byte{},
+				},
+				Status: certificatesv1beta1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1beta1.RequestConditionType("test"),
+							Status:  corev1.ConditionTrue,
+							Reason:  "test",
+							Message: "test",
+						},
+						{
+							Type:    certificatesv1beta1.CertificateApproved,
+							Status:  corev1.ConditionTrue,
+							Reason:  "AutoApproved",
+							Message: "Auto approving yurthub node client certificate",
+						},
+					},
 				},
 			},
-			exp: false,
 		},
-		{
-			desc: "is yurthub server related certificate request",
-			csr: &certificatesv1.CertificateSigningRequest{
+		"yurt-tunnel server related tls server certificate request": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "tunnel-server-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
 					SignerName: certificatesv1.KubeletServingSignerName,
 					Usages: []certificatesv1.KeyUsage{
@@ -76,14 +183,48 @@ func TestIsYurtCSR(t *testing.T) {
 						certificatesv1.UsageKeyEncipherment,
 						certificatesv1.UsageServerAuth,
 					},
-					Request: newCSRData("system:node:xxx", []string{user.NodesGroup}, []string{}, []net.IP{net.ParseIP("127.0.0.1")}),
+					Request: newCSRData("system:node:xxx", []string{user.NodesGroup}, []string{"x-tunnel-server-svc"}, []net.IP{net.ParseIP("127.0.0.1")}),
+				},
+			},
+			csrV1Supported: true,
+			skipRequest:    true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "tunnel-server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					SignerName: certificatesv1.KubeletServingSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageServerAuth,
+					},
+					// don't compare CSR data
+					Request: []byte{},
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateApproved,
+							Status:  corev1.ConditionTrue,
+							Reason:  "AutoApproved",
+							Message: "Auto approving openyurt tls server certificate",
+						},
+					},
 				},
 			},
-			exp: true,
 		},
-		{
-			desc: "is yurthub node client related certificate request",
-			csr: &certificatesv1.CertificateSigningRequest{
+		"yurt-tunnel-server related proxy client certificate request": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "tunnel-server-proxy-client-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
 					SignerName: certificatesv1.KubeAPIServerClientSignerName,
 					Usages: []certificatesv1.KeyUsage{
@@ -91,14 +232,20 @@ func TestIsYurtCSR(t *testing.T) {
 						certificatesv1.UsageKeyEncipherment,
 						certificatesv1.UsageClientAuth,
 					},
-					Request: newCSRData("system:node:xxx", []string{token.YurtHubCSROrg, user.NodesGroup, "openyurt:tenant:xxx"}, []string{}, []net.IP{}),
+					Request: newCSRData(constants.YurtTunnelProxyClientCSRCN, []string{constants.YurtTunnelCSROrg}, []string{}, []net.IP{}),
 				},
 			},
-			exp: true,
-		},
-		{
-			desc: "yurthub node client csr with unknown org",
-			csr: &certificatesv1.CertificateSigningRequest{
+			csrV1Supported: true,
+			skipRequest:    true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "tunnel-server-proxy-client-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
 					SignerName: certificatesv1.KubeAPIServerClientSignerName,
 					Usages: []certificatesv1.KeyUsage{
@@ -106,29 +253,48 @@ func TestIsYurtCSR(t *testing.T) {
 						certificatesv1.UsageKeyEncipherment,
 						certificatesv1.UsageClientAuth,
 					},
-					Request: newCSRData("system:node:xxx", []string{token.YurtHubCSROrg, user.NodesGroup, "unknown org"}, []string{}, []net.IP{}),
+					// don't compare CSR data
+					Request: []byte{},
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateApproved,
+							Status:  corev1.ConditionTrue,
+							Reason:  "AutoApproved",
+							Message: "Auto approving tunnel-server proxy client certificate",
+						},
+					},
 				},
 			},
-			exp: false,
 		},
-		{
-			desc: "is yurt-tunnel server related tls server certificate request",
-			csr: &certificatesv1.CertificateSigningRequest{
+		"yurt-tunnel-agent related certificate request": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "tunnel-agent-client-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
-					SignerName: certificatesv1.KubeletServingSignerName,
+					SignerName: certificatesv1.KubeAPIServerClientSignerName,
 					Usages: []certificatesv1.KeyUsage{
 						certificatesv1.UsageDigitalSignature,
 						certificatesv1.UsageKeyEncipherment,
-						certificatesv1.UsageServerAuth,
+						certificatesv1.UsageClientAuth,
 					},
-					Request: newCSRData("system:node:xxx", []string{user.NodesGroup}, []string{"x-tunnel-server-svc"}, []net.IP{net.ParseIP("127.0.0.1")}),
+					Request: newCSRData(constants.YurtTunnelAgentCSRCN, []string{constants.YurtTunnelCSROrg}, []string{"node-name-xxx"}, []net.IP{net.ParseIP("127.0.0.1")}),
 				},
 			},
-			exp: true,
-		},
-		{
-			desc: "is yurt-tunnel-server related proxy client certificate request",
-			csr: &certificatesv1.CertificateSigningRequest{
+			csrV1Supported: true,
+			skipRequest:    true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "tunnel-agent-client-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
 					SignerName: certificatesv1.KubeAPIServerClientSignerName,
 					Usages: []certificatesv1.KeyUsage{
@@ -136,14 +302,91 @@ func TestIsYurtCSR(t *testing.T) {
 						certificatesv1.UsageKeyEncipherment,
 						certificatesv1.UsageClientAuth,
 					},
-					Request: newCSRData(constants.YurtTunnelProxyClientCSRCN, []string{constants.YurtTunnelCSROrg}, []string{}, []net.IP{}),
+					// don't compare CSR data
+					Request: []byte{},
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateApproved,
+							Status:  corev1.ConditionTrue,
+							Reason:  "AutoApproved",
+							Message: "Auto approving tunnel-agent client certificate",
+						},
+					},
 				},
 			},
-			exp: true,
 		},
-		{
-			desc: "is yurt-tunnel-agent related certificate request",
-			csr: &certificatesv1.CertificateSigningRequest{
+		"it is not a certificate request": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					Request: pem.EncodeToMemory(
+						&pem.Block{
+							Type: "PUBLIC KEY",
+						}),
+				},
+			},
+			csrV1Supported: true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					Request: pem.EncodeToMemory(
+						&pem.Block{
+							Type: "PUBLIC KEY",
+						}),
+				},
+			},
+		},
+		"can not parse certificate request": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					Request: pem.EncodeToMemory(
+						&pem.Block{
+							Type:  "CERTIFICATE REQUEST",
+							Bytes: []byte(`MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAlRuRnThUjU8/prwYxbty`),
+						}),
+				},
+			},
+			csrV1Supported: true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					Request: pem.EncodeToMemory(
+						&pem.Block{
+							Type:  "CERTIFICATE REQUEST",
+							Bytes: []byte(`MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAlRuRnThUjU8/prwYxbty`),
+						}),
+				},
+			},
+		},
+		"yurthub node client csr with unknown org": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
 				Spec: certificatesv1.CertificateSigningRequestSpec{
 					SignerName: certificatesv1.KubeAPIServerClientSignerName,
 					Usages: []certificatesv1.KeyUsage{
@@ -151,88 +394,214 @@ func TestIsYurtCSR(t *testing.T) {
 						certificatesv1.UsageKeyEncipherment,
 						certificatesv1.UsageClientAuth,
 					},
-					Request: newCSRData(constants.YurtTunnelAgentCSRCN, []string{constants.YurtTunnelCSROrg}, []string{"node-name-xxx"}, []net.IP{net.ParseIP("127.0.0.1")}),
+					Request: csrData,
 				},
 			},
-			exp: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.desc, func(t *testing.T) {
-			act, msg := isYurtCSR(tt.csr)
-			if act != tt.exp {
-				t.Errorf("the value we want is %v, but the actual value is %v, and the message: %s", tt.exp, act, msg)
-			}
-		})
-	}
-}
-
-func TestCheckCertApprovalCondition(t *testing.T) {
-	tests := []struct {
-		desc   string
-		status *certificatesv1.CertificateSigningRequestStatus
-		exp    struct {
-			approved bool
-			denied   bool
-		}
-	}{
-		{
-			desc: "approved",
-			status: &certificatesv1.CertificateSigningRequestStatus{
-				Conditions: []certificatesv1.CertificateSigningRequestCondition{
-					{
-						Type: certificatesv1.CertificateApproved,
+			csrV1Supported: true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					SignerName: certificatesv1.KubeAPIServerClientSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageClientAuth,
 					},
+					Request: csrData,
 				},
+				Status: certificatesv1.CertificateSigningRequestStatus{},
 			},
-			exp: struct {
-				approved bool
-				denied   bool
-			}{approved: true, denied: false},
 		},
-		{
-			desc: "denied",
-			status: &certificatesv1.CertificateSigningRequestStatus{
-				Conditions: []certificatesv1.CertificateSigningRequestCondition{
-					{
-						Type: certificatesv1.CertificateDenied,
+		"csr has already been approved": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					SignerName: certificatesv1.KubeletServingSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageServerAuth,
+					},
+					Request: newCSRData("system:node:xxx", []string{user.NodesGroup}, []string{}, []net.IP{net.ParseIP("127.0.0.1")}),
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateApproved,
+							Status:  corev1.ConditionTrue,
+							Reason:  "init approved",
+							Message: "csr has already been approved",
+						},
+					},
+				},
+			},
+			csrV1Supported: true,
+			skipRequest:    true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					SignerName: certificatesv1.KubeletServingSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageServerAuth,
+					},
+					// don't compare CSR data
+					Request: []byte{},
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateApproved,
+							Status:  corev1.ConditionTrue,
+							Reason:  "init approved",
+							Message: "csr has already been approved",
+						},
 					},
 				},
 			},
-			exp: struct {
-				approved bool
-				denied   bool
-			}{approved: false, denied: true},
 		},
-		{
-			desc: "approved and denied",
-			status: &certificatesv1.CertificateSigningRequestStatus{
-				Conditions: []certificatesv1.CertificateSigningRequestCondition{
-					{
-						Type: certificatesv1.CertificateApproved,
+		"csr has already been denied": {
+			obj: &certificatesv1.CertificateSigningRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					SignerName: certificatesv1.KubeletServingSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageServerAuth,
 					},
-					{
-						Type: certificatesv1.CertificateDenied,
+					Request: newCSRData("system:node:xxx", []string{user.NodesGroup}, []string{}, []net.IP{net.ParseIP("127.0.0.1")}),
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateDenied,
+							Status:  corev1.ConditionTrue,
+							Reason:  "init denied",
+							Message: "csr has already been denied",
+						},
+					},
+				},
+			},
+			csrV1Supported: true,
+			skipRequest:    true,
+			expectedObj: &certificatesv1.CertificateSigningRequest{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "CertificateSigningRequest",
+					APIVersion: "certificates.k8s.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "server-csr",
+					Namespace: "default",
+				},
+				Spec: certificatesv1.CertificateSigningRequestSpec{
+					SignerName: certificatesv1.KubeletServingSignerName,
+					Usages: []certificatesv1.KeyUsage{
+						certificatesv1.UsageDigitalSignature,
+						certificatesv1.UsageKeyEncipherment,
+						certificatesv1.UsageServerAuth,
+					},
+					// don't compare CSR data
+					Request: []byte{},
+				},
+				Status: certificatesv1.CertificateSigningRequestStatus{
+					Conditions: []certificatesv1.CertificateSigningRequestCondition{
+						{
+							Type:    certificatesv1.CertificateDenied,
+							Status:  corev1.ConditionTrue,
+							Reason:  "init denied",
+							Message: "csr has already been denied",
+						},
 					},
 				},
 			},
-			exp: struct {
-				approved bool
-				denied   bool
-			}{approved: true, denied: true},
 		},
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.desc, func(t *testing.T) {
-			actApproved, actDenied := checkCertApprovalCondition(tt.status)
-			act := struct {
-				approved bool
-				denied   bool
-			}{approved: actApproved, denied: actDenied}
-			if act != tt.exp {
-				t.Errorf("the value we want is %+v, but the actual value is %+v", tt.exp, act)
+	scheme := runtime.NewScheme()
+	if err := clientgoscheme.AddToScheme(scheme); err != nil {
+		t.Fatal("Fail to add kubernetes clint-go custom resource")
+	}
+
+	for k, tc := range testcases {
+		t.Run(k, func(t *testing.T) {
+			obj := tc.obj.DeepCopyObject()
+			accessor := meta.NewAccessor()
+			name, _ := accessor.Name(obj)
+			ns, _ := accessor.Namespace(obj)
+			req := reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      name,
+					Namespace: ns,
+				},
+			}
+			c := fakeclient.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tc.obj).WithStatusSubresource([]client.Object{&certificatesv1beta1.CertificateSigningRequest{}}...).Build()
+			csrApproverController := &ReconcileCsrApprover{
+				Client:         c,
+				csrV1Supported: tc.csrV1Supported,
+			}
+
+			_, err := csrApproverController.Reconcile(context.Background(), req)
+			if err != nil {
+				t.Errorf("expect an error, but got no error")
+			} else {
+				if err != nil {
+					t.Errorf("expect no error, but got an error, %v", err)
+					return
+				}
+
+				if tc.csrV1Supported {
+					v1Instance := &certificatesv1.CertificateSigningRequest{}
+					err = csrApproverController.Get(context.Background(), req.NamespacedName, v1Instance)
+					if err != nil {
+						t.Errorf("couldn't get csr v1 instance, %v", err)
+						return
+					}
+					// clear resource version and csr data
+					accessor.SetResourceVersion(v1Instance, "")
+					if tc.skipRequest {
+						v1Instance.Spec.Request = []byte{}
+					}
+					if !reflect.DeepEqual(v1Instance, tc.expectedObj) {
+						t.Errorf("expect object %#+v\n, but got %#+v\n", tc.expectedObj, v1Instance)
+					}
+				} else {
+					v1beta1Instance := &certificatesv1beta1.CertificateSigningRequest{}
+					err = csrApproverController.Get(context.Background(), req.NamespacedName, v1beta1Instance)
+					if err != nil {
+						t.Errorf("couldn't get csr, %v", err)
+						return
+					}
+					// clear resource version and csr data
+					accessor.SetResourceVersion(v1beta1Instance, "")
+					if tc.skipRequest {
+						v1beta1Instance.Spec.Request = []byte{}
+					}
+					if !reflect.DeepEqual(v1beta1Instance, tc.expectedObj) {
+						t.Errorf("expect object %#+v\n, but got %#+v\n", tc.expectedObj, v1beta1Instance)
+					}
+				}
 			}
 		})
 	}