Skip to content

Commit 964a416

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2407 from sbueringer/pr-auth-metrics
⚠ Introduce Metrics Options struct & secure metrics serving
2 parents 5aa6a71 + e59161e commit 964a416

File tree

17 files changed

+1477
-212
lines changed

17 files changed

+1477
-212
lines changed

‎examples/scratch-env/go.sum

Lines changed: 59 additions & 0 deletions
Large diffs are not rendered by default.

‎go.mod

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ require (
2222
k8s.io/api v0.28.0-beta.0
2323
k8s.io/apiextensions-apiserver v0.28.0-beta.0
2424
k8s.io/apimachinery v0.28.0-beta.0
25+
k8s.io/apiserver v0.28.0-beta.0
2526
k8s.io/client-go v0.28.0-beta.0
2627
k8s.io/component-base v0.28.0-beta.0
2728
k8s.io/klog/v2 v2.100.1
@@ -30,21 +31,34 @@ require (
3031
)
3132

3233
require (
34+
github.com/NYTimes/gziphandler v1.1.1 // indirect
35+
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230305170008-8188dc5388df // indirect
36+
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect
3337
github.com/beorn7/perks v1.0.1 // indirect
38+
github.com/blang/semver/v4 v4.0.0 // indirect
39+
github.com/cenkalti/backoff/v4 v4.2.1 // indirect
3440
github.com/cespare/xxhash/v2 v2.2.0 // indirect
41+
github.com/coreos/go-semver v0.3.1 // indirect
42+
github.com/coreos/go-systemd/v22 v22.5.0 // indirect
3543
github.com/davecgh/go-spew v1.1.1 // indirect
3644
github.com/emicklei/go-restful/v3 v3.9.0 // indirect
45+
github.com/felixge/httpsnoop v1.0.3 // indirect
46+
github.com/go-logr/stdr v1.2.2 // indirect
3747
github.com/go-openapi/jsonpointer v0.19.6 // indirect
3848
github.com/go-openapi/jsonreference v0.20.2 // indirect
3949
github.com/go-openapi/swag v0.22.3 // indirect
4050
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
4151
github.com/gogo/protobuf v1.3.2 // indirect
4252
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
4353
github.com/golang/protobuf v1.5.3 // indirect
54+
github.com/google/cel-go v0.16.0 // indirect
4455
github.com/google/gnostic-models v0.6.8 // indirect
4556
github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
4657
github.com/google/uuid v1.3.0 // indirect
58+
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
59+
github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 // indirect
4760
github.com/imdario/mergo v0.3.6 // indirect
61+
github.com/inconshreveable/mousetrap v1.1.0 // indirect
4862
github.com/josharian/intern v1.0.0 // indirect
4963
github.com/json-iterator/go v1.1.12 // indirect
5064
github.com/mailru/easyjson v0.7.7 // indirect
@@ -55,20 +69,44 @@ require (
5569
github.com/pkg/errors v0.9.1 // indirect
5670
github.com/prometheus/common v0.44.0 // indirect
5771
github.com/prometheus/procfs v0.10.1 // indirect
72+
github.com/spf13/cobra v1.7.0 // indirect
5873
github.com/spf13/pflag v1.0.5 // indirect
74+
github.com/stoewer/go-strcase v1.2.0 // indirect
75+
go.etcd.io/etcd/api/v3 v3.5.9 // indirect
76+
go.etcd.io/etcd/client/pkg/v3 v3.5.9 // indirect
77+
go.etcd.io/etcd/client/v3 v3.5.9 // indirect
78+
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.35.0 // indirect
79+
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.35.1 // indirect
80+
go.opentelemetry.io/otel v1.10.0 // indirect
81+
go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.10.0 // indirect
82+
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.10.0 // indirect
83+
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.10.0 // indirect
84+
go.opentelemetry.io/otel/metric v0.31.0 // indirect
85+
go.opentelemetry.io/otel/sdk v1.10.0 // indirect
86+
go.opentelemetry.io/otel/trace v1.10.0 // indirect
87+
go.opentelemetry.io/proto/otlp v0.19.0 // indirect
5988
go.uber.org/multierr v1.11.0 // indirect
89+
golang.org/x/crypto v0.11.0 // indirect
6090
golang.org/x/net v0.12.0 // indirect
6191
golang.org/x/oauth2 v0.8.0 // indirect
92+
golang.org/x/sync v0.2.0 // indirect
6293
golang.org/x/term v0.10.0 // indirect
6394
golang.org/x/text v0.11.0 // indirect
6495
golang.org/x/time v0.3.0 // indirect
6596
golang.org/x/tools v0.9.3 // indirect
6697
google.golang.org/appengine v1.6.7 // indirect
98+
google.golang.org/genproto v0.0.0-20230526161137-0005af68ea54 // indirect
99+
google.golang.org/genproto/googleapis/api v0.0.0-20230525234035-dd9d682886f9 // indirect
100+
google.golang.org/genproto/googleapis/rpc v0.0.0-20230525234030-28d5490b6b19 // indirect
101+
google.golang.org/grpc v1.54.0 // indirect
67102
google.golang.org/protobuf v1.30.0 // indirect
68103
gopkg.in/inf.v0 v0.9.1 // indirect
104+
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
69105
gopkg.in/yaml.v2 v2.4.0 // indirect
70106
gopkg.in/yaml.v3 v3.0.1 // indirect
107+
k8s.io/kms v0.28.0-beta.0 // indirect
71108
k8s.io/kube-openapi v0.0.0-20230717233707-2695361300d9 // indirect
109+
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.1.2 // indirect
72110
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
73111
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
74112
)

‎go.sum

Lines changed: 446 additions & 0 deletions
Large diffs are not rendered by default.

‎pkg/builder/builder_suite_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ import (
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
"k8s.io/apimachinery/pkg/runtime/schema"
2828
"k8s.io/client-go/rest"
29+
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
2930

3031
"sigs.k8s.io/controller-runtime/pkg/envtest"
3132
"sigs.k8s.io/controller-runtime/pkg/internal/testing/addr"
3233
logf "sigs.k8s.io/controller-runtime/pkg/log"
3334
"sigs.k8s.io/controller-runtime/pkg/log/zap"
34-
"sigs.k8s.io/controller-runtime/pkg/metrics"
3535
"sigs.k8s.io/controller-runtime/pkg/webhook"
3636
)
3737

@@ -57,7 +57,7 @@ var _ = BeforeSuite(func() {
5757
Expect(err).NotTo(HaveOccurred())
5858

5959
// Prevent the metrics listener being created
60-
metrics.DefaultBindAddress = "0"
60+
metricsserver.DefaultBindAddress = "0"
6161

6262
webhook.DefaultPort, _, err = addr.Suggest("")
6363
Expect(err).NotTo(HaveOccurred())
@@ -67,7 +67,7 @@ var _ = AfterSuite(func() {
6767
Expect(testenv.Stop()).To(Succeed())
6868

6969
// Put the DefaultBindAddress back
70-
metrics.DefaultBindAddress = ":8080"
70+
metricsserver.DefaultBindAddress = ":8080"
7171

7272
// Change the webhook.DefaultPort back to the original default.
7373
webhook.DefaultPort = 9443

‎pkg/controller/controller_suite_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ import (
2626
"k8s.io/client-go/kubernetes"
2727
"k8s.io/client-go/kubernetes/scheme"
2828
"k8s.io/client-go/rest"
29+
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
2930

3031
"sigs.k8s.io/controller-runtime/pkg/controller/controllertest"
3132
"sigs.k8s.io/controller-runtime/pkg/envtest"
3233
logf "sigs.k8s.io/controller-runtime/pkg/log"
3334
"sigs.k8s.io/controller-runtime/pkg/log/zap"
34-
"sigs.k8s.io/controller-runtime/pkg/metrics"
3535
crscheme "sigs.k8s.io/controller-runtime/pkg/scheme"
3636
)
3737

@@ -79,12 +79,12 @@ var _ = BeforeSuite(func() {
7979
Expect(err).NotTo(HaveOccurred())
8080

8181
// Prevent the metrics listener being created
82-
metrics.DefaultBindAddress = "0"
82+
metricsserver.DefaultBindAddress = "0"
8383
})
8484

8585
var _ = AfterSuite(func() {
8686
Expect(testenv.Stop()).To(Succeed())
8787

8888
// Put the DefaultBindAddress back
89-
metrics.DefaultBindAddress = ":8080"
89+
metricsserver.DefaultBindAddress = ":8080"
9090
})

‎pkg/manager/internal.go

Lines changed: 7 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"time"
2929

3030
"github.com/go-logr/logr"
31-
"github.com/prometheus/client_golang/prometheus/promhttp"
3231
"k8s.io/apimachinery/pkg/api/meta"
3332
"k8s.io/apimachinery/pkg/runtime"
3433
kerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -44,7 +43,7 @@ import (
4443
"sigs.k8s.io/controller-runtime/pkg/healthz"
4544
"sigs.k8s.io/controller-runtime/pkg/internal/httpserver"
4645
intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
47-
"sigs.k8s.io/controller-runtime/pkg/metrics"
46+
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
4847
"sigs.k8s.io/controller-runtime/pkg/webhook"
4948
)
5049

@@ -57,7 +56,6 @@ const (
5756

5857
defaultReadinessEndpoint = "/readyz"
5958
defaultLivenessEndpoint = "/healthz"
60-
defaultMetricsEndpoint = "/metrics"
6159
)
6260

6361
var _ Runnable = &controllerManager{}
@@ -84,11 +82,8 @@ type controllerManager struct {
8482
// on shutdown
8583
leaderElectionReleaseOnCancel bool
8684

87-
// metricsListener is used to serve prometheus metrics
88-
metricsListener net.Listener
89-
90-
// metricsExtraHandlers contains extra handlers to register on http server that serves metrics.
91-
metricsExtraHandlers map[string]http.Handler
85+
// metricsServer is used to serve prometheus metrics
86+
metricsServer metricsserver.Server
9287

9388
// healthProbeListener is used to serve liveness probe
9489
healthProbeListener net.Listener
@@ -184,28 +179,6 @@ func (cm *controllerManager) add(r Runnable) error {
184179
return cm.runnables.Add(r)
185180
}
186181

187-
// AddMetricsExtraHandler adds extra handler served on path to the http server that serves metrics.
188-
func (cm *controllerManager) AddMetricsExtraHandler(path string, handler http.Handler) error {
189-
cm.Lock()
190-
defer cm.Unlock()
191-
192-
if cm.started {
193-
return fmt.Errorf("unable to add new metrics handler because metrics endpoint has already been created")
194-
}
195-
196-
if path == defaultMetricsEndpoint {
197-
return fmt.Errorf("overriding builtin %s endpoint is not allowed", defaultMetricsEndpoint)
198-
}
199-
200-
if _, found := cm.metricsExtraHandlers[path]; found {
201-
return fmt.Errorf("can't register extra handler by duplicate path %q on metrics http server", path)
202-
}
203-
204-
cm.metricsExtraHandlers[path] = handler
205-
cm.logger.V(2).Info("Registering metrics http server extra handler", "path", path)
206-
return nil
207-
}
208-
209182
// AddHealthzCheck allows you to add Healthz checker.
210183
func (cm *controllerManager) AddHealthzCheck(name string, check healthz.Checker) error {
211184
cm.Lock()
@@ -296,27 +269,6 @@ func (cm *controllerManager) GetControllerOptions() config.Controller {
296269
return cm.controllerConfig
297270
}
298271

299-
func (cm *controllerManager) addMetricsServer() error {
300-
mux := http.NewServeMux()
301-
srv := httpserver.New(mux)
302-
303-
handler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{
304-
ErrorHandling: promhttp.HTTPErrorOnError,
305-
})
306-
// TODO(JoelSpeed): Use existing Kubernetes machinery for serving metrics
307-
mux.Handle(defaultMetricsEndpoint, handler)
308-
for path, extraHandler := range cm.metricsExtraHandlers {
309-
mux.Handle(path, extraHandler)
310-
}
311-
312-
return cm.add(&server{
313-
Kind: "metrics",
314-
Log: cm.logger.WithValues("path", defaultMetricsEndpoint),
315-
Server: srv,
316-
Listener: cm.metricsListener,
317-
})
318-
}
319-
320272
func (cm *controllerManager) addHealthProbeServer() error {
321273
mux := http.NewServeMux()
322274
srv := httpserver.New(mux)
@@ -410,8 +362,10 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) {
410362
// Metrics should be served whether the controller is leader or not.
411363
// (If we don't serve metrics for non-leaders, prometheus will still scrape
412364
// the pod but will get a connection refused).
413-
if cm.metricsListener != nil {
414-
if err := cm.addMetricsServer(); err != nil {
365+
if cm.metricsServer != nil {
366+
// Note: We are adding the metrics server directly to HTTPServers here as matching on the
367+
// metricsserver.Server interface in cm.runnables.Add would be very brittle.
368+
if err := cm.runnables.HTTPServers.Add(cm.metricsServer, nil); err != nil {
415369
return fmt.Errorf("failed to add metrics server: %w", err)
416370
}
417371
}

‎pkg/manager/internal/integration/manager_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3535
"k8s.io/apimachinery/pkg/runtime"
3636
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
37+
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
3738

3839
ctrl "sigs.k8s.io/controller-runtime"
3940
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -145,7 +146,7 @@ var _ = Describe("manger.Manager Start", func() {
145146
Scheme: scheme,
146147
HealthProbeBindAddress: ":0",
147148
// Disable metrics to avoid port conflicts.
148-
MetricsBindAddress: "0",
149+
Metrics: metricsserver.Options{BindAddress: "0"},
149150
WebhookServer: webhook.NewServer(webhook.Options{
150151
Port: env.WebhookInstallOptions.LocalServingPort,
151152
Host: env.WebhookInstallOptions.LocalServingHost,

‎pkg/manager/manager.go

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"k8s.io/client-go/tools/leaderelection/resourcelock"
3434
"k8s.io/client-go/tools/record"
3535
"k8s.io/utils/pointer"
36+
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
3637

3738
"sigs.k8s.io/controller-runtime/pkg/cache"
3839
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -43,7 +44,6 @@ import (
4344
intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
4445
"sigs.k8s.io/controller-runtime/pkg/leaderelection"
4546
"sigs.k8s.io/controller-runtime/pkg/log"
46-
"sigs.k8s.io/controller-runtime/pkg/metrics"
4747
"sigs.k8s.io/controller-runtime/pkg/recorder"
4848
"sigs.k8s.io/controller-runtime/pkg/webhook"
4949
)
@@ -65,13 +65,6 @@ type Manager interface {
6565
// election was configured.
6666
Elected() <-chan struct{}
6767

68-
// AddMetricsExtraHandler adds an extra handler served on path to the http server that serves metrics.
69-
// Might be useful to register some diagnostic endpoints e.g. pprof. Note that these endpoints meant to be
70-
// sensitive and shouldn't be exposed publicly.
71-
// If the simple path -> handler mapping offered here is not enough, a new http server/listener should be added as
72-
// Runnable to the manager via Add method.
73-
AddMetricsExtraHandler(path string, handler http.Handler) error
74-
7568
// AddHealthzCheck allows you to add Healthz checker
7669
AddHealthzCheck(name string, check healthz.Checker) error
7770

@@ -219,10 +212,8 @@ type Options struct {
219212
// between tries of actions. Default is 2 seconds.
220213
RetryPeriod *time.Duration
221214

222-
// MetricsBindAddress is the TCP address that the controller should bind to
223-
// for serving prometheus metrics.
224-
// It can be set to "0" to disable the metrics serving.
225-
MetricsBindAddress string
215+
// Metrics are the metricsserver.Options that will be used to create the metricsserver.Server.
216+
Metrics metricsserver.Options
226217

227218
// HealthProbeBindAddress is the TCP address that the controller should bind to
228219
// for serving health probes
@@ -243,8 +234,8 @@ type Options struct {
243234
PprofBindAddress string
244235

245236
// WebhookServer is an externally configured webhook.Server. By default,
246-
// a Manager will create a default server using Port, Host, and CertDir;
247-
// if this is set, the Manager will use this server instead.
237+
// a Manager will create a server via webhook.NewServer with default settings.
238+
// If this is set, the Manager will use this server instead.
248239
WebhookServer webhook.Server
249240

250241
// BaseContext is the function that provides Context values to Runnables
@@ -279,7 +270,7 @@ type Options struct {
279270
// Dependency injection for testing
280271
newRecorderProvider func(config *rest.Config, httpClient *http.Client, scheme *runtime.Scheme, logger logr.Logger, makeBroadcaster intrec.EventBroadcasterProducer) (*intrec.Provider, error)
281272
newResourceLock func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error)
282-
newMetricsListener func(addr string) (net.Listener, error)
273+
newMetricsServer func(options metricsserver.Options, config *rest.Config, httpClient *http.Client) (metricsserver.Server, error)
283274
newHealthProbeListener func(addr string) (net.Listener, error)
284275
newPprofListener func(addr string) (net.Listener, error)
285276
}
@@ -383,16 +374,12 @@ func New(config *rest.Config, options Options) (Manager, error) {
383374
}
384375
}
385376

386-
// Create the metrics listener. This will throw an error if the metrics bind
387-
// address is invalid or already in use.
388-
metricsListener, err := options.newMetricsListener(options.MetricsBindAddress)
377+
// Create the metrics server.
378+
metricsServer, err := options.newMetricsServer(options.Metrics, config, cluster.GetHTTPClient())
389379
if err != nil {
390380
return nil, err
391381
}
392382

393-
// By default we have no extra endpoints to expose on metrics http server.
394-
metricsExtraHandlers := make(map[string]http.Handler)
395-
396383
// Create health probes listener. This will throw an error if the bind
397384
// address is invalid or already in use.
398385
healthProbeListener, err := options.newHealthProbeListener(options.HealthProbeBindAddress)
@@ -417,8 +404,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
417404
errChan: errChan,
418405
recorderProvider: recorderProvider,
419406
resourceLock: resourceLock,
420-
metricsListener: metricsListener,
421-
metricsExtraHandlers: metricsExtraHandlers,
407+
metricsServer: metricsServer,
422408
controllerConfig: options.Controller,
423409
logger: options.Logger,
424410
elected: make(chan struct{}),
@@ -464,8 +450,8 @@ func (o Options) AndFrom(loader config.ControllerManagerConfiguration) (Options,
464450
o.Cache.DefaultNamespaces = map[string]cache.Config{newObj.CacheNamespace: {}}
465451
}
466452

467-
if o.MetricsBindAddress == "" && newObj.Metrics.BindAddress != "" {
468-
o.MetricsBindAddress = newObj.Metrics.BindAddress
453+
if o.Metrics.BindAddress == "" && newObj.Metrics.BindAddress != "" {
454+
o.Metrics.BindAddress = newObj.Metrics.BindAddress
469455
}
470456

471457
if o.HealthProbeBindAddress == "" && newObj.Health.HealthProbeBindAddress != "" {
@@ -616,8 +602,8 @@ func setOptionsDefaults(options Options) Options {
616602
}
617603
}
618604

619-
if options.newMetricsListener == nil {
620-
options.newMetricsListener = metrics.NewListener
605+
if options.newMetricsServer == nil {
606+
options.newMetricsServer = metricsserver.NewServer
621607
}
622608
leaseDuration, renewDeadline, retryPeriod := defaultLeaseDuration, defaultRenewDeadline, defaultRetryPeriod
623609
if options.LeaseDuration == nil {

0 commit comments

Comments
 (0)