Merge pull request #358 from jottofar/bug-1809195

Bug 1809195: Send CVO metrics over https

Author: openshift-merge-robot

cmd/start.go

@@ -30,5 +30,7 @@ func init() {
 	cmd.PersistentFlags().BoolVar(&opts.EnableAutoUpdate, "enable-auto-update", opts.EnableAutoUpdate, "Enables the autoupdate controller.")
 	cmd.PersistentFlags().BoolVar(&opts.EnableDefaultClusterVersion, "enable-default-cluster-version", opts.EnableDefaultClusterVersion, "Allows the operator to create a ClusterVersion object if one does not already exist.")
 	cmd.PersistentFlags().StringVar(&opts.ReleaseImage, "release-image", opts.ReleaseImage, "The Openshift release image url.")
+	cmd.PersistentFlags().StringVar(&opts.ServingCertFile, "serving-cert-file", opts.ServingCertFile, "The X.509 certificate file for serving metrics over HTTPS.  You must set both --serving-cert-file and --serving-key-file, or neither.")
+	cmd.PersistentFlags().StringVar(&opts.ServingKeyFile, "serving-key-file", opts.ServingKeyFile, "The X.509 key file for serving metrics over HTTPS.  You must set both --serving-cert-file and --serving-key-file, or neither.")
 	rootCmd.AddCommand(cmd)
 }

go.mod

@@ -4,33 +4,34 @@ go 1.13
 
 require (
 	github.com/blang/semver v3.5.0+incompatible
+	github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292
 	github.com/davecgh/go-spew v1.1.1
 	github.com/evanphx/json-patch v4.5.0+incompatible // indirect
 	github.com/ghodss/yaml v1.0.0
-	github.com/gogo/protobuf v1.3.0 // indirect
 	github.com/golang/groupcache v0.0.0-20191002201903-404acd9df4cc // indirect
 	github.com/google/go-cmp v0.3.1 // indirect
 	github.com/google/uuid v1.1.1
 	github.com/googleapis/gnostic v0.3.1 // indirect
 	github.com/hashicorp/golang-lru v0.5.3 // indirect
 	github.com/imdario/mergo v0.3.8 // indirect
-	github.com/openshift/api v0.0.0-20191028120151-c556078b427f
-	github.com/openshift/client-go v0.0.0-20191001081553-3b0e988f8cb0
+	github.com/openshift/api v0.0.0-20200210091934-a0e53e94816b
+	github.com/openshift/client-go v0.0.0-20200116152001-92a2713fa240
+	github.com/openshift/library-go v0.0.0-20200303185131-81598fff9efa
 	github.com/pkg/errors v0.8.1
 	github.com/prometheus/client_golang v1.1.0
 	github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4
 	github.com/prometheus/common v0.7.0 // indirect
 	github.com/prometheus/procfs v0.0.5 // indirect
 	github.com/spf13/cobra v0.0.5
-	golang.org/x/crypto v0.0.0-20191002192127-34f69633bfdc
+	golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550
 	golang.org/x/sys v0.0.0-20191003212358-c178f38b412c // indirect
 	golang.org/x/time v0.0.0-20190921001708-c4c64cad1fd0
 	google.golang.org/appengine v1.6.4 // indirect
 	k8s.io/api v0.17.3
-	k8s.io/apiextensions-apiserver v0.17.0
+	k8s.io/apiextensions-apiserver v0.17.1
 	k8s.io/apimachinery v0.17.3
 	k8s.io/client-go v0.17.3
 	k8s.io/klog v1.0.0
-	k8s.io/kube-aggregator v0.17.0
+	k8s.io/kube-aggregator v0.17.1
 	k8s.io/utils v0.0.0-20191114184206-e782cd3c129f
 )

go.sum

@@ -26,6 +26,8 @@ spec:
           - "--release-image={{.ReleaseImage}}"
           - "--enable-auto-update=false"
           - "--enable-default-cluster-version=true"
+          - "--serving-cert-file=/etc/tls/serving-cert/tls.crt"
+          - "--serving-key-file=/etc/tls/serving-cert/tls.key"
           - "--v=4"
         resources:
           requests:
@@ -39,6 +41,9 @@ spec:
           - mountPath: /etc/cvo/updatepayloads
             name: etc-cvo-updatepayloads
             readOnly: true
+          - mountPath: /etc/tls/serving-cert
+            name: serving-cert
+            readOnly: true
         env:
           - name: KUBERNETES_SERVICE_PORT # allows CVO to communicate with apiserver directly on same host.
             value: "6443"
@@ -80,3 +85,6 @@ spec:
         - name: etc-cvo-updatepayloads
           hostPath:
             path: /etc/cvo/updatepayloads
+        - name: serving-cert
+          secret:
+            secretName: cluster-version-operator-serving-cert

install/0000_00_cluster-version-operator_03_deployment.yaml

@@ -9,9 +9,13 @@ metadata:
     exclude.release.openshift.io/internal-openshift-hosted: "true"
 spec:
   endpoints:
+  - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
   - interval: 30s
     port: metrics
