Switch namespaced deploy to pruning blacklist (#616)

* Switch namespaced deploy to pruning blacklist
* Add an overide list and tests

lib/krane/cluster_resource_discovery.rb

@@ -16,41 +16,19 @@ def crds
       end
     end
 
-    def global_resource_kinds
-      @globals ||= fetch_resources(namespaced: false).map { |g| g["kind"] }
-    end
-
     def prunable_resources(namespaced:)
       black_list = %w(Namespace Node)
       api_versions = fetch_api_versions
 
       fetch_resources(namespaced: namespaced).map do |resource|
         next unless resource['verbs'].one? { |v| v == "delete" }
         next if black_list.include?(resource['kind'])
-        version = api_versions[resource['apigroup'].to_s].last
+        group_versions = api_versions[resource['apigroup'].to_s]
+        version = version_for_kind(group_versions, resource['kind'])
         [resource['apigroup'], version, resource['kind']].compact.join("/")
       end.compact
     end
 
-    private
-
-    # kubectl api-versions returns a list of group/version strings e.g. autoscaling/v2beta2
-    # A kind may not exist in all versions of the group.
-    def fetch_api_versions
-      raw, _, st = kubectl.run("api-versions", attempts: 5, use_namespace: false)
-      # The "core" group is represented by an empty string
-      versions = { "" => %w(v1) }
-      if st.success?
-        rows = raw.split("\n")
-        rows.each do |group_version|
-          group, version = group_version.split("/")
-          versions[group] ||= []
-          versions[group] << version
-        end
-      end
-      versions
-    end
-
     # kubectl api-resources -o wide returns 5 columns
     # NAME SHORTNAMES APIGROUP NAMESPACED KIND VERBS
     # SHORTNAMES and APIGROUP may be blank
@@ -85,6 +63,39 @@ def fetch_resources(namespaced: false)
       end
     end
 
+    private
+
+    # kubectl api-versions returns a list of group/version strings e.g. autoscaling/v2beta2
+    # A kind may not exist in all versions of the group.
+    def fetch_api_versions
+      raw, _, st = kubectl.run("api-versions", attempts: 5, use_namespace: false)
+      # The "core" group is represented by an empty string
+      versions = { "" => %w(v1) }
+      if st.success?
+        rows = raw.split("\n")
+        rows.each do |group_version|
+          group, version = group_version.split("/")
+          versions[group] ||= []
+          versions[group] << version
+        end
+      end
+      versions
+    end
+
+    def version_for_kind(versions, kind)
+      # Override list for kinds that don't appear in the lastest version of a group
+      version_override = { "CronJob" => "v1beta1", "VolumeAttachment" => "v1beta1",
+                           "CSIDriver" => "v1beta1", "Ingress" => "v1beta1", "CSINode" => "v1beta1" }
+
+      pattern = /v(?<major>\d+)(?<pre>alpha|beta)?(?<minor>\d+)?/
+      latest = versions.sort_by do |version|
+        match = version.match(pattern)
+        pre = { "alpha" => 0, "beta" => 1, nil => 2 }.fetch(match[:pre])
+        [match[:major].to_i, pre, match[:minor].to_i]
+      end.last
+      version_override.fetch(kind, latest)
+    end
+
     def fetch_crds
       raw_json, _, st = kubectl.run("get", "CustomResourceDefinition", output: "json", attempts: 5,
         use_namespace: false)

lib/krane/deploy_task.rb

@@ -8,5 +8,9 @@ def initialize(**args)
       raise "Use Krane::DeployGlobalTask to deploy global resources" if args[:allow_globals]
       super(args.merge(allow_globals: false))
     end
+
+    def prune_whitelist
+      cluster_resource_discoverer.prunable_resources(namespaced: true)
+    end
   end
 end

lib/krane/task_config.rb

@@ -15,7 +15,7 @@ def initialize(context, namespace, logger = nil)
     def global_kinds
       @global_kinds ||= begin
         cluster_resource_discoverer = ClusterResourceDiscovery.new(task_config: self)
-        cluster_resource_discoverer.global_resource_kinds
+        cluster_resource_discoverer.fetch_resources(namespaced: false).map { |g| g["kind"] }
       end
     end
   end

test/fixtures/for_unit_tests/api_resources_namespaced.txt

@@ -0,0 +1,31 @@
+NAME                        SHORTNAMES   APIGROUP                    NAMESPACED   KIND                       VERBS
+bindings                                                             true         Binding                    [create]
+configmaps                  cm                                       true         ConfigMap                  [create delete deletecollection get list patch update watch]
+endpoints                   ep                                       true         Endpoints                  [create delete deletecollection get list patch update watch]
+events                      ev                                       true         Event                      [create delete deletecollection get list patch update watch]
+limitranges                 limits                                   true         LimitRange                 [create delete deletecollection get list patch update watch]
+persistentvolumeclaims      pvc                                      true         PersistentVolumeClaim      [create delete deletecollection get list patch update watch]
+pods                        po                                       true         Pod                        [create delete deletecollection get list patch update watch]
+podtemplates                                                         true         PodTemplate                [create delete deletecollection get list patch update watch]
+replicationcontrollers      rc                                       true         ReplicationController      [create delete deletecollection get list patch update watch]
+resourcequotas              quota                                    true         ResourceQuota              [create delete deletecollection get list patch update watch]
+secrets                                                              true         Secret                     [create delete deletecollection get list patch update watch]
+serviceaccounts             sa                                       true         ServiceAccount             [create delete deletecollection get list patch update watch]
+services                    svc                                      true         Service                    [create delete get list patch update watch]
+controllerrevisions                      apps                        true         ControllerRevision         [create delete deletecollection get list patch update watch]
+daemonsets                  ds           apps                        true         DaemonSet                  [create delete deletecollection get list patch update watch]
+deployments                 deploy       apps                        true         Deployment                 [create delete deletecollection get list patch update watch]
+replicasets                 rs           apps                        true         ReplicaSet                 [create delete deletecollection get list patch update watch]
+statefulsets                sts          apps                        true         StatefulSet                [create delete deletecollection get list patch update watch]
+localsubjectaccessreviews                authorization.k8s.io        true         LocalSubjectAccessReview   [create]
+horizontalpodautoscalers    hpa          autoscaling                 true         HorizontalPodAutoscaler    [create delete deletecollection get list patch update watch]
+cronjobs                    cj           batch                       true         CronJob                    [create delete deletecollection get list patch update watch]
+jobs                                     batch                       true         Job                        [create delete deletecollection get list patch update watch]
+leases                                   coordination.k8s.io         true         Lease                      [create delete deletecollection get list patch update watch]
+events                      ev           events.k8s.io               true         Event                      [create delete deletecollection get list patch update watch]
+ingresses                   ing          extensions                  true         Ingress                    [create delete deletecollection get list patch update watch]
+ingresses                   ing          networking.k8s.io           true         Ingress                    [create delete deletecollection get list patch update watch]
+networkpolicies             netpol       networking.k8s.io           true         NetworkPolicy              [create delete deletecollection get list patch update watch]
+poddisruptionbudgets        pdb          policy                      true         PodDisruptionBudget        [create delete deletecollection get list patch update watch]
+rolebindings                             rbac.authorization.k8s.io   true         RoleBinding                [create delete deletecollection get list patch update watch]
+roles                                    rbac.authorization.k8s.io   true         Role                       [create delete deletecollection get list patch update watch]

test/helpers/fixture_deploy_helper.rb

@@ -185,8 +185,7 @@ def namespace_globals(fixtures, selector)
     fixtures.each do |_, kinds_map|
       kinds_map.each do |_, resources|
         resources.each do |resource|
-          resource["metadata"]["name"] = (resource["metadata"]["name"] + @namespace)[0..63]
-          resource["metadata"]["name"] += "0" if resource["metadata"]["name"].end_with?("-")
+          resource["metadata"]["name"] = (resource["metadata"]["name"] + Digest::MD5.hexdigest(@namespace))[0..63]
           resource["metadata"]["labels"] ||= {}
           resource["metadata"]["labels"][selector_key] = selector_value
         end

test/unit/cluster_resource_discovery_test.rb

@@ -2,41 +2,62 @@
 require 'test_helper'
 
 class ClusterResourceDiscoveryTest < Krane::TestCase
-  def test_global_resource_kinds_failure
+  def test_fetch_resources_failure
     crd = mocked_cluster_resource_discovery(nil, success: false)
-    kinds = crd.global_resource_kinds
-    assert_equal(kinds, [])
+    resources = crd.fetch_resources
+    assert_equal(resources, [])
   end
 
-  def test_global_resource_kinds_success
-    crd = mocked_cluster_resource_discovery(api_resources_full_response)
-    kinds = crd.global_resource_kinds
-    assert_equal(kinds.length, api_resources_full_response.split("\n").length - 1)
+  def test_fetch_resources_not_namespaced
+    crd = mocked_cluster_resource_discovery(api_resources_not_namespaced_full_response)
+    kinds = crd.fetch_resources(namespaced: false).map { |r| r['kind'] }
+    assert_equal(kinds.length, api_resources_not_namespaced_full_response.split("\n").length - 1)
     %w(MutatingWebhookConfiguration ComponentStatus CustomResourceDefinition).each do |kind|
       assert_includes(kinds, kind)
     end
   end
 
-  def test_prunable_resources
+  def test_fetch_resources_namespaced
+    crd = mocked_cluster_resource_discovery(api_resources_namespaced_full_response, namespaced: true)
+    kinds = crd.fetch_resources(namespaced: true).map { |r| r['kind'] }
+    assert_equal(kinds.length, api_resources_namespaced_full_response.split("\n").length - 1)
+    %w(ConfigMap CronJob Deployment).each do |kind|
+      assert_includes(kinds, kind)
+    end
+  end
+
+  def test_prunable_global_resources
     Krane::Kubectl.any_instance.stubs(:run).with("api-versions", attempts: 5, use_namespace: false)
       .returns([api_versions_full_response, "", stub(success?: true)])
-    crd = mocked_cluster_resource_discovery(api_resources_full_response)
+    crd = mocked_cluster_resource_discovery(api_resources_not_namespaced_full_response)
     kinds = crd.prunable_resources(namespaced: false)
 
     assert_equal(kinds.length, 13)
-    %w(scheduling.k8s.io/v1beta1/PriorityClass storage.k8s.io/v1beta1/StorageClass).each do |kind|
-      assert_includes(kinds, kind)
+    %w(PriorityClass StorageClass).each do |expected_kind|
+      assert kinds.one? { |k| k.include?(expected_kind) }
     end
     %w(node namespace).each do |black_lised_kind|
       assert_empty kinds.select { |k| k.downcase.include?(black_lised_kind) }
     end
   end
 
+  def test_prunable_namespaced_resources
+    Krane::Kubectl.any_instance.stubs(:run).with("api-versions", attempts: 5, use_namespace: false)
+      .returns([api_versions_full_response, "", stub(success?: true)])
+    crd = mocked_cluster_resource_discovery(api_resources_namespaced_full_response, namespaced: true)
+    kinds = crd.prunable_resources(namespaced: true)
+
+    assert_equal(kinds.length, 28)
+    %w(ConfigMap CronJob Deployment).each do |expected_kind|
+      assert kinds.one? { |k| k.include?(expected_kind) }
+    end
+  end
+
   private
 
-  def mocked_cluster_resource_discovery(response, success: true)
+  def mocked_cluster_resource_discovery(response, success: true, namespaced: false)
     Krane::Kubectl.any_instance.stubs(:run)
-      .with("api-resources", "--namespaced=false", attempts: 5, use_namespace: false, output: "wide")
+      .with("api-resources", "--namespaced=#{namespaced}", attempts: 5, use_namespace: false, output: "wide")
       .returns([response, "", stub(success?: success)])
     Krane::ClusterResourceDiscovery.new(task_config: task_config, namespace_tags: [])
   end
@@ -45,7 +66,11 @@ def api_versions_full_response
     File.read(File.join(fixture_path('for_unit_tests'), "api_versions.txt"))
   end
 
-  def api_resources_full_response
-    File.read(File.join(fixture_path('for_unit_tests'), "api_resources.txt"))
+  def api_resources_namespaced_full_response
+    File.read(File.join(fixture_path('for_unit_tests'), "api_resources_namespaced.txt"))
+  end
+
+  def api_resources_not_namespaced_full_response
+    File.read(File.join(fixture_path('for_unit_tests'), "api_resources_global.txt"))
   end
 end

test/unit/krane/kubernetes_resource/horizontal_pod_autoscaler_test.rb

@@ -4,16 +4,6 @@
 class HorizontalPodAutoscalerTest < Krane::TestCase
   include ResourceCacheTestHelper
 
-  # We can't get integration coverage for HPA right now because the metrics server just isn't reliable enough on our CI
-  def test_hpa_is_whitelisted_for_pruning
-    Krane::Kubectl.any_instance.expects("run")
-      .with("get", "CustomResourceDefinition", output: "json", attempts: 5, use_namespace: false)
-      .returns(['{ "items": [] }', "", SystemExit.new(0)])
-    task = Krane::DeployTask.new(namespace: 'test', context: KubeclientHelper::TEST_CONTEXT,
-      current_sha: 'foo', template_paths: [''], logger: logger)
-    assert(task.prune_whitelist.one? { |whitelisted_type| whitelisted_type.include?("HorizontalPodAutoscaler") })
-  end
-
   def test_hpa_succeeds_when_scaling_is_active
     conditions = [{
       "lastTransitionTime" => 5.seconds.ago,