diff --git a/controller/cmd/main.go b/controller/cmd/main.go index 05d1703a3..dd358da52 100644 --- a/controller/cmd/main.go +++ b/controller/cmd/main.go @@ -341,25 +341,18 @@ func extractOIDCConfigs(reader client.Reader, namespace string) []login.OIDCConf return nil } - // Try new config format first rawConfig, ok := configmap.Data["config"] - if ok { - var cfg config.Config - if err := yaml.UnmarshalStrict([]byte(rawConfig), &cfg); err == nil { - return jwtAuthenticatorsToOIDCConfigs(cfg.Authentication.JWT) - } + if !ok { + return nil } - // Fall back to legacy authentication format - rawAuth, ok := configmap.Data["authentication"] - if ok { - var auth config.Authentication - if err := yaml.Unmarshal([]byte(rawAuth), &auth); err == nil { - return jwtAuthenticatorsToOIDCConfigs(auth.JWT) - } + var cfg config.Config + if err := yaml.UnmarshalStrict([]byte(rawConfig), &cfg); err != nil { + setupLog.Error(err, "unable to parse config for OIDC config extraction") + return nil } - return nil + return jwtAuthenticatorsToOIDCConfigs(cfg.Authentication.JWT) } // jwtAuthenticatorsToOIDCConfigs converts JWT authenticators to login OIDC configs diff --git a/controller/cmd/main_test.go b/controller/cmd/main_test.go new file mode 100644 index 000000000..cbe9abdd4 --- /dev/null +++ b/controller/cmd/main_test.go @@ -0,0 +1,260 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestExtractOIDCConfigs(t *testing.T) { + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add corev1 to scheme: %v", err) + } + + tests := []struct { + name string + configmap *corev1.ConfigMap + wantCount int + wantNil bool + }{ + { + name: "valid config key with JWT authenticators", + configmap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jumpstarter-controller", + Namespace: "default", + }, + Data: map[string]string{ + "config": ` +authentication: + internal: + prefix: "internal:" + tokenLifetime: "43800h" + jwt: + - issuer: + url: "https://dex.example.com" + audiences: + - "jumpstarter" + claimMappings: + username: + claim: "email" + prefix: "" +`, + }, + }, + wantCount: 1, + }, + { + name: "valid config key with multiple JWT authenticators including localhost", + configmap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jumpstarter-controller", + Namespace: "default", + }, + Data: map[string]string{ + "config": ` +authentication: + internal: + prefix: "internal:" + tokenLifetime: "43800h" + jwt: + - issuer: + url: "https://localhost:8085" + audiences: + - "jumpstarter" + claimMappings: + username: + claim: "sub" + prefix: "" + - issuer: + url: "https://dex.example.com" + audiences: + - "jumpstarter" + claimMappings: + username: + claim: "email" + prefix: "" +`, + }, + }, + wantCount: 1, // localhost issuer should be skipped + }, + { + name: "missing config key returns nil", + configmap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jumpstarter-controller", + Namespace: "default", + }, + Data: map[string]string{ + "router": `default: {endpoint: "router.example.com:443"}`, + }, + }, + wantNil: true, + }, + { + name: "invalid YAML in config key returns nil", + configmap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jumpstarter-controller", + Namespace: "default", + }, + Data: map[string]string{ + "config": `{{{invalid yaml`, + }, + }, + wantNil: true, + }, + { + name: "missing configmap returns nil", + configmap: nil, + wantNil: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scheme) + if tt.configmap != nil { + builder = builder.WithObjects(tt.configmap) + } + fakeClient := builder.Build() + + configs := extractOIDCConfigs(fakeClient.(client.Reader), "default") + + if tt.wantNil { + if configs != nil { + t.Errorf("expected nil, got %v", configs) + } + return + } + + if configs == nil { + t.Fatal("expected non-nil configs, got nil") + } + + if len(configs) != tt.wantCount { + t.Errorf("expected %d configs, got %d", tt.wantCount, len(configs)) + } + }) + } +} + +func TestExtractOIDCConfigsContent(t *testing.T) { + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add corev1 to scheme: %v", err) + } + + configmap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jumpstarter-controller", + Namespace: "default", + }, + Data: map[string]string{ + "config": ` +authentication: + internal: + prefix: "internal:" + tokenLifetime: "43800h" + jwt: + - issuer: + url: "https://dex.example.com" + audiences: + - "jumpstarter" + - "jumpstarter-cli" + claimMappings: + username: + claim: "email" + prefix: "" +`, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(configmap). + Build() + + configs := extractOIDCConfigs(fakeClient.(client.Reader), "default") + + if len(configs) != 1 { + t.Fatalf("expected 1 config, got %d", len(configs)) + } + + cfg := configs[0] + if cfg.Issuer != "https://dex.example.com" { + t.Errorf("expected issuer 'https://dex.example.com', got %q", cfg.Issuer) + } + if cfg.ClientID != "jumpstarter-cli" { + t.Errorf("expected clientID 'jumpstarter-cli', got %q", cfg.ClientID) + } + if len(cfg.Audiences) != 2 { + t.Errorf("expected 2 audiences, got %d", len(cfg.Audiences)) + } +} + +// TestExtractOIDCConfigsIgnoresLegacyKey is a dedicated regression test +// ensuring that a ConfigMap containing only the legacy "authentication" key +// (without the "config" key) does NOT produce any OIDC configurations. +// This prevents accidental reintroduction of the legacy fallback removed in +// this PR. +func TestExtractOIDCConfigsIgnoresLegacyKey(t *testing.T) { + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add corev1 to scheme: %v", err) + } + + configmap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jumpstarter-controller", + Namespace: "default", + }, + Data: map[string]string{ + // Only the legacy key -- this must NOT be read + "authentication": ` +jwt: + - issuer: + url: "https://dex.example.com" + audiences: + - "jumpstarter" + claimMappings: + username: + claim: "email" + prefix: "" +`, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(configmap). + Build() + + configs := extractOIDCConfigs(fakeClient.(client.Reader), "default") + + if configs != nil { + t.Errorf("expected nil when only legacy 'authentication' key is present, got %v", configs) + } +} diff --git a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py index e9d789d7d..1c46df558 100755 --- a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py +++ b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py @@ -358,7 +358,6 @@ class Model(BaseModel): enabled: Optional[bool] = Field( None, description="Whether to enable jumpstarter controller" ) - authenticationConfig: Optional[str] = None config: Optional[JumpstarterConfig] = None namespace: Optional[str] = Field( None, diff --git a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yaml b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yaml index 7f75608d9..bd700baa4 100644 --- a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yaml +++ b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yaml @@ -10,11 +10,6 @@ metadata: deployment.timestamp: {{ .Values.global.timestamp | quote }} {{ end }} data: - # backwards compatibility - # TODO: remove in 0.7.0 - {{ if .Values.authenticationConfig }} - authentication: {{- .Values.authenticationConfig | toYaml | indent 1 }} - {{ end }} config: | {{ .Values.config | toYaml | indent 4 }} router: | diff --git a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.json b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.json index 021e046dc..4046245dc 100644 --- a/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.json +++ b/controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.json @@ -1342,18 +1342,6 @@ "description": "Whether to enable jumpstarter controller", "title": "Enabled" }, - "authenticationConfig": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "default": null, - "title": "Authenticationconfig" - }, "config": { "anyOf": [ { diff --git a/controller/internal/config/config.go b/controller/internal/config/config.go index 0b0913910..80ab749a0 100644 --- a/controller/internal/config/config.go +++ b/controller/internal/config/config.go @@ -3,11 +3,9 @@ package config import ( "context" "fmt" - "time" "github.com/jumpstarter-dev/jumpstarter-controller/internal/oidc" "google.golang.org/grpc" - "google.golang.org/grpc/keepalive" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/yaml" @@ -67,29 +65,6 @@ func LoadConfiguration( return nil, "", nil, nil, nil, err } - rawAuthenticationConfiguration, ok := configmap.Data["authentication"] - if ok { - // backwards compatibility - // TODO: remove in 0.7.0 - authenticator, prefix, err := oidc.LoadAuthenticationConfiguration( - ctx, - scheme, - []byte(rawAuthenticationConfiguration), - signer, - certificateAuthority, - ) - if err != nil { - return nil, "", nil, nil, nil, err - } - - return authenticator, prefix, router, []grpc.ServerOption{ - grpc.KeepaliveEnforcementPolicy(keepalive.EnforcementPolicy{ - MinTime: 1 * time.Second, - PermitWithoutStream: true, - }), - }, &Provisioning{Enabled: false}, nil - } - rawConfig, ok := configmap.Data["config"] if !ok { return nil, "", nil, nil, nil, fmt.Errorf("LoadConfiguration: missing config section") diff --git a/controller/internal/oidc/config.go b/controller/internal/oidc/config.go deleted file mode 100644 index 67550125b..000000000 --- a/controller/internal/oidc/config.go +++ /dev/null @@ -1,106 +0,0 @@ -package oidc - -import ( - "context" - "os" - - jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" - "k8s.io/apiserver/pkg/apis/apiserver" - apiserverv1beta1 "k8s.io/apiserver/pkg/apis/apiserver/v1beta1" - "k8s.io/apiserver/pkg/authentication/authenticator" - tokenunion "k8s.io/apiserver/pkg/authentication/token/union" - "k8s.io/apiserver/pkg/server/dynamiccertificates" - "k8s.io/apiserver/plugin/pkg/authenticator/token/oidc" -) - -func LoadAuthenticationConfiguration( - ctx context.Context, - scheme *runtime.Scheme, - configuration []byte, - signer *Signer, - certificateAuthority string, -) (authenticator.Token, string, error) { - var authenticationConfiguration jumpstarterdevv1alpha1.AuthenticationConfiguration - if err := runtime.DecodeInto( - serializer.NewCodecFactory(scheme, serializer.EnableStrict). - UniversalDecoder(jumpstarterdevv1alpha1.GroupVersion), - configuration, - &authenticationConfiguration, - ); err != nil { - return nil, "", err - } - - if authenticationConfiguration.Internal.Prefix == "" { - authenticationConfiguration.Internal.Prefix = "internal:" - } - - authenticationConfiguration.JWT = append(authenticationConfiguration.JWT, apiserverv1beta1.JWTAuthenticator{ - Issuer: apiserverv1beta1.Issuer{ - URL: signer.Issuer(), - CertificateAuthority: certificateAuthority, - Audiences: []string{signer.Audience()}, - }, - ClaimMappings: apiserverv1beta1.ClaimMappings{ - Username: apiserverv1beta1.PrefixedClaimOrExpression{ - Claim: "sub", - Prefix: &authenticationConfiguration.Internal.Prefix, - }, - }, - }) - - authn, err := newJWTAuthenticator( - ctx, - scheme, - authenticationConfiguration, - ) - if err != nil { - return nil, "", err - } - return authn, authenticationConfiguration.Internal.Prefix, nil -} - -// Reference: https://github.com/kubernetes/kubernetes/blob/v1.32.1/pkg/kubeapiserver/authenticator/config.go#L244 -func newJWTAuthenticator( - ctx context.Context, - scheme *runtime.Scheme, - config jumpstarterdevv1alpha1.AuthenticationConfiguration, -) (authenticator.Token, error) { - var jwtAuthenticators []authenticator.Token - for _, jwtAuthenticator := range config.JWT { - var oidcCAContent oidc.CAContentProvider - if len(jwtAuthenticator.Issuer.CertificateAuthority) > 0 { - var oidcCAError error - if _, err := os.Stat(jwtAuthenticator.Issuer.CertificateAuthority); err == nil { - oidcCAContent, oidcCAError = dynamiccertificates.NewDynamicCAContentFromFile( - "oidc-authenticator", - jwtAuthenticator.Issuer.CertificateAuthority, - ) - jwtAuthenticator.Issuer.CertificateAuthority = "" - } else { - oidcCAContent, oidcCAError = dynamiccertificates.NewStaticCAContent( - "oidc-authenticator", - []byte(jwtAuthenticator.Issuer.CertificateAuthority), - ) - } - if oidcCAError != nil { - return nil, oidcCAError - } - } - var jwtAuthenticatorUnversioned apiserver.JWTAuthenticator - if err := scheme.Convert(&jwtAuthenticator, &jwtAuthenticatorUnversioned, nil); err != nil { - return nil, err - } - oidcAuth, err := oidc.New(ctx, oidc.Options{ - JWTAuthenticator: jwtAuthenticatorUnversioned, - CAContentProvider: oidcCAContent, - SupportedSigningAlgs: oidc.AllValidSigningAlgorithms(), - }) - if err != nil { - return nil, err - } - jwtAuthenticators = append(jwtAuthenticators, oidcAuth) - } - return tokenunion.NewFailOnError(jwtAuthenticators...), nil -} diff --git a/python/docs/source/getting-started/configuration/authentication.md b/python/docs/source/getting-started/configuration/authentication.md index 509561329..ddb9f28e0 100644 --- a/python/docs/source/getting-started/configuration/authentication.md +++ b/python/docs/source/getting-started/configuration/authentication.md @@ -194,8 +194,8 @@ $ helm repo add dex https://charts.dexidp.io $ helm install --namespace dex --wait -f values.yaml dex dex/dex ``` -3. Configure Jumpstarter to trust Dex. Use this configuration for - `jumpstarter-controller.authenticationConfiguration` during installation: +3. Configure Jumpstarter to trust Dex. Set `spec.authentication.jwt` on your + `Jumpstarter` resource: ```yaml spec: