Merge pull request #29377 from openshift-cherrypick-robot/cherry-pick-29176-to-release-4.18

OCPBUGS-48474: Replace RunHostCmd with Exec function to censor bearer token being exposed

Author: openshift-merge-bot[bot]

test/extended/prometheus/prometheus.go

@@ -27,7 +27,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"
@@ -454,8 +453,7 @@ var _ = g.Describe("[sig-instrumentation] Prometheus [apigroup:image.openshift.i
 	defer g.GinkgoRecover()
 	ctx := context.TODO()
 	var (
-		oc = exutil.NewCLIWithPodSecurityLevel("prometheus", admissionapi.LevelBaseline)
-
+		oc                                                                  = exutil.NewCLIWithPodSecurityLevel("prometheus", admissionapi.LevelBaseline)
 		queryURL, prometheusURL, querySvcURL, prometheusSvcURL, bearerToken string
 	)
 
@@ -505,7 +503,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.
@@ -523,7 +521,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.PollUntilContextTimeout(context.Background(), 10*time.Second, 2*time.Minute, true, func(context.Context) (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
@@ -939,15 +937,6 @@ func findMetricLabels(f *dto.MetricFamily, labels map[string]string, match strin
 	return result
 }
 
-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

@@ -34,8 +34,7 @@ import (
 var _ = g.Describe("[sig-network][Feature:Router]", func() {
 	defer g.GinkgoRecover()
 	var (
-		oc = exutil.NewCLIWithPodSecurityLevel("router-metrics", admissionapi.LevelBaseline)
-
+		oc                              = exutil.NewCLIWithPodSecurityLevel("router-metrics", admissionapi.LevelBaseline)
 		username, password, bearerToken string
 		metricsPort                     int32
 		execPodName, ns, host           string
@@ -153,9 +152,8 @@ 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))
 				o.Expect(err).NotTo(o.HaveOccurred())
 
@@ -234,7 +232,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) }()
 
@@ -278,7 +276,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", url), token)
+				contents, err := prometheus.GetBearerTokenURLViaPod(oc, execPod.Name, fmt.Sprintf("%s/api/v1/targets?state=active", url), token)
 				o.Expect(err).NotTo(o.HaveOccurred())
 
 				targets := &promTargets{}
@@ -440,15 +438,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"
@@ -931,7 +932,8 @@ 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
@@ -940,6 +942,16 @@ func (c *CLI) start(stdOutBuff, stdErrBuff *bytes.Buffer) (*exec.Cmd, error) {
 	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

@@ -435,3 +435,16 @@ func MustJoinUrlPath(base string, paths ...string) string {
 	}
 	return path
 }
+
+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
+}