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

A plugin for testing kubernetes objects using conftest/open policy agent #146

Closed
wants to merge 1 commit into from

Conversation

garethr
Copy link

@garethr garethr commented May 13, 2019

This plugin allow for running conftest tests against a running Kubernetes cluster.
This makes it both easy to check a cluster matches some arbitrary policy and also
makes writing open policy agent code easier as you can quickly test it against a
real cluster with the minimum of fuss.

$ kubectl krew install --manifest plugin/conftest.yaml  -v4
I0513 22:28:08.542388    7886 install.go:150] --manifest specified, not ensuring plugin index
I0513 22:28:08.543105    7886 install.go:119] Will install plugin: conftest
Installing plugin: conftest
I0513 22:28:08.543132    7886 install.go:74] Looking for installed versions
I0513 22:28:08.543148    7886 util.go:79] Searching for installed versions of conftest in "/home/garethr/.krew/bin"
I0513 22:28:08.543176    7886 install.go:83] Finding download target for plugin conftest
I0513 22:28:08.543189    7886 util.go:38] Using os=linux arch=amd64
I0513 22:28:08.543200    7886 util.go:61] Matching platform for labels(arch=amd64,os=linux)
I0513 22:28:08.543272    7886 util.go:68] Found matching platform with index (0)
I0513 22:28:08.543287    7886 util.go:131] Matching plugin version is 5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a
I0513 22:28:08.543303    7886 install.go:46] Creating download dir "/tmp/krew-downloads/conftest"
I0513 22:28:08.543403    7886 install.go:61] Getting sha256 (5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a) signed version
I0513 22:28:08.543421    7886 downloader.go:36] Fetching "https://github.com/instrumenta/conftest/archive/v0.5.2.zip"
I0513 22:28:10.836287    7886 downloader.go:43] Reading download data into memory
I0513 22:28:11.317941    7886 downloader.go:48] Read 45106 bytes of download data into memory
I0513 22:28:11.318010    7886 downloader.go:174] detected .zip file
I0513 22:28:11.318032    7886 downloader.go:55] Extracting download zip to "/tmp/krew-downloads/conftest"
I0513 22:28:11.330572    7886 move.go:155] Creating plugin dir "/home/garethr/.krew/store/conftest"
I0513 22:28:11.331392    7886 move.go:161] Creating temp plugin move operations dir "/tmp/krew-temp-move923716619"
I0513 22:28:11.331450    7886 move.go:125] Finding move targets from "/tmp/krew-downloads/conftest" to "/tmp/krew-temp-move923716619" with file operation=index.FileOperation{From:"/*/plugin/*.sh", To:"."}
I0513 22:28:11.331514    7886 move.go:44] Trying to move single file directly from="/tmp/krew-downloads/conftest" to="/tmp/krew-temp-move923716619" with file operation=index.FileOperation{From:"/*/plugin/*.sh", To:"."}
I0513 22:28:11.331684    7886 move.go:52] Wasn't a single file, proceeding with Glob move
I0513 22:28:11.331990    7886 move.go:132] Move file from "/tmp/krew-downloads/conftest/conftest-0.5.2/plugin/kubectl-conftest.sh" to "/tmp/krew-temp-move923716619/kubectl-conftest.sh"
I0513 22:28:11.332105    7886 move.go:141] Move operations are complete
I0513 22:28:11.332132    7886 move.go:172] Move directory "/tmp/krew-temp-move923716619" to "/home/garethr/.krew/store/conftest/5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a"
I0513 22:28:11.336782    7886 install.go:159] No file found at "/home/garethr/.krew/bin/kubectl-conftest"
I0513 22:28:11.336838    7886 install.go:146] Creating symlink from "/home/garethr/.krew/store/conftest/5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a/kubectl-conftest.sh" to "/home/garethr/.krew/bin/kubectl-conftest"
I0513 22:28:11.336964    7886 install.go:150] Created symlink at "/home/garethr/.krew/bin/kubectl-conftest"
CAVEATS:
\
 |  This plugin needs the following programs:
 |  * conftest
 |  * jq
/
Installed plugin: conftest
garethr@surface-go ~/p/conftest> kubectl conftest
A Kubectl plugin for using Conftest to test objects in Kubernetes using Open Policy Agent

See https://github.com/instrumenta/conftest for more information

Usage:
   kubectl test (TYPE[.VERSION][.GROUP] [NAME] | TYPE[.VERSION][.GROUP]/NAME)

Checklist for plugin developers:

  • Read the Plugin Naming Guide (for new plugins)
  • Verify the installation from URL or a local archive works (kubectl krew install --manifest=[...] --archive=[...])

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 13, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: garethr
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: juanvallejo

If they are not already assigned, you can assign the PR to them by writing /assign @juanvallejo in a comment when ready.

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:

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

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 13, 2019
@garethr
Copy link
Author

garethr commented May 13, 2019

The CI failure appears unrelated to this change, rather a matter of mismatched types in some of the validation tooling.

@ahmetb
Copy link
Member

ahmetb commented May 14, 2019

The failure should go away if you rebase.

@corneliusweig
Copy link
Contributor

Hey @garethr , I think I read your newsletter. Keep up the good work!

homepage: https://github.com/instrumenta/conftest
caveats: |
This plugin needs the following programs:
* conftest
Copy link
Member

@ahmetb ahmetb May 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the plugin itself a dependency?
Or does the plugin have the same name as another tool?

Copy link
Author

@garethr garethr May 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the later.. The plugin provides a kubectl context to conftest, which is a standalone application https://github.com/instrumenta/conftest

I went the shell script plugin route for speed of implentation. If folks find it interesting I'll build a separate binary that uses go-client and conftest as a library to avoid the need for the shell dependencies.

@ahmetb
Copy link
Member

ahmetb commented May 15, 2019

(I wanted to move the discussion from code review thread to here for more visibility.)

@garethr yes this (depending on the actual tool through a separate installation) is certainly a bit unusual.

We don't see plugins that have a "bash script wrapper" as their entrypoint.

I see here you’re basically parsing cmd-line args to invoke various subcommands of the actual conftest tool.

I think you have 2 options:

Option 1: Handle plugin logic inside the executable

You can actually do this "inside" the conftest tool, by looking at args[0] which will end with .../kubectl-conftest.sh when it’s invoked as a plugin.

This way you ship still one binary (conftest) which handles invocation as plugin logic, too.

Option 2: Keep wrapper script, but ship the binary, too.

In the files: field, you can copy both kubectl-conftest.sh and the conftext[.exe] executable. (This requires bundling both into the plugin archive which you should probably do anyway.)

Then, in the script, you can use something like this to relatively invoke the actual conftest tool:

SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
conftest="${SCRIPT_DIR}/conftest"
...
"${conftest}" arg1 arg2 ...

@garethr
Copy link
Author

garethr commented May 16, 2019

Thanks. I'll likely try and do 2, then something closer to 1.

@ahmetb
Copy link
Member

ahmetb commented May 16, 2019

/hold
ok let me know when you’re done.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 16, 2019
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 17, 2019
@garethr garethr changed the title A plugin for testing kubernetes objects using conftest/open policy agent[conftest](url) A plugin for testing kubernetes objects using conftest/open policy agent May 17, 2019
@garethr
Copy link
Author

garethr commented May 17, 2019

@ahmetb thanks, changes made. I'm now shipping the plugin in the released packages. And installing it specifically for the plugin as discussed. You no longer need the separate installation.

@ahmetb
Copy link
Member

ahmetb commented May 17, 2019

🤔 seems like it’s still copying out only the .sh. Are you sure it copies out the binary from the archive?

@garethr
Copy link
Author

garethr commented May 17, 2019

@ahmetb ah, more haste less speed. Should be fixed now I think.

@ahmetb
Copy link
Member

ahmetb commented May 17, 2019

Here's some hot feedback:

kubectl conftest
...
Usage:
   kubectl test (TYPE[.VERSION][.GROUP] [NAME] | TYPE[.VERSION][.GROUP]/NAME)

this shouldn't be kubectl test?

-h probably shouldn't fail (--help works):

kubectl conftest -h
FATA[0000] Problem building rego compiler: open policy: no such file or directory

Overall I couldn't really use it but it's probably because I don't use OPA.

But also the --help wasn't much helpful.

I'm ok with merging as is. Let me know if you want to fix these first.

@garethr
Copy link
Author

garethr commented May 18, 2019

Thanks for testing, I've added an -h flag and also corrected the example comment.

$ kubectl conftest -h
A Kubectl plugin for using Conftest to test objects in Kubernetes using Open Policy Agent

