Merge pull request #115 from JoshVanL/readiness-check

Makes readiness probe rely on proxy initialization on issuer

Author: jetstack-bot

cmd/app/options/kube_oidc_proxy.go

@@ -13,6 +13,8 @@ type TokenPassthroughOptions struct {
 type KubeOIDCProxyOptions struct {
 	DisableImpersonation bool
 	TokenPassthrough     TokenPassthroughOptions
+
+	ReadinessProbePort int
 }
 
 func (t *TokenPassthroughOptions) AddFlags(fs *pflag.FlagSet) {
@@ -34,5 +36,8 @@ func (k *KubeOIDCProxyOptions) AddFlags(fs *pflag.FlagSet) {
 		"(Alpha) Disable the impersonation of authenticated requests. All "+
 			"authenticated requests will be forwarded as is.")
 
+	fs.IntVarP(&k.ReadinessProbePort, "readiness-probe-port", "P", 8080,
+		"Port to expose readiness probe.")
+
 	k.TokenPassthrough.AddFlags(fs)
 }

cmd/app/run.go

@@ -6,14 +6,11 @@ import (
 	"fmt"
 	"net"
 	"strconv"
-	"time"
 
 	"github.com/spf13/cobra"
-	"k8s.io/apiserver/pkg/authentication/request/bearertoken"
 	"k8s.io/apiserver/pkg/server"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/apiserver/pkg/util/term"
-	"k8s.io/apiserver/plugin/pkg/authenticator/token/oidc"
 	"k8s.io/client-go/rest"
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/component-base/cli/globalflag"
@@ -22,11 +19,12 @@ import (
 	"github.com/jetstack/kube-oidc-proxy/pkg/probe"
 	"github.com/jetstack/kube-oidc-proxy/pkg/proxy"
 	"github.com/jetstack/kube-oidc-proxy/pkg/proxy/tokenreview"
+	"github.com/jetstack/kube-oidc-proxy/pkg/util"
 	"github.com/jetstack/kube-oidc-proxy/pkg/version"
 )
 
 const (
-	readinessProbePort = 8080
+	appName = "kube-oidc-proxy"
 )
 
 func NewRunCommand(stopCh <-chan struct{}) *cobra.Command {
@@ -38,7 +36,7 @@ func NewRunCommand(stopCh <-chan struct{}) *cobra.Command {
 		BindPort:    6443,
 		Required:    true,
 		ServerCert: apiserveroptions.GeneratableKeyCert{
-			PairName:      "kube-oidc-proxy",
+			PairName:      appName,
 			CertDirectory: "/var/run/kubernetes",
 		},
 	}
@@ -48,11 +46,9 @@ func NewRunCommand(stopCh <-chan struct{}) *cobra.Command {
 
 	clientConfigOptions := options.NewClientFlags()
 
-	healthCheck := probe.New(strconv.Itoa(readinessProbePort))
-
 	// proxy command
 	cmd := &cobra.Command{
-		Use:  "kube-oidc-proxy",
+		Use:  appName,
 		Long: "kube-oidc-proxy is a reverse proxy to authenticate users to Kubernetes API servers with Open ID Connect Authentication.",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			var err error
@@ -65,19 +61,19 @@ func NewRunCommand(stopCh <-chan struct{}) *cobra.Command {
 				return err
 			}
 
-			if ssoptionsWithLB.SecureServingOptions.BindPort == readinessProbePort {
+			if ssoptionsWithLB.SecureServingOptions.BindPort == kopOptions.ReadinessProbePort {
 				return errors.New("unable to securely serve on port 8080, used by readiness prob")
 			}
 
 			var restConfig *rest.Config
-
 			if clientConfigOptions.ClientFlagsChanged(cmd) {
 				// one or more client flags have been set to use client flag built
 				// config
 				restConfig, err = clientConfigOptions.ToRESTConfig()
 				if err != nil {
 					return err
 				}
+
 			} else {
 				// no client flags have been set so default to in-cluster config
 				restConfig, err = rest.InClusterConfig()
@@ -86,24 +82,7 @@ func NewRunCommand(stopCh <-chan struct{}) *cobra.Command {
 				}
 			}
 
-			// oidc config
-			oidcAuther, err := oidc.New(oidc.Options{
-				APIAudiences:         oidcOptions.APIAudiences,
-				CAFile:               oidcOptions.CAFile,
-				ClientID:             oidcOptions.ClientID,
-				GroupsClaim:          oidcOptions.GroupsClaim,
-				GroupsPrefix:         oidcOptions.GroupsPrefix,
-				IssuerURL:            oidcOptions.IssuerURL,
-				RequiredClaims:       oidcOptions.RequiredClaims,
-				SupportedSigningAlgs: oidcOptions.SigningAlgs,
-				UsernameClaim:        oidcOptions.UsernameClaim,
-				UsernamePrefix:       oidcOptions.UsernamePrefix,
-			})
-			if err != nil {
-				return err
-			}
-
-			// Init token reviewer if enabled
+			// Initialise token reviewer if enabled
 			var tokenReviewer *tokenreview.TokenReview
 			if kopOptions.TokenPassthrough.Enabled {
 				tokenReviewer, err = tokenreview.New(restConfig, kopOptions.TokenPassthrough.Audiences)
@@ -112,8 +91,7 @@ func NewRunCommand(stopCh <-chan struct{}) *cobra.Command {
 				}
 			}
 
-			// oidc auther from config
-			reqAuther := bearertoken.New(oidcAuther)
+			// Initialise Secure Serving Config
 			secureServingInfo := new(server.SecureServingInfo)
 			if err := ssoptionsWithLB.ApplyTo(&secureServingInfo, nil); err != nil {
 				return err
@@ -124,17 +102,30 @@ func NewRunCommand(stopCh <-chan struct{}) *cobra.Command {
 				DisableImpersonation: kopOptions.DisableImpersonation,
 			}
 
-			p := proxy.New(restConfig, reqAuther,
+			// Initialise proxy with OIDC token authenticator
+			p, err := proxy.New(restConfig, oidcOptions,
 				tokenReviewer, secureServingInfo, proxyOptions)
+			if err != nil {
+				return err
+			}
 
-			// run proxy
-			waitCh, err := p.Run(stopCh)
+			// Create a fake JWT to set up readiness probe
+			fakeJWT, err := util.FakeJWT(oidcOptions.IssuerURL, oidcOptions.APIAudiences)
 			if err != nil {
 				return err
 			}
 
-			time.Sleep(time.Second * 3)
-			healthCheck.SetReady()
+			// Start readiness probe
+			if err := probe.Run(strconv.Itoa(kopOptions.ReadinessProbePort),
+				fakeJWT, p.OIDCTokenAuthenticator()); err != nil {
+				return err
+			}
+
+			// Run proxy
+			waitCh, err := p.Run(stopCh)
+			if err != nil {
+				return err
+			}
 
 			<-waitCh
 

pkg/probe/probe.go

@@ -2,26 +2,32 @@
 package probe
 
 import (
-	"errors"
+	"context"
+	"fmt"
 	"net"
 	"net/http"
-	"sync"
+	"strings"
 	"time"
 
-	"k8s.io/klog"
-
 	"github.com/heptiolabs/healthcheck"
+	"k8s.io/apiserver/pkg/authentication/authenticator"
+	"k8s.io/klog"
 )
 
 type HealthCheck struct {
 	handler healthcheck.Handler
-	mu      sync.Mutex
-	ready   bool
+
+	oidcAuther authenticator.Token
+	fakeJWT    string
+
+	ready bool
 }
 
-func New(port string) *HealthCheck {
+func Run(port, fakeJWT string, oidcAuther authenticator.Token) error {
 	h := &HealthCheck{
-		handler: healthcheck.NewHandler(),
+		handler:    healthcheck.NewHandler(),
+		oidcAuther: oidcAuther,
+		fakeJWT:    fakeJWT,
 	}
 
 	h.handler.AddReadinessCheck("secure serving", h.Check)
@@ -36,30 +42,25 @@ func New(port string) *HealthCheck {
 		}
 	}()
 
-	return h
+	return nil
 }
 
 func (h *HealthCheck) Check() error {
-	h.mu.Lock()
-	defer h.mu.Unlock()
-
-	if !h.ready {
-		return errors.New("not ready")
+	if h.ready {
+		return nil
 	}
 
-	return nil
-}
-
-func (h *HealthCheck) SetReady() {
-	h.mu.Lock()
-	defer h.mu.Unlock()
+	_, _, err := h.oidcAuther.AuthenticateToken(context.Background(), h.fakeJWT)
+	if err != nil && strings.HasSuffix(err.Error(), "authenticator not initialized") {
+		err = fmt.Errorf("OIDC provider not yet initialized: %s", err)
+		klog.V(4).Infof(err.Error())
+		return err
+	}
 
 	h.ready = true
-}
 
-func (h *HealthCheck) SetNotReady() {
-	h.mu.Lock()
-	defer h.mu.Unlock()
+	klog.Info("OIDC provider initialized, proxy ready")
+	klog.V(4).Infof("OIDC provider initialized, readiness check returned error: %s", err)
 
-	h.ready = false
+	return nil
 }

pkg/probe/probe_test.go

@@ -2,36 +2,77 @@
 package probe
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"testing"
-	"time"
+
+	"k8s.io/apiserver/pkg/authentication/authenticator"
 
 	"github.com/jetstack/kube-oidc-proxy/pkg/util"
 )
 
-func Test_Check(t *testing.T) {
+type fakeTokenAuthenticator struct {
+	returnErr bool
+}
+
+var _ authenticator.Token = &fakeTokenAuthenticator{}
+
+func (f *fakeTokenAuthenticator) AuthenticateToken(context.Context, string) (*authenticator.Response, bool, error) {
+	if f.returnErr {
+		return nil, false, errors.New("foo bar authenticator not initialized")
+	}
+
+	return nil, false, errors.New("some other error")
+}
+
+func TestRun(t *testing.T) {
+	f := &fakeTokenAuthenticator{
+		returnErr: true,
+	}
+
 	port, err := util.FreePort()
 	if err != nil {
-		t.Fatalf("unexpected error: %s", err)
+		t.Error(err.Error())
+		t.FailNow()
+	}
+
+	fakeJWT, err := util.FakeJWT("issuer", nil)
+	if err != nil {
+		t.Error(err.Error())
+		t.FailNow()
 	}
 
-	p := New(port)
-	time.Sleep(time.Second)
+	if err := Run(port, fakeJWT, f); err != nil {
+		t.Error(err.Error())
+		t.FailNow()
+	}
 
 	url := fmt.Sprintf("http://0.0.0.0:%s", port)
 
-	resp, err := http.Get(url + "/ready")
-	if err != nil {
-		t.Fatalf("unexpected error: %s", err)
+	var resp *http.Response
+	var i int
+
+	for {
+		resp, err = http.Get(url + "/ready")
+		if err == nil {
+			break
+		}
+
+		if i >= 5 {
+			t.Errorf("unexpected error: %s", err)
+			t.FailNow()
+		}
+		i++
 	}
 
 	if resp.StatusCode != 503 {
 		t.Errorf("expected ready probe to be responding and not ready, exp=%d got=%d",
 			503, resp.StatusCode)
 	}
 
-	p.SetReady()
+	f.returnErr = false
 
 	resp, err = http.Get(url + "/ready")
 	if err != nil {
@@ -43,15 +84,18 @@ func Test_Check(t *testing.T) {
 			200, resp.StatusCode)
 	}
 
-	p.SetNotReady()
+	// Once the authenticator has returned with an non-initialised error, then
+	// should always return ready
+
+	f.returnErr = true
 
 	resp, err = http.Get(url + "/ready")
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
 
-	if resp.StatusCode != 503 {
-		t.Errorf("expected ready probe to be responding and not ready, exp=%d got=%d",
-			503, resp.StatusCode)
+	if resp.StatusCode != 200 {
+		t.Errorf("expected ready probe to be responding and ready, exp=%d got=%d",
+			200, resp.StatusCode)
 	}
 }