diff options
Diffstat (limited to 'spec')
56 files changed, 2243 insertions, 153 deletions
diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 42917d0d505..26a532ee01d 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -274,11 +274,43 @@ describe Projects::ClustersController do context 'when creates a cluster' do it 'creates a new cluster' do expect(ClusterProvisionWorker).to receive(:perform_async) + expect { go }.to change { Clusters::Cluster.count } .and change { Clusters::Platforms::Kubernetes.count } + expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) + + expect(project.clusters.first).to be_user + expect(project.clusters.first).to be_kubernetes + end + end + + context 'when creates a RBAC-enabled cluster' do + let(:params) do + { + cluster: { + name: 'new-cluster', + platform_kubernetes_attributes: { + api_url: 'http://my-url', + token: 'test', + namespace: 'aaa', + authorization_type: 'rbac' + } + } + } + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + + expect { go }.to change { Clusters::Cluster.count } + .and change { Clusters::Platforms::Kubernetes.count } + + expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) + expect(project.clusters.first).to be_user expect(project.clusters.first).to be_kubernetes + expect(project.clusters.first).to be_platform_kubernetes_rbac end end end diff --git a/spec/factories/clusters/platforms/kubernetes.rb b/spec/factories/clusters/platforms/kubernetes.rb index 89f6ddebf6a..36ac2372204 100644 --- a/spec/factories/clusters/platforms/kubernetes.rb +++ b/spec/factories/clusters/platforms/kubernetes.rb @@ -16,5 +16,9 @@ FactoryBot.define do platform_kubernetes.ca_cert = File.read(pem_file) end end + + trait :rbac_enabled do + authorization_type :rbac + end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 1215b04913e..17e457c04a5 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -130,6 +130,33 @@ FactoryBot.define do end end + # Build a custom repository by specifying a hash of `filename => content` in + # the transient `files` attribute. Each file will be created in its own + # commit, operating against the master branch. So, the following call: + # + # create(:project, :custom_repo, files: { 'foo/a.txt' => 'foo', 'b.txt' => bar' }) + # + # will create a repository containing two files, and two commits, in master + trait :custom_repo do + transient do + files {} + end + + after :create do |project, evaluator| + raise "Failed to create repository!" unless project.create_repository + + evaluator.files.each do |filename, content| + project.repository.create_file( + project.creator, + filename, + content, + message: "Automatically created file #{filename}", + branch_name: 'master' + ) + end + end + end + # Test repository - https://gitlab.com/gitlab-org/gitlab-test trait :repository do test_repo diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 1064f72c271..2ba4d4918ff 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -5,8 +5,8 @@ describe 'File blob', :js do let(:project) { create(:project, :public, :repository) } - def visit_blob(path, anchor: nil, ref: 'master') - visit project_blob_path(project, File.join(ref, path), anchor: anchor) + def visit_blob(path, anchor: nil, ref: 'master', legacy_render: nil) + visit project_blob_path(project, File.join(ref, path), anchor: anchor, legacy_render: legacy_render) wait_for_requests end @@ -142,6 +142,52 @@ describe 'File blob', :js do end end + context 'Markdown rendering' do + before do + project.add_maintainer(project.creator) + + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add RedCarpet and CommonMark Markdown ", + file_path: 'files/commonmark/file.md', + file_content: "1. one\n - sublist\n" + ).execute + end + + context 'when rendering default markdown' do + before do + visit_blob('files/commonmark/file.md') + + wait_for_requests + end + + it 'renders using CommonMark' do + aggregate_failures do + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end + end + end + + context 'when rendering legacy markdown' do + before do + visit_blob('files/commonmark/file.md', legacy_render: 1) + + wait_for_requests + end + + it 'renders using RedCarpet' do + aggregate_failures do + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end + end + end + context 'Markdown file (stored in LFS)' do before do project.add_maintainer(project.creator) diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index 0e036b4ea68..d5b20605860 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -7,6 +7,7 @@ describe 'Editing file blob', :js do let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') } let(:branch) { 'master' } let(:file_path) { project.repository.ls_files(project.repository.root_ref)[1] } + let(:readme_file_path) { 'README.md' } context 'as a developer' do let(:user) { create(:user) } @@ -20,14 +21,19 @@ describe 'Editing file blob', :js do def edit_and_commit(commit_changes: true) wait_for_requests find('.js-edit-blob').click - find('#editor') - execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")') + fill_editor(content: "class NextFeature\\nend\\n") if commit_changes click_button 'Commit changes' end end + def fill_editor(content: "class NextFeature\\nend\\n") + wait_for_requests + find('#editor') + execute_script("ace.edit('editor').setValue('#{content}')") + end + context 'from MR diff' do before do visit diffs_project_merge_request_path(project, merge_request) @@ -63,6 +69,30 @@ describe 'Editing file blob', :js do expect(new_line_count).to be > 0 end end + + context 'when rendering the preview' do + it 'renders content with CommonMark' do + visit project_edit_blob_path(project, tree_join(branch, readme_file_path)) + fill_editor(content: "1. one\\n - sublist\\n") + click_link 'Preview' + wait_for_requests + + # the above generates two seperate lists (not embedded) in CommonMark + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end + + it 'renders content with RedCarpet when legacy_render is set' do + visit project_edit_blob_path(project, tree_join(branch, readme_file_path), legacy_render: 1) + fill_editor(content: "1. one\\n - sublist\\n") + click_link 'Preview' + wait_for_requests + + # the above generates a sublist list in RedCarpet + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end end context 'visit blob edit' do diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb index babf47cc341..ec968bfcf7d 100644 --- a/spec/features/projects/clusters/user_spec.rb +++ b/spec/features/projects/clusters/user_spec.rb @@ -38,6 +38,28 @@ describe 'User Cluster', :js do end end + context 'rbac_clusters feature flag is enabled' do + before do + stub_feature_flags(rbac_clusters: true) + + fill_in 'cluster_name', with: 'dev-cluster' + fill_in 'cluster_platform_kubernetes_attributes_api_url', with: 'http://example.com' + fill_in 'cluster_platform_kubernetes_attributes_token', with: 'my-token' + check 'cluster_platform_kubernetes_attributes_authorization_type' + click_button 'Add Kubernetes cluster' + end + + it 'user sees a cluster details page' do + expect(page).to have_content('Kubernetes cluster integration') + expect(page.find_field('cluster[name]').value).to eq('dev-cluster') + expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) + .to have_content('http://example.com') + expect(page.find_field('cluster[platform_kubernetes_attributes][token]').value) + .to have_content('my-token') + expect(page.find_field('cluster[platform_kubernetes_attributes][authorization_type]', disabled: true)).to be_checked + end + end + context 'when user filled form with invalid parameters' do before do click_button 'Add Kubernetes cluster' diff --git a/spec/features/projects/files/project_owner_creates_license_file_spec.rb b/spec/features/projects/files/project_owner_creates_license_file_spec.rb index ac6c8c337fa..6762460971f 100644 --- a/spec/features/projects/files/project_owner_creates_license_file_spec.rb +++ b/spec/features/projects/files/project_owner_creates_license_file_spec.rb @@ -36,7 +36,7 @@ describe 'Projects > Files > Project owner creates a license file', :js do end it 'project maintainer creates a license file from the "Add license" link' do - click_link 'Add License' + click_link 'Add license' expect(page).to have_content('New file') expect(current_path).to eq( diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb index 801291c1f77..0b8474fb87a 100644 --- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb +++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -10,7 +10,7 @@ describe 'Projects > Files > Project owner sees a link to create a license file it 'project maintainer creates a license file from a template' do visit project_path(project) - click_on 'Add License' + click_on 'Add license' expect(page).to have_content('New file') expect(current_path).to eq( diff --git a/spec/features/projects/settings/integration_settings_spec.rb b/spec/features/projects/settings/integration_settings_spec.rb index 8745ff72df0..32959969f54 100644 --- a/spec/features/projects/settings/integration_settings_spec.rb +++ b/spec/features/projects/settings/integration_settings_spec.rb @@ -51,6 +51,7 @@ describe 'Projects > Settings > Integration settings' do fill_in 'hook_url', with: url check 'Tag push events' + fill_in 'hook_push_events_branch_filter', with: 'master' check 'Enable SSL verification' check 'Job events' diff --git a/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb b/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb index 0405e21a0d7..b8326edd4fd 100644 --- a/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb +++ b/spec/features/projects/show/user_sees_setup_shortcut_buttons_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'Projects > Show > User sees setup shortcut buttons' do - # For "New file", "Add License" functionality, + # For "New file", "Add license" functionality, # see spec/features/projects/files/project_owner_creates_license_file_spec.rb # see spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -58,9 +58,9 @@ describe 'Projects > Show > User sees setup shortcut buttons' do end end - it '"Add License" button linked to new file populated for a license' do - page.within('.project-stats') do - expect(page).to have_link('Add License', href: presenter.add_license_path) + it '"Add license" button linked to new file populated for a license' do + page.within('.project-metadata') do + expect(page).to have_link('Add license', href: presenter.add_license_path) end end @@ -201,13 +201,13 @@ describe 'Projects > Show > User sees setup shortcut buttons' do end end - it 'no "Add License" button if the project already has a license' do + it 'no "Add license" button if the project already has a license' do visit project_path(project) expect(project.repository.license_blob).not_to be_nil page.within('.project-stats') do - expect(page).not_to have_link('Add License') + expect(page).not_to have_link('Add license') end end diff --git a/spec/features/projects/wiki/markdown_preview_spec.rb b/spec/features/projects/wiki/markdown_preview_spec.rb index ed5f8105487..f505023d1d0 100644 --- a/spec/features/projects/wiki/markdown_preview_spec.rb +++ b/spec/features/projects/wiki/markdown_preview_spec.rb @@ -162,6 +162,34 @@ describe 'Projects > Wiki > User previews markdown changes', :js do expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>") end end + + context 'when rendering the preview' do + it 'renders content with CommonMark' do + create_wiki_page 'a-page/b-page/c-page/common-mark' + click_link 'Edit' + + fill_in :wiki_content, with: "1. one\n - sublist\n" + click_on "Preview" + + # the above generates two seperate lists (not embedded) in CommonMark + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end + + it 'renders content with RedCarpet when legacy_render is set' do + wiki_page = create(:wiki_page, + wiki: project.wiki, + attrs: { title: 'home', content: "Empty content" }) + visit(project_wiki_edit_path(project, wiki_page, legacy_render: 1)) + + fill_in :wiki_content, with: "1. one\n - sublist\n" + click_on "Preview" + + # the above generates a sublist list in RedCarpet + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end end it "does not linkify double brackets inside code blocks as expected" do diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 56ed0c936a6..22e3a99072f 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe 'Project' do include ProjectForksHelper + include MobileHelpers describe 'creating from template' do let(:user) { create(:user) } @@ -54,25 +55,72 @@ describe 'Project' do it 'parses Markdown' do project.update_attribute(:description, 'This is **my** project') visit path - expect(page).to have_css('.project-home-desc > p > strong') + expect(page).to have_css('.project-description > .project-description-markdown > p > strong') end it 'passes through html-pipeline' do project.update_attribute(:description, 'This project is the :poop:') visit path - expect(page).to have_css('.project-home-desc > p > gl-emoji') + expect(page).to have_css('.project-description > .project-description-markdown > p > gl-emoji') end it 'sanitizes unwanted tags' do project.update_attribute(:description, "```\ncode\n```") visit path - expect(page).not_to have_css('.project-home-desc code') + expect(page).not_to have_css('.project-description code') end it 'permits `rel` attribute on links' do project.update_attribute(:description, 'https://google.com/') visit path - expect(page).to have_css('.project-home-desc a[rel]') + expect(page).to have_css('.project-description a[rel]') + end + + context 'read more', :js do + let(:read_more_selector) { '.read-more-container' } + let(:read_more_trigger_selector) { '.project-home-desc .js-read-more-trigger' } + + it 'does not display "read more" link on desktop breakpoint' do + project.update_attribute(:description, 'This is **my** project') + visit path + + expect(find(read_more_trigger_selector, visible: false)).not_to be_visible + end + + it 'displays "read more" link on mobile breakpoint' do + project.update_attribute(:description, 'This is **my** project') + visit path + resize_screen_xs + + find(read_more_trigger_selector).click + + expect(page).to have_css('.project-description .is-expanded') + end + end + end + + describe 'copy clone URL to clipboard', :js do + let(:project) { create(:project, :repository) } + let(:path) { project_path(project) } + + before do + sign_in(create(:admin)) + visit path + end + + context 'desktop component' do + it 'shows on md and larger breakpoints' do + expect(find('.git-clone-holder')).to be_visible + expect(find('.mobile-git-clone', visible: false)).not_to be_visible + end + end + + context 'mobile component' do + it 'shows mobile component on sm and smaller breakpoints' do + resize_screen_xs + expect(find('.mobile-git-clone')).to be_visible + expect(find('.git-clone-holder', visible: false)).not_to be_visible + end end end diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index 3fe0b60b18f..367a479f62a 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -68,23 +68,45 @@ describe 'Snippet', :js do end end - context 'with cached Redcarpet html' do - let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION) } + context 'Markdown rendering' do + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content) } let(:file_name) { 'test.md' } let(:content) { "1. one\n - sublist\n" } - it 'renders correctly' do - expect(page).to have_xpath("//ol//li//ul") + context 'when rendering default markdown' do + it 'renders using CommonMark' do + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end end - end - context 'with cached CommonMark html' do - let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } - let(:file_name) { 'test.md' } - let(:content) { "1. one\n - sublist\n" } + context 'when rendering legacy markdown' do + before do + visit snippet_path(snippet, legacy_render: 1) - it 'renders correctly' do - expect(page).not_to have_xpath("//ol//li//ul") + wait_for_requests + end + + it 'renders using RedCarpet' do + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end + + context 'with cached CommonMark html' do + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + + it 'renders correctly' do + expect(page).not_to have_xpath("//ol//li//ul") + end + end + + context 'with cached Redcarpet html' do + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION) } + + it 'renders correctly' do + expect(page).to have_xpath("//ol//li//ul") + end end end diff --git a/spec/finders/template_finder_spec.rb b/spec/finders/template_finder_spec.rb new file mode 100644 index 00000000000..1d399e8194f --- /dev/null +++ b/spec/finders/template_finder_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe TemplateFinder do + using RSpec::Parameterized::TableSyntax + + describe '#build' do + where(:type, :expected_class) do + :dockerfiles | described_class + :gitignores | described_class + :gitlab_ci_ymls | described_class + :licenses | ::LicenseTemplateFinder + end + + with_them do + subject { described_class.build(type) } + + it { is_expected.to be_a(expected_class) } + end + end + + describe '#execute' do + where(:type, :vendored_name) do + :dockerfiles | 'Binary' + :gitignores | 'Actionscript' + :gitlab_ci_ymls | 'Android' + end + + with_them do + it 'returns all vendored templates when no name is specified' do + result = described_class.new(type).execute + + expect(result).to include(have_attributes(name: vendored_name)) + end + + it 'returns only the specified vendored template when a name is specified' do + result = described_class.new(type, name: vendored_name).execute + + expect(result).to have_attributes(name: vendored_name) + end + + it 'returns nil when an unknown name is specified' do + result = described_class.new(type, name: 'unknown').execute + + expect(result).to be_nil + end + end + end +end diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index 597648b064d..9a29ac26eff 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -25,17 +25,17 @@ describe MarkupHelper do let(:actual) { "#{merge_request.to_reference} -> #{commit.to_reference} -> #{issue.to_reference}" } it "links to the merge request" do - expected = project_merge_request_path(project, merge_request) + expected = urls.project_merge_request_path(project, merge_request) expect(helper.markdown(actual)).to match(expected) end it "links to the commit" do - expected = project_commit_path(project, commit) + expected = urls.project_commit_path(project, commit) expect(helper.markdown(actual)).to match(expected) end it "links to the issue" do - expected = project_issue_path(project, issue) + expected = urls.project_issue_path(project, issue) expect(helper.markdown(actual)).to match(expected) end end @@ -46,7 +46,7 @@ describe MarkupHelper do let(:second_issue) { create(:issue, project: second_project) } it 'links to the issue' do - expected = project_issue_path(second_project, second_issue) + expected = urls.project_issue_path(second_project, second_issue) expect(markdown(actual, project: second_project)).to match(expected) end end @@ -93,7 +93,7 @@ describe MarkupHelper do # First issue link expect(doc.css('a')[1].attr('href')) - .to eq project_issue_path(project, issues[0]) + .to eq urls.project_issue_path(project, issues[0]) expect(doc.css('a')[1].text).to eq issues[0].to_reference # Internal commit link @@ -102,7 +102,7 @@ describe MarkupHelper do # Second issue link expect(doc.css('a')[3].attr('href')) - .to eq project_issue_path(project, issues[1]) + .to eq urls.project_issue_path(project, issues[1]) expect(doc.css('a')[3].text).to eq issues[1].to_reference # Trailing commit link @@ -128,7 +128,7 @@ describe MarkupHelper do # First issue link expect(doc.css('a')[1].attr('href')) - .to eq project_issue_path(project, issues[0]) + .to eq urls.project_issue_path(project, issues[0]) expect(doc.css('a')[1].text).to eq issues[0].to_reference # Internal commit link @@ -137,7 +137,7 @@ describe MarkupHelper do # Second issue link expect(doc.css('a')[3].attr('href')) - .to eq project_issue_path(project, issues[1]) + .to eq urls.project_issue_path(project, issues[1]) expect(doc.css('a')[3].text).to eq issues[1].to_reference # Trailing commit link @@ -183,7 +183,7 @@ describe MarkupHelper do doc = Nokogiri::HTML.parse(rendered) expect(doc.css('a')[0].attr('href')) - .to eq project_issue_path(project, issue) + .to eq urls.project_issue_path(project, issue) expect(doc.css('a')[0].text).to eq issue.to_reference wrapped = helper.link_to_html(rendered, link) @@ -207,6 +207,17 @@ describe MarkupHelper do expect(helper).to receive(:markdown_unsafe).with('wiki content', pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page", + issuable_state_filter_enabled: true) + + helper.render_wiki_content(@wiki) + end + + it 'uses Wiki pipeline for markdown files with RedCarpet if feature disabled' do + stub_feature_flags(commonmark_for_repositories: false) + allow(@wiki).to receive(:format).and_return(:markdown) + + expect(helper).to receive(:markdown_unsafe).with('wiki content', + pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page", issuable_state_filter_enabled: true, markdown_engine: :redcarpet) helper.render_wiki_content(@wiki) @@ -259,10 +270,18 @@ describe MarkupHelper do expect(helper.markup('foo.md', content, rendered: '<p>NOEL</p>')).to eq('<p>NOEL</p>') end - it 'defaults to Redcarpet' do - expect(helper).to receive(:markdown_unsafe).with(content, hash_including(markdown_engine: :redcarpet)).and_return('NOEL') + it 'defaults to CommonMark' do + expect(helper.markup('foo.md', 'x^2')).to include('x^2') + end - expect(helper.markup('foo.md', content)).to eq('NOEL') + it 'honors markdown_engine for RedCarpet' do + expect(helper.markup('foo.md', 'x^2', { markdown_engine: :redcarpet })).to include('x<sup>2</sup>') + end + + it 'uses RedCarpet if feature disabled' do + stub_feature_flags(commonmark_for_repositories: false) + + expect(helper.markup('foo.md', 'x^2', { markdown_engine: :redcarpet })).to include('x<sup>2</sup>') end end @@ -414,4 +433,8 @@ describe MarkupHelper do expect(helper.cross_project_reference(project, issue)).to include(project.full_path) end end + + def urls + Gitlab::Routing.url_helpers + end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index cbd4ff0fb4a..976b6c312b4 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -470,4 +470,16 @@ describe ProjectsHelper do end end end + + describe '#legacy_render_context' do + it 'returns the redcarpet engine' do + params = { legacy_render: '1' } + + expect(helper.legacy_render_context(params)).to include(markdown_engine: :redcarpet) + end + + it 'returns nothing' do + expect(helper.legacy_render_context({})).to be_empty + end + end end diff --git a/spec/javascripts/fixtures/projects.rb b/spec/javascripts/fixtures/projects.rb index 57c78182abc..d98f7f55b20 100644 --- a/spec/javascripts/fixtures/projects.rb +++ b/spec/javascripts/fixtures/projects.rb @@ -6,6 +6,7 @@ describe 'Projects (JavaScript fixtures)', type: :controller do let(:admin) { create(:admin) } let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} let(:project) { create(:project, namespace: namespace, path: 'builds-project') } + let(:project_with_repo) { create(:project, :repository, description: 'Code and stuff') } let(:project_variable_populated) { create(:project, namespace: namespace, path: 'builds-project2') } let!(:variable1) { create(:ci_variable, project: project_variable_populated) } let!(:variable2) { create(:ci_variable, project: project_variable_populated) } @@ -35,6 +36,15 @@ describe 'Projects (JavaScript fixtures)', type: :controller do store_frontend_fixture(response, example.description) end + it 'projects/overview.html.raw' do |example| + get :show, + namespace_id: project_with_repo.namespace.to_param, + id: project_with_repo + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end + it 'projects/edit.html.raw' do |example| get :edit, namespace_id: project.namespace.to_param, diff --git a/spec/javascripts/read_more_spec.js b/spec/javascripts/read_more_spec.js new file mode 100644 index 00000000000..b1af0f80a50 --- /dev/null +++ b/spec/javascripts/read_more_spec.js @@ -0,0 +1,23 @@ +import initReadMore from '~/read_more'; + +describe('Read more click-to-expand functionality', () => { + const fixtureName = 'projects/overview.html.raw'; + + preloadFixtures(fixtureName); + + beforeEach(() => { + loadFixtures(fixtureName); + }); + + describe('expands target element', () => { + it('adds "is-expanded" class to target element', () => { + const target = document.querySelector('.read-more-container'); + const trigger = document.querySelector('.js-read-more-trigger'); + initReadMore(); + + trigger.click(); + + expect(target.classList.contains('is-expanded')).toEqual(true); + }); + }); +}); diff --git a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb index 88ae4c1e07a..52b8c9be647 100644 --- a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb @@ -121,6 +121,13 @@ describe Banzai::Pipeline::WikiPipeline do expect(output).to include("href=\"#{relative_url_root}/wiki_link_ns/wiki_link_project/wikis/page\"") end + it 'rewrites non-file links (with spaces) to be at the scope of the wiki root' do + markdown = "[Link to Page](page slug)" + output = described_class.to_html(markdown, project: project, project_wiki: project_wiki, page_slug: page.slug) + + expect(output).to include("href=\"#{relative_url_root}/wiki_link_ns/wiki_link_project/wikis/page%20slug\"") + end + it "rewrites file links to be at the scope of the current directory" do markdown = "[Link to Page](page.md)" output = described_class.to_html(markdown, project: project, project_wiki: project_wiki, page_slug: page.slug) @@ -134,6 +141,13 @@ describe Banzai::Pipeline::WikiPipeline do expect(output).to include("href=\"#{relative_url_root}/wiki_link_ns/wiki_link_project/wikis/start-page#title\"") end + + it 'rewrites links (with spaces) with anchor' do + markdown = '[Link to Header](start page#title)' + output = described_class.to_html(markdown, project: project, project_wiki: project_wiki, page_slug: page.slug) + + expect(output).to include("href=\"#{relative_url_root}/wiki_link_ns/wiki_link_project/wikis/start%20page#title\"") + end end describe "when creating root links" do diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 8bb5a843484..48c0ba8a653 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -119,6 +119,10 @@ describe Feature do expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey end + it 'returns true for undefined feature with default_enabled' do + expect(described_class.enabled?(:some_random_feature_flag, default_enabled: true)).to be_truthy + end + it 'returns false for existing disabled feature in the database' do described_class.disable(:disabled_feature_flag) @@ -160,6 +164,10 @@ describe Feature do expect(described_class.disabled?(:some_random_feature_flag)).to be_truthy end + it 'returns false for undefined feature with default_enabled' do + expect(described_class.disabled?(:some_random_feature_flag, default_enabled: true)).to be_falsey + end + it 'returns true for existing disabled feature in the database' do described_class.disable(:disabled_feature_flag) diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 6769f64f950..2c9758401b7 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -1,4 +1,5 @@ -require 'spec_helper' +require 'fast_spec_helper' +require_dependency 'active_model' describe Gitlab::Ci::Config::Entry::Job do let(:entry) { described_class.new(config, name: :rspec) } @@ -81,6 +82,15 @@ describe Gitlab::Ci::Config::Entry::Job do end end + context 'when extends key is not a string' do + let(:config) { { extends: 123 } } + + it 'returns error about wrong value type' do + expect(entry).not_to be_valid + expect(entry.errors).to include "job extends should be a string" + end + end + context 'when retry value is not correct' do context 'when it is not a numeric value' do let(:config) { { retry: true } } @@ -124,6 +134,8 @@ describe Gitlab::Ci::Config::Entry::Job do describe '#relevant?' do it 'is a relevant entry' do + entry = described_class.new({ script: 'rspec' }, name: :rspec) + expect(entry).to be_relevant end end diff --git a/spec/lib/gitlab/ci/config/extendable/entry_spec.rb b/spec/lib/gitlab/ci/config/extendable/entry_spec.rb new file mode 100644 index 00000000000..0a148375d11 --- /dev/null +++ b/spec/lib/gitlab/ci/config/extendable/entry_spec.rb @@ -0,0 +1,227 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::Config::Extendable::Entry do + describe '.new' do + context 'when entry key is not included in the context hash' do + it 'raises error' do + expect { described_class.new(:test, something: 'something') } + .to raise_error StandardError, 'Invalid entry key!' + end + end + end + + describe '#value' do + it 'reads a hash value from the context' do + entry = described_class.new(:test, test: 'something') + + expect(entry.value).to eq 'something' + end + end + + describe '#extensible?' do + context 'when entry has inheritance defined' do + it 'is extensible' do + entry = described_class.new(:test, test: { extends: 'something' }) + + expect(entry).to be_extensible + end + end + + context 'when entry does not have inheritance specified' do + it 'is not extensible' do + entry = described_class.new(:test, test: { script: 'something' }) + + expect(entry).not_to be_extensible + end + end + + context 'when entry value is not a hash' do + it 'is not extensible' do + entry = described_class.new(:test, test: 'something') + + expect(entry).not_to be_extensible + end + end + end + + describe '#extends_key' do + context 'when entry is extensible' do + it 'returns symbolized extends key value' do + entry = described_class.new(:test, test: { extends: 'something' }) + + expect(entry.extends_key).to eq :something + end + end + + context 'when entry is not extensible' do + it 'returns nil' do + entry = described_class.new(:test, test: 'something') + + expect(entry.extends_key).to be_nil + end + end + end + + describe '#ancestors' do + let(:parent) do + described_class.new(:test, test: { extends: 'something' }) + end + + let(:child) do + described_class.new(:job, { job: { script: 'something' } }, parent) + end + + it 'returns ancestors keys' do + expect(child.ancestors).to eq [:test] + end + end + + describe '#base_hash!' do + subject { described_class.new(:test, hash) } + + context 'when base hash is not extensible' do + let(:hash) do + { + template: { script: 'rspec' }, + test: { extends: 'template' } + } + end + + it 'returns unchanged base hash' do + expect(subject.base_hash!).to eq(script: 'rspec') + end + end + + context 'when base hash is extensible too' do + let(:hash) do + { + first: { script: 'rspec' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + it 'extends the base hash first' do + expect(subject.base_hash!).to eq(extends: 'first', script: 'rspec') + end + + it 'mutates original context' do + subject.base_hash! + + expect(hash.fetch(:second)).to eq(extends: 'first', script: 'rspec') + end + end + end + + describe '#extend!' do + subject { described_class.new(:test, hash) } + + context 'when extending a non-hash value' do + let(:hash) do + { + first: 'my value', + test: { extends: 'first' } + } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::InvalidExtensionError, + /invalid base hash/) + end + end + + context 'when extending unknown key' do + let(:hash) do + { test: { extends: 'something' } } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::InvalidExtensionError, + /unknown key/) + end + end + + context 'when extending a hash correctly' do + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + let(:result) do + { + first: { script: 'my value' }, + second: { extends: 'first', script: 'my value' }, + test: { extends: 'second', script: 'my value' } + } + end + + it 'returns extended part of the hash' do + expect(subject.extend!).to eq result[:test] + end + + it 'mutates original context' do + subject.extend! + + expect(hash).to eq result + end + end + + context 'when hash is not extensible' do + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { value: 'something' } + } + end + + it 'returns original key value' do + expect(subject.extend!).to eq(value: 'something') + end + + it 'does not mutate orignal context' do + original = hash.deep_dup + + subject.extend! + + expect(hash).to eq original + end + end + + context 'when circular depenency gets detected' do + let(:hash) do + { test: { extends: 'test' } } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::CircularDependencyError, + /circular dependency detected/) + end + end + + context 'when nesting level is too deep' do + before do + stub_const("#{described_class}::MAX_NESTING_LEVELS", 0) + end + + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + it 'raises an error' do + expect { subject.extend! } + .to raise_error(described_class::NestingTooDeepError) + end + end + end +end diff --git a/spec/lib/gitlab/ci/config/extendable_spec.rb b/spec/lib/gitlab/ci/config/extendable_spec.rb new file mode 100644 index 00000000000..90213f6603d --- /dev/null +++ b/spec/lib/gitlab/ci/config/extendable_spec.rb @@ -0,0 +1,228 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::Config::Extendable do + subject { described_class.new(hash) } + + describe '#each' do + context 'when there is extendable entry in the hash' do + let(:test) do + { extends: 'something', only: %w[master] } + end + + let(:hash) do + { something: { script: 'ls' }, test: test } + end + + it 'yields control' do + expect { |b| subject.each(&b) }.to yield_control + end + end + end + + describe '#to_hash' do + context 'when hash does not contain extensions' do + let(:hash) do + { + test: { script: 'test' }, + production: { + script: 'deploy', + only: { variables: %w[$SOMETHING] } + } + } + end + + it 'does not modify the hash' do + expect(subject.to_hash).to eq hash + end + end + + context 'when hash has a single simple extension' do + let(:hash) do + { + something: { + script: 'deploy', + only: { variables: %w[$SOMETHING] } + }, + + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + } + } + end + + it 'extends a hash with a deep reverse merge' do + expect(subject.to_hash).to eq( + something: { + script: 'deploy', + only: { variables: %w[$SOMETHING] } + }, + + test: { + extends: 'something', + script: 'ls', + only: { + refs: %w[master], + variables: %w[$SOMETHING] + } + } + ) + end + end + + context 'when a hash uses recursive extensions' do + let(:hash) do + { + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + }, + + build: { + extends: 'something', + stage: 'build' + }, + + deploy: { + stage: 'deploy', + extends: '.first' + }, + + something: { + extends: '.first', + script: 'exec', + only: { variables: %w[$SOMETHING] } + }, + + '.first': { + script: 'run', + only: { kubernetes: 'active' } + } + } + end + + it 'extends a hash with a deep reverse merge' do + expect(subject.to_hash).to eq( + '.first': { + script: 'run', + only: { kubernetes: 'active' } + }, + + something: { + extends: '.first', + script: 'exec', + only: { + kubernetes: 'active', + variables: %w[$SOMETHING] + } + }, + + deploy: { + script: 'run', + stage: 'deploy', + only: { kubernetes: 'active' }, + extends: '.first' + }, + + build: { + extends: 'something', + script: 'exec', + stage: 'build', + only: { + kubernetes: 'active', + variables: %w[$SOMETHING] + } + }, + + test: { + extends: 'something', + script: 'ls', + only: { + refs: %w[master], + variables: %w[$SOMETHING], + kubernetes: 'active' + } + } + ) + end + end + + context 'when nested circular dependecy has been detected' do + let(:hash) do + { + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + }, + + something: { + extends: '.first', + script: 'deploy', + only: { variables: %w[$SOMETHING] } + }, + + '.first': { + extends: 'something', + script: 'run', + only: { kubernetes: 'active' } + } + } + end + + it 'raises an error about circular dependency' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::CircularDependencyError) + end + end + + context 'when circular dependecy to self has been detected' do + let(:hash) do + { + test: { + extends: 'test', + script: 'ls', + only: { refs: %w[master] } + } + } + end + + it 'raises an error about circular dependency' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::CircularDependencyError) + end + end + + context 'when invalid extends value is specified' do + let(:hash) do + { something: { extends: 1, script: 'ls' } } + end + + it 'raises an error about invalid extension' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::InvalidExtensionError) + end + end + + context 'when extensible entry has non-hash inheritance defined' do + let(:hash) do + { + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + }, + + something: 'some text' + } + end + + it 'raises an error about invalid base' do + expect { subject.to_hash } + .to raise_error(described_class::Entry::InvalidExtensionError) + end + end + end +end diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 2e204da307d..5a78ce783dd 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -1,4 +1,6 @@ -require 'spec_helper' +require 'fast_spec_helper' + +require_dependency 'active_model' describe Gitlab::Ci::Config do let(:config) do @@ -42,6 +44,36 @@ describe Gitlab::Ci::Config do end end + context 'when using extendable hash' do + let(:yml) do + <<-EOS + image: ruby:2.2 + + rspec: + script: rspec + + test: + extends: rspec + image: ruby:alpine + EOS + end + + it 'correctly extends the hash' do + hash = { + image: 'ruby:2.2', + rspec: { script: 'rspec' }, + test: { + extends: 'rspec', + image: 'ruby:alpine', + script: 'rspec' + } + } + + expect(config).to be_valid + expect(config.to_hash).to eq hash + end + end + context 'when config is invalid' do context 'when yml is incorrect' do let(:yml) { '// invalid' } @@ -49,7 +81,7 @@ describe Gitlab::Ci::Config do describe '.new' do it 'raises error' do expect { config }.to raise_error( - ::Gitlab::Ci::Config::Loader::FormatError, + described_class::ConfigError, /Invalid configuration format/ ) end @@ -75,5 +107,21 @@ describe Gitlab::Ci::Config do end end end + + context 'when invalid extended hash has been provided' do + let(:yml) do + <<-EOS + test: + extends: test + script: rspec + EOS + end + + it 'raises an error' do + expect { config }.to raise_error( + described_class::ConfigError, /circular dependency detected/ + ) + end + end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index e73cdc54a15..fcbdf71a4e9 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -562,6 +562,58 @@ module Gitlab end end + context 'when using `extends`' do + let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) } + + subject { config_processor.builds.first } + + context 'when using simple `extends`' do + let(:config) do + <<~YAML + .template: + script: test + + rspec: + extends: .template + image: ruby:alpine + YAML + end + + it 'correctly extends rspec job' do + expect(config_processor.builds).to be_one + expect(subject.dig(:commands)).to eq 'test' + expect(subject.dig(:options, :image, :name)).to eq 'ruby:alpine' + end + end + + context 'when using recursive `extends`' do + let(:config) do + <<~YAML + rspec: + extends: .test + script: rspec + when: always + + .template: + before_script: + - bundle install + + .test: + extends: .template + script: test + image: image:test + YAML + end + + it 'correctly extends rspec job' do + expect(config_processor.builds).to be_one + expect(subject.dig(:commands)).to eq "bundle install\nrspec" + expect(subject.dig(:options, :image, :name)).to eq 'image:test' + expect(subject.dig(:when)).to eq 'always' + end + end + end + describe "When" do %w(on_success on_failure always).each do |when_state| it "returns #{when_state} when defined" do @@ -1309,6 +1361,14 @@ module Gitlab .to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:rspec:only variables invalid expression syntax') end + + it 'returns errors if extended hash configuration is invalid' do + config = YAML.dump({ rspec: { extends: 'something', script: 'test' } }) + + expect { Gitlab::Ci::YamlProcessor.new(config) } + .to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, + 'rspec: unknown key in `extends`') + end end describe "Validate configuration templates" do diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 0a1e3eb83d3..579f175c4a8 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -416,6 +416,7 @@ ProjectHook: - type - service_id - push_events +- push_events_branch_filter - issues_events - merge_requests_events - tag_push_events diff --git a/spec/lib/gitlab/kubernetes/cluster_role_binding_spec.rb b/spec/lib/gitlab/kubernetes/cluster_role_binding_spec.rb new file mode 100644 index 00000000000..4a669408025 --- /dev/null +++ b/spec/lib/gitlab/kubernetes/cluster_role_binding_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Kubernetes::ClusterRoleBinding do + let(:cluster_role_binding) { described_class.new(name, cluster_role_name, subjects) } + let(:name) { 'cluster-role-binding-name' } + let(:cluster_role_name) { 'cluster-admin' } + + let(:subjects) { [{ kind: 'ServiceAccount', name: 'sa', namespace: 'ns' }] } + + describe '#generate' do + let(:role_ref) do + { + apiGroup: 'rbac.authorization.k8s.io', + kind: 'ClusterRole', + name: cluster_role_name + } + end + + let(:resource) do + ::Kubeclient::Resource.new( + metadata: { name: name }, + roleRef: role_ref, + subjects: subjects + ) + end + + subject { cluster_role_binding.generate } + + it 'should build a Kubeclient Resource' do + is_expected.to eq(resource) + end + end +end diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb index 341f71a3e49..25c3b37753d 100644 --- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb @@ -5,9 +5,18 @@ describe Gitlab::Kubernetes::Helm::Api do let(:helm) { described_class.new(client) } let(:gitlab_namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } let(:namespace) { Gitlab::Kubernetes::Namespace.new(gitlab_namespace, client) } - let(:application) { create(:clusters_applications_prometheus) } - - let(:command) { application.install_command } + let(:application_name) { 'app-name' } + let(:rbac) { false } + let(:files) { {} } + + let(:command) do + Gitlab::Kubernetes::Helm::InstallCommand.new( + name: application_name, + chart: 'chart-name', + rbac: rbac, + files: files + ) + end subject { helm } @@ -28,6 +37,8 @@ describe Gitlab::Kubernetes::Helm::Api do before do allow(client).to receive(:create_pod).and_return(nil) allow(client).to receive(:create_config_map).and_return(nil) + allow(client).to receive(:create_service_account).and_return(nil) + allow(client).to receive(:create_cluster_role_binding).and_return(nil) allow(namespace).to receive(:ensure_exists!).once end @@ -39,7 +50,7 @@ describe Gitlab::Kubernetes::Helm::Api do end context 'with a ConfigMap' do - let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.files).generate } + let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application_name, files).generate } it 'creates a ConfigMap on kubeclient' do expect(client).to receive(:create_config_map).with(resource).once @@ -47,6 +58,96 @@ describe Gitlab::Kubernetes::Helm::Api do subject.install(command) end end + + context 'without a service account' do + it 'does not create a service account on kubeclient' do + expect(client).not_to receive(:create_service_account) + expect(client).not_to receive(:create_cluster_role_binding) + + subject.install(command) + end + end + + context 'with a service account' do + let(:command) { Gitlab::Kubernetes::Helm::InitCommand.new(name: application_name, files: files, rbac: rbac) } + + context 'rbac-enabled cluster' do + let(:rbac) { true } + + let(:service_account_resource) do + Kubeclient::Resource.new(metadata: { name: 'tiller', namespace: 'gitlab-managed-apps' }) + end + + let(:cluster_role_binding_resource) do + Kubeclient::Resource.new( + metadata: { name: 'tiller-admin' }, + roleRef: { apiGroup: 'rbac.authorization.k8s.io', kind: 'ClusterRole', name: 'cluster-admin' }, + subjects: [{ kind: 'ServiceAccount', name: 'tiller', namespace: 'gitlab-managed-apps' }] + ) + end + + context 'service account and cluster role binding does not exist' do + before do + expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) + expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) + end + + it 'creates a service account, followed the cluster role binding on kubeclient' do + expect(client).to receive(:create_service_account).with(service_account_resource).once.ordered + expect(client).to receive(:create_cluster_role_binding).with(cluster_role_binding_resource).once.ordered + + subject.install(command) + end + end + + context 'service account already exists' do + before do + expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_return(service_account_resource) + expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_raise(Kubeclient::HttpError.new(404, 'Not found', nil)) + end + + it 'updates the service account, followed by creating the cluster role binding' do + expect(client).to receive(:update_service_account).with(service_account_resource).once.ordered + expect(client).to receive(:create_cluster_role_binding).with(cluster_role_binding_resource).once.ordered + + subject.install(command) + end + end + + context 'service account and cluster role binding already exists' do + before do + expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_return(service_account_resource) + expect(client).to receive('get_cluster_role_binding').with('tiller-admin').and_return(cluster_role_binding_resource) + end + + it 'updates the service account, followed by creating the cluster role binding' do + expect(client).to receive(:update_service_account).with(service_account_resource).once.ordered + expect(client).to receive(:update_cluster_role_binding).with(cluster_role_binding_resource).once.ordered + + subject.install(command) + end + end + + context 'a non-404 error is thrown' do + before do + expect(client).to receive('get_service_account').with('tiller', 'gitlab-managed-apps').and_raise(Kubeclient::HttpError.new(401, 'Unauthorized', nil)) + end + + it 'raises an error' do + expect { subject.install(command) }.to raise_error(Kubeclient::HttpError) + end + end + end + + context 'legacy abac cluster' do + it 'does not create a service account on kubeclient' do + expect(client).not_to receive(:create_service_account) + expect(client).not_to receive(:create_cluster_role_binding) + + subject.install(command) + end + end + end end describe '#status' do diff --git a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb index d50616e95e8..aacae78be43 100644 --- a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb @@ -2,14 +2,24 @@ require 'spec_helper' describe Gitlab::Kubernetes::Helm::BaseCommand do let(:application) { create(:clusters_applications_helm) } + let(:rbac) { false } + let(:test_class) do Class.new do include Gitlab::Kubernetes::Helm::BaseCommand + def initialize(rbac) + @rbac = rbac + end + def name "test-class-name" end + def rbac? + @rbac + end + def files { some: 'value' @@ -19,7 +29,7 @@ describe Gitlab::Kubernetes::Helm::BaseCommand do end let(:base_command) do - test_class.new + test_class.new(rbac) end subject { base_command } @@ -34,6 +44,14 @@ describe Gitlab::Kubernetes::Helm::BaseCommand do it 'should returns a kubeclient resoure with pod content for application' do is_expected.to be_an_instance_of ::Kubeclient::Resource end + + context 'when rbac is true' do + let(:rbac) { true } + + it 'also returns a kubeclient resource' do + is_expected.to be_an_instance_of ::Kubeclient::Resource + end + end end describe '#pod_name' do diff --git a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb index dcbc046cf00..72dc1817936 100644 --- a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb @@ -2,9 +2,135 @@ require 'spec_helper' describe Gitlab::Kubernetes::Helm::InitCommand do let(:application) { create(:clusters_applications_helm) } - let(:commands) { 'helm init --tiller-tls --tiller-tls-verify --tls-ca-cert /data/helm/helm/config/ca.pem --tiller-tls-cert /data/helm/helm/config/cert.pem --tiller-tls-key /data/helm/helm/config/key.pem >/dev/null' } + let(:rbac) { false } + let(:files) { {} } + let(:init_command) { described_class.new(name: application.name, files: files, rbac: rbac) } - subject { described_class.new(name: application.name, files: {}) } + let(:commands) do + <<~EOS + helm init --tiller-tls --tiller-tls-verify --tls-ca-cert /data/helm/helm/config/ca.pem --tiller-tls-cert /data/helm/helm/config/cert.pem --tiller-tls-key /data/helm/helm/config/key.pem >/dev/null + EOS + end + + subject { init_command } it_behaves_like 'helm commands' + + context 'on a rbac-enabled cluster' do + let(:rbac) { true } + + it_behaves_like 'helm commands' do + let(:commands) do + <<~EOS + helm init --tiller-tls --tiller-tls-verify --tls-ca-cert /data/helm/helm/config/ca.pem --tiller-tls-cert /data/helm/helm/config/cert.pem --tiller-tls-key /data/helm/helm/config/key.pem --service-account tiller >/dev/null + EOS + end + end + end + + describe '#rbac?' do + subject { init_command.rbac? } + + context 'rbac is enabled' do + let(:rbac) { true } + + it { is_expected.to be_truthy } + end + + context 'rbac is not enabled' do + let(:rbac) { false } + + it { is_expected.to be_falsey } + end + end + + describe '#config_map_resource' do + let(:metadata) do + { + name: 'values-content-configuration-helm', + namespace: 'gitlab-managed-apps', + labels: { name: 'values-content-configuration-helm' } + } + end + + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: files) } + + subject { init_command.config_map_resource } + + it 'returns a KubeClient resource with config map content for the application' do + is_expected.to eq(resource) + end + end + + describe '#pod_resource' do + subject { init_command.pod_resource } + + context 'rbac is enabled' do + let(:rbac) { true } + + it 'generates a pod that uses the tiller serviceAccountName' do + expect(subject.spec.serviceAccountName).to eq('tiller') + end + end + + context 'rbac is not enabled' do + let(:rbac) { false } + + it 'generates a pod that uses the default serviceAccountName' do + expect(subject.spec.serviceAcccountName).to be_nil + end + end + end + + describe '#service_account_resource' do + let(:resource) do + Kubeclient::Resource.new(metadata: { name: 'tiller', namespace: 'gitlab-managed-apps' }) + end + + subject { init_command.service_account_resource } + + context 'rbac is enabled' do + let(:rbac) { true } + + it 'generates a Kubeclient resource for the tiller ServiceAccount' do + is_expected.to eq(resource) + end + end + + context 'rbac is not enabled' do + let(:rbac) { false } + + it 'generates nothing' do + is_expected.to be_nil + end + end + end + + describe '#cluster_role_binding_resource' do + let(:resource) do + Kubeclient::Resource.new( + metadata: { name: 'tiller-admin' }, + roleRef: { apiGroup: 'rbac.authorization.k8s.io', kind: 'ClusterRole', name: 'cluster-admin' }, + subjects: [{ kind: 'ServiceAccount', name: 'tiller', namespace: 'gitlab-managed-apps' }] + ) + end + + subject { init_command.cluster_role_binding_resource } + + context 'rbac is enabled' do + let(:rbac) { true } + + it 'generates a Kubeclient resource for the ClusterRoleBinding for tiller' do + is_expected.to eq(resource) + end + end + + context 'rbac is not enabled' do + let(:rbac) { false } + + it 'generates nothing' do + is_expected.to be_nil + end + end + end end diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 982e2f41043..f28941ce58f 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -3,14 +3,17 @@ require 'rails_helper' describe Gitlab::Kubernetes::Helm::InstallCommand do let(:files) { { 'ca.pem': 'some file content' } } let(:repository) { 'https://repository.example.com' } + let(:rbac) { false } let(:version) { '1.2.3' } let(:install_command) do described_class.new( name: 'app-name', chart: 'chart-name', + rbac: rbac, files: files, - version: version, repository: repository + version: version, + repository: repository ) end @@ -21,19 +24,76 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do <<~EOS helm init --client-only >/dev/null helm repo add app-name https://repository.example.com - helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + #{helm_install_comand} + EOS + end + + let(:helm_install_comand) do + <<~EOS.squish + helm install chart-name + --name app-name + --tls + --tls-ca-cert /data/helm/app-name/config/ca.pem + --tls-cert /data/helm/app-name/config/cert.pem + --tls-key /data/helm/app-name/config/key.pem + --version 1.2.3 + --namespace gitlab-managed-apps + -f /data/helm/app-name/config/values.yaml >/dev/null EOS end end + context 'when rbac is true' do + let(:rbac) { true } + + it_behaves_like 'helm commands' do + let(:commands) do + <<~EOS + helm init --client-only >/dev/null + helm repo add app-name https://repository.example.com + #{helm_install_command} + EOS + end + + let(:helm_install_command) do + <<~EOS.squish + helm install chart-name + --name app-name + --tls + --tls-ca-cert /data/helm/app-name/config/ca.pem + --tls-cert /data/helm/app-name/config/cert.pem + --tls-key /data/helm/app-name/config/key.pem + --version 1.2.3 + --set rbac.create\\=true,rbac.enabled\\=true + --namespace gitlab-managed-apps + -f /data/helm/app-name/config/values.yaml >/dev/null + EOS + end + end + end + context 'when there is no repository' do let(:repository) { nil } it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only >/dev/null - helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm init --client-only >/dev/null + #{helm_install_command} + EOS + end + + let(:helm_install_command) do + <<~EOS.squish + helm install chart-name + --name app-name + --tls + --tls-ca-cert /data/helm/app-name/config/ca.pem + --tls-cert /data/helm/app-name/config/cert.pem + --tls-key /data/helm/app-name/config/key.pem + --version 1.2.3 + --namespace gitlab-managed-apps + -f /data/helm/app-name/config/values.yaml >/dev/null EOS end end @@ -45,9 +105,19 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only >/dev/null - helm repo add app-name https://repository.example.com - helm install chart-name --name app-name --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm init --client-only >/dev/null + helm repo add app-name https://repository.example.com + #{helm_install_command} + EOS + end + + let(:helm_install_command) do + <<~EOS.squish + helm install chart-name + --name app-name + --version 1.2.3 + --namespace gitlab-managed-apps + -f /data/helm/app-name/config/values.yaml >/dev/null EOS end end @@ -59,14 +129,63 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do it_behaves_like 'helm commands' do let(:commands) do <<~EOS - helm init --client-only >/dev/null - helm repo add app-name https://repository.example.com - helm install chart-name --name app-name --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm init --client-only >/dev/null + helm repo add app-name https://repository.example.com + #{helm_install_command} + EOS + end + + let(:helm_install_command) do + <<~EOS.squish + helm install chart-name + --name app-name + --tls + --tls-ca-cert /data/helm/app-name/config/ca.pem + --tls-cert /data/helm/app-name/config/cert.pem + --tls-key /data/helm/app-name/config/key.pem + --namespace gitlab-managed-apps + -f /data/helm/app-name/config/values.yaml >/dev/null EOS end end end + describe '#rbac?' do + subject { install_command.rbac? } + + context 'rbac is enabled' do + let(:rbac) { true } + + it { is_expected.to be_truthy } + end + + context 'rbac is not enabled' do + let(:rbac) { false } + + it { is_expected.to be_falsey } + end + end + + describe '#pod_resource' do + subject { install_command.pod_resource } + + context 'rbac is enabled' do + let(:rbac) { true } + + it 'generates a pod that uses the tiller serviceAccountName' do + expect(subject.spec.serviceAccountName).to eq('tiller') + end + end + + context 'rbac is not enabled' do + let(:rbac) { false } + + it 'generates a pod that uses the default serviceAccountName' do + expect(subject.spec.serviceAcccountName).to be_nil + end + end + end + describe '#config_map_resource' do let(:metadata) do { @@ -84,4 +203,20 @@ describe Gitlab::Kubernetes::Helm::InstallCommand do is_expected.to eq(resource) end end + + describe '#service_account_resource' do + subject { install_command.service_account_resource } + + it 'returns nothing' do + is_expected.to be_nil + end + end + + describe '#cluster_role_binding_resource' do + subject { install_command.cluster_role_binding_resource } + + it 'returns nothing' do + is_expected.to be_nil + end + end end diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb index ec64193c0b2..b333b334f36 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -5,8 +5,9 @@ describe Gitlab::Kubernetes::Helm::Pod do let(:app) { create(:clusters_applications_prometheus) } let(:command) { app.install_command } let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } + let(:service_account_name) { nil } - subject { described_class.new(command, namespace) } + subject { described_class.new(command, namespace, service_account_name: service_account_name) } context 'with a command' do it 'should generate a Kubeclient::Resource' do @@ -58,6 +59,20 @@ describe Gitlab::Kubernetes::Helm::Pod do expect(volume.configMap['items'].first['key']).to eq(:'values.yaml') expect(volume.configMap['items'].first['path']).to eq(:'values.yaml') end + + it 'should have no serviceAccountName' do + spec = subject.generate.spec + expect(spec.serviceAccountName).to be_nil + end + + context 'with a service_account_name' do + let(:service_account_name) { 'sa' } + + it 'should use the serviceAccountName provided' do + spec = subject.generate.spec + expect(spec.serviceAccountName).to eq(service_account_name) + end + end end end end diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb new file mode 100644 index 00000000000..9146729d139 --- /dev/null +++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb @@ -0,0 +1,247 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Kubernetes::KubeClient do + include KubernetesHelpers + + let(:api_url) { 'https://kubernetes.example.com/prefix' } + let(:api_groups) { ['api', 'apis/rbac.authorization.k8s.io'] } + let(:api_version) { 'v1' } + let(:kubeclient_options) { { auth_options: { bearer_token: 'xyz' } } } + + let(:client) { described_class.new(api_url, api_groups, api_version, kubeclient_options) } + + before do + stub_kubeclient_discover(api_url) + end + + describe '#hashed_clients' do + subject { client.hashed_clients } + + it 'has keys from api groups' do + expect(subject.keys).to match_array api_groups + end + + it 'has values of Kubeclient::Client' do + expect(subject.values).to all(be_an_instance_of Kubeclient::Client) + end + end + + describe '#clients' do + subject { client.clients } + + it 'is not empty' do + is_expected.to be_present + end + + it 'is an array of Kubeclient::Client objects' do + is_expected.to all(be_an_instance_of Kubeclient::Client) + end + + it 'has each API group url' do + expected_urls = api_groups.map { |group| "#{api_url}/#{group}" } + + expect(subject.map(&:api_endpoint).map(&:to_s)).to match_array(expected_urls) + end + + it 'has the kubeclient options' do + subject.each do |client| + expect(client.auth_options).to eq({ bearer_token: 'xyz' }) + end + end + + it 'has the api_version' do + subject.each do |client| + expect(client.instance_variable_get(:@api_version)).to eq('v1') + end + end + end + + describe '#core_client' do + subject { client.core_client } + + it 'is a Kubeclient::Client' do + is_expected.to be_an_instance_of Kubeclient::Client + end + + it 'has the core API endpoint' do + expect(subject.api_endpoint.to_s).to match(%r{\/api\Z}) + end + end + + describe '#rbac_client' do + subject { client.rbac_client } + + it 'is a Kubeclient::Client' do + is_expected.to be_an_instance_of Kubeclient::Client + end + + it 'has the RBAC API group endpoint' do + expect(subject.api_endpoint.to_s).to match(%r{\/apis\/rbac.authorization.k8s.io\Z}) + end + end + + describe '#extensions_client' do + subject { client.extensions_client } + + let(:api_groups) { ['apis/extensions'] } + + it 'is a Kubeclient::Client' do + is_expected.to be_an_instance_of Kubeclient::Client + end + + it 'has the extensions API group endpoint' do + expect(subject.api_endpoint.to_s).to match(%r{\/apis\/extensions\Z}) + end + end + + describe '#discover!' do + it 'makes a discovery request for each API group' do + client.discover! + + api_groups.each do |api_group| + discovery_url = api_url + '/' + api_group + '/v1' + expect(WebMock).to have_requested(:get, discovery_url).once + end + end + end + + describe 'core API' do + let(:core_client) { client.core_client } + + [ + :get_pods, + :get_secrets, + :get_config_map, + :get_pod, + :get_namespace, + :get_service, + :get_service_account, + :delete_pod, + :create_config_map, + :create_namespace, + :create_pod, + :create_service_account, + :update_config_map, + :update_service_account + ].each do |method| + describe "##{method}" do + it 'delegates to the core client' do + expect(client).to delegate_method(method).to(:core_client) + end + + it 'responds to the method' do + expect(client).to respond_to method + end + end + end + end + + describe 'rbac API group' do + let(:rbac_client) { client.rbac_client } + + [ + :create_cluster_role_binding, + :get_cluster_role_binding, + :update_cluster_role_binding + ].each do |method| + describe "##{method}" do + it 'delegates to the rbac client' do + expect(client).to delegate_method(method).to(:rbac_client) + end + + it 'responds to the method' do + expect(client).to respond_to method + end + + context 'no rbac client' do + let(:api_groups) { ['api'] } + + it 'throws an error' do + expect { client.public_send(method) }.to raise_error(Module::DelegationError) + end + end + end + end + end + + describe 'extensions API group' do + let(:api_groups) { ['apis/extensions'] } + let(:api_version) { 'v1beta1' } + let(:extensions_client) { client.extensions_client } + + describe '#get_deployments' do + it 'delegates to the extensions client' do + expect(client).to delegate_method(:get_deployments).to(:extensions_client) + end + + it 'responds to the method' do + expect(client).to respond_to :get_deployments + end + + context 'no extensions client' do + let(:api_groups) { ['api'] } + let(:api_version) { 'v1' } + + it 'throws an error' do + expect { client.get_deployments }.to raise_error(Module::DelegationError) + end + end + end + end + + describe 'non-entity methods' do + it 'does not proxy for non-entity methods' do + expect(client.clients.first).to respond_to :proxy_url + + expect(client).not_to respond_to :proxy_url + end + + it 'throws an error' do + expect { client.proxy_url }.to raise_error(NoMethodError) + end + end + + describe '#get_pod_log' do + let(:core_client) { client.core_client } + + it 'is delegated to the core client' do + expect(client).to delegate_method(:get_pod_log).to(:core_client) + end + + context 'when no core client' do + let(:api_groups) { ['apis/extensions'] } + + it 'throws an error' do + expect { client.get_pod_log('pod-name') }.to raise_error(Module::DelegationError) + end + end + end + + describe '#watch_pod_log' do + let(:core_client) { client.core_client } + + it 'is delegated to the core client' do + expect(client).to delegate_method(:watch_pod_log).to(:core_client) + end + + context 'when no core client' do + let(:api_groups) { ['apis/extensions'] } + + it 'throws an error' do + expect { client.watch_pod_log('pod-name') }.to raise_error(Module::DelegationError) + end + end + end + + describe 'methods that do not exist on any client' do + it 'throws an error' do + expect { client.non_existent_method }.to raise_error(NoMethodError) + end + + it 'returns false for respond_to' do + expect(client.respond_to?(:non_existent_method)).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/kubernetes/service_account_spec.rb b/spec/lib/gitlab/kubernetes/service_account_spec.rb new file mode 100644 index 00000000000..8da9e932dc3 --- /dev/null +++ b/spec/lib/gitlab/kubernetes/service_account_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Kubernetes::ServiceAccount do + let(:name) { 'a_service_account' } + let(:namespace_name) { 'a_namespace' } + let(:service_account) { described_class.new(name, namespace_name) } + + it { expect(service_account.name).to eq(name) } + it { expect(service_account.namespace_name).to eq(namespace_name) } + + describe '#generate' do + let(:resource) do + ::Kubeclient::Resource.new(metadata: { name: name, namespace: namespace_name }) + end + + subject { service_account.generate } + + it 'should build a Kubeclient Resource' do + is_expected.to eq(resource) + end + end +end diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index 4e7bd433a9c..ee6d6fc961f 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -42,6 +42,65 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do subscriber.sql(event) end + + context 'events are internal to Rails or irrelevant' do + let(:schema_event) do + double( + :event, + name: 'sql.active_record', + payload: { + sql: "SELECT attr.attname FROM pg_attribute attr INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = any(cons.conkey) WHERE cons.contype = 'p' AND cons.conrelid = '\"projects\"'::regclass", + name: 'SCHEMA', + connection_id: 135, + statement_name: nil, + binds: [] + }, + duration: 0.7 + ) + end + + let(:begin_event) do + double( + :event, + name: 'sql.active_record', + payload: { + sql: "BEGIN", + name: nil, + connection_id: 231, + statement_name: nil, + binds: [] + }, + duration: 1.1 + ) + end + + let(:commit_event) do + double( + :event, + name: 'sql.active_record', + payload: { + sql: "COMMIT", + name: nil, + connection_id: 212, + statement_name: nil, + binds: [] + }, + duration: 1.6 + ) + end + + it 'skips schema/begin/commit sql commands' do + expect(subscriber).to receive(:current_transaction) + .at_least(:once) + .and_return(transaction) + + expect(transaction).not_to receive(:increment) + + subscriber.sql(schema_event) + subscriber.sql(begin_event) + subscriber.sql(commit_event) + end + end end end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index de6dd2a9fea..1ec1fe10744 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -166,4 +166,20 @@ describe Gitlab::UsageData do expect(subject[:recorded_at]).to be_a(Time) end end + + describe '#count' do + let(:relation) { double(:relation) } + + it 'returns the count when counting succeeds' do + allow(relation).to receive(:count).and_return(1) + + expect(described_class.count(relation)).to eq(1) + end + + it 'returns the fallback value when counting fails' do + allow(relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid.new('')) + + expect(described_class.count(relation, fallback: 15)).to eq(15) + end + end end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index e5b2bdc8a4e..2c37cd20ecc 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -47,5 +47,19 @@ describe Clusters::Applications::Helm do cert = OpenSSL::X509::Certificate.new(subject.files[:'cert.pem']) expect(cert.not_after).to be > 999.years.from_now end + + describe 'rbac' do + context 'non rbac cluster' do + it { expect(subject).not_to be_rbac } + end + + context 'rbac cluster' do + before do + helm.cluster.platform_kubernetes.rbac! + end + + it { expect(subject).to be_rbac } + end + end end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index 21f75ced8c3..c55953c8d22 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -88,9 +88,18 @@ describe Clusters::Applications::Ingress do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') expect(subject.version).to eq('0.23.0') + expect(subject).not_to be_rbac expect(subject.files).to eq(ingress.files) end + context 'on a rbac enabled cluster' do + before do + ingress.cluster.platform_kubernetes.rbac! + end + + it { is_expected.to be_rbac } + end + context 'application failed to install previously' do let(:ingress) { create(:clusters_applications_ingress, :errored, version: 'nginx') } diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index 027b732681b..591a01d78a9 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -51,10 +51,19 @@ describe Clusters::Applications::Jupyter do expect(subject.name).to eq('jupyter') expect(subject.chart).to eq('jupyter/jupyterhub') expect(subject.version).to eq('v0.6') + expect(subject).not_to be_rbac expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') expect(subject.files).to eq(jupyter.files) end + context 'on a rbac enabled cluster' do + before do + jupyter.cluster.platform_kubernetes.rbac! + end + + it { is_expected.to be_rbac } + end + context 'application failed to install previously' do let(:jupyter) { create(:clusters_applications_jupyter, :errored, version: '0.0.1') } diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 26b75c75e1d..f34b4ece8db 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe Clusters::Applications::Prometheus do + include KubernetesHelpers + include_examples 'cluster application core specs', :clusters_applications_prometheus include_examples 'cluster application status specs', :cluster_application_prometheus @@ -107,26 +109,14 @@ describe Clusters::Applications::Prometheus do end context 'cluster has kubeclient' do - let(:kubernetes_url) { 'http://example.com' } - let(:k8s_discover_response) do - { - resources: [ - { - name: 'service', - kind: 'Service' - } - ] - } - end - - let(:kube_client) { Kubeclient::Client.new(kubernetes_url) } + let(:kubernetes_url) { subject.cluster.platform_kubernetes.api_url } + let(:kube_client) { subject.cluster.kubeclient.core_client } - let(:cluster) { create(:cluster) } - subject { create(:clusters_applications_prometheus, cluster: cluster) } + subject { create(:clusters_applications_prometheus) } before do - allow(kube_client.rest_client).to receive(:get).and_return(k8s_discover_response.to_json) - allow(subject.cluster).to receive(:kubeclient).and_return(kube_client) + subject.cluster.platform_kubernetes.namespace = 'a-namespace' + stub_kubeclient_discover(subject.cluster.platform_kubernetes.api_url) end it 'creates proxy prometheus rest client' do @@ -134,7 +124,7 @@ describe Clusters::Applications::Prometheus do end it 'creates proper url' do - expect(subject.prometheus_client.url).to eq('http://example.com/api/v1/namespaces/gitlab-managed-apps/service/prometheus-prometheus-server:80/proxy') + expect(subject.prometheus_client.url).to eq("#{kubernetes_url}/api/v1/namespaces/gitlab-managed-apps/services/prometheus-prometheus-server:80/proxy") end it 'copies options and headers from kube client to proxy client' do @@ -164,9 +154,18 @@ describe Clusters::Applications::Prometheus do expect(subject.name).to eq('prometheus') expect(subject.chart).to eq('stable/prometheus') expect(subject.version).to eq('6.7.3') + expect(subject).not_to be_rbac expect(subject.files).to eq(prometheus.files) end + context 'on a rbac enabled cluster' do + before do + prometheus.cluster.platform_kubernetes.rbac! + end + + it { is_expected.to be_rbac } + end + context 'application failed to install previously' do let(:prometheus) { create(:clusters_applications_prometheus, :errored, version: '2.0.0') } diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index d84f125e246..eda8d519f60 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -46,10 +46,19 @@ describe Clusters::Applications::Runner do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') expect(subject.version).to eq('0.1.31') + expect(subject).not_to be_rbac expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.files).to eq(gitlab_runner.files) end + context 'on a rbac enabled cluster' do + before do + gitlab_runner.cluster.platform_kubernetes.rbac! + end + + it { is_expected.to be_rbac } + end + context 'application failed to install previously' do let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 6f66515b45f..2727191eb9b 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -13,6 +13,10 @@ describe Clusters::Cluster do it { is_expected.to delegate_method(:status_reason).to(:provider) } it { is_expected.to delegate_method(:status_name).to(:provider) } it { is_expected.to delegate_method(:on_creation?).to(:provider) } + it { is_expected.to delegate_method(:active?).to(:platform_kubernetes).with_prefix } + it { is_expected.to delegate_method(:rbac?).to(:platform_kubernetes).with_prefix } + it { is_expected.to delegate_method(:installed?).to(:application_helm).with_prefix } + it { is_expected.to delegate_method(:installed?).to(:application_ingress).with_prefix } it { is_expected.to respond_to :project } describe '.enabled' do diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index ab7f89f9bf4..66198d5ee2b 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -92,6 +92,30 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching end end + describe '#kubeclient' do + subject { kubernetes.kubeclient } + + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured, namespace: 'a-namespace') } + + it { is_expected.to be_an_instance_of(Gitlab::Kubernetes::KubeClient) } + end + + describe '#rbac?' do + subject { kubernetes.rbac? } + + let(:kubernetes) { build(:cluster_platform_kubernetes, :configured) } + + context 'when authorization type is rbac' do + let(:kubernetes) { build(:cluster_platform_kubernetes, :rbac_enabled, :configured) } + + it { is_expected.to be_truthy } + end + + context 'when authorization type is nil' do + it { is_expected.to be_falsey } + end + end + describe '#actual_namespace' do subject { kubernetes.actual_namespace } diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb index 621d2d38eae..265abd6bd72 100644 --- a/spec/models/concerns/triggerable_hooks_spec.rb +++ b/spec/models/concerns/triggerable_hooks_spec.rb @@ -40,4 +40,28 @@ RSpec.describe TriggerableHooks do end end end + + describe '.select_active' do + it 'returns hooks that match the active filter' do + TestableHook.create!(url: 'http://example1.com', push_events: true) + TestableHook.create!(url: 'http://example2.com', push_events: true) + filter1 = double(:filter1) + filter2 = double(:filter2) + allow(ActiveHookFilter).to receive(:new).exactly(2).times.and_return(filter1, filter2) + expect(filter1).to receive(:matches?).and_return(true) + expect(filter2).to receive(:matches?).and_return(false) + + hooks = TestableHook.push_hooks.order_id_asc + expect(hooks.select_active(:push_hooks, {})).to eq [hooks.first] + end + + it 'returns empty list if no hooks match the active filter' do + TestableHook.create!(url: 'http://example1.com', push_events: true) + filter = double(:filter) + allow(ActiveHookFilter).to receive(:new).and_return(filter) + expect(filter).to receive(:matches?).and_return(false) + + expect(TestableHook.push_hooks.select_active(:push_hooks, {})).to eq [] + end + end end diff --git a/spec/models/hooks/active_hook_filter_spec.rb b/spec/models/hooks/active_hook_filter_spec.rb new file mode 100644 index 00000000000..df7edda2213 --- /dev/null +++ b/spec/models/hooks/active_hook_filter_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +describe ActiveHookFilter do + subject(:filter) { described_class.new(hook) } + + describe '#matches?' do + context 'for push event hooks' do + let(:hook) do + create(:project_hook, push_events: true, push_events_branch_filter: branch_filter) + end + + context 'branch filter is specified' do + let(:branch_filter) { 'master' } + + it 'returns true if branch matches' do + expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true + end + + it 'returns false if branch does not match' do + expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to be false + end + + it 'returns false if ref is nil' do + expect(filter.matches?(:push_hooks, {})).to be false + end + + context 'branch filter contains wildcard' do + let(:branch_filter) { 'features/*' } + + it 'returns true if branch matches' do + expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch' })).to be true + expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch/something' })).to be true + end + + it 'returns false if branch does not match' do + expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false + end + end + end + + context 'branch filter is not specified' do + let(:branch_filter) { nil } + + it 'returns true' do + expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true + end + end + + context 'branch filter is empty string' do + let(:branch_filter) { '' } + + it 'acts like branch is not specified' do + expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true + end + end + end + + context 'for non-push-events hooks' do + let(:hook) do + create(:project_hook, issues_events: true, push_events: false, push_events_branch_filter: '') + end + + it 'returns true as branch filters are not yet supported for these' do + expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to be true + end + end + end +end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index ea6d6e53ef5..a4181631f01 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -35,6 +35,26 @@ describe WebHook do it { is_expected.not_to allow_values("foo\nbar", "foo\r\nbar").for(:token) } end + + describe 'push_events_branch_filter' do + it { is_expected.to allow_values("good_branch_name", "another/good-branch_name").for(:push_events_branch_filter) } + it { is_expected.to allow_values("").for(:push_events_branch_filter) } + it { is_expected.not_to allow_values("bad branch name", "bad~branchname").for(:push_events_branch_filter) } + + it 'gets rid of whitespace' do + hook.push_events_branch_filter = ' branch ' + hook.save + + expect(hook.push_events_branch_filter).to eq('branch') + end + + it 'stores whitespace only as empty' do + hook.push_events_branch_filter = ' ' + hook.save + + expect(hook.push_events_branch_filter).to eq('') + end + end end describe 'execute' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 7cfffbde42f..264632dba4b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3734,21 +3734,45 @@ describe Project do end describe '#execute_hooks' do - it 'executes the projects hooks with the specified scope' do - hook1 = create(:project_hook, merge_requests_events: true, tag_push_events: false) - hook2 = create(:project_hook, merge_requests_events: false, tag_push_events: true) - project = create(:project, hooks: [hook1, hook2]) + let(:data) { { ref: 'refs/heads/master', data: 'data' } } + it 'executes active projects hooks with the specified scope' do + hook = create(:project_hook, merge_requests_events: false, push_events: true) + expect(ProjectHook).to receive(:select_active) + .with(:push_hooks, data) + .and_return([hook]) + project = create(:project, hooks: [hook]) expect_any_instance_of(ProjectHook).to receive(:async_execute).once - project.execute_hooks({}, :tag_push_hooks) + project.execute_hooks(data, :push_hooks) + end + + it 'does not execute project hooks that dont match the specified scope' do + hook = create(:project_hook, merge_requests_events: true, push_events: false) + project = create(:project, hooks: [hook]) + + expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once + + project.execute_hooks(data, :push_hooks) + end + + it 'does not execute project hooks which are not active' do + hook = create(:project_hook, push_events: true) + expect(ProjectHook).to receive(:select_active) + .with(:push_hooks, data) + .and_return([]) + project = create(:project, hooks: [hook]) + + expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once + + project.execute_hooks(data, :push_hooks) end it 'executes the system hooks with the specified scope' do - expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with({ data: 'data' }, :merge_request_hooks) + expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(data, :merge_request_hooks) project = build(:project) - project.execute_hooks({ data: 'data' }, :merge_request_hooks) + project.execute_hooks(data, :merge_request_hooks) end it 'executes the system hooks when inside a transaction' do @@ -3763,7 +3787,7 @@ describe Project do # actually get to the `after_commit` hook that queues these jobs. expect do project.transaction do - project.execute_hooks({ data: 'data' }, :merge_request_hooks) + project.execute_hooks(data, :merge_request_hooks) end end.not_to raise_error # Sidekiq::Worker::EnqueueFromTransactionError end diff --git a/spec/presenters/project_presenter_spec.rb b/spec/presenters/project_presenter_spec.rb index 01085dbcb49..d9fb27e101e 100644 --- a/spec/presenters/project_presenter_spec.rb +++ b/spec/presenters/project_presenter_spec.rb @@ -159,39 +159,76 @@ describe ProjectPresenter do end end + context 'statistics anchors (empty repo)' do + let(:project) { create(:project, :empty_repo) } + let(:presenter) { described_class.new(project, current_user: user) } + + describe '#files_anchor_data' do + it 'returns files data' do + expect(presenter.files_anchor_data).to have_attributes(enabled: true, + label: 'Files (0 Bytes)', + link: nil) + end + end + + describe '#commits_anchor_data' do + it 'returns commits data' do + expect(presenter.commits_anchor_data).to have_attributes(enabled: true, + label: 'Commits (0)', + link: nil) + end + end + + describe '#branches_anchor_data' do + it 'returns branches data' do + expect(presenter.branches_anchor_data).to have_attributes(enabled: true, + label: "Branches (0)", + link: nil) + end + end + + describe '#tags_anchor_data' do + it 'returns tags data' do + expect(presenter.tags_anchor_data).to have_attributes(enabled: true, + label: "Tags (0)", + link: nil) + end + end + end + context 'statistics anchors' do let(:project) { create(:project, :repository) } let(:presenter) { described_class.new(project, current_user: user) } describe '#files_anchor_data' do it 'returns files data' do - expect(presenter.files_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Files (0 Bytes)', - link: presenter.project_tree_path(project))) + expect(presenter.files_anchor_data).to have_attributes(enabled: true, + label: 'Files (0 Bytes)', + link: presenter.project_tree_path(project)) end end describe '#commits_anchor_data' do it 'returns commits data' do - expect(presenter.commits_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Commits (0)', - link: presenter.project_commits_path(project, project.repository.root_ref))) + expect(presenter.commits_anchor_data).to have_attributes(enabled: true, + label: 'Commits (0)', + link: presenter.project_commits_path(project, project.repository.root_ref)) end end describe '#branches_anchor_data' do it 'returns branches data' do - expect(presenter.branches_anchor_data).to eq(OpenStruct.new(enabled: true, - label: "Branches (#{project.repository.branches.size})", - link: presenter.project_branches_path(project))) + expect(presenter.branches_anchor_data).to have_attributes(enabled: true, + label: "Branches (#{project.repository.branches.size})", + link: presenter.project_branches_path(project)) end end describe '#tags_anchor_data' do it 'returns tags data' do - expect(presenter.tags_anchor_data).to eq(OpenStruct.new(enabled: true, - label: "Tags (#{project.repository.tags.size})", - link: presenter.project_tags_path(project))) + expect(presenter.tags_anchor_data).to have_attributes(enabled: true, + label: "Tags (#{project.repository.tags.size})", + link: presenter.project_tags_path(project)) end end @@ -199,10 +236,10 @@ describe ProjectPresenter do it 'returns new file data if user can push' do project.add_developer(user) - expect(presenter.new_file_anchor_data).to eq(OpenStruct.new(enabled: false, - label: "New file", - link: presenter.project_new_blob_path(project, 'master'), - class_modifier: 'new')) + expect(presenter.new_file_anchor_data).to have_attributes(enabled: false, + label: "New file", + link: presenter.project_new_blob_path(project, 'master'), + class_modifier: 'new') end it 'returns nil if user cannot push' do @@ -227,9 +264,9 @@ describe ProjectPresenter do project.add_developer(user) allow(project.repository).to receive(:readme).and_return(nil) - expect(presenter.readme_anchor_data).to eq(OpenStruct.new(enabled: false, - label: 'Add Readme', - link: presenter.add_readme_path)) + expect(presenter.readme_anchor_data).to have_attributes(enabled: false, + label: 'Add Readme', + link: presenter.add_readme_path) end end @@ -237,9 +274,9 @@ describe ProjectPresenter do it 'returns anchor data' do allow(project.repository).to receive(:readme).and_return(double(name: 'readme')) - expect(presenter.readme_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Readme', - link: presenter.readme_path)) + expect(presenter.readme_anchor_data).to have_attributes(enabled: true, + label: 'Readme', + link: presenter.readme_path) end end end @@ -250,9 +287,9 @@ describe ProjectPresenter do project.add_developer(user) allow(project.repository).to receive(:changelog).and_return(nil) - expect(presenter.changelog_anchor_data).to eq(OpenStruct.new(enabled: false, - label: 'Add Changelog', - link: presenter.add_changelog_path)) + expect(presenter.changelog_anchor_data).to have_attributes(enabled: false, + label: 'Add Changelog', + link: presenter.add_changelog_path) end end @@ -260,9 +297,9 @@ describe ProjectPresenter do it 'returns anchor data' do allow(project.repository).to receive(:changelog).and_return(double(name: 'foo')) - expect(presenter.changelog_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Changelog', - link: presenter.changelog_path)) + expect(presenter.changelog_anchor_data).to have_attributes(enabled: true, + label: 'Changelog', + link: presenter.changelog_path) end end end @@ -273,9 +310,9 @@ describe ProjectPresenter do project.add_developer(user) allow(project.repository).to receive(:license_blob).and_return(nil) - expect(presenter.license_anchor_data).to eq(OpenStruct.new(enabled: false, - label: 'Add License', - link: presenter.add_license_path)) + expect(presenter.license_anchor_data).to have_attributes(enabled: false, + label: 'Add license', + link: presenter.add_license_path) end end @@ -283,9 +320,9 @@ describe ProjectPresenter do it 'returns anchor data' do allow(project.repository).to receive(:license_blob).and_return(double(name: 'foo')) - expect(presenter.license_anchor_data).to eq(OpenStruct.new(enabled: true, - label: presenter.license_short_name, - link: presenter.license_path)) + expect(presenter.license_anchor_data).to have_attributes(enabled: true, + label: presenter.license_short_name, + link: presenter.license_path) end end end @@ -296,9 +333,9 @@ describe ProjectPresenter do project.add_developer(user) allow(project.repository).to receive(:contribution_guide).and_return(nil) - expect(presenter.contribution_guide_anchor_data).to eq(OpenStruct.new(enabled: false, - label: 'Add Contribution guide', - link: presenter.add_contribution_guide_path)) + expect(presenter.contribution_guide_anchor_data).to have_attributes(enabled: false, + label: 'Add Contribution guide', + link: presenter.add_contribution_guide_path) end end @@ -306,9 +343,9 @@ describe ProjectPresenter do it 'returns anchor data' do allow(project.repository).to receive(:contribution_guide).and_return(double(name: 'foo')) - expect(presenter.contribution_guide_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Contribution guide', - link: presenter.contribution_guide_path)) + expect(presenter.contribution_guide_anchor_data).to have_attributes(enabled: true, + label: 'Contribution guide', + link: presenter.contribution_guide_path) end end end @@ -318,9 +355,9 @@ describe ProjectPresenter do it 'returns anchor data' do allow(project).to receive(:auto_devops_enabled?).and_return(true) - expect(presenter.autodevops_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Auto DevOps enabled', - link: nil)) + expect(presenter.autodevops_anchor_data).to have_attributes(enabled: true, + label: 'Auto DevOps enabled', + link: nil) end end @@ -330,9 +367,9 @@ describe ProjectPresenter do allow(project).to receive(:auto_devops_enabled?).and_return(false) allow(project.repository).to receive(:gitlab_ci_yml).and_return(nil) - expect(presenter.autodevops_anchor_data).to eq(OpenStruct.new(enabled: false, - label: 'Enable Auto DevOps', - link: presenter.project_settings_ci_cd_path(project, anchor: 'autodevops-settings'))) + expect(presenter.autodevops_anchor_data).to have_attributes(enabled: false, + label: 'Enable Auto DevOps', + link: presenter.project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) end end end @@ -343,9 +380,9 @@ describe ProjectPresenter do project.add_maintainer(user) cluster = create(:cluster, projects: [project]) - expect(presenter.kubernetes_cluster_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Kubernetes configured', - link: presenter.project_cluster_path(project, cluster))) + expect(presenter.kubernetes_cluster_anchor_data).to have_attributes(enabled: true, + label: 'Kubernetes configured', + link: presenter.project_cluster_path(project, cluster)) end it 'returns link to clusters page if more than one exists' do @@ -353,17 +390,17 @@ describe ProjectPresenter do create(:cluster, :production_environment, projects: [project]) create(:cluster, projects: [project]) - expect(presenter.kubernetes_cluster_anchor_data).to eq(OpenStruct.new(enabled: true, - label: 'Kubernetes configured', - link: presenter.project_clusters_path(project))) + expect(presenter.kubernetes_cluster_anchor_data).to have_attributes(enabled: true, + label: 'Kubernetes configured', + link: presenter.project_clusters_path(project)) end it 'returns link to create a cluster if no cluster exists' do project.add_maintainer(user) - expect(presenter.kubernetes_cluster_anchor_data).to eq(OpenStruct.new(enabled: false, - label: 'Add Kubernetes cluster', - link: presenter.new_project_cluster_path(project))) + expect(presenter.kubernetes_cluster_anchor_data).to have_attributes(enabled: false, + label: 'Add Kubernetes cluster', + link: presenter.new_project_cluster_path(project)) end end @@ -380,9 +417,9 @@ describe ProjectPresenter do allow(project.repository).to receive(:koding_yml).and_return(nil) allow(Gitlab::CurrentSettings).to receive(:koding_enabled?).and_return(true) - expect(presenter.koding_anchor_data).to eq(OpenStruct.new(enabled: false, - label: 'Set up Koding', - link: presenter.add_koding_stack_path)) + expect(presenter.koding_anchor_data).to have_attributes(enabled: false, + label: 'Set up Koding', + link: presenter.add_koding_stack_path) end it 'returns nil if user cannot push' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4de834bf93a..e987eee6e91 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -353,6 +353,15 @@ describe API::MergeRequests do end end + it 'returns the commits behind the target branch when include_diverged_commits_count is present' do + allow_any_instance_of(merge_request.class).to receive(:diverged_commits_count).and_return(1) + + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), include_diverged_commits_count: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response['diverged_commits_count']).to eq(1) + end + it "returns a 404 error if merge_request_iid not found" do get api("/projects/#{project.id}/merge_requests/999", user) expect(response).to have_gitlab_http_status(404) diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index d3f81cc038d..87997a48dc9 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -9,7 +9,8 @@ describe API::ProjectHooks, 'ProjectHooks' do :all_events_enabled, project: project, url: 'http://example.com', - enable_ssl_verification: true) + enable_ssl_verification: true, + push_events_branch_filter: 'master') end before do @@ -38,6 +39,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response.first['pipeline_events']).to eq(true) expect(json_response.first['wiki_page_events']).to eq(true) expect(json_response.first['enable_ssl_verification']).to eq(true) + expect(json_response.first['push_events_branch_filter']).to eq('master') end end @@ -90,7 +92,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect do post api("/projects/#{project.id}/hooks", user), url: "http://example.com", issues_events: true, confidential_issues_events: true, wiki_page_events: true, - job_events: true + job_events: true, push_events_branch_filter: 'some-feature-branch' end.to change {project.hooks.count}.by(1) expect(response).to have_gitlab_http_status(201) @@ -106,6 +108,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['pipeline_events']).to eq(false) expect(json_response['wiki_page_events']).to eq(true) expect(json_response['enable_ssl_verification']).to eq(true) + expect(json_response['push_events_branch_filter']).to eq('some-feature-branch') expect(json_response).not_to include('token') end @@ -132,7 +135,12 @@ describe API::ProjectHooks, 'ProjectHooks' do end it "returns a 422 error if url not valid" do - post api("/projects/#{project.id}/hooks", user), "url" => "ftp://example.com" + post api("/projects/#{project.id}/hooks", user), url: "ftp://example.com" + expect(response).to have_gitlab_http_status(422) + end + + it "returns a 422 error if branch filter is not valid" do + post api("/projects/#{project.id}/hooks", user), url: "http://example.com", push_events_branch_filter: '~badbranchname/' expect(response).to have_gitlab_http_status(422) end end diff --git a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb index 03eeffe6483..892b393c307 100644 --- a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb +++ b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb @@ -328,6 +328,22 @@ describe RuboCop::Cop::LineBreakAroundConditionalBlock do expect(cop.offenses).to be_empty end + it "doesn't flag violation for #{conditional} preceded by a rescue" do + source = <<~RUBY + def a_method + do_something + rescue + #{conditional} condition + do_something + end + end + RUBY + + inspect_source(source) + + expect(cop.offenses).to be_empty + end + it "doesn't flag violation for #{conditional} followed by a rescue" do source = <<~RUBY def a_method diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb index 507909d9231..b69977c812a 100644 --- a/spec/services/preview_markdown_service_spec.rb +++ b/spec/services/preview_markdown_service_spec.rb @@ -101,4 +101,11 @@ describe PreviewMarkdownService do expect(result[:markdown_engine]).to eq :common_mark end + + it 'honors the legacy_render parameter' do + service = described_class.new(project, user, { legacy_render: '1' }) + result = service.execute + + expect(result[:markdown_engine]).to eq :redcarpet + end end diff --git a/spec/support/helpers/kubernetes_helpers.rb b/spec/support/helpers/kubernetes_helpers.rb index 683a64504a1..994a2aaef90 100644 --- a/spec/support/helpers/kubernetes_helpers.rb +++ b/spec/support/helpers/kubernetes_helpers.rb @@ -16,6 +16,7 @@ module KubernetesHelpers def stub_kubeclient_discover(api_url) WebMock.stub_request(:get, api_url + '/api/v1').to_return(kube_response(kube_v1_discovery_body)) WebMock.stub_request(:get, api_url + '/apis/extensions/v1beta1').to_return(kube_response(kube_v1beta1_discovery_body)) + WebMock.stub_request(:get, api_url + '/apis/rbac.authorization.k8s.io/v1').to_return(kube_response(kube_v1_rbac_authorization_discovery_body)) end def stub_kubeclient_pods(response = nil) @@ -66,7 +67,8 @@ module KubernetesHelpers "resources" => [ { "name" => "pods", "namespaced" => true, "kind" => "Pod" }, { "name" => "deployments", "namespaced" => true, "kind" => "Deployment" }, - { "name" => "secrets", "namespaced" => true, "kind" => "Secret" } + { "name" => "secrets", "namespaced" => true, "kind" => "Secret" }, + { "name" => "services", "namespaced" => true, "kind" => "Service" } ] } end @@ -77,7 +79,20 @@ module KubernetesHelpers "resources" => [ { "name" => "pods", "namespaced" => true, "kind" => "Pod" }, { "name" => "deployments", "namespaced" => true, "kind" => "Deployment" }, - { "name" => "secrets", "namespaced" => true, "kind" => "Secret" } + { "name" => "secrets", "namespaced" => true, "kind" => "Secret" }, + { "name" => "services", "namespaced" => true, "kind" => "Service" } + ] + } + end + + def kube_v1_rbac_authorization_discovery_body + { + "kind" => "APIResourceList", + "resources" => [ + { "name" => "clusterrolebindings", "namespaced" => false, "kind" => "ClusterRoleBinding" }, + { "name" => "clusterroles", "namespaced" => false, "kind" => "ClusterRole" }, + { "name" => "rolebindings", "namespaced" => true, "kind" => "RoleBinding" }, + { "name" => "roles", "namespaced" => true, "kind" => "Role" } ] } end diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb index c54a871b157..4061a8d1bc9 100644 --- a/spec/support/helpers/stub_feature_flags.rb +++ b/spec/support/helpers/stub_feature_flags.rb @@ -4,8 +4,8 @@ module StubFeatureFlags # @param [Hash] features where key is feature name and value is boolean whether enabled or not def stub_feature_flags(features) features.each do |feature_name, enabled| - allow(Feature).to receive(:enabled?).with(feature_name) { enabled } - allow(Feature).to receive(:enabled?).with(feature_name.to_s) { enabled } + allow(Feature).to receive(:enabled?).with(feature_name, any_args) { enabled } + allow(Feature).to receive(:enabled?).with(feature_name.to_s, any_args) { enabled } end end end diff --git a/spec/validators/branch_filter_validator_spec.rb b/spec/validators/branch_filter_validator_spec.rb new file mode 100644 index 00000000000..3be54827431 --- /dev/null +++ b/spec/validators/branch_filter_validator_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe BranchFilterValidator do + let(:validator) { described_class.new(attributes: [:push_events_branch_filter]) } + let(:hook) { build(:project_hook) } + + describe '#validates_each' do + it 'allows valid branch names' do + validator.validate_each(hook, :push_events_branch_filter, "good_branch_name") + validator.validate_each(hook, :push_events_branch_filter, "another/good_branch_name") + expect(hook.errors.empty?).to be true + end + + it 'disallows bad branch names' do + validator.validate_each(hook, :push_events_branch_filter, "bad branch~name") + expect(hook.errors[:push_events_branch_filter].empty?).to be false + end + + it 'allows wildcards' do + validator.validate_each(hook, :push_events_branch_filter, "features/*") + validator.validate_each(hook, :push_events_branch_filter, "features/*/bla") + validator.validate_each(hook, :push_events_branch_filter, "*-stable") + expect(hook.errors.empty?).to be true + end + + it 'gets rid of whitespace' do + filter = ' master ' + validator.validate_each(hook, :push_events_branch_filter, filter) + + expect(filter).to eq 'master' + end + + # Branch names can be quite long but in practice aren't over 255 so 4000 should + # be enough space for a list of branch names but we can increase if needed. + it 'limits length to 4000 chars' do + filter = 'a' * 4001 + validator.validate_each(hook, :push_events_branch_filter, filter) + + expect(hook.errors[:push_events_branch_filter].empty?).to be false + end + end +end diff --git a/spec/views/projects/_home_panel.html.haml_spec.rb b/spec/views/projects/_home_panel.html.haml_spec.rb index b56940a9613..fc1fe5739c3 100644 --- a/spec/views/projects/_home_panel.html.haml_spec.rb +++ b/spec/views/projects/_home_panel.html.haml_spec.rb @@ -9,6 +9,7 @@ describe 'projects/_home_panel' do allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:can?).with(user, :read_project, project).and_return(false) + allow(project).to receive(:license_anchor_data).and_return(false) end context 'when user is signed in' do @@ -63,6 +64,7 @@ describe 'projects/_home_panel' do allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:can?).with(user, :read_project, project).and_return(false) + allow(project).to receive(:license_anchor_data).and_return(false) end context 'has no badges' do @@ -71,8 +73,7 @@ describe 'projects/_home_panel' do it 'should not render any badge' do render - expect(rendered).to have_selector('.project-badges') - expect(rendered).not_to have_selector('.project-badges > a') + expect(rendered).not_to have_selector('.project-badges') end end @@ -118,6 +119,7 @@ describe 'projects/_home_panel' do assign(:project, project) allow(view).to receive(:current_user).and_return(user) + allow(project).to receive(:license_anchor_data).and_return(false) end context 'user can read project' do |