See https://github.com/instrumenta/conftest for more information

Usage:
   kubectl conftest (TYPE[.VERSION][.GROUP] [NAME] | TYPE[.VERSION][.GROUP]/NAME)

Once this is available I'll be adding usage examples to the project README which is linked above.

Conftest relies on a policy folder to store the rego policies you want to test. If you do want to try it out grab the policy folder from this example https://github.com/instrumenta/conftest/tree/master/examples/kubernetes and try with:

$ kubectl conftest service
   Found service hello-kubernetes but services are not allowed
   Found service kubernetes but services are not allowed

@corneliusweig
Copy link
Contributor

Hey @garethr, I think conftest is a great tool and it deserves to be featured so that k8s users get aware of it. However, I'm a bit worried that it requires so many extra speps to be useful. As far as I understand, you need to have a policy folder with rego rules. This means that interested users need to dig deep into the conftest manual before they can get started. Your suggested starting point is to clone an example rules repo. At that point, a user might as well also download conftest and I don't see the advantage to invoke conftest via kubectl.


Further, my feeling is that conftest is by spirit not an extension of kubectl, but rather a standalone k8s tool. We haven't defined yet, what are the criteria for a krew plugin, but looking at what we have, I would say: tools that extend existing kubectl functionality. Only very few require extra configuration. And in that regard, I don't see how conftest fits in. To me it looks more like a tool from the k8s kosmos, but very different from kubectl.

@garethr
Copy link
Author

garethr commented May 18, 2019

@corneliusweig your characterisation of conftest is correct. It sounds like it's a good test of "we haven't defined yet what are the criteria for a krew plugin" as you say.

The following isn't a plea for inclusion, more my rationale for creating a kubectl plugin in the first place and packaging for krew. I'd much rather have the general conversation and help define that policy. No rush on the plugin itself.

kubectl plugins are just binaries with a certain name, so I could simply ask folks to download kubectl-conftest and they will have the plugin. That's obviously true of anyone building anything, the CLI plugin model is inherently open.

Krew provides a nice a nice veneer of management over that, and a handy discovery tool. My assumption was, like the CLI plugin model, the data powering that discovery and management tool was open (albeit with some sensible rules around naming and other bits.)

It's absolutely fine to decide that the krew index is actually heavily curated, though it would be good to establish better guidelines and a formal approval process if that's the direction of travel.

I do think that will lead to two sets of kubectl plugins, those managed by Krew and everything else. That makes plugin management harder for users in my opinion.

It may be worth separating the krew tool from the index. Keep the index heavily curated, but do one or more of the following:

  • allow installation direct from a URL to the manifest
  • maintain a separate less-curated index (ala Helm unstable) that users can opt in to
  • allow folks to create there own index (ala Homebrew taps)

Hopefully that's a useful viewpoint, and not just specific to this particular plugin. I'll be at KubeCon as well if it's worth an in-person conversation.

@ahmetb
Copy link
Member

ahmetb commented May 18, 2019

Thanks for the valuable discussion. re: @corneliusweig’s point:

it requires so many extra steps to be useful

I'm not convinced that this would be the admission criteria. We have admitted similar plugins like kubectl oidc-login that are relevant to a subset of users in the past.

The rule of thumb I’m using is whether it would fit to be a kubectl subcommand, or is it "mostly" related to a third-party software. There's precedent for both cases. For example, we have kubectl kubesec-scan which actually only works against kubesec.io service.

re: @garethr’s points:

I'd much rather have the general conversation and help define that policy.

This would be appreciated. Indeed, right now all open PRs in this repo are on-hold because they fall into a gray area as for whether they should be distributed as plugins or not.

It may be worth separating the krew tool from the index. Keep the index heavily curated, but do one or more of the following:

  • allow installation direct from a URL to the manifest

This came up many times before, but we couldn’t find the time to do it yet. It’s the direction that we want to go. We have an open issue kubernetes-sigs/krew#23 but I haven’t been able to follow up with a concrete proposal. It will happen, just a matter of time.


I’ll be at KubeCon and have plenty of time to chat about these. Let’s try to catch up.

…y agent

This plugin allow for running conftest tests against a running Kubernetes cluster.
This makes it both easy to check a cluster matches some arbitrary policy and also
makes writing open policy agent code easier as you can quickly test it against a
real cluster with the minimum of fuss.

