Add support for service account auth for incluster deployments#4552
Add support for service account auth for incluster deployments#4552tylergmuir wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
|
|
|
Welcome @tylergmuir! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tylergmuir The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
This is also a potential option to address #1801. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
illume
left a comment
There was a problem hiding this comment.
Thanks for this.
Can you please check the failing github checks?
For the github message, we do a Linux kernel style. It’s documented in the contributing guide on the website, or look at other git log messages in the backend folder.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “use service account token” authentication mode for in-cluster deployments, enabling setups where an external OIDC proxy handles end-user auth while Headlamp uses a Kubernetes service account token to talk to the API server.
Changes:
- Introduces backend flags/config for
--use-service-account-tokenand--service-account-token-path, and wires them into in-cluster context creation. - Updates Helm chart values + deployment args to pass the new flags, with added chart render test cases/fixtures.
- Adds a backend unit test for the new flags.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/headlamp/values.yaml | Adds chart values for enabling SA-token auth and optional token path. |
| charts/headlamp/templates/deployment.yaml | Emits the new CLI args into the Deployment manifest. |
| charts/headlamp/tests/test_cases/service-account-token*.yaml | Adds Helm template test inputs for the new values. |
| charts/headlamp/tests/expected_templates/service-account-token*.yaml | Adds expected rendered outputs for the new Helm test cases. |
| backend/pkg/config/config.go | Adds config fields/constants + CLI flags for SA-token auth. |
| backend/pkg/config/config_test.go | Adds flag parsing test coverage for the new flags. |
| backend/pkg/headlampconfig/headlampConfig.go | Extends HeadlampCFG with SA-token settings. |
| backend/cmd/server.go | Maps parsed config into HeadlampCFG including new SA-token fields. |
| backend/cmd/headlamp.go | Passes SA-token options into in-cluster context creation (and logs a warning). |
| backend/pkg/kubeconfig/kubeconfig.go | Sets in-cluster kubeconfig AuthInfo.TokenFile when opt-in is enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- if .Values.config.useServiceAccountToken }} | ||
| - "-use-service-account-token" | ||
| {{- end }} | ||
| {{- if and .Values.config.useServiceAccountToken .Values.config.serviceAccountTokenPath }} |
There was a problem hiding this comment.
The Helm template adds -use-service-account-token (and the token path flag) even if .Values.config.inCluster is false. That will cause the backend to fail config validation/startup because --use-service-account-token is only valid in in-cluster mode. Gate these args behind and .Values.config.inCluster .Values.config.useServiceAccountToken (and similarly for the path).
| {{- if .Values.config.useServiceAccountToken }} | |
| - "-use-service-account-token" | |
| {{- end }} | |
| {{- if and .Values.config.useServiceAccountToken .Values.config.serviceAccountTokenPath }} | |
| {{- if and .Values.config.inCluster .Values.config.useServiceAccountToken }} | |
| - "-use-service-account-token" | |
| {{- end }} | |
| {{- if and .Values.config.inCluster .Values.config.useServiceAccountToken .Values.config.serviceAccountTokenPath }} |
| { | ||
| name: "use_service_account_token_flag", | ||
| args: []string{"go run ./cmd", "--use-service-account-token", "--service-account-token-path=/custom/token/path"}, | ||
| verify: func(t *testing.T, conf *config.Config) { | ||
| assert.Equal(t, true, conf.UseServiceAccountToken) | ||
| assert.Equal(t, "/custom/token/path", conf.ServiceAccountTokenPath) | ||
| }, |
There was a problem hiding this comment.
This test enables --use-service-account-token without also setting --in-cluster. config.Parse() runs Validate() and should return an error for this combination, so this test will fail. Add --in-cluster to the args (or update the validation expectations if the intent is to support it out of cluster).
| f.String("proxy-urls", "", "Allow proxy requests to specified URLs") | ||
| f.Bool("enable-helm", false, "Enable Helm operations") | ||
| f.Bool("use-service-account-token", false, "Use the service account token for in-cluster authentication") | ||
| f.String("service-account-token-path", DefaultServiceAccountTokenPath, "Path to the service account token") |
There was a problem hiding this comment.
--service-account-token-path is given a non-empty default value. This makes it hard to distinguish "flag not set" from "set", and it also prevents mode validation similar to the OIDC flags (because the value is always non-empty). Consider defaulting this flag to an empty string and applying the default path only when --use-service-account-token is true (the kubeconfig code already has this fallback).
| f.String("service-account-token-path", DefaultServiceAccountTokenPath, "Path to the service account token") | |
| f.String("service-account-token-path", "", "Path to the service account token") |
| inClusterAuthInfo := &api.AuthInfo{} | ||
| if useServiceAccountToken { | ||
| if serviceAccountTokenPath == "" { | ||
| serviceAccountTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token" |
There was a problem hiding this comment.
The default token path is hard-coded here. Since you already have clusterConfig from rest.InClusterConfig(), consider using clusterConfig.BearerTokenFile as the default when serviceAccountTokenPath is empty. That keeps the behavior aligned with client-go defaults if they ever change and avoids duplicating the path constant.
| serviceAccountTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token" | |
| if clusterConfig.BearerTokenFile != "" { | |
| serviceAccountTokenPath = clusterConfig.BearerTokenFile | |
| } else { | |
| serviceAccountTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token" | |
| } |
| useServiceAccountToken: false | ||
| # -- path to the service account token file | ||
| # if useServiceAccountToken is true, this path will be used to read the service account token | ||
| # if not set, the default path will be used: /var/run/secrets/kubernetes.io/serviceaccount/token |
There was a problem hiding this comment.
Trailing whitespace at the end of the comment line (after .../serviceaccount/token). Please remove it to avoid noisy diffs/lint issues.
| # if not set, the default path will be used: /var/run/secrets/kubernetes.io/serviceaccount/token | |
| # if not set, the default path will be used: /var/run/secrets/kubernetes.io/serviceaccount/token |
The backend now supports two flags that can be used to make the incluster deployment authenticated by default, this allows the users to use OIDC proxies in front of headlamp without having to deal with service account token. For more details refer the issue
Fixes: #3606
This is a re-attempt to implement the abandoned PR in #3607.