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

Allow most printable ASCII chars for object label key (b-c) #13

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kb2ma
Copy link
Contributor

@kb2ma kb2ma commented Sep 20, 2022

Ensure all dependents of this package work before merging this PR!

Extends acceptable label characters to printable ASCII characters except space, single/double quotes and backtick. Updates minimal acceptable version of @types/dockerode to ensure build succeeds.

As of 2023-03-22 this PR still is blocked on an overall system solution between clients and server. This PR is useful; however older clients likely will fail in some way if they receive a service containing the extra characters.

Fixes #12

@kb2ma kb2ma added the versionbot/pr-draft Draft PR - Don't merge this PR automatically label Sep 20, 2022
@kb2ma kb2ma requested review from dfunckt and Page- September 20, 2022 16:10
@dfunckt
Copy link
Collaborator

dfunckt commented Sep 21, 2022

We need to make sure the Supervisor never applies labels to containers/images because I think Docker will error.

Also, please do not merge before the rest of product is prepared to handle this change.

@kb2ma
Copy link
Contributor Author

kb2ma commented Sep 21, 2022

We need to make sure the Supervisor never applies labels to containers/images because I think Docker will error.

I created a PR on the Supervisor to accept a Target State with the expanded character set. I'll verify it does not apply a label itself.

@kb2ma
Copy link
Contributor Author

kb2ma commented Sep 21, 2022

Also, please do not merge before the rest of product is prepared to handle this change.

What preparation is required for balena-builder? balena-cli seems to work OK with the updated character set. IIRC I can balena container inspect a container with expanded characters.

@dfunckt
Copy link
Collaborator

dfunckt commented Sep 22, 2022

I mean please do not merge this PR before the Supervisor one is merged and deployed. Also, it’s typically better to introduce changes to the builder first and then the CLI.

@kb2ma kb2ma marked this pull request as draft September 22, 2022 11:12
Change-type: patch
Signed-off-by: Ken Bannister <[email protected]>
Change-type: patch
Signed-off-by: Ken Bannister <[email protected]>
@kb2ma kb2ma changed the title Allow most printable ASCII chars for object label key Allow most printable ASCII chars for object label key (b-c) Oct 22, 2022
karaxuna added a commit to karaxuna/balena-compose that referenced this pull request Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
versionbot/pr-draft Draft PR - Don't merge this PR automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support a wider range of object label characters
2 participants