Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support k8s oidc auth #429

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Support k8s oidc auth #429

merged 1 commit into from
Nov 19, 2024

Conversation

Ugzuzg
Copy link
Contributor

@Ugzuzg Ugzuzg commented Nov 14, 2024

Description

Fixes #412

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make test) and all tests pass.
  • I have run relevant integration test locally (make integration-xxx for affected packages)
  • I have run relevant e2e test locally (make e2e-xxx for disruptors, or cluster related changes)
  • Any dependent changes have been merged and published in downstream modules

@Ugzuzg Ugzuzg requested a review from a team as a code owner November 14, 2024 19:40
@Ugzuzg Ugzuzg requested review from szkiba and removed request for a team November 14, 2024 19:40
@CLAassistant
Copy link

CLAassistant commented Nov 14, 2024

CLA assistant check
All committers have signed the CLA.

@szkiba szkiba requested review from pablochacin and removed request for szkiba November 18, 2024 09:27
@pablochacin
Copy link
Collaborator

Hi @Ugzuzg thanks for submitting this contribution. As I mentioned in the issue you opened (#429) my main concern is about how to test this change. As I can see from the checklist you haven't tested it yourself. I ran the integration tests and haven't noticed any regression, but I can't tell if it will actually solve the issue.

I suppose you have an environment where you can test it. Is there a way you can test it without we releasing a new version?

@Ugzuzg
Copy link
Contributor Author

Ugzuzg commented Nov 18, 2024

I have tested it locally and was able to send disruptions using OIDC auth config. I haven't checked any of the testing checkboxes, because all of them seem to be concerned with running automated tests, and don't apply to the manual test that I have done.

Copy link
Collaborator

@pablochacin pablochacin left a comment

Choose a reason for hiding this comment

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

LGTM

@pablochacin
Copy link
Collaborator

@Ugzuzg the linter is failing due to this error. Can you please add a comment to this line explaining why this is needed? Something like // initialize auth plugins

  Error: pkg/utils/kubernetes.go:9:2: blank-imports: a blank import should be only in a main or test package, or have a comment justifying it (revive)
  	_ "k8s.io/client-go/plugin/pkg/client/auth"
  	^

@Ugzuzg
Copy link
Contributor Author

Ugzuzg commented Nov 19, 2024

@pablochacin, now linter is passing locally.

@pablochacin pablochacin merged commit 37925a5 into grafana:main Nov 19, 2024
8 checks passed
@Ugzuzg Ugzuzg deleted the k8s-auth branch November 19, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no Auth Provider found for name "oidc"
3 participants