diff options
57 files changed, 871 insertions, 784 deletions
diff --git a/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue b/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue index 641343b8150..d04d0ff2a6d 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue +++ b/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue @@ -22,10 +22,7 @@ const { mapState: mapSecurityGroupsState, mapActions: mapSecurityGroupsActions, } = createNamespacedHelpers('securityGroups'); -const { - mapState: mapInstanceTypesState, - mapActions: mapInstanceTypesActions, -} = createNamespacedHelpers('instanceTypes'); +const { mapState: mapInstanceTypesState } = createNamespacedHelpers('instanceTypes'); export default { components: { @@ -265,12 +262,10 @@ export default { mounted() { this.fetchRegions(); this.fetchRoles(); - this.fetchInstanceTypes(); }, methods: { ...mapActions([ 'createCluster', - 'signOut', 'setClusterName', 'setEnvironmentScope', 'setKubernetesVersion', @@ -290,7 +285,6 @@ export default { ...mapRolesActions({ fetchRoles: 'fetchItems' }), ...mapKeyPairsActions({ fetchKeyPairs: 'fetchItems' }), ...mapSecurityGroupsActions({ fetchSecurityGroups: 'fetchItems' }), - ...mapInstanceTypesActions({ fetchInstanceTypes: 'fetchItems' }), setRegionAndFetchVpcsAndKeyPairs(region) { this.setRegion({ region }); this.setVpc({ vpc: null }); @@ -316,11 +310,6 @@ export default { {{ s__('ClusterIntegration|Enter the details for your Amazon EKS Kubernetes cluster') }} </h2> <div class="mb-3" v-html="kubernetesIntegrationHelpText"></div> - <div class="mb-3"> - <button class="btn btn-link js-sign-out" @click.prevent="signOut()"> - {{ s__('ClusterIntegration|Select a different AWS role') }} - </button> - </div> <div class="form-group"> <label class="label-bold" for="eks-cluster-name">{{ s__('ClusterIntegration|Kubernetes cluster name') diff --git a/app/assets/javascripts/create_cluster/eks_cluster/components/service_credentials_form.vue b/app/assets/javascripts/create_cluster/eks_cluster/components/service_credentials_form.vue index 50a23536451..1dd4c468ae6 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/components/service_credentials_form.vue +++ b/app/assets/javascripts/create_cluster/eks_cluster/components/service_credentials_form.vue @@ -28,7 +28,7 @@ export default { }, data() { return { - roleArn: '', + roleArn: this.$store.state.roleArn, }; }, computed: { diff --git a/app/assets/javascripts/create_cluster/eks_cluster/index.js b/app/assets/javascripts/create_cluster/eks_cluster/index.js index 27f859d8972..fb993a7aa59 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/index.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/index.js @@ -12,20 +12,14 @@ export default el => { kubernetesIntegrationHelpPath, accountAndExternalIdsHelpPath, createRoleArnHelpPath, - getRolesPath, - getRegionsPath, - getKeyPairsPath, - getVpcsPath, - getSubnetsPath, - getSecurityGroupsPath, - getInstanceTypesPath, externalId, accountId, + instanceTypes, hasCredentials, createRolePath, createClusterPath, - signOutPath, externalLinkIcon, + roleArn, } = el.dataset; return new Vue({ @@ -35,18 +29,10 @@ export default el => { hasCredentials: parseBoolean(hasCredentials), externalId, accountId, + instanceTypes: JSON.parse(instanceTypes), createRolePath, createClusterPath, - signOutPath, - }, - apiPaths: { - getRolesPath, - getRegionsPath, - getKeyPairsPath, - getVpcsPath, - getSubnetsPath, - getSecurityGroupsPath, - getInstanceTypesPath, + roleArn, }, }), components: { diff --git a/app/assets/javascripts/create_cluster/eks_cluster/services/aws_services_facade.js b/app/assets/javascripts/create_cluster/eks_cluster/services/aws_services_facade.js index 21b87d525cf..601ff6f9adc 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/services/aws_services_facade.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/services/aws_services_facade.js @@ -1,58 +1,98 @@ -import axios from '~/lib/utils/axios_utils'; - -export default apiPaths => ({ - fetchRoles() { - return axios - .get(apiPaths.getRolesPath) - .then(({ data: { roles } }) => - roles.map(({ role_name: name, arn: value }) => ({ name, value })), - ); - }, - fetchKeyPairs({ region }) { - return axios - .get(apiPaths.getKeyPairsPath, { params: { region } }) - .then(({ data: { key_pairs: keyPairs } }) => - keyPairs.map(({ key_name }) => ({ name: key_name, value: key_name })), - ); - }, - fetchRegions() { - return axios.get(apiPaths.getRegionsPath).then(({ data: { regions } }) => - regions.map(({ region_name }) => ({ - name: region_name, - value: region_name, +import AWS from 'aws-sdk/global'; +import EC2 from 'aws-sdk/clients/ec2'; +import IAM from 'aws-sdk/clients/iam'; + +const lookupVpcName = ({ Tags: tags, VpcId: id }) => { + const nameTag = tags.find(({ Key: key }) => key === 'Name'); + + return nameTag ? nameTag.Value : id; +}; + +export const DEFAULT_REGION = 'us-east-2'; + +export const setAWSConfig = ({ awsCredentials }) => { + AWS.config = { + ...awsCredentials, + region: DEFAULT_REGION, + }; +}; + +export const fetchRoles = () => { + const iam = new IAM(); + + return iam + .listRoles() + .promise() + .then(({ Roles: roles }) => roles.map(({ RoleName: name, Arn: value }) => ({ name, value }))); +}; + +export const fetchRegions = () => { + const ec2 = new EC2(); + + return ec2 + .describeRegions() + .promise() + .then(({ Regions: regions }) => + regions.map(({ RegionName: name }) => ({ + name, + value: name, })), ); - }, - fetchVpcs({ region }) { - return axios.get(apiPaths.getVpcsPath, { params: { region } }).then(({ data: { vpcs } }) => - vpcs.map(({ vpc_id }) => ({ - value: vpc_id, - name: vpc_id, +}; + +export const fetchKeyPairs = ({ region }) => { + const ec2 = new EC2({ region }); + + return ec2 + .describeKeyPairs() + .promise() + .then(({ KeyPairs: keyPairs }) => keyPairs.map(({ KeyName: name }) => ({ name, value: name }))); +}; + +export const fetchVpcs = ({ region }) => { + const ec2 = new EC2({ region }); + + return ec2 + .describeVpcs() + .promise() + .then(({ Vpcs: vpcs }) => + vpcs.map(vpc => ({ + value: vpc.VpcId, + name: lookupVpcName(vpc), })), ); - }, - fetchSubnets({ vpc, region }) { - return axios - .get(apiPaths.getSubnetsPath, { params: { vpc_id: vpc, region } }) - .then(({ data: { subnets } }) => - subnets.map(({ subnet_id }) => ({ name: subnet_id, value: subnet_id })), - ); - }, - fetchSecurityGroups({ vpc, region }) { - return axios - .get(apiPaths.getSecurityGroupsPath, { params: { vpc_id: vpc, region } }) - .then(({ data: { security_groups: securityGroups } }) => - securityGroups.map(({ group_name: name, group_id: value }) => ({ name, value })), - ); - }, - fetchInstanceTypes() { - return axios - .get(apiPaths.getInstanceTypesPath) - .then(({ data: { instance_types: instanceTypes } }) => - instanceTypes.map(({ instance_type_name }) => ({ - name: instance_type_name, - value: instance_type_name, - })), - ); - }, -}); +}; + +export const fetchSubnets = ({ vpc, region }) => { + const ec2 = new EC2({ region }); + + return ec2 + .describeSubnets({ + Filters: [ + { + Name: 'vpc-id', + Values: [vpc], + }, + ], + }) + .promise() + .then(({ Subnets: subnets }) => subnets.map(({ SubnetId: id }) => ({ value: id, name: id }))); +}; + +export const fetchSecurityGroups = ({ region, vpc }) => { + const ec2 = new EC2({ region }); + + return ec2 + .describeSecurityGroups({ + Filters: [ + { + Name: 'vpc-id', + Values: [vpc], + }, + ], + }) + .promise() + .then(({ SecurityGroups: securityGroups }) => + securityGroups.map(({ GroupName: name, GroupId: value }) => ({ name, value })), + ); +}; diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/actions.js b/app/assets/javascripts/create_cluster/eks_cluster/store/actions.js index 72f15263a8f..e96e6d6e4f8 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/actions.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/actions.js @@ -1,6 +1,8 @@ import * as types from './mutation_types'; +import { setAWSConfig } from '../services/aws_services_facade'; import axios from '~/lib/utils/axios_utils'; import createFlash from '~/flash'; +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; const getErrorMessage = data => { const errorKey = Object.keys(data)[0]; @@ -28,7 +30,7 @@ export const createRole = ({ dispatch, state: { createRolePath } }, payload) => role_arn: payload.roleArn, role_external_id: payload.externalId, }) - .then(() => dispatch('createRoleSuccess')) + .then(({ data }) => dispatch('createRoleSuccess', convertObjectPropsToCamelCase(data))) .catch(error => dispatch('createRoleError', { error })); }; @@ -36,7 +38,8 @@ export const requestCreateRole = ({ commit }) => { commit(types.REQUEST_CREATE_ROLE); }; -export const createRoleSuccess = ({ commit }) => { +export const createRoleSuccess = ({ commit }, awsCredentials) => { + setAWSConfig({ awsCredentials }); commit(types.CREATE_ROLE_SUCCESS); }; @@ -117,9 +120,3 @@ export const setInstanceType = ({ commit }, payload) => { export const setNodeCount = ({ commit }, payload) => { commit(types.SET_NODE_COUNT, payload); }; - -export const signOut = ({ commit, state: { signOutPath } }) => - axios - .delete(signOutPath) - .then(() => commit(types.SIGN_OUT)) - .catch(({ response: { data } }) => createFlash(getErrorMessage(data))); diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/cluster_dropdown/index.js b/app/assets/javascripts/create_cluster/eks_cluster/store/cluster_dropdown/index.js index 07a5821c47d..0b19589215c 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/cluster_dropdown/index.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/cluster_dropdown/index.js @@ -3,11 +3,11 @@ import actions from './actions'; import mutations from './mutations'; import state from './state'; -const createStore = fetchFn => ({ +const createStore = ({ fetchFn, initialState }) => ({ actions: actions(fetchFn), getters, mutations, - state: state(), + state: Object.assign(state(), initialState || {}), }); export default createStore; diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/index.js b/app/assets/javascripts/create_cluster/eks_cluster/store/index.js index 5982fc8a2fd..09fd560240d 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/index.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/index.js @@ -6,12 +6,17 @@ import state from './state'; import clusterDropdownStore from './cluster_dropdown'; -import awsServicesFactory from '../services/aws_services_facade'; +import { + fetchRoles, + fetchRegions, + fetchKeyPairs, + fetchVpcs, + fetchSubnets, + fetchSecurityGroups, +} from '../services/aws_services_facade'; -const createStore = ({ initialState, apiPaths }) => { - const awsServices = awsServicesFactory(apiPaths); - - return new Vuex.Store({ +const createStore = ({ initialState }) => + new Vuex.Store({ actions, getters, mutations, @@ -19,34 +24,33 @@ const createStore = ({ initialState, apiPaths }) => { modules: { roles: { namespaced: true, - ...clusterDropdownStore(awsServices.fetchRoles), + ...clusterDropdownStore({ fetchFn: fetchRoles }), }, regions: { namespaced: true, - ...clusterDropdownStore(awsServices.fetchRegions), + ...clusterDropdownStore({ fetchFn: fetchRegions }), }, keyPairs: { namespaced: true, - ...clusterDropdownStore(awsServices.fetchKeyPairs), + ...clusterDropdownStore({ fetchFn: fetchKeyPairs }), }, vpcs: { namespaced: true, - ...clusterDropdownStore(awsServices.fetchVpcs), + ...clusterDropdownStore({ fetchFn: fetchVpcs }), }, subnets: { namespaced: true, - ...clusterDropdownStore(awsServices.fetchSubnets), + ...clusterDropdownStore({ fetchFn: fetchSubnets }), }, securityGroups: { namespaced: true, - ...clusterDropdownStore(awsServices.fetchSecurityGroups), + ...clusterDropdownStore({ fetchFn: fetchSecurityGroups }), }, instanceTypes: { namespaced: true, - ...clusterDropdownStore(awsServices.fetchInstanceTypes), + ...clusterDropdownStore({ initialState: { items: initialState.instanceTypes } }), }, }, }); -}; export default createStore; diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/mutation_types.js b/app/assets/javascripts/create_cluster/eks_cluster/store/mutation_types.js index f9204cc2207..9dee6abae5f 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/mutation_types.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/mutation_types.js @@ -13,7 +13,6 @@ export const SET_GITLAB_MANAGED_CLUSTER = 'SET_GITLAB_MANAGED_CLUSTER'; export const REQUEST_CREATE_ROLE = 'REQUEST_CREATE_ROLE'; export const CREATE_ROLE_SUCCESS = 'CREATE_ROLE_SUCCESS'; export const CREATE_ROLE_ERROR = 'CREATE_ROLE_ERROR'; -export const SIGN_OUT = 'SIGN_OUT'; export const REQUEST_CREATE_CLUSTER = 'REQUEST_CREATE_CLUSTER'; export const CREATE_CLUSTER_SUCCESS = 'CREATE_CLUSTER_SUCCESS'; export const CREATE_CLUSTER_ERROR = 'CREATE_CLUSTER_ERROR'; diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/mutations.js b/app/assets/javascripts/create_cluster/eks_cluster/store/mutations.js index aa04c8f7079..c331d27d255 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/mutations.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/mutations.js @@ -60,7 +60,4 @@ export default { state.isCreatingCluster = false; state.createClusterError = error; }, - [types.SIGN_OUT](state) { - state.hasCredentials = false; - }, }; diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/state.js b/app/assets/javascripts/create_cluster/eks_cluster/store/state.js index 2e3a05a9187..20434dcce98 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/state.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/state.js @@ -12,6 +12,8 @@ export default () => ({ accountId: '', externalId: '', + roleArn: '', + clusterName: '', environmentScope: '*', kubernetesVersion, diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 33ae778769a..1ed8da57927 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -228,10 +228,6 @@ class ApplicationController < ActionController::Base end end - def respond_201 - head :created - end - def respond_422 head :unprocessable_entity end diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index 0295f36732c..f4b74b14c0b 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -8,7 +8,7 @@ class Clusters::ClustersController < Clusters::BaseController 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, :revoke_aws_role, :aws_proxy] + before_action :authorize_create_cluster!, only: [:new, :authorize_aws_role] before_action :authorize_update_cluster!, only: [:update] before_action :authorize_admin_cluster!, only: [:destroy, :clear_cache] before_action :update_applications_status, only: [:cluster_status] @@ -42,6 +42,7 @@ class Clusters::ClustersController < Clusters::BaseController if params[:provider] == 'aws' @aws_role = current_user.aws_role || Aws::Role.new @aws_role.ensure_role_external_id! + @instance_types = load_instance_types.to_json elsif params[:provider] == 'gcp' redirect_to @authorize_url if @authorize_url && !@valid_gcp_token @@ -145,21 +146,9 @@ class Clusters::ClustersController < Clusters::BaseController end def authorize_aws_role - role = current_user.build_aws_role(create_role_params) - - 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 + response = Clusters::Aws::AuthorizeRoleService.new( + current_user, + params: aws_role_params ).execute render json: response.body, status: response.status @@ -268,7 +257,7 @@ class Clusters::ClustersController < Clusters::BaseController ) end - def create_role_params + def aws_role_params params.require(:cluster).permit(:role_arn, :role_external_id) end @@ -314,6 +303,19 @@ class Clusters::ClustersController < Clusters::BaseController end 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 load_instance_types + stack_template = File.read(Rails.root.join('vendor', 'aws', 'cloudformation', 'eks_cluster.yaml')) + instance_types = YAML.safe_load(stack_template).dig('Parameters', 'NodeInstanceType', 'AllowedValues') + + instance_types.map { |type| Hash(name: type, value: type) } + end + def update_applications_status @cluster.applications.each(&:schedule_status_update) end diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb index 56a66dd38db..ba21ccfb169 100644 --- a/app/controllers/projects/error_tracking_controller.rb +++ b/app/controllers/projects/error_tracking_controller.rb @@ -77,8 +77,10 @@ class Projects::ErrorTrackingController < Projects::ApplicationController return if handle_errors(result) + result_with_syntax_highlight = Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(result[:latest_event]) + render json: { - error: serialize_error_event(result[:latest_event]) + error: serialize_error_event(result_with_syntax_highlight) } end diff --git a/app/models/clusters/providers/aws.rb b/app/models/clusters/providers/aws.rb index 78eb75ddcc0..faf587fb83d 100644 --- a/app/models/clusters/providers/aws.rb +++ b/app/models/clusters/providers/aws.rb @@ -8,9 +8,11 @@ module Clusters self.table_name = 'cluster_providers_aws' + DEFAULT_REGION = 'us-east-1' + belongs_to :cluster, inverse_of: :provider_aws, class_name: 'Clusters::Cluster' - default_value_for :region, 'us-east-1' + default_value_for :region, DEFAULT_REGION default_value_for :num_nodes, 3 default_value_for :instance_type, 'm5.large' diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index 19216281e48..793ea3c29c3 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -9,7 +9,7 @@ class DeployKey < Key scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) } scope :are_public, -> { where(public: true) } - scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, :namespace] }) } + scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, namespace: :route] }) } ignore_column :can_push, remove_after: '2019-12-15', remove_with: '12.6' @@ -24,7 +24,7 @@ class DeployKey < Key end def almost_orphaned? - self.deploy_keys_projects.count == 1 + self.deploy_keys_projects.size == 1 end def destroyed_when_orphaned? @@ -44,7 +44,11 @@ class DeployKey < Key end def deploy_keys_project_for(project) - deploy_keys_projects.find_by(project: project) + if association(:deploy_keys_projects).loaded? + deploy_keys_projects.find { |dkp| dkp.project_id.eql?(project&.id) } + else + deploy_keys_projects.find_by(project: project) + end end def projects_with_write_access diff --git a/app/models/user.rb b/app/models/user.rb index 6a29de20d86..698848c5b16 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1001,7 +1001,7 @@ class User < ApplicationRecord end def project_deploy_keys - DeployKey.in_projects(authorized_projects.select(:id)).distinct(:id) + @project_deploy_keys ||= DeployKey.in_projects(authorized_projects.select(:id)).distinct(:id) end def highest_role diff --git a/app/policies/deploy_key_policy.rb b/app/policies/deploy_key_policy.rb index 7f0ec011e79..b117bb57921 100644 --- a/app/policies/deploy_key_policy.rb +++ b/app/policies/deploy_key_policy.rb @@ -3,10 +3,7 @@ class DeployKeyPolicy < BasePolicy with_options scope: :subject, score: 0 condition(:private_deploy_key) { @subject.private? } - - # rubocop: disable CodeReuse/ActiveRecord - condition(:has_deploy_key) { @user.project_deploy_keys.exists?(id: @subject.id) } - # rubocop: enable CodeReuse/ActiveRecord + condition(:has_deploy_key) { @user.project_deploy_keys.any? { |pdk| pdk.id.eql?(@subject.id) } } rule { anonymous }.prevent_all diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb index 7677e6f026f..6b1d82e7557 100644 --- a/app/presenters/clusterable_presenter.rb +++ b/app/presenters/clusterable_presenter.rb @@ -29,18 +29,10 @@ 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 diff --git a/app/presenters/instance_clusterable_presenter.rb b/app/presenters/instance_clusterable_presenter.rb index 34d3f347689..0c267fd5735 100644 --- a/app/presenters/instance_clusterable_presenter.rb +++ b/app/presenters/instance_clusterable_presenter.rb @@ -67,16 +67,6 @@ class InstanceClusterablePresenter < ClusterablePresenter 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/presenters/projects/settings/deploy_keys_presenter.rb b/app/presenters/projects/settings/deploy_keys_presenter.rb index 9bb7fe13593..66211d02696 100644 --- a/app/presenters/projects/settings/deploy_keys_presenter.rb +++ b/app/presenters/projects/settings/deploy_keys_presenter.rb @@ -3,6 +3,8 @@ module Projects module Settings class DeployKeysPresenter < Gitlab::View::Presenter::Simple + include Gitlab::Utils::StrongMemoize + presents :project delegate :size, to: :enabled_keys, prefix: true delegate :size, to: :available_project_keys, prefix: true @@ -13,37 +15,45 @@ module Projects end def enabled_keys - project.deploy_keys + strong_memoize(:enabled_keys) do + project.deploy_keys.with_projects + end end def available_keys - current_user - .accessible_deploy_keys - .id_not_in(enabled_keys.select(:id)) - .with_projects + strong_memoize(:available_keys) do + current_user + .accessible_deploy_keys + .id_not_in(enabled_keys.select(:id)) + .with_projects + end end def available_project_keys - current_user - .project_deploy_keys - .id_not_in(enabled_keys.select(:id)) - .with_projects + strong_memoize(:available_project_keys) do + current_user + .project_deploy_keys + .id_not_in(enabled_keys.select(:id)) + .with_projects + end end def available_public_keys - DeployKey - .are_public - .id_not_in(enabled_keys.select(:id)) - .id_not_in(available_project_keys.select(:id)) - .with_projects + strong_memoize(:available_public_keys) do + DeployKey + .are_public + .id_not_in(enabled_keys.select(:id)) + .id_not_in(available_project_keys.select(:id)) + .with_projects + end end def as_json serializer = DeployKeySerializer.new # rubocop: disable CodeReuse/Serializer - opts = { user: current_user, project: project } + opts = { user: current_user, project: project, readable_project_ids: readable_project_ids } { - enabled_keys: serializer.represent(enabled_keys.with_projects, opts), + enabled_keys: serializer.represent(enabled_keys, opts), available_project_keys: serializer.represent(available_project_keys, opts), public_keys: serializer.represent(available_public_keys, opts) } @@ -56,6 +66,26 @@ module Projects def form_partial_path 'projects/deploy_keys/form' end + + private + + # Caching all readable project ids for the user that are associated with the queried deploy keys + def readable_project_ids + strong_memoize(:readable_projects_by_id) do + Set.new(user_readable_project_ids) + end + end + + # rubocop: disable CodeReuse/ActiveRecord + def user_readable_project_ids + project_ids = (available_keys + available_project_keys + available_public_keys) + .flat_map { |deploy_key| deploy_key.deploy_keys_projects.map(&:project_id) } + .compact + .uniq + + current_user.authorized_projects(Gitlab::Access::GUEST).id_in(project_ids).pluck(:id) + end + # rubocop: enable CodeReuse/ActiveRecord end end end diff --git a/app/serializers/deploy_key_entity.rb b/app/serializers/deploy_key_entity.rb index 9a558d12bec..2682a47fbaa 100644 --- a/app/serializers/deploy_key_entity.rb +++ b/app/serializers/deploy_key_entity.rb @@ -11,8 +11,7 @@ class DeployKeyEntity < Grape::Entity expose :updated_at expose :deploy_keys_projects, using: DeployKeysProjectEntity do |deploy_key| deploy_key.deploy_keys_projects.select do |deploy_key_project| - !deploy_key_project.project&.pending_delete? && - Ability.allowed?(options[:user], :read_project, deploy_key_project.project) + !deploy_key_project.project&.pending_delete? && (allowed_to_read_project?(deploy_key_project.project) || options[:user].admin?) end end expose :can_edit @@ -23,4 +22,12 @@ class DeployKeyEntity < Grape::Entity Ability.allowed?(options[:user], :update_deploy_key, object) || Ability.allowed?(options[:user], :update_deploy_keys_project, object.deploy_keys_project_for(options[:project])) end + + def allowed_to_read_project?(project) + if options[:readable_project_ids] + options[:readable_project_ids].include?(project.id) + else + Ability.allowed?(options[:user], :read_project, project) + end + end end diff --git a/app/services/clusters/aws/authorize_role_service.rb b/app/services/clusters/aws/authorize_role_service.rb new file mode 100644 index 00000000000..6eafce0597e --- /dev/null +++ b/app/services/clusters/aws/authorize_role_service.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Clusters + module Aws + class AuthorizeRoleService + attr_reader :user + + Response = Struct.new(:status, :body) + + ERRORS = [ + ActiveRecord::RecordInvalid, + Clusters::Aws::FetchCredentialsService::MissingRoleError, + ::Aws::Errors::MissingCredentialsError, + ::Aws::STS::Errors::ServiceError + ].freeze + + def initialize(user, params:) + @user = user + @params = params + end + + def execute + @role = create_or_update_role! + + Response.new(:ok, credentials) + rescue *ERRORS + Response.new(:unprocessable_entity, {}) + end + + private + + attr_reader :role, :params + + def create_or_update_role! + if role = user.aws_role + role.update!(params) + + role + else + user.create_aws_role!(params) + end + end + + def credentials + Clusters::Aws::FetchCredentialsService.new(role).execute + end + end + end +end diff --git a/app/services/clusters/aws/fetch_credentials_service.rb b/app/services/clusters/aws/fetch_credentials_service.rb index 2724d4b657b..33efc4cc120 100644 --- a/app/services/clusters/aws/fetch_credentials_service.rb +++ b/app/services/clusters/aws/fetch_credentials_service.rb @@ -7,9 +7,8 @@ module Clusters MissingRoleError = Class.new(StandardError) - def initialize(provision_role, region:, provider: nil) + def initialize(provision_role, provider: nil) @provision_role = provision_role - @region = region @provider = provider end @@ -20,13 +19,14 @@ module Clusters client: client, role_arn: provision_role.role_arn, role_session_name: session_name, - external_id: provision_role.role_external_id + external_id: provision_role.role_external_id, + policy: session_policy ).credentials end private - attr_reader :provider, :region + attr_reader :provider def client ::Aws::STS::Client.new(credentials: gitlab_credentials, region: region) @@ -44,6 +44,26 @@ module Clusters Gitlab::CurrentSettings.eks_secret_access_key end + def region + provider&.region || Clusters::Providers::Aws::DEFAULT_REGION + end + + ## + # If we haven't created a provider record yet, + # we restrict ourselves to read only access so + # that we can safely expose credentials to the + # frontend (to be used when populating the + # creation form). + def session_policy + if provider.nil? + File.read(read_only_policy) + end + end + + def read_only_policy + Rails.root.join('vendor', 'aws', 'iam', "eks_cluster_read_only_policy.json") + end + def session_name if provider.present? "gitlab-eks-cluster-#{provider.cluster_id}-user-#{provision_role.user_id}" diff --git a/app/services/clusters/aws/proxy_service.rb b/app/services/clusters/aws/proxy_service.rb deleted file mode 100644 index df8fc480005..00000000000 --- a/app/services/clusters/aws/proxy_service.rb +++ /dev/null @@ -1,134 +0,0 @@ -# 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/services/clusters/kubernetes/kubernetes.rb b/app/services/clusters/kubernetes.rb index 59cb1c4b3a9..59cb1c4b3a9 100644 --- a/app/services/clusters/kubernetes/kubernetes.rb +++ b/app/services/clusters/kubernetes.rb diff --git a/app/views/clusters/clusters/aws/_new.html.haml b/app/views/clusters/clusters/aws/_new.html.haml index 795b80bfb6f..d89e6965dac 100644 --- a/app/views/clusters/clusters/aws/_new.html.haml +++ b/app/views/clusters/clusters/aws/_new.html.haml @@ -5,19 +5,12 @@ - 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, + 'role-arn' => @aws_role.role_arn, + 'instance-types' => @instance_types, 'kubernetes-integration-help-path' => help_page_path('user/project/clusters/index'), 'account-and-external-ids-help-path' => help_page_path('user/project/clusters/add_remove_clusters.md', anchor: 'eks-cluster'), 'create-role-arn-help-path' => help_page_path('user/project/clusters/add_remove_clusters.md', anchor: 'eks-cluster'), - 'external-link-icon' => icon('external-link'), - 'has-credentials' => @aws_role.role_arn.present?.to_s } } + 'external-link-icon' => icon('external-link') } } diff --git a/changelogs/unreleased/20890-soft-delete-user-api-status-code.yml b/changelogs/unreleased/20890-soft-delete-user-api-status-code.yml new file mode 100644 index 00000000000..5ade53f152d --- /dev/null +++ b/changelogs/unreleased/20890-soft-delete-user-api-status-code.yml @@ -0,0 +1,5 @@ +--- +title: Adds 409 when user cannot be soft deleted through the API +merge_request: 21037 +author: +type: fixed diff --git a/changelogs/unreleased/21059-optimize-deploy-keys-index-page.yml b/changelogs/unreleased/21059-optimize-deploy-keys-index-page.yml new file mode 100644 index 00000000000..18a3b275641 --- /dev/null +++ b/changelogs/unreleased/21059-optimize-deploy-keys-index-page.yml @@ -0,0 +1,5 @@ +--- +title: Optimize loading the repository deploy keys page +merge_request: 20970 +author: +type: performance diff --git a/changelogs/unreleased/35411-syntax-highlight-for-sentry-stacktrace.yml b/changelogs/unreleased/35411-syntax-highlight-for-sentry-stacktrace.yml new file mode 100644 index 00000000000..0f824a879d3 --- /dev/null +++ b/changelogs/unreleased/35411-syntax-highlight-for-sentry-stacktrace.yml @@ -0,0 +1,5 @@ +--- +title: Add syntax highlight for Sentry error stack trace +merge_request: 21182 +author: +type: added diff --git a/config/routes.rb b/config/routes.rb index 3167517ab16..5a1330d01d2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -145,11 +145,6 @@ Rails.application.routes.draw do 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/db/migrate/20191121193110_add_issue_links_type.rb b/db/migrate/20191121193110_add_issue_links_type.rb new file mode 100644 index 00000000000..61ef2e7d7e8 --- /dev/null +++ b/db/migrate/20191121193110_add_issue_links_type.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIssueLinksType < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :issue_links, :link_type, :integer, default: 0, limit: 2 + end + + def down + remove_column :issue_links, :link_type + end +end diff --git a/db/migrate/20191205145647_add_index_to_projects_deploy_keys_deploy_key.rb b/db/migrate/20191205145647_add_index_to_projects_deploy_keys_deploy_key.rb new file mode 100644 index 00000000000..f9cdc226e4d --- /dev/null +++ b/db/migrate/20191205145647_add_index_to_projects_deploy_keys_deploy_key.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToProjectsDeployKeysDeployKey < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + DOWNTIME = false + + def up + add_concurrent_index :deploy_keys_projects, :deploy_key_id + end + + def down + remove_concurrent_index :deploy_keys_projects, :deploy_key_id + end +end diff --git a/db/schema.rb b/db/schema.rb index c677944bad6..a369db5e4df 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1305,6 +1305,7 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do t.datetime "created_at" t.datetime "updated_at" t.boolean "can_push", default: false, null: false + t.index ["deploy_key_id"], name: "index_deploy_keys_projects_on_deploy_key_id" t.index ["project_id"], name: "index_deploy_keys_projects_on_project_id" end @@ -2049,6 +2050,7 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do t.integer "target_id", null: false t.datetime "created_at" t.datetime "updated_at" + t.integer "link_type", limit: 2, default: 0, null: false t.index ["source_id", "target_id"], name: "index_issue_links_on_source_id_and_target_id", unique: true t.index ["source_id"], name: "index_issue_links_on_source_id" t.index ["target_id"], name: "index_issue_links_on_target_id" diff --git a/doc/api/users.md b/doc/api/users.md index c82a5e23c8e..4491a339d25 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -430,7 +430,7 @@ e.g. when renaming the email address to some existing one. ## User deletion Deletes a user. Available only for administrators. -This returns a `204 No Content` status code if the operation was successfully or `404` if the resource was not found. +This returns a `204 No Content` status code if the operation was successfully, `404` if the resource was not found or `409` if the user cannot be soft deleted. ``` DELETE /users/:id diff --git a/lib/api/users.rb b/lib/api/users.rb index ff0b1e87b03..3b54d95504f 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -452,6 +452,7 @@ module API user = User.find_by(id: params[:id]) not_found!('User') unless user + conflict!('User cannot be removed while is the sole-owner of a group') unless user.can_be_removed? || params[:hard_delete] destroy_conditionally!(user) do user.delete_async(deleted_by: current_user, params: params) diff --git a/lib/gitlab/error_tracking/stack_trace_highlight_decorator.rb b/lib/gitlab/error_tracking/stack_trace_highlight_decorator.rb new file mode 100644 index 00000000000..a403275fd4e --- /dev/null +++ b/lib/gitlab/error_tracking/stack_trace_highlight_decorator.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + module StackTraceHighlightDecorator + extend self + + def decorate(error_event) + ::Gitlab::ErrorTracking::ErrorEvent.new( + issue_id: error_event.issue_id, + date_received: error_event.date_received, + stack_trace_entries: highlight_stack_trace(error_event.stack_trace_entries) + ) + end + + private + + def highlight_stack_trace(stack_trace) + stack_trace.map do |entry| + highlight_stack_trace_entry(entry) + end + end + + def highlight_stack_trace_entry(entry) + return entry unless entry['context'] + + entry.merge('context' => highlight_entry_context(entry['filename'], entry['context'])) + end + + def highlight_entry_context(filename, context) + language = Rouge::Lexer.guess_by_filename(filename).tag + + context.map do |line_number, line_of_code| + [ + line_number, + # Passing nil for the blob name allows skipping linking dependencies for the line_of_code + Gitlab::Highlight.highlight(nil, line_of_code, language: language) + ] + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e5ca8f39991..3b9370fee12 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4145,9 +4145,6 @@ msgstr "" msgid "ClusterIntegration|Select a VPC to use for your EKS Cluster resources. To use a new VPC, first create one on %{startLink}Amazon Web Services %{externalLinkIcon} %{endLink}." msgstr "" -msgid "ClusterIntegration|Select a different AWS role" -msgstr "" - msgid "ClusterIntegration|Select a region to choose a Key Pair" msgstr "" diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 4184c7e611a..f27519496df 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -381,10 +381,15 @@ describe Admin::ClustersController do post :authorize_aws_role, params: params end + before do + allow(Clusters::Aws::FetchCredentialsService).to receive(:new) + .and_return(double(execute: double)) + end + it 'creates an Aws::Role record' do expect { go }.to change { Aws::Role.count } - expect(response.status).to eq 201 + expect(response.status).to eq 200 role = Aws::Role.last expect(role.user).to eq admin @@ -409,27 +414,6 @@ 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 'DELETE clear cluster cache' do let(:cluster) { create(:cluster, :instance) } let!(:kubernetes_namespace) do diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index d47122f051e..cf90d388a61 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -443,10 +443,15 @@ describe Groups::ClustersController do post :authorize_aws_role, params: params.merge(group_id: group) end + before do + allow(Clusters::Aws::FetchCredentialsService).to receive(:new) + .and_return(double(execute: double)) + end + it 'creates an Aws::Role record' do expect { go }.to change { Aws::Role.count } - expect(response.status).to eq 201 + expect(response.status).to eq 200 role = Aws::Role.last expect(role.user).to eq user @@ -476,32 +481,6 @@ 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 'DELETE clear cluster cache' do let(:cluster) { create(:cluster, :group, groups: [group]) } let!(:kubernetes_namespace) do diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 5efac44615f..ab8bfc0cabe 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -444,10 +444,15 @@ describe Projects::ClustersController do post :authorize_aws_role, params: params.merge(namespace_id: project.namespace, project_id: project) end + before do + allow(Clusters::Aws::FetchCredentialsService).to receive(:new) + .and_return(double(execute: double)) + end + it 'creates an Aws::Role record' do expect { go }.to change { Aws::Role.count } - expect(response.status).to eq 201 + expect(response.status).to eq 200 role = Aws::Role.last expect(role.user).to eq user @@ -477,32 +482,6 @@ 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 'DELETE clear cluster cache' do let(:cluster) { create(:cluster, :project, projects: [project]) } let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) } diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index 8b1ca2efab2..2c7c99eabf6 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -46,17 +46,27 @@ describe Projects::DeployKeysController do create(:deploy_keys_project, project: project_private, deploy_key: create(:another_deploy_key)) end - before do - project2.add_developer(user) + context 'when user has access to all projects where deploy keys are used' do + before do + project2.add_developer(user) + end + + it 'returns json in a correct format' do + get :index, params: params.merge(format: :json) + + expect(json_response.keys).to match_array(%w(enabled_keys available_project_keys public_keys)) + expect(json_response['enabled_keys'].count).to eq(1) + expect(json_response['available_project_keys'].count).to eq(1) + expect(json_response['public_keys'].count).to eq(1) + end end - it 'returns json in a correct format' do - get :index, params: params.merge(format: :json) + context 'when user has no access to all projects where deploy keys are used' do + it 'returns json in a correct format' do + get :index, params: params.merge(format: :json) - expect(json_response.keys).to match_array(%w(enabled_keys available_project_keys public_keys)) - expect(json_response['enabled_keys'].count).to eq(1) - expect(json_response['available_project_keys'].count).to eq(1) - expect(json_response['public_keys'].count).to eq(1) + expect(json_response['available_project_keys'].count).to eq(0) + end end end end diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index ab99e44e4ca..e5585d7b52d 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -384,6 +384,10 @@ describe Projects::ErrorTrackingController do ).permit! end + subject(:get_stack_trace) do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + end + before do expect(ErrorTracking::IssueLatestEventService) .to receive(:new).with(project, user, permitted_params) @@ -398,7 +402,7 @@ describe Projects::ErrorTrackingController do end it 'returns no data' do - get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + get_stack_trace expect(response).to have_gitlab_http_status(:no_content) end @@ -408,16 +412,21 @@ describe Projects::ErrorTrackingController do before do expect(issue_stack_trace_service).to receive(:execute) .and_return(status: :success, latest_event: error_event) + + get_stack_trace end let(:error_event) { build(:error_tracking_error_event) } it 'returns an error' do - get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) - expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('error_tracking/issue_stack_trace') - expect(json_response['error']).to eq(error_event.as_json) + end + + it 'highlights stack trace source code' do + expect(json_response['error']).to eq( + Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(error_event).as_json + ) end end @@ -431,7 +440,7 @@ describe Projects::ErrorTrackingController do end it 'returns 400 with message' do - get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + get_stack_trace expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to eq(error_message) @@ -450,7 +459,7 @@ describe Projects::ErrorTrackingController do end it 'returns http_status with message' do - get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + get_stack_trace expect(response).to have_gitlab_http_status(http_status) expect(json_response['message']).to eq(error_message) diff --git a/spec/factories/error_tracking/error_event.rb b/spec/factories/error_tracking/error_event.rb index 44c127e7bf5..c4dcd67bc9f 100644 --- a/spec/factories/error_tracking/error_event.rb +++ b/spec/factories/error_tracking/error_event.rb @@ -5,12 +5,40 @@ FactoryBot.define do issue_id { 'id' } date_received { Time.now.iso8601 } stack_trace_entries do - { - 'stacktrace' => - { - 'frames' => [{ 'file' => 'test.rb' }] - } - } + [ + { + 'function' => 'puts', + 'lineNo' => 14, + 'filename' => 'hello_world.rb', + 'context' => [ + [10, "# Ruby example\n"], + [11, "class HelloWorld\n"], + [12, " def self.message\n"], + [13, " @name = 'World'\n"], + [14, " puts \"Hello \#{@name}\"\n"], + [15, " end\n"], + [16, "end\n"] + ] + }, + { + 'function' => 'print', + 'lineNo' => 6, + 'filename' => 'HelloWorld.swift', + 'context' => [ + [1, "// Swift example\n"], + [2, "struct HelloWorld {\n"], + [3, " let name = \"World\"\n"], + [4, "\n"], + [5, " static func message() {\n"], + [6, " print(\"Hello, \\(self.name)\")\n"], + [7, " }\n"], + [8, "}\n"] + ] + }, + { + 'filename' => 'blank.txt' + } + ] end skip_create diff --git a/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json b/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json index a684dd0496a..e2eeefcdd53 100644 --- a/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json +++ b/spec/fixtures/api/schemas/error_tracking/error_stack_trace.json @@ -7,7 +7,7 @@ ], "properties": { "issue_id": { "type": ["string", "integer"] }, - "stack_trace_entries": { "type": "object" }, + "stack_trace_entries": { "type": "array" }, "date_received": { "type": "string" } }, "additionalProperties": false diff --git a/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js b/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js index 25d613d64ed..d3992c6751c 100644 --- a/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js +++ b/spec/frontend/create_cluster/eks_cluster/components/eks_cluster_configuration_form_spec.js @@ -27,13 +27,11 @@ describe('EksClusterConfigurationForm', () => { let subnetsActions; let keyPairsActions; let securityGroupsActions; - let instanceTypesActions; let vm; beforeEach(() => { state = eksClusterFormState(); actions = { - signOut: jest.fn(), createCluster: jest.fn(), setClusterName: jest.fn(), setEnvironmentScope: jest.fn(), @@ -66,9 +64,6 @@ describe('EksClusterConfigurationForm', () => { securityGroupsActions = { fetchItems: jest.fn(), }; - instanceTypesActions = { - fetchItems: jest.fn(), - }; rolesState = { ...clusterDropdownStoreState(), }; @@ -127,7 +122,6 @@ describe('EksClusterConfigurationForm', () => { instanceTypes: { namespaced: true, state: instanceTypesState, - actions: instanceTypesActions, }, }, }); @@ -164,7 +158,6 @@ describe('EksClusterConfigurationForm', () => { }); }; - const findSignOutButton = () => vm.find('.js-sign-out'); const findCreateClusterButton = () => vm.find('.js-create-cluster'); const findClusterNameInput = () => vm.find('[id=eks-cluster-name]'); const findEnvironmentScopeInput = () => vm.find('[id=eks-environment-scope]'); @@ -187,15 +180,6 @@ describe('EksClusterConfigurationForm', () => { it('fetches available roles', () => { expect(rolesActions.fetchItems).toHaveBeenCalled(); }); - - it('fetches available instance types', () => { - expect(instanceTypesActions.fetchItems).toHaveBeenCalled(); - }); - }); - - it('dispatches signOut action when sign out button is clicked', () => { - findSignOutButton().trigger('click'); - expect(actions.signOut).toHaveBeenCalled(); }); it('sets isLoadingRoles to RoleDropdown loading property', () => { diff --git a/spec/frontend/create_cluster/eks_cluster/services/aws_services_facade_spec.js b/spec/frontend/create_cluster/eks_cluster/services/aws_services_facade_spec.js index 146bcc04569..490a2775b67 100644 --- a/spec/frontend/create_cluster/eks_cluster/services/aws_services_facade_spec.js +++ b/spec/frontend/create_cluster/eks_cluster/services/aws_services_facade_spec.js @@ -1,42 +1,62 @@ -import AxiosMockAdapter from 'axios-mock-adapter'; -import awsServicesFacadeFactory from '~/create_cluster/eks_cluster/services/aws_services_facade'; -import axios from '~/lib/utils/axios_utils'; +import AWS from 'aws-sdk/global'; +import EC2 from 'aws-sdk/clients/ec2'; +import { + setAWSConfig, + fetchRoles, + fetchRegions, + fetchKeyPairs, + fetchVpcs, + fetchSubnets, + fetchSecurityGroups, + DEFAULT_REGION, +} from '~/create_cluster/eks_cluster/services/aws_services_facade'; + +const mockListRolesPromise = jest.fn(); +const mockDescribeRegionsPromise = jest.fn(); +const mockDescribeKeyPairsPromise = jest.fn(); +const mockDescribeVpcsPromise = jest.fn(); +const mockDescribeSubnetsPromise = jest.fn(); +const mockDescribeSecurityGroupsPromise = jest.fn(); + +jest.mock('aws-sdk/clients/iam', () => + jest.fn().mockImplementation(() => ({ + listRoles: jest.fn().mockReturnValue({ promise: mockListRolesPromise }), + })), +); + +jest.mock('aws-sdk/clients/ec2', () => + jest.fn().mockImplementation(() => ({ + describeRegions: jest.fn().mockReturnValue({ promise: mockDescribeRegionsPromise }), + describeKeyPairs: jest.fn().mockReturnValue({ promise: mockDescribeKeyPairsPromise }), + describeVpcs: jest.fn().mockReturnValue({ promise: mockDescribeVpcsPromise }), + describeSubnets: jest.fn().mockReturnValue({ promise: mockDescribeSubnetsPromise }), + describeSecurityGroups: jest + .fn() + .mockReturnValue({ promise: mockDescribeSecurityGroupsPromise }), + })), +); describe('awsServicesFacade', () => { - let apiPaths; - let axiosMock; - let awsServices; let region; let vpc; beforeEach(() => { - apiPaths = { - getKeyPairsPath: '/clusters/aws/api/key_pairs', - getRegionsPath: '/clusters/aws/api/regions', - getRolesPath: '/clusters/aws/api/roles', - getSecurityGroupsPath: '/clusters/aws/api/security_groups', - getSubnetsPath: '/clusters/aws/api/subnets', - getVpcsPath: '/clusters/aws/api/vpcs', - getInstanceTypesPath: '/clusters/aws/api/instance_types', - }; region = 'west-1'; vpc = 'vpc-2'; - awsServices = awsServicesFacadeFactory(apiPaths); - axiosMock = new AxiosMockAdapter(axios); }); - describe('when fetchRegions succeeds', () => { - let regions; - let regionsOutput; + it('setAWSConfig configures AWS SDK with provided credentials and default region', () => { + const awsCredentials = { + accessKeyId: 'access-key', + secretAccessKey: 'secret-key', + sessionToken: 'session-token', + }; - beforeEach(() => { - regions = [{ region_name: 'east-1' }, { region_name: 'west-2' }]; - regionsOutput = regions.map(({ region_name: name }) => ({ name, value: name })); - axiosMock.onGet(apiPaths.getRegionsPath).reply(200, { regions }); - }); + setAWSConfig({ awsCredentials }); - it('return list of roles where each item has a name and value', () => { - expect(awsServices.fetchRegions()).resolves.toEqual(regionsOutput); + expect(AWS.config).toEqual({ + ...awsCredentials, + region: DEFAULT_REGION, }); }); @@ -46,15 +66,32 @@ describe('awsServicesFacade', () => { beforeEach(() => { roles = [ - { role_name: 'admin', arn: 'aws::admin' }, - { role_name: 'read-only', arn: 'aws::read-only' }, + { RoleName: 'admin', Arn: 'aws::admin' }, + { RoleName: 'read-only', Arn: 'aws::read-only' }, ]; - rolesOutput = roles.map(({ role_name: name, arn: value }) => ({ name, value })); - axiosMock.onGet(apiPaths.getRolesPath).reply(200, { roles }); + rolesOutput = roles.map(({ RoleName: name, Arn: value }) => ({ name, value })); + + mockListRolesPromise.mockResolvedValueOnce({ Roles: roles }); }); it('return list of regions where each item has a name and value', () => { - expect(awsServices.fetchRoles()).resolves.toEqual(rolesOutput); + expect(fetchRoles()).resolves.toEqual(rolesOutput); + }); + }); + + describe('when fetchRegions succeeds', () => { + let regions; + let regionsOutput; + + beforeEach(() => { + regions = [{ RegionName: 'east-1' }, { RegionName: 'west-2' }]; + regionsOutput = regions.map(({ RegionName: name }) => ({ name, value: name })); + + mockDescribeRegionsPromise.mockResolvedValueOnce({ Regions: regions }); + }); + + it('return list of roles where each item has a name and value', () => { + expect(fetchRegions()).resolves.toEqual(regionsOutput); }); }); @@ -63,15 +100,19 @@ describe('awsServicesFacade', () => { let keyPairsOutput; beforeEach(() => { - keyPairs = [{ key_pair: 'key-pair' }, { key_pair: 'key-pair-2' }]; - keyPairsOutput = keyPairs.map(({ key_name: name }) => ({ name, value: name })); - axiosMock - .onGet(apiPaths.getKeyPairsPath, { params: { region } }) - .reply(200, { key_pairs: keyPairs }); + keyPairs = [{ KeyName: 'key-pair' }, { KeyName: 'key-pair-2' }]; + keyPairsOutput = keyPairs.map(({ KeyName: name }) => ({ name, value: name })); + + mockDescribeKeyPairsPromise.mockResolvedValueOnce({ KeyPairs: keyPairs }); + }); + + it('instantatiates ec2 service with provided region', () => { + fetchKeyPairs({ region }); + expect(EC2).toHaveBeenCalledWith({ region }); }); it('return list of key pairs where each item has a name and value', () => { - expect(awsServices.fetchKeyPairs({ region })).resolves.toEqual(keyPairsOutput); + expect(fetchKeyPairs({ region })).resolves.toEqual(keyPairsOutput); }); }); @@ -80,13 +121,37 @@ describe('awsServicesFacade', () => { let vpcsOutput; beforeEach(() => { - vpcs = [{ vpc_id: 'vpc-1' }, { vpc_id: 'vpc-2' }]; - vpcsOutput = vpcs.map(({ vpc_id: name }) => ({ name, value: name })); - axiosMock.onGet(apiPaths.getVpcsPath, { params: { region } }).reply(200, { vpcs }); + vpcs = [{ VpcId: 'vpc-1', Tags: [] }, { VpcId: 'vpc-2', Tags: [] }]; + vpcsOutput = vpcs.map(({ VpcId: vpcId }) => ({ name: vpcId, value: vpcId })); + + mockDescribeVpcsPromise.mockResolvedValueOnce({ Vpcs: vpcs }); + }); + + it('instantatiates ec2 service with provided region', () => { + fetchVpcs({ region }); + expect(EC2).toHaveBeenCalledWith({ region }); }); it('return list of vpcs where each item has a name and value', () => { - expect(awsServices.fetchVpcs({ region })).resolves.toEqual(vpcsOutput); + expect(fetchVpcs({ region })).resolves.toEqual(vpcsOutput); + }); + }); + + describe('when vpcs has a Name tag', () => { + const vpcName = 'vpc name'; + const vpcId = 'vpc id'; + let vpcs; + let vpcsOutput; + + beforeEach(() => { + vpcs = [{ VpcId: vpcId, Tags: [{ Key: 'Name', Value: vpcName }] }]; + vpcsOutput = [{ name: vpcName, value: vpcId }]; + + mockDescribeVpcsPromise.mockResolvedValueOnce({ Vpcs: vpcs }); + }); + + it('uses name tag value as the vpc name', () => { + expect(fetchVpcs({ region })).resolves.toEqual(vpcsOutput); }); }); @@ -95,15 +160,14 @@ describe('awsServicesFacade', () => { let subnetsOutput; beforeEach(() => { - subnets = [{ subnet_id: 'vpc-1' }, { subnet_id: 'vpc-2' }]; - subnetsOutput = subnets.map(({ subnet_id }) => ({ name: subnet_id, value: subnet_id })); - axiosMock - .onGet(apiPaths.getSubnetsPath, { params: { region, vpc_id: vpc } }) - .reply(200, { subnets }); + subnets = [{ SubnetId: 'subnet-1' }, { SubnetId: 'subnet-2' }]; + subnetsOutput = subnets.map(({ SubnetId }) => ({ name: SubnetId, value: SubnetId })); + + mockDescribeSubnetsPromise.mockResolvedValueOnce({ Subnets: subnets }); }); it('return list of subnets where each item has a name and value', () => { - expect(awsServices.fetchSubnets({ region, vpc })).resolves.toEqual(subnetsOutput); + expect(fetchSubnets({ region, vpc })).resolves.toEqual(subnetsOutput); }); }); @@ -113,40 +177,19 @@ describe('awsServicesFacade', () => { beforeEach(() => { securityGroups = [ - { group_name: 'admin group', group_id: 'group-1' }, - { group_name: 'basic group', group_id: 'group-2' }, + { GroupName: 'admin group', GroupId: 'group-1' }, + { GroupName: 'basic group', GroupId: 'group-2' }, ]; - securityGroupsOutput = securityGroups.map(({ group_id: value, group_name: name }) => ({ + securityGroupsOutput = securityGroups.map(({ GroupId: value, GroupName: name }) => ({ name, value, })); - axiosMock - .onGet(apiPaths.getSecurityGroupsPath, { params: { region, vpc_id: vpc } }) - .reply(200, { security_groups: securityGroups }); - }); - it('return list of security groups where each item has a name and value', () => { - expect(awsServices.fetchSecurityGroups({ region, vpc })).resolves.toEqual( - securityGroupsOutput, - ); + mockDescribeSecurityGroupsPromise.mockResolvedValueOnce({ SecurityGroups: securityGroups }); }); - }); - - describe('when fetchInstanceTypes succeeds', () => { - let instanceTypes; - let instanceTypesOutput; - beforeEach(() => { - instanceTypes = [{ instance_type_name: 't2.small' }, { instance_type_name: 't2.medium' }]; - instanceTypesOutput = instanceTypes.map(({ instance_type_name }) => ({ - name: instance_type_name, - value: instance_type_name, - })); - axiosMock.onGet(apiPaths.getInstanceTypesPath).reply(200, { instance_types: instanceTypes }); - }); - - it('return list of instance types where each item has a name and value', () => { - expect(awsServices.fetchInstanceTypes()).resolves.toEqual(instanceTypesOutput); + it('return list of security groups where each item has a name and value', () => { + expect(fetchSecurityGroups({ region, vpc })).resolves.toEqual(securityGroupsOutput); }); }); }); diff --git a/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js b/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js index 578d82cfc26..fda1f71b1f9 100644 --- a/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js +++ b/spec/frontend/create_cluster/eks_cluster/store/actions_spec.js @@ -1,5 +1,4 @@ import testAction from 'helpers/vuex_action_helper'; - import MockAdapter from 'axios-mock-adapter'; import createState from '~/create_cluster/eks_cluster/store/state'; import * as actions from '~/create_cluster/eks_cluster/store/actions'; @@ -21,7 +20,6 @@ import { CREATE_ROLE_ERROR, REQUEST_CREATE_CLUSTER, CREATE_CLUSTER_ERROR, - SIGN_OUT, } from '~/create_cluster/eks_cluster/store/mutation_types'; import axios from '~/lib/utils/axios_utils'; import createFlash from '~/flash'; @@ -64,7 +62,6 @@ describe('EKS Cluster Store Actions', () => { state = { ...createState(), createRolePath: '/clusters/roles/', - signOutPath: '/aws/signout', createClusterPath: '/clusters/', }; }); @@ -102,6 +99,10 @@ describe('EKS Cluster Store Actions', () => { roleArn: 'role_arn', externalId: 'externalId', }; + const response = { + accessKeyId: 'access-key-id', + secretAccessKey: 'secret-key-id', + }; describe('when request succeeds', () => { beforeEach(() => { @@ -110,7 +111,7 @@ describe('EKS Cluster Store Actions', () => { role_arn: payload.roleArn, role_external_id: payload.externalId, }) - .reply(201); + .reply(201, response); }); it('dispatches createRoleSuccess action', () => @@ -119,7 +120,7 @@ describe('EKS Cluster Store Actions', () => { payload, state, [], - [{ type: 'requestCreateRole' }, { type: 'createRoleSuccess' }], + [{ type: 'requestCreateRole' }, { type: 'createRoleSuccess', payload: response }], )); }); @@ -281,14 +282,4 @@ describe('EKS Cluster Store Actions', () => { expect(createFlash).toHaveBeenCalledWith(payload.name[0]); }); }); - - describe('signOut', () => { - beforeEach(() => { - mock.onDelete(state.signOutPath).reply(200, null); - }); - - it('commits signOut mutation', () => { - testAction(actions.signOut, null, state, [{ type: SIGN_OUT }]); - }); - }); }); diff --git a/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js b/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js index 0fb392f5eea..8bb014d4758 100644 --- a/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js +++ b/spec/frontend/create_cluster/eks_cluster/store/mutations_spec.js @@ -16,7 +16,6 @@ import { CREATE_ROLE_ERROR, REQUEST_CREATE_CLUSTER, CREATE_CLUSTER_ERROR, - SIGN_OUT, } from '~/create_cluster/eks_cluster/store/mutation_types'; import createState from '~/create_cluster/eks_cluster/store/state'; import mutations from '~/create_cluster/eks_cluster/store/mutations'; @@ -159,15 +158,4 @@ describe('Create EKS cluster store mutations', () => { expect(state.createClusterError).toBe(error); }); }); - - describe(`mutation ${SIGN_OUT}`, () => { - beforeEach(() => { - state.hasCredentials = true; - mutations[SIGN_OUT](state); - }); - - it('sets hasCredentials to false', () => { - expect(state.hasCredentials).toBe(false); - }); - }); }); diff --git a/spec/lib/gitlab/error_tracking/stack_trace_highlight_decorator_spec.rb b/spec/lib/gitlab/error_tracking/stack_trace_highlight_decorator_spec.rb new file mode 100644 index 00000000000..04ef5ba516e --- /dev/null +++ b/spec/lib/gitlab/error_tracking/stack_trace_highlight_decorator_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ErrorTracking::StackTraceHighlightDecorator do + let(:error_event) { build(:error_tracking_error_event) } + + describe '.decorate' do + subject(:decorate) { described_class.decorate(error_event) } + + it 'does not change issue_id' do + expect(decorate.issue_id).to eq(error_event.issue_id) + end + + it 'does not change date_received' do + expect(decorate.date_received).to eq(error_event.date_received) + end + + it 'decorates the stack trace context' do + expect(decorate.stack_trace_entries).to eq( + [ + { + 'function' => 'puts', + 'lineNo' => 14, + 'filename' => 'hello_world.rb', + 'context' => [ + [10, '<span id="LC1" class="line" lang="ruby"><span class="c1"># Ruby example</span></span>'], + [11, '<span id="LC1" class="line" lang="ruby"><span class="k">class</span> <span class="nc">HelloWorld</span></span>'], + [12, '<span id="LC1" class="line" lang="ruby"> <span class="k">def</span> <span class="nc">self</span><span class="o">.</span><span class="nf">message</span></span>'], + [13, '<span id="LC1" class="line" lang="ruby"> <span class="vi">@name</span> <span class="o">=</span> <span class="s1">\'World\'</span></span>'], + [14, %Q[<span id="LC1" class="line" lang="ruby"> <span class="nb">puts</span> <span class="s2">"Hello </span><span class="si">\#{</span><span class="vi">@name</span><span class="si">}</span><span class="s2">"</span></span>]], + [15, '<span id="LC1" class="line" lang="ruby"> <span class="k">end</span></span>'], + [16, '<span id="LC1" class="line" lang="ruby"><span class="k">end</span></span>'] + ] + }, + { + 'function' => 'print', + 'lineNo' => 6, + 'filename' => 'HelloWorld.swift', + 'context' => [ + [1, '<span id="LC1" class="line" lang="swift"><span class="c1">// Swift example</span></span>'], + [2, '<span id="LC1" class="line" lang="swift"><span class="kd">struct</span> <span class="kt">HelloWorld</span> <span class="p">{</span></span>'], + [3, '<span id="LC1" class="line" lang="swift"> <span class="k">let</span> <span class="nv">name</span> <span class="o">=</span> <span class="s">"World"</span></span>'], + [4, '<span id="LC1" class="line" lang="swift"></span>'], + [5, '<span id="LC1" class="line" lang="swift"> <span class="kd">static</span> <span class="kd">func</span> <span class="nf">message</span><span class="p">()</span> <span class="p">{</span></span>'], + [6, '<span id="LC1" class="line" lang="swift"> <span class="nf">print</span><span class="p">(</span><span class="s">"Hello, </span><span class="se">\\(</span><span class="k">self</span><span class="o">.</span><span class="n">name</span><span class="se">)</span><span class="s">"</span><span class="p">)</span></span>'], + [7, '<span id="LC1" class="line" lang="swift"> <span class="p">}</span></span>'], + [8, '<span id="LC1" class="line" lang="swift"><span class="p">}</span></span>'] + ] + }, + { + 'filename' => 'blank.txt' + } + ] + ) + end + end +end diff --git a/spec/presenters/instance_clusterable_presenter_spec.rb b/spec/presenters/instance_clusterable_presenter_spec.rb index 3e7ee7a0ff6..4265e2fcb69 100644 --- a/spec/presenters/instance_clusterable_presenter_spec.rb +++ b/spec/presenters/instance_clusterable_presenter_spec.rb @@ -21,20 +21,6 @@ describe InstanceClusterablePresenter do 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 - describe '#clear_cluster_cache_path' do subject { presenter.clear_cluster_cache_path(cluster) } diff --git a/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb b/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb index de58733c8ea..b9cb60e414f 100644 --- a/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb +++ b/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb @@ -5,11 +5,6 @@ require 'spec_helper' describe Projects::Settings::DeployKeysPresenter do let(:project) { create(:project) } let(:user) { create(:user) } - let(:deploy_key) { create(:deploy_key, public: true) } - - let!(:deploy_keys_project) do - create(:deploy_keys_project, project: project, deploy_key: deploy_key) - end subject(:presenter) do described_class.new(project, current_user: user) @@ -20,6 +15,12 @@ describe Projects::Settings::DeployKeysPresenter do end describe '#enabled_keys' do + let!(:deploy_key) { create(:deploy_key, public: true) } + + let!(:deploy_keys_project) do + create(:deploy_keys_project, project: project, deploy_key: deploy_key) + end + it 'returns currently enabled keys' do expect(presenter.enabled_keys).to eq [deploy_keys_project.deploy_key] end @@ -53,4 +54,54 @@ describe Projects::Settings::DeployKeysPresenter do expect(presenter.available_project_keys_size).to eq(1) end end + + context 'prevent N + 1 queries' do + before do + create_records + + project.add_maintainer(user) + end + + def create_records + other_project = create(:project) + other_project.add_maintainer(user) + + create(:deploy_keys_project, project: project, deploy_key: create(:deploy_key)) + create(:deploy_keys_project, project: other_project, deploy_key: create(:deploy_key)) + create(:deploy_key, public: true) + end + + def execute_with_query_count + ActiveRecord::QueryRecorder.new { execute_presenter }.count + end + + def execute_presenter + described_class.new(project, current_user: user).as_json + end + + it 'returns correct counts' do + result = execute_presenter + + expect(result[:enabled_keys].size).to eq(1) + expect(result[:available_project_keys].size).to eq(1) + expect(result[:public_keys].size).to eq(1) + end + + it 'does not increase the query count' do + execute_presenter # make sure everything is cached + + count_before = execute_with_query_count + + 3.times { create_records } + + count_after = execute_with_query_count + + expect(count_after).to eq(count_before) + + result = execute_presenter + expect(result[:enabled_keys].size).to eq(4) + expect(result[:available_project_keys].size).to eq(4) + expect(result[:public_keys].size).to eq(4) + end + end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 1a1e80f1ce3..29088a42fbe 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1261,6 +1261,25 @@ describe API::Users do expect { Namespace.find(namespace.id) }.to raise_error ActiveRecord::RecordNotFound end + context "sole owner of a group" do + let!(:group) { create(:group).tap { |group| group.add_owner(user) } } + + context "hard delete disabled" do + it "does not delete user" do + perform_enqueued_jobs { delete api("/users/#{user.id}", admin)} + expect(response).to have_gitlab_http_status(409) + end + end + + context "hard delete enabled" do + it "delete user and group", :sidekiq_might_not_need_inline do + perform_enqueued_jobs { delete api("/users/#{user.id}?hard_delete=true", admin)} + expect(response).to have_gitlab_http_status(204) + expect(Group.exists?(group.id)).to be_falsy + end + end + end + it_behaves_like '412 response' do let(:request) { api("/users/#{user.id}", admin) } end diff --git a/spec/services/clusters/aws/authorize_role_service_spec.rb b/spec/services/clusters/aws/authorize_role_service_spec.rb new file mode 100644 index 00000000000..3ef332558a2 --- /dev/null +++ b/spec/services/clusters/aws/authorize_role_service_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::Aws::AuthorizeRoleService do + let(:user) { create(:user) } + let(:credentials) { instance_double(Aws::Credentials) } + let(:credentials_service) { instance_double(Clusters::Aws::FetchCredentialsService, execute: credentials) } + + let(:params) do + params = ActionController::Parameters.new({ + cluster: { + role_arn: 'arn:my-role', + role_external_id: 'external-id' + } + }) + + params.require(:cluster).permit(:role_arn, :role_external_id) + end + + subject { described_class.new(user, params: params).execute } + + before do + allow(Clusters::Aws::FetchCredentialsService).to receive(:new) + .with(instance_of(Aws::Role)).and_return(credentials_service) + end + + context 'role does not exist' do + it 'creates an Aws::Role record and returns a set of credentials' do + expect(user).to receive(:create_aws_role!) + .with(params).and_call_original + + expect(subject.status).to eq(:ok) + expect(subject.body).to eq(credentials) + end + end + + context 'role already exists' do + let(:role) { create(:aws_role, user: user) } + + it 'updates the existing Aws::Role record and returns a set of credentials' do + expect(role).to receive(:update!) + .with(params).and_call_original + + expect(subject.status).to eq(:ok) + expect(subject.body).to eq(credentials) + end + end + + context 'errors' do + shared_examples 'bad request' do + it 'returns an empty hash' do + expect(subject.status).to eq(:unprocessable_entity) + expect(subject.body).to eq({}) + end + end + + context 'cannot create role' do + before do + allow(user).to receive(:create_aws_role!) + .and_raise(ActiveRecord::RecordInvalid.new(user)) + end + + include_examples 'bad request' + end + + context 'client errors' do + before do + allow(credentials_service).to receive(:execute).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 + end + end +end diff --git a/spec/services/clusters/aws/fetch_credentials_service_spec.rb b/spec/services/clusters/aws/fetch_credentials_service_spec.rb index 726d1c30603..9194947c67f 100644 --- a/spec/services/clusters/aws/fetch_credentials_service_spec.rb +++ b/spec/services/clusters/aws/fetch_credentials_service_spec.rb @@ -5,19 +5,18 @@ require 'spec_helper' describe Clusters::Aws::FetchCredentialsService do describe '#execute' do let(:user) { create(:user) } - let(:provider) { create(:cluster_provider_aws) } + let(:provider) { create(:cluster_provider_aws, region: 'ap-southeast-2') } 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: region) } let(:assumed_role) { instance_double(Aws::AssumeRoleCredentials, credentials: assumed_role_credentials) } let(:assumed_role_credentials) { double } - subject { described_class.new(provision_role, region: region, provider: provider).execute } + subject { described_class.new(provision_role, provider: provider).execute } context 'provision role is configured' do let(:provision_role) { create(:aws_role, user: user) } @@ -39,19 +38,30 @@ describe Clusters::Aws::FetchCredentialsService do client: sts_client, role_arn: provision_role.role_arn, role_session_name: session_name, - external_id: provision_role.role_external_id + external_id: provision_role.role_external_id, + policy: session_policy ).and_return(assumed_role) end context 'provider is specified' do + let(:region) { provider.region } let(:session_name) { "gitlab-eks-cluster-#{provider.cluster_id}-user-#{user.id}" } + let(:session_policy) { nil } it { is_expected.to eq assumed_role_credentials } end context 'provider is not specifed' do let(:provider) { nil } + let(:region) { Clusters::Providers::Aws::DEFAULT_REGION } let(:session_name) { "gitlab-eks-autofill-user-#{user.id}" } + let(:session_policy) { 'policy-document' } + + before do + allow(File).to receive(:read) + .with(Rails.root.join('vendor', 'aws', 'iam', 'eks_cluster_read_only_policy.json')) + .and_return(session_policy) + end it { is_expected.to eq assumed_role_credentials } end diff --git a/spec/services/clusters/aws/proxy_service_spec.rb b/spec/services/clusters/aws/proxy_service_spec.rb deleted file mode 100644 index 7b0e0512b95..00000000000 --- a/spec/services/clusters/aws/proxy_service_spec.rb +++ /dev/null @@ -1,210 +0,0 @@ -# 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/spec/services/clusters/kubernetes_spec.rb b/spec/services/clusters/kubernetes_spec.rb new file mode 100644 index 00000000000..7f2c5e0461d --- /dev/null +++ b/spec/services/clusters/kubernetes_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::Kubernetes do + it { is_expected.to be_const_defined(:GITLAB_SERVICE_ACCOUNT_NAME) } + it { is_expected.to be_const_defined(:GITLAB_SERVICE_ACCOUNT_NAMESPACE) } + it { is_expected.to be_const_defined(:GITLAB_ADMIN_TOKEN_NAME) } + it { is_expected.to be_const_defined(:GITLAB_CLUSTER_ROLE_BINDING_NAME) } + it { is_expected.to be_const_defined(:GITLAB_CLUSTER_ROLE_NAME) } + it { is_expected.to be_const_defined(:PROJECT_CLUSTER_ROLE_NAME) } + it { is_expected.to be_const_defined(:GITLAB_KNATIVE_SERVING_ROLE_NAME) } + it { is_expected.to be_const_defined(:GITLAB_KNATIVE_SERVING_ROLE_BINDING_NAME) } + it { is_expected.to be_const_defined(:GITLAB_CROSSPLANE_DATABASE_ROLE_NAME) } + it { is_expected.to be_const_defined(:GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME) } + it { is_expected.to be_const_defined(:GITLAB_KNATIVE_VERSION_ROLE_NAME) } + it { is_expected.to be_const_defined(:GITLAB_KNATIVE_VERSION_ROLE_BINDING_NAME) } + it { is_expected.to be_const_defined(:KNATIVE_SERVING_NAMESPACE) } +end diff --git a/vendor/aws/iam/eks_cluster_read_only_policy.json b/vendor/aws/iam/eks_cluster_read_only_policy.json new file mode 100644 index 00000000000..425b9a3eff9 --- /dev/null +++ b/vendor/aws/iam/eks_cluster_read_only_policy.json @@ -0,0 +1,17 @@ +{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "iam:ListRoles", + "ec2:DescribeKeyPairs", + "ec2:DescribeRegions", + "ec2:DescribeSecurityGroups", + "ec2:DescribeSubnets", + "ec2:DescribeVpcs" + ], + "Resource": "*" + } + ] +} |