-    scheme: http
+    scheme: https
+    tlsConfig:
+      caFile: /etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt
+      serverName: cluster-version-operator.openshift-cluster-version.svc
   namespaceSelector:
     matchNames:
     - openshift-cluster-version

install/0000_90_cluster-version-operator_02_servicemonitor.yaml

@@ -4,15 +4,20 @@ package start
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
+	"io/ioutil"
 	"math/rand"
+	"net"
 	"net/http"
 	"os"
 	"os/signal"
 	"sync"
 	"syscall"
 	"time"
 
+	"github.com/cockroachdb/cmux"
+
 	"github.com/google/uuid"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	v1 "k8s.io/api/core/v1"
@@ -34,6 +39,7 @@ import (
 	"github.com/openshift/cluster-version-operator/pkg/autoupdate"
 	"github.com/openshift/cluster-version-operator/pkg/cvo"
 	"github.com/openshift/cluster-version-operator/pkg/internal"
+	"github.com/openshift/library-go/pkg/crypto"
 )
 
 const (
@@ -49,7 +55,9 @@ const (
 
 // Options are the valid inputs to starting the CVO.
 type Options struct {
-	ReleaseImage string
+	ReleaseImage    string
+	ServingCertFile string
+	ServingKeyFile  string
 
 	Kubeconfig string
 	NodeName   string
@@ -103,6 +111,12 @@ func (o *Options) Run() error {
 	if o.ReleaseImage == "" {
 		return fmt.Errorf("missing --release-image flag, it is required")
 	}
+	if o.ServingCertFile == "" && o.ServingKeyFile != "" {
+		return fmt.Errorf("--serving-key-file was set, so --serving-cert-file must also be set")
+	}
+	if o.ServingKeyFile == "" && o.ServingCertFile != "" {
+		return fmt.Errorf("--serving-cert-file was set, so --serving-key-file must also be set")
+	}
 	if len(o.PayloadOverride) > 0 {
 		klog.Warningf("Using an override payload directory for testing only: %s", o.PayloadOverride)
 	}
@@ -153,16 +167,79 @@ func (o *Options) Run() error {
 	return nil
 }
 
+func (o *Options) makeTLSConfig() (*tls.Config, error) {
+	// Load the initial certificate contents.
+	certBytes, err := ioutil.ReadFile(o.ServingCertFile)
+	if err != nil {
+		return nil, err
+	}
+	keyBytes, err := ioutil.ReadFile(o.ServingKeyFile)
+	if err != nil {
+		return nil, err
+	}
+	certificate, err := tls.X509KeyPair(certBytes, keyBytes)
+	if err != nil {
+		return nil, err
+	}
+
+	return crypto.SecureTLSConfig(&tls.Config{
+		GetCertificate: func(_ *tls.ClientHelloInfo) (*tls.Certificate, error) {
+			return &certificate, nil
+		},
+	}), nil
+}
+
 func (o *Options) run(ctx context.Context, controllerCtx *Context, lock *resourcelock.ConfigMapLock) {
 	// listen on metrics
 	if len(o.ListenAddr) > 0 {
-		mux := http.NewServeMux()
-		mux.Handle("/metrics", promhttp.Handler())
+		handler := http.NewServeMux()
+		handler.Handle("/metrics", promhttp.Handler())
+		tcpl, err := net.Listen("tcp", o.ListenAddr)
+		if err != nil {
+			klog.Fatalf("Listen error: %v", err)
+		}
+
+		// if a TLS connection was requested, set up a connection mux that will send TLS requests to
+		// the TLS server but send HTTP requests to the HTTP server. Preserves the ability for legacy
+		// HTTP, needed during upgrade, while still allowing TLS certs and end to end metrics protection.
+		m := cmux.New(tcpl)
+
+		// match HTTP first
+		httpl := m.Match(cmux.HTTP1())
 		go func() {
-			if err := http.ListenAndServe(o.ListenAddr, mux); err != nil {
-				klog.Fatalf("Unable to start metrics server: %v", err)
+			s := &http.Server{
+				Handler: handler,
+			}
+			if err := s.Serve(httpl); err != cmux.ErrListenerClosed {
+				klog.Fatalf("HTTP serve error: %v", err)
 			}
 		}()
+
+		if o.ServingCertFile != "" || o.ServingKeyFile != "" {
+			tlsConfig, err := o.makeTLSConfig()
+			if err != nil {
+				klog.Fatalf("Failed to create TLS config: %v", err)
+			}
+
+			tlsListener := tls.NewListener(m.Match(cmux.Any()), tlsConfig)
+			klog.Infof("Metrics port listening for HTTP and HTTPS on %v", o.ListenAddr)
+			go func() {
+				s := &http.Server{
+					Handler: handler,
+				}
+				if err := s.Serve(tlsListener); err != cmux.ErrListenerClosed {
+					klog.Fatalf("HTTPS serve error: %v", err)
+				}
+			}()
+
+			go func() {
+				if err := m.Serve(); err != nil {
+					klog.Errorf("CMUX serve error: %v", err)
+				}
+			}()
+		} else {
+			klog.Infof("Metrics port listening for HTTP on %v", o.ListenAddr)
+		}
 	}
 
 	exit := make(chan struct{})