From ceb44884247dec396023ffe002580706be00042f Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Thu, 9 Apr 2026 15:58:49 +0000 Subject: [PATCH 1/4] Remove backwards-compatible authentication config (deprecated since 0.7.0) Remove the legacy `authentication` configmap key and `authenticationConfig` Helm value that were kept for backwards compatibility with a TODO to remove in 0.7.0. The project is now past v0.8.1, so this cleanup is overdue. Also removes the now-unused `oidc.LoadAuthenticationConfiguration` function and updates documentation to reference the current config approach. Closes #530 Co-Authored-By: Claude Opus 4.6 --- .../charts/jumpstarter-controller/model.py | 1 - .../templates/cms/controller-cm.yaml | 5 - .../jumpstarter-controller/values.schema.json | 12 -- controller/internal/config/config.go | 25 ----- controller/internal/oidc/config.go | 106 ------------------ .../configuration/authentication.md | 4 +- 6 files changed, 2 insertions(+), 151 deletions(-) delete mode 100644 controller/internal/oidc/config.go 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: From d26f74586d0c9b018e939cb97dc615d49965d227 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Mon, 13 Apr 2026 18:36:46 +0000 Subject: [PATCH 2/4] Remove legacy authentication fallback from extractOIDCConfigs() The extractOIDCConfigs() function in controller/cmd/main.go still had a fallback block reading configmap.Data["authentication"], which was missed during the initial cleanup. This removes that legacy path so the function only reads from the "config" key, consistent with the rest of this PR. Also improves error handling by logging parse errors instead of silently swallowing them. Co-Authored-By: Claude Opus 4.6 --- controller/cmd/main.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) 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 From 9511fcb49219fcae3fec702c01072b0bcc571920 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Wed, 15 Apr 2026 10:38:36 +0000 Subject: [PATCH 3/4] Add tests for extractOIDCConfigs() to prevent legacy key regression Table-driven tests verify that extractOIDCConfigs() only reads from the "config" key in the ConfigMap, and that the legacy "authentication" key is no longer used. Covers valid parsing, localhost skipping, missing keys, invalid YAML, and missing ConfigMap scenarios. Co-Authored-By: Claude Opus 4.6 --- controller/cmd/main_test.go | 283 ++++++++++++++++++++++++++++++++++++ 1 file changed, 283 insertions(+) create mode 100644 controller/cmd/main_test.go diff --git a/controller/cmd/main_test.go b/controller/cmd/main_test.go new file mode 100644 index 000000000..95e8fa5a4 --- /dev/null +++ b/controller/cmd/main_test.go @@ -0,0 +1,283 @@ +/* +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: "legacy authentication key only should NOT produce OIDC configs", + configmap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jumpstarter-controller", + Namespace: "default", + }, + Data: map[string]string{ + "authentication": ` +jwt: + - issuer: + url: "https://dex.example.com" + audiences: + - "jumpstarter" + claimMappings: + username: + claim: "email" + prefix: "" +`, + }, + }, + wantNil: true, + }, + { + 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) + } +} From a55365a00d517e7037e1f948d4bff3f0c2045108 Mon Sep 17 00:00:00 2001 From: Ambient Code Bot Date: Wed, 15 Apr 2026 20:34:43 +0000 Subject: [PATCH 4/4] Remove duplicate legacy key test case from table-driven tests The table-driven case "legacy authentication key only should NOT produce OIDC configs" duplicated the dedicated TestExtractOIDCConfigsIgnoresLegacyKey function. Keep the dedicated function for clearer regression intent. Co-Authored-By: Claude Opus 4.6 --- controller/cmd/main_test.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/controller/cmd/main_test.go b/controller/cmd/main_test.go index 95e8fa5a4..cbe9abdd4 100644 --- a/controller/cmd/main_test.go +++ b/controller/cmd/main_test.go @@ -100,29 +100,6 @@ authentication: }, wantCount: 1, // localhost issuer should be skipped }, - { - name: "legacy authentication key only should NOT produce OIDC configs", - configmap: &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "jumpstarter-controller", - Namespace: "default", - }, - Data: map[string]string{ - "authentication": ` -jwt: - - issuer: - url: "https://dex.example.com" - audiences: - - "jumpstarter" - claimMappings: - username: - claim: "email" - prefix: "" -`, - }, - }, - wantNil: true, - }, { name: "missing config key returns nil", configmap: &corev1.ConfigMap{