Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Valid Semver 2.0 not compatible with kubernetes labels #115055

Open
itay-grudev opened this issue Jan 13, 2023 · 18 comments
Open

Valid Semver 2.0 not compatible with kubernetes labels #115055

itay-grudev opened this issue Jan 13, 2023 · 18 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.

Comments

@itay-grudev
Copy link

itay-grudev commented Jan 13, 2023

What happened?

Semver is allowed to have the following syntax: 1.2.3+45, where 45 is the build number. We are using this versioning approach, and when we tried to put that into the app.kubernetes.io/version label we got the following error:

a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')

I think this is a slight oversight and I think the regex there should be extended to:

(([A-Za-z0-9][-A-Za-z0-9_+.]*)?[A-Za-z0-9])?
                         ^ plus sign added here

From what I understand adding support for a + character doesn't have any additional implications and should be safe.

What did you expect to happen?

I expected that a valid Semver is accepted as a label value.

How can we reproduce it (as minimally and precisely as possible)?

Add the following K8s label to any object:

metadata:
  labels:
    app.kubernetes.io/version: 1.2.3+45

Example:

apiVersion: v1
kind: ServiceAccount
metadata:
  name: test
  labels:
    app.kubernetes.io/version: 1.2.3+45

Kubernetes version

Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.0", GitCommit:"b46a3f887ca979b1a5d14fd39cb1af43e7e5d12d", GitTreeState:"clean", BuildDate:"2022-12-09T16:23:44Z", GoVersion:"go1.19.4", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"24+", GitVersion:"v1.24.8-eks-ffeb93d", GitCommit:"abb98ec0631dfe573ec5eae40dc48fd8f2017424", GitTreeState:"clean", BuildDate:"2022-11-29T18:45:03Z", GoVersion:"go1.18.8", Compiler:"gc", Platform:"linux/amd64"}

Cloud provider

AWS EKS

OS version

Bottlerocket OS 1.11.1 (aws-k8s-1.24)
@itay-grudev itay-grudev added the kind/bug Categorizes issue or PR as related to a bug. label Jan 13, 2023
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 13, 2023
@k8s-ci-robot
Copy link
Contributor

@itay-grudev: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

@neolit123
Copy link
Member

/sig architecture api-machinery

@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 15, 2023
@sftim
Copy link
Contributor

sftim commented Jan 16, 2023

What version of SemVer are you using @itay-grudev?

@sftim
Copy link
Contributor

sftim commented Jan 16, 2023

If we relax the restriction on labels (see https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set), we might need to update lots of software in the ecosystem with the new definition of what characters are valid.

@sftim
Copy link
Contributor

sftim commented Jan 16, 2023

We could also perhaps define a new annotation (not label) such as app.kubernetes.io/semantic-version-1.0 and app.kubernetes.io/semantic-version-2.0, and encourage people to use these.

@itay-grudev
Copy link
Author

itay-grudev commented Jan 16, 2023

What version of SemVer are you using @itay-grudev?

@sftim, AFAIK the +build_number was introduced in Semver 2.0, which is the version we are following.

@liggitt liggitt added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/bug Categorizes issue or PR as related to a bug. labels Jan 18, 2023
@liggitt
Copy link
Member

liggitt commented Jan 18, 2023

I think the odds of expanding the label value character set are low. @sftim's comment about there being many distributed components / software that would not understand/expect the expanded characters is accurate. As a simple example, label values read from the API today do not contain any characters that require URL-encoding, so a labelSelector query could be constructed directly using a validated label value. With the addition of a + character, that would no longer be true.

@liggitt liggitt added the priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. label Jan 18, 2023
@sftim
Copy link
Contributor

sftim commented Jan 18, 2023

What do people think about the annotation idea?

@cici37
Copy link
Contributor

cici37 commented Jan 19, 2023

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 19, 2023
@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 19, 2023
@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 19, 2023
@itay-grudev
Copy link
Author

@sftim I think using annotations instead is a good solution.

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label May 19, 2023
@na-jakobs
Copy link

@itay-grudev @sftim This is needed to support the SemVer standard. Any news on this?

@sftim
Copy link
Contributor

sftim commented Dec 12, 2023

/uncc

I'm not tracking this work.

@na-jakobs
Copy link

na-jakobs commented Dec 12, 2023

Can the regex be updated to support the industry standard https://semver.org/spec/v2.0.0.html ?

Error: unable to upgrade <app>: pre-upgrade hooks failed: warning: Hook pre-upgrade
deployment/templates/imagepull-secrets.yaml failed: 1 error occurred:
	* Secret "ghcr.io" is invalid: [metadata.labels: Invalid value: "5.0.1-RC.1+GCP": a valid label
	must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and
	end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for
	validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?'), metadata.labels: Invalid
	value: "deployment-5.0.1-RC.1+GCP": a valid label must be an empty string or consist of alphanumeric
	characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', 
	or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')]

edit: the new annotation would be a nice way to implement this

@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 11, 2024
@itay-grudev
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 12, 2024
@k8s-triage-robot
Copy link

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-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.
Projects
None yet
Development

No branches or pull requests

8 participants