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

WIP: Support OIDC Client Authentication #793

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dmolik
Copy link

@dmolik dmolik commented Feb 21, 2024

Globally support OIDC Client Authentication, this is for output sinks proxied by something like OAuth2-Proxy

  • attempt to support .well-known/oidc-configuration

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area config

/area outputs

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #712

Special notes for your reviewer:

Globally support OIDC Client Authentication, this is for output
sinks proxied by something like OAuth2-Proxy

- attempt to support .well-known/oidc-configuration

Signed-off-by: Dan Molik <[email protected]>
@poiana
Copy link

poiana commented Feb 21, 2024

Welcome @dmolik! It looks like this is your first PR to falcosecurity/falcosidekick 🎉

@poiana poiana requested review from Issif and leogr February 21, 2024 13:17
@poiana poiana added the size/L label Feb 21, 2024
@dmolik
Copy link
Author

dmolik commented Feb 21, 2024

so this is just a first stab at OIDC output support, I'm looking for feedback on approach, and I know I will need to update tests and documentation

@Issif Issif changed the title Support OIDC Client Authentication WIP: Support OIDC Client Authentication Feb 21, 2024
@Issif Issif added this to the 2.29.0 milestone Feb 21, 2024
@@ -1,6 +1,8 @@
module github.com/falcosecurity/falcosidekick

go 1.20
go 1.21
Copy link
Member

@Issif Issif Feb 21, 2024

Choose a reason for hiding this comment

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

Can you avoid to bump up the Go version please, to avoid to break anything aside.

Copy link
Author

@dmolik dmolik Feb 21, 2024

Choose a reason for hiding this comment

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

this was pulled in from the kube 1.29 version bump

@Issif
Copy link
Member

Issif commented Feb 21, 2024

Interesting proposal, thnaks. I edited the title to specify it's a WIP, ping when you want a deeper review. Thanks. I enabled the CI btw.

@poiana
Copy link

poiana commented Feb 21, 2024

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Instructions 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/test-infra repository.

@poiana
Copy link

poiana commented Mar 11, 2024

LGTM label has been added.

Git tree hash: e3c4fa41292b36ea2a39a75d580d3986d6e4e9e6

@Issif
Copy link
Member

Issif commented Mar 12, 2024

/remove-approve

@poiana
Copy link

poiana commented Mar 12, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dmolik
Once this PR has been reviewed and has the lgtm label, please ask for approval from issif. For more information see the Kubernetes Code Review Process.

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

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

@Issif
Copy link
Member

Issif commented Mar 12, 2024

(sorry, a miss click in the GH app on phone made me approved this PR)

@dmolik
Copy link
Author

dmolik commented Mar 12, 2024

okay no worries, I've got to rebase this against master, and add config and docs

@Issif
Copy link
Member

Issif commented Apr 23, 2024

What's the status on your side for this PR? Is it ready for a review?

@dmolik
Copy link
Author

dmolik commented Apr 23, 2024

I'm going to merge main again, and test locally via minikube, I'll let you know in a day or two.

@Issif
Copy link
Member

Issif commented Jun 11, 2024

Hi,

are you still working on that PR?

@Issif Issif modified the milestones: 2.29.0, 2.30 Jun 24, 2024
@poiana
Copy link

poiana commented Sep 22, 2024

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

@dmolik
Copy link
Author

dmolik commented Sep 22, 2024

/remove-lifecycle stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

OIDC Authentication for Alertmanager and Loki
3 participants