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

feat: allow commas in --image-label but not multiple labels #1190

Closed
wants to merge 1 commit into from

Conversation

LouisBrunner
Copy link

Resolves #622

Context

Currently, --image-label expects a list of key=value labels separated by commas. This means it is impossible to have labels whose value contain commas.

This is problematic for descriptions (as shown in #622) but also labels that mandate commas like moby.buildkit.frontend.caps (e.g. moby.buildkit.frontend.caps=moby.buildkit.frontend.inputs,moby.buildkit.frontend.contexts).

Those commas also can't be escaped to make pflag keep the values as one label.

Fix

TL;DR: this change might be considered a breaking change due to CLI flags not doing the exact same thing as before.

The issue can be solved easily by replacing StringSliceVar with StringArrayVar. However, it does mean --image-label foo=bar,baz=qux will stay as ["foo=bar,baz=qux"] instead of the current ["foo=bar", "baz=qux"] which might be unexpected for current users of --image-label.

From the name (--image-label) I did expect it to take only a single value instead of a comma-separated one but the description (Which labels (key=value) to add to the image.) seems to say the opposite.

Alternative

Trying to support both options in one flag (--image-label) seems difficult and quite error-prone so might it be possible to consider adding a new option (e.g. --image-add-label, --image-single-label, etc) which can support label with commas?

Copy link

github-actions bot commented Mar 4, 2024

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant