Kubernetes transport #205

coderanger · 2017-11-06T06:33:53Z

Includes a test harness using the same tests as for Docker.

This uses kubectl exec internally. Windows pods are not supported at this time, mostly because I have no way to test them. Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

adamleff · 2017-11-06T20:18:41Z

Thanks for the contribution, @coderanger! We're discussing this one internally in relation to some other kubernetes work we've been discussing. Hopefully we'll have more feedback for you next week.

chris-rock

@coderanger Thank you very much for this huge improvement. We always wanted to add k8s as native supported transport, therefore this is a great step into the right direction. I left some comments to better understand your design.

chris-rock · 2017-11-23T08:32:32Z

.travis.yml

@@ -40,6 +40,16 @@ matrix:
  - rvm: 2.4.1
    script: bundle exec rake $SUITE
    env: SUITE="test:docker config=test-travis-ubuntu.yml"
+  - rvm: 2.4.1


Nice to have the integration tests included. That is great!

chris-rock · 2017-11-23T08:33:09Z

test/integration/kubernetes_test.rb

@@ -0,0 +1,152 @@
+# encoding: utf-8


Can we move that file to test/integration/k8s/test.rb?

Sure, this was used to match with the existing Docker tests.

chris-rock · 2017-11-23T08:33:31Z

test/integration/kubernetes_test.rb

@@ -0,0 +1,152 @@
+# encoding: utf-8
+#
+# Copyright 2017, Noah Kantrowitz


Please change that to Author.

chris-rock · 2017-11-23T08:35:20Z

test/integration/helper.rb

@@ -31,7 +31,7 @@ def root_group(os)
    end

    def selinux_label(backend, path = nil)
-      return nil if backend.class.to_s =~ /docker/i
+      return nil if backend.class.to_s =~ /docker|kubernetes/i


We are preparing inspec/inspec#1661 in Train and InSpec. kubernetes will become a platform in train. At this point, we are adding a k8s transport for the execution inside of containers only. We think k8s-exec is a better name for this specific implementation.

I don't understand, that PR says "platform" would be things like RedHat or Ubuntu, in which case Kubernetes will not be a platform. The OS image used in the underlying container would be the platform, just like with Docker.

chris-rock · 2017-11-23T08:37:12Z

lib/train/transports/kubernetes.rb

+      end
+
+      def run_command(cmd)
+        kubectl_cmd = [options[:kubectl_path], 'exec']


This implementation relies on shelling out to kubectl rather than using the API directly. This limits the usage and decreases the usability of train/inspec. Is there any reason why you decided against using https://github.com/abonas/kubeclient directly?

A couple of reasons:

Stability, the APIs change a lot and kubectl/client-go will always be the first class citizen in the ecosystem.

Performance, Ruby network perf is not great and because we're using it as the file transport system too it would be annoying.

Support, Kubeclient (and pretty much all other client libs) don't actually implement the exec API. Most libraries just do the REST API, which here wouldn't actually help with anything. The exec API is weirdly undocumented but it initiates over an HTTP POST and then upgrades to SPDY (which will be replaced with websockets soon). Implementing a client for this in Ruby would be very annoying.

chris-rock · 2017-11-23T08:38:35Z

lib/train/transports/kubernetes.rb

@@ -0,0 +1,72 @@
+# encoding: utf-8
+#
+# Copyright 2017, Noah Kantrowitz


Please change to author

Copyright is correct as I remain the owner of this code. Unless you're requesting I assign copyright to Chef Software, which would be unusual?

chris-rock · 2017-11-23T08:40:57Z

test/integration/tests/path_folder_test.rb

@@ -76,7 +72,11 @@

    it 'has selinux label handling' do
      res = Test.selinux_label(backend, file.path)
-      file.selinux_label.must_equal(res)
+      if res.nil?


Why do you expect that is a result from a method is nil, that this is okay?

As noted above, you already returned nil here during the Docker tests, and I extended that to also return nil on Kubernetes tests. You weren't actually handling that in most of the tests so I fixed that while making the Kubernetes tests pass.

* Add cache connection to train * Update caching to be on BaseConnection * Refactor tests and cleanup logic * Move caching into base_connection and privatize connection calls * Update mock transport to use base caching. * Add author tag and caching comments. * Added exception for invalid cache type and small logic refactor. Signed-off-by: Jared Quick <jquick@chef.io>

aaronlippold · 2018-08-16T02:05:09Z

How would this connect to openShift as well? Would we want a individual openshift or oc:// transport or would it be a "type" of k8r transport do you think?

frezbo · 2018-08-16T05:36:58Z

@aaronlippold as OC still uses the same k8s API server, it think it would be ok to use the same transport that k8s uses. Maybe in inspec we could alias k8s transport to oc.

chris-rock · 2018-08-16T07:13:37Z

I agree with @frezbo here, we should keep it to plain k8s. I also do not recommend to use an alias since this may confuses the community. The confusion may come from the fact that this implementation uses kubectl. I would like to avoid that and instead talk to the k8s api directly. Then we are talking about API compatible solutions not binary compatible solutions

frezbo · 2018-08-16T07:16:19Z

+1 on the usage of API. I hope we use this: https://github.com/kubernetes-client/ruby seems there are a lot of k8s ruby API's

chris-rock · 2018-08-16T07:17:45Z

That is a great reference @frezbo I missed that we have near official clients for ruby now

coderanger · 2018-08-16T12:34:30Z

No, it's not a great a reference, that's an abandoned project from 8 months ago. And it doesn't expose the pod execution API because almost no clients do. I explained why I used kubectl last year and all the same reasons apply.

frezbo · 2018-08-16T12:39:26Z

