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

cmd/validate-krew-manifest: initial implementation #151

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

ahmetb
Copy link
Member

@ahmetb ahmetb commented Feb 3, 2019

This tool takes a manifest file and runs certain validation steps. It's meant
to test plugin manifests added to krew-index install correctly (so far it has
found one broken manifest).

Fixes #148.
Related to #149 but doesn't fix it.

@codecov-io
Copy link

codecov-io commented Feb 3, 2019

Codecov Report

Merging #151 into master will decrease coverage by 1.99%.
The diff coverage is 37.63%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #151   +/-   ##
=======================================
- Coverage   52.91%   50.91%   -2%     
=======================================
  Files          12       13    +1     
  Lines         618      711   +93     
=======================================
+ Hits          327      362   +35     
- Misses        240      297   +57     
- Partials       51       52    +1
Impacted Files Coverage Δ
pkg/index/validate.go 94.11% <ø> (ø) ⬆️
cmd/validate-krew-manifest/main.go 37.63% <37.63%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8466d44...b36f0f6. Read the comment docs.

@ahmetb ahmetb force-pushed the manifest-validator-tool branch 3 times, most recently from c3c59a0 to e5248f6 Compare February 3, 2019 22:29
@ahmetb
Copy link
Member Author

ahmetb commented Feb 4, 2019

cc: @juanvallejo
this is currently hacky but it'll help us validate issues in submitted manifest files.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
@juanvallejo
Copy link

@ahmetb trying this out locally. Currently running into the following error:

$ go run cmd/validate-krew-manifest/main.go --manifest="~/gopaths/plugins/src/github.com/GoogleContainerTools/krew-index/plugins/change-ns.yaml"
I0204 13:36:44.913634   15279 main.go:57] structural validation OK
I0204 13:36:44.913756   15279 main.go:65] all spec.platform[] items are used
I0204 13:36:44.913850   15279 main.go:71] no overlapping spec.platform[].selector
I0204 13:36:44.913857   15279 main.go:75] installing spec.platform[0]
F0204 13:36:44.990111   15279 main.go:39] spec.platforms[0] failed to install: plugin install command failed: Error: unknown command "krew" for "kubectl"
        Run 'kubectl --help' for usage.
        unknown command "krew" for "kubectl"
        : exit status 1
exit status 255

@ahmetb
Copy link
Member Author

ahmetb commented Feb 4, 2019

@juanvallejo you need to install krew first. The validator runs kubectl krew install with KREW_OS, KREW_ARCH for each spec.platform[] and with a temp KREW_ROOT dir to see if installation goes through.

For example, the install failed to symlink "change-ns" on windows, because the bin field for windows was lacking the .exe suffix.

@juanvallejo
Copy link

@ahmetb yep thanks, had some path issues with kubectl. Appears to be working now

@ahmetb
Copy link
Member Author

ahmetb commented Feb 4, 2019

Ideally we'd take some of the validation here (like "has overlapping platform selectors", "all selectors map to at least one supported os/arch") and put it in pkg/index#Validate() method in the future –so there's one path for validation.

@juanvallejo
Copy link

lgtm

@ahmetb ahmetb merged commit 50fc264 into kubernetes-sigs:master Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hack:CI/CD tooling to verify manifests
4 participants