```console
$ kubectl krew install --manifest plugin/conftest.yaml  -v4
I0513 22:28:08.542388    7886 install.go:150] --manifest specified, not ensuring plugin index
I0513 22:28:08.543105    7886 install.go:119] Will install plugin: conftest
Installing plugin: conftest
I0513 22:28:08.543132    7886 install.go:74] Looking for installed versions
I0513 22:28:08.543148    7886 util.go:79] Searching for installed versions of conftest in "/home/garethr/.krew/bin"
I0513 22:28:08.543176    7886 install.go:83] Finding download target for plugin conftest
I0513 22:28:08.543189    7886 util.go:38] Using os=linux arch=amd64
I0513 22:28:08.543200    7886 util.go:61] Matching platform for labels(arch=amd64,os=linux)
I0513 22:28:08.543272    7886 util.go:68] Found matching platform with index (0)
I0513 22:28:08.543287    7886 util.go:131] Matching plugin version is 5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a
I0513 22:28:08.543303    7886 install.go:46] Creating download dir "/tmp/krew-downloads/conftest"
I0513 22:28:08.543403    7886 install.go:61] Getting sha256 (5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a) signed version
I0513 22:28:08.543421    7886 downloader.go:36] Fetching "https://github.com/instrumenta/conftest/archive/v0.5.2.zip"
I0513 22:28:10.836287    7886 downloader.go:43] Reading download data into memory
I0513 22:28:11.317941    7886 downloader.go:48] Read 45106 bytes of download data into memory
I0513 22:28:11.318010    7886 downloader.go:174] detected .zip file
I0513 22:28:11.318032    7886 downloader.go:55] Extracting download zip to "/tmp/krew-downloads/conftest"
I0513 22:28:11.330572    7886 move.go:155] Creating plugin dir "/home/garethr/.krew/store/conftest"
I0513 22:28:11.331392    7886 move.go:161] Creating temp plugin move operations dir "/tmp/krew-temp-move923716619"
I0513 22:28:11.331450    7886 move.go:125] Finding move targets from "/tmp/krew-downloads/conftest" to "/tmp/krew-temp-move923716619" with file operation=index.FileOperation{From:"/*/plugin/*.sh", To:"."}
I0513 22:28:11.331514    7886 move.go:44] Trying to move single file directly from="/tmp/krew-downloads/conftest" to="/tmp/krew-temp-move923716619" with file operation=index.FileOperation{From:"/*/plugin/*.sh", To:"."}
I0513 22:28:11.331684    7886 move.go:52] Wasn't a single file, proceeding with Glob move
I0513 22:28:11.331990    7886 move.go:132] Move file from "/tmp/krew-downloads/conftest/conftest-0.5.2/plugin/kubectl-conftest.sh" to "/tmp/krew-temp-move923716619/kubectl-conftest.sh"
I0513 22:28:11.332105    7886 move.go:141] Move operations are complete
I0513 22:28:11.332132    7886 move.go:172] Move directory "/tmp/krew-temp-move923716619" to "/home/garethr/.krew/store/conftest/5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a"
I0513 22:28:11.336782    7886 install.go:159] No file found at "/home/garethr/.krew/bin/kubectl-conftest"
I0513 22:28:11.336838    7886 install.go:146] Creating symlink from "/home/garethr/.krew/store/conftest/5b587c11e4b1de8679c39e07a514c68e54de57987fc3eb32dc7946e78994359a/kubectl-conftest.sh" to "/home/garethr/.krew/bin/kubectl-conftest"
I0513 22:28:11.336964    7886 install.go:150] Created symlink at "/home/garethr/.krew/bin/kubectl-conftest"
CAVEATS:
\
 |  This plugin needs the following programs:
 |  * jq
/
Installed plugin: conftest
garethr@surface-go ~/p/conftest> kubectl conftest
A Kubectl plugin for using Conftest to test objects in Kubernetes using Open Policy Agent

See https://github.com/instrumenta/conftest for more information

Usage:
   kubectl test (TYPE[.VERSION][.GROUP] [NAME] | TYPE[.VERSION][.GROUP]/NAME)
```
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 18, 2019
@corneliusweig
Copy link
Contributor

corneliusweig commented Aug 18, 2019

/remove-lifecycle stale
We need to revisit this plugin after finalizing the plugin criteria

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 18, 2019
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 16, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 16, 2019
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants