From 0c506c02dd4dda23be6fc2ebd17a9c4b6b54c234 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Mon, 4 Mar 2024 18:18:21 +0800 Subject: [PATCH 1/7] Implement query API to list available remote dev cluster agents wip: add working impl to retrieve directly mapped cluster agents wip: add working impl to retrieve candidate agents in group Fix rubocop errors wip: add impl to retrieve agents in group available for workspaces Fix rubocop errors Fix more rubocop errors Update impl to only retrieve agents with remote dev enabled Move field to namespace type Rebuild graphql docs Add license check for new field Return error in case of inadequate permissions Fix permission issues Add validations to ensure namespaces contain cluster agents Fix rubocop issues Cleanup TODOs and document validation function Fix rubocop issues Implement finder to return available cluster agents with spec Fix rubocop errors Fix more rubocop errors Make naming of filter param consistent Clean up commented code from resolver Remove unrequired check for access violations WIP: add resolver specs Add missing scenarios under resolver specs and update the resolver Rename spec file to follow naming conventions Update finder to return RuntimeError instead of ArgumentError Remove redundant class methods Refactor filter logic into validations class and update finder Fix rubocop errors Use symbol instead of hardcoded string Remove unneeded enum types Recompile graphql docs Fix specs for validations Remove redundant test from finder spec Update query spec to not use mocks Use namespace instead of groups in field description Rebuild graphql docs Undo unnecessary changes to the policy --- doc/api/graphql/reference/index.md | 40 ++++++ .../cluster_agents_finder.rb | 19 +++ ee/app/graphql/ee/types/namespace_type.rb | 7 ++ .../agents_for_namespace_resolver.rb | 34 +++++ .../group_cluster_agent_filter_enum.rb | 13 ++ ...lopment_namespace_cluster_agent_mapping.rb | 2 + .../validations.rb | 53 ++++++++ .../cluster_agents_finder_spec.rb | 92 ++++++++++++++ .../validations_spec.rb | 86 +++++++++++++ .../with_available_filter_arg_spec.rb | 116 ++++++++++++++++++ 10 files changed, 462 insertions(+) create mode 100644 ee/app/finders/remote_development/cluster_agents_finder.rb create mode 100644 ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb create mode 100644 ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb create mode 100644 ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb create mode 100644 ee/spec/finders/remote_development/cluster_agents_finder_spec.rb create mode 100644 ee/spec/lib/remote_development/namespace_cluster_agent_mappings/validations_spec.rb create mode 100644 ee/spec/requests/api/graphql/remote_development/namespace/remote_development_cluster_agents/with_available_filter_arg_spec.rb diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 092a40cb186298..1b174b973b7004 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -21782,6 +21782,22 @@ four standard [pagination arguments](#pagination-arguments): | ---- | ---- | ----------- | | `sort` | [`GroupReleaseSort`](#groupreleasesort) | Sort group releases by given criteria. | +##### `Group.remoteDevelopmentClusterAgents` + +Cluster agents in the namespace with remote development capabilities. + +Returns [`ClusterAgentConnection`](#clusteragentconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#pagination-arguments): +`before: String`, `after: String`, `first: Int`, and `last: Int`. + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `filter` | [`GroupClusterAgentFilter!`](#groupclusteragentfilter) | Filter the types of cluster agents to return. | + ##### `Group.runnerCloudProvisioning` Information used for provisioning the runner on a cloud provider. Returns `null` if `:google_cloud_support_feature_flag` feature flag is disabled, or the GitLab instance is not a SaaS instance. @@ -25102,6 +25118,22 @@ four standard [pagination arguments](#pagination-arguments): | `withIssuesEnabled` | [`Boolean`](#boolean) | Return only projects with issues enabled. | | `withMergeRequestsEnabled` | [`Boolean`](#boolean) | Return only projects with merge requests enabled. | +##### `Namespace.remoteDevelopmentClusterAgents` + +Cluster agents in the namespace with remote development capabilities. + +Returns [`ClusterAgentConnection`](#clusteragentconnection). + +This field returns a [connection](#connections). It accepts the +four standard [pagination arguments](#pagination-arguments): +`before: String`, `after: String`, `first: Int`, and `last: Int`. + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `filter` | [`GroupClusterAgentFilter!`](#groupclusteragentfilter) | Filter the types of cluster agents to return. | + ##### `Namespace.scanExecutionPolicies` Scan Execution Policies of the namespace. @@ -32819,6 +32851,14 @@ Values for sorting artifacts. | `UPLOAD_TIME_ASC` | Ordered by `upload_time` in ascending order. | | `UPLOAD_TIME_DESC` | Ordered by `upload_time` in descending order. | +### `GroupClusterAgentFilter` + +Possible filter types for remote development cluster agents in a group. + +| Value | Description | +| ----- | ----------- | +| `AVAILABLE` | Cluster agents in the group that can be used for hosting workspaces. | + ### `GroupMemberRelation` Group member relation. diff --git a/ee/app/finders/remote_development/cluster_agents_finder.rb b/ee/app/finders/remote_development/cluster_agents_finder.rb new file mode 100644 index 00000000000000..cceba252b51972 --- /dev/null +++ b/ee/app/finders/remote_development/cluster_agents_finder.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module RemoteDevelopment + class ClusterAgentsFinder + def self.execute(namespace:, filter:) + case filter + when :AVAILABLE + relevant_mappings = RemoteDevelopmentNamespaceClusterAgentMapping.for_namespaces(namespace.traversal_ids) + relevant_mappings = NamespaceClusterAgentMappings::Validations.filter_valid_namespace_cluster_agent_mappings( + namespace_cluster_agent_mappings: relevant_mappings + ) + + Clusters::Agent.id_in(relevant_mappings.map(&:cluster_agent_id)).with_remote_development_enabled + else + raise "Unsupported value for filter: #{filter}" + end + end + end +end diff --git a/ee/app/graphql/ee/types/namespace_type.rb b/ee/app/graphql/ee/types/namespace_type.rb index eba03dd43571b7..497c885a8c330e 100644 --- a/ee/app/graphql/ee/types/namespace_type.rb +++ b/ee/app/graphql/ee/types/namespace_type.rb @@ -139,6 +139,13 @@ module NamespaceType alpha: { milestone: '16.9' }, authorize: :modify_product_analytics_settings + field :remote_development_cluster_agents, + ::Types::Clusters::AgentType.connection_type, + extras: [:lookahead], + null: true, + description: 'Cluster agents in the namespace with remote development capabilities', + resolver: ::Resolvers::RemoteDevelopment::AgentsForNamespaceResolver + def product_analytics_stored_events_limit object.root_ancestor.product_analytics_stored_events_limit end diff --git a/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb b/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb new file mode 100644 index 00000000000000..7a1ec958a36206 --- /dev/null +++ b/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Resolvers + module RemoteDevelopment + class AgentsForNamespaceResolver < ::Resolvers::BaseResolver + include Gitlab::Graphql::Authorize::AuthorizeResource + + type Types::Clusters::AgentType.connection_type, null: true + + argument :filter, Types::RemoteDevelopment::GroupClusterAgentFilterEnum, + required: true, + description: 'Filter the types of cluster agents to return.' + + def resolve(**args) + unless License.feature_available?(:remote_development) + raise_resource_not_available_error! "'remote_development' licensed feature is not available" + end + + unless Feature.enabled?(:remote_development_namespace_agent_authorization, @object.root_ancestor) + raise_resource_not_available_error!( + "'remote_development_namespace_agent_authorization' feature flag is disabled" + ) + end + + raise_resource_not_available_error! unless @object.group_namespace? + + ::RemoteDevelopment::ClusterAgentsFinder.execute( + namespace: @object, + filter: args[:filter].to_sym + ) + end + end + end +end diff --git a/ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb b/ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb new file mode 100644 index 00000000000000..200617af1d7dcb --- /dev/null +++ b/ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Types + module RemoteDevelopment + class GroupClusterAgentFilterEnum < BaseEnum + graphql_name 'GroupClusterAgentFilter' + description 'Possible filter types for remote development cluster agents in a group' + + value 'AVAILABLE', + description: "Cluster agents in the group that can be used for hosting workspaces.", value: 'AVAILABLE' + end + end +end diff --git a/ee/app/models/remote_development/remote_development_namespace_cluster_agent_mapping.rb b/ee/app/models/remote_development/remote_development_namespace_cluster_agent_mapping.rb index 1fee4f910cc235..caba0a492010b0 100644 --- a/ee/app/models/remote_development/remote_development_namespace_cluster_agent_mapping.rb +++ b/ee/app/models/remote_development/remote_development_namespace_cluster_agent_mapping.rb @@ -15,5 +15,7 @@ class RemoteDevelopmentNamespaceClusterAgentMapping < ApplicationRecord validates :namespace, presence: true validates :agent, presence: true validates :user, presence: true + + scope :for_namespaces, ->(namespace_ids) { where(namespace_id: namespace_ids) } end end diff --git a/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb b/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb new file mode 100644 index 00000000000000..f0919108785f1d --- /dev/null +++ b/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module NamespaceClusterAgentMappings + class Validations + # The function checks and filters clusters agents that reside within a namespace. All other + # agents are excluded from the response. A cluster agent is said to reside within a namespace + # if the namespace id is present in the traversal ids of the project bound to the cluster agent + # + # @param [Array] + # namespace_cluster_agent_mappings + # @return [Array] + def self.filter_valid_namespace_cluster_agent_mappings(namespace_cluster_agent_mappings:) + agent_ids = namespace_cluster_agent_mappings.map(&:cluster_agent_id) + traversal_ids_for_agents = traversal_ids_for_cluster_agents(cluster_agent_ids: agent_ids) + namespace_cluster_agent_mappings.filter do |mapping| + traversal_ids_for_agents.fetch(mapping.cluster_agent_id, []).include?(mapping.namespace_id) + end + end + + def self.traversal_ids_for_cluster_agents(cluster_agent_ids:) + agents_by_id = Clusters::Agent.id_in(cluster_agent_ids).index_by(&:id) + + projects_by_id = Project.id_in(agents_by_id.values.map(&:project_id)).index_by(&:id) + + project_namespaces_by_id = + Namespaces::ProjectNamespace + .id_in(projects_by_id.values.map(&:project_namespace_id)) + .index_by(&:id) + + cluster_agent_ids.each_with_object({}) do |cluster_agent_id, hash| + next unless agents_by_id.has_key?(cluster_agent_id) + + agent = agents_by_id[cluster_agent_id] + + # projects_by_id must contain agent.project_id as "agents" table has a ON CASCADE DELETE constraint with + # respect to the "projects" table. As such, if an agent can be retrieved from the database, + # so should its project + project = projects_by_id[agent.project_id] + + # project_namespaces_by_id must contain project.project_namespace_id as "projects" table has a + # ON CASCADE DELETE constraint with respect to the projects table. As such, if a project can be retrieved + # from the database, so should its project_namespace + project_namespace = project_namespaces_by_id[project.project_namespace_id] + + hash[cluster_agent_id] = project_namespace.traversal_ids + end + end + + private_class_method :traversal_ids_for_cluster_agents + end + end +end diff --git a/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb b/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb new file mode 100644 index 00000000000000..b886c7e704efd3 --- /dev/null +++ b/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe RemoteDevelopment::ClusterAgentsFinder, feature_category: :remote_development do + let_it_be(:user) { create(:user) } + let_it_be(:root_agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + let_it_be(:nested_agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + let_it_be_with_reload(:root_namespace) do + create(:group, + projects: [root_agent.project], + children: [ + create(:group, + projects: [nested_agent.project] + ) + ] + ) + end + + let(:nested_namespace) { root_namespace.children.first } + let(:namespace) { root_namespace } + let(:filter) { :AVAILABLE } + + before_all do + create( + :remote_development_namespace_cluster_agent_mapping, + user: user, + agent: nested_agent, + namespace: root_namespace + ) + end + + subject(:response) do + described_class.execute( + namespace: namespace, + filter: filter + ).to_a + end + + describe 'with filter_type set to AVAILABLE' do + context 'when all cluster agents are bound to the namespace' do + it 'returns all cluster agents passed in the parameters' do + expect(response).to eq([nested_agent]) + end + end + + context 'when cluster agents are bound to ancestors of the namespace' do + let(:namespace) { nested_namespace } + + it 'returns cluster agents including those bound to the ancestors' do + expect(response).to eq([nested_agent]) + end + end + + context 'when the same cluster agent is bound to a namespace as well as its ancestors' do + # Set this up in a way such that same agent is mapped to two namespaces: + # the namespace in the request as well as its ancestor + before do + create( + :remote_development_namespace_cluster_agent_mapping, + user: user, + namespace: nested_namespace, + agent: nested_agent + ) + end + + let(:namespace) { nested_namespace } + + it 'returns distinct cluster agents in the response' do + expect(response).to eq([nested_agent]) + end + end + + context 'when a bound cluster agent does not have remote development enabled' do + before do + nested_agent.remote_development_agent_config.update!(enabled: false) + end + + it 'ignores agents with remote development disabled in the response' do + expect(response).to eq([]) + end + end + end + + describe 'with an invalid value for filter_type' do + let(:filter) { "some_invalid_value" } + + it 'raises a RuntimeError' do + expect { response }.to raise_error(RuntimeError, "Unsupported value for filter: #{filter}") + end + end +end diff --git a/ee/spec/lib/remote_development/namespace_cluster_agent_mappings/validations_spec.rb b/ee/spec/lib/remote_development/namespace_cluster_agent_mappings/validations_spec.rb new file mode 100644 index 00000000000000..eeb30209338b22 --- /dev/null +++ b/ee/spec/lib/remote_development/namespace_cluster_agent_mappings/validations_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::NamespaceClusterAgentMappings::Validations, feature_category: :remote_development do + describe 'filter_valid_namespace_cluster_agent_mappings' do + let_it_be(:user) { create(:user) } + let_it_be(:root_agent) { create(:cluster_agent) } + let_it_be(:nested_agent) { create(:cluster_agent) } + let_it_be(:root_namespace) do + create(:group, + projects: [root_agent.project], + children: [ + create(:group, + projects: [nested_agent.project] + ) + ] + ) + end + + let(:namespace) { root_namespace } + let(:namespace_cluster_agent_mappings) do + [ + build_stubbed( + :remote_development_namespace_cluster_agent_mapping, + user: user, + namespace: namespace, + agent: root_agent + ), + build_stubbed( + :remote_development_namespace_cluster_agent_mapping, + user: user, + namespace: namespace, + agent: nested_agent + ) + ] + end + + subject(:response) do + described_class.filter_valid_namespace_cluster_agent_mappings( + namespace_cluster_agent_mappings: namespace_cluster_agent_mappings + ) + end + + context 'when cluster agents exist within the namespace' do + it 'returns all cluster agents passed in the parameters' do + expect(response).to eq(namespace_cluster_agent_mappings) + end + end + + context 'when a cluster agent does not exist within the mapped namespace' do + # With this, all namespace-agent mappings will be bound to the nested namespace. + # As such, one of the mappings will be between the nested namespace and the root agent which is considered + # invalid and should be excluded from the results + let(:namespace) { root_namespace.children.first } + + it 'returns cluster agents excluding those that do not reside in the namespace' do + mappings_with_nested_agent = namespace_cluster_agent_mappings.filter do |mapping| + mapping.agent == nested_agent + end + + expect(response).to eq(mappings_with_nested_agent) + end + end + + context 'when a non-existent cluster agent is passed in the parameters' do + let(:nested_agent) { build_stubbed(:cluster_agent) } + + it 'returns cluster agents excluding those are non-existent' do + mappings_without_nested_agent = namespace_cluster_agent_mappings.filter do |mapping| + mapping.agent != nested_agent + end + + expect(response).to eq(mappings_without_nested_agent) + end + end + + context 'when an empty list of agents is passed in the parameters' do + let(:namespace_cluster_agent_mappings) { [] } + + it 'returns an empty array' do + expect(response).to eq([]) + end + end + end +end diff --git a/ee/spec/requests/api/graphql/remote_development/namespace/remote_development_cluster_agents/with_available_filter_arg_spec.rb b/ee/spec/requests/api/graphql/remote_development/namespace/remote_development_cluster_agents/with_available_filter_arg_spec.rb new file mode 100644 index 00000000000000..328f0ebe03bf27 --- /dev/null +++ b/ee/spec/requests/api/graphql/remote_development/namespace/remote_development_cluster_agents/with_available_filter_arg_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.namespace.remote_development_cluster_agents(filter: AVAILABLE)', + feature_category: :remote_development do + include GraphqlHelpers + include StubFeatureFlags + + let_it_be(:user) { create(:user) } + let(:stub_finder_response) { [agent] } + let_it_be(:current_user) { user } + # Setup cluster and user such that the user has the bare minimum permissions + # to be able to receive the agent when calling the API i.e. the user has Developer access + # to the agent project ONLY (and not a group-level access) + let_it_be(:agent) do + create(:ee_cluster_agent, :in_group, :with_remote_development_agent_config).tap do |agent| + agent.project.add_developer(user) + end + end + + let_it_be(:namespace) { agent.project.namespace } + let_it_be(:namespace_agent_mapping) do + create( + :remote_development_namespace_cluster_agent_mapping, + user: user, + agent: agent, + namespace: namespace + ) + end + + let(:fields) do + <<~QUERY + nodes { + #{all_graphql_fields_for('cluster_agents'.classify, max_depth: 1)} + } + QUERY + end + + let(:agent_names_in_response) { subject.pluck('name') } + let(:query) do + graphql_query_for( + :namespace, + { full_path: namespace.full_path }, + query_graphql_field( + :remote_development_cluster_agents, + { filter: :AVAILABLE }, + fields + ) + ) + end + + subject { graphql_data.dig('namespace', 'remoteDevelopmentClusterAgents', 'nodes') } + + before do + stub_licensed_features(remote_development: true) + end + + context 'when the params are valid' do + it 'returns a cluster agent' do + post_graphql(query, current_user: current_user) + + expect(agent_names_in_response).to eq([agent.name]) + end + end + + context 'when remote_development feature is unlicensed' do + before do + stub_licensed_features(remote_development: false) + end + + it 'returns an error' do + post_graphql(query, current_user: current_user) + + expect_graphql_errors_to_include "'remote_development' licensed feature is not available" + end + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(remote_development_namespace_agent_authorization: false) + end + + it 'returns an error' do + post_graphql(query, current_user: current_user) + + expect_graphql_errors_to_include "'remote_development_namespace_agent_authorization' feature flag is disabled" + end + end + + context 'when the provided namespace is not a group namespace' do + let(:namespace) { agent.project.project_namespace } + + it 'returns an error' do + post_graphql(query, current_user: current_user) + + expect_graphql_errors_to_include "does not exist or you don't have permission to perform this action" + end + end + + context 'when user does not have access to the project' do + # simulate test conditions by creating the maximum privileged user that does/should + # not have the permission to access the agent + let(:current_user) do + create(:user).tap do |user| + agent.project.add_reporter(user) + end + end + + it 'skips agents for which the user does not have access' do + post_graphql(query, current_user: current_user) + + expect(agent_names_in_response).to eq([]) + end + end +end -- GitLab From 6750aaffd4a47ae1dec7ba7d089ee9099702af54 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Thu, 18 Apr 2024 18:48:25 +0800 Subject: [PATCH 2/7] Rename reference in enum from group to namespace --- doc/api/graphql/reference/index.md | 20 +++++++++---------- .../agents_for_namespace_resolver.rb | 2 +- .../group_cluster_agent_filter_enum.rb | 13 ------------ .../namespace_cluster_agent_filter_enum.rb | 13 ++++++++++++ 4 files changed, 24 insertions(+), 24 deletions(-) delete mode 100644 ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb create mode 100644 ee/app/graphql/types/remote_development/namespace_cluster_agent_filter_enum.rb diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 1b174b973b7004..c9198be8ca66f7 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -21796,7 +21796,7 @@ four standard [pagination arguments](#pagination-arguments): | Name | Type | Description | | ---- | ---- | ----------- | -| `filter` | [`GroupClusterAgentFilter!`](#groupclusteragentfilter) | Filter the types of cluster agents to return. | +| `filter` | [`NamespaceClusterAgentFilter!`](#namespaceclusteragentfilter) | Filter the types of cluster agents to return. | ##### `Group.runnerCloudProvisioning` @@ -25132,7 +25132,7 @@ four standard [pagination arguments](#pagination-arguments): | Name | Type | Description | | ---- | ---- | ----------- | -| `filter` | [`GroupClusterAgentFilter!`](#groupclusteragentfilter) | Filter the types of cluster agents to return. | +| `filter` | [`NamespaceClusterAgentFilter!`](#namespaceclusteragentfilter) | Filter the types of cluster agents to return. | ##### `Namespace.scanExecutionPolicies` @@ -32851,14 +32851,6 @@ Values for sorting artifacts. | `UPLOAD_TIME_ASC` | Ordered by `upload_time` in ascending order. | | `UPLOAD_TIME_DESC` | Ordered by `upload_time` in descending order. | -### `GroupClusterAgentFilter` - -Possible filter types for remote development cluster agents in a group. - -| Value | Description | -| ----- | ----------- | -| `AVAILABLE` | Cluster agents in the group that can be used for hosting workspaces. | - ### `GroupMemberRelation` Group member relation. @@ -33430,6 +33422,14 @@ Different toggles for changing mutator behavior. | `REMOVE` | Performs a removal operation. | | `REPLACE` | Performs a replace operation. | +### `NamespaceClusterAgentFilter` + +Possible filter types for remote development cluster agents in a namespace. + +| Value | Description | +| ----- | ----------- | +| `AVAILABLE` | Cluster agents in the namespace that can be used for hosting workspaces. | + ### `NamespaceProjectSort` Values for sorting projects. diff --git a/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb b/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb index 7a1ec958a36206..c3effd5d16036d 100644 --- a/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb +++ b/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb @@ -7,7 +7,7 @@ class AgentsForNamespaceResolver < ::Resolvers::BaseResolver type Types::Clusters::AgentType.connection_type, null: true - argument :filter, Types::RemoteDevelopment::GroupClusterAgentFilterEnum, + argument :filter, Types::RemoteDevelopment::NamespaceClusterAgentFilterEnum, required: true, description: 'Filter the types of cluster agents to return.' diff --git a/ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb b/ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb deleted file mode 100644 index 200617af1d7dcb..00000000000000 --- a/ee/app/graphql/types/remote_development/group_cluster_agent_filter_enum.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module Types - module RemoteDevelopment - class GroupClusterAgentFilterEnum < BaseEnum - graphql_name 'GroupClusterAgentFilter' - description 'Possible filter types for remote development cluster agents in a group' - - value 'AVAILABLE', - description: "Cluster agents in the group that can be used for hosting workspaces.", value: 'AVAILABLE' - end - end -end diff --git a/ee/app/graphql/types/remote_development/namespace_cluster_agent_filter_enum.rb b/ee/app/graphql/types/remote_development/namespace_cluster_agent_filter_enum.rb new file mode 100644 index 00000000000000..8298a18a2daa73 --- /dev/null +++ b/ee/app/graphql/types/remote_development/namespace_cluster_agent_filter_enum.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Types + module RemoteDevelopment + class NamespaceClusterAgentFilterEnum < BaseEnum + graphql_name 'NamespaceClusterAgentFilter' + description 'Possible filter types for remote development cluster agents in a namespace' + + value 'AVAILABLE', + description: "Cluster agents in the namespace that can be used for hosting workspaces.", value: 'AVAILABLE' + end + end +end -- GitLab From 738b79749ee3235761d577f847b5a757d90c5988 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Thu, 25 Apr 2024 03:26:37 +0000 Subject: [PATCH 3/7] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Chad Woolley --- .../namespace_cluster_agent_mappings/validations.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb b/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb index f0919108785f1d..ea5d9217316ee8 100644 --- a/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb +++ b/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb @@ -7,8 +7,7 @@ class Validations # agents are excluded from the response. A cluster agent is said to reside within a namespace # if the namespace id is present in the traversal ids of the project bound to the cluster agent # - # @param [Array] - # namespace_cluster_agent_mappings + # @param [Array] namespace_cluster_agent_mappings # @return [Array] def self.filter_valid_namespace_cluster_agent_mappings(namespace_cluster_agent_mappings:) agent_ids = namespace_cluster_agent_mappings.map(&:cluster_agent_id) -- GitLab From 52e32338a136ca9827a0abcde55895d78ff43365 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Thu, 25 Apr 2024 11:28:35 +0800 Subject: [PATCH 4/7] Add missing YARD tags for the private function --- .../namespace_cluster_agent_mappings/validations.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb b/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb index ea5d9217316ee8..11709fcc99b5f5 100644 --- a/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb +++ b/ee/lib/remote_development/namespace_cluster_agent_mappings/validations.rb @@ -17,6 +17,8 @@ def self.filter_valid_namespace_cluster_agent_mappings(namespace_cluster_agent_m end end + # @param [Array] cluster_agent_ids + # @return [Hash] def self.traversal_ids_for_cluster_agents(cluster_agent_ids:) agents_by_id = Clusters::Agent.id_in(cluster_agent_ids).index_by(&:id) -- GitLab From cf86e217e0b016eec5512e7e23af2ea842d2e5e3 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Thu, 25 Apr 2024 11:37:21 +0800 Subject: [PATCH 5/7] Rename describe blocks to context in spec --- .../finders/remote_development/cluster_agents_finder_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb b/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb index b886c7e704efd3..cfe7a4a0cdb0be 100644 --- a/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb +++ b/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb @@ -37,7 +37,7 @@ ).to_a end - describe 'with filter_type set to AVAILABLE' do + context 'with filter_type set to AVAILABLE' do context 'when all cluster agents are bound to the namespace' do it 'returns all cluster agents passed in the parameters' do expect(response).to eq([nested_agent]) @@ -82,7 +82,7 @@ end end - describe 'with an invalid value for filter_type' do + context 'with an invalid value for filter_type' do let(:filter) { "some_invalid_value" } it 'raises a RuntimeError' do -- GitLab From ac781805c396548ec32fb5f1377c2e37d7778735 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Thu, 25 Apr 2024 11:43:26 +0800 Subject: [PATCH 6/7] Use lowercase sym as filter value in agents finder --- ee/app/finders/remote_development/cluster_agents_finder.rb | 2 +- .../remote_development/agents_for_namespace_resolver.rb | 2 +- .../finders/remote_development/cluster_agents_finder_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ee/app/finders/remote_development/cluster_agents_finder.rb b/ee/app/finders/remote_development/cluster_agents_finder.rb index cceba252b51972..8b74eb76b03793 100644 --- a/ee/app/finders/remote_development/cluster_agents_finder.rb +++ b/ee/app/finders/remote_development/cluster_agents_finder.rb @@ -4,7 +4,7 @@ module RemoteDevelopment class ClusterAgentsFinder def self.execute(namespace:, filter:) case filter - when :AVAILABLE + when :available relevant_mappings = RemoteDevelopmentNamespaceClusterAgentMapping.for_namespaces(namespace.traversal_ids) relevant_mappings = NamespaceClusterAgentMappings::Validations.filter_valid_namespace_cluster_agent_mappings( namespace_cluster_agent_mappings: relevant_mappings diff --git a/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb b/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb index c3effd5d16036d..0d77e6131256e1 100644 --- a/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb +++ b/ee/app/graphql/resolvers/remote_development/agents_for_namespace_resolver.rb @@ -26,7 +26,7 @@ def resolve(**args) ::RemoteDevelopment::ClusterAgentsFinder.execute( namespace: @object, - filter: args[:filter].to_sym + filter: args[:filter].downcase.to_sym ) end end diff --git a/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb b/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb index cfe7a4a0cdb0be..476357b3a0195a 100644 --- a/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb +++ b/ee/spec/finders/remote_development/cluster_agents_finder_spec.rb @@ -19,7 +19,7 @@ let(:nested_namespace) { root_namespace.children.first } let(:namespace) { root_namespace } - let(:filter) { :AVAILABLE } + let(:filter) { :available } before_all do create( @@ -37,7 +37,7 @@ ).to_a end - context 'with filter_type set to AVAILABLE' do + context 'with filter_type set to available' do context 'when all cluster agents are bound to the namespace' do it 'returns all cluster agents passed in the parameters' do expect(response).to eq([nested_agent]) -- GitLab From 129ea78e35b622e6a2f452afdfff0a19667f01d0 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Thu, 25 Apr 2024 13:22:56 +0800 Subject: [PATCH 7/7] Add spec for mapping model --- ...nt_namespace_cluster_agent_mapping_spec.rb | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 ee/spec/models/remote_development/remote_development_namespace_cluster_agent_mapping_spec.rb diff --git a/ee/spec/models/remote_development/remote_development_namespace_cluster_agent_mapping_spec.rb b/ee/spec/models/remote_development/remote_development_namespace_cluster_agent_mapping_spec.rb new file mode 100644 index 00000000000000..c78b1bfd2cc2fa --- /dev/null +++ b/ee/spec/models/remote_development/remote_development_namespace_cluster_agent_mapping_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::RemoteDevelopmentNamespaceClusterAgentMapping, feature_category: :remote_development do + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:namespace) { create(:group) } + let_it_be_with_reload(:agent) { create(:cluster_agent) } + + subject(:namespace_cluster_agent_mapping) do + create( + :remote_development_namespace_cluster_agent_mapping, + user: user, + agent: agent, + namespace: namespace + ) + end + + describe 'associations' do + context "for belongs_to" do + it do + is_expected + .to belong_to(:user) + .class_name('User') + .with_foreign_key(:creator_id) + .inverse_of(:created_remote_development_namespace_cluster_agent_mappings) + end + + it do + is_expected + .to belong_to(:namespace) + .inverse_of(:remote_development_namespace_cluster_agent_mappings) + end + + it do + is_expected + .to belong_to(:agent) + .class_name('Clusters::Agent') + .with_foreign_key(:cluster_agent_id) + .inverse_of(:remote_development_namespace_cluster_agent_mappings) + end + end + + context 'when from factory' do + it 'has correct associations from factory' do + expect(namespace_cluster_agent_mapping.user).to eq(user) + expect(namespace_cluster_agent_mapping.agent).to eq(agent) + expect(namespace_cluster_agent_mapping.namespace).to eq(namespace) + end + end + end +end -- GitLab