Replace RunHostCmd with Exec function to censor bearer token being exposed

Author: KeerthanaAP

test/extended/prometheus/prometheus.go

@@ -29,7 +29,6 @@ import (
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2e "k8s.io/kubernetes/test/e2e/framework"
-	e2eoutput "k8s.io/kubernetes/test/e2e/framework/pod/output"
 	e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
 	admissionapi "k8s.io/pod-security-admission/api"
 	"sigs.k8s.io/yaml"
@@ -343,7 +342,7 @@ var _ = g.Describe("[sig-instrumentation] Prometheus [apigroup:image.openshift.i
 			err := helper.RunQueries(context.TODO(), oc.NewPrometheusClient(context.TODO()), tests, oc)
 			o.Expect(err).NotTo(o.HaveOccurred())
 
-			e2e.Logf("Telemetry is enabled: %s", bearerToken)
+			e2e.Logf("Telemetry is enabled")
 
 			if err != nil {
 				// Making the test flaky until monitoring team fixes the rate limit issue.
@@ -361,7 +360,7 @@ var _ = g.Describe("[sig-instrumentation] Prometheus [apigroup:image.openshift.i
 			g.By("checking the prometheus metrics path")
 			var metrics map[string]*dto.MetricFamily
 			o.Expect(wait.PollImmediate(10*time.Second, 2*time.Minute, func() (bool, error) {
-				results, err := getBearerTokenURLViaPod(ns, execPod.Name, fmt.Sprintf("%s/metrics", prometheusSvcURL), bearerToken)
+				results, err := helper.GetBearerTokenURLViaPod(oc, execPod.Name, fmt.Sprintf("%s/metrics", prometheusSvcURL), bearerToken)
 				if err != nil {
 					e2e.Logf("unable to get metrics: %v", err)
 					return false, nil
@@ -769,15 +768,6 @@ func getBearerTokenURL(url, bearer string) (string, error) {
 	return string(output), nil
 }
 
-func getBearerTokenURLViaPod(ns, execPodName, url, bearer string) (string, error) {
-	cmd := fmt.Sprintf("curl -s -k -H 'Authorization: Bearer %s' %q", bearer, url)
-	output, err := e2eoutput.RunHostCmd(ns, execPodName, cmd)
-	if err != nil {
-		return "", fmt.Errorf("host command failed: %v\n%s", err, output)
-	}
-	return output, nil
-}
-
 // telemetryIsEnabled returns (nil, nil) if Telemetry is enabled,
 // (error, nil) if Telemetry is not enabled, and (_, error) if it fails
 // to determine whether or not Telemetry is enabled.

test/extended/router/metrics.go

@@ -30,6 +30,7 @@ import (
 	admissionapi "k8s.io/pod-security-admission/api"
 
 	exutil "github.com/openshift/origin/test/extended/util"
+	"github.com/openshift/origin/test/extended/util/prometheus"
 
 	configv1 "github.com/openshift/api/config/v1"
 	routev1 "github.com/openshift/api/route/v1"
@@ -152,7 +153,7 @@ var _ = g.Describe("[sig-network][Feature:Router]", func() {
 			p := expfmt.TextParser{}
 
 			err = wait.PollImmediate(2*time.Second, 240*time.Second, func() (bool, error) {
-				results, err = getBearerTokenURLViaPod(ns, execPodName, fmt.Sprintf("http://%s/metrics", net.JoinHostPort(host, strconv.Itoa(int(metricsPort)))), bearerToken)
+				results, err = prometheus.GetBearerTokenURLViaPod(oc, execPodName, fmt.Sprintf("http://%s/metrics", net.JoinHostPort(host, strconv.Itoa(int(metricsPort)))), bearerToken)
 				o.Expect(err).NotTo(o.HaveOccurred())
 
 				metrics, err = p.TextToMetricFamilies(bytes.NewBufferString(results))
@@ -225,7 +226,7 @@ var _ = g.Describe("[sig-network][Feature:Router]", func() {
 			time.Sleep(15 * time.Second)
 
 			g.By("checking that some metrics are not reset to 0 after router restart")
-			updatedResults, err := getBearerTokenURLViaPod(ns, execPodName, fmt.Sprintf("http://%s/metrics", net.JoinHostPort(host, strconv.Itoa(int(metricsPort)))), bearerToken)
+			updatedResults, err := prometheus.GetBearerTokenURLViaPod(oc, execPodName, fmt.Sprintf("http://%s/metrics", net.JoinHostPort(host, strconv.Itoa(int(metricsPort)))), bearerToken)
 			o.Expect(err).NotTo(o.HaveOccurred())
 			defer func() { e2e.Logf("final metrics:\n%s", updatedResults) }()
 
@@ -267,7 +268,7 @@ var _ = g.Describe("[sig-network][Feature:Router]", func() {
 			}()
 
 			o.Expect(wait.PollImmediate(10*time.Second, 5*time.Minute, func() (bool, error) {
-				contents, err := getBearerTokenURLViaPod(ns, execPod.Name, fmt.Sprintf("%s/api/v1/targets?state=active", prometheusURL), token)
+				contents, err := prometheus.GetBearerTokenURLViaPod(oc, execPod.Name, fmt.Sprintf("%s/api/v1/targets?state=active", prometheusURL), token)
 				o.Expect(err).NotTo(o.HaveOccurred())
 
 				targets := &promTargets{}
@@ -448,15 +449,6 @@ func getAuthenticatedURLViaPod(ns, execPodName, url, user, pass string) (string,
 	return output, nil
 }
 
-func getBearerTokenURLViaPod(ns, execPodName, url, bearer string) (string, error) {
-	cmd := fmt.Sprintf("curl -s -k -H 'Authorization: Bearer %s' %q", bearer, url)
-	output, err := e2eoutput.RunHostCmd(ns, execPodName, cmd)
-	if err != nil {
-		return "", fmt.Errorf("host command failed: %v\n%s", err, output)
-	}
-	return output, nil
-}
-
 func waitForAdmittedRoute(maxInterval time.Duration, client routev1client.RouteV1Interface, ns, name, ingressName string, errorOnRejection bool) (string, error) {
 	var routeHost string
 	err := wait.PollImmediate(time.Second, maxInterval, func() (bool, error) {

test/extended/util/client.go

@@ -16,6 +16,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"runtime/debug"
 	"strings"
 	"time"
@@ -907,15 +908,25 @@ func (c *CLI) start(stdOutBuff, stdErrBuff *bytes.Buffer) (*exec.Cmd, error) {
 	}
 	cmd := exec.Command(c.execPath, c.finalArgs...)
 	cmd.Stdin = c.stdin
-	framework.Logf("Running '%s %s'", c.execPath, strings.Join(c.finalArgs, " "))
-
+	// Redact any bearer token information from the log.
+	framework.Logf("Running '%s %s'", c.execPath, redactBearerToken(c.finalArgs))
 	cmd.Stdout = stdOutBuff
 	cmd.Stderr = stdErrBuff
 	err := cmd.Start()
 
 	return cmd, err
 }
 
+func redactBearerToken(finalArgs []string) string {
+	args := strings.Join(finalArgs, " ")
+	if strings.Contains(args, "Authorization: Bearer") {
+		// redact bearer token
+		re := regexp.MustCompile(`Authorization:\s+Bearer.*\s+`)
+		args = re.ReplaceAllString(args, "Authorization: Bearer <redacted> ")
+	}
+	return args
+}
+
 // getStartingIndexForLastN calculates a byte offset in a byte slice such that when using
 // that offset, we get the last N (size) bytes.
 func getStartingIndexForLastN(byteString []byte, size int) int {

test/extended/util/prometheus/helpers.go

@@ -430,3 +430,16 @@ func ForEachAlertingRule(rules map[string][]promv1.AlertingRule, f func(a promv1
 
 	return fmt.Errorf("Incompliant rules detected:\n\n%s", strings.Join(allViolations.List(), "\n"))
 }
+
+func GetBearerTokenURLViaPod(oc *exutil.CLI, execPodName, url, bearer string) (string, error) {
+	auth := fmt.Sprintf("Authorization: Bearer %s", bearer)
+	stdout, stderr, err := oc.AsAdmin().Run("exec").Args(execPodName, "--", "curl", "-s", "-k", "-H", auth, url).Outputs()
+	if err != nil {
+		return "", fmt.Errorf("command failed: %v\nstderr: %s\nstdout:%s", err, stderr, stdout)
+	}
+	// Terminate stdout with a newline to avoid an unexpected end of stream error.
+	if len(stdout) > 0 {
+		stdout = stdout + "\n"
+	}
+	return stdout, err
+}