Webhook authentication option

[sboardwell]

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

What?

• add the ability to pass webhook authentication parameters. e.g.
  • with WEBHOOK_USERNAME and WEBHOOK_PASSWORD[_FILE] environment variables
  • making use of NETRC with something like https://go.dev/src/cmd/go/internal/auth/netrc.go

Why?

• some endpoints are not open by default
• adding the username and credentials in the URL á la https://user:[email protected] is ugly

[thockin]

We have a dash --username flag already and ask pass URL support. Does that not do what you need? If not, can yo be more specific about what you need?

I am not familiar with WEBHOOK_* variables but that doesn't mean they're not a thing. I just need to learn about them. In general, I am happy to support other auth modes. I just need to know what they are and what they're for and how to test them.

If you could help me craft an E2E test, that would be an awesome way to start.

Message ID: @.***>

[sboardwell]

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 :smile: - 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]

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 https://github.com/kubernetes/git-sync/blob/1894192b0f68641a24835cb3d23a01d7bcb98cc7/test_e2e.sh#L1978

[thockin]

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]

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...

• although envFrom: {} exists, I do not generally like to have credentials as environment variables
• using the --password-file allows pointing to a file
  • in kubernetes, this would involved mounting a secret
• if we are mounting anyway, why not have:
  • (less work) a single flag --config-file pointing to a file containing the environment variables GIT_SYNC_USERNAME=my-user á la https://kubernetes.io/docs/tasks/configmap-secret/managing-secret-using-config-file/#specify-unencoded-data-when-creating-a-secret
  • (more work) point to a directory --config-dir where all the configuration values are files /secret/mount/GIT_SYNC_USERNAME containing the username, etc (kubernetes allows key mapping if necessary with https://kubernetes.io/docs/tasks/inject-data-application/distribute-credentials-secure/#project-secret-keys-to-specific-file-paths)

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
  • https://kubernetes.io/docs/concepts/configuration/configmap/#mounted-configmaps-are-updated-automatically
  • imagine being able to change the log level temporarily on a live instance
  • this would need some config reload logic in the app, but still an exciting prospect IMO

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

[thockin]

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]

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

[heliochronix]

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]

I am open to ideas, they just need someone to flesh them out.

On Thu, Jul 6, 2023, 8:50 PM Miles Simpson @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/kubernetes/git-sync/issues/752#issuecomment-1624636056, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKWAVHVHQNVLALGOHPWSATXO6BPFANCNFSM6AAAAAAYT6PGZE . You are receiving this because you commented.Message ID: @.***>

[k8s-triage-robot]

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-triage-robot]

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-triage-robot]

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]

@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-triage-robot]

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-triage-robot]

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

[sboardwell]

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