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

Webhook authentication option #752

Open
sboardwell opened this issue May 30, 2023 · 16 comments
Open

Webhook authentication option #752

sboardwell opened this issue May 30, 2023 · 16 comments

Comments

@sboardwell
Copy link

sboardwell commented May 30, 2023

Hi, I may have missed something here but if not, do you think something like this would be possible?

What?

Why?

  • some endpoints are not open by default
  • adding the username and credentials in the URL á la https://user:token@acme.org is ugly
@thockin
Copy link
Member

thockin commented May 30, 2023 via email

@sboardwell
Copy link
Author

I am talking about the authentication when calling the webhook URL, not the git sync itself.

We have a dash --username flag already and ask pass URL support

But are they not used for git auth as opposed to webhook endpoint auth?

I am not familiar with WEBHOOK_* variables but that doesn't mean they're
not a thing

Oh no, they don't exist 😄 - it would have been analogous to the GIT_SYNC_USERNAME and GIT_SYNC_PASSWORD variables. The NETRC thing does exist though and is quite widely used.

Let me have a look at the current e2e tests and I'll get back to you.

@sboardwell
Copy link
Author

sboardwell commented May 30, 2023

I would guess we could add another container config under _test_tools similar to https://github.com/koolwithk/devops-tools/tree/main/docker/httpd-basic-auth and use that container to simulate an secured webhook endpoint. Calling without basic auth returns a 403, correct basic auth a 200.

The tests would be e2e::webhook_authenticated_success and e2e::webhook_authenticated_failure
with the template being

function e2e::webhook_success() {

@thockin
Copy link
Member

thockin commented May 30, 2023

I am talking about the authentication when calling the webhook URL, not the git sync itself.

Ahh! I see. I could buy adding --webhook-username and --webhook-password flags and/or netrc support. Mounting a volume is always a little more work than passing a flag, so I'd probably opt for flags first and netrc if someone really wants it.

The slightly harder part is getting that e2e set up that demands basic-auth.

Is this something you want to do a pull-request for? If not, I can ACK it as a feature request and I'll get to it when I have some time.

@sboardwell
Copy link
Author

sboardwell commented May 31, 2023

Great. I can have a look at creating a pull-request. Not sure if I'll have time this week but will definitely try to get it in sooner rather than later.

Could we have a --webhook-password-file as well if we are going down that route?


Slightly related, but before I create extra noise with a ticket. Would there be interest in discussing using kubernetes volume mounts for configuration in addition to environment variables and flags? Please tell if this is too far fetched but...

The advantages I see are:

  • the values can be kept in a single place (separation of concerns - config vs app)
  • the config file could be a secret, meaning it would be "slightly" more secure - one could choose which bits are shown as env vars, and which are read
  • mounts are updated automatically, potentially opening the door on-demand changes without restarting the pod

Anyway, let me know. I can put it in a separate ticket if there is interest.

@thockin
Copy link
Member

thockin commented May 31, 2023

Feel free to open a new issue to discuss the config-file topic. Seems plausible on it's face, but need to think thru the intersection and logging and such

@sboardwell
Copy link
Author

Still on this btw. Fell ill this week so not able to do anything as yet.

@heliochronix
Copy link

heliochronix commented Jul 7, 2023

Could there also be an option to set a "Authorization: Bearer <token>" header? For example, RESTful APIs seem to make use of such headers. As an example: https://developers.home-assistant.io/docs/api/rest/

@thockin
Copy link
Member

thockin commented Jul 7, 2023 via email

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 24, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 23, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

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.

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 24, 2024
@thockin thockin reopened this Mar 25, 2024
@thockin thockin removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Mar 25, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 23, 2024
@thockin thockin removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 23, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 21, 2024
@thockin thockin removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 21, 2024
@sboardwell
Copy link
Author

/remove-lifecycle stale - I still like this idea and will set time aside for implementation in the coming weeks.

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

No branches or pull requests

5 participants