ytt icon indicating copy to clipboard operation
ytt copied to clipboard

Ytt removes quotes for hex strings, which makes them ambiguous

Open Xander-Polishchuk opened this issue 2 years ago • 13 comments

What steps did you take:

echo 'hex-string: "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"' > template.yaml
ytt -f template.yaml

What happened:

quotes removed

What did you expect:

quotes preserved, as hex string format might contain specific encoding where string format required (e.g. upper/lower case letters provides checksum)

Environment:

  • ytt version (use ytt --version): 0.45.0
  • OS (e.g. from /etc/os-release): Fedora 36

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

Xander-Polishchuk avatar Mar 31 '23 09:03 Xander-Polishchuk

This issue is being marked as stale due to a long period of inactivity and will be closed in 5 days if there is no response.

github-actions[bot] avatar May 11 '23 00:05 github-actions[bot]

Hey any updates? Still an issue!

Xander-Polishchuk avatar May 11 '23 13:05 Xander-Polishchuk

@Xander-Polishchuk - Thanks for reporting this issue. Yes, we are able to replicate it.

This might be because ytt uses the yaml.v2 library to marshal the YAML. We had similar issue fixed in ytt v0.37.0 for forcing quotes for strings.

However, this specific length hex string is not getting output as expected. Here are some examples that I ran on the ytt playground - if a string has 18 or less number of characters, the quotes are retained. image

Also, we are exploring go yaml v3 parser to use in ytt for certain scenarios and that would probably resolve this issue because it preserves the original representation of octals, hexadecimals etc.

Meanwhile, please let us know if you are blocked on this or if any workaround worked for you. Thanks for reaching out.

vmunishwar avatar Jun 06 '23 00:06 vmunishwar

Hey @vmunishwar, thanks for a response.

The only workaround is fallback to decimal encoding, but such fallback removes ability to check checksum, therefore not the complete one and opens space for errors.

Xander-Polishchuk avatar Jun 12 '23 04:06 Xander-Polishchuk

So i was checking this issue and this won't be fixed even if we use yaml v3 in ytt. There are open issues for exact problem in yaml project here https://github.com/go-yaml/yaml/issues/435 and https://github.com/go-yaml/yaml/issues/703

prembhaskal avatar Jan 04 '24 07:01 prembhaskal

That's very regretful decision, I wish you could contribute the fix to the yaml library or drop it altogether in favor of predictable results. Main while we considering dropping ytt in our projects in favor of other templating engines due to lack of WYSIWYG before and after templating.

Xander-Polishchuk avatar Jan 04 '24 09:01 Xander-Polishchuk

@Xander-Polishchuk We will discuss this issue in our next community meeting to get input from experts.

  • I believe dropping yamlv3 for something else is definitely not an option right now (may be in future because yaml/v3 seems to be unmaintained)
  • Another option to overcome this issue could be to fix it in the vendored yaml.v3
  • Another would be to find a suitable workaround for the issue ( I understand you still don't have a workaround for it??)

prembhaskal avatar Jan 07 '24 15:01 prembhaskal

@Xander-Polishchuk just do

hex-string: !!str 0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed # even without quotes!

Result should be

hex-string: "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"

This is described in YAML spec

oxfn avatar May 14 '24 14:05 oxfn

@oxfn doesn't work, ytt still omit quotes

echo 'hex-string: !!str "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"' > template.yaml
ytt -f template.yaml
# hex-string: 0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed

ytt version - develop

Also generally it won't be a good solution, just a workaround

Xander-Polishchuk avatar May 15 '24 02:05 Xander-Polishchuk

@Xander-Polishchuk Well, we're both right. I've tested on small values like 0x1234 and it makes a string. Serialization behavior somehow depends on number value. This is how it works:

!!str 0x1234 -> "0x1234"
!!str 0x12345678abcdef -> "0x12345678abcdef"
!!str 0x12345678abcdef01 -> "0x12345678abcdef01"
!!str 0x12345678abcdef01f -> 0x12345678abcdef01f
!!str 0x012345678abcdef01 -> "0x012345678abcdef01" # WTF?
!!str 0x012345678abcdef012 -> 0x012345678abcdef012

!!str 0x000000000000000011112222 -> "0x000000000000000011112222" # ?!

In fact maximum value for string is 0xFFFFFFFFFFFFFFFF, thus 0x10000000000000000 will be serialized as number

oxfn avatar May 15 '24 17:05 oxfn

@oxfn the issue is with the underlying go/yaml library https://github.com/go-yaml/yaml/issues/435 , when a hex string is bigger than max integer representation in golang (for the int type), the parseInt fails and the number is treated as string and hence not quoted.

prembhaskal avatar May 18 '24 16:05 prembhaskal

Are you considering go away from go-yaml? As it seems project is dead and doesn't accept any PRs and fixes.

Xander-Polishchuk avatar Jun 06 '24 06:06 Xander-Polishchuk

@Xander-Polishchuk so we had discussed this issue in one of the community meetings and yes, we were considering if moving to https://github.com/kubernetes-sigs/yaml makes more sense now. Though this particular issue exists there too. so one idea was to provide fix in the k8s-sigs/yaml (if they are open to PRs) Other idea was to fork and fix issue ourselves (less people favored this one)

prembhaskal avatar Jun 09 '24 16:06 prembhaskal