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

Filter pattern type: strict-semver #2729

Closed
cstaud opened this issue Jan 9, 2020 · 3 comments
Closed

Filter pattern type: strict-semver #2729

cstaud opened this issue Jan 9, 2020 · 3 comments

Comments

@cstaud
Copy link

cstaud commented Jan 9, 2020

Describe the feature
Actual there exist 3 filter pattern types: glob, regexp and semver. semver is documented to filter for Semantic Versioning, but it isnt. While semver.org describes a valid semver with the X.Y.Z patter, this implementation allows a lot more. So for flux just an integer is seen as a semver. Pls have a look at this: regex101. I would call this a bug..., but as changing this behavior would break things I would go for another filter: strict-semver

To describe the actual failure that occured:

  • We build images with 8 chars - git sha as tag.
  • It happens that this 8 chars is composed of only digits (=integer).
  • We also build images with a valid semver. (Releases)

This integer is detected as a semver and deployed.
Deployments stuck on this tag as a 8 digit integer is > the valid semver releases.

What would the new user story look like?
0. Use strict-semver as tag filter.

  1. Only valid Semantic Versions are filtered.

Expected behavior
Because a change/fix of this behavior would break things for others, I would like to have a filter like: strict-semver. strict-semver should filter only valid Semantic Versions.

@cstaud cstaud added blocked-needs-validation Issue is waiting to be validated before we can proceed enhancement labels Jan 9, 2020
@stefanprodan
Copy link
Member

stefanprodan commented Jan 9, 2020

Can you post here an example of a semver filter and a miss-match of 8 charts, Flux uses https://github.com/Masterminds/semver, I'm going to update Flux to the latest version of that package and write a test if you can provide me with the above.

@cstaud
Copy link
Author

cstaud commented Jan 9, 2020

At first thank you for your support. We already noticed this, the implementation there is also wrong.
If you have a look at the implementation the regex used there matches also non-valid contrains. Have a look at this

But i will give you a more detailed explanation:

    flux.weave.works/automated: "true"
    flux.weave.works/tag.rat: "semver:*"
  1. docker build -t myservice:0.0.1 . && docker push ...
    1.1 myservice:0.0.1 is deployed
  2. docker build -t myservice:12345678 . && docker push ...
    2.1 myservice:12345678 is deployed (THIS should not happen as this is not a valid semver)
  3. docker build -t myservice:0.0.2 . && docker push ...
    3.1 myservice:0.0.2 is NOT deployed as 12345678 > 0.0.2

Pls ask if you need more information on this.

@kingdonb
Copy link
Member

kingdonb commented Feb 26, 2021

It appears this issue has been taken into account in the redesigned Flux v2, per linked discussions in the image-reflector-controller and source-controller above.

I think after reading the discussion there, they have settled for a particular implementation that is aligned with what Helm itself uses for semver parsing. I am not certain it will behave differently after a brief reading of the code and discussion there. I have a bone to pick with the semver:* automation, since it implies that Flux should apply any release at all without delay or review, regardless of signals which indicate that someone should be making a manual update here (a semver major increment means a breaking change, that could mean anything including adjustments to the deployment spec is required, or new environment settings, or additional secrets and configmaps that should be attached.)

Someone should be in the position to increment the automation policy for a major version change in the PR where this review is completed, but I am not here to argue this point.

I am not certain if this issue is still affecting you, or if it can be considered resolved by Flux v2. Flux v1 is in maintenance mode now, and is not adding any new features unless they are critical. As Flux contrib efforts have been focused on Flux v2, the Flux project has moved to a new repo, fluxcd/flux2

In the interest of reducing the number of open issues not directly related to supporting Flux v1 in maintenance mode, and respecting you may have moved on already, I will go ahead and close out this issue for now. Thanks for using Flux!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants