Issue with leading `v`s

[Pluies]

What happened:

In some cases, Zeitgeist is having some issues with leading vs in semver versions.

For example, the following dependencies.yaml config:

  - name: sloth
    version: 0.11.0
    scheme: semver
    sensitivity: minor
    upstream:
      flavour: github
      url: slok/sloth
    refPaths:
      - path: src/layers/3-observability/sloth.tf
        match: zeitgeist:sloth

Sees v0.12.0 as a new version and tries to upgrade it as:

  - name: sloth
    version: v0.12.0
    scheme: semver
    sensitivity: minor
    upstream:
      flavour: github
      url: slok/sloth
    refPaths:
      - path: src/layers/3-observability/sloth.tf
        match: zeitgeist:sloth

(Notice the added leading v)

And the matching update in the file:

image:
  repository: ghcr.io/slok/sloth
-  tag: v0.11.0 # zeitgeist:sloth
+  tag: vv0.12.0 # zeitgeist:sloth

What you expected to happen:

When the leading v is omitted in the dependencies.yaml, I expect zeitgeist to not add it anywhere else, and upgrade only the version instead

How to reproduce it (as minimally and precisely as possible):

See above

Anything else we need to know?:

Environment:

standard

[saschagrunert]

Thank you for the report @Pluies, do you plan to invest more time into the investigation and possible fix? :)

[Pluies]

Yes! 😄 I'm mostly putting this here as I keep hitting the issue in our setup and going "ah I really should fix this".

Looking into this some more; I'm not actually sure it's a zeitgeist issue as much as an issue in our own pipelines, where we run a two-step process:

1. Get all potential upgrades

      - name: Generate zeitgeist dependency upgrade list
        id: zeitgeist
        run: |
          ./zeitgeist export --output-file dependency_output.json --output-format json
          echo "MATRIX=$(jq --compact-output '{ "include": . }' < dependency_output.json)" >> "$GITHUB_OUTPUT"

2. Set version & create individual PRs

      - name: Upgrade dependency
        run: |
          ./zeitgeist set-version ${{ matrix.name }} ${{ matrix.new_version }}

      - name: Create Pull Request
        uses: peter-evans/create-pull-request@v6
        with:
          token: ${{ secrets.ZEITGEIST_PAT }}
          author: Zeitgeist ✨ <infrastructure@dune.com>
          commit-message: "[zeitgeist] upgrade ${{ matrix.name }} to ${{ matrix.new_version }}"
          base: main
          branch: "zeitgeist/${{ matrix.name }}"
          title: "Zeitgeist: upgrade ${{ matrix.name }} to ${{ matrix.new_version }}"
          body: "Upgrade ${{ matrix.name }} to ${{ matrix.new_version }}"

So here zeitgeist set-version does its job well; and zeitgeist export is arguably too lenient in accepting the original version without a leading v, and exporting the new version with a leading v (as it is the actual new tag).

An easy workaround is to fix the dependency declaration by adding the leading v in the first place. This should be the recommended fix really.

The other option would be to make Zeitgeist stricter by not accepting non-v-prefixed-semvers if the tag is v-prefixed, but that's a bit of a breaking change and really is only an issue in this two-step process... so I'm not sure it's the best course of action.

[TheEdgeOfRage]

I think we can add a setting to zeitgeist (per dependency on the config) that tells zeitgeist whether that dep should have a leading v or not. There are cases where for example the tag on github has a v, but when referencing a version in your repo you need to omit it (e.g. tf providers). If we have a per-dependency flag to control for whether we need the v or not, regardless of whether the source tag has it, it would solve both of these issues

[Pluies]

That's a fair point. Expanding here on the tf provider example, we have the following entry in dependencies.yaml:

  - name: terraform-provider-aws
    version: 5.93.0
    scheme: semver
    sensitivity: major
    upstream:
      flavour: github
      url: hashicorp/terraform-provider-aws
    refPaths:
      - path: example/backend.tf
        match: zeitgeist:terraform-provider-aws

Because the terraform syntax to declare a provider is:

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "= 5.93.0" # zeitgeist:terraform-provider-aws
    }
}

(Notice: version only, no leading v accepted)

But the actual releases of the hashicorp/terraform-provider-aws provider on github all have leading vs.

So the workaround I suggested above (adding the leading v in the dependency) does not work.

Adding an extra bit of config sounds sane. I'm thinking this could be a new respect_leading_v field that would:

• If the version field contains a leading v: output leading vs as well
• Conversely, if the version doesn't contain a leading v: strip out leading vs from outputs

And default it to false to keep the current behaviour and not break backwards compatibility.

Another option is to have something like strip_leading_v, that would always strip the leading v in versions. But we'd need to be careful on the wording and docs around it.

Wdyt?

(Side note: I've also seen instances where a project starts with releases without a leading v, then at some point decides leading vs are the way to go and start using a leading v... This is such a specific issue that zeitgeist is likely to have trouble handling it regardless of the logic we write, so let's not even try to support this use-case :) )

[TheEdgeOfRage]

respect_leading_v doesn't seem quite intuitive to me. I think I'd prefer the strip_leading_v option, or something obvious like that

[justaugustus]

As a note, the leading v is not actually part of a semantic version.
I haven't read through all of the context here, but stripping the leading v before interpretation feels more appropriate.

ref https://semver.org/#is-v123-a-semantic-version:

No, “v1.2.3” is not a semantic version. However, prefixing a semantic version with a “v” is a common way (in English) to indicate it is a version number. Abbreviating “version” as “v” is often seen with version control. Example: git tag v1.2.3 -m "Release version 1.2.3", in which case “v1.2.3” is a tag name and the semantic version is “1.2.3”.

[TheEdgeOfRage]

Yeah, zeitgeist could just not deal with the v prefix at all and always strip it out when fetching versions. Then in the place where you use the version, it can ignore any prefix that's there and it's up to the user to add the prefix as necessary

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[TheEdgeOfRage]

/remove-lifecycle rotten