feat: skip install on validate-krew-manifest

[caarlos0]

I'm working in integrating the generation of krew manifests into GoReleaser, and one thing I would like to do there is a test to validate that test-generated manifests are valid...

But, while testing, the URLs are all faked, so, installing them won't work.

I would still like to be able to validate everything else though, so I added a new flag which we can pass -install=false so it won't actually install anything...

Happy to hear your thoughts on this!

Cheers!

[k8s-ci-robot]

Welcome @caarlos0!

It looks like this is your first PR to kubernetes-sigs/krew 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/krew has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[ahmetb]

I am supportive of this. But I also want to point out that very frequently we encounter issues with the archive files themselves (e.g. wrong paths in the manifest causing the plugin to fail installing, or validateLicenseFileExists which this PR deactivates). This PR can still help you do some basic structural validation.

How do you plan on calling this program via goreleaser? Will you need a Go package, or does the existing binary suffice?

[caarlos0]

How do you plan on calling this program via goreleaser? Will you need a Go package, or does the existing binary suffice?

right now the plan was to run it only on our own generated files on test phase (aka our golden files).

its already doing so using my fork: https://github.com/goreleaser/goreleaser/blob/a5719cf0095bd1394cb9e8706b73a826139eaf74/internal/pipe/krew/krew_test.go#L794-L806

[ahmetb]

right now the plan was to run it only on our own generated files on test phase (aka our golden files).

IMO this is probably not quite necessary. The plugin spec has been fairly stable now for 2+ years, and if we were to introduce a change like apiVersion bump, we'd most likely do it in a way that's backwards compatible (because there are 150+ manifests out there that we cannot possibly update, and we don't wanna break all that).

If you ask me, there's no need to test your golden files. If somebody using Goreleaser notices their auto-generated manifest is not accepted by Krew's validator, they would let you know by opening an issue. what do you think?

[caarlos0]

IMO this is probably not quite necessary. The plugin spec has been fairly stable now for 2+ years, and if we were to introduce a change like apiVersion bump, we'd most likely do it in a way that's backwards compatible (because there are 150+ manifests out there that we cannot possibly update, and we don't wanna break all that).

the idea is to make sure the files we generate are valid...

[ahmetb]

the idea is to make sure the files we generate are valid...

okay :D I was mostly proposing that you can probably validate them manually (using eyes) and then check them in. If you want to test them as well, it's acceptable.

I think we can add this, I am not too hesitant since this is mostly a tool we don't ship to users.

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmetb, caarlos0

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ahmetb]

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

[caarlos0]

Nice, thanks @ahmetb !