custom-metrics-apiserver icon indicating copy to clipboard operation
custom-metrics-apiserver copied to clipboard

Published 20 hours ago • verified publisher icon kubernetes-sigs

Validate metric name while listing available metrics and getting metric values

Open invidian opened this issue 4 years ago • 9 comments
trafficstars

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.

https://github.com/kubernetes/kubernetes/issues/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/[email protected]/pkg/api/validation/path#IsValidPathSegmentName should be used.

invidian avatar Sep 30 '21 14:09 invidian

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

k8s-triage-robot avatar Dec 29 '21 15:12 k8s-triage-robot

/remove-lifecycle stale

invidian avatar Dec 29 '21 16:12 invidian

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

k8s-triage-robot avatar Mar 29 '22 16:03 k8s-triage-robot

/remove-lifecycle stale

invidian avatar Mar 29 '22 17:03 invidian

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

k8s-triage-robot avatar Jun 27 '22 17:06 k8s-triage-robot

/remove-lifecycle stale

invidian avatar Jun 27 '22 18:06 invidian

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

This bot triages issues and PRs 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 or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

k8s-triage-robot avatar Sep 25 '22 19:09 k8s-triage-robot

/remove-lifecycle stale

invidian avatar Sep 27 '22 10:09 invidian

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

k8s-triage-robot avatar Jan 19 '24 09:01 k8s-triage-robot