🐛 make ENVTEST_K8S_VERSION respect go.mod replace directives

[Shubhamag12]

fixes: #5088

this fix ensures ENVTEST uses the replaced k8s.io/api version when replace directives are present in go.mod

[k8s-ci-robot]

Hi @Shubhamag12. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Shubhamag12
Once this PR has been reviewed and has the lgtm label, please assign kavinjsir for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vitorfloriano]

Thanks for the PR!

I was wondering how this would behave in a developer workflow that involves a local fork?

For example, if someone is testing against a local clone of k8s.io/api with a go.mod directive like replace k8s.io/api v1.28.0 => ../my-api-fork (suppose the developer needs to test changes against an unreleased version of Kubernetes or debug an issue in an upstream dependency)

What would the awk '{print $NF}' command resolve to in that case? I'm not sure it would be a version string.

Perhaps @dmvolod can chime in?

[Shubhamag12]

@vitorfloriano That's a good catch, local path replacements would break this approach. I will verify this case and will try to come up with more robust approach. Thanks!

[dmvolod]

Thanks for the PR!

I was wondering how this would behave in a developer workflow that involves a local fork?

For example, if someone is testing against a local clone of k8s.io/api with a go.mod directive like replace k8s.io/api v1.28.0 => ../my-api-fork (suppose the developer needs to test changes against an unreleased version of Kubernetes or debug an issue in an upstream dependency)

What would the awk '{print $NF}' command resolve to in that case? I'm not sure it would be a version string.

Perhaps @dmvolod can chime in?

Yes, @vitorfloriano this is good point.
In case of replacement with path, both current and new approaches are not working as expected.
This should be polished a bit with new inputs.

[Shubhamag12]

Hi @vitorfloriano @dmvolod

I tested the local path replacement case and my current fix breaks:

With k8s.io/api v0.34.1 => ../api-fork:

• The current fix outputs 1.0 (tries to parse the path as version)

I can update the fix to handle these cases:
go list -m k8s.io/api | awk '{if($$NF ~ /^v[0-9]/) print $$NF; else print $$2}'

This checks if the replacement starts with "v" + digit. If yes, use replacement; otherwise, fall back to original version.

one question here: Should we fall back to the original version in this case? as go list -m only gives the declared version

[vitorfloriano]

one question here: Should we fall back to the original version in this case? as go list -m only gives the declared version

Honestly, I don't know. It seems to me that a more reliable approach would be to warn the user about not being able to resolve the version and ask them to set the version as env var, but that would require a separate program. 🤔

[dmvolod]

I think that if the last field when getting the version is not the version, you should throw an error if possible and tell the user that this variable should be defined manually, like

ENVTEST_K8S_VERSION=v1.31.1 make ...

[Shubhamag12]

@dmvolod Thanks for your suggestion

Here is output with this change:

ENVTEST_K8S_VERSION ?= $(shell go list -m k8s.io/api | \
  awk '{ \
    if ($$NF ~ /^v[0-9]/) { \
      gsub(/^v/, "", $$NF); \
      split($$NF, v, "."); \
      major=v[1]; minor=v[2]; \
      if (major == 0) major=1; \
      printf "%d.%d", major, minor \
    } else { \
      print "Error: Cannot detect version. Set ENVTEST_K8S_VERSION manually." > "/dev/stderr"; \
      exit 1 \
    } \
  }' \

1. Without any replace
->  make setup-envtest 
Downloading sigs.k8s.io/controller-runtime/tools/setup-envtest@release-0.22
Setting up envtest binaries for Kubernetes version 1.34..

2. With env replace
->  go mod edit -replace=k8s.io/api=k8s.io/api@v0.33.0 
-> make setup-envtest
Setting up envtest binaries for Kubernetes version 1.33...

3. With local fork replace
-> go mod edit -replace=k8s.io/api=../api-fork 
-> go list -m k8s.io/api
k8s.io/api v0.34.0 => ../fake-api-fork
-> make setup-envtest
Error: Cannot detect version. Set ENVTEST_K8S_VERSION manually.

4. ENVTEST_K8S_VERSION=1.31 make setup-envtest
Setting up envtest binaries for Kubernetes version 1.31...

Let me know if this looks good!