-
Notifications
You must be signed in to change notification settings - Fork 179
Description
Problem
Right now, it is possible for custom/external metrics adapter implemented using this library to expose and list metrics with names, which do not match Kubernetes resource naming convention, being as example:
The Pod "Foo" is invalid: metadata.name: Invalid value: "Foo": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
Listing and getting metrics using uppercase characters work fine when you use kubectl get --raw
, however, it does not work when one requests the metric e.g. using HPA object. Attempt to do so, results in the following error on HPA object:
invalid metrics (1 invalid out of 1), first error is: failed to get external metric E2E: unable to get external metric newrelic-metrics-adapter-e2e-tests-5rl58/E2E/nil: unable to fetch metrics from external metrics API: getting fresh external metric value: getting metric value: metric "e2e" not configured
This is because either a client or API server downcases the requested resource name, so requesting metric named E2E
results in client sending request for resource e2e
.
Solution
I think values received from methods in ExternalMetricsProvider should be sanitized before sending this data back to the client.
Notes
If needed, this can be implemented as an opt-in feature, to avoid breaking changes.
kubernetes/kubernetes#72996 also talks about it. Perhaps other characters like /
should also be not allowed. However, I'm not able to find a full list of forbidden characters.
EDIT:
It seems to me that ideally https://pkg.go.dev/k8s.io/apimachinery@v0.22.2/pkg/api/validation/path#IsValidPathSegmentName should be used.