From 7d46ea0bf5b2ebd299aa100deee68c93fdd38ff9 Mon Sep 17 00:00:00 2001 From: Jan Fajerski Date: Fri, 7 Jan 2022 15:18:44 +0100 Subject: [PATCH] prometheus: remove ui access Type: feature removal Problem: We expose the full prometheus http api to authorized users through a route, i.e. its externally available. We want to remove access to the UI for a reduced support footprint, while maintaining access to `/api` and `/federate`. Solution: Alter the existing Route object to be limited to `/api` and create a new Route object to expose `/federate`. For the externalUrl configuration (used in backlinks in alerts for example) the console route is now used. Another solution considered was to alter the oauth-proxy config. This is discarded due to oauth-proxy feature not alligning as needed and any change there also impacting the Service object that exposes the deployment cluster-internal. Issue: https://issues.redhat.com/browse/MON-1631 Signed-off-by: Jan Fajerski --- .../{route.yaml => api-route.yaml} | 1 + assets/prometheus-k8s/federate-route.yaml | 15 +++++++++ assets/prometheus-k8s/prometheus.yaml | 1 + jsonnet/components/prometheus.libsonnet | 30 +++++++++++++++-- pkg/manifests/manifests.go | 33 +++++++++++-------- pkg/manifests/manifests_test.go | 6 +--- pkg/tasks/configsharing.go | 2 +- pkg/tasks/prometheus.go | 27 +++++++++++---- 8 files changed, 88 insertions(+), 27 deletions(-) rename assets/prometheus-k8s/{route.yaml => api-route.yaml} (95%) create mode 100644 assets/prometheus-k8s/federate-route.yaml diff --git a/assets/prometheus-k8s/route.yaml b/assets/prometheus-k8s/api-route.yaml similarity index 95% rename from assets/prometheus-k8s/route.yaml rename to assets/prometheus-k8s/api-route.yaml index bdd0689daf..40e8d1751d 100644 --- a/assets/prometheus-k8s/route.yaml +++ b/assets/prometheus-k8s/api-route.yaml @@ -4,6 +4,7 @@ metadata: name: prometheus-k8s namespace: openshift-monitoring spec: + path: /api port: targetPort: web tls: diff --git a/assets/prometheus-k8s/federate-route.yaml b/assets/prometheus-k8s/federate-route.yaml new file mode 100644 index 0000000000..14a1aa9211 --- /dev/null +++ b/assets/prometheus-k8s/federate-route.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Route +metadata: + name: prometheus-k8s-federate + namespace: openshift-monitoring +spec: + path: /federate + port: + targetPort: web + tls: + insecureEdgeTerminationPolicy: Redirect + termination: Reencrypt + to: + kind: Service + name: prometheus-k8s diff --git a/assets/prometheus-k8s/prometheus.yaml b/assets/prometheus-k8s/prometheus.yaml index 6a7d664803..cc5e72f967 100644 --- a/assets/prometheus-k8s/prometheus.yaml +++ b/assets/prometheus-k8s/prometheus.yaml @@ -161,6 +161,7 @@ spec: memory: 10Mi enableFeatures: [] externalLabels: {} + externalURL: https://prometheus-k8s.openshift-monitoring.svc:9091 image: quay.io/prometheus/prometheus:v2.32.1 listenLocal: true nodeSelector: diff --git a/jsonnet/components/prometheus.libsonnet b/jsonnet/components/prometheus.libsonnet index 74f720a1a5..eb2c23e1b5 100644 --- a/jsonnet/components/prometheus.libsonnet +++ b/jsonnet/components/prometheus.libsonnet @@ -22,8 +22,8 @@ function(params) data: {}, }, - // OpenShift route to access the Prometheus UI. - route: { + // OpenShift route to access the Prometheus api. + apiRoute: { apiVersion: 'v1', kind: 'Route', metadata: { @@ -35,6 +35,31 @@ function(params) kind: 'Service', name: 'prometheus-k8s', }, + path: '/api', + port: { + targetPort: 'web', + }, + tls: { + termination: 'Reencrypt', + insecureEdgeTerminationPolicy: 'Redirect', + }, + }, + }, + + // OpenShift route to access the Prometheus federate endpoint. + federateRoute: { + apiVersion: 'v1', + kind: 'Route', + metadata: { + name: 'prometheus-k8s-federate', + namespace: cfg.namespace, + }, + spec: { + to: { + kind: 'Service', + name: 'prometheus-k8s', + }, + path: '/federate', port: { targetPort: 'web', }, @@ -316,6 +341,7 @@ function(params) 'kube-rbac-proxy', 'metrics-client-certs', ], + externalURL: 'https://prometheus-k8s.openshift-monitoring.svc:9091', configMaps: ['serving-certs-ca-bundle', 'kubelet-serving-ca-bundle', 'metrics-client-ca'], probeNamespaceSelector: cfg.namespaceSelector, podMonitorNamespaceSelector: cfg.namespaceSelector, diff --git a/pkg/manifests/manifests.go b/pkg/manifests/manifests.go index 4e1f98f3f6..eb951e0e35 100644 --- a/pkg/manifests/manifests.go +++ b/pkg/manifests/manifests.go @@ -115,7 +115,8 @@ var ( PrometheusK8sProxySecret = "prometheus-k8s/proxy-secret.yaml" PrometheusRBACProxySecret = "prometheus-k8s/kube-rbac-proxy-secret.yaml" PrometheusUserWorkloadRBACProxySecret = "prometheus-user-workload/kube-rbac-proxy-secret.yaml" - PrometheusK8sRoute = "prometheus-k8s/route.yaml" + PrometheusK8sAPIRoute = "prometheus-k8s/api-route.yaml" + PrometheusK8sFederateRoute = "prometheus-k8s/federate-route.yaml" PrometheusK8sHtpasswd = "prometheus-k8s/htpasswd-secret.yaml" PrometheusK8sServingCertsCABundle = "prometheus-k8s/serving-certs-ca-bundle.yaml" PrometheusK8sKubeletServingCABundle = "prometheus-k8s/kubelet-serving-ca-bundle.yaml" @@ -323,14 +324,6 @@ func NewFactory(namespace, namespaceUserWorkload string, c *Config, infrastructu } } -func (f *Factory) PrometheusExternalURL(host string) *url.URL { - return &url.URL{ - Scheme: "https", - Host: host, - Path: "/", - } -} - func (f *Factory) AlertmanagerExternalURL(host string) *url.URL { return &url.URL{ Scheme: "https", @@ -1344,8 +1337,19 @@ func (f *Factory) PrometheusK8sThanosSidecarServiceMonitor() (*monv1.ServiceMoni return s, nil } -func (f *Factory) PrometheusK8sRoute() (*routev1.Route, error) { - r, err := f.NewRoute(f.assets.MustNewAssetReader(PrometheusK8sRoute)) +func (f *Factory) PrometheusK8sAPIRoute() (*routev1.Route, error) { + r, err := f.NewRoute(f.assets.MustNewAssetReader(PrometheusK8sAPIRoute)) + if err != nil { + return nil, err + } + + r.Namespace = f.namespace + + return r, nil +} + +func (f *Factory) PrometheusK8sFederateRoute() (*routev1.Route, error) { + r, err := f.NewRoute(f.assets.MustNewAssetReader(PrometheusK8sFederateRoute)) if err != nil { return nil, err } @@ -1405,7 +1409,7 @@ func (f *Factory) PrometheusK8sTrustedCABundle() (*v1.ConfigMap, error) { return cm, nil } -func (f *Factory) PrometheusK8s(host string, grpcTLS *v1.Secret, trustedCABundleCM *v1.ConfigMap) (*monv1.Prometheus, error) { +func (f *Factory) PrometheusK8s(grpcTLS *v1.Secret, trustedCABundleCM *v1.ConfigMap) (*monv1.Prometheus, error) { p, err := f.NewPrometheus(f.assets.MustNewAssetReader(PrometheusK8s)) if err != nil { return nil, err @@ -1420,7 +1424,10 @@ func (f *Factory) PrometheusK8s(host string, grpcTLS *v1.Secret, trustedCABundle } p.Spec.Image = &f.config.Images.Prometheus - p.Spec.ExternalURL = f.PrometheusExternalURL(host).String() + + if f.consoleConfig != nil { + p.Spec.ExternalURL = f.consoleConfig.Status.ConsoleURL + "/monitoring" + } if f.config.ClusterMonitoringConfiguration.PrometheusK8sConfig.Resources != nil { p.Spec.Resources = *f.config.ClusterMonitoringConfiguration.PrometheusK8sConfig.Resources diff --git a/pkg/manifests/manifests_test.go b/pkg/manifests/manifests_test.go index cb81b0c69b..b1e410fd30 100644 --- a/pkg/manifests/manifests_test.go +++ b/pkg/manifests/manifests_test.go @@ -375,7 +375,7 @@ func TestUnconfiguredManifests(t *testing.T) { t.Fatal(err) } - _, err = f.PrometheusK8s("prometheus-k8s.openshift-monitoring.svc", &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, nil) + _, err = f.PrometheusK8s(&v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, nil) if err != nil { t.Fatal(err) } @@ -1005,7 +1005,6 @@ func TestPrometheusK8sRemoteWrite(t *testing.T) { f := NewFactory("openshift-monitoring", "openshift-user-workload-monitoring", c, defaultInfrastructureReader(), &fakeProxyReader{}, NewAssets(assetsPath), &APIServerConfig{}, &configv1.Console{}) p, err := f.PrometheusK8s( - "prometheus-k8s.openshift-monitoring.svc", &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, ) @@ -1068,7 +1067,6 @@ ingress: f := NewFactory("openshift-monitoring", "openshift-user-workload-monitoring", c, defaultInfrastructureReader(), &fakeProxyReader{}, NewAssets(assetsPath), &APIServerConfig{}, &configv1.Console{}) p, err := f.PrometheusK8s( - "prometheus-k8s.openshift-monitoring.svc", &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, ) @@ -1384,7 +1382,6 @@ func TestPrometheusK8sAdditionalAlertManagerConfigsSecret(t *testing.T) { f := NewFactory("openshift-monitoring", "openshift-user-workload-monitoring", c, defaultInfrastructureReader(), &fakeProxyReader{}, NewAssets(assetsPath), &APIServerConfig{}, &configv1.Console{}) p, err := f.PrometheusK8s( - "prometheus-k8s.openshift-monitoring.svc", &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, ) @@ -2640,7 +2637,6 @@ func TestNonHighlyAvailableInfrastructure(t *testing.T) { name: "Prometheus", getSpec: func(f *Factory) (spec, error) { p, err := f.PrometheusK8s( - "prometheus-k8s.openshift-monitoring.svc", &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, &v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, ) diff --git a/pkg/tasks/configsharing.go b/pkg/tasks/configsharing.go index 64dcd6a990..3ed12d990e 100644 --- a/pkg/tasks/configsharing.go +++ b/pkg/tasks/configsharing.go @@ -38,7 +38,7 @@ func NewConfigSharingTask(client *client.Client, factory *manifests.Factory, con } func (t *ConfigSharingTask) Run(ctx context.Context) error { - promRoute, err := t.factory.PrometheusK8sRoute() + promRoute, err := t.factory.PrometheusK8sAPIRoute() if err != nil { return errors.Wrap(err, "initializing Prometheus Route failed") } diff --git a/pkg/tasks/prometheus.go b/pkg/tasks/prometheus.go index 991a066254..ef811b913b 100644 --- a/pkg/tasks/prometheus.go +++ b/pkg/tasks/prometheus.go @@ -64,19 +64,34 @@ func (t *PrometheusTask) Run(ctx context.Context) error { return errors.Wrap(err, "creating kubelet serving CA Bundle ConfigMap failed") } - r, err := t.factory.PrometheusK8sRoute() + r, err := t.factory.PrometheusK8sAPIRoute() if err != nil { - return errors.Wrap(err, "initializing Prometheus Route failed") + return errors.Wrap(err, "initializing Prometheus API Route failed") } err = t.client.CreateRouteIfNotExists(ctx, r) if err != nil { - return errors.Wrap(err, "creating Prometheus Route failed") + return errors.Wrap(err, "creating Prometheus API Route failed") } - host, err := t.client.WaitForRouteReady(ctx, r) + _, err = t.client.WaitForRouteReady(ctx, r) if err != nil { - return errors.Wrap(err, "waiting for Prometheus Route to become ready failed") + return errors.Wrap(err, "waiting for Prometheus API Route to become ready failed") + } + + fr, err := t.factory.PrometheusK8sFederateRoute() + if err != nil { + return errors.Wrap(err, "initializing Prometheus Federate Route failed") + } + + err = t.client.CreateRouteIfNotExists(ctx, fr) + if err != nil { + return errors.Wrap(err, "creating Prometheus Federate Route failed") + } + + _, err = t.client.WaitForRouteReady(ctx, fr) + if err != nil { + return errors.Wrap(err, "waiting for Prometheus Federate Route to become ready failed") } ps, err := t.factory.PrometheusK8sProxySecret() @@ -365,7 +380,7 @@ func (t *PrometheusTask) Run(ctx context.Context) error { } klog.V(4).Info("initializing Prometheus object") - p, err := t.factory.PrometheusK8s(host, s, trustedCA) + p, err := t.factory.PrometheusK8s(s, trustedCA) if err != nil { return errors.Wrap(err, "initializing Prometheus object failed") }