Improve support for description field on CiliumNetworkPolicy

This commit fixes position of the description field to be top level as
per specification. Along with this change common module was refactored
to have #generate method on it.

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
       }