search

tektoncd / cli

A CLI for interacting with Tekton!
Apache License 2.0
431 stars 250 forks source link

the "annotate" flag does not parse the annotation value correctly if it contains comma #2402

Open psturc opened 2 months ago

psturc commented 2 months ago

Versions and Operating System

Expected Behavior

when using --annotate parameter with tkn bundle push command, the annotation's value is parsed correctly if it contains comma: ,

Actual Behavior

it fails with an error: Error: invalid input format for param parameter: <the rest of the annotation value string after comma>

Steps to Reproduce the Problem

Run

➜  ~ tkn bundle push --annotate 'test=a,b' quay.io/my-quay-username/my-repo:test-tag -f some-task.yaml
*Warning*: This is an experimental command, it's usage and behavior can change in the next release(s)
Error: invalid input format for param parameter: b

Additional Info

the --annotate parameter was introduced in this commit

chmeliik commented 2 months ago

Some more investigation from a slack channel:

Ha, I found out why it does that. It uses pflag.StringSlice to parse the --annotate argument, which is very clever and, in this case, very inconvenient

It means you can pass two annotations at the same time like --annotate 'a=b,x=y' But you can't really pass an annotation that contains a comma :confused:

can the comma be escaped?

Kind of - turns out StringSlice parses the argument as a CSV file using https://pkg.go.dev/encoding/csv, so you can in fact quote them tkn bundle push --annotate '"foo=one, two",bar=three'

Then double-quotes would cause issues instead, but those can be escaped by using double double-quotes tkn bundle push --annotate '"foo=""Hello there!"", said Kenobi."'

TLDR - it is possible to use the flag as is, but really hard. Parsing the --annotate (and also --labels) values with StringArray, which doesn't do this CSV splitting, would be much easier to deal with.

But would also be a breaking change for anyone who's relying on the StringSlice behavior