Merge branch 'extract_netpol_common_info' into 'master'

Extract common information from NetworkPolicy

See merge request gitlab-org/gitlab!38003

ee/spec/controllers/projects/security/network_policies_controller_spec.rb

@@ -147,7 +147,7 @@ RSpec.describe Projects::Security::NetworkPoliciesController do
         Gitlab::Kubernetes::NetworkPolicy.new(
           name: 'policy',
           namespace: 'another',
-          pod_selector: { matchLabels: { role: 'db' } },
+          selector: { matchLabels: { role: 'db' } },
           ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
         )
       end
@@ -184,7 +184,7 @@ RSpec.describe Projects::Security::NetworkPoliciesController do
       Gitlab::Kubernetes::NetworkPolicy.new(
         name: 'policy',
         namespace: 'another',
-        pod_selector: { matchLabels: { role: 'db' } },
+        selector: { matchLabels: { role: 'db' } },
         ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
       )
     end
@@ -248,7 +248,7 @@ RSpec.describe Projects::Security::NetworkPoliciesController do
       Gitlab::Kubernetes::NetworkPolicy.new(
         name: 'policy',
         namespace: 'another',
-        pod_selector: { matchLabels: { role: 'db' } },
+        selector: { matchLabels: { role: 'db' } },
         ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
       )
     end

ee/spec/services/network_policies/deploy_resource_service_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe NetworkPolicies::DeployResourceService do
     Gitlab::Kubernetes::NetworkPolicy.new(
       name: 'policy',
       namespace: 'another',
-      pod_selector: { matchLabels: { role: 'db' } },
+      selector: { matchLabels: { role: 'db' } },
       ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
     )
   end

ee/spec/services/network_policies/resources_service_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe NetworkPolicies::ResourcesService do
     Gitlab::Kubernetes::NetworkPolicy.new(
       name: 'policy',
       namespace: 'another',
-      pod_selector: { matchLabels: { role: 'db' } },
+      selector: { matchLabels: { role: 'db' } },
       ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
     )
   end

lib/gitlab/kubernetes/network_policy.rb

@@ -3,14 +3,15 @@
 module Gitlab
   module Kubernetes
     class NetworkPolicy
-      DISABLED_BY_LABEL = :'network-policy.gitlab.com/disabled_by'
+      include NetworkPolicyCommon
+      extend ::Gitlab::Utils::Override
 
-      def initialize(name:, namespace:, pod_selector:, ingress:, labels: nil, creation_timestamp: nil, policy_types: ["Ingress"], egress: nil)
+      def initialize(name:, namespace:, selector:, ingress:, labels: nil, creation_timestamp: nil, policy_types: ["Ingress"], egress: nil)
         @name = name
         @namespace = namespace
         @labels = labels
         @creation_timestamp = creation_timestamp
-        @pod_selector = pod_selector
+        @selector = selector
         @policy_types = policy_types
         @ingress = ingress
         @egress = egress
@@ -28,7 +29,7 @@ module Gitlab
           name: metadata[:name],
           namespace: metadata[:namespace],
           labels: metadata[:labels],
-          pod_selector: spec[:podSelector],
+          selector: spec[:podSelector],
           policy_types: spec[:policyTypes],
           ingress: spec[:ingress],
           egress: spec[:egress]
@@ -48,81 +49,30 @@ module Gitlab
           namespace: metadata[:namespace],
           labels: metadata[:labels]&.to_h,
           creation_timestamp: metadata[:creationTimestamp],
-          pod_selector: spec[:podSelector],
+          selector: spec[:podSelector],
           policy_types: spec[:policyTypes],
           ingress: spec[:ingress],
           egress: spec[:egress]
         )
       end
 
-      def generate
-        ::Kubeclient::Resource.new.tap do |resource|
-          resource.metadata = metadata
-          resource.spec = spec
-        end
-      end
-
-      def as_json(opts = nil)
-        {
-          name: name,
-          namespace: namespace,
-          creation_timestamp: creation_timestamp,
-          manifest: manifest,
-          is_autodevops: autodevops?,
-          is_enabled: enabled?
-        }
-      end
-
-      def autodevops?
-        return false unless labels
-
-        !labels[:chart].nil? && labels[:chart].start_with?('auto-deploy-app-')
-      end
-
-      # podSelector selects pods that should be targeted by this
-      # policy. We can narrow selection by requiring this policy to
-      # match our custom labels. Since DISABLED_BY label will not be
-      # on any pod a policy will be effectively disabled.
-      def enabled?
-        return true unless pod_selector&.key?(:matchLabels)
-
-        !pod_selector[:matchLabels]&.key?(DISABLED_BY_LABEL)
-      end
-
-      def enable
-        return if enabled?
-
-        pod_selector[:matchLabels].delete(DISABLED_BY_LABEL)
-      end
-
-      def disable
-        @pod_selector ||= {}
-        pod_selector[:matchLabels] ||= {}
-        pod_selector[:matchLabels].merge!(DISABLED_BY_LABEL => 'gitlab')
-      end
-
       private
 
-      attr_reader :name, :namespace, :labels, :creation_timestamp, :pod_selector, :policy_types, :ingress, :egress
+      attr_reader :name, :namespace, :labels, :creation_timestamp, :policy_types, :ingress, :egress
 
-      def metadata
-        meta = { name: name, namespace: namespace }
-        meta[:labels] = labels if labels
-        meta
+      def selector
+        @selector ||= {}
       end
 
+      override :spec
       def spec
         {
-          podSelector: pod_selector,
+          podSelector: selector,
           policyTypes: policy_types,
           ingress: ingress,
           egress: egress
         }
       end
-
-      def manifest
-        YAML.dump({ metadata: metadata, spec: spec }.deep_stringify_keys)
-      end
     end
   end
 end

lib/gitlab/kubernetes/network_policy_common.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Kubernetes
+    module NetworkPolicyCommon
+      DISABLED_BY_LABEL = :'network-policy.gitlab.com/disabled_by'
+
+      def generate
+        ::Kubeclient::Resource.new.tap do |resource|
+          resource.metadata = metadata
+          resource.spec = spec
+        end
+      end
+
+      def as_json(opts = nil)
+        {
+          name: name,
+          namespace: namespace,
+          creation_timestamp: creation_timestamp,
+          manifest: manifest,
+          is_autodevops: autodevops?,
+          is_enabled: enabled?
+        }
+      end
+
+      def autodevops?
+        return false unless labels
+
+        !labels[:chart].nil? && labels[:chart].start_with?('auto-deploy-app-')
+      end
+
+      # selector selects pods that should be targeted by this
+      # policy. It can represent podSelector, nodeSelector or
+      # endpointSelector  We can narrow selection by requiring
+      # this policy to match our custom labels. Since DISABLED_BY
+      # label will not be on any pod a policy will be effectively disabled.
+      def enabled?
+        return true unless selector&.key?(:matchLabels)
+
+        !selector[:matchLabels]&.key?(DISABLED_BY_LABEL)
+      end
+
+      def enable
+        return if enabled?
+
+        selector[:matchLabels].delete(DISABLED_BY_LABEL)
+      end
+
+      def disable
+        selector[:matchLabels] ||= {}
+        selector[:matchLabels].merge!(DISABLED_BY_LABEL => 'gitlab')
+      end
+
+      private
+
+      def metadata
+        meta = { name: name, namespace: namespace }
+        meta[:labels] = labels if labels
+        meta
+      end
+
+      def spec
+        raise NotImplementedError
+      end
+
+      def manifest
+        YAML.dump({ metadata: metadata, spec: spec }.deep_stringify_keys)
+      end
+    end
+  end
+end

spec/lib/gitlab/kubernetes/network_policy_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Gitlab::Kubernetes::NetworkPolicy do
       name: name,
       namespace: namespace,
       creation_timestamp: '2020-04-14T00:08:30Z',
-      pod_selector: pod_selector,
+      selector: pod_selector,
       policy_types: %w(Ingress Egress),
       ingress: ingress,
       egress: egress
@@ -37,6 +37,20 @@ RSpec.describe Gitlab::Kubernetes::NetworkPolicy do
     ]
   end
 
+  include_examples 'network policy common specs' do
+    let(:selector) { pod_selector }
+    let(:policy) do
+      described_class.new(
+        name: name,
+        namespace: namespace,
+        selector: selector,
+        ingress: ingress,
+        labels: labels
+      )
+    end
+    let(:spec) { { podSelector: selector, policyTypes: ["Ingress"], ingress: ingress, egress: nil } }
+  end
+
   describe '.from_yaml' do
     let(:manifest) do
       <<~POLICY
@@ -193,202 +207,4 @@ RSpec.describe Gitlab::Kubernetes::NetworkPolicy do
       it { is_expected.to be_nil }
     end
   end
-
-  describe '#generate' do
-    let(:resource) do
-      ::Kubeclient::Resource.new(
-        metadata: { name: name, namespace: namespace },
-        spec: { podSelector: pod_selector, policyTypes: %w(Ingress Egress), ingress: ingress, egress: egress }
-      )
-    end
-
-    subject { policy.generate }
-
-    it { is_expected.to eq(resource) }
-  end
-
-  describe '#as_json' do
-    let(:json_policy) do
-      {
-        name: name,
-        namespace: namespace,
-        creation_timestamp: '2020-04-14T00:08:30Z',
-        manifest: YAML.dump(
-          {
-            metadata: { name: name, namespace: namespace },
-            spec: { podSelector: pod_selector, policyTypes: %w(Ingress Egress), ingress: ingress, egress: egress }
-          }.deep_stringify_keys
-        ),
-        is_autodevops: false,
-        is_enabled: true
-      }
-    end
-
-    subject { policy.as_json }
-
-    it { is_expected.to eq(json_policy) }
-  end
-
-  describe '#autodevops?' do
-    subject { policy.autodevops? }
-
-    let(:chart) { nil }
-    let(:policy) do
-      described_class.new(
-        name: name,
-        namespace: namespace,
-        labels: { chart: chart },
-        pod_selector: pod_selector,
-        ingress: ingress
-      )
-    end
-
-    it { is_expected.to be false }
-
-    context 'with non-autodevops chart' do
-      let(:chart) { 'foo' }
-
-      it { is_expected.to be false }
-    end
-
-    context 'with autodevops chart' do
-      let(:chart) { 'auto-deploy-app-0.6.0' }
-
-      it { is_expected.to be true }
-    end
-  end
-
-  describe '#enabled?' do
-    subject { policy.enabled? }
-
-    let(:pod_selector) { nil }
-    let(:policy) do
-      described_class.new(
-        name: name,
-        namespace: namespace,
-        pod_selector: pod_selector,
-        ingress: ingress
-      )
-    end
-
-    it { is_expected.to be true }
-
-    context 'with empty pod_selector' do
-      let(:pod_selector) { {} }
-
-      it { is_expected.to be true }
-    end
-
-    context 'with nil matchLabels in pod_selector' do
-      let(:pod_selector) { { matchLabels: nil } }
-
-      it { is_expected.to be true }
-    end
-
-    context 'with empty matchLabels in pod_selector' do
-      let(:pod_selector) { { matchLabels: {} } }
-
-      it { is_expected.to be true }
-    end
-
-    context 'with disabled_by label in matchLabels in pod_selector' do
-      let(:pod_selector) do
-        { matchLabels: { Gitlab::Kubernetes::NetworkPolicy::DISABLED_BY_LABEL => 'gitlab' } }
-      end
-
-      it { is_expected.to be false }
-    end
-  end
-
-  describe '#enable' do
-    subject { policy.enabled? }
-
-    let(:pod_selector) { nil }
-    let(:policy) do
-      described_class.new(
-        name: name,
-        namespace: namespace,
-        pod_selector: pod_selector,
-        ingress: ingress
-      )
-    end
-
-    before do
-      policy.enable
-    end
-
-    it { is_expected.to be true }
-
-    context 'with empty pod_selector' do
-      let(:pod_selector) { {} }
-
-      it { is_expected.to be true }
-    end
-
-    context 'with nil matchLabels in pod_selector' do
-      let(:pod_selector) { { matchLabels: nil } }
-
-      it { is_expected.to be true }
-    end
-
-    context 'with empty matchLabels in pod_selector' do
-      let(:pod_selector) { { matchLabels: {} } }
-
-      it { is_expected.to be true }
-    end
-
-    context 'with disabled_by label in matchLabels in pod_selector' do
-      let(:pod_selector) do
-        { matchLabels: { Gitlab::Kubernetes::NetworkPolicy::DISABLED_BY_LABEL => 'gitlab' } }
-      end
-
-      it { is_expected.to be true }
-    end
-  end
-
-  describe '#disable' do
-    subject { policy.enabled? }
-
-    let(:pod_selector) { nil }
-    let(:policy) do
-      described_class.new(
-        name: name,
-        namespace: namespace,
-        pod_selector: pod_selector,
-        ingress: ingress
-      )
-    end
-
-    before do
-      policy.disable
-    end
-
-    it { is_expected.to be false }
-
-    context 'with empty pod_selector' do
-      let(:pod_selector) { {} }
-
-      it { is_expected.to be false }
-    end
-
-    context 'with nil matchLabels in pod_selector' do
-      let(:pod_selector) { { matchLabels: nil } }
-
-      it { is_expected.to be false }
-    end
-
-    context 'with empty matchLabels in pod_selector' do
-      let(:pod_selector) { { matchLabels: {} } }
-
-      it { is_expected.to be false }
-    end
-
-    context 'with disabled_by label in matchLabels in pod_selector' do
-      let(:pod_selector) do
-        { matchLabels: { Gitlab::Kubernetes::NetworkPolicy::DISABLED_BY_LABEL => 'gitlab' } }
-      end
-
-      it { is_expected.to be false }
-    end
-  end
 end

spec/support/shared_examples/lib/gitlab/kubernetes/network_policy_common_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'network policy common specs' do
+  let(:name) { 'example-name' }
+  let(:namespace) { 'example-namespace' }
+  let(:labels) { nil }
+
+  describe 'generate' do
+    let(:resource) do
+      ::Kubeclient::Resource.new(
+        metadata: { name: name, namespace: namespace },
+        spec: spec
+      )
+    end
+
+    subject { policy.generate }
+
+    it { is_expected.to eq(resource) }
+  end
+
+  describe 'as_json' do
+    let(:json_policy) do
+      {
+        name: name,
+        namespace: namespace,
+        creation_timestamp: nil,
+        manifest: YAML.dump(
+          {
+            metadata: { name: name, namespace: namespace },
+            spec: spec
+          }.deep_stringify_keys
+        ),
+        is_autodevops: false,
+        is_enabled: true
+      }
+    end
+
+    subject { policy.as_json }
+
+    it { is_expected.to eq(json_policy) }
+  end
+
+  describe 'autodevops?' do
+    subject { policy.autodevops? }
+
+    let(:labels) { { chart: chart } }
+    let(:chart) { nil }
+
+    it { is_expected.to be false }
+
+    context 'with non-autodevops chart' do
+      let(:chart) { 'foo' }
+
+      it { is_expected.to be false }
+    end
+
+    context 'with autodevops chart' do
+      let(:chart) { 'auto-deploy-app-0.6.0' }
+
+      it { is_expected.to be true }
+    end
+  end
+
+  describe 'enabled?' do
+    subject { policy.enabled? }
+
+    let(:selector) { nil }
+
+    it { is_expected.to be true }
+
+    context 'with empty selector' do
+      let(:selector) { {} }
+
+      it { is_expected.to be true }
+    end
+
+    context 'with nil matchLabels in selector' do
+      let(:selector) { { matchLabels: nil } }
+
+      it { is_expected.to be true }
+    end
+
+    context 'with empty matchLabels in selector' do
+      let(:selector) { { matchLabels: {} } }
+
+      it { is_expected.to be true }
+    end
+
+    context 'with disabled_by label in matchLabels in selector' do
+      let(:selector) do
+        { matchLabels: { Gitlab::Kubernetes::NetworkPolicyCommon::DISABLED_BY_LABEL => 'gitlab' } }
+      end
+
+      it { is_expected.to be false }
+    end
+  end
+
+  describe 'enable' do
+    subject { policy.enabled? }
+
+    let(:selector) { nil }
+
+    before do
+      policy.enable
+    end
+
+    it { is_expected.to be true }
+
+    context 'with empty selector' do
+      let(:selector) { {} }
+
+      it { is_expected.to be true }
+    end
+
+    context 'with nil matchLabels in selector' do
+      let(:selector) { { matchLabels: nil } }
+
+      it { is_expected.to be true }
+    end
+
+    context 'with empty matchLabels in selector' do
+      let(:selector) { { matchLabels: {} } }
+
+      it { is_expected.to be true }
+    end
+
+    context 'with disabled_by label in matchLabels in selector' do
+      let(:selector) do
+        { matchLabels: { Gitlab::Kubernetes::NetworkPolicyCommon::DISABLED_BY_LABEL => 'gitlab' } }
+      end
+
+      it { is_expected.to be true }
+    end
+  end
+
+  describe 'disable' do
+    subject { policy.enabled? }
+
+    let(:selector) { nil }
+
+    before do
+      policy.disable
+    end
+
+    it { is_expected.to be false }
+
+    context 'with empty selector' do
+      let(:selector) { {} }
+
+      it { is_expected.to be false }
+    end
+
+    context 'with nil matchLabels in selector' do
+      let(:selector) { { matchLabels: nil } }
+
+      it { is_expected.to be false }
+    end
+
+    context 'with empty matchLabels in selector' do
+      let(:selector) { { matchLabels: {} } }
+
+      it { is_expected.to be false }
+    end
+
+    context 'with disabled_by label in matchLabels in selector' do
+      let(:selector) do
+        { matchLabels: { Gitlab::Kubernetes::NetworkPolicyCommon::DISABLED_BY_LABEL => 'gitlab' } }
+      end
+
+      it { is_expected.to be false }
+    end
+  end
+end