Merge pull request #102152 from ankeesler/exec-plugin-integration-test

test/integration/client: test exec calls metric

test/integration/client/exec_test.go

@@ -27,6 +27,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"reflect"
 	"strings"
 	"sync"
 	"testing"
@@ -36,6 +37,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/informers"
 	clientset "k8s.io/client-go/kubernetes"
@@ -43,6 +45,7 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
+	"k8s.io/client-go/tools/metrics"
 	"k8s.io/client-go/transport"
 	"k8s.io/client-go/util/cert"
 	"k8s.io/client-go/util/connrotation"
@@ -54,6 +57,7 @@ import (
 
 // These constants are used to communicate behavior to the testdata/exec-plugin.sh test fixture.
 const (
+	exitCodeEnvVar   = "EXEC_PLUGIN_EXEC_CODE"
 	outputEnvVar     = "EXEC_PLUGIN_OUTPUT"
 	outputFileEnvVar = "EXEC_PLUGIN_OUTPUT_FILE"
 )
@@ -81,6 +85,23 @@ func (s *syncedHeaderValues) get() [][]string {
 	return s.data
 }
 
+type execPluginCall struct {
+	exitCode   int
+	callStatus string
+}
+
+type execPluginMetrics struct {
+	calls []execPluginCall
+}
+
+func (m *execPluginMetrics) Increment(exitCode int, callStatus string) {
+	m.calls = append(m.calls, execPluginCall{exitCode: exitCode, callStatus: callStatus})
+}
+
+var execPluginMetricsComparer = cmp.Comparer(func(a, b *execPluginMetrics) bool {
+	return reflect.DeepEqual(a, b)
+})
+
 func TestExecPluginViaClient(t *testing.T) {
 	result, clientAuthorizedToken, clientCertFileName, clientKeyFileName := startTestServer(t)
 
@@ -94,7 +115,9 @@ func TestExecPluginViaClient(t *testing.T) {
 		clientConfigFunc              func(*rest.Config)
 		wantAuthorizationHeaderValues [][]string
 		wantCertificate               *tls.Certificate
+		wantGetCertificateErrorPrefix string
 		wantClientErrorPrefix         string
+		wantMetrics                   *execPluginMetrics
 	}{
 		{
 			name: "unauthorized token",
@@ -115,6 +138,13 @@ func TestExecPluginViaClient(t *testing.T) {
 			wantAuthorizationHeaderValues: [][]string{{"Bearer unauthorized"}},
 			wantCertificate:               &tls.Certificate{},
 			wantClientErrorPrefix:         "Unauthorized",
+			wantMetrics: &execPluginMetrics{
+				calls: []execPluginCall{
+					// 2 calls since we preemptively refresh the creds upon a 401 HTTP response.
+					{exitCode: 0, callStatus: "no_error"},
+					{exitCode: 0, callStatus: "no_error"},
+				},
+			},
 		},
 		{
 			name: "unauthorized certificate",
@@ -136,6 +166,13 @@ func TestExecPluginViaClient(t *testing.T) {
 			wantAuthorizationHeaderValues: [][]string{nil},
 			wantCertificate:               x509KeyPair(unauthorizedCert, unauthorizedKey, true),
 			wantClientErrorPrefix:         "Unauthorized",
+			wantMetrics: &execPluginMetrics{
+				calls: []execPluginCall{
+					// 2 calls since we preemptively refresh the creds upon a 401 HTTP response.
+					{exitCode: 0, callStatus: "no_error"},
+					{exitCode: 0, callStatus: "no_error"},
+				},
+			},
 		},
 		{
 			name: "authorized token",
@@ -155,6 +192,7 @@ func TestExecPluginViaClient(t *testing.T) {
 			},
 			wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}},
 			wantCertificate:               &tls.Certificate{},
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
 		},
 		{
 			name: "authorized certificate",
@@ -175,6 +213,7 @@ func TestExecPluginViaClient(t *testing.T) {
 			},
 			wantAuthorizationHeaderValues: [][]string{nil},
 			wantCertificate:               loadX509KeyPair(clientCertFileName, clientKeyFileName),
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
 		},
 		{
 			name: "authorized token and certificate",
@@ -196,6 +235,7 @@ func TestExecPluginViaClient(t *testing.T) {
 			},
 			wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}},
 			wantCertificate:               loadX509KeyPair(clientCertFileName, clientKeyFileName),
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
 		},
 		{
 			name: "unauthorized token and authorized certificate favors authorized certificate",
@@ -217,6 +257,7 @@ func TestExecPluginViaClient(t *testing.T) {
 			},
 			wantAuthorizationHeaderValues: [][]string{{"Bearer client-unauthorized-token"}},
 			wantCertificate:               loadX509KeyPair(clientCertFileName, clientKeyFileName),
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
 		},
 		{
 			name: "authorized token and unauthorized certificate favors authorized token",
@@ -238,6 +279,7 @@ func TestExecPluginViaClient(t *testing.T) {
 			},
 			wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}},
 			wantCertificate:               x509KeyPair([]byte(unauthorizedCert), []byte(unauthorizedKey), true),
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
 		},
 		{
 			name: "unauthorized token and unauthorized certificate",
@@ -260,6 +302,13 @@ func TestExecPluginViaClient(t *testing.T) {
 			wantAuthorizationHeaderValues: [][]string{{"Bearer client-unauthorized-token"}},
 			wantCertificate:               x509KeyPair(unauthorizedCert, unauthorizedKey, true),
 			wantClientErrorPrefix:         "Unauthorized",
+			wantMetrics: &execPluginMetrics{
+				calls: []execPluginCall{
+					// 2 calls since we preemptively refresh the creds upon a 401 HTTP response.
+					{exitCode: 0, callStatus: "no_error"},
+					{exitCode: 0, callStatus: "no_error"},
+				},
+			},
 		},
 		{
 			name: "good token with static auth basic creds favors static auth basic creds",
@@ -282,6 +331,10 @@ func TestExecPluginViaClient(t *testing.T) {
 			wantAuthorizationHeaderValues: [][]string{{"Basic " + basicAuthHeaderValue("unauthorized", "unauthorized")}},
 			wantCertificate:               &tls.Certificate{},
 			wantClientErrorPrefix:         "Unauthorized",
+			// I don't think we should be calling the exec plugin here. We don't call the exec
+			// plugin in the case where bearer tokens are already present, and this case is
+			// similar. See https://github.com/kubernetes/kubernetes/pull/102175.
+			wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
 		},
 		{
 			name: "good token with static auth bearer token favors static auth bearer token",
@@ -302,6 +355,7 @@ func TestExecPluginViaClient(t *testing.T) {
 			},
 			wantAuthorizationHeaderValues: [][]string{{"Bearer some-unauthorized-token"}},
 			wantClientErrorPrefix:         "Unauthorized",
+			wantMetrics:                   &execPluginMetrics{},
 		},
 		{
 			// This is not the behavior we would expect, see
@@ -325,16 +379,59 @@ func TestExecPluginViaClient(t *testing.T) {
 			},
 			wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}},
 			wantCertificate:               x509KeyPair(unauthorizedCert, unauthorizedKey, false),
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}},
+		},
+		{
+			name: "unknown binary",
+			clientConfigFunc: func(c *rest.Config) {
+				c.ExecProvider.Command = "does not exist"
+			},
+			wantGetCertificateErrorPrefix: "exec: executable does not exist not found",
+			wantClientErrorPrefix:         `Get "https`,
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 1, callStatus: "plugin_not_found_error"}}},
+		},
+		{
+			name: "binary not executable",
+			clientConfigFunc: func(c *rest.Config) {
+				c.ExecProvider.Command = "./testdata/exec-plugin-not-executable.sh"
+			},
+			wantGetCertificateErrorPrefix: "exec: fork/exec ./testdata/exec-plugin-not-executable.sh: permission denied",
+			wantClientErrorPrefix:         `Get "https`,
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 1, callStatus: "client_internal_error"}}},
+		},
+		{
+			name: "binary fails",
+			clientConfigFunc: func(c *rest.Config) {
+				c.ExecProvider.Env = []clientcmdapi.ExecEnvVar{
+					{
+						Name:  exitCodeEnvVar,
+						Value: "10",
+					},
+				}
+			},
+			wantGetCertificateErrorPrefix: "exec: executable testdata/exec-plugin.sh failed with exit code 10",
+			wantClientErrorPrefix:         `Get "https`,
+			wantMetrics:                   &execPluginMetrics{calls: []execPluginCall{{exitCode: 10, callStatus: "plugin_execution_error"}}},
 		},
 	}
 	for _, test := range tests {
+		test := test
 		t.Run(test.name, func(t *testing.T) {
+			actualMetrics := captureMetrics(t)
+
 			var authorizationHeaderValues syncedHeaderValues
 			clientConfig := rest.AnonymousClientConfig(result.ClientConfig)
 			clientConfig.ExecProvider = &clientcmdapi.ExecConfig{
 				Command: "testdata/exec-plugin.sh",
 				// TODO(ankeesler): move to v1 once exec plugins go GA.
 				APIVersion: "client.authentication.k8s.io/v1beta1",
+				Args: []string{
+					// If we didn't have this arg, then some metrics assertions might fail because
+					// the authenticator may be pulled from a globalCache and therefore it may have
+					// already fetched a valid credential.
+					"--random-arg-to-avoid-authenticator-cache-hits",
+					rand.String(10),
+				},
 			}
 			clientConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper {
 				return roundTripperFunc(func(req *http.Request) (*http.Response, error) {
@@ -361,6 +458,11 @@ func TestExecPluginViaClient(t *testing.T) {
 				t.Fatal(err)
 			}
 
+			// Validate that the proper metrics were set.
+			if diff := cmp.Diff(test.wantMetrics, actualMetrics, execPluginMetricsComparer); diff != "" {
+				t.Error("unexpected metrics; -want, +got:\n" + diff)
+			}
+
 			// Validate that the right token is used.
 			if diff := cmp.Diff(test.wantAuthorizationHeaderValues, authorizationHeaderValues.get()); diff != "" {
 				t.Error("unexpected authorization header values; -want, +got:\n" + diff)
@@ -377,7 +479,11 @@ func TestExecPluginViaClient(t *testing.T) {
 				}
 			} else {
 				cert, err := tlsConfig.GetClientCertificate(&tls.CertificateRequestInfo{})
-				if err != nil {
+				if len(test.wantGetCertificateErrorPrefix) != 0 {
+					if err == nil || !strings.HasPrefix(err.Error(), test.wantGetCertificateErrorPrefix) {
+						t.Fatalf(`got %q, wanted "%s..."`, err, test.wantGetCertificateErrorPrefix)
+					}
+				} else if err != nil {
 					t.Fatal(err)
 				}
 				if diff := cmp.Diff(test.wantCertificate, cert); diff != "" {
@@ -388,6 +494,17 @@ func TestExecPluginViaClient(t *testing.T) {
 	}
 }
 
+func captureMetrics(t *testing.T) *execPluginMetrics {
+	previousCallsMetric := metrics.ExecPluginCalls
+	t.Cleanup(func() {
+		metrics.ExecPluginCalls = previousCallsMetric
+	})
+
+	actualMetrics := &execPluginMetrics{}
+	metrics.ExecPluginCalls = actualMetrics
+	return actualMetrics
+}
+
 // objectMetaSansResourceVersionComparer compares two metav1.ObjectMeta's except for their resource
 // versions. Since the underlying integration test etcd is shared, these resource versions may jump
 // past the next sequential number for sequential API calls in the test.
@@ -666,7 +783,7 @@ func startTestServer(t *testing.T) (result *kubeapiservertesting.TestServer, cli
 	clientAuthorizedToken = "client-authorized-token"
 	tokenFileName := writeTokenFile(t, clientAuthorizedToken)
 	clientCAFileName, clientSigningCert, clientSigningKey := writeCACertFiles(t, certDir)
-	clientCertFileName, clientKeyFileName = writeCerts(t, clientSigningCert, clientSigningKey, certDir, 30*time.Second)
+	clientCertFileName, clientKeyFileName = writeCerts(t, clientSigningCert, clientSigningKey, certDir, time.Hour)
 	result = kubeapiservertesting.StartTestServerOrDie(
 		t,
 		nil,

test/integration/client/testdata/exec-plugin-not-executable.sh

@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+
+# Copyright 2018 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This file is not executable, and therefore it will not be run.
+exit 1

test/integration/client/testdata/exec-plugin.sh

@@ -23,4 +23,6 @@ if [[ -n "${EXEC_PLUGIN_OUTPUT_FILE-""}" ]]; then
   exit
 fi
 
-echo "$EXEC_PLUGIN_OUTPUT"
+echo "${EXEC_PLUGIN_OUTPUT-""}"
+
+exit "${EXEC_PLUGIN_EXEC_CODE-0}"