Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Parse image refs in HelmReleases with >2 elements #2620

Merged
merged 1 commit into from
Nov 19, 2019

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Nov 19, 2019

This commit fixes the parsing of image references with more than
2 elements in HelmRelease resources.

The previous version made the assumption that any given image value
given to image.ParseRef() would result in a image.Ref with only
the image name set. This assumption is however wrong as the parser
will set the first element of the passed string split by / to be
the domain in case there are more than 2 elements in total, resulting
in the first element to be overwritten by a new domain value, if
detected at a later stage.

The solution is to construct the full image ref before calling
image.ParseRef(), preventing a partial parse of any image element
snippets that may result in a misinterpretation of the image reference.

Fixes #2618

@hiddeco hiddeco force-pushed the bug/hr-multi-element-img branch 2 times, most recently from 6d3faa8 to 28de73c Compare November 19, 2019 14:52
@hiddeco hiddeco requested a review from squaremo November 19, 2019 14:52
@hiddeco
Copy link
Member Author

hiddeco commented Nov 19, 2019

/review-roulette

This commit fixes the parsing of image references with more than
2 elements in `HelmRelease` resources.

The previous version made the assumption that any given image value
given to `image.ParseRef()` would result in a `image.Ref` with only
the image name set. This assumption is however wrong as the parser
will set the first element of the passed string split by `/` to be
the domain in case there are more than 2 elements in total, resulting
in the first element to be overwritten by a new domain value, if
detected at a later stage.

The solution is to construct the full image ref before calling
`image.ParseRef()`, preventing a partial parse of any image element
snippets that may result in a misinterpretation of the image reference.
@hiddeco hiddeco force-pushed the bug/hr-multi-element-img branch from 28de73c to d25a4a9 Compare November 19, 2019 15:41
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job reformulating this. It is simpler to understand now, too, I think.

@hiddeco hiddeco merged commit db52f1c into master Nov 19, 2019
@hiddeco hiddeco deleted the bug/hr-multi-element-img branch November 19, 2019 16:30
@2opremio 2opremio added this to the 1.16.0 milestone Nov 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fluxcd failed to automate helmrelease chart-image
3 participants