diff options
31 files changed, 884 insertions, 78 deletions
diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index 5dd4040628f..5d6ce4f342c 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -3,12 +3,12 @@ class Clusters::ClustersController < Clusters::BaseController include RoutableActions - before_action :cluster, except: [:index, :new, :create_gcp, :create_user, :authorize_aws_role] + before_action :cluster, only: [:cluster_status, :show, :update, :destroy] before_action :generate_gcp_authorize_url, only: [:new] before_action :validate_gcp_token, only: [:new] before_action :gcp_cluster, only: [:new] before_action :user_cluster, only: [:new] - before_action :authorize_create_cluster!, only: [:new, :authorize_aws_role] + before_action :authorize_create_cluster!, only: [:new, :authorize_aws_role, :revoke_aws_role, :aws_proxy] before_action :authorize_update_cluster!, only: [:update] before_action :authorize_admin_cluster!, only: [:destroy] before_action :update_applications_status, only: [:cluster_status] @@ -117,6 +117,19 @@ class Clusters::ClustersController < Clusters::BaseController end end + def create_aws + @aws_cluster = ::Clusters::CreateService + .new(current_user, create_aws_cluster_params) + .execute + .present(current_user: current_user) + + if @aws_cluster.persisted? + head :created, location: @aws_cluster.show_path + else + render status: :unprocessable_entity, json: @aws_cluster.errors + end + end + def create_user @user_cluster = ::Clusters::CreateService .new(current_user, create_user_cluster_params) @@ -140,6 +153,21 @@ class Clusters::ClustersController < Clusters::BaseController role.save ? respond_201 : respond_422 end + def revoke_aws_role + current_user.aws_role&.destroy + + head :no_content + end + + def aws_proxy + response = Clusters::Aws::ProxyService.new( + current_user.aws_role, + params: params + ).execute + + render json: response.body, status: response.status + end + private def destroy_params @@ -200,6 +228,28 @@ class Clusters::ClustersController < Clusters::BaseController ) end + def create_aws_cluster_params + params.require(:cluster).permit( + :enabled, + :name, + :environment_scope, + :managed, + provider_aws_attributes: [ + :key_name, + :role_arn, + :region, + :vpc_id, + :instance_type, + :num_nodes, + :security_group_id, + subnet_ids: [] + ]).merge( + provider_type: :aws, + platform_type: :kubernetes, + clusterable: clusterable.subject + ) + end + def create_user_cluster_params params.require(:cluster).permit( :enabled, diff --git a/app/helpers/clusters_helper.rb b/app/helpers/clusters_helper.rb index 0cfb45a12e5..0037c49f134 100644 --- a/app/helpers/clusters_helper.rb +++ b/app/helpers/clusters_helper.rb @@ -40,7 +40,7 @@ module ClustersHelper def has_rbac_enabled?(cluster) return cluster.platform_kubernetes_rbac? if cluster.platform_kubernetes - !cluster.provider.legacy_abac? + cluster.provider.has_rbac_enabled? end end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 0db1fe9d6dc..ac2b63b194f 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -56,6 +56,7 @@ module Clusters has_many :kubernetes_namespaces accepts_nested_attributes_for :provider_gcp, update_only: true + accepts_nested_attributes_for :provider_aws, update_only: true accepts_nested_attributes_for :platform_kubernetes, update_only: true validates :name, cluster_name: true @@ -73,6 +74,7 @@ module Clusters delegate :status, to: :provider, allow_nil: true delegate :status_reason, to: :provider, allow_nil: true delegate :on_creation?, to: :provider, allow_nil: true + delegate :knative_pre_installed?, to: :provider, allow_nil: true delegate :active?, to: :platform_kubernetes, prefix: true, allow_nil: true delegate :rbac?, to: :platform_kubernetes, prefix: true, allow_nil: true @@ -258,10 +260,6 @@ module Clusters end end - def knative_pre_installed? - provider&.knative_pre_installed? - end - private def unique_management_project_environment_scope diff --git a/app/models/clusters/providers/aws.rb b/app/models/clusters/providers/aws.rb index 89eb56aa41f..78eb75ddcc0 100644 --- a/app/models/clusters/providers/aws.rb +++ b/app/models/clusters/providers/aws.rb @@ -9,7 +9,6 @@ module Clusters self.table_name = 'cluster_providers_aws' belongs_to :cluster, inverse_of: :provider_aws, class_name: 'Clusters::Cluster' - belongs_to :created_by_user, class_name: 'User' default_value_for :region, 'us-east-1' default_value_for :num_nodes, 3 @@ -55,6 +54,18 @@ module Clusters ::Aws::Credentials.new(access_key_id, secret_access_key, session_token) end end + + def has_rbac_enabled? + true + end + + def knative_pre_installed? + false + end + + def created_by_user + cluster.user + end end end end diff --git a/app/models/clusters/providers/gcp.rb b/app/models/clusters/providers/gcp.rb index f871674676f..2ca7d0249dc 100644 --- a/app/models/clusters/providers/gcp.rb +++ b/app/models/clusters/providers/gcp.rb @@ -54,6 +54,10 @@ module Clusters assign_attributes(operation_id: operation_id) end + def has_rbac_enabled? + !legacy_abac + end + def knative_pre_installed? cloud_run? end diff --git a/app/models/project_services/data_fields.rb b/app/models/project_services/data_fields.rb index cffb493d569..cf406a784ce 100644 --- a/app/models/project_services/data_fields.rb +++ b/app/models/project_services/data_fields.rb @@ -50,7 +50,7 @@ module DataFields end def data_fields_present? - data_fields.persisted? + data_fields.present? rescue NotImplementedError false end diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb index d6f67c1f2e5..2306f55f1f4 100644 --- a/app/presenters/clusterable_presenter.rb +++ b/app/presenters/clusterable_presenter.rb @@ -29,10 +29,18 @@ class ClusterablePresenter < Gitlab::View::Presenter::Delegated new_polymorphic_path([clusterable, :cluster], options) end + def aws_api_proxy_path(resource) + polymorphic_path([clusterable, :clusters], action: :aws_proxy, resource: resource) + end + def authorize_aws_role_path polymorphic_path([clusterable, :clusters], action: :authorize_aws_role) end + def revoke_aws_role_path + polymorphic_path([clusterable, :clusters], action: :revoke_aws_role) + end + def create_user_clusters_path polymorphic_path([clusterable, :clusters], action: :create_user) end @@ -41,6 +49,10 @@ class ClusterablePresenter < Gitlab::View::Presenter::Delegated polymorphic_path([clusterable, :clusters], action: :create_gcp) end + def create_aws_clusters_path + polymorphic_path([clusterable, :clusters], action: :create_aws) + end + def cluster_status_cluster_path(cluster, params = {}) raise NotImplementedError end diff --git a/app/presenters/instance_clusterable_presenter.rb b/app/presenters/instance_clusterable_presenter.rb index f820c0f6b42..c6572e8ce71 100644 --- a/app/presenters/instance_clusterable_presenter.rb +++ b/app/presenters/instance_clusterable_presenter.rb @@ -52,11 +52,26 @@ class InstanceClusterablePresenter < ClusterablePresenter create_gcp_admin_clusters_path end + override :create_aws_clusters_path + def create_aws_clusters_path + create_aws_admin_clusters_path + end + override :authorize_aws_role_path def authorize_aws_role_path authorize_aws_role_admin_clusters_path end + override :revoke_aws_role_path + def revoke_aws_role_path + revoke_aws_role_admin_clusters_path + end + + override :aws_api_proxy_path + def aws_api_proxy_path(resource) + aws_proxy_admin_clusters_path(resource: resource) + end + override :empty_state_help_text def empty_state_help_text s_('ClusterIntegration|Adding an integration will share the cluster across all projects.') diff --git a/app/services/clusters/aws/fetch_credentials_service.rb b/app/services/clusters/aws/fetch_credentials_service.rb index 29442208c62..2724d4b657b 100644 --- a/app/services/clusters/aws/fetch_credentials_service.rb +++ b/app/services/clusters/aws/fetch_credentials_service.rb @@ -3,11 +3,13 @@ module Clusters module Aws class FetchCredentialsService - attr_reader :provider + attr_reader :provision_role MissingRoleError = Class.new(StandardError) - def initialize(provider) + def initialize(provision_role, region:, provider: nil) + @provision_role = provision_role + @region = region @provider = provider end @@ -24,12 +26,10 @@ module Clusters private - def provision_role - provider.created_by_user.aws_role - end + attr_reader :provider, :region def client - ::Aws::STS::Client.new(credentials: gitlab_credentials, region: provider.region) + ::Aws::STS::Client.new(credentials: gitlab_credentials, region: region) end def gitlab_credentials @@ -45,7 +45,11 @@ module Clusters end def session_name - "gitlab-eks-cluster-#{provider.cluster_id}-user-#{provider.created_by_user_id}" + if provider.present? + "gitlab-eks-cluster-#{provider.cluster_id}-user-#{provision_role.user_id}" + else + "gitlab-eks-autofill-user-#{provision_role.user_id}" + end end end end diff --git a/app/services/clusters/aws/provision_service.rb b/app/services/clusters/aws/provision_service.rb index 6a3b0dffae2..35fe8433b4d 100644 --- a/app/services/clusters/aws/provision_service.rb +++ b/app/services/clusters/aws/provision_service.rb @@ -21,7 +21,7 @@ module Clusters end rescue Clusters::Aws::FetchCredentialsService::MissingRoleError provider.make_errored!('Amazon role is not configured') - rescue ::Aws::Errors::MissingCredentialsError, Settingslogic::MissingSetting + rescue ::Aws::Errors::MissingCredentialsError provider.make_errored!('Amazon credentials are not configured') rescue ::Aws::STS::Errors::ServiceError => e provider.make_errored!("Amazon authentication failed; #{e.message}") @@ -31,8 +31,16 @@ module Clusters private + def provision_role + provider.created_by_user&.aws_role + end + def credentials - @credentials ||= Clusters::Aws::FetchCredentialsService.new(provider).execute + @credentials ||= Clusters::Aws::FetchCredentialsService.new( + provision_role, + provider: provider, + region: provider.region + ).execute end def configure_provider_credentials diff --git a/app/services/clusters/aws/proxy_service.rb b/app/services/clusters/aws/proxy_service.rb new file mode 100644 index 00000000000..df8fc480005 --- /dev/null +++ b/app/services/clusters/aws/proxy_service.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +module Clusters + module Aws + class ProxyService + DEFAULT_REGION = 'us-east-1' + + BadRequest = Class.new(StandardError) + Response = Struct.new(:status, :body) + + def initialize(role, params:) + @role = role + @params = params + end + + def execute + api_response = request_from_api! + + Response.new(:ok, api_response.to_hash) + rescue *service_errors + Response.new(:bad_request, {}) + end + + private + + attr_reader :role, :params + + def request_from_api! + case requested_resource + when 'key_pairs' + ec2_client.describe_key_pairs + + when 'instance_types' + instance_types + + when 'roles' + iam_client.list_roles + + when 'regions' + ec2_client.describe_regions + + when 'security_groups' + raise BadRequest unless vpc_id.present? + + ec2_client.describe_security_groups(vpc_filter) + + when 'subnets' + raise BadRequest unless vpc_id.present? + + ec2_client.describe_subnets(vpc_filter) + + when 'vpcs' + ec2_client.describe_vpcs + + else + raise BadRequest + end + end + + def requested_resource + params[:resource] + end + + def vpc_id + params[:vpc_id] + end + + def region + params[:region] || DEFAULT_REGION + end + + def vpc_filter + { + filters: [{ + name: "vpc-id", + values: [vpc_id] + }] + } + end + + ## + # Unfortunately the EC2 API doesn't provide a list of + # possible instance types. There is a workaround, using + # the Pricing API, but instead of requiring the + # user to grant extra permissions for this we use the + # values that validate the CloudFormation template. + def instance_types + { + instance_types: cluster_stack_instance_types.map { |type| Hash(instance_type_name: type) } + } + end + + def cluster_stack_instance_types + YAML.safe_load(stack_template).dig('Parameters', 'NodeInstanceType', 'AllowedValues') + end + + def stack_template + File.read(Rails.root.join('vendor', 'aws', 'cloudformation', 'eks_cluster.yaml')) + end + + def ec2_client + ::Aws::EC2::Client.new(client_options) + end + + def iam_client + ::Aws::IAM::Client.new(client_options) + end + + def credentials + Clusters::Aws::FetchCredentialsService.new(role, region: region).execute + end + + def client_options + { + credentials: credentials, + region: region, + http_open_timeout: 5, + http_read_timeout: 10 + } + end + + def service_errors + [ + BadRequest, + Clusters::Aws::FetchCredentialsService::MissingRoleError, + ::Aws::Errors::MissingCredentialsError, + ::Aws::EC2::Errors::ServiceError, + ::Aws::IAM::Errors::ServiceError, + ::Aws::STS::Errors::ServiceError + ] + end + end + end +end diff --git a/app/views/clusters/clusters/_banner.html.haml b/app/views/clusters/clusters/_banner.html.haml index 4b4278075a6..7d97aaccbcf 100644 --- a/app/views/clusters/clusters/_banner.html.haml +++ b/app/views/clusters/clusters/_banner.html.haml @@ -1,10 +1,10 @@ .hidden.js-cluster-error.bs-callout.bs-callout-danger{ role: 'alert' } - = s_('ClusterIntegration|Something went wrong while creating your Kubernetes cluster on Google Kubernetes Engine') + = s_('ClusterIntegration|Something went wrong while creating your Kubernetes cluster') %p.js-error-reason .hidden.js-cluster-creating.bs-callout.bs-callout-info{ role: 'alert' } %span.spinner.spinner-dark.spinner-sm{ 'aria-label': 'Loading' } - %span.prepend-left-4= s_('ClusterIntegration|Kubernetes cluster is being created on Google Kubernetes Engine...') + %span.prepend-left-4= s_('ClusterIntegration|Kubernetes cluster is being created...') .hidden.row.js-cluster-api-unreachable.bs-callout.bs-callout-warning{ role: 'alert' } .col-11 @@ -19,4 +19,4 @@ %button.js-close-banner.close.cluster-application-banner-close.h-100.m-0= "×" .hidden.js-cluster-success.bs-callout.bs-callout-success{ role: 'alert' } - = s_("ClusterIntegration|Kubernetes cluster was successfully created on Google Kubernetes Engine.") + = s_("ClusterIntegration|Kubernetes cluster was successfully created.") diff --git a/app/views/clusters/clusters/aws/_new.html.haml b/app/views/clusters/clusters/aws/_new.html.haml index fe8b606af70..48467f88f52 100644 --- a/app/views/clusters/clusters/aws/_new.html.haml +++ b/app/views/clusters/clusters/aws/_new.html.haml @@ -4,6 +4,15 @@ - else .js-create-eks-cluster-form-container{ data: { 'gitlab-managed-cluster-help-path' => help_page_path('user/project/clusters/index.md', anchor: 'gitlab-managed-clusters'), 'create-role-path' => clusterable.authorize_aws_role_path, + 'sign-out-path' => clusterable.revoke_aws_role_path, + 'create-cluster-path' => clusterable.create_aws_clusters_path, + 'get-roles-path' => clusterable.aws_api_proxy_path('roles'), + 'get-regions-path' => clusterable.aws_api_proxy_path('regions'), + 'get-key-pairs-path' => clusterable.aws_api_proxy_path('key_pairs'), + 'get-vpcs-path' => clusterable.aws_api_proxy_path('vpcs'), + 'get-subnets-path' => clusterable.aws_api_proxy_path('subnets'), + 'get-security-groups-path' => clusterable.aws_api_proxy_path('security_groups'), + 'get-instance-types-path' => clusterable.aws_api_proxy_path('instance_types'), 'account-id' => Gitlab::CurrentSettings.eks_account_id, 'external-id' => @aws_role.role_external_id, 'kubernetes-integration-help-path' => help_page_path('user/project/clusters/index'), diff --git a/changelogs/unreleased/34770-error-500-for-api-v4-projects-id-services-jira-nomethoderror-undefi.yml b/changelogs/unreleased/34770-error-500-for-api-v4-projects-id-services-jira-nomethoderror-undefi.yml new file mode 100644 index 00000000000..68de843402d --- /dev/null +++ b/changelogs/unreleased/34770-error-500-for-api-v4-projects-id-services-jira-nomethoderror-undefi.yml @@ -0,0 +1,5 @@ +--- +title: Fix project service API 500 error +merge_request: 19367 +author: +type: fixed diff --git a/config/routes.rb b/config/routes.rb index 23e97fe32dd..9fb4d94f068 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -142,7 +142,13 @@ Rails.application.routes.draw do collection do post :create_user post :create_gcp + post :create_aws post :authorize_aws_role + delete :revoke_aws_role + + scope :aws do + get 'api/:resource', to: 'clusters#aws_proxy', as: :aws_proxy + end end member do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0bd1b6411ea..6390b11e838 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3790,13 +3790,13 @@ msgstr "" msgid "ClusterIntegration|Kubernetes cluster details" msgstr "" -msgid "ClusterIntegration|Kubernetes cluster is being created on Google Kubernetes Engine..." +msgid "ClusterIntegration|Kubernetes cluster is being created..." msgstr "" msgid "ClusterIntegration|Kubernetes cluster name" msgstr "" -msgid "ClusterIntegration|Kubernetes cluster was successfully created on Google Kubernetes Engine." +msgid "ClusterIntegration|Kubernetes cluster was successfully created." msgstr "" msgid "ClusterIntegration|Kubernetes clusters allow you to use review apps, deploy your applications, run your pipelines, and much more in an easy way." @@ -4039,7 +4039,7 @@ msgstr "" msgid "ClusterIntegration|Something went wrong on our end." msgstr "" -msgid "ClusterIntegration|Something went wrong while creating your Kubernetes cluster on Google Kubernetes Engine" +msgid "ClusterIntegration|Something went wrong while creating your Kubernetes cluster" msgstr "" msgid "ClusterIntegration|Something went wrong while installing %{title}" diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index d3192593a78..e6decd24f0a 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -248,6 +248,69 @@ describe Admin::ClustersController do end end + describe 'POST #create_aws' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_aws_attributes: { + key_name: 'key', + role_arn: 'arn:role', + region: 'region', + vpc_id: 'vpc', + instance_type: 'instance type', + num_nodes: 3, + security_group_id: 'security group', + subnet_ids: %w(subnet1 subnet2) + } + } + } + end + + def post_create_aws + post :create_aws, params: params + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { post_create_aws }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Aws.count } + + cluster = Clusters::Cluster.instance_type.first + + expect(response.status).to eq(201) + expect(response.location).to eq(admin_cluster_path(cluster)) + expect(cluster).to be_aws + expect(cluster).to be_kubernetes + end + + context 'params are invalid' do + let(:params) do + { + cluster: { name: '' } + } + end + + it 'does not create a cluster' do + expect { post_create_aws }.not_to change { Clusters::Cluster.count } + + expect(response.status).to eq(422) + expect(response.content_type).to eq('application/json') + expect(response.body).to include('is invalid') + end + end + + describe 'security' do + before do + allow(WaitForClusterCreationWorker).to receive(:perform_in) + end + + it { expect { post_create_aws }.to be_allowed_for(:admin) } + it { expect { post_create_aws }.to be_denied_for(:user) } + it { expect { post_create_aws }.to be_denied_for(:external) } + end + end + describe 'POST #create_user' do let(:params) do { @@ -363,6 +426,27 @@ describe Admin::ClustersController do end end + describe 'DELETE revoke AWS role for EKS cluster' do + let!(:role) { create(:aws_role, user: admin) } + + def go + delete :revoke_aws_role + end + + it 'deletes the Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 204 + expect(admin.reload_aws_role).to be_nil + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + describe 'GET #cluster_status' do let(:cluster) { create(:cluster, :providing_by_gcp, :instance) } diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 538a270f567..d027405703b 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -372,6 +372,74 @@ describe Groups::ClustersController do end end + describe 'POST #create_aws' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_aws_attributes: { + key_name: 'key', + role_arn: 'arn:role', + region: 'region', + vpc_id: 'vpc', + instance_type: 'instance type', + num_nodes: 3, + security_group_id: 'security group', + subnet_ids: %w(subnet1 subnet2) + } + } + } + end + + def post_create_aws + post :create_aws, params: params.merge(group_id: group) + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { post_create_aws }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Aws.count } + + cluster = group.clusters.first + + expect(response.status).to eq(201) + expect(response.location).to eq(group_cluster_path(group, cluster)) + expect(cluster).to be_aws + expect(cluster).to be_kubernetes + end + + context 'params are invalid' do + let(:params) do + { + cluster: { name: '' } + } + end + + it 'does not create a cluster' do + expect { post_create_aws }.not_to change { Clusters::Cluster.count } + + expect(response.status).to eq(422) + expect(response.content_type).to eq('application/json') + expect(response.body).to include('is invalid') + end + end + + describe 'security' do + before do + allow(WaitForClusterCreationWorker).to receive(:perform_in) + end + + it { expect { post_create_aws }.to be_allowed_for(:admin) } + it { expect { post_create_aws }.to be_allowed_for(:owner).of(group) } + it { expect { post_create_aws }.to be_allowed_for(:maintainer).of(group) } + it { expect { post_create_aws }.to be_denied_for(:developer).of(group) } + it { expect { post_create_aws }.to be_denied_for(:reporter).of(group) } + it { expect { post_create_aws }.to be_denied_for(:guest).of(group) } + it { expect { post_create_aws }.to be_denied_for(:user) } + it { expect { post_create_aws }.to be_denied_for(:external) } + end + end + describe 'POST authorize AWS role for EKS cluster' do let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } let(:role_external_id) { '12345' } @@ -422,6 +490,32 @@ describe Groups::ClustersController do end end + describe 'DELETE revoke AWS role for EKS cluster' do + let!(:role) { create(:aws_role, user: user) } + + def go + delete :revoke_aws_role, params: { group_id: group } + end + + it 'deletes the Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 204 + expect(user.reload_aws_role).to be_nil + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(group) } + it { expect { go }.to be_allowed_for(:maintainer).of(group) } + it { expect { go }.to be_denied_for(:developer).of(group) } + it { expect { go }.to be_denied_for(:reporter).of(group) } + it { expect { go }.to be_denied_for(:guest).of(group) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + describe 'GET cluster_status' do let(:cluster) { create(:cluster, :providing_by_gcp, cluster_type: :group_type, groups: [group]) } diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 1b6b0ff025e..5a0512a042e 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -373,6 +373,74 @@ describe Projects::ClustersController do end end + describe 'POST #create_aws' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_aws_attributes: { + key_name: 'key', + role_arn: 'arn:role', + region: 'region', + vpc_id: 'vpc', + instance_type: 'instance type', + num_nodes: 3, + security_group_id: 'security group', + subnet_ids: %w(subnet1 subnet2) + } + } + } + end + + def post_create_aws + post :create_aws, params: params.merge(namespace_id: project.namespace, project_id: project) + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { post_create_aws }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Aws.count } + + cluster = project.clusters.first + + expect(response.status).to eq(201) + expect(response.location).to eq(project_cluster_path(project, cluster)) + expect(cluster).to be_aws + expect(cluster).to be_kubernetes + end + + context 'params are invalid' do + let(:params) do + { + cluster: { name: '' } + } + end + + it 'does not create a cluster' do + expect { post_create_aws }.not_to change { Clusters::Cluster.count } + + expect(response.status).to eq(422) + expect(response.content_type).to eq('application/json') + expect(response.body).to include('is invalid') + end + end + + describe 'security' do + before do + allow(WaitForClusterCreationWorker).to receive(:perform_in) + end + + it { expect { post_create_aws }.to be_allowed_for(:admin) } + it { expect { post_create_aws }.to be_allowed_for(:owner).of(project) } + it { expect { post_create_aws }.to be_allowed_for(:maintainer).of(project) } + it { expect { post_create_aws }.to be_denied_for(:developer).of(project) } + it { expect { post_create_aws }.to be_denied_for(:reporter).of(project) } + it { expect { post_create_aws }.to be_denied_for(:guest).of(project) } + it { expect { post_create_aws }.to be_denied_for(:user) } + it { expect { post_create_aws }.to be_denied_for(:external) } + end + end + describe 'POST authorize AWS role for EKS cluster' do let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } let(:role_external_id) { '12345' } @@ -423,6 +491,32 @@ describe Projects::ClustersController do end end + describe 'DELETE revoke AWS role for EKS cluster' do + let!(:role) { create(:aws_role, user: user) } + + def go + delete :revoke_aws_role, params: { namespace_id: project.namespace, project_id: project } + end + + it 'deletes the Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 204 + expect(user.reload_aws_role).to be_nil + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:maintainer).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + describe 'GET cluster_status' do let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } diff --git a/spec/factories/clusters/providers/aws.rb b/spec/factories/clusters/providers/aws.rb index dc18762607b..e4b10aa5f33 100644 --- a/spec/factories/clusters/providers/aws.rb +++ b/spec/factories/clusters/providers/aws.rb @@ -3,7 +3,6 @@ FactoryBot.define do factory :cluster_provider_aws, class: Clusters::Providers::Aws do association :cluster, platform_type: :kubernetes, provider_type: :aws - created_by_user factory: :user role_arn { 'arn:aws:iam::123456789012:role/role-name' } vpc_id { 'vpc-00000000000000000' } diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index b5ab9faa14b..bdc946a9c98 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -67,17 +67,17 @@ describe 'Gcp Cluster', :js do it 'user sees a cluster details page and creation status' do subject - expect(page).to have_content('Kubernetes cluster is being created on Google Kubernetes Engine...') + expect(page).to have_content('Kubernetes cluster is being created...') Clusters::Cluster.last.provider.make_created! - expect(page).to have_content('Kubernetes cluster was successfully created on Google Kubernetes Engine') + expect(page).to have_content('Kubernetes cluster was successfully created') end it 'user sees a error if something wrong during creation' do subject - expect(page).to have_content('Kubernetes cluster is being created on Google Kubernetes Engine...') + expect(page).to have_content('Kubernetes cluster is being created...') Clusters::Cluster.last.provider.make_errored!('Something wrong!') diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 198a3dfd37a..253866c8425 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -29,6 +29,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to delegate_method(:status).to(:provider) } it { is_expected.to delegate_method(:status_reason).to(:provider) } it { is_expected.to delegate_method(:on_creation?).to(:provider) } + it { is_expected.to delegate_method(:knative_pre_installed?).to(:provider) } it { is_expected.to delegate_method(:active?).to(:platform_kubernetes).with_prefix } it { is_expected.to delegate_method(:rbac?).to(:platform_kubernetes).with_prefix } it { is_expected.to delegate_method(:available?).to(:application_helm).with_prefix } @@ -916,26 +917,4 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do end end end - - describe '#knative_pre_installed?' do - subject { cluster.knative_pre_installed? } - - context 'with a GCP provider without cloud_run' do - let(:cluster) { create(:cluster, :provided_by_gcp) } - - it { is_expected.to be_falsey } - end - - context 'with a GCP provider with cloud_run' do - let(:cluster) { create(:cluster, :provided_by_gcp, :cloud_run_enabled) } - - it { is_expected.to be_truthy } - end - - context 'with a user provider' do - let(:cluster) { create(:cluster, :provided_by_user) } - - it { is_expected.to be_falsey } - end - end end diff --git a/spec/models/clusters/providers/aws_spec.rb b/spec/models/clusters/providers/aws_spec.rb index 95549d015c8..05d6e63288e 100644 --- a/spec/models/clusters/providers/aws_spec.rb +++ b/spec/models/clusters/providers/aws_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' describe Clusters::Providers::Aws do it { is_expected.to belong_to(:cluster) } - it { is_expected.to belong_to(:created_by_user) } it { is_expected.to validate_length_of(:key_name).is_at_least(1).is_at_most(255) } it { is_expected.to validate_length_of(:region).is_at_least(1).is_at_most(255) } @@ -108,4 +107,28 @@ describe Clusters::Providers::Aws do it { is_expected.to eq credentials } end + + describe '#created_by_user' do + let(:provider) { create(:cluster_provider_aws) } + + subject { provider.created_by_user } + + it { is_expected.to eq provider.cluster.user } + end + + describe '#has_rbac_enabled?' do + let(:provider) { create(:cluster_provider_aws) } + + subject { provider.has_rbac_enabled? } + + it { is_expected.to be_truthy } + end + + describe '#knative_pre_installed?' do + let(:provider) { create(:cluster_provider_aws) } + + subject { provider.knative_pre_installed? } + + it { is_expected.to be_falsey } + end end diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index 15e152519b4..e2fd777d131 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/spec/models/clusters/providers/gcp_spec.rb @@ -78,12 +78,20 @@ describe Clusters::Providers::Gcp do end end - describe '#legacy_abac?' do - let(:gcp) { build(:cluster_provider_gcp) } + describe '#has_rbac_enabled?' do + subject { gcp.has_rbac_enabled? } + + context 'when cluster is legacy_abac' do + let(:gcp) { create(:cluster_provider_gcp, :abac_enabled) } + + it { is_expected.to be_falsey } + end - subject { gcp } + context 'when cluster is not legacy_abac' do + let(:gcp) { create(:cluster_provider_gcp) } - it { is_expected.not_to be_legacy_abac } + it { is_expected.to be_truthy } + end end describe '#knative_pre_installed?' do diff --git a/spec/models/project_services/data_fields_spec.rb b/spec/models/project_services/data_fields_spec.rb index 146db0ae227..6b388a7222b 100644 --- a/spec/models/project_services/data_fields_spec.rb +++ b/spec/models/project_services/data_fields_spec.rb @@ -74,6 +74,12 @@ describe DataFields do expect(service.url_changed?).to be_falsy end end + + describe 'data_fields_present?' do + it 'returns true from the issue tracker service' do + expect(service.data_fields_present?).to be true + end + end end context 'when data are stored in data_fields' do @@ -92,6 +98,18 @@ describe DataFields do end end + context 'when service and data_fields are not persisted' do + let(:service) do + JiraService.new + end + + describe 'data_fields_present?' do + it 'returns true' do + expect(service.data_fields_present?).to be true + end + end + end + context 'when data are stored in properties' do let(:service) { create(:jira_service, :without_properties_callback, properties: properties) } diff --git a/spec/presenters/instance_clusterable_presenter_spec.rb b/spec/presenters/instance_clusterable_presenter_spec.rb new file mode 100644 index 00000000000..9f1268379f5 --- /dev/null +++ b/spec/presenters/instance_clusterable_presenter_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe InstanceClusterablePresenter do + include Gitlab::Routing.url_helpers + + let(:presenter) { described_class.new(instance) } + let(:cluster) { create(:cluster, :provided_by_gcp, :instance) } + let(:instance) { cluster.instance } + + describe '#create_aws_clusters_path' do + subject { described_class.new(instance).create_aws_clusters_path } + + it { is_expected.to eq(create_aws_admin_clusters_path) } + end + + describe '#authorize_aws_role_path' do + subject { described_class.new(instance).authorize_aws_role_path } + + it { is_expected.to eq(authorize_aws_role_admin_clusters_path) } + end + + describe '#revoke_aws_role_path' do + subject { described_class.new(instance).revoke_aws_role_path } + + it { is_expected.to eq(revoke_aws_role_admin_clusters_path) } + end + + describe '#aws_api_proxy_path' do + let(:resource) { 'resource' } + + subject { described_class.new(instance).aws_api_proxy_path(resource) } + + it { is_expected.to eq(aws_proxy_admin_clusters_path(resource: resource)) } + end +end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 8cf888d81ce..a080b59173f 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -102,7 +102,7 @@ describe API::Services do expect(json_response['properties'].keys).to match_array(service_instance.api_field_names) end - it "returns empty hash if properties and data fields are empty" do + it "returns empty hash or nil values if properties and data fields are empty" do # deprecated services are not valid for update initialized_service.update_attribute(:properties, {}) @@ -114,7 +114,7 @@ describe API::Services do get api("/projects/#{project.id}/services/#{dashed_service}", user) expect(response).to have_gitlab_http_status(200) - expect(json_response['properties'].keys).to be_empty + expect(json_response['properties'].values.compact).to be_empty end it "returns error when authenticated but not a project owner" do diff --git a/spec/services/clusters/aws/fetch_credentials_service_spec.rb b/spec/services/clusters/aws/fetch_credentials_service_spec.rb index 09f3be0534d..726d1c30603 100644 --- a/spec/services/clusters/aws/fetch_credentials_service_spec.rb +++ b/spec/services/clusters/aws/fetch_credentials_service_spec.rb @@ -4,21 +4,23 @@ require 'spec_helper' describe Clusters::Aws::FetchCredentialsService do describe '#execute' do + let(:user) { create(:user) } let(:provider) { create(:cluster_provider_aws) } let(:gitlab_access_key_id) { 'gitlab-access-key-id' } let(:gitlab_secret_access_key) { 'gitlab-secret-access-key' } + let(:region) { 'us-east-1' } let(:gitlab_credentials) { Aws::Credentials.new(gitlab_access_key_id, gitlab_secret_access_key) } - let(:sts_client) { Aws::STS::Client.new(credentials: gitlab_credentials, region: provider.region) } + let(:sts_client) { Aws::STS::Client.new(credentials: gitlab_credentials, region: region) } let(:assumed_role) { instance_double(Aws::AssumeRoleCredentials, credentials: assumed_role_credentials) } let(:assumed_role_credentials) { double } - subject { described_class.new(provider).execute } + subject { described_class.new(provision_role, region: region, provider: provider).execute } context 'provision role is configured' do - let(:provision_role) { create(:aws_role, user: provider.created_by_user) } + let(:provision_role) { create(:aws_role, user: user) } before do stub_application_setting(eks_access_key_id: gitlab_access_key_id) @@ -29,25 +31,34 @@ describe Clusters::Aws::FetchCredentialsService do .and_return(gitlab_credentials) expect(Aws::STS::Client).to receive(:new) - .with(credentials: gitlab_credentials, region: provider.region) + .with(credentials: gitlab_credentials, region: region) .and_return(sts_client) expect(Aws::AssumeRoleCredentials).to receive(:new) .with( client: sts_client, role_arn: provision_role.role_arn, - role_session_name: "gitlab-eks-cluster-#{provider.cluster_id}-user-#{provider.created_by_user_id}", + role_session_name: session_name, external_id: provision_role.role_external_id ).and_return(assumed_role) end - it { is_expected.to eq assumed_role_credentials } + context 'provider is specified' do + let(:session_name) { "gitlab-eks-cluster-#{provider.cluster_id}-user-#{user.id}" } + + it { is_expected.to eq assumed_role_credentials } + end + + context 'provider is not specifed' do + let(:provider) { nil } + let(:session_name) { "gitlab-eks-autofill-user-#{user.id}" } + + it { is_expected.to eq assumed_role_credentials } + end end context 'provision role is not configured' do - before do - expect(provider.created_by_user.aws_role).to be_nil - end + let(:provision_role) { nil } it 'raises an error' do expect { subject }.to raise_error(described_class::MissingRoleError, 'AWS provisioning role not configured') diff --git a/spec/services/clusters/aws/provision_service_spec.rb b/spec/services/clusters/aws/provision_service_spec.rb index 5c044b8732e..927ffaef002 100644 --- a/spec/services/clusters/aws/provision_service_spec.rb +++ b/spec/services/clusters/aws/provision_service_spec.rb @@ -6,6 +6,7 @@ describe Clusters::Aws::ProvisionService do describe '#execute' do let(:provider) { create(:cluster_provider_aws) } + let(:provision_role) { create(:aws_role, user: provider.created_by_user) } let(:client) { instance_double(Aws::CloudFormation::Client, create_stack: true) } let(:cloudformation_template) { double } let(:credentials) do @@ -34,7 +35,8 @@ describe Clusters::Aws::ProvisionService do before do allow(Clusters::Aws::FetchCredentialsService).to receive(:new) - .with(provider).and_return(double(execute: credentials)) + .with(provision_role, provider: provider, region: provider.region) + .and_return(double(execute: credentials)) allow(provider).to receive(:api_client) .and_return(client) @@ -107,15 +109,6 @@ describe Clusters::Aws::ProvisionService do include_examples 'provision error', 'Amazon credentials are not configured' end - context 'AWS credentials are not configured' do - before do - allow(Clusters::Aws::FetchCredentialsService).to receive(:new) - .and_raise(Settingslogic::MissingSetting) - end - - include_examples 'provision error', 'Amazon credentials are not configured' - end - context 'Authentication failure' do before do allow(Clusters::Aws::FetchCredentialsService).to receive(:new) diff --git a/spec/services/clusters/aws/proxy_service_spec.rb b/spec/services/clusters/aws/proxy_service_spec.rb new file mode 100644 index 00000000000..7b0e0512b95 --- /dev/null +++ b/spec/services/clusters/aws/proxy_service_spec.rb @@ -0,0 +1,210 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::Aws::ProxyService do + let(:role) { create(:aws_role) } + let(:credentials) { instance_double(Aws::Credentials) } + let(:client_instance) { instance_double(client) } + + let(:region) { 'region' } + let(:vpc_id) { } + let(:params) do + ActionController::Parameters.new({ + resource: resource, + region: region, + vpc_id: vpc_id + }) + end + + subject { described_class.new(role, params: params).execute } + + context 'external resources' do + before do + allow(Clusters::Aws::FetchCredentialsService).to receive(:new) do + double(execute: credentials) + end + + allow(client).to receive(:new) + .with( + credentials: credentials, region: region, + http_open_timeout: 5, http_read_timeout: 10) + .and_return(client_instance) + end + + shared_examples 'bad request' do + it 'returns an empty hash' do + expect(subject.status).to eq :bad_request + expect(subject.body).to eq({}) + end + end + + describe 'key_pairs' do + let(:client) { Aws::EC2::Client } + let(:resource) { 'key_pairs' } + let(:response) { double(to_hash: :key_pairs) } + + it 'requests a list of key pairs' do + expect(client_instance).to receive(:describe_key_pairs).once.and_return(response) + expect(subject.status).to eq :ok + expect(subject.body).to eq :key_pairs + end + end + + describe 'roles' do + let(:client) { Aws::IAM::Client } + let(:resource) { 'roles' } + let(:response) { double(to_hash: :roles) } + + it 'requests a list of roles' do + expect(client_instance).to receive(:list_roles).once.and_return(response) + expect(subject.status).to eq :ok + expect(subject.body).to eq :roles + end + end + + describe 'regions' do + let(:client) { Aws::EC2::Client } + let(:resource) { 'regions' } + let(:response) { double(to_hash: :regions) } + + it 'requests a list of regions' do + expect(client_instance).to receive(:describe_regions).once.and_return(response) + expect(subject.status).to eq :ok + expect(subject.body).to eq :regions + end + end + + describe 'security_groups' do + let(:client) { Aws::EC2::Client } + let(:resource) { 'security_groups' } + let(:response) { double(to_hash: :security_groups) } + + include_examples 'bad request' + + context 'VPC is specified' do + let(:vpc_id) { 'vpc-1' } + + it 'requests a list of security groups for a VPC' do + expect(client_instance).to receive(:describe_security_groups).once + .with(filters: [{ name: 'vpc-id', values: [vpc_id] }]) + .and_return(response) + expect(subject.status).to eq :ok + expect(subject.body).to eq :security_groups + end + end + end + + describe 'subnets' do + let(:client) { Aws::EC2::Client } + let(:resource) { 'subnets' } + let(:response) { double(to_hash: :subnets) } + + include_examples 'bad request' + + context 'VPC is specified' do + let(:vpc_id) { 'vpc-1' } + + it 'requests a list of subnets for a VPC' do + expect(client_instance).to receive(:describe_subnets).once + .with(filters: [{ name: 'vpc-id', values: [vpc_id] }]) + .and_return(response) + expect(subject.status).to eq :ok + expect(subject.body).to eq :subnets + end + end + end + + describe 'vpcs' do + let(:client) { Aws::EC2::Client } + let(:resource) { 'vpcs' } + let(:response) { double(to_hash: :vpcs) } + + it 'requests a list of VPCs' do + expect(client_instance).to receive(:describe_vpcs).once.and_return(response) + expect(subject.status).to eq :ok + expect(subject.body).to eq :vpcs + end + end + + context 'errors' do + let(:client) { Aws::EC2::Client } + + context 'unknown resource' do + let(:resource) { 'instances' } + + include_examples 'bad request' + end + + context 'client and configuration errors' do + let(:resource) { 'vpcs' } + + before do + allow(client_instance).to receive(:describe_vpcs).and_raise(error) + end + + context 'error fetching credentials' do + let(:error) { Aws::STS::Errors::ServiceError.new(nil, 'error message') } + + include_examples 'bad request' + end + + context 'credentials not configured' do + let(:error) { Aws::Errors::MissingCredentialsError.new('error message') } + + include_examples 'bad request' + end + + context 'role not configured' do + let(:error) { Clusters::Aws::FetchCredentialsService::MissingRoleError.new('error message') } + + include_examples 'bad request' + end + + context 'EC2 error' do + let(:error) { Aws::EC2::Errors::ServiceError.new(nil, 'error message') } + + include_examples 'bad request' + end + + context 'IAM error' do + let(:error) { Aws::IAM::Errors::ServiceError.new(nil, 'error message') } + + include_examples 'bad request' + end + + context 'STS error' do + let(:error) { Aws::STS::Errors::ServiceError.new(nil, 'error message') } + + include_examples 'bad request' + end + end + end + end + + context 'local resources' do + describe 'instance_types' do + let(:resource) { 'instance_types' } + let(:cloudformation_template) { double } + let(:instance_types) { double(dig: %w(t3.small)) } + + before do + allow(File).to receive(:read) + .with(Rails.root.join('vendor', 'aws', 'cloudformation', 'eks_cluster.yaml')) + .and_return(cloudformation_template) + + allow(YAML).to receive(:safe_load) + .with(cloudformation_template) + .and_return(instance_types) + end + + it 'returns a list of instance types' do + expect(subject.status).to eq :ok + expect(subject.body).to have_key(:instance_types) + expect(subject.body[:instance_types]).to match_array([ + instance_type_name: 't3.small' + ]) + end + end + end +end diff --git a/vendor/aws/cloudformation/eks_cluster.yaml b/vendor/aws/cloudformation/eks_cluster.yaml index ac09fc7ccca..c32f54d66dc 100644 --- a/vendor/aws/cloudformation/eks_cluster.yaml +++ b/vendor/aws/cloudformation/eks_cluster.yaml @@ -1,5 +1,5 @@ --- -AWSTemplateFormatVersion: 2010-09-09 +AWSTemplateFormatVersion: "2010-09-09" Description: GitLab EKS Cluster Parameters: @@ -189,7 +189,7 @@ Resources: Type: AWS::IAM::Role Properties: AssumeRolePolicyDocument: - Version: 2012-10-17 + Version: "2012-10-17" Statement: - Effect: Allow Principal: |