Merge branch '336947-fix-404-error-when-editing-predefined-policies' into 'master'

Fix 404 error when editing predefined network policies

See merge request gitlab-org/gitlab!68728

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/policy_editor.vue

@@ -40,7 +40,7 @@ export default {
       return this.isEditing ? this.policyType : this.newPolicyType;
     },
     isEditing() {
-      return Boolean(this.existingPolicy);
+      return Boolean(this.existingPolicy?.creation_timestamp || this.existingPolicy?.updatedAt);
     },
     policyTypes() {
       return Object.values(POLICY_TYPE_COMPONENT_OPTIONS);

ee/app/services/network_policies/find_resource_service.rb

@@ -27,12 +27,21 @@ module NetworkPolicies
     def get_policy
       client = @platform.kubeclient
       if @kind == Gitlab::Kubernetes::CiliumNetworkPolicy::KIND
-        resource = client.get_cilium_network_policy(@resource_name, @kubernetes_namespace)
+        resource = find_cilium_policy_or_predefined(client)
         Gitlab::Kubernetes::CiliumNetworkPolicy.from_resource(resource)
       elsif @kind == Gitlab::Kubernetes::NetworkPolicy::KIND
         resource = client.get_network_policy(@resource_name, @kubernetes_namespace)
         Gitlab::Kubernetes::NetworkPolicy.from_resource(resource)
       end
     end
+
+    def find_cilium_policy_or_predefined(client)
+      client.get_cilium_network_policy(@resource_name, @kubernetes_namespace)
+    rescue Kubeclient::ResourceNotFoundError
+      policy_yaml = Gitlab::Kubernetes::CiliumNetworkPolicy::PREDEFINED_POLICIES[@resource_name]
+      raise if policy_yaml.nil?
+
+      Gitlab::Kubernetes::CiliumNetworkPolicy.from_yaml(policy_yaml).resource
+    end
   end
 end

ee/spec/frontend/threat_monitoring/components/policy_editor/policy_editor_spec.js

@@ -93,9 +93,9 @@ describe('PolicyEditor component', () => {
 
   describe('when an existing policy is present', () => {
     it.each`
-      policyType                 | option                                         | existingPolicy                  | findComponent
-      ${'container_policy'}      | ${POLICY_TYPE_COMPONENT_OPTIONS.container}     | ${{ manifest: mockL3Manifest }} | ${findNeworkPolicyEditor}
-      ${'scan_execution_policy'} | ${POLICY_TYPE_COMPONENT_OPTIONS.scanExecution} | ${mockDastScanExecutionObject}  | ${findScanExecutionPolicyEditor}
+      policyType                 | option                                         | existingPolicy                                                     | findComponent
+      ${'container_policy'}      | ${POLICY_TYPE_COMPONENT_OPTIONS.container}     | ${{ manifest: mockL3Manifest, updatedAt: '2020-04-14T00:08:30Z' }} | ${findNeworkPolicyEditor}
+      ${'scan_execution_policy'} | ${POLICY_TYPE_COMPONENT_OPTIONS.scanExecution} | ${mockDastScanExecutionObject}                                     | ${findScanExecutionPolicyEditor}
     `(
       'renders the disabled form select for existing policy of type $policyType',
       async ({ existingPolicy, findComponent, option, policyType }) => {

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_execution_policy/lib/from_yaml_spec.js

@@ -6,6 +6,6 @@ import {
 
 describe('fromYaml', () => {
   it('returns policy object', () => {
-    expect(fromYaml(mockDastScanExecutionManifest)).toStrictEqual(mockDastScanExecutionObject);
+    expect(fromYaml(mockDastScanExecutionManifest)).toMatchObject(mockDastScanExecutionObject);
   });
 });

ee/spec/frontend/threat_monitoring/mocks/mock_data.js

@@ -23,6 +23,7 @@ rules:
   - type: pipeline
     branches:
       - main
+updatedAt: '2020-04-14T00:08:30Z'
 actions:
   - scan: dast
     site_profile: required_site_profile
@@ -35,6 +36,7 @@ export const mockDastScanExecutionObject = {
   description: 'This policy enforces pipeline configuration to have a job with DAST scan',
   enabled: false,
   rules: [{ type: 'pipeline', branches: ['main'] }],
+  updatedAt: '2020-04-14T00:08:30Z',
   actions: [
     {
       scan: 'dast',

ee/spec/services/network_policies/find_resource_service_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe NetworkPolicies::FindResourceService do
-  let(:service) { described_class.new(resource_name: 'policy', environment: environment, kind: kind) }
+  let(:resource_name) { 'policy' }
+  let(:service) { described_class.new(resource_name: resource_name, environment: environment, kind: kind) }
   let(:environment) { instance_double('Environment', deployment_platform: platform, deployment_namespace: 'namespace') }
   let(:platform) { instance_double('Clusters::Platforms::Kubernetes', kubeclient: kubeclient) }
   let(:kubeclient) { double('Kubeclient::Client') }
@@ -49,6 +50,42 @@ RSpec.describe NetworkPolicies::FindResourceService do
         expect(subject).to be_success
         expect(subject.payload.as_json).to eq(policy.as_json)
       end
+
+      context 'when it was not found in the cluster' do
+        before do
+          allow(kubeclient).to receive(:get_cilium_network_policy).with(resource_name, environment.deployment_namespace).and_raise(Kubeclient::ResourceNotFoundError.new(404, 'policy not found', {}))
+        end
+
+        let(:policy) do
+          {
+            creation_timestamp: nil,
+            environment_ids: [],
+            is_autodevops: false,
+            is_enabled: false,
+            name: "drop-outbound",
+            namespace: nil
+          }
+        end
+
+        context 'and has name reserved for predefined policy' do
+          let(:resource_name) { 'drop-outbound' }
+
+          it 'returns success response with predefined policy' do
+            expect(subject).to be_success
+            expect(subject.payload.as_json).to include(policy)
+          end
+        end
+
+        context 'and has name different from any predefined policy' do
+          let(:resource_name) { 'not-predefined-policy' }
+
+          it 'returns success response with predefined policy' do
+            expect(subject).to be_error
+            expect(subject.http_status).to eq(:bad_request)
+            expect(subject.message).to eq('Kubernetes error: policy not found')
+          end
+        end
+      end
     end
 
     context 'with invalid policy kind' do
@@ -57,7 +94,7 @@ RSpec.describe NetworkPolicies::FindResourceService do
       it 'returns error response' do
         expect(subject).to be_error
         expect(subject.http_status).to eq(:bad_request)
-        expect(subject.message).not_to be_nil
+        expect(subject.message).to eq('Invalid or unsupported policy kind')
       end
     end
 
@@ -67,7 +104,7 @@ RSpec.describe NetworkPolicies::FindResourceService do
       it 'returns error response' do
         expect(subject).to be_error
         expect(subject.http_status).to eq(:bad_request)
-        expect(subject.message).not_to be_nil
+        expect(subject.message).to eq('Environment does not have deployment platform')
       end
     end
 

lib/gitlab/kubernetes/cilium_network_policy.rb

@@ -9,6 +9,36 @@ module Gitlab
       API_VERSION = "cilium.io/v2"
       KIND = 'CiliumNetworkPolicy'
 
+      PREDEFINED_POLICIES = {
+        'allow-inbound-http' => <<~YAML.rstrip,
+          apiVersion: cilium.io/v2
+          kind: CiliumNetworkPolicy
+          metadata:
+            name: allow-inbound-http
+          spec:
+            endpointSelector:
+              matchLabels:
+                network-policy.gitlab.com/disabled_by: gitlab
+            ingress:
+            - toPorts:
+              - ports:
+                - port: '80'
+                - port: '443'
+        YAML
+        'drop-outbound' => <<~YAML.rstrip
+          apiVersion: cilium.io/v2
+          kind: CiliumNetworkPolicy
+          metadata:
+            name: drop-outbound
+          spec:
+            endpointSelector:
+              matchLabels:
+                network-policy.gitlab.com/disabled_by: gitlab
+            egress:
+            - {}
+        YAML
+      }.freeze
+
       # We are modeling existing kubernetes resource and don't have
       # control over amount of parameters.
       # rubocop:disable Metrics/ParameterLists