Check the versionSelection before install/update #4222
Conversation
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
| } | ||
|
|
||
| // Ensure the selected version can be, actually installed to let the user know before installing | ||
| elegibleVersion, err := versions.HighestConstrainedVersion([]string{pkgVersion}, vendirversions.VersionSelection{Semver: versionSelection}) |
There was a problem hiding this comment.
This is how kapp-ctrl is actually doing that; which makes me wonder: should we use vendir from carvel instead of other semver dependencies?
There was a problem hiding this comment.
If it suits our needs, sure.
There was a problem hiding this comment.
Although IMO, this assertion belongs in the backend world, we should ensure we only display "valid" versions in the user-faced UI selectors.
I agree (that we should only present the valid versions for upgrading), but that wouldn't obviate the need for this change would it? (given that it's an API, so a request could come in to install an invalid version regardless of what we present in the UX).
| } | ||
|
|
||
| // Ensure the selected version can be, actually installed to let the user know before installing | ||
| elegibleVersion, err := versions.HighestConstrainedVersion([]string{pkgVersion}, vendirversions.VersionSelection{Semver: versionSelection}) |
There was a problem hiding this comment.
If it suits our needs, sure.
Description of the change
I've noticed I've been having some issues when installing some carvel packages: they got "installed" but never reconcillied.
The problem was that we were requesting a version that should not be selected. For instance, a prerelease when they are disabled by default.
Idem when updating: nothing prevents one from selecting a previous version, regardless of the default config.
Benefits
The version will be checked before installing, this way the user will get a
friendlymessage.Possible drawbacks
Although IMO, this assertion belongs in the backend world, we should ensure we only display "valid" versions in the user-faced UI selectors.
Applicable issues
Additional information
Marked as a draft as tests are still missing.