Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pick the most recent chart/tag for ambiguous semver matches #172

Merged
merged 1 commit into from
Oct 27, 2020

Conversation

wingsofovnia
Copy link
Contributor

@wingsofovnia wingsofovnia commented Oct 17, 2020

As per Semver para 11, build metadata does not figure into precedence. This means if two versions have equal MAJOR, MINOR, PATCH tokens, but different build metadata (para 10), the order is 💥 undefined. . Given that we don't simply match version string but chart versions, we have additional metadata, such as chartVersion.Created, that we can use to further clarify the logic without violating Semver. This behavior also fits the principle of least surprise better.

Such a change is useful for dev environments that are expected to get the latest build of the same app/chart. In the current setup, this cannot be done, especially when (and usually does) build metadata contains data that doesn't sort well (git hashes, sigs, etc.).

The same approach applies to GitRepository resources that make use of a semver tag strategy. It is possible to use a timestamp of the tag's target commit: ebfb118

Derived from #171 to decouple this change from migrating to masterminds/semver.

Related discussion fluxcd/flux2#355

cc @hiddeco @stefanprodan

@wingsofovnia wingsofovnia changed the title Pick the most recent chart for ambiguous version matches Pick the most recent chart/tag for ambiguous semver matches Oct 17, 2020
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks a lot for also adding it to the Git semver tag feature 🏅

@hiddeco hiddeco added area/git Git related issues and pull requests area/helm Helm related issues and pull requests enhancement New feature or request labels Oct 19, 2020
@wingsofovnia
Copy link
Contributor Author

@stefanprodan any updates on this?

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @wingsofovnia 🏅

Can you please rebase with main and signoff your commits

Signed-off-by: Illia Ovchynnikov <illia.ovchynnikov@gmail.com>
@hiddeco hiddeco merged commit a2537f8 into fluxcd:main Oct 27, 2020
@wingsofovnia wingsofovnia deleted the feature/chro-order branch October 27, 2020 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests area/helm Helm related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants