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

Standardize installation of golang tools with better version support #3715

Closed
lukas016 opened this issue Dec 20, 2023 · 1 comment · Fixed by #3718
Closed

Standardize installation of golang tools with better version support #3715

lukas016 opened this issue Dec 20, 2023 · 1 comment · Fixed by #3718
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@lukas016
Copy link
Contributor

lukas016 commented Dec 20, 2023

What do you want to happen?

Hi,

i have small problem with default makefile where linter and envtest don't have logic for check of specific version:

https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go#L248
https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go#L139

it creates problem with upgrading of tools, where some developers still use old version because they didn't manually delete previous version.
Another additional problem linked with it is using of latest version for envtest. many developers use different version, if they join on project later.

We have small extension for makefile designed primary for shared cache in pipeline which can rename binary and our binaries names contain version too which solve problem with upgrading of tools.

## Tool Binaries
KUSTOMIZE ?= $(LOCALBIN)/kustomize-$(KUSTOMIZE_VERSION)
ENVTEST ?= $(LOCALBIN)/setup-envtest-$(ENVTEST_VERSION)
GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)
KUSTOMIZE_VERSION ?= v5.3.0
ENVTEST_VERSION ?= release-0.15
GOLANGCI_LINT_VERSION ?= v1.55.2
....
.PHONY: envtest
envtest: $(LOCALBIN) ## Download envtest-setup locally if necessary.
	$(call go-install-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest,${ENVTEST_VERSION})
...
# go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist
# $1 - target path with name of binary (ideally with version)
# $2 - package url which can be installed
# $3 - specific version of package
define go-install-tool
@[ -f $(1) ] || { \
set -e; \
version=@$(3) ;\
url=$(2) ;\
if echo $$version | grep 'v[2-9][0-9]*' -q; then \
        echo $$url | grep '/v[2-9][0-9]*/' -q || version="/$$(printf $$version | grep -o 'v[2-9][0-9]*')$$version" ;\
fi ;\
echo "Downloading $${url}$${version}" ;\
GOBIN=$(GOBIN) go install $${url}$${version} ;\
binary=$$(echo "$$url" | rev | cut -d'/' -f 1 | rev) ;\
mv "$(GOBIN)/$${binary}" $(1) ;\
}
endef

Local bin folder:

$ ls bin
controller-gen-v0.9.0  golangci-lint-v1.53.3  kustomize-v3.8.7  setup-envtest-release-0.15

What do you think about this improvement?

Extra Labels

No response

@camilamacedo86
Copy link
Member

Hi @lukas016,

Thank you for raising your suggestion

Current Behavior:

Currently, the Makefile uses any available binary tool without considering its version. If a tool is found, it is used, regardless of whether it matches the specific version defined in the Makefile.
Proposed Change:

Your proposed change suggests modifying each target in the Makefile to:

  1. Check if the binary tool exists.
  2. Verify if the binary tool's version matches the specific version defined in the Makefile.
  3. If the binary tool is missing or does not match the defined version, the Makefile would download the correct version of the binary tool.

I encourage you to go ahead and open a PR with these proposed changes. I'll be here to review and provide feedback on your PR as needed. Please feel free to reach out if you have any questions or need assistance during the process.

Thank you for taking the initiative, and I look forward to seeing your contributions.

Best regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants