Merge branch 'cnp-description' into 'master'

Improve support for description field on CiliumNetworkPolicy

See merge request gitlab-org/gitlab!41722

changelogs/unreleased/cnp-description.yml

+---
+title: Improve support for description field on CiliumNetworkPolicy
+merge_request: 41722
+author:
+type: changed

lib/gitlab/kubernetes/cilium_network_policy.rb

@@ -9,8 +9,12 @@ module Gitlab
       API_VERSION = "cilium.io/v2"
       KIND = 'CiliumNetworkPolicy'
 
-      def initialize(name:, namespace:, selector:, ingress:, resource_version: nil, labels: nil, creation_timestamp: nil, egress: nil)
+      # We are modeling existing kubernetes resource and don't have
+      # control over amount of parameters.
+      # rubocop:disable Metrics/ParameterLists
+      def initialize(name:, namespace:, selector:, ingress:, resource_version: nil, description: nil, labels: nil, creation_timestamp: nil, egress: nil)
         @name = name
+        @description = description
         @namespace = namespace
         @labels = labels
         @creation_timestamp = creation_timestamp
@@ -19,15 +23,7 @@ module Gitlab
         @ingress = ingress
         @egress = egress
       end
-
-      def generate
-        ::Kubeclient::Resource.new.tap do |resource|
-          resource.kind = KIND
-          resource.apiVersion = API_VERSION
-          resource.metadata = metadata
-          resource.spec = spec
-        end
-      end
+      # rubocop:enable Metrics/ParameterLists
 
       def self.from_yaml(manifest)
         return unless manifest
@@ -39,6 +35,7 @@ module Gitlab
         spec = policy[:spec]
         self.new(
           name: metadata[:name],
+          description: policy[:description],
           namespace: metadata[:namespace],
           resource_version: metadata[:resourceVersion],
           labels: metadata[:labels],
@@ -58,6 +55,7 @@ module Gitlab
         spec = resource[:spec].to_h
         self.new(
           name: metadata[:name],
+          description: resource[:description],
           namespace: metadata[:namespace],
           resource_version: metadata[:resourceVersion],
           labels: metadata[:labels]&.to_h,
@@ -68,26 +66,39 @@ module Gitlab
         )
       end
 
+      override :resource
+      def resource
+        resource = {
+          apiVersion: API_VERSION,
+          kind: KIND,
+          metadata: metadata,
+          spec: spec
+        }
+        resource[:description] = description if description
+        resource
+      end
+
       private
 
-      attr_reader :name, :namespace, :labels, :creation_timestamp, :resource_version, :ingress, :egress
+      attr_reader :name, :description, :namespace, :labels, :creation_timestamp, :resource_version, :ingress, :egress
 
       def selector
         @selector ||= {}
       end
 
-      override :spec
+      def metadata
+        meta = { name: name, namespace: namespace }
+        meta[:labels] = labels if labels
+        meta[:resourceVersion] = resource_version if resource_version
+        meta
+      end
+
       def spec
         {
           endpointSelector: selector,
           ingress: ingress,
           egress: egress
-        }.compact!
-      end
-
-      override :kind
-      def kind
-        KIND
+        }.compact
       end
     end
   end

lib/gitlab/kubernetes/network_policy.rb

@@ -19,13 +19,6 @@ module Gitlab
         @egress = egress
       end
 
-      def generate
-        ::Kubeclient::Resource.new.tap do |resource|
-          resource.metadata = metadata
-          resource.spec = spec
-        end
-      end
-
       def self.from_yaml(manifest)
         return unless manifest
 
@@ -65,6 +58,15 @@ module Gitlab
         )
       end
 
+      override :resource
+      def resource
+        {
+          kind: KIND,
+          metadata: metadata,
+          spec: spec
+        }
+      end
+
       private
 
       attr_reader :name, :namespace, :labels, :creation_timestamp, :policy_types, :ingress, :egress
@@ -73,7 +75,12 @@ module Gitlab
         @selector ||= {}
       end
 
-      override :spec
+      def metadata
+        meta = { name: name, namespace: namespace }
+        meta[:labels] = labels if labels
+        meta
+      end
+
       def spec
         {
           podSelector: selector,
@@ -82,11 +89,6 @@ module Gitlab
           egress: egress
         }
       end
-
-      override :kind
-      def kind
-        KIND
-      end
     end
   end
 end

lib/gitlab/kubernetes/network_policy_common.rb

@@ -5,6 +5,10 @@ module Gitlab
     module NetworkPolicyCommon
       DISABLED_BY_LABEL = :'network-policy.gitlab.com/disabled_by'
 
+      def generate
+        ::Kubeclient::Resource.new(resource)
+      end
+
       def as_json(opts = nil)
         {
           name: name,
@@ -46,23 +50,12 @@ module Gitlab
 
       private
 
-      def metadata
-        meta = { name: name, namespace: namespace }
-        meta[:labels] = labels if labels
-        meta[:resourceVersion] = resource_version if defined?(resource_version)
-        meta
-      end
-
-      def spec
-        raise NotImplementedError
-      end
-
-      def kind
+      def resource
         raise NotImplementedError
       end
 
       def manifest
-        YAML.dump({ kind: kind, metadata: metadata, spec: spec }.deep_stringify_keys)
+        YAML.dump(resource.deep_stringify_keys)
       end
     end
   end

spec/lib/gitlab/kubernetes/cilium_network_policy_spec.rb

@@ -7,23 +7,27 @@ RSpec.describe Gitlab::Kubernetes::CiliumNetworkPolicy do
     described_class.new(
       name: name,
       namespace: namespace,
-      creation_timestamp: '2020-04-14T00:08:30Z',
-      endpoint_selector: endpoint_selector,
+      description: description,
+      selector: selector,
       ingress: ingress,
       egress: egress,
-      description: description
+      labels: labels,
+      resource_version: resource_version
     )
   end
 
   let(:resource) do
     ::Kubeclient::Resource.new(
-      kind: partial_class_name,
-      apiVersion: "cilium.io/v2",
+      apiVersion: Gitlab::Kubernetes::CiliumNetworkPolicy::API_VERSION,
+      kind: Gitlab::Kubernetes::CiliumNetworkPolicy::KIND,
       metadata: { name: name, namespace: namespace, resourceVersion: resource_version },
-      spec: { endpointSelector: endpoint_selector, ingress: ingress }
+      spec: { endpointSelector: endpoint_selector, ingress: ingress, egress: egress },
+      description: description
     )
   end
 
+  let(:selector) { endpoint_selector }
+  let(:labels) { nil }
   let(:name) { 'example-name' }
   let(:namespace) { 'example-namespace' }
   let(:endpoint_selector) { { matchLabels: { role: 'db' } } }
@@ -48,34 +52,14 @@ RSpec.describe Gitlab::Kubernetes::CiliumNetworkPolicy do
     ]
   end
 
-  include_examples 'network policy common specs' do
-    let(:selector) { endpoint_selector}
-    let(:policy) do
-      described_class.new(
-        name: name,
-        namespace: namespace,
-        selector: selector,
-        ingress: ingress,
-        labels: labels,
-        resource_version: resource_version
-      )
-    end
-
-    let(:spec) { { endpointSelector: selector, ingress: ingress } }
-    let(:metadata) { { name: name, namespace: namespace, resourceVersion: resource_version } }
-  end
-
-  describe '#generate' do
-    subject { policy.generate }
-
-    it { is_expected.to eq(resource) }
-  end
+  include_examples 'network policy common specs'
 
   describe '.from_yaml' do
     let(:manifest) do
       <<~POLICY
         apiVersion: cilium.io/v2
         kind: CiliumNetworkPolicy
+        description: example-description
         metadata:
           name: example-name
           namespace: example-namespace
@@ -88,6 +72,9 @@ RSpec.describe Gitlab::Kubernetes::CiliumNetworkPolicy do
           - fromEndpoints:
             - matchLabels:
                 project: myproject
+          egress:
+          - ports:
+            - port: 5978
       POLICY
     end
 
@@ -167,18 +154,20 @@ RSpec.describe Gitlab::Kubernetes::CiliumNetworkPolicy do
   describe '.from_resource' do
     let(:resource) do
       ::Kubeclient::Resource.new(
+        description: description,
         metadata: {
           name: name, namespace: namespace, creationTimestamp: '2020-04-14T00:08:30Z',
           labels: { app: 'foo' }, resourceVersion: resource_version
         },
-        spec: { endpointSelector: endpoint_selector, ingress: ingress, egress: nil, labels: nil, description: nil }
+        spec: { endpointSelector: endpoint_selector, ingress: ingress, egress: nil, labels: nil }
       )
     end
 
     let(:generated_resource) do
       ::Kubeclient::Resource.new(
-        kind: partial_class_name,
-        apiVersion: "cilium.io/v2",
+        apiVersion: Gitlab::Kubernetes::CiliumNetworkPolicy::API_VERSION,
+        kind: Gitlab::Kubernetes::CiliumNetworkPolicy::KIND,
+        description: description,
         metadata: { name: name, namespace: namespace, resourceVersion: resource_version, labels: { app: 'foo' } },
         spec: { endpointSelector: endpoint_selector, ingress: ingress }
       )
@@ -197,7 +186,7 @@ RSpec.describe Gitlab::Kubernetes::CiliumNetworkPolicy do
     context 'with resource without metadata' do
       let(:resource) do
         ::Kubeclient::Resource.new(
-          spec: { endpointSelector: endpoint_selector, ingress: ingress, egress: nil, labels: nil, description: nil }
+          spec: { endpointSelector: endpoint_selector, ingress: ingress, egress: nil, labels: nil }
         )
       end
 
@@ -214,4 +203,50 @@ RSpec.describe Gitlab::Kubernetes::CiliumNetworkPolicy do
       it { is_expected.to be_nil }
     end
   end
+
+  describe '#resource' do
+    subject { policy.resource }
+
+    let(:resource) do
+      {
+        apiVersion: Gitlab::Kubernetes::CiliumNetworkPolicy::API_VERSION,
+        kind: Gitlab::Kubernetes::CiliumNetworkPolicy::KIND,
+        metadata: { name: name, namespace: namespace, resourceVersion: resource_version },
+        spec: { endpointSelector: endpoint_selector, ingress: ingress, egress: egress },
+        description: description
+      }
+    end
+
+    it { is_expected.to eq(resource) }
+
+    context 'with labels' do
+      let(:labels) { { app: 'foo' } }
+
+      before do
+        resource[:metadata][:labels] = { app: 'foo' }
+      end
+
+      it { is_expected.to eq(resource) }
+    end
+
+    context 'without resource_version' do
+      let(:resource_version) { nil }
+
+      before do
+        resource[:metadata].delete(:resourceVersion)
+      end
+
+      it { is_expected.to eq(resource) }
+    end
+
+    context 'with nil egress' do
+      let(:egress) { nil }
+
+      before do
+        resource[:spec].delete(:egress)
+      end
+
+      it { is_expected.to eq(resource) }
+    end
+  end
 end

spec/lib/gitlab/kubernetes/network_policy_spec.rb

@@ -7,21 +7,22 @@ RSpec.describe Gitlab::Kubernetes::NetworkPolicy do
     described_class.new(
       name: name,
       namespace: namespace,
-      creation_timestamp: '2020-04-14T00:08:30Z',
-      selector: pod_selector,
-      policy_types: %w(Ingress Egress),
+      selector: selector,
       ingress: ingress,
-      egress: egress
+      labels: labels
     )
   end
 
   let(:resource) do
     ::Kubeclient::Resource.new(
+      kind: Gitlab::Kubernetes::NetworkPolicy::KIND,
       metadata: { name: name, namespace: namespace },
       spec: { podSelector: pod_selector, policyTypes: %w(Ingress), ingress: ingress, egress: nil }
     )
   end
 
+  let(:selector) { pod_selector }
+  let(:labels) { nil }
   let(:name) { 'example-name' }
   let(:namespace) { 'example-namespace' }
   let(:pod_selector) { { matchLabels: { role: 'db' } } }
@@ -44,27 +45,7 @@ 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 } }
-    let(:metadata) { { name: name, namespace: namespace } }
-  end
-
-  describe '#generate' do
-    subject { policy.generate }
-
-    it { is_expected.to eq(resource) }
-  end
+  include_examples 'network policy common specs'
 
   describe '.from_yaml' do
     let(:manifest) do
@@ -180,6 +161,7 @@ RSpec.describe Gitlab::Kubernetes::NetworkPolicy do
 
     let(:generated_resource) do
       ::Kubeclient::Resource.new(
+        kind: Gitlab::Kubernetes::NetworkPolicy::KIND,
         metadata: { name: name, namespace: namespace, labels: { app: 'foo' } },
         spec: { podSelector: pod_selector, policyTypes: %w(Ingress), ingress: ingress, egress: nil }
       )
@@ -215,4 +197,31 @@ RSpec.describe Gitlab::Kubernetes::NetworkPolicy do
       it { is_expected.to be_nil }
     end
   end
+
+  describe '#resource' do
+    subject { policy.resource }
+
+    let(:resource) do
+      {
+        kind: Gitlab::Kubernetes::NetworkPolicy::KIND,
+        metadata: { name: name, namespace: namespace },
+        spec: { podSelector: pod_selector, policyTypes: %w(Ingress), ingress: ingress, egress: nil }
+      }
+    end
+
+    it { is_expected.to eq(resource) }
+
+    context 'with labels' do
+      let(:labels) { { app: 'foo' } }
+      let(:resource) do
+        {
+          kind: Gitlab::Kubernetes::NetworkPolicy::KIND,
+          metadata: { name: name, namespace: namespace, labels: { app: 'foo' } },
+          spec: { podSelector: pod_selector, policyTypes: %w(Ingress), ingress: ingress, egress: nil }
+        }
+      end
+
+      it { is_expected.to eq(resource) }
+    end
+  end
 end

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

@@ -5,19 +5,19 @@ RSpec.shared_examples 'network policy common specs' do
   let(:namespace) { 'example-namespace' }
   let(:labels) { nil }
 
+  describe '#generate' do
+    subject { policy.generate }
+
+    it { is_expected.to eq(Kubeclient::Resource.new(policy.resource)) }
+  end
+
   describe 'as_json' do
     let(:json_policy) do
       {
         name: name,
         namespace: namespace,
         creation_timestamp: nil,
-        manifest: YAML.dump(
-          {
-            kind: described_class::KIND,
-            metadata: metadata,
-            spec: spec
-          }.deep_stringify_keys
-        ),
+        manifest: YAML.dump(policy.resource.deep_stringify_keys),
         is_autodevops: false,
         is_enabled: true
       }