hetznercloud / hcloud-cloud-controller-manager

Kubernetes cloud-controller-manager for Hetzner Cloud
Apache License 2.0
743 stars 118 forks source link

feat: allow arbitrary length API tokens #752

Open kamushadenes opened 1 month ago

kamushadenes commented 1 month ago

Context

We have developed a soon-to-be-open-source proxy that forces specific labels in order to provide scoped API access, and that doesn't expose the real API token. This was created to have better control of resources inside the same project (as API tokens currently lack granularity), and to be able to use a single project securely, given that it isn't possible to create a project via the API.

One of it's operating modes is using JWT as a virtual self-validating token, which can't have a fixed size.

This support is required to make full use of it inside a Kubernetes cluster.

The feature is behind a default-false flag so it shouldn't interfere with current behavior.

Related

https://github.com/kubernetes/autoscaler/pull/7285 https://github.com/hetznercloud/csi-driver/pull/724

apricote commented 1 month ago

Hey @kamushadenes,

I will respond here, but the same applies to the other two PRs:

This sounds very interesting. I am not sure if a flag is necessary, or if we just want to the whole validation. I will talk to the team responsible for tokens next week and will report back afterwards.

apricote commented 1 month ago

Hey @kamushadenes,

we talked about this today internally. We would prefer not to add any additional flags or environment variables to disable the check.

We also do not think that the check is strictly necessary. Instead we would prefer to change the errors when len(token) != 64 to a warning message but allow the token. This would also help us in the future if we ever change the format of our API tokens.

Do you want to update your PRs to log warnings or should we do work on that?

kamushadenes commented 1 month ago

Hey @apricote, thanks for getting back!

Makes total sense, I'll update the PRs in a couple hours when my day starts.

kamushadenes commented 1 month ago

Done!

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.73%. Comparing base (6b132c1) to head (d989b20).

Files with missing lines Patch % Lines
internal/config/config.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #752 +/- ## ========================================== - Coverage 72.00% 71.73% -0.27% ========================================== Files 31 30 -1 Lines 2650 2473 -177 ========================================== - Hits 1908 1774 -134 + Misses 553 525 -28 + Partials 189 174 -15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.