That's sad. I really hope all other API's gets the same love as go-client.

coderanger · 2018-08-16T12:41:31Z

Maybe in a long long time when the API stabilizes more fully. Until then, that seems highly unlikely.

coderanger · 2018-08-16T12:42:49Z

As for the original question, the oc command is mostly just an extension to kubectl so to make that work we would just need to set kubectl_path: 'oc'.

clintoncwolfe · 2018-10-25T19:26:03Z

So, I thought I'd chime in here.

This PR is about creating a Kubernetes transport to talk to a running container within a pod, which is absolutely a great idea, and very much wanted. The behavior is very well specified through integration tests.

It was submitted prior to our efforts to make Train plugins more practical; today we'd shuffle this into a plugin, in a separate repo with its own lifecycle and distributed as an independent gem.

Regarding implementation. From my perspective, we have two concerns:

A desire to use the API vs driving kubectl. For my part, I don't much care, so long as the user never need know ( though a requirement for having kubectl available is reasonable). But I don't think we should change the target URI schema based on the implementation details; I'm against using something like kubectl://. If/when a solid API and Ruby library becomes available, the plugin could internally refactor to rely on it, and no targets need change.
I do however have concerns about how to model auditing the internals of a container (ie, the exec use case, which is interested in files and commands, and would target one container at a time) and auditing the cluster as a whole. You'd want to be able to examine things like what services are available, networking, etc. Our train design today can't support talking to more than one target (ie, you're either talking to one container, or you're talking to one API endpoint). So, perhaps there are two Train plugins here - one for talking to the internals of containers, and one for auditing the cluster. Naming things is hard; we could use train-k8s for the cluster, and train-k8s-container for the use case exemplified here. We currently have a 1:1 coupling between plugin names and target URIs, so those would be used as -t k8s://... to target a cluster and -t k8s-containter://... to examine inside a container.

Thoughts?

clintoncwolfe · 2019-03-15T14:32:11Z

We will one day get around to adding a kubernetes transport; it will likely be a separate plugin, however, and may be "chimeric" (internally using multiple means to answer questions - perhaps both kubectl and some ruby api (a cursory glance indicates that https://github.com/kubernetes-client/ruby appears to be active as of right now)).

This is excellent work, and when a kube plugin is started in earnest, we'll certainly refer to this (with attribution and copyright if we're outright copying).

For now though, this PR isn't likely to ever be merged into train (and train has 18 months of growth since this was last touched) so, I'm going to close this. It's linked from the issue, so it won't get misplaced.

bgeesaman · 2019-05-23T17:43:01Z

https://github.com/bgeesaman/inspec-k8s which relies on https://github.com/bgeesaman/train-kubernetes was recently released. train-k8s was already reserved on rubygems, so train-kubernetes attempts to permit assertions on Kubernetes API objects and their specs (the "k8s" transport mentioned by @clintoncwolfe above). It does not implement kubectl exec like functionality like this PR does (the "k8s-container" transport mentioned above). Feedback is welcomed.

coderanger added 3 commits November 5, 2017 22:31

Add a Kubernetes transport.

cdefb51

This uses kubectl exec internally. Windows pods are not supported at this time, mostly because I have no way to test them. Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

Check the family at example run time, rather than statically.

992e433

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

Some tweaks to SELinux behavior for Kubernetes.

6f7ef7a

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

coderanger requested a review from a team November 6, 2017 06:33

coderanger force-pushed the kubernetes branch from 8cd24d7 to 6f7ef7a Compare November 6, 2017 06:34

Feed the Rubocop.

0479e95

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

coderanger mentioned this pull request Nov 6, 2017

Adds support for kitchen-kubernetes. inspec/kitchen-inspec#148

Closed

chris-rock suggested changes Nov 23, 2017

View reviewed changes

clintoncwolfe mentioned this pull request Oct 25, 2018

Add support for kubernetes transport #354

Open

clintoncwolfe closed this Mar 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubernetes transport #205

Kubernetes transport #205

coderanger commented Nov 6, 2017

adamleff commented Nov 6, 2017

chris-rock left a comment

chris-rock Nov 23, 2017

chris-rock Nov 23, 2017

coderanger Nov 23, 2017

chris-rock Nov 23, 2017

chris-rock Nov 23, 2017

coderanger Nov 23, 2017

chris-rock Nov 23, 2017

coderanger Nov 23, 2017

chris-rock Nov 23, 2017

coderanger Nov 23, 2017

chris-rock Nov 23, 2017

coderanger Nov 23, 2017

aaronlippold commented Aug 16, 2018

frezbo commented Aug 16, 2018

chris-rock commented Aug 16, 2018

frezbo commented Aug 16, 2018

chris-rock commented Aug 16, 2018

coderanger commented Aug 16, 2018

frezbo commented Aug 16, 2018

coderanger commented Aug 16, 2018

coderanger commented Aug 16, 2018

clintoncwolfe commented Oct 25, 2018

clintoncwolfe commented Mar 15, 2019

bgeesaman commented May 23, 2019

Kubernetes transport #205

Kubernetes transport #205

Conversation

coderanger commented Nov 6, 2017

adamleff commented Nov 6, 2017

chris-rock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronlippold commented Aug 16, 2018

frezbo commented Aug 16, 2018

chris-rock commented Aug 16, 2018

frezbo commented Aug 16, 2018

chris-rock commented Aug 16, 2018

coderanger commented Aug 16, 2018

frezbo commented Aug 16, 2018

coderanger commented Aug 16, 2018

coderanger commented Aug 16, 2018

clintoncwolfe commented Oct 25, 2018

clintoncwolfe commented Mar 15, 2019

bgeesaman commented May 23, 2019