diff options
Diffstat (limited to 'spec/services')
20 files changed, 801 insertions, 284 deletions
diff --git a/spec/services/ci/process_build_service_spec.rb b/spec/services/ci/process_build_service_spec.rb index 9a53b32394d..704685417bb 100644 --- a/spec/services/ci/process_build_service_spec.rb +++ b/spec/services/ci/process_build_service_spec.rb @@ -98,47 +98,19 @@ describe Ci::ProcessBuildService, '#execute' do let(:build) { create(:ci_build, :created, :schedulable, user: user, project: project) } - context 'when ci_enable_scheduled_build is enabled' do - before do - stub_feature_flags(ci_enable_scheduled_build: true) - end - - context 'when current status is success' do - let(:current_status) { 'success' } - - it 'changes the build status' do - expect { subject }.to change { build.status }.to('scheduled') - end - end - - context 'when current status is failed' do - let(:current_status) { 'failed' } + context 'when current status is success' do + let(:current_status) { 'success' } - it 'does not change the build status' do - expect { subject }.to change { build.status }.to('skipped') - end + it 'changes the build status' do + expect { subject }.to change { build.status }.to('scheduled') end end - context 'when ci_enable_scheduled_build is disabled' do - before do - stub_feature_flags(ci_enable_scheduled_build: false) - end - - context 'when current status is success' do - let(:current_status) { 'success' } - - it 'changes the build status' do - expect { subject }.to change { build.status }.to('manual') - end - end - - context 'when current status is failed' do - let(:current_status) { 'failed' } + context 'when current status is failed' do + let(:current_status) { 'failed' } - it 'does not change the build status' do - expect { subject }.to change { build.status }.to('skipped') - end + it 'does not change the build status' do + expect { subject }.to change { build.status }.to('skipped') end end end diff --git a/spec/services/ci/run_scheduled_build_service_spec.rb b/spec/services/ci/run_scheduled_build_service_spec.rb index 2c921dac238..be2aad33ef4 100644 --- a/spec/services/ci/run_scheduled_build_service_spec.rb +++ b/spec/services/ci/run_scheduled_build_service_spec.rb @@ -7,10 +7,6 @@ describe Ci::RunScheduledBuildService do subject { described_class.new(project, user).execute(build) } - before do - stub_feature_flags(ci_enable_scheduled_build: true) - end - context 'when user can update build' do before do project.add_developer(user) diff --git a/spec/services/clusters/create_service_spec.rb b/spec/services/clusters/create_service_spec.rb index 3959295c13e..274880f2c49 100644 --- a/spec/services/clusters/create_service_spec.rb +++ b/spec/services/clusters/create_service_spec.rb @@ -5,18 +5,43 @@ describe Clusters::CreateService do let(:project) { create(:project) } let(:user) { create(:user) } - subject { described_class.new(user, params).execute(project: project, access_token: access_token) } + subject { described_class.new(user, params).execute(access_token: access_token) } context 'when provider is gcp' do context 'when project has no clusters' do context 'when correct params' do - include_context 'valid cluster create params' + let(:params) do + { + name: 'test-cluster', + provider_type: :gcp, + provider_gcp_attributes: { + gcp_project_id: 'gcp-project', + zone: 'us-central1-a', + num_nodes: 1, + machine_type: 'machine_type-a', + legacy_abac: 'true' + }, + clusterable: project + } + end include_examples 'create cluster service success' end context 'when invalid params' do - include_context 'invalid cluster create params' + let(:params) do + { + name: 'test-cluster', + provider_type: :gcp, + provider_gcp_attributes: { + gcp_project_id: '!!!!!!!', + zone: 'us-central1-a', + num_nodes: 1, + machine_type: 'machine_type-a' + }, + clusterable: project + } + end include_examples 'create cluster service error' end diff --git a/spec/services/clusters/gcp/fetch_operation_service_spec.rb b/spec/services/clusters/gcp/fetch_operation_service_spec.rb index e2fa93904c5..55f123ee786 100644 --- a/spec/services/clusters/gcp/fetch_operation_service_spec.rb +++ b/spec/services/clusters/gcp/fetch_operation_service_spec.rb @@ -24,7 +24,7 @@ describe Clusters::Gcp::FetchOperationService do end end - context 'when suceeded to fetch operation' do + context 'when succeeded to fetch operation' do before do stub_cloud_platform_get_zone_operation(gcp_project_id, zone, operation_id) end diff --git a/spec/services/clusters/gcp/finalize_creation_service_spec.rb b/spec/services/clusters/gcp/finalize_creation_service_spec.rb index 0f484222228..7fbb6cf2cf5 100644 --- a/spec/services/clusters/gcp/finalize_creation_service_spec.rb +++ b/spec/services/clusters/gcp/finalize_creation_service_spec.rb @@ -1,156 +1,176 @@ +# frozen_string_literal: true + require 'spec_helper' -describe Clusters::Gcp::FinalizeCreationService do +describe Clusters::Gcp::FinalizeCreationService, '#execute' do include GoogleApi::CloudPlatformHelpers include KubernetesHelpers - describe '#execute' do - let(:cluster) { create(:cluster, :project, :providing_by_gcp) } - let(:provider) { cluster.provider } - let(:platform) { cluster.platform } - let(:gcp_project_id) { provider.gcp_project_id } - let(:zone) { provider.zone } - let(:cluster_name) { cluster.name } + let(:cluster) { create(:cluster, :project, :providing_by_gcp) } + let(:provider) { cluster.provider } + let(:platform) { cluster.platform } + let(:endpoint) { '111.111.111.111' } + let(:api_url) { 'https://' + endpoint } + let(:username) { 'sample-username' } + let(:password) { 'sample-password' } + let(:secret_name) { 'gitlab-token' } + let(:token) { 'sample-token' } + let(:namespace) { "#{cluster.project.path}-#{cluster.project.id}" } - subject { described_class.new.execute(provider) } + subject { described_class.new.execute(provider) } - shared_examples 'success' do - it 'configures provider and kubernetes' do - subject + shared_examples 'success' do + it 'configures provider and kubernetes' do + subject - expect(provider).to be_created - end + expect(provider).to be_created end - shared_examples 'error' do - it 'sets an error to provider object' do - subject + it 'properly configures database models' do + subject - expect(provider.reload).to be_errored - end + cluster.reload + + expect(provider.endpoint).to eq(endpoint) + expect(platform.api_url).to eq(api_url) + expect(platform.ca_cert).to eq(Base64.decode64(load_sample_cert)) + expect(platform.username).to eq(username) + expect(platform.password).to eq(password) + expect(platform.token).to eq(token) + end + + it 'creates kubernetes namespace model' do + subject + + kubernetes_namespace = cluster.reload.kubernetes_namespace + expect(kubernetes_namespace).to be_persisted + expect(kubernetes_namespace.namespace).to eq(namespace) + expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account") + expect(kubernetes_namespace.service_account_token).to be_present end + end + + shared_examples 'error' do + it 'sets an error to provider object' do + subject - context 'when suceeded to fetch gke cluster info' do - let(:endpoint) { '111.111.111.111' } - let(:api_url) { 'https://' + endpoint } - let(:username) { 'sample-username' } - let(:password) { 'sample-password' } - let(:secret_name) { 'gitlab-token' } + expect(provider.reload).to be_errored + end + end + shared_examples 'kubernetes information not successfully fetched' do + context 'when failed to fetch gke cluster info' do before do - stub_cloud_platform_get_zone_cluster( - gcp_project_id, zone, cluster_name, - { - endpoint: endpoint, - username: username, - password: password - } - ) + stub_cloud_platform_get_zone_cluster_error(provider.gcp_project_id, provider.zone, cluster.name) end - context 'service account and token created' do - before do - stub_kubeclient_discover(api_url) - stub_kubeclient_create_service_account(api_url) - stub_kubeclient_create_secret(api_url) - end - - shared_context 'kubernetes token successfully fetched' do - let(:token) { 'sample-token' } - - before do - stub_kubeclient_get_secret( - api_url, - { - metadata_name: secret_name, - token: Base64.encode64(token) - } ) - end - end - - context 'provider legacy_abac is enabled' do - include_context 'kubernetes token successfully fetched' - - it_behaves_like 'success' - - it 'properly configures database models' do - subject - - cluster.reload - - expect(provider.endpoint).to eq(endpoint) - expect(platform.api_url).to eq(api_url) - expect(platform.ca_cert).to eq(Base64.decode64(load_sample_cert)) - expect(platform.username).to eq(username) - expect(platform.password).to eq(password) - expect(platform).to be_abac - expect(platform.authorization_type).to eq('abac') - expect(platform.token).to eq(token) - end - end - - context 'provider legacy_abac is disabled' do - before do - provider.legacy_abac = false - end - - include_context 'kubernetes token successfully fetched' - - context 'cluster role binding created' do - before do - stub_kubeclient_create_cluster_role_binding(api_url) - end - - it_behaves_like 'success' - - it 'properly configures database models' do - subject - - cluster.reload - - expect(provider.endpoint).to eq(endpoint) - expect(platform.api_url).to eq(api_url) - expect(platform.ca_cert).to eq(Base64.decode64(load_sample_cert)) - expect(platform.username).to eq(username) - expect(platform.password).to eq(password) - expect(platform).to be_rbac - expect(platform.token).to eq(token) - end - end - end - - context 'when token is empty' do - before do - stub_kubeclient_get_secret(api_url, token: '', metadata_name: secret_name) - end - - it_behaves_like 'error' - end - - context 'when failed to fetch kubernetes token' do - before do - stub_kubeclient_get_secret_error(api_url, secret_name) - end - - it_behaves_like 'error' - end - - context 'when service account fails to create' do - before do - stub_kubeclient_create_service_account_error(api_url) - end - - it_behaves_like 'error' - end + it_behaves_like 'error' + end + + context 'when token is empty' do + let(:token) { '' } + + it_behaves_like 'error' + end + + context 'when failed to fetch kubernetes token' do + before do + stub_kubeclient_get_secret_error(api_url, secret_name, namespace: 'default') end + + it_behaves_like 'error' end - context 'when failed to fetch gke cluster info' do + context 'when service account fails to create' do before do - stub_cloud_platform_get_zone_cluster_error(gcp_project_id, zone, cluster_name) + stub_kubeclient_create_service_account_error(api_url, namespace: 'default') end it_behaves_like 'error' end end + + shared_context 'kubernetes information successfully fetched' do + before do + stub_cloud_platform_get_zone_cluster( + provider.gcp_project_id, provider.zone, cluster.name, + { + endpoint: endpoint, + username: username, + password: password + } + ) + + stub_kubeclient_discover(api_url) + stub_kubeclient_get_namespace(api_url) + stub_kubeclient_create_namespace(api_url) + stub_kubeclient_create_service_account(api_url) + stub_kubeclient_create_secret(api_url) + + stub_kubeclient_get_secret( + api_url, + { + metadata_name: secret_name, + token: Base64.encode64(token), + namespace: 'default' + } + ) + + stub_kubeclient_get_namespace(api_url, namespace: namespace) + stub_kubeclient_create_service_account(api_url, namespace: namespace) + stub_kubeclient_create_secret(api_url, namespace: namespace) + + stub_kubeclient_get_secret( + api_url, + { + metadata_name: "#{namespace}-token", + token: Base64.encode64(token), + namespace: namespace + } + ) + end + end + + context 'With a legacy ABAC cluster' do + before do + provider.legacy_abac = true + end + + include_context 'kubernetes information successfully fetched' + + it_behaves_like 'success' + + it 'uses ABAC authorization type' do + subject + cluster.reload + + expect(platform).to be_abac + expect(platform.authorization_type).to eq('abac') + end + + it_behaves_like 'kubernetes information not successfully fetched' + end + + context 'With an RBAC cluster' do + before do + provider.legacy_abac = false + + stub_kubeclient_create_cluster_role_binding(api_url) + stub_kubeclient_create_role_binding(api_url, namespace: namespace) + end + + include_context 'kubernetes information successfully fetched' + + it_behaves_like 'success' + + it 'uses RBAC authorization type' do + subject + cluster.reload + + expect(platform).to be_rbac + expect(platform.authorization_type).to eq('rbac') + end + + it_behaves_like 'kubernetes information not successfully fetched' + end end diff --git a/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb new file mode 100644 index 00000000000..fc922218ad0 --- /dev/null +++ b/spec/services/clusters/gcp/kubernetes/create_or_update_namespace_service_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::Gcp::Kubernetes::CreateOrUpdateNamespaceService, '#execute' do + include KubernetesHelpers + + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:platform) { cluster.platform } + let(:api_url) { 'https://kubernetes.example.com' } + let(:project) { cluster.project } + let(:cluster_project) { cluster.cluster_project } + + subject do + described_class.new( + cluster: cluster, + kubernetes_namespace: kubernetes_namespace + ).execute + end + + shared_context 'kubernetes requests' do + before do + stub_kubeclient_discover(api_url) + stub_kubeclient_get_namespace(api_url) + stub_kubeclient_create_service_account(api_url) + stub_kubeclient_create_secret(api_url) + + stub_kubeclient_get_namespace(api_url, namespace: namespace) + stub_kubeclient_create_service_account(api_url, namespace: namespace) + stub_kubeclient_create_secret(api_url, namespace: namespace) + + stub_kubeclient_get_secret( + api_url, + { + metadata_name: "#{namespace}-token", + token: Base64.encode64('sample-token'), + namespace: namespace + } + ) + end + end + + context 'when kubernetes namespace is not persisted' do + let(:namespace) { "#{project.path}-#{project.id}" } + + let(:kubernetes_namespace) do + build(:cluster_kubernetes_namespace, + cluster: cluster, + project: cluster_project.project, + cluster_project: cluster_project) + end + + include_context 'kubernetes requests' + + it 'creates a Clusters::KubernetesNamespace' do + expect do + subject + end.to change(Clusters::KubernetesNamespace, :count).by(1) + end + + it 'creates project service account' do + expect_any_instance_of(Clusters::Gcp::Kubernetes::CreateServiceAccountService).to receive(:execute).once + + subject + end + + it 'configures kubernetes token' do + subject + + kubernetes_namespace.reload + expect(kubernetes_namespace.namespace).to eq(namespace) + expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account") + expect(kubernetes_namespace.encrypted_service_account_token).to be_present + end + end + + context 'when there is a Kubernetes Namespace associated' do + let(:namespace) { 'new-namespace' } + + let(:kubernetes_namespace) do + create(:cluster_kubernetes_namespace, + cluster: cluster, + project: cluster_project.project, + cluster_project: cluster_project) + end + + include_context 'kubernetes requests' + + before do + platform.update_column(:namespace, 'new-namespace') + end + + it 'does not create any Clusters::KubernetesNamespace' do + subject + + expect(cluster.kubernetes_namespace).to eq(kubernetes_namespace) + end + + it 'creates project service account' do + expect_any_instance_of(Clusters::Gcp::Kubernetes::CreateServiceAccountService).to receive(:execute).once + + subject + end + + it 'updates Clusters::KubernetesNamespace' do + subject + + kubernetes_namespace.reload + + expect(kubernetes_namespace.namespace).to eq(namespace) + expect(kubernetes_namespace.service_account_name).to eq("#{namespace}-service-account") + expect(kubernetes_namespace.encrypted_service_account_token).to be_present + end + end +end diff --git a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb index b096f1fa4fb..588edff85d4 100644 --- a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb +++ b/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb @@ -1,94 +1,165 @@ # frozen_string_literal: true - require 'spec_helper' describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do include KubernetesHelpers - let(:service) { described_class.new(kubeclient, rbac: rbac) } + let(:api_url) { 'http://111.111.111.111' } + let(:platform_kubernetes) { cluster.platform_kubernetes } + let(:cluster_project) { cluster.cluster_project } + let(:project) { cluster_project.project } + let(:cluster) do + create(:cluster, + :project, :provided_by_gcp, + platform_kubernetes: create(:cluster_platform_kubernetes, :configured)) + end + + let(:kubeclient) do + Gitlab::Kubernetes::KubeClient.new( + api_url, + auth_options: { username: 'admin', password: 'xxx' } + ) + end - describe '#execute' do - let(:rbac) { false } - let(:api_url) { 'http://111.111.111.111' } - let(:username) { 'admin' } - let(:password) { 'xxx' } + shared_examples 'creates service account and token' do + it 'creates a kubernetes service account' do + subject + + expect(WebMock).to have_requested(:post, api_url + "/api/v1/namespaces/#{namespace}/serviceaccounts").with( + body: hash_including( + kind: 'ServiceAccount', + metadata: { name: service_account_name, namespace: namespace } + ) + ) + end - let(:kubeclient) do - Gitlab::Kubernetes::KubeClient.new( - api_url, - auth_options: { username: username, password: password } + it 'creates a kubernetes secret' do + subject + + expect(WebMock).to have_requested(:post, api_url + "/api/v1/namespaces/#{namespace}/secrets").with( + body: hash_including( + kind: 'Secret', + metadata: { + name: token_name, + namespace: namespace, + annotations: { + 'kubernetes.io/service-account.name': service_account_name + } + }, + type: 'kubernetes.io/service-account-token' + ) ) end + end + + before do + stub_kubeclient_discover(api_url) + stub_kubeclient_get_namespace(api_url, namespace: namespace) + stub_kubeclient_create_service_account(api_url, namespace: namespace ) + stub_kubeclient_create_secret(api_url, namespace: namespace) + end + + describe '.gitlab_creator' do + let(:namespace) { 'default' } + let(:service_account_name) { 'gitlab' } + let(:token_name) { 'gitlab-token' } + + subject { described_class.gitlab_creator(kubeclient, rbac: rbac).execute } + + context 'with ABAC cluster' do + let(:rbac) { false } + + it_behaves_like 'creates service account and token' + end - subject { service.execute } + context 'with RBAC cluster' do + let(:rbac) { true } - context 'when params are correct' do before do - stub_kubeclient_discover(api_url) - stub_kubeclient_create_service_account(api_url) - stub_kubeclient_create_secret(api_url) - end + cluster.platform_kubernetes.rbac! - shared_examples 'creates service account and token' do - it 'creates a kubernetes service account' do - subject + stub_kubeclient_create_cluster_role_binding(api_url) + end - expect(WebMock).to have_requested(:post, api_url + '/api/v1/namespaces/default/serviceaccounts').with( - body: hash_including( - kind: 'ServiceAccount', - metadata: { name: 'gitlab', namespace: 'default' } - ) - ) - end - - it 'creates a kubernetes secret of type ServiceAccountToken' do - subject - - expect(WebMock).to have_requested(:post, api_url + '/api/v1/namespaces/default/secrets').with( - body: hash_including( - kind: 'Secret', - metadata: { - name: 'gitlab-token', - namespace: 'default', - annotations: { - 'kubernetes.io/service-account.name': 'gitlab' - } - }, - type: 'kubernetes.io/service-account-token' - ) + it_behaves_like 'creates service account and token' + + it 'should create a cluster role binding with cluster-admin access' do + subject + + expect(WebMock).to have_requested(:post, api_url + "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings").with( + body: hash_including( + kind: 'ClusterRoleBinding', + metadata: { name: 'gitlab-admin' }, + roleRef: { + apiGroup: 'rbac.authorization.k8s.io', + kind: 'ClusterRole', + name: 'cluster-admin' + }, + subjects: [ + { + kind: 'ServiceAccount', + name: service_account_name, + namespace: namespace + } + ] ) - end + ) end + end + end + + describe '.namespace_creator' do + let(:namespace) { "#{project.path}-#{project.id}" } + let(:service_account_name) { "#{namespace}-service-account" } + let(:token_name) { "#{namespace}-token" } + + subject do + described_class.namespace_creator( + kubeclient, + service_account_name: service_account_name, + service_account_namespace: namespace, + rbac: rbac + ).execute + end + + context 'with ABAC cluster' do + let(:rbac) { false } + + it_behaves_like 'creates service account and token' + end + + context 'With RBAC enabled cluster' do + let(:rbac) { true } + + before do + cluster.platform_kubernetes.rbac! - context 'abac enabled cluster' do - it_behaves_like 'creates service account and token' + stub_kubeclient_create_role_binding(api_url, namespace: namespace) end - context 'rbac enabled cluster' do - let(:rbac) { true } - - before do - stub_kubeclient_create_cluster_role_binding(api_url) - end - - it_behaves_like 'creates service account and token' - - it 'creates a kubernetes cluster role binding' do - subject - - expect(WebMock).to have_requested(:post, api_url + '/apis/rbac.authorization.k8s.io/v1/clusterrolebindings').with( - body: hash_including( - kind: 'ClusterRoleBinding', - metadata: { name: 'gitlab-admin' }, - roleRef: { - apiGroup: 'rbac.authorization.k8s.io', - kind: 'ClusterRole', - name: 'cluster-admin' - }, - subjects: [{ kind: 'ServiceAccount', namespace: 'default', name: 'gitlab' }] - ) + it_behaves_like 'creates service account and token' + + it 'creates a namespaced role binding with edit access' do + subject + + expect(WebMock).to have_requested(:post, api_url + "/apis/rbac.authorization.k8s.io/v1/namespaces/#{namespace}/rolebindings").with( + body: hash_including( + kind: 'RoleBinding', + metadata: { name: "gitlab-#{namespace}", namespace: "#{namespace}" }, + roleRef: { + apiGroup: 'rbac.authorization.k8s.io', + kind: 'ClusterRole', + name: 'edit' + }, + subjects: [ + { + kind: 'ServiceAccount', + name: service_account_name, + namespace: namespace + } + ] ) - end + ) end end end diff --git a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb index 2355827fa5a..4d1a6bb7b3a 100644 --- a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb +++ b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb @@ -1,56 +1,48 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' describe Clusters::Gcp::Kubernetes::FetchKubernetesTokenService do + include KubernetesHelpers + describe '#execute' do let(:api_url) { 'http://111.111.111.111' } - let(:username) { 'admin' } - let(:password) { 'xxx' } + let(:namespace) { 'my-namespace' } + let(:service_account_token_name) { 'gitlab-token' } let(:kubeclient) do Gitlab::Kubernetes::KubeClient.new( api_url, - auth_options: { username: username, password: password } + auth_options: { username: 'admin', password: 'xxx' } ) end - subject { described_class.new(kubeclient).execute } + subject { described_class.new(kubeclient, service_account_token_name, namespace).execute } context 'when params correct' do let(:decoded_token) { 'xxx.token.xxx' } let(:token) { Base64.encode64(decoded_token) } - let(:secret_json) do - { - 'metadata': { - name: 'gitlab-token' - }, - 'data': { - 'token': token - } - } - end - - before do - allow_any_instance_of(Kubeclient::Client) - .to receive(:get_secret).and_return(secret_json) - end - context 'when gitlab-token exists' do - let(:metadata_name) { 'gitlab-token' } + before do + stub_kubeclient_discover(api_url) + stub_kubeclient_get_secret( + api_url, + { + metadata_name: service_account_token_name, + namespace: namespace, + token: token + } + ) + end it { is_expected.to eq(decoded_token) } end context 'when gitlab-token does not exist' do - let(:secret_json) { {} } - - it { is_expected.to be_nil } - end - - context 'when token is nil' do - let(:token) { nil } + before do + allow(kubeclient).to receive(:get_secret).and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) + end it { is_expected.to be_nil } end diff --git a/spec/services/clusters/gcp/provision_service_spec.rb b/spec/services/clusters/gcp/provision_service_spec.rb index f48afdc83b2..c0bdac40938 100644 --- a/spec/services/clusters/gcp/provision_service_spec.rb +++ b/spec/services/clusters/gcp/provision_service_spec.rb @@ -26,7 +26,7 @@ describe Clusters::Gcp::ProvisionService do end end - context 'when suceeded to request provision' do + context 'when succeeded to request provision' do before do stub_cloud_platform_create_cluster(gcp_project_id, zone) end diff --git a/spec/services/clusters/update_service_spec.rb b/spec/services/clusters/update_service_spec.rb index dcd75b6912d..a1b20c61116 100644 --- a/spec/services/clusters/update_service_spec.rb +++ b/spec/services/clusters/update_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Clusters::UpdateService do + include KubernetesHelpers + describe '#execute' do subject { described_class.new(cluster.user, params).execute(cluster) } @@ -34,6 +36,11 @@ describe Clusters::UpdateService do } end + before do + allow(ClusterPlatformConfigureWorker).to receive(:perform_async) + stub_kubeclient_get_namespace('https://kubernetes.example.com', namespace: 'my-namespace') + end + it 'updates namespace' do is_expected.to eq(true) expect(cluster.platform.namespace).to eq('custom-namespace') diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index d71ccfb4334..dd8a1cee074 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -177,7 +177,7 @@ describe Groups::TransferService, :postgresql do it 'should add an error on group' do transfer_service.execute(new_parent_group) - expect(transfer_service.error).to eq('Transfer failed: Validation failed: Path has already been taken') + expect(transfer_service.error).to eq('Transfer failed: Validation failed: Group URL has already been taken') end end @@ -347,7 +347,7 @@ describe Groups::TransferService, :postgresql do end end - context 'when transfering a group with nested groups and projects' do + context 'when transferring a group with nested groups and projects' do let!(:group) { create(:group, :public) } let!(:project1) { create(:project, :repository, :private, namespace: group) } let!(:subgroup1) { create(:group, :private, parent: group) } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 07aa8449a66..bd519e7f077 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -343,7 +343,42 @@ describe Issues::UpdateService, :mailer do end end - context 'when the milestone change' do + context 'when the milestone is removed' do + let!(:non_subscriber) { create(:user) } + + let!(:subscriber) do + create(:user) do |u| + issue.toggle_subscription(u, project) + project.add_developer(u) + end + end + + it_behaves_like 'system notes for milestones' + + it 'sends notifications for subscribers of changed milestone' do + issue.milestone = create(:milestone) + + issue.save + + perform_enqueued_jobs do + update_issue(milestone_id: "") + end + + should_email(subscriber) + should_not_email(non_subscriber) + end + end + + context 'when the milestone is changed' do + let!(:non_subscriber) { create(:user) } + + let!(:subscriber) do + create(:user) do |u| + issue.toggle_subscription(u, project) + project.add_developer(u) + end + end + it 'marks todos as done' do update_issue(milestone: create(:milestone)) @@ -351,6 +386,15 @@ describe Issues::UpdateService, :mailer do end it_behaves_like 'system notes for milestones' + + it 'sends notifications for subscribers of changed milestone' do + perform_enqueued_jobs do + update_issue(milestone: create(:milestone)) + end + + should_email(subscriber) + should_not_email(non_subscriber) + end end context 'when the labels change' do @@ -374,7 +418,7 @@ describe Issues::UpdateService, :mailer do let!(:non_subscriber) { create(:user) } let!(:subscriber) do - create(:user).tap do |u| + create(:user) do |u| label.toggle_subscription(u, project) project.add_developer(u) end diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 274624aa8bb..3e33a165e55 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -6,7 +6,7 @@ describe MergeRequests::GetUrlsService do let(:project) { create(:project, :public, :repository) } let(:service) { described_class.new(project) } let(:source_branch) { "merge-test" } - let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } + let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new/#{source_branch}" } let(:show_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" } @@ -117,7 +117,7 @@ describe MergeRequests::GetUrlsService do let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/markdown" } let(:changes) { "#{new_branch_changes}\n#{existing_branch_changes}" } - let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" } + let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.namespace.name}/#{project.path}/merge_requests/new/new_branch" } it 'returns 2 urls for both creating new and showing merge request' do result = service.execute(changes) diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 2536c6e2514..61c6ba7d550 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -306,6 +306,66 @@ describe MergeRequests::RefreshService do end end + context 'forked projects with the same source branch name as target branch' do + let!(:first_commit) do + @fork_project.repository.create_file(@user, 'test1.txt', 'Test data', + message: 'Test commit', + branch_name: 'master') + end + let!(:second_commit) do + @fork_project.repository.create_file(@user, 'test2.txt', 'More test data', + message: 'Second test commit', + branch_name: 'master') + end + let!(:forked_master_mr) do + create(:merge_request, + source_project: @fork_project, + source_branch: 'master', + target_branch: 'master', + target_project: @project) + end + let(:force_push_commit) { @project.commit('feature').id } + + it 'should reload a new diff for a push to the forked project' do + expect do + service.new(@fork_project, @user).execute(@oldrev, first_commit, 'refs/heads/master') + reload_mrs + end.to change { forked_master_mr.merge_request_diffs.count }.by(1) + end + + it 'should reload a new diff for a force push to the source branch' do + expect do + service.new(@fork_project, @user).execute(@oldrev, force_push_commit, 'refs/heads/master') + reload_mrs + end.to change { forked_master_mr.merge_request_diffs.count }.by(1) + end + + it 'should reload a new diff for a force push to the target branch' do + expect do + service.new(@project, @user).execute(@oldrev, force_push_commit, 'refs/heads/master') + reload_mrs + end.to change { forked_master_mr.merge_request_diffs.count }.by(1) + end + + it 'should reload a new diff for a push to the target project that contains a commit in the MR' do + expect do + service.new(@project, @user).execute(@oldrev, first_commit, 'refs/heads/master') + reload_mrs + end.to change { forked_master_mr.merge_request_diffs.count }.by(1) + end + + it 'should not increase the diff count for a new push to target branch' do + new_commit = @project.repository.create_file(@user, 'new-file.txt', 'A new file', + message: 'This is a test', + branch_name: 'master') + + expect do + service.new(@project, @user).execute(@newrev, new_commit, 'refs/heads/master') + reload_mrs + end.not_to change { forked_master_mr.merge_request_diffs.count } + end + end + context 'push to origin repo target branch after fork project was removed' do before do @fork_project.destroy diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 21f369a3818..546c9f277c5 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -60,6 +60,17 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin subject.execute end + + it 'avoids N+1 queries', :request_store do + current_user + merge_request + + control_count = ActiveRecord::QueryRecorder.new do + subject.execute + end.count + + expect { subject.execute }.not_to exceed_query_limit(control_count) + end end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 55dfab81c26..1b599ba11b6 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -315,7 +315,42 @@ describe MergeRequests::UpdateService, :mailer do end end - context 'when the milestone change' do + context 'when the milestone is removed' do + let!(:non_subscriber) { create(:user) } + + let!(:subscriber) do + create(:user) do |u| + merge_request.toggle_subscription(u, project) + project.add_developer(u) + end + end + + it_behaves_like 'system notes for milestones' + + it 'sends notifications for subscribers of changed milestone' do + merge_request.milestone = create(:milestone) + + merge_request.save + + perform_enqueued_jobs do + update_merge_request(milestone_id: "") + end + + should_email(subscriber) + should_not_email(non_subscriber) + end + end + + context 'when the milestone is changed' do + let!(:non_subscriber) { create(:user) } + + let!(:subscriber) do + create(:user) do |u| + merge_request.toggle_subscription(u, project) + project.add_developer(u) + end + end + it 'marks pending todos as done' do update_merge_request({ milestone: create(:milestone) }) @@ -323,6 +358,15 @@ describe MergeRequests::UpdateService, :mailer do end it_behaves_like 'system notes for milestones' + + it 'sends notifications for subscribers of changed milestone' do + perform_enqueued_jobs do + update_merge_request(milestone: create(:milestone)) + end + + should_email(subscriber) + should_not_email(non_subscriber) + end end context 'when the labels change' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 68a361fa882..2d8da7673dc 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -13,6 +13,54 @@ describe NotificationService, :mailer do end end + shared_examples 'altered milestone notification on issue' do + it 'sends the email to the correct people' do + should_email(subscriber_to_new_milestone) + issue.assignees.each do |a| + should_email(a) + end + should_email(@u_watcher) + should_email(@u_guest_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_email(@subscribed_participant) + should_email(@watcher_and_subscriber) + should_not_email(@u_guest_custom) + should_not_email(@u_committer) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_lazy_participant) + should_not_email(issue.author) + should_not_email(@u_disabled) + should_not_email(@u_custom_global) + should_not_email(@u_mentioned) + end + end + + shared_examples 'altered milestone notification on merge request' do + it 'sends the email to the correct people' do + should_email(subscriber_to_new_milestone) + merge_request.assignees.each do |a| + should_email(a) + end + should_email(@u_watcher) + should_email(@u_guest_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_email(@subscribed_participant) + should_email(@watcher_and_subscriber) + should_not_email(@u_guest_custom) + should_not_email(@u_committer) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_lazy_participant) + should_not_email(merge_request.author) + should_not_email(@u_disabled) + should_not_email(@u_custom_global) + should_not_email(@u_mentioned) + end + end + shared_examples 'notifications for new mentions' do it 'sends no emails when no new mentions are present' do send_notifications @@ -952,6 +1000,96 @@ describe NotificationService, :mailer do end end + describe '#removed_milestone_issue' do + it_behaves_like 'altered milestone notification on issue' do + let(:milestone) { create(:milestone, project: project, issues: [issue]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } + + before do + notification.removed_milestone_issue(issue, issue.author) + end + end + + context 'confidential issues' do + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:guest) { create(:user) } + let(:admin) { create(:admin) } + let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) } + let(:milestone) { create(:milestone, project: project, issues: [confidential_issue]) } + + it "emails subscribers of the issue's milestone that can read the issue" do + project.add_developer(member) + project.add_guest(guest) + + confidential_issue.subscribe(non_member, project) + confidential_issue.subscribe(author, project) + confidential_issue.subscribe(assignee, project) + confidential_issue.subscribe(member, project) + confidential_issue.subscribe(guest, project) + confidential_issue.subscribe(admin, project) + + reset_delivered_emails! + + notification.removed_milestone_issue(confidential_issue, @u_disabled) + + should_not_email(non_member) + should_not_email(guest) + should_email(author) + should_email(assignee) + should_email(member) + should_email(admin) + end + end + end + + describe '#changed_milestone_issue' do + it_behaves_like 'altered milestone notification on issue' do + let(:new_milestone) { create(:milestone, project: project, issues: [issue]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } + + before do + notification.changed_milestone_issue(issue, new_milestone, issue.author) + end + end + + context 'confidential issues' do + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:non_member) { create(:user) } + let(:member) { create(:user) } + let(:guest) { create(:user) } + let(:admin) { create(:admin) } + let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) } + let(:new_milestone) { create(:milestone, project: project, issues: [confidential_issue]) } + + it "emails subscribers of the issue's milestone that can read the issue" do + project.add_developer(member) + project.add_guest(guest) + + confidential_issue.subscribe(non_member, project) + confidential_issue.subscribe(author, project) + confidential_issue.subscribe(assignee, project) + confidential_issue.subscribe(member, project) + confidential_issue.subscribe(guest, project) + confidential_issue.subscribe(admin, project) + + reset_delivered_emails! + + notification.changed_milestone_issue(confidential_issue, new_milestone, @u_disabled) + + should_not_email(non_member) + should_not_email(guest) + should_email(author) + should_email(assignee) + should_email(member) + should_email(admin) + end + end + end + describe '#close_issue' do before do update_custom_notification(:close_issue, @u_guest_custom, resource: project) @@ -1304,6 +1442,28 @@ describe NotificationService, :mailer do end end + describe '#removed_milestone_merge_request' do + it_behaves_like 'altered milestone notification on merge request' do + let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + + before do + notification.removed_milestone_merge_request(merge_request, merge_request.author) + end + end + end + + describe '#changed_milestone_merge_request' do + it_behaves_like 'altered milestone notification on merge request' do + let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + + before do + notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) + end + end + end + describe '#merge_request_unmergeable' do it "sends email to merge request author" do notification.merge_request_unmergeable(merge_request) diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index e6ffa2b957b..06f865dc848 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -125,7 +125,7 @@ describe Projects::ImportService do project.import_type = 'bitbucket' end - it 'succeeds if repository import is successfull' do + it 'succeeds if repository import is successful' do expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true) expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true) expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return({}) diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 1411723fb9e..2e07d4f8013 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -125,7 +125,7 @@ describe Projects::TransferService do it { expect(project.errors.messages[:new_namespace].first).to eq 'Please select a new namespace for your project.' } end - context 'disallow transfering of project with tags' do + context 'disallow transferring of project with tags' do let(:container_repository) { create(:container_repository) } before do diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 41a170e4f25..e513ee7ae44 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -315,7 +315,7 @@ describe QuickActions::InterpretService do end shared_examples 'award command' do - it 'toggle award 100 emoji if content containts /award :100:' do + it 'toggle award 100 emoji if content contains /award :100:' do _, updates = service.execute(content, issuable) expect(updates).to eq(emoji_award: "100") @@ -1395,7 +1395,7 @@ describe QuickActions::InterpretService do it 'includes the formatted duration and proper verb' do _, explanations = service.explain(content, issue) - expect(explanations).to eq(['Substracts 2h spent time.']) + expect(explanations).to eq(['Subtracts 2h spent time.']) end end |