Skip to content

feat: allow OIDC cookies when in-cluster is false via flag#4487

Open
beep-boopp wants to merge 1 commit intokubernetes-sigs:mainfrom
beep-boopp:main
Open

feat: allow OIDC cookies when in-cluster is false via flag#4487
beep-boopp wants to merge 1 commit intokubernetes-sigs:mainfrom
beep-boopp:main

Conversation

@beep-boopp
Copy link

@beep-boopp beep-boopp commented Jan 29, 2026

Summary

Adds the --oidc-use-cookie flag to allow OIDC authentication configuration when Headlamp is running in a local or development environment (non–in-cluster mode).

Related Issue

Fixes #4481

Changes

  • pkg/config/config.go

    • Added the OidcUseCookie flag.
    • Updated Validate() logic to permit OIDC settings when this flag is enabled.
  • pkg/config/config_test.go

    • Added test cases to TestParseBasic to verify the success path.
    • Added test cases to TestParseErrors to ensure proper error messaging when the flag is missing.
  • cmd/server.go

    • Wired OidcUseCookie from the config parser into the internal server configuration.
  • Error messaging

    • Improved validation errors to clearly indicate the requirement for either in-cluster mode or the --oidc-use-cookie flag.

Steps to Test

  1. Build the backend:
    go build ./cmd/headlamp

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 29, 2026
@k8s-ci-robot k8s-ci-robot requested a review from ashu8912 January 29, 2026 17:41
@k8s-ci-robot
Copy link
Contributor

Welcome @beep-boopp!

It looks like this is your first PR to kubernetes-sigs/headlamp 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/headlamp has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot requested a review from sniok January 29, 2026 17:41
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 29, 2026
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 7, 2026
@illume illume requested a review from Copilot February 8, 2026 13:31
Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please have a look at fixing the merge conflict?

We use linux kernel style git commit messages. Please see the contributing guide and the section in there about git commit messages.

Also, please remove Signed-off-by, as that is not needed in this repo. Thanks!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for enabling OIDC token cookies when Headlamp is not running in in-cluster mode, via a new --oidc-use-cookie config/flag, intended to unblock local/dev usage with OIDC-based kubeconfigs.

Changes:

  • Add oidc-use-cookie flag/config field and relax OIDC validation rules when it is enabled.
  • Extend internal HeadlampCFG to carry the new setting.
  • Allow Helm release routes to pull auth tokens from cookies when in-cluster or oidc-use-cookie is enabled.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
backend/pkg/headlampconfig/headlampConfig.go Adds OidcUseCookie to internal backend config struct.
backend/pkg/config/config.go Adds OidcUseCookie config + flag; updates Validate() gating logic and error message.
backend/pkg/config/config_test.go Updates expected validation error substring for non-in-cluster OIDC flags.
backend/cmd/headlamp.go Enables cookie-token behavior for Helm release handler when in-cluster or flag-enabled; initializes oidcUseCookie from HeadlampCFG.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@beep-boopp
Copy link
Author

Can you please have a look at fixing the merge conflict?

We use linux kernel style git commit messages. Please see the contributing guide and the section in there about git commit messages.

Also, please remove Signed-off-by, as that is not needed in this repo. Thanks!

Thanks for the review. I'll go over the requested changes, fix the merge conflicts, and update the commit message style (including removing the sign-off).

@k8s-ci-robot k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 8, 2026
@beep-boopp beep-boopp force-pushed the main branch 2 times, most recently from 32fbbc4 to dd72d6a Compare February 8, 2026 15:13
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Feb 8, 2026
@beep-boopp
Copy link
Author

Hey! I've updated the PR to address the feedback. I've switched the commit message to the requested style , cleaned up that multiline error string, and added a success case to the unit tests as suggested. If you spot any other adjustments needed, just let me know.

@beep-boopp
Copy link
Author

Hey, sorry for the extra push, the linter caught a formatting issue and a long line I missed. All fixed now

@mudit06mah
Copy link
Contributor

@beep-boopp Please address the review from copilot

@beep-boopp
Copy link
Author

@beep-boopp Please address the review from copilot

All good, I've already pushed the fixes and addressed those Copilot points. Resolving the threads now.

@mudit06mah
Copy link
Contributor

All good, I've already pushed the fixes and addressed those Copilot points. Resolving the threads now.

Why has it not reflected in your Files Changed then?
image

Please look at other suggestions as well

@beep-boopp
Copy link
Author

All good, I've already pushed the fixes and addressed those Copilot points. Resolving the threads now.

Why has it not reflected in your Files Changed then? image

Please look at other suggestions as well

@mudit06mah The fix is already in server.go (as shown in my latest commit). I manually mapped conf.OidcUseCookie to the internal config struct, which is why GitHub says the suggestion can't be applied ,the code has already been updated to handle the wiring. I have made all the changes requested

@mudit06mah
Copy link
Contributor

@beep-boopp You have indeed made the wiring changes, it was that copilot messages did not have the outdated tag that got me confused.
Your changes LGTM,
Please change your commit message to something like:

backend: cmd: pkg: Allow OIDC cookies when in-cluster is false via flag

(First letter of commit message is capitalized)
Also change your PR Title to match your commit message and update the PR body for the new changes as well :)

@beep-boopp
Copy link
Author

@beep-boopp You have indeed made the wiring changes, it was that copilot messages did not have the outdated tag that got me confused. Your changes LGTM, Please change your commit message to something like:

backend: cmd: pkg: Allow OIDC cookies when in-cluster is false via flag

(First letter of commit message is capitalized) Also change your PR Title to match your commit message and update the PR body for the new changes as well :)

@mudit06mah No worries, I've updated the commit message to follow the repo's area-prefix style and refreshed the PR body with the full summary and test steps. GitHub isn't letting me edit the PR title itself (probably a permission thing), so that's all set on my end.

@beep-boopp beep-boopp requested a review from illume February 9, 2026 09:49
@illume illume requested a review from Copilot February 9, 2026 12:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Can you please see the open review notes?

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 9, 2026
@beep-boopp
Copy link
Author

Thanks.

Can you please see the open review notes?

Thanks for the review!
I’ve removed the redundant OidcUseCookie assignment , good catch.
For test coverage, I added TestOidcUseCookieLogic to specifically verify the new behavior (promoting the token from the auth cookie to the Authorization header when the flag is enabled). I chose a focused unit test to validate the logic directly, rather than adding a handler level test that would require substantial mocking of the store, Kubernetes client, and router.
If you feel coverage at the handler level is necessary for this change, I’m happy to add it.

@beep-boopp beep-boopp requested a review from illume February 10, 2026 15:56
@mudit06mah
Copy link
Contributor

@beep-boopp Could you also add this flag to values.yaml and deployment.yaml for the helm charts? Update the documentation as well.

@beep-boopp
Copy link
Author

@mudit06mah Thanks for the review! I've updated the PR to include the full Helm chart configuration (values.yaml, deployment.yaml) and the documentation in README.md to ensure this feature is deployable out-of-the-box.
@illume The PR is now feature-complete with backend logic, unit tests, and full Helm support. Ready for your final look.

Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing those changes.

There’s a github check failing.

Please run the linter locally to see:
npm run backend:lint

@beep-boopp beep-boopp force-pushed the main branch 2 times, most recently from 0939b55 to d062f7d Compare February 11, 2026 14:55
@beep-boopp
Copy link
Author

@illume All checks are passing now. Ready for your review. Thanks for your patience!

Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 thanks!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: beep-boopp, illume

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 12, 2026
@illume
Copy link
Contributor

illume commented Feb 12, 2026

@beep-boopp I’ll wait a little while before merging this to give folks a chance to have a look and provide feedback. Are you able to ask in the headlamp channel on the kubernetes slack for someone to review?

I asked the original issue reporter for feedback, maybe they will have time to respond too.

thanks again!!

@beep-boopp
Copy link
Author

@beep-boopp I’ll wait a little while before merging this to give folks a chance to have a look and provide feedback. Are you able to ask in the headlamp channel on the kubernetes slack for someone to review?

I asked the original issue reporter for feedback, maybe they will have time to respond too.

thanks again!!

Thanks for the approval!
I just posted a note in the #headlamp channel on Kubernetes Slack to ask for community feedback. I'll keep an eye out for any comments there

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 13, 2026
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 15, 2026
@beep-boopp
Copy link
Author

@illume Just a heads up, I’ve rebased onto the latest main to resolve the conflicts caused by the recent backend refactor .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow OIDC authentication for kubeconfig clusters when inCluster mode is false

4 participants