Skip to content

Report an oversized expects= integer instead of panicking - #1002

Open
arpitjain099 wants to merge 1 commit into
carvel-dev:developfrom
arpitjain099:fix/expects-oversized-int
Open

Report an oversized expects= integer instead of panicking#1002
arpitjain099 wants to merge 1 commit into
carvel-dev:developfrom
arpitjain099:fix/expects-oversized-int

Conversation

@arpitjain099

Copy link
Copy Markdown

MatchAnnotationExpectsKwarg.checkInt tries Int64() and then Uint64(), and panics if neither conversion succeeds:

panic("Unsure how to convert starlark.Int to int")

That value comes straight out of the template, so an overlay like this crashes ytt:

#@overlay/match by=overlay.all, expects=99999999999999999999999999999999999999999
panic: Unsure how to convert starlark.Int to int
overlay.MatchAnnotationExpectsKwarg.checkInt(...) match_annotation_expects_kwarg.go:145

A negative bignum does the same. An integer that fits in neither type can never equal a node count, so it is a bad input rather than an impossible state, and the other malformed expects= values are already reported as errors. This returns one in the same shape.

The test drives it through RunWithFiles, which is the path the CLI takes. It panics without the change.

Noted while verifying, not fixed here since it is a separate code path: --data-value-yaml n=<bignum> panics in pkg/template/core/go_value.go with a different message, so that one wants its own change.

checkInt tries Int64() then Uint64() and panics when neither fits. The
value comes straight from an overlay annotation, so a template with

  #@overlay/match by=overlay.all, expects=99999999999999999999999999999999999999999

crashed ytt with 'Unsure how to convert starlark.Int to int' rather than
telling the author what was wrong. A negative bignum does the same.

Such a value can never equal a node count, so report it like the other
malformed expects= inputs already are.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants