From 3b4bad9bcb8bec7ca360ce2eceee986aaf21f87c Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Wed, 8 Nov 2023 09:50:56 +0530 Subject: [PATCH 1/2] Update noinspection comments to add issue link --- .../remote_development/network_policy_egress_validator_spec.rb | 2 +- spec/validators/ip_cidr_array_validator_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/validators/remote_development/network_policy_egress_validator_spec.rb b/ee/spec/validators/remote_development/network_policy_egress_validator_spec.rb index 9c1a957b1e8717..1c24c6ae7d1130 100644 --- a/ee/spec/validators/remote_development/network_policy_egress_validator_spec.rb +++ b/ee/spec/validators/remote_development/network_policy_egress_validator_spec.rb @@ -17,7 +17,7 @@ using RSpec::Parameterized::TableSyntax - # noinspection RubyMismatchedArgumentType - RubyMine is resolving `#|` from Array, instead of Rspec::Parameterized + # noinspection RubyMismatchedArgumentType - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-32041 where(:egress, :validity, :errors) do # rubocop:disable Layout/LineLength -- The RSpec table syntax often requires long lines for errors nil | false | { egress: ['must be an array'] } diff --git a/spec/validators/ip_cidr_array_validator_spec.rb b/spec/validators/ip_cidr_array_validator_spec.rb index 4ea46d714c2921..6adb0bc70dbcd6 100644 --- a/spec/validators/ip_cidr_array_validator_spec.rb +++ b/spec/validators/ip_cidr_array_validator_spec.rb @@ -17,7 +17,7 @@ using RSpec::Parameterized::TableSyntax - # noinspection RubyMismatchedArgumentType - RubyMine is resolving `#|` from Array, instead of Rspec::Parameterized + # noinspection RubyMismatchedArgumentType - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-32041 where(:cidr_array, :validity, :errors) do # rubocop:disable Layout/LineLength -- The RSpec table syntax often requires long lines for errors nil | false | { cidr_array: ["must be an array of CIDR values"] } -- GitLab From fd798cccdd1e47caeb0b00770c5e7c8e9b0ca685 Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Tue, 7 Nov 2023 16:21:46 +0530 Subject: [PATCH 2/2] Update network policy egress in remote development agent configs Set all workspaces of the agent to force include all resources when the configuration of the agent is updated. --- .../agent_config/updater.rb | 17 +++++- .../agent_config/updater_spec.rb | 56 ++++++++++++++++++- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/ee/lib/remote_development/agent_config/updater.rb b/ee/lib/remote_development/agent_config/updater.rb index 16e639ed2c2083..7bde5a8574746d 100644 --- a/ee/lib/remote_development/agent_config/updater.rb +++ b/ee/lib/remote_development/agent_config/updater.rb @@ -1,9 +1,20 @@ # frozen_string_literal: true +# noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 module RemoteDevelopment module AgentConfig class Updater include Messages + NETWORK_POLICY_EGRESS_DEFAULT = [ + { + allow: "0.0.0.0/0", + except: [ + - "10.0.0.0/8", + - "172.16.0.0/12", + - "192.168.0.0/16" + ] + } + ].freeze # @param [Hash] value # @return [Result] @@ -19,16 +30,16 @@ def self.update(value) model_instance = RemoteDevelopmentAgentConfig.find_or_initialize_by(agent: agent) # rubocop:todo CodeReuse/ActiveRecord -- Use a finder class here model_instance.enabled = config_from_agent_config_file.fetch(:enabled, false) - # noinspection RubyResolve model_instance.dns_zone = config_from_agent_config_file[:dns_zone] - # noinspection RubyResolve model_instance.network_policy_enabled = config_from_agent_config_file.fetch(:network_policy, {}).fetch(:enabled, true) - # noinspection RubyResolve + model_instance.network_policy_egress = + config_from_agent_config_file.fetch(:network_policy, {}).fetch(:egress, NETWORK_POLICY_EGRESS_DEFAULT) model_instance.gitlab_workspaces_proxy_namespace = config_from_agent_config_file.fetch(:gitlab_workspaces_proxy, {}).fetch(:namespace, 'gitlab-workspaces') if model_instance.save + model_instance.workspaces.update_all(force_include_all_resources: true) Result.ok(AgentConfigUpdateSuccessful.new({ remote_development_agent_config: model_instance })) else Result.err(AgentConfigUpdateFailed.new({ errors: model_instance.errors })) diff --git a/ee/spec/lib/remote_development/agent_config/updater_spec.rb b/ee/spec/lib/remote_development/agent_config/updater_spec.rb index dcdf38e5ec4e06..e9f519e6e0edd1 100644 --- a/ee/spec/lib/remote_development/agent_config/updater_spec.rb +++ b/ee/spec/lib/remote_development/agent_config/updater_spec.rb @@ -8,11 +8,21 @@ let(:enabled) { true } let(:dns_zone) { 'my-awesome-domain.me' } let(:network_policy_present) { false } + let(:default_network_policy_egress) { RemoteDevelopment::AgentConfig::Updater::NETWORK_POLICY_EGRESS_DEFAULT } + let(:network_policy_egress) { default_network_policy_egress } let(:network_policy_enabled) { true } - let(:network_policy) do + let(:network_policy_without_egress) do { enabled: network_policy_enabled } end + let(:network_policy_with_egress) do + { + enabled: network_policy_enabled, + egress: network_policy_egress + } + end + + let(:network_policy) { network_policy_without_egress } let(:gitlab_workspaces_proxy_present) { false } let(:gitlab_workspaces_proxy_namespace) { 'gitlab-workspaces' } let(:gitlab_workspaces_proxy) do @@ -20,6 +30,8 @@ end let_it_be(:agent) { create(:cluster_agent) } + let_it_be(:workspace1) { create(:workspace, force_include_all_resources: false) } + let_it_be(:workspace2) { create(:workspace, force_include_all_resources: false) } let(:config) do remote_development_config = { @@ -59,12 +71,14 @@ expect(config_instance.enabled).to eq(enabled) expect(config_instance.dns_zone).to eq(dns_zone) expect(config_instance.network_policy_enabled).to eq(network_policy_enabled) + expect(config_instance.network_policy_egress.map(&:deep_symbolize_keys)).to eq(network_policy_egress) expect(config_instance.gitlab_workspaces_proxy_namespace).to eq(gitlab_workspaces_proxy_namespace) expect(result) .to be_ok_result(RemoteDevelopment::Messages::AgentConfigUpdateSuccessful.new( { remote_development_agent_config: config_instance } )) + expect(config_instance.workspaces).to all(have_attributes(force_include_all_resources: true)) end context 'when enabled is not present in the config passed' do @@ -90,12 +104,14 @@ expect(config_instance.enabled).to eq(enabled) expect(config_instance.dns_zone).to eq(dns_zone) expect(config_instance.network_policy_enabled).to eq(network_policy_enabled) + expect(config_instance.network_policy_egress.map(&:deep_symbolize_keys)).to eq(network_policy_egress) expect(config_instance.gitlab_workspaces_proxy_namespace).to eq(gitlab_workspaces_proxy_namespace) expect(result) .to be_ok_result(RemoteDevelopment::Messages::AgentConfigUpdateSuccessful.new( { remote_development_agent_config: config_instance } )) + expect(config_instance.workspaces).to all(have_attributes(force_include_all_resources: true)) end end @@ -109,12 +125,46 @@ expect(config_instance.enabled).to eq(enabled) expect(config_instance.dns_zone).to eq(dns_zone) expect(config_instance.network_policy_enabled).to eq(network_policy_enabled) + expect(config_instance.network_policy_egress.map(&:deep_symbolize_keys)).to eq(network_policy_egress) + expect(config_instance.gitlab_workspaces_proxy_namespace).to eq(gitlab_workspaces_proxy_namespace) + + expect(result) + .to be_ok_result(RemoteDevelopment::Messages::AgentConfigUpdateSuccessful.new( + { remote_development_agent_config: config_instance } + )) + expect(config_instance.workspaces).to all(have_attributes(force_include_all_resources: true)) + end + end + + context 'when network_policy.egress is explicitly specified in the config passed' do + let(:network_policy_egress) do + [ + { + allow: "0.0.0.0/0", + except: [ + - "10.0.0.0/8" + ] + } + ].freeze + end + + let(:network_policy) { network_policy_with_egress } + + it 'creates a config record with specified value and returns an ok Result containing the agent config' do + expect { result }.to change { RemoteDevelopment::RemoteDevelopmentAgentConfig.count } + + config_instance = agent.reload.remote_development_agent_config + expect(config_instance.enabled).to eq(enabled) + expect(config_instance.dns_zone).to eq(dns_zone) + expect(config_instance.network_policy_enabled).to eq(network_policy_enabled) + expect(config_instance.network_policy_egress.map(&:deep_symbolize_keys)).to eq(network_policy_egress) expect(config_instance.gitlab_workspaces_proxy_namespace).to eq(gitlab_workspaces_proxy_namespace) expect(result) .to be_ok_result(RemoteDevelopment::Messages::AgentConfigUpdateSuccessful.new( { remote_development_agent_config: config_instance } )) + expect(config_instance.workspaces).to all(have_attributes(force_include_all_resources: true)) end end end @@ -132,12 +182,14 @@ expect(config_instance.enabled).to eq(enabled) expect(config_instance.dns_zone).to eq(dns_zone) expect(config_instance.network_policy_enabled).to eq(network_policy_enabled) + expect(config_instance.network_policy_egress.map(&:deep_symbolize_keys)).to eq(network_policy_egress) expect(config_instance.gitlab_workspaces_proxy_namespace).to eq(gitlab_workspaces_proxy_namespace) expect(result) .to be_ok_result(RemoteDevelopment::Messages::AgentConfigUpdateSuccessful.new( { remote_development_agent_config: config_instance } )) + expect(config_instance.workspaces).to all(have_attributes(force_include_all_resources: true)) end end @@ -151,12 +203,14 @@ expect(config_instance.enabled).to eq(enabled) expect(config_instance.dns_zone).to eq(dns_zone) expect(config_instance.network_policy_enabled).to eq(network_policy_enabled) + expect(config_instance.network_policy_egress.map(&:deep_symbolize_keys)).to eq(network_policy_egress) expect(config_instance.gitlab_workspaces_proxy_namespace).to eq(gitlab_workspaces_proxy_namespace) expect(result) .to be_ok_result(RemoteDevelopment::Messages::AgentConfigUpdateSuccessful.new( { remote_development_agent_config: config_instance } )) + expect(config_instance.workspaces).to all(have_attributes(force_include_all_resources: true)) end end end -- GitLab