Skip to content

Validate metric name while listing available metrics and getting metric values #95

@invidian

Description

@invidian

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind/bugCategorizes issue or PR as related to a bug.lifecycle/frozenIndicates that an issue or PR should not be auto-closed due to staleness.needs-triageIndicates an issue or PR lacks a `triage/foo` label and requires one.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions