From 478335660e225b02f1f80aead7f50a2f85bad075 Mon Sep 17 00:00:00 2001 From: Shilpa-Gokul Date: Thu, 10 Oct 2024 11:38:30 +0530 Subject: [PATCH] Replace RunHostCmd with oc exec function to censor bearer token being exposed --- test/extended/prometheus/prometheus.go | 17 +++-------------- test/extended/router/metrics.go | 19 ++++--------------- test/extended/util/client.go | 14 +++++++++++++- test/extended/util/prometheus/helpers.go | 13 +++++++++++++ 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/test/extended/prometheus/prometheus.go b/test/extended/prometheus/prometheus.go index e14121454d6a..cb2f4e561227 100644 --- a/test/extended/prometheus/prometheus.go +++ b/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 @@ -929,15 +927,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. diff --git a/test/extended/router/metrics.go b/test/extended/router/metrics.go index 8293ea84ab4b..d290dd4ab622 100644 --- a/test/extended/router/metrics.go +++ b/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) { diff --git a/test/extended/util/client.go b/test/extended/util/client.go index ae7e373a4b5b..c17b7d840612 100644 --- a/test/extended/util/client.go +++ b/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 ") + } + 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 { diff --git a/test/extended/util/prometheus/helpers.go b/test/extended/util/prometheus/helpers.go index cbccb3f9367c..5a3b4642889b 100644 --- a/test/extended/util/prometheus/helpers.go +++ b/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 +}