Fixed the k8s client impersonation for authroization

Signed-off-by: WillardHu <wei.hu@daocloud.io>

2 files changed, 85 insertions, 20 deletions

diff --git a/cloud/pkg/common/client/impersonation.go b/cloud/pkg/common/client/impersonation.go
index a905a48a1..743f6e0cb 100644
--- a/cloud/pkg/common/client/impersonation.go
+++ b/cloud/pkg/common/client/impersonation.go
@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 
@@ -38,12 +39,12 @@ func newForK8sConfigOrDie(c *rest.Config, enableImpersonation bool) *kubernetes.
 
 	httpClient, err := httpClientFor(&configShallowCopy, enableImpersonation)
 	if err != nil {
-		panic(err)
+		panic(fmt.Errorf("failed to create a httpclient for the clientset, err: %v", err))
 	}
 
 	cs, err := kubernetes.NewForConfigAndClient(&configShallowCopy, httpClient)
 	if err != nil {
-		panic(err)
+		panic(fmt.Errorf("failed to create a clientset, err: %v", err))
 	}
 	return cs
 }
@@ -52,12 +53,12 @@ func newForDynamicConfigOrDie(c *rest.Config, enableImpersonation bool) *dynamic
 	configShallowCopy := dynamic.ConfigFor(c)
 	httpClient, err := httpClientFor(configShallowCopy, enableImpersonation)
 	if err != nil {
-		panic(err)
+		panic(fmt.Errorf("failed to create a httpclient for the dynamic-client, err: %v", err))
 	}
 
 	cs, err := dynamic.NewForConfigAndClient(configShallowCopy, httpClient)
 	if err != nil {
-		panic(err)
+		panic(fmt.Errorf("failed to create a dynamic-client, err: %v", err))
 	}
 	return cs
 }
@@ -71,12 +72,12 @@ func newForCrdConfigOrDie(c *rest.Config, enableImpersonation bool) *crdClientse
 
 	httpClient, err := httpClientFor(&configShallowCopy, enableImpersonation)
 	if err != nil {
-		panic(err)
+		panic(fmt.Errorf("failed to create a httpclient for the crd clientset, err: %v", err))
 	}
 
 	cs, err := crdClientset.NewForConfigAndClient(&configShallowCopy, httpClient)
 	if err != nil {
-		panic(err)
+		panic(fmt.Errorf("failed to create a crd clientset, err: %v", err))
 	}
 	return cs
 }
@@ -102,21 +103,18 @@ type impersonationRoundTripper struct {
 }
 
 func (r *impersonationRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
-	// extract user and group from context and set impersonation headers
-	var userStr, groupStr string
-	user := req.Context().Value(authenticationv1.ImpersonateUserHeader)
-	if user != nil && r.enable {
-		userStr = user.(string)
-		req.Header.Set(authenticationv1.ImpersonateUserHeader, userStr)
-	}
-	group := req.Context().Value(authenticationv1.ImpersonateGroupHeader)
-	if group != nil && r.enable {
-		groupStr = group.(string)
-		for _, g := range strings.Split(groupStr, "|") {
-			req.Header.Set(authenticationv1.ImpersonateGroupHeader, g)
+	var user, group string
+	if r.enable {
+		if v := req.Context().Value(authenticationv1.ImpersonateUserHeader); v != nil {
+			user = v.(string)
+			req.Header.Set(authenticationv1.ImpersonateUserHeader, user)
+		}
+		if v := req.Context().Value(authenticationv1.ImpersonateGroupHeader); v != nil {
+			group = v.(string)
+			req.Header[authenticationv1.ImpersonateGroupHeader] = strings.Split(group, "|")
 		}
 	}
-
-	klog.V(4).Infof("KubeClient: request.method=%s, request.path=%s, user=%q, group= %q", req.Method, req.URL.Path, userStr, groupStr)
+	klog.V(4).Infof("KubeClient: request.method=%s, request.path=%s, user=%q, group= %q",
+		req.Method, req.URL.Path, user, group)
 	return r.rt.RoundTrip(req)
 }
diff --git a/cloud/pkg/common/client/impersonation_test.go b/cloud/pkg/common/client/impersonation_test.go
new file mode 100644
index 000000000..1733f0cc4
--- /dev/null
+++ b/cloud/pkg/common/client/impersonation_test.go
@@ -0,0 +1,67 @@
+package client
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	authenticationv1 "k8s.io/api/authentication/v1"
+
+	ctxutl "github.com/kubeedge/kubeedge/cloud/pkg/common/context"
+)
+
+type fakeNextRoundTripper struct {
+	enable bool
+}
+
+func (f *fakeNextRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+	if f.enable {
+		if vals := req.Header[authenticationv1.ImpersonateUserHeader]; len(vals) == 0 || vals[0] == "" {
+			return nil, fmt.Errorf("invalid request header %s", authenticationv1.ImpersonateUserHeader)
+		}
+		if vals := req.Header[authenticationv1.ImpersonateGroupHeader]; len(vals) == 0 || vals[0] == "" {
+			return nil, fmt.Errorf("invalid request header %s", authenticationv1.ImpersonateGroupHeader)
+		}
+	} else {
+		if vals := req.Header[authenticationv1.ImpersonateUserHeader]; len(vals) > 0 {
+			return nil, fmt.Errorf("invalid request header %s", authenticationv1.ImpersonateUserHeader)
+		}
+		if vals := req.Header[authenticationv1.ImpersonateGroupHeader]; len(vals) > 0 {
+			return nil, fmt.Errorf("invalid request header %s", authenticationv1.ImpersonateGroupHeader)
+		}
+	}
+	return nil, nil
+}
+
+func TestRoundTrip(t *testing.T) {
+	cases := []struct {
+		name   string
+		enable bool
+	}{
+		{name: "enable impersonation", enable: true},
+		{name: "disable impersonation", enable: false},
+	}
+
+	url, err := url.Parse("http://localhost:6443/apis")
+	assert.NoError(t, err)
+	ctx := ctxutl.WithEdgeNode(context.TODO(), "test-node")
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			req := &http.Request{
+				Method: http.MethodGet,
+				URL:    url,
+				Header: make(http.Header),
+			}
+			r := &impersonationRoundTripper{
+				enable: c.enable,
+				rt:     &fakeNextRoundTripper{enable: c.enable},
+			}
+			_, err := r.RoundTrip(req.WithContext(ctx))
+			assert.NoError(t, err)
+		})
+	}
+}