Merge branch '334392-add-environment-to-network-policies-graphql' into 'master'

Extend NetworkPolicy GraphQL API with `Environments` field

See merge request gitlab-org/gitlab!65314

app/finders/environments/environments_finder.rb

@@ -16,6 +16,7 @@ module Environments
       environments = project.environments
       environments = by_name(environments)
       environments = by_search(environments)
+      environments = by_ids(environments)
 
       # Raises InvalidStatesError if params[:states] contains invalid states.
       by_states(environments)
@@ -47,6 +48,14 @@ module Environments
       end
     end
 
+    def by_ids(environments)
+      if params[:environment_ids].present?
+        environments.for_id(params[:environment_ids])
+      else
+        environments
+      end
+    end
+
     def environments_with_states(environments)
       # Convert to array of strings
       states = Array(params[:states]).map(&:to_s)

doc/api/graphql/reference/index.md

@@ -10949,6 +10949,7 @@ Represents the network policy.
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | <a id="networkpolicyenabled"></a>`enabled` | [`Boolean!`](#boolean) | Indicates whether this policy is enabled. |
+| <a id="networkpolicyenvironments"></a>`environments` | [`EnvironmentConnection`](#environmentconnection) | Environments where this policy is applied. (see [Connections](#connections)) |
 | <a id="networkpolicyfromautodevops"></a>`fromAutoDevops` | [`Boolean!`](#boolean) | Indicates whether this policy is created from AutoDevops. |
 | <a id="networkpolicyname"></a>`name` | [`String!`](#string) | Name of the policy. |
 | <a id="networkpolicynamespace"></a>`namespace` | [`String!`](#string) | Namespace of the policy. |

ee/app/graphql/resolvers/network_policy_resolver.rb

@@ -29,7 +29,9 @@ module Resolvers
           updated_at: Time.iso8601(policy_json[:creation_timestamp]),
           yaml: policy_json[:manifest],
           from_auto_devops: policy_json[:is_autodevops],
-          enabled: policy_json[:is_enabled]
+          enabled: policy_json[:is_enabled],
+          environment_ids: policy_json[:environment_ids],
+          project: project
         }
       end
     end

ee/app/graphql/types/network_policy_type.rb

@@ -35,5 +35,21 @@ module Types
           Types::TimeType,
           null: false,
           description: 'Timestamp of when the policy YAML was last updated.'
+
+    field :environments,
+          Types::EnvironmentType.connection_type,
+          null: true,
+          description: 'Environments where this policy is applied.'
+
+    def environments
+      BatchLoader::GraphQL.for(object[:environment_ids]).batch do |policy_environment_ids, loader|
+        finder = ::Environments::EnvironmentsFinder.new(object[:project], context[:current_user], environment_ids: policy_environment_ids.flatten.uniq)
+        environments_by_id = finder.execute.index_by(&:id)
+
+        policy_environment_ids.each do |ids|
+          loader.call(ids, environments_by_id.values_at(*ids))
+        end
+      end
+    end
   end
 end

ee/app/services/network_policies/resources_service.rb

@@ -17,8 +17,8 @@ module NetworkPolicies
       policies = []
       errors = []
 
-      @kubeclient_info.each do |platform, namespace|
-        policies_per_environment, error_per_environment = execute_per_environment(platform, namespace)
+      @kubeclient_info.each do |platform, (namespace, environment_ids)|
+        policies_per_environment, error_per_environment = execute_per_environment(platform, namespace, environment_ids)
         policies += policies_per_environment if policies_per_environment
         errors << error_per_environment if error_per_environment
       end
@@ -33,13 +33,13 @@ module NetworkPolicies
       track_usage_event(:clusters_using_network_policies_ui, platform.cluster_id)
     end
 
-    def execute_per_environment(platform, namespace)
+    def execute_per_environment(platform, namespace, environment_ids)
       policies = platform.kubeclient
         .get_network_policies(namespace: namespace)
-        .map { |resource| Gitlab::Kubernetes::NetworkPolicy.from_resource(resource) }
+        .map { |resource| Gitlab::Kubernetes::NetworkPolicy.from_resource(resource, environment_ids) }
       policies += platform.kubeclient
         .get_cilium_network_policies(namespace: namespace)
-        .map { |resource| Gitlab::Kubernetes::CiliumNetworkPolicy.from_resource(resource) }
+        .map { |resource| Gitlab::Kubernetes::CiliumNetworkPolicy.from_resource(resource, environment_ids) }
       track_usage_data_for_cluster(platform, policies)
       [policies, nil]
     rescue Kubeclient::HttpError => e
@@ -47,7 +47,7 @@ module NetworkPolicies
     end
 
     def has_deployment_platform?(kubeclient_info)
-      kubeclient_info.any? { |platform, namespace| platform.present? }
+      kubeclient_info.any? { |platform, _| platform.present? }
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
@@ -63,7 +63,7 @@ module NetworkPolicies
         .order(updated_at: :desc)
         .preload(:platform_kubernetes)
         .group_by(&:namespace)
-        .map { |namespace, kubernetes_namespaces| [kubernetes_namespaces.first.platform_kubernetes, namespace] }
+        .map { |namespace, kubernetes_namespaces| [kubernetes_namespaces.first.platform_kubernetes, [namespace, kubernetes_namespaces.map(&:environment_id)]] }
     end
     # rubocop: enable CodeReuse/ActiveRecord
   end

ee/spec/graphql/resolvers/network_policy_resolver_spec.rb

@@ -16,7 +16,8 @@ RSpec.describe Resolvers::NetworkPolicyResolver do
       namespace: 'another',
       creation_timestamp: time_now.iso8601,
       selector: { matchLabels: { role: 'db' } },
-      ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
+      ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }],
+      environment_ids: [1, 2]
     )
   end
 
@@ -27,7 +28,8 @@ RSpec.describe Resolvers::NetworkPolicyResolver do
       creation_timestamp: time_now.iso8601,
       resource_version: '102',
       selector: { matchLabels: { role: 'db' } },
-      ingress: [{ endpointFrom: [{ matchLabels: { project: 'myproject' } }] }]
+      ingress: [{ endpointFrom: [{ matchLabels: { project: 'myproject' } }] }],
+      environment_ids: [3, 4]
     )
   end
 
@@ -97,7 +99,9 @@ RSpec.describe Resolvers::NetworkPolicyResolver do
               enabled: true,
               yaml: policy.as_json[:manifest],
               updated_at: time_now,
-              from_auto_devops: false
+              from_auto_devops: false,
+              environment_ids: [1, 2],
+              project: project
             },
             {
               name: 'cilium_policy',
@@ -105,7 +109,9 @@ RSpec.describe Resolvers::NetworkPolicyResolver do
               enabled: true,
               yaml: cilium_policy.as_json[:manifest],
               updated_at: time_now,
-              from_auto_devops: false
+              from_auto_devops: false,
+              environment_ids: [3, 4],
+              project: project
             }
           ]
           expect(resolve_network_policies).to eq(expected_resolved)

ee/spec/graphql/types/network_policy_type_spec.rb

@@ -12,7 +12,54 @@ RSpec.describe GitlabSchema.types['NetworkPolicy'] do
       :enabled,
       :from_auto_devops,
       :yaml,
-      :updated_at
+      :updated_at,
+      :environments
     )
   end
+
+  describe '#environments' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:project) { create(:project, namespace: user.namespace) }
+    let_it_be(:environment_1) { create(:environment, project: project) }
+    let_it_be(:environment_2) { create(:environment, project: project) }
+    let_it_be(:environment_3) { create(:environment, project: project) }
+
+    let(:policy_1) { double(as_json: { creation_timestamp: Time.current.iso8601, project: project, environment_ids: [environment_1.id] }) }
+    let(:policy_2) { double(as_json: { creation_timestamp: Time.current.iso8601, project: project, environment_ids: [environment_1.id, environment_2.id] }) }
+    let(:service_response_single_environment) { double(success?: true, payload: [policy_1]) }
+    let(:service_response_multiple_environments) { double(success?: true, payload: [policy_1, policy_2]) }
+
+    let(:query) do
+      %(
+        query {
+          project(fullPath: "#{project.full_path}") {
+            networkPolicies {
+              nodes {
+                environments {
+                  nodes {
+                    id
+                    name
+                  }
+                }
+              }
+            }
+          }
+        }
+      )
+    end
+
+    it 'avoids N+1 database queries' do
+      allow_next_instance_of(NetworkPolicies::ResourcesService) do |service|
+        allow(service).to receive(:execute).and_return(service_response_single_environment)
+      end
+
+      control_count = ActiveRecord::QueryRecorder.new { GitlabSchema.execute(query, context: { current_user: user }) }.count
+
+      allow_next_instance_of(NetworkPolicies::ResourcesService) do |service|
+        allow(service).to receive(:execute).and_return(service_response_multiple_environments)
+      end
+
+      expect { GitlabSchema.execute(query, context: { current_user: user }) }.not_to exceed_query_limit(control_count)
+    end
+  end
 end

ee/spec/services/network_policies/resources_service_spec.rb

@@ -16,7 +16,8 @@ RSpec.describe NetworkPolicies::ResourcesService do
       name: 'policy',
       namespace: 'another',
       selector: { matchLabels: { role: 'db' } },
-      ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
+      ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }],
+      environment_ids: [environment.id]
     )
   end
 
@@ -26,7 +27,8 @@ RSpec.describe NetworkPolicies::ResourcesService do
       namespace: 'another',
       resource_version: '102',
       selector: { matchLabels: { role: 'db' } },
-      ingress: [{ endpointFrom: [{ matchLabels: { project: 'myproject' } }] }]
+      ingress: [{ endpointFrom: [{ matchLabels: { project: 'myproject' } }] }],
+      environment_ids: [environment.id]
     )
   end
 
@@ -109,7 +111,8 @@ RSpec.describe NetworkPolicies::ResourcesService do
           name: 'policy_2',
           namespace: 'another_2',
           selector: { matchLabels: { role: 'db' } },
-          ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }]
+          ingress: [{ from: [{ namespaceSelector: { matchLabels: { project: 'myproject' } } }] }],
+          environment_ids: [environment.id]
         )
       end
 

lib/gitlab/kubernetes/cilium_network_policy.rb

@@ -12,7 +12,7 @@ module Gitlab
       # 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, annotations: nil)
+      def initialize(name:, namespace:, selector:, ingress:, resource_version: nil, description: nil, labels: nil, creation_timestamp: nil, egress: nil, annotations: nil, environment_ids: [])
         @name = name
         @description = description
         @namespace = namespace
@@ -23,6 +23,7 @@ module Gitlab
         @ingress = ingress
         @egress = egress
         @annotations = annotations
+        @environment_ids = environment_ids
       end
       # rubocop:enable Metrics/ParameterLists
 
@@ -49,7 +50,7 @@ module Gitlab
         nil
       end
 
-      def self.from_resource(resource)
+      def self.from_resource(resource, environment_ids = [])
         return unless resource
         return if !resource[:metadata] || !resource[:spec]
 
@@ -65,7 +66,8 @@ module Gitlab
           creation_timestamp: metadata[:creationTimestamp],
           selector: spec[:endpointSelector],
           ingress: spec[:ingress],
-          egress: spec[:egress]
+          egress: spec[:egress],
+          environment_ids: environment_ids
         )
       end
 
@@ -83,7 +85,7 @@ module Gitlab
 
       private
 
-      attr_reader :name, :description, :namespace, :labels, :creation_timestamp, :resource_version, :ingress, :egress, :annotations
+      attr_reader :name, :description, :namespace, :labels, :creation_timestamp, :resource_version, :ingress, :egress, :annotations, :environment_ids
 
       def selector
         @selector ||= {}

lib/gitlab/kubernetes/network_policy.rb

@@ -8,7 +8,8 @@ module Gitlab
 
       KIND = 'NetworkPolicy'
 
-      def initialize(name:, namespace:, selector:, ingress:, labels: nil, creation_timestamp: nil, policy_types: ["Ingress"], egress: nil)
+      # rubocop:disable Metrics/ParameterLists
+      def initialize(name:, namespace:, selector:, ingress:, labels: nil, creation_timestamp: nil, policy_types: ["Ingress"], egress: nil, environment_ids: [])
         @name = name
         @namespace = namespace
         @labels = labels
@@ -17,7 +18,9 @@ module Gitlab
         @policy_types = policy_types
         @ingress = ingress
         @egress = egress
+        @environment_ids = environment_ids
       end
+      # rubocop:enable Metrics/ParameterLists
 
       def self.from_yaml(manifest)
         return unless manifest
@@ -40,7 +43,7 @@ module Gitlab
         nil
       end
 
-      def self.from_resource(resource)
+      def self.from_resource(resource, environment_ids = [])
         return unless resource
         return if !resource[:metadata] || !resource[:spec]
 
@@ -54,7 +57,8 @@ module Gitlab
           selector: spec[:podSelector],
           policy_types: spec[:policyTypes],
           ingress: spec[:ingress],
-          egress: spec[:egress]
+          egress: spec[:egress],
+          environment_ids: environment_ids
         )
       end
 
@@ -69,7 +73,7 @@ module Gitlab
 
       private
 
-      attr_reader :name, :namespace, :labels, :creation_timestamp, :policy_types, :ingress, :egress
+      attr_reader :name, :namespace, :labels, :creation_timestamp, :policy_types, :ingress, :egress, :environment_ids
 
       def selector
         @selector ||= {}

lib/gitlab/kubernetes/network_policy_common.rb

@@ -16,7 +16,8 @@ module Gitlab
           creation_timestamp: creation_timestamp,
           manifest: manifest,
           is_autodevops: autodevops?,
-          is_enabled: enabled?
+          is_enabled: enabled?,
+          environment_ids: environment_ids
         }
       end
 

spec/finders/environments/environments_finder_spec.rb

@@ -3,9 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe Environments::EnvironmentsFinder do
-  let(:project) { create(:project, :repository) }
-  let(:user) { project.creator }
-  let(:environment) { create(:environment, :available, project: project) }
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { project.creator }
+  let_it_be(:environment) { create(:environment, :available, project: project) }
+  let_it_be(:environment_stopped) { create(:environment, :stopped, name: 'test2', project: project) }
+  let_it_be(:environment_available) { create(:environment, :available, name: 'test3', project: project) }
 
   before do
     project.add_maintainer(user)
@@ -13,18 +15,18 @@ RSpec.describe Environments::EnvironmentsFinder do
 
   describe '#execute' do
     context 'with states parameter' do
-      let(:stopped_environment) { create(:environment, :stopped, project: project) }
+      let_it_be(:stopped_environment) { create(:environment, :stopped, project: project) }
 
       it 'returns environments with the requested state' do
         result = described_class.new(project, user, states: 'available').execute
 
-        expect(result).to contain_exactly(environment)
+        expect(result).to contain_exactly(environment, environment_available)
       end
 
       it 'returns environments with any of the requested states' do
         result = described_class.new(project, user, states: %w(available stopped)).execute
 
-        expect(result).to contain_exactly(environment, stopped_environment)
+        expect(result).to contain_exactly(environment, environment_stopped, environment_available, stopped_environment)
       end
 
       it 'raises exception when requested state is invalid' do
@@ -37,25 +39,30 @@ RSpec.describe Environments::EnvironmentsFinder do
         it 'returns environments with the requested state' do
           result = described_class.new(project, user, states: :available).execute
 
-          expect(result).to contain_exactly(environment)
+          expect(result).to contain_exactly(environment, environment_available)
         end
 
         it 'returns environments with any of the requested states' do
           result = described_class.new(project, user, states: [:available, :stopped]).execute
 
-          expect(result).to contain_exactly(environment, stopped_environment)
+          expect(result).to contain_exactly(environment, environment_stopped, environment_available, stopped_environment)
         end
       end
     end
 
     context 'with search and states' do
-      let(:environment2) { create(:environment, :stopped, name: 'test2', project: project) }
-      let(:environment3) { create(:environment, :available, name: 'test3', project: project) }
-
       it 'searches environments by name and state' do
         result = described_class.new(project, user, search: 'test', states: :available).execute
 
-        expect(result).to contain_exactly(environment3)
+        expect(result).to contain_exactly(environment_available)
+      end
+    end
+
+    context 'with id' do
+      it 'searches environments by name and state' do
+        result = described_class.new(project, user, search: 'test', environment_ids: [environment_available.id]).execute
+
+        expect(result).to contain_exactly(environment_available)
       end
     end
   end

spec/lib/gitlab/kubernetes/cilium_network_policy_spec.rb

@@ -206,6 +206,14 @@ RSpec.describe Gitlab::Kubernetes::CiliumNetworkPolicy do
 
       it { is_expected.to be_nil }
     end
+
+    context 'with environment_ids' do
+      subject { Gitlab::Kubernetes::CiliumNetworkPolicy.from_resource(resource, [1, 2, 3]) }
+
+      it 'includes environment_ids in as_json result' do
+        expect(subject.as_json).to include(environment_ids: [1, 2, 3])
+      end
+    end
   end
 
   describe '#resource' do

spec/lib/gitlab/kubernetes/network_policy_spec.rb

@@ -196,6 +196,14 @@ RSpec.describe Gitlab::Kubernetes::NetworkPolicy do
 
       it { is_expected.to be_nil }
     end
+
+    context 'with environment_ids' do
+      subject { Gitlab::Kubernetes::NetworkPolicy.from_resource(resource, [1, 2, 3]) }
+
+      it 'includes environment_ids in as_json result' do
+        expect(subject.as_json).to include(environment_ids: [1, 2, 3])
+      end
+    end
   end
 
   describe '#resource' do

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

@@ -19,7 +19,8 @@ RSpec.shared_examples 'network policy common specs' do
         creation_timestamp: nil,
         manifest: YAML.dump(policy.resource.deep_stringify_keys),
         is_autodevops: false,
-        is_enabled: true
+        is_enabled: true,
+        environment_ids: []
       }
     end