From b98b2e157da64639277d7a37dc054c7c6ec97ca3 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 26 Aug 2025 00:14:34 -0700 Subject: [PATCH] pkg/cli/admin/inspectalerts: Pass through --certificate-authority, etc. Instead of passing tidbits from the rest.Config through to transport.HTTPWrappersForConfig by hand, use rest.TransportFor. That picks up all the goodies, like TLSClientConfig's CAFile (which is fed by --certificate-authority) and Insecure (which is fed by --insecure-skip-tls-verify). Which makes it possible to invoke both inspect-alerts and also other Go consumers like 'oc adm upgrade recommend' and 'oc adm upgrade status' with options to trust a Thanos-fronting Ingress that they wouldn't otherwise trust. That can be helpful when running against test clusters, where the Thanos-fronting Ingress is less likely to have an X.509 certificate signed by a broadly-trusted Certificate Authority. We still want the clear message about the token requirement though, so there's a new ValidateRESTConfig that callers can wash their rest.Config by before converting it to a RoundTripper. Otherwise the you-didn't-use-a-bearer-token error messages look fairly opaque: ... failed to get alerts from Thanos: unable to get /api/v1/alerts from URI in the openshift-monitoring/thanos-querier Route: thanos-querier-openshift-monitoring.apps...->GET status code=401 I'm also shifting user-agent configuration earlier (to right after the rest.Config's creation) and pushing it out to each of the three commands, now that I'm no longer getting down into the transport weeds within the getWith... helper function. --- pkg/cli/admin/inspectalerts/inspectalerts.go | 51 +++++++++++--------- pkg/cli/admin/upgrade/recommend/alerts.go | 12 ++++- pkg/cli/admin/upgrade/recommend/recommend.go | 2 + pkg/cli/admin/upgrade/status/status.go | 13 ++++- 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/pkg/cli/admin/inspectalerts/inspectalerts.go b/pkg/cli/admin/inspectalerts/inspectalerts.go index cd8759b233..8aa4cfdd80 100644 --- a/pkg/cli/admin/inspectalerts/inspectalerts.go +++ b/pkg/cli/admin/inspectalerts/inspectalerts.go @@ -15,7 +15,6 @@ import ( "k8s.io/apimachinery/pkg/util/errors" "k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/client-go/rest" - "k8s.io/client-go/transport" "k8s.io/klog/v2" kcmdutil "k8s.io/kubectl/pkg/cmd/util" @@ -64,8 +63,12 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string } o.RESTConfig = cfg + o.RESTConfig.UserAgent = rest.DefaultKubernetesUserAgent() + "(inspect-alerts)" + if err = ValidateRESTConfig(o.RESTConfig); err != nil { + return err + } - routeClient, err := routev1client.NewForConfig(cfg) + routeClient, err := routev1client.NewForConfig(o.RESTConfig) if err != nil { return err } @@ -77,7 +80,12 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string } func (o *options) Run(ctx context.Context) error { - alertBytes, err := GetAlerts(ctx, o.getRoute, o.RESTConfig.BearerToken) + roundTripper, err := rest.TransportFor(o.RESTConfig) + if err != nil { + return err + } + + alertBytes, err := GetAlerts(ctx, roundTripper, o.getRoute) if err != nil { return err } @@ -86,15 +94,25 @@ func (o *options) Run(ctx context.Context) error { return err } +// ValidateRESTConfig validates a rest.Config for alert retrieval, +// requiring the use of BearerToken, because the platform Thanos rejects +// other forms of authentication. +func ValidateRESTConfig(restConfig *rest.Config) error { + if restConfig.BearerToken == "" && restConfig.BearerTokenFile == "" { + return fmt.Errorf("no token is currently in use for this session") + } + return nil +} + // GetAlerts gets alerts (both firing and pending) from openshift-monitoring Thanos. -func GetAlerts(ctx context.Context, getRoute RouteGetter, bearerToken string) ([]byte, error) { +func GetAlerts(ctx context.Context, roundTripper http.RoundTripper, getRoute RouteGetter) ([]byte, error) { uri := &url.URL{ // configure everything except Host, which will come from the Route Scheme: "https", Path: "/api/v1/alerts", } // if we end up going this way, probably port to github.com/prometheus/client_golang/api/prometheus/v1 NewAPI - alertBytes, err := getWithBearer(ctx, getRoute, "openshift-monitoring", "thanos-querier", uri, bearerToken) + alertBytes, err := getWithRoundTripper(ctx, roundTripper, getRoute, "openshift-monitoring", "thanos-querier", uri) if err != nil { return alertBytes, fmt.Errorf("failed to get alerts from Thanos: %w", err) } @@ -104,31 +122,16 @@ func GetAlerts(ctx context.Context, getRoute RouteGetter, bearerToken string) ([ return alertBytes, nil } -// getWithBearer gets a Route by namespace/name, constructs a URI using +// getWithRoundTripper gets a Route by namespace/name, constructs a URI using // status.ingress[].host and the path argument, and performs GETs on that -// URI using Bearer authentication with the token argument. -func getWithBearer(ctx context.Context, getRoute RouteGetter, namespace, name string, baseURI *url.URL, bearerToken string) ([]byte, error) { - if len(bearerToken) == 0 { - return nil, fmt.Errorf("no token is currently in use for this session") - } - +// URI. +func getWithRoundTripper(ctx context.Context, roundTripper http.RoundTripper, getRoute RouteGetter, namespace, name string, baseURI *url.URL) ([]byte, error) { route, err := getRoute(ctx, namespace, name, metav1.GetOptions{}) if err != nil { return nil, err } - withDebugWrappers, err := transport.HTTPWrappersForConfig( - &transport.Config{ - UserAgent: rest.DefaultKubernetesUserAgent() + "(inspect-alerts)", - BearerToken: bearerToken, - }, - http.DefaultTransport, - ) - if err != nil { - return nil, err - } - - client := &http.Client{Transport: withDebugWrappers} + client := &http.Client{Transport: roundTripper} errs := make([]error, 0, len(route.Status.Ingress)) for _, ingress := range route.Status.Ingress { uri := *baseURI diff --git a/pkg/cli/admin/upgrade/recommend/alerts.go b/pkg/cli/admin/upgrade/recommend/alerts.go index 014bd94ce5..2beeb05e3a 100644 --- a/pkg/cli/admin/upgrade/recommend/alerts.go +++ b/pkg/cli/admin/upgrade/recommend/alerts.go @@ -9,6 +9,7 @@ import ( routev1 "github.com/openshift/api/route/v1" routev1client "github.com/openshift/client-go/route/clientset/versioned/typed/route/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" "github.com/openshift/oc/pkg/cli/admin/inspectalerts" "github.com/openshift/oc/pkg/cli/admin/upgrade/status" @@ -34,6 +35,15 @@ func (o *options) alerts(ctx context.Context) ([]acceptableCondition, error) { } alertsBytes = o.mockData.alerts } else { + if err := inspectalerts.ValidateRESTConfig(o.RESTConfig); err != nil { + return nil, err + } + + roundTripper, err := rest.TransportFor(o.RESTConfig) + if err != nil { + return nil, err + } + client, err := routev1client.NewForConfig(o.RESTConfig) if err != nil { return nil, err @@ -42,7 +52,7 @@ func (o *options) alerts(ctx context.Context) ([]acceptableCondition, error) { return client.Routes(namespace).Get(ctx, name, opts) } - alertsBytes, err = inspectalerts.GetAlerts(ctx, routeGetter, o.RESTConfig.BearerToken) + alertsBytes, err = inspectalerts.GetAlerts(ctx, roundTripper, routeGetter) if err != nil { return nil, err } diff --git a/pkg/cli/admin/upgrade/recommend/recommend.go b/pkg/cli/admin/upgrade/recommend/recommend.go index 2701f0316d..d066743783 100644 --- a/pkg/cli/admin/upgrade/recommend/recommend.go +++ b/pkg/cli/admin/upgrade/recommend/recommend.go @@ -105,6 +105,8 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string if err != nil { return err } + o.RESTConfig.UserAgent = rest.DefaultKubernetesUserAgent() + "(upgrade-recommend)" + o.Client, err = configv1client.NewForConfig(o.RESTConfig) if err != nil { return err diff --git a/pkg/cli/admin/upgrade/status/status.go b/pkg/cli/admin/upgrade/status/status.go index b3346d4461..26f5be1be5 100644 --- a/pkg/cli/admin/upgrade/status/status.go +++ b/pkg/cli/admin/upgrade/status/status.go @@ -19,6 +19,7 @@ import ( "k8s.io/cli-runtime/pkg/genericiooptions" appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/rest" kcmdutil "k8s.io/kubectl/pkg/cmd/util" configv1 "github.com/openshift/api/config/v1" @@ -107,6 +108,12 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string if err != nil { return err } + cfg.UserAgent = rest.DefaultKubernetesUserAgent() + "(upgrade-status)" + + roundTripper, err := rest.TransportFor(cfg) + if err != nil { + return err + } configClient, err := configv1client.NewForConfig(cfg) if err != nil { return err @@ -140,7 +147,11 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string o.AppsClient = appsClient o.getAlerts = func(ctx context.Context) ([]byte, error) { - return inspectalerts.GetAlerts(ctx, routeGetter, cfg.BearerToken) + if err := inspectalerts.ValidateRESTConfig(cfg); err != nil { + return nil, err + } + + return inspectalerts.GetAlerts(ctx, roundTripper, routeGetter) } } else { err := o.mockData.load()