From 584acb2cfe32e2a8683aee299fad7def6bc78ea7 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 20 May 2021 09:10:49 -0400 Subject: [PATCH] test/integration/client: test exec calls metric Signed-off-by: Andrew Keesler --- test/integration/client/exec_test.go | 121 +++++++++++++++++- .../testdata/exec-plugin-not-executable.sh | 18 +++ .../client/testdata/exec-plugin.sh | 4 +- 3 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 test/integration/client/testdata/exec-plugin-not-executable.sh diff --git a/test/integration/client/exec_test.go b/test/integration/client/exec_test.go index fcaae20748f..3d97d6b248b 100644 --- a/test/integration/client/exec_test.go +++ b/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, diff --git a/test/integration/client/testdata/exec-plugin-not-executable.sh b/test/integration/client/testdata/exec-plugin-not-executable.sh new file mode 100644 index 00000000000..8f957349cd9 --- /dev/null +++ b/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 diff --git a/test/integration/client/testdata/exec-plugin.sh b/test/integration/client/testdata/exec-plugin.sh index de20bcea27b..9052761e2f5 100755 --- a/test/integration/client/testdata/exec-plugin.sh +++ b/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}"