Refactor the secret loading into a prerunner and added e2e tests

.gitignore

@@ -3,5 +3,5 @@ bin/
 .makerc
 .vimrc
 logs/
-cluster/kubeconfig
+**/cluster/kubeconfig
 certs/

cmd/main.go

@@ -22,13 +22,21 @@ import (
 	"github.com/tetratelabs/run"
 	"github.com/tetratelabs/run/pkg/signal"
 	"github.com/tetratelabs/telemetry"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
 	"github.com/tetrateio/authservice-go/internal"
 	"github.com/tetrateio/authservice-go/internal/oidc"
 	"github.com/tetrateio/authservice-go/internal/server"
 )
 
 func main() {
+	k8sClient, err := getKubeClient()
+	if err != nil {
+		fmt.Printf("error creting k8s client: %v\n", err)
+		os.Exit(-1)
+	}
+
 	var (
 		configFile  = &internal.LocalConfigFile{}
 		logging     = internal.NewLogSystem(log.New(), &configFile.Config)
@@ -37,6 +45,7 @@ func main() {
 		envoyAuthz  = server.NewExtAuthZFilter(&configFile.Config, jwks, sessions)
 		authzServer = server.New(&configFile.Config, envoyAuthz.Register)
 		healthz     = server.NewHealthServer(&configFile.Config)
+		secrets     = internal.NewSecretLoader(&configFile.Config, k8sClient)
 	)
 
 	configLog := run.NewPreRunner("config-log", func() error {
@@ -52,6 +61,7 @@ func main() {
 	g.Register(
 		configFile,        // load the configuration
 		logging,           // set up the logging system
+		secrets,           // load the secrets and update the configuration
 		configLog,         // log the configuration
 		jwks,              // start the JWKS provider
 		sessions,          // start the session store
@@ -65,3 +75,18 @@ func main() {
 		os.Exit(-1)
 	}
 }
+
+// getKubeClient returns a new Kubernetes client used to load secrets.
+func getKubeClient() (client.Client, error) {
+	cfg, err := config.GetConfig()
+	if err != nil {
+		return nil, fmt.Errorf("error getting kube config: %w", err)
+	}
+
+	cl, err := client.New(cfg, client.Options{})
+	if err != nil {
+		return nil, fmt.Errorf("errot creating kube client: %w", err)
+	}
+
+	return cl, nil
+}

e2e/istio/cluster/manifests/authservice.yaml

@@ -94,6 +94,39 @@ spec:
           configMap:
             name: authservice-config
 ---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: authservice-secrets
+  namespace: authservice
+rules:
+  - apiGroups: [""]
+    resources: ["secrets"]
+    verbs: ["get", "watch", "list"]
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: authservice-secrets
+  namespace: authservice
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: authservice-secrets
+subjects:
+  - kind: ServiceAccount
+    name: authservice
+    namespace: authservice
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: client-secret
+  namespace: authservice
+type: Opaque
+stringData:
+  client-secret: "authservice-secret"
+---
 kind: ConfigMap
 apiVersion: v1
 metadata:
@@ -116,7 +149,10 @@ data:
                 "configuration_uri": "http://keycloak.keycloak:8080/realms/master/.well-known/openid-configuration",
                 "callback_uri": "https://http-echo.authservice.internal/callback",
                 "client_id": "authservice",
-                "client_secret": "authservice-secret",
+                "client_secret_ref": {
+                  "namespace": "authservice",
+                  "name": "client-secret"
+                },
                 "cookie_name_prefix": "authservice",
                 "id_token": {
                   "preamble": "Bearer",

internal/config.go

@@ -15,7 +15,6 @@
 package internal
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"net/url"
@@ -26,10 +25,6 @@ import (
 	"github.com/tetratelabs/run"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
 	configv1 "github.com/tetrateio/authservice-go/config/gen/go/v1"
 	oidcv1 "github.com/tetrateio/authservice-go/config/gen/go/v1/oidc"
@@ -49,9 +44,6 @@ var (
 	ErrHealthPortInUse     = errors.New("health port is already in use by listen port")
 	ErrMustNotBeRootPath   = errors.New("must not be root path")
 	ErrMustBeDifferentPath = errors.New("must be different path")
-
-	// kubeClient is the in-cluster Kubernetes client used to retrieve the client secret from a Kubernetes secret.
-	kubeClient client.Client
 )
 
 // LocalConfigFile is a run.Config that loads the configuration file.
@@ -125,12 +117,6 @@ func (l *LocalConfigFile) Validate() error {
 		return err
 	}
 
-	// Initialize the client secret from the Kubernetes secret if it is a referenced
-	// Kubernetes secret.
-	if err = initClientSecret(&l.Config); err != nil {
-		return err
-	}
-
 	// Now that all defaults are set and configurations are merged, validate all final settings
 	return l.Config.ValidateAll()
 }
@@ -291,82 +277,3 @@ func hasRootPath(uri string) bool {
 func isRootPath(path string) bool {
 	return path == "/" || path == ""
 }
-
-func initClientSecret(cfg *configv1.Config) error {
-	var (
-		err  error
-		errs []error
-	)
-
-	for _, fc := range cfg.Chains {
-		for _, f := range fc.Filters {
-			if f.GetOidc() != nil && f.GetOidc().GetClientSecretRef() != nil {
-				if kubeClient == nil {
-					kubeClient, err = getKubeClient()
-					if err != nil {
-						return fmt.Errorf("error creating kube client: %w", err)
-					}
-				}
-
-				clientSecret, err := getClientSecretFromK8s(f.GetOidc(), kubeClient)
-				if err != nil {
-					errs = append(errs, fmt.Errorf("error getting client secret: %w", err))
-				}
-
-				f.GetOidc().ClientSecretConfig = &oidcv1.OIDCConfig_ClientSecret{
-					ClientSecret: clientSecret,
-				}
-			}
-		}
-	}
-
-	return errors.Join(errs...)
-}
-
-const (
-	defaultNamespace = "default"
-	clientSecretKey  = "client-secret"
-)
-
-// getClientSecretFromK8s retrieves the client secret from the referenced Kubernetes secret.
-func getClientSecretFromK8s(c *oidcv1.OIDCConfig, cl client.Client) (string, error) {
-	if c.GetClientSecretRef() == nil {
-		return "", fmt.Errorf("client secret ref not found")
-	}
-
-	namespace := c.GetClientSecretRef().Namespace
-	if namespace == "" {
-		namespace = defaultNamespace
-	}
-	secretName := types.NamespacedName{
-		Namespace: namespace,
-		Name:      c.GetClientSecretRef().Name,
-	}
-
-	secret := &v1.Secret{}
-	err := cl.Get(context.Background(), secretName, secret)
-	if err != nil {
-		return "", fmt.Errorf("error getting secret: %w", err)
-	}
-
-	clientSecretBytes, ok := secret.Data[clientSecretKey]
-	if !ok || len(clientSecretBytes) == 0 {
-		return "", fmt.Errorf("client secret not found in secret %s", secretName.String())
-	}
-
-	return string(clientSecretBytes), nil
-}
-
-func getKubeClient() (client.Client, error) {
-	cfg, err := config.GetConfig()
-	if err != nil {
-		return nil, fmt.Errorf("error getting kube config: %w", err)
-	}
-
-	cl, err := client.New(cfg, client.Options{})
-	if err != nil {
-		return nil, fmt.Errorf("errot creating kube client: %w", err)
-	}
-
-	return cl, nil
-}

internal/config_test.go

@@ -24,9 +24,6 @@ import (
 	"github.com/tetratelabs/telemetry"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/structpb"
-	v1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	configv1 "github.com/tetrateio/authservice-go/config/gen/go/v1"
 	mockv1 "github.com/tetrateio/authservice-go/config/gen/go/v1/mock"
@@ -266,96 +263,3 @@ func TestConfigToJSONString(t *testing.T) {
 		})
 	}
 }
-
-func TestLoadOIDCClientSecret(t *testing.T) {
-	validSecret := &v1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "test-secret",
-		},
-		Data: map[string][]byte{
-			clientSecretKey: []byte("fake-client-secret"),
-		},
-	}
-	invalidSecret := &v1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "invalid-secret",
-		},
-		Data: map[string][]byte{
-			clientSecretKey + "-invalid": []byte("fake-client-secret"),
-		},
-	}
-
-	// inject kube client for the test
-	kubeClient = fake.NewClientBuilder().WithObjects(validSecret, invalidSecret).Build()
-
-	tests := []struct {
-		name       string
-		configFile string
-		want       *configv1.Config
-		error      string
-	}{
-		{
-			name:       "valid-secret",
-			configFile: "oidc-with-valid-secret-ref",
-			want: &configv1.Config{
-				ListenAddress: "0.0.0.0",
-				ListenPort:    8080,
-				LogLevel:      "debug",
-				Threads:       1,
-				Chains: []*configv1.FilterChain{
-					{
-						Name: "oidc",
-						Filters: []*configv1.Filter{
-							{
-								Type: &configv1.Filter_Oidc{
-									Oidc: &oidcv1.OIDCConfig{
-										AuthorizationUri:        "http://fake",
-										TokenUri:                "http://fake",
-										CallbackUri:             "http://fake/callback",
-										JwksConfig:              &oidcv1.OIDCConfig_Jwks{Jwks: "fake-jwks"},
-										ClientId:                "fake-client-id",
-										ClientSecretConfig:      &oidcv1.OIDCConfig_ClientSecret{ClientSecret: "fake-client-secret"},
-										CookieNamePrefix:        "",
-										IdToken:                 &oidcv1.TokenConfig{Preamble: "Bearer", Header: "authorization"},
-										ProxyUri:                "http://fake",
-										RedisSessionStoreConfig: &oidcv1.RedisConfig{ServerUri: "redis://localhost:6379/0"},
-										Scopes:                  []string{scopeOIDC},
-										Logout:                  &oidcv1.LogoutConfig{Path: "/logout", RedirectUri: "http://fake"},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			error: "",
-		},
-		{
-			name:       "valid-secret",
-			configFile: "oidc-with-invalid-secret-ref",
-			error:      "client secret not found in secret default/invalid-secret",
-		},
-		{
-			name:       "valid-secret",
-			configFile: "oidc-with-non-existing-secret-ref",
-			error:      "secrets \"non-existing-secret\" not found",
-		},
-	}
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			var cfg LocalConfigFile
-			g := run.Group{Logger: telemetry.NoopLogger()}
-			g.Register(&cfg)
-			err := g.Run("", "--config-path", fmt.Sprintf("testdata/%s.json", tc.configFile))
-			if tc.error != "" {
-				require.Error(t, err)
-				require.Contains(t, err.Error(), tc.error)
-			} else {
-				require.NoError(t, err)
-				require.True(t, proto.Equal(tc.want, &cfg.Config))
-			}
-		})
-	}
-}