From 07e42c5c425b5a781398fea2aaf7eeb8a825724f Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 23 Nov 2017 13:17:10 +0100 Subject: [PATCH 01/10] Clarify the docs review process and mention the supported repos --- doc/development/writing_documentation.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/doc/development/writing_documentation.md b/doc/development/writing_documentation.md index 68ba3dd2da3..b6def7ef541 100644 --- a/doc/development/writing_documentation.md +++ b/doc/development/writing_documentation.md @@ -152,12 +152,23 @@ CE and EE. ## Previewing the changes live If you want to preview the doc changes of your merge request live, you can use -the manual `review-docs-deploy` job in your merge request. +the manual `review-docs-deploy` job in your merge request. You will need at +least Master permissions to be able to run it and is currently enabled for the +following projects: + +- https://gitlab.com/gitlab-org/gitlab-ce +- https://gitlab.com/gitlab-org/gitlab-ee + +NOTE: **Note:** +You will need to push a branch to those repositories, it doesn't work for forks. TIP: **Tip:** If your branch contains only documentation changes, you can use [special branch names](#testing) to avoid long running pipelines. +In the mini pipeline graph, you should see an `>>` icon. Clicking on it will +reveal the `review-docs-deploy` job. Hit the play button for the job to start. + ![Manual trigger a docs build](img/manual_build_docs.png) This job will: From 20f78421c82d626a3b43924146b7878fe4777ae8 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 30 Nov 2017 14:26:08 +0100 Subject: [PATCH 02/10] Cache the forks in a namespace in the RequestStore On the `show` of a project that is part of a fork network. We check if the user already created a fork of this project in their personal namespace. We do this in several places, so caching the result of this query in the request store prevents us from repeating it. --- app/models/namespace.rb | 12 +++++++++- ...bvl-limit-fork-queries-on-project-show.yml | 5 ++++ spec/controllers/projects_controller_spec.rb | 23 +++++++++++++++++++ spec/models/namespace_spec.rb | 10 +++++++- 4 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml diff --git a/app/models/namespace.rb b/app/models/namespace.rb index fa76729a702..901dbf2ba69 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -139,7 +139,17 @@ class Namespace < ActiveRecord::Base def find_fork_of(project) return nil unless project.fork_network - project.fork_network.find_forks_in(projects).first + if RequestStore.active? + forks_in_namespace = RequestStore.fetch("namespaces:#{id}:forked_projects") do + Hash.new do |found_forks, project| + found_forks[project] = project.fork_network.find_forks_in(projects).first + end + end + + forks_in_namespace[project] + else + project.fork_network.find_forks_in(projects).first + end end def lfs_enabled? diff --git a/changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml b/changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml new file mode 100644 index 00000000000..299d9bf6b9c --- /dev/null +++ b/changelogs/unreleased/bvl-limit-fork-queries-on-project-show.yml @@ -0,0 +1,5 @@ +--- +title: Reduce requests for project forks on show page of projects that have forks +merge_request: 15663 +author: +type: performance diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index e7ab714c550..39ee5e8691b 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -261,6 +261,29 @@ describe ProjectsController do expect(response).to redirect_to(namespace_project_path) end end + + context 'when the project is forked and has a repository', :request_store do + let(:public_project) { create(:project, :public, :repository) } + + render_views + + before do + # View the project as a user that does not have any rights + sign_in(create(:user)) + + fork_project(public_project) + end + + it 'does not increase the number of queries when the project is forked' do + # When a project is part of a fork network, we check if the `current_user` + # has a fork in their own namespace. We query this several times. Caching + # the result in the RequestStore brings the number of queries for this + # request down from 64 to 59. + + expect { get(:show, namespace_id: public_project.namespace, id: public_project) } + .not_to exceed_query_limit(59) + end + end end describe "#update" do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 90b768f595e..3817f20bfe7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -531,7 +531,7 @@ describe Namespace do end end - describe '#has_forks_of?' do + describe '#find_fork_of?' do let(:project) { create(:project, :public) } let!(:forked_project) { fork_project(project, namespace.owner, namespace: namespace) } @@ -550,5 +550,13 @@ describe Namespace do expect(other_namespace.find_fork_of(project)).to eq(other_fork) end + + context 'with request store enabled', :request_store do + it 'only queries once' do + expect(project.fork_network).to receive(:find_forks_in).once.and_call_original + + 2.times { namespace.find_fork_of(project) } + end + end end end From 3d4ba90c5007cca3e8619afe8f40675962a9bc73 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 4 Dec 2017 16:58:44 +0100 Subject: [PATCH 03/10] Count occurrences of a specific query in the query recorder. --- spec/controllers/projects_controller_spec.rb | 10 ++++------ spec/support/query_recorder.rb | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 39ee5e8691b..e4b2bbb7c51 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -264,24 +264,22 @@ describe ProjectsController do context 'when the project is forked and has a repository', :request_store do let(:public_project) { create(:project, :public, :repository) } + let(:other_user) { create(:user) } render_views before do # View the project as a user that does not have any rights - sign_in(create(:user)) + sign_in(other_user) fork_project(public_project) end it 'does not increase the number of queries when the project is forked' do - # When a project is part of a fork network, we check if the `current_user` - # has a fork in their own namespace. We query this several times. Caching - # the result in the RequestStore brings the number of queries for this - # request down from 64 to 59. + expected_query = /#{public_project.fork_network.find_forks_in(other_user.namespace).to_sql}/ expect { get(:show, namespace_id: public_project.namespace, id: public_project) } - .not_to exceed_query_limit(59) + .not_to exceed_query_limit(1).for_query(expected_query) end end end diff --git a/spec/support/query_recorder.rb b/spec/support/query_recorder.rb index 369775db462..8cf8f45a8b2 100644 --- a/spec/support/query_recorder.rb +++ b/spec/support/query_recorder.rb @@ -41,7 +41,8 @@ RSpec::Matchers.define :exceed_query_limit do |expected| supports_block_expectations match do |block| - query_count(&block) > expected_count + threshold + @subject_block = block + actual_count > expected_count + threshold end failure_message_when_negated do |actual| @@ -55,6 +56,11 @@ RSpec::Matchers.define :exceed_query_limit do |expected| self end + def for_query(query) + @query = query + self + end + def threshold @threshold.to_i end @@ -68,12 +74,15 @@ RSpec::Matchers.define :exceed_query_limit do |expected| end def actual_count - @recorder.count + @actual_count ||= if @query + recorder.log.select { |recorded| recorded =~ @query }.size + else + recorder.count + end end - def query_count(&block) - @recorder = ActiveRecord::QueryRecorder.new(&block) - @recorder.count + def recorder + @recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block) end def count_queries(queries) From f91c5c5bbe15adc1cc951bc734123c2994622f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 4 Dec 2017 12:58:24 -0300 Subject: [PATCH 04/10] Backport Squash/Rebase refactor from EE --- lib/gitlab/git/repository.rb | 148 +++++++++++++++++++++++++++++++++-- 1 file changed, 140 insertions(+), 8 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 867fc2a42f6..d50de2fb6c1 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -18,6 +18,8 @@ module Gitlab GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE ].freeze SEARCH_CONTEXT_LINES = 3 + REBASE_WORKTREE_PREFIX = 'rebase'.freeze + SQUASH_WORKTREE_PREFIX = 'squash'.freeze NoRepository = Class.new(StandardError) InvalidBlobName = Class.new(StandardError) @@ -1090,13 +1092,8 @@ module Gitlab raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ') raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00") - command = [Gitlab.config.git.bin_path] + %w[update-ref --stdin -z] input = "update #{ref_path}\x00#{ref}\x00\x00" - output, status = circuit_breaker.perform do - popen(command, path) { |stdin| stdin.write(input) } - end - - raise GitError, output unless status.zero? + run_git!(%w[update-ref --stdin -z]) { |stdin| stdin.write(input) } end def fetch_ref(source_repository, source_ref:, target_ref:) @@ -1118,14 +1115,22 @@ module Gitlab end # Refactoring aid; allows us to copy code from app/models/repository.rb - def run_git(args, env: {}, nice: false) + def run_git(args, chdir: path, env: {}, nice: false, &block) cmd = [Gitlab.config.git.bin_path, *args] cmd.unshift("nice") if nice circuit_breaker.perform do - popen(cmd, path, env) + popen(cmd, chdir, env, &block) end end + def run_git!(args, chdir: path, env: {}, nice: false, &block) + output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block) + + raise GitError, output unless status.zero? + + output + end + # Refactoring aid; allows us to copy code from app/models/repository.rb def run_git_with_timeout(args, timeout, env: {}) circuit_breaker.perform do @@ -1195,6 +1200,64 @@ module Gitlab raise GitError.new("Could not fsck repository:\n#{output}") unless status.zero? end + def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:) + rebase_path = worktree_path(REBASE_WORKTREE_PREFIX, rebase_id) + env = git_env_for_user(user) + + with_worktree(rebase_path, branch, env: env) do + run_git!( + %W(pull --rebase #{remote_repository.path} #{remote_branch}), + chdir: rebase_path, env: env + ) + + rebase_sha = run_git!(%w(rev-parse HEAD), chdir: rebase_path, env: env).strip + + Gitlab::Git::OperationService.new(user, self) + .update_branch(branch, rebase_sha, branch_sha) + + rebase_sha + end + end + + def rebase_in_progress?(rebase_id) + fresh_worktree?(worktree_path(REBASE_WORKTREE_PREFIX, rebase_id)) + end + + def squash(user, squash_id, branch:, start_sha:, end_sha:, author:, message:) + squash_path = worktree_path(SQUASH_WORKTREE_PREFIX, squash_id) + env = git_env_for_user(user).merge( + 'GIT_AUTHOR_NAME' => author.name, + 'GIT_AUTHOR_EMAIL' => author.email + ) + diff_range = "#{start_sha}...#{end_sha}" + diff_files = run_git!( + %W(diff --name-only --diff-filter=a --binary #{diff_range}) + ).chomp + + with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do + # Apply diff of the `diff_range` to the worktree + diff = run_git!(%W(diff --binary #{diff_range})) + run_git!(%w(apply --index), chdir: squash_path, env: env) do |stdin| + stdin.write(diff) + end + + # Commit the `diff_range` diff + run_git!(%W(commit --no-verify --message #{message}), chdir: squash_path, env: env) + + # Return the squash sha. May print a warning for ambiguous refs, but + # we can ignore that with `--quiet` and just take the SHA, if present. + # HEAD here always refers to the current HEAD commit, even if there is + # another ref called HEAD. + run_git!( + %w(rev-parse --quiet --verify HEAD), chdir: squash_path, env: env + ).chomp + end + end + + def squash_in_progress?(squash_id) + fresh_worktree?(worktree_path(SQUASH_WORKTREE_PREFIX, squash_id)) + end + def gitaly_repository Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository) end @@ -1231,6 +1294,57 @@ module Gitlab private + def fresh_worktree?(path) + File.exist?(path) && !clean_stuck_worktree(path) + end + + def with_worktree(worktree_path, branch, sparse_checkout_files: nil, env:) + base_args = %w(worktree add --detach) + + # Note that we _don't_ want to test for `.present?` here: If the caller + # passes an non nil empty value it means it still wants sparse checkout + # but just isn't interested in any file, perhaps because it wants to + # checkout files in by a changeset but that changeset only adds files. + if sparse_checkout_files + # Create worktree without checking out + run_git!(base_args + ['--no-checkout', worktree_path], env: env) + worktree_git_path = run_git!(%w(rev-parse --git-dir), chdir: worktree_path) + + configure_sparse_checkout(worktree_git_path, sparse_checkout_files) + + # After sparse checkout configuration, checkout `branch` in worktree + run_git!(%W(checkout --detach #{branch}), chdir: worktree_path, env: env) + else + # Create worktree and checkout `branch` in it + run_git!(base_args + [worktree_path, branch], env: env) + end + + yield + ensure + FileUtils.rm_rf(worktree_path) if File.exist?(worktree_path) + FileUtils.rm_rf(worktree_git_path) if worktree_git_path && File.exist?(worktree_git_path) + end + + def clean_stuck_worktree(path) + return false unless File.mtime(path) < 15.minutes.ago + + FileUtils.rm_rf(path) + true + end + + # Adding a worktree means checking out the repository. For large repos, + # this can be very expensive, so set up sparse checkout for the worktree + # to only check out the files we're interested in. + def configure_sparse_checkout(worktree_git_path, files) + run_git!(%w(config core.sparseCheckout true)) + + return if files.empty? + + worktree_info_path = File.join(worktree_git_path, 'info') + FileUtils.mkdir_p(worktree_info_path) + File.write(File.join(worktree_info_path, 'sparse-checkout'), files) + end + def rugged_fetch_source_branch(source_repository, source_branch, local_ref) with_repo_branch_commit(source_repository, source_branch) do |commit| if commit @@ -1242,6 +1356,24 @@ module Gitlab end end + def worktree_path(prefix, id) + id = id.to_s + raise ArgumentError, "worktree id can't be empty" unless id.present? + raise ArgumentError, "worktree id can't contain slashes " if id.include?("/") + + File.join(path, 'gitlab-worktree', "#{prefix}-#{id}") + end + + def git_env_for_user(user) + { + 'GIT_COMMITTER_NAME' => user.name, + 'GIT_COMMITTER_EMAIL' => user.email, + 'GL_ID' => Gitlab::GlId.gl_id(user), + 'GL_PROTOCOL' => Gitlab::Git::Hook::GL_PROTOCOL, + 'GL_REPOSITORY' => gl_repository + } + end + # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. def branches_filter(filter: nil, sort_by: nil) # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37464 From 2286681e1ca5cfbdbb6167cc4f31f0933ee584a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 30 Nov 2017 20:04:57 -0300 Subject: [PATCH 05/10] Add missing attr_accessor to Gitlab::Git::Conflict::File --- lib/gitlab/git/conflict/file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/git/conflict/file.rb b/lib/gitlab/git/conflict/file.rb index fc1595f1faf..b2a625e08fa 100644 --- a/lib/gitlab/git/conflict/file.rb +++ b/lib/gitlab/git/conflict/file.rb @@ -2,7 +2,7 @@ module Gitlab module Git module Conflict class File - attr_reader :content, :their_path, :our_path, :our_mode, :repository + attr_reader :content, :their_path, :our_path, :our_mode, :repository, :commit_oid def initialize(repository, commit_oid, conflict, content) @repository = repository From 359b65beac43e009b715c2db048e06b6f96b0ee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Fri, 1 Dec 2017 18:15:52 -0300 Subject: [PATCH 06/10] Use `String#end_with?` instead of `String#ends_with?` The former is in Ruby's core lib, so is more flexible. --- lib/gitlab/git/conflict/resolver.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb index df509c5f4ce..de8cce41b6d 100644 --- a/lib/gitlab/git/conflict/resolver.rb +++ b/lib/gitlab/git/conflict/resolver.rb @@ -75,7 +75,7 @@ module Gitlab resolved_lines = file.resolve_lines(params[:sections]) new_file = resolved_lines.map { |line| line[:full_line] }.join("\n") - new_file << "\n" if file.our_blob.data.ends_with?("\n") + new_file << "\n" if file.our_blob.data.end_with?("\n") elsif params[:content] new_file = file.resolve_content(params[:content]) end From f3433d8a6801d369752526c1d92a35048e263e20 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Sun, 3 Dec 2017 20:43:10 -0600 Subject: [PATCH 07/10] Backport changes from refactor sidebar weight block Vue and move to Issue Boards See https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3566 --- app/assets/javascripts/boards/models/issue.js | 5 + app/assets/javascripts/gl_dropdown.js | 3 +- app/assets/javascripts/right_sidebar.js | 6 +- .../javascripts/sidebar/mount_sidebar.js | 104 ++++++++++++++++++ .../javascripts/sidebar/sidebar_bundle.js | 102 +---------------- .../javascripts/sidebar/sidebar_mediator.js | 34 +++--- .../sidebar/stores/sidebar_store.js | 5 + spec/javascripts/boards/issue_spec.js | 6 + spec/javascripts/sidebar/mock_data.js | 84 ++++++++++++-- .../sidebar/sidebar_mediator_spec.js | 27 ++++- .../javascripts/sidebar/sidebar_store_spec.js | 6 + 11 files changed, 250 insertions(+), 132 deletions(-) create mode 100644 app/assets/javascripts/sidebar/mount_sidebar.js diff --git a/app/assets/javascripts/boards/models/issue.js b/app/assets/javascripts/boards/models/issue.js index 10f85c1d676..81edd95bf2b 100644 --- a/app/assets/javascripts/boards/models/issue.js +++ b/app/assets/javascripts/boards/models/issue.js @@ -20,6 +20,7 @@ class ListIssue { this.isFetching = { subscriptions: true, }; + this.isLoading = {}; this.sidebarInfoEndpoint = obj.issue_sidebar_endpoint; this.toggleSubscriptionEndpoint = obj.toggle_subscription_endpoint; @@ -86,6 +87,10 @@ class ListIssue { this.isFetching[key] = value; } + setLoadingState(key, value) { + this.isLoading[key] = value; + } + update (url) { const data = { issue: { diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 4e7a6e54f90..7ca783d3af6 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -514,10 +514,11 @@ GitLabDropdown = (function() { const dropdownToggle = this.dropdown.find('.dropdown-menu-toggle'); const hasFilterBulkUpdate = dropdownToggle.hasClass('js-filter-bulk-update'); + const shouldRefreshOnOpen = dropdownToggle.hasClass('js-gl-dropdown-refresh-on-open'); const hasMultiSelect = dropdownToggle.hasClass('js-multiselect'); // Makes indeterminate items effective - if (this.fullData && hasFilterBulkUpdate) { + if (this.fullData && (shouldRefreshOnOpen || hasFilterBulkUpdate)) { this.parseData(this.fullData); } diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index a41548bd694..fa7f6825d7e 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -15,7 +15,7 @@ import Cookies from 'js-cookie'; Sidebar.prototype.removeListeners = function () { this.sidebar.off('click', '.sidebar-collapsed-icon'); - $('.dropdown').off('hidden.gl.dropdown'); + this.sidebar.off('hidden.gl.dropdown'); $('.dropdown').off('loading.gl.dropdown'); $('.dropdown').off('loaded.gl.dropdown'); $(document).off('click', '.js-sidebar-toggle'); @@ -25,7 +25,7 @@ import Cookies from 'js-cookie'; const $document = $(document); this.sidebar.on('click', '.sidebar-collapsed-icon', this, this.sidebarCollapseClicked); - $('.dropdown').on('hidden.gl.dropdown', this, this.onSidebarDropdownHidden); + this.sidebar.on('hidden.gl.dropdown', this, this.onSidebarDropdownHidden); $('.dropdown').on('loading.gl.dropdown', this.sidebarDropdownLoading); $('.dropdown').on('loaded.gl.dropdown', this.sidebarDropdownLoaded); @@ -180,7 +180,7 @@ import Cookies from 'js-cookie'; var $block, sidebar; sidebar = e.data; e.preventDefault(); - $block = $(this).closest('.block'); + $block = $(e.target).closest('.block'); return sidebar.sidebarDropdownHidden($block); }; diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js new file mode 100644 index 00000000000..4032f156b15 --- /dev/null +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -0,0 +1,104 @@ +import Vue from 'vue'; +import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; +import SidebarAssignees from './components/assignees/sidebar_assignees'; +import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue'; +import SidebarMoveIssue from './lib/sidebar_move_issue'; +import LockIssueSidebar from './components/lock/lock_issue_sidebar.vue'; +import sidebarParticipants from './components/participants/sidebar_participants.vue'; +import sidebarSubscriptions from './components/subscriptions/sidebar_subscriptions.vue'; +import Translate from '../vue_shared/translate'; + +Vue.use(Translate); + +function mountConfidentialComponent(mediator) { + const el = document.getElementById('js-confidential-entry-point'); + + if (!el) return; + + const dataNode = document.getElementById('js-confidential-issue-data'); + const initialData = JSON.parse(dataNode.innerHTML); + + const ConfidentialComp = Vue.extend(ConfidentialIssueSidebar); + + new ConfidentialComp({ + propsData: { + isConfidential: initialData.is_confidential, + isEditable: initialData.is_editable, + service: mediator.service, + }, + }).$mount(el); +} + +function mountLockComponent(mediator) { + const el = document.getElementById('js-lock-entry-point'); + + if (!el) return; + + const dataNode = document.getElementById('js-lock-issue-data'); + const initialData = JSON.parse(dataNode.innerHTML); + + const LockComp = Vue.extend(LockIssueSidebar); + + new LockComp({ + propsData: { + isLocked: initialData.is_locked, + isEditable: initialData.is_editable, + mediator, + issuableType: gl.utils.isInIssuePage() ? 'issue' : 'merge_request', + }, + }).$mount(el); +} + +function mountParticipantsComponent() { + const el = document.querySelector('.js-sidebar-participants-entry-point'); + + if (!el) return; + + // eslint-disable-next-line no-new + new Vue({ + el, + components: { + sidebarParticipants, + }, + render: createElement => createElement('sidebar-participants', {}), + }); +} + +function mountSubscriptionsComponent() { + const el = document.querySelector('.js-sidebar-subscriptions-entry-point'); + + if (!el) return; + + // eslint-disable-next-line no-new + new Vue({ + el, + components: { + sidebarSubscriptions, + }, + render: createElement => createElement('sidebar-subscriptions', {}), + }); +} + +function mount(mediator) { + const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees'); + // Only create the sidebarAssignees vue app if it is found in the DOM + // We currently do not use sidebarAssignees for the MR page + if (sidebarAssigneesEl) { + new Vue(SidebarAssignees).$mount(sidebarAssigneesEl); + } + + mountConfidentialComponent(mediator); + mountLockComponent(mediator); + mountParticipantsComponent(); + mountSubscriptionsComponent(); + + new SidebarMoveIssue( + mediator, + $('.js-move-issue'), + $('.js-move-issue-confirmation-button'), + ).init(); + + new Vue(SidebarTimeTracking).$mount('#issuable-time-tracker'); +} + +export default mount; diff --git a/app/assets/javascripts/sidebar/sidebar_bundle.js b/app/assets/javascripts/sidebar/sidebar_bundle.js index 2650bb725d4..f78287e504b 100644 --- a/app/assets/javascripts/sidebar/sidebar_bundle.js +++ b/app/assets/javascripts/sidebar/sidebar_bundle.js @@ -1,110 +1,12 @@ -import Vue from 'vue'; -import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; -import SidebarAssignees from './components/assignees/sidebar_assignees'; -import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue'; -import SidebarMoveIssue from './lib/sidebar_move_issue'; -import LockIssueSidebar from './components/lock/lock_issue_sidebar.vue'; -import sidebarParticipants from './components/participants/sidebar_participants.vue'; -import sidebarSubscriptions from './components/subscriptions/sidebar_subscriptions.vue'; -import Translate from '../vue_shared/translate'; - import Mediator from './sidebar_mediator'; - -Vue.use(Translate); - -function mountConfidentialComponent(mediator) { - const el = document.getElementById('js-confidential-entry-point'); - - if (!el) return; - - const dataNode = document.getElementById('js-confidential-issue-data'); - const initialData = JSON.parse(dataNode.innerHTML); - - const ConfidentialComp = Vue.extend(ConfidentialIssueSidebar); - - new ConfidentialComp({ - propsData: { - isConfidential: initialData.is_confidential, - isEditable: initialData.is_editable, - service: mediator.service, - }, - }).$mount(el); -} - -function mountLockComponent(mediator) { - const el = document.getElementById('js-lock-entry-point'); - - if (!el) return; - - const dataNode = document.getElementById('js-lock-issue-data'); - const initialData = JSON.parse(dataNode.innerHTML); - - const LockComp = Vue.extend(LockIssueSidebar); - - new LockComp({ - propsData: { - isLocked: initialData.is_locked, - isEditable: initialData.is_editable, - mediator, - issuableType: gl.utils.isInIssuePage() ? 'issue' : 'merge_request', - }, - }).$mount(el); -} - -function mountParticipantsComponent() { - const el = document.querySelector('.js-sidebar-participants-entry-point'); - - if (!el) return; - - // eslint-disable-next-line no-new - new Vue({ - el, - components: { - sidebarParticipants, - }, - render: createElement => createElement('sidebar-participants', {}), - }); -} - -function mountSubscriptionsComponent() { - const el = document.querySelector('.js-sidebar-subscriptions-entry-point'); - - if (!el) return; - - // eslint-disable-next-line no-new - new Vue({ - el, - components: { - sidebarSubscriptions, - }, - render: createElement => createElement('sidebar-subscriptions', {}), - }); -} +import mountSidebar from './mount_sidebar'; function domContentLoaded() { const sidebarOptions = JSON.parse(document.querySelector('.js-sidebar-options').innerHTML); const mediator = new Mediator(sidebarOptions); mediator.fetch(); - const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees'); - // Only create the sidebarAssignees vue app if it is found in the DOM - // We currently do not use sidebarAssignees for the MR page - if (sidebarAssigneesEl) { - new Vue(SidebarAssignees).$mount(sidebarAssigneesEl); - } - - mountConfidentialComponent(mediator); - mountLockComponent(mediator); - mountParticipantsComponent(); - mountSubscriptionsComponent(); - - new SidebarMoveIssue( - mediator, - $('.js-move-issue'), - $('.js-move-issue-confirmation-button'), - ).init(); - - new Vue(SidebarTimeTracking).$mount('#issuable-time-tracker'); + mountSidebar(mediator); } document.addEventListener('DOMContentLoaded', domContentLoaded); diff --git a/app/assets/javascripts/sidebar/sidebar_mediator.js b/app/assets/javascripts/sidebar/sidebar_mediator.js index 2bda5a47791..d4c07a188b3 100644 --- a/app/assets/javascripts/sidebar/sidebar_mediator.js +++ b/app/assets/javascripts/sidebar/sidebar_mediator.js @@ -5,19 +5,23 @@ import Store from './stores/sidebar_store'; export default class SidebarMediator { constructor(options) { if (!SidebarMediator.singleton) { - this.store = new Store(options); - this.service = new Service({ - endpoint: options.endpoint, - toggleSubscriptionEndpoint: options.toggleSubscriptionEndpoint, - moveIssueEndpoint: options.moveIssueEndpoint, - projectsAutocompleteEndpoint: options.projectsAutocompleteEndpoint, - }); - SidebarMediator.singleton = this; + this.initSingleton(options); } return SidebarMediator.singleton; } + initSingleton(options) { + this.store = new Store(options); + this.service = new Service({ + endpoint: options.endpoint, + toggleSubscriptionEndpoint: options.toggleSubscriptionEndpoint, + moveIssueEndpoint: options.moveIssueEndpoint, + projectsAutocompleteEndpoint: options.projectsAutocompleteEndpoint, + }); + SidebarMediator.singleton = this; + } + assignYourself() { this.store.addAssignee(this.store.currentUser); } @@ -35,17 +39,21 @@ export default class SidebarMediator { } fetch() { - this.service.get() + return this.service.get() .then(response => response.json()) .then((data) => { - this.store.setAssigneeData(data); - this.store.setTimeTrackingData(data); - this.store.setParticipantsData(data); - this.store.setSubscriptionsData(data); + this.processFetchedData(data); }) .catch(() => new Flash('Error occurred when fetching sidebar data')); } + processFetchedData(data) { + this.store.setAssigneeData(data); + this.store.setTimeTrackingData(data); + this.store.setParticipantsData(data); + this.store.setSubscriptionsData(data); + } + toggleSubscription() { this.store.setFetchingState('subscriptions', true); return this.service.toggleSubscription() diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index 3150221b685..73eb25e2333 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -15,6 +15,7 @@ export default class SidebarStore { participants: true, subscriptions: true, }; + this.isLoading = {}; this.autocompleteProjects = []; this.moveToProjectId = 0; this.isLockDialogOpen = false; @@ -55,6 +56,10 @@ export default class SidebarStore { this.isFetching[key] = value; } + setLoadingState(key, value) { + this.isLoading[key] = value; + } + addAssignee(assignee) { if (!this.findAssignee(assignee)) { this.assignees.push(assignee); diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js index ccde657789a..10b88878c2a 100644 --- a/spec/javascripts/boards/issue_spec.js +++ b/spec/javascripts/boards/issue_spec.js @@ -146,6 +146,12 @@ describe('Issue model', () => { expect(issue.isFetching.subscriptions).toBe(false); }); + it('sets loading state', () => { + issue.setLoadingState('foo', true); + + expect(issue.isLoading.foo).toBe(true); + }); + describe('update', () => { it('passes assignee ids when there are assignees', (done) => { spyOn(Vue.http, 'patch').and.callFake((url, data) => { diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js index 0682b463043..3b094d20838 100644 --- a/spec/javascripts/sidebar/mock_data.js +++ b/spec/javascripts/sidebar/mock_data.js @@ -1,6 +1,6 @@ /* eslint-disable quote-props*/ -const sidebarMockData = { +const RESPONSE_MAP = { 'GET': { '/gitlab-org/gitlab-shell/issues/5.json': { id: 45, @@ -66,6 +66,65 @@ const sidebarMockData = { }, labels: [], }, + '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar': { + assignees: [ + { + name: 'User 0', + username: 'user0', + id: 22, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/user0', + }, + { + name: 'Marguerite Bartell', + username: 'tajuana', + id: 18, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/tajuana', + }, + { + name: 'Laureen Ritchie', + username: 'michaele.will', + id: 16, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/michaele.will', + }, + ], + human_time_estimate: null, + human_total_time_spent: null, + participants: [ + { + name: 'User 0', + username: 'user0', + id: 22, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/user0', + }, + { + name: 'Marguerite Bartell', + username: 'tajuana', + id: 18, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/tajuana', + }, + { + name: 'Laureen Ritchie', + username: 'michaele.will', + id: 16, + state: 'active', + avatar_url: 'http: //www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', + web_url: 'http: //localhost:3001/michaele.will', + }, + ], + subscribed: true, + time_estimate: 0, + total_time_spent: 0, + }, '/autocomplete/projects?project_id=15': [ { 'id': 0, @@ -113,9 +172,10 @@ const sidebarMockData = { }, }; -export default { +const mockData = { + responseMap: RESPONSE_MAP, mediator: { - endpoint: '/gitlab-org/gitlab-shell/issues/5.json', + endpoint: '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar', toggleSubscriptionEndpoint: '/gitlab-org/gitlab-shell/issues/5/toggle_subscription', moveIssueEndpoint: '/gitlab-org/gitlab-shell/issues/5/move', projectsAutocompleteEndpoint: '/autocomplete/projects?project_id=15', @@ -141,12 +201,14 @@ export default { name: 'Administrator', username: 'root', }, - - sidebarMockInterceptor(request, next) { - const body = sidebarMockData[request.method.toUpperCase()][request.url]; - - next(request.respondWith(JSON.stringify(body), { - status: 200, - })); - }, }; + +mockData.sidebarMockInterceptor = function (request, next) { + const body = this.responseMap[request.method.toUpperCase()][request.url]; + + next(request.respondWith(JSON.stringify(body), { + status: 200, + })); +}.bind(mockData); + +export default mockData; diff --git a/spec/javascripts/sidebar/sidebar_mediator_spec.js b/spec/javascripts/sidebar/sidebar_mediator_spec.js index 7deb1fd2118..14c34d5a78c 100644 --- a/spec/javascripts/sidebar/sidebar_mediator_spec.js +++ b/spec/javascripts/sidebar/sidebar_mediator_spec.js @@ -33,10 +33,29 @@ describe('Sidebar mediator', () => { .catch(done.fail); }); - it('fetches the data', () => { - spyOn(this.mediator.service, 'get').and.callThrough(); - this.mediator.fetch(); - expect(this.mediator.service.get).toHaveBeenCalled(); + it('fetches the data', (done) => { + const mockData = Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar']; + spyOn(this.mediator, 'processFetchedData').and.callThrough(); + + this.mediator.fetch() + .then(() => { + expect(this.mediator.processFetchedData).toHaveBeenCalledWith(mockData); + }) + .then(done) + .catch(done.fail); + }); + + it('processes fetched data', () => { + const mockData = Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar']; + this.mediator.processFetchedData(mockData); + + expect(this.mediator.store.assignees).toEqual(mockData.assignees); + expect(this.mediator.store.humanTimeEstimate).toEqual(mockData.human_time_estimate); + expect(this.mediator.store.humanTotalTimeSpent).toEqual(mockData.human_total_time_spent); + expect(this.mediator.store.participants).toEqual(mockData.participants); + expect(this.mediator.store.subscribed).toEqual(mockData.subscribed); + expect(this.mediator.store.timeEstimate).toEqual(mockData.time_estimate); + expect(this.mediator.store.totalTimeSpent).toEqual(mockData.total_time_spent); }); it('sets moveToProjectId', () => { diff --git a/spec/javascripts/sidebar/sidebar_store_spec.js b/spec/javascripts/sidebar/sidebar_store_spec.js index 51dee64fb93..ea4eae1e23f 100644 --- a/spec/javascripts/sidebar/sidebar_store_spec.js +++ b/spec/javascripts/sidebar/sidebar_store_spec.js @@ -120,6 +120,12 @@ describe('Sidebar store', () => { expect(this.store.isFetching.participants).toEqual(false); }); + it('sets loading state', () => { + this.store.setLoadingState('assignees', true); + + expect(this.store.isLoading.assignees).toEqual(true); + }); + it('set time tracking data', () => { this.store.setTimeTrackingData(Mock.time); expect(this.store.timeEstimate).toEqual(Mock.time.time_estimate); From b0e2e21b7db5cc80f6a17b19354e91dd7fcf6c8c Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 5 Dec 2017 09:01:46 +0000 Subject: [PATCH 08/10] remove ambiguity about which resource type to be using for new sessions --- app/views/devise/passwords/edit.html.haml | 4 ++-- app/views/devise/sessions/_new_base.html.haml | 2 +- app/views/devise/shared/_links.erb | 10 +++++----- app/views/devise/shared/_sign_in_link.html.haml | 2 +- app/views/devise/shared/_signup_box.html.haml | 2 +- app/views/errors/omniauth_error.html.haml | 2 +- .../unreleased/39367-fix-new-email-session-path.yml | 5 +++++ 7 files changed, 16 insertions(+), 11 deletions(-) create mode 100644 changelogs/unreleased/39367-fix-new-email-session-path.yml diff --git a/app/views/devise/passwords/edit.html.haml b/app/views/devise/passwords/edit.html.haml index eb0e6701627..35dafb3e980 100644 --- a/app/views/devise/passwords/edit.html.haml +++ b/app/views/devise/passwords/edit.html.haml @@ -1,7 +1,7 @@ = render 'devise/shared/tab_single', tab_title:'Change your password' .login-box .login-body - = form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :put, class: 'gl-show-field-errors' }) do |f| + = form_for(resource, as: resource_name, url: password_path(:user), html: { method: :put, class: 'gl-show-field-errors' }) do |f| .devise-errors = devise_error_messages! = f.hidden_field :reset_password_token @@ -17,5 +17,5 @@ .clearfix.prepend-top-20 %p %span.light Didn't receive a confirmation email? - = link_to "Request a new one", new_confirmation_path(resource_name) + = link_to "Request a new one", new_confirmation_path(:user) = render 'devise/shared/sign_in_link' diff --git a/app/views/devise/sessions/_new_base.html.haml b/app/views/devise/sessions/_new_base.html.haml index 4095f30c369..41462f503cb 100644 --- a/app/views/devise/sessions/_new_base.html.haml +++ b/app/views/devise/sessions/_new_base.html.haml @@ -11,6 +11,6 @@ = f.check_box :remember_me, class: 'remember-me-checkbox' %span Remember me .pull-right.forgot-password - = link_to "Forgot your password?", new_password_path(resource_name) + = link_to "Forgot your password?", new_password_path(:user) .submit-container.move-submit-down = f.submit "Sign in", class: "btn btn-save" diff --git a/app/views/devise/shared/_links.erb b/app/views/devise/shared/_links.erb index 6e1cc244f26..cb934434c28 100644 --- a/app/views/devise/shared/_links.erb +++ b/app/views/devise/shared/_links.erb @@ -1,19 +1,19 @@ <%- if controller_name != 'sessions' %> - <%= link_to "Sign in", new_session_path(resource_name), class: "btn" %>
+ <%= link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: "btn" %>
<% end -%> <%- if devise_mapping.registerable? && controller_name != 'registrations' && allow_signup? %> - <%= link_to "Sign up", new_registration_path(resource_name) %>
+ <%= link_to "Sign up", new_registration_path(:user) %>
<% end -%> <%- if devise_mapping.recoverable? && controller_name != 'passwords' %> -<%= link_to "Forgot your password?", new_password_path(resource_name), class: "btn" %>
+<%= link_to "Forgot your password?", new_password_path(:user), class: "btn" %>
<% end -%> <%- if devise_mapping.confirmable? && controller_name != 'confirmations' %> - <%= link_to "Didn't receive confirmation instructions?", new_confirmation_path(resource_name) %>
+ <%= link_to "Didn't receive confirmation instructions?", new_confirmation_path(:user) %>
<% end -%> <%- if devise_mapping.lockable? && resource_class.unlock_strategy_enabled?(:email) && controller_name != 'unlocks' %> - <%= link_to "Didn't receive unlock instructions?", new_unlock_path(resource_name) %>
+ <%= link_to "Didn't receive unlock instructions?", new_unlock_path(:user) %>
<% end -%> diff --git a/app/views/devise/shared/_sign_in_link.html.haml b/app/views/devise/shared/_sign_in_link.html.haml index 289bf40f3de..77ef103cc47 100644 --- a/app/views/devise/shared/_sign_in_link.html.haml +++ b/app/views/devise/shared/_sign_in_link.html.haml @@ -1,4 +1,4 @@ %p %span.light Already have login and password? - = link_to "Sign in", new_session_path(resource_name) + = link_to "Sign in", new_session_path(:user, redirect_to_referer: 'yes') diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index 298604dee8c..2554b2688bb 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -31,4 +31,4 @@ %p %span.light Didn't receive a confirmation email? = succeed '.' do - = link_to "Request a new one", new_confirmation_path(resource_name) + = link_to "Request a new one", new_confirmation_path(:user) diff --git a/app/views/errors/omniauth_error.html.haml b/app/views/errors/omniauth_error.html.haml index 20b7fa471a0..a2a4c75daad 100644 --- a/app/views/errors/omniauth_error.html.haml +++ b/app/views/errors/omniauth_error.html.haml @@ -9,7 +9,7 @@ %p Try logging in using your username or email. If you have forgotten your password, try recovering it = link_to "Sign in", new_session_path(:user), class: 'btn primary' - = link_to "Recover password", new_password_path(resource_name), class: 'btn secondary' + = link_to "Recover password", new_password_path(:user), class: 'btn secondary' %hr %p.light If none of the options work, try contacting a GitLab administrator. diff --git a/changelogs/unreleased/39367-fix-new-email-session-path.yml b/changelogs/unreleased/39367-fix-new-email-session-path.yml new file mode 100644 index 00000000000..73485d9d1a9 --- /dev/null +++ b/changelogs/unreleased/39367-fix-new-email-session-path.yml @@ -0,0 +1,5 @@ +--- +title: Confirming email with invalid token should no longer generate an error +merge_request: 15726 +author: +type: fixed From 1a2ba2af33f8e455488c024cd11b657953797e5d Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Tue, 5 Dec 2017 11:47:45 +0000 Subject: [PATCH 09/10] Backport Extract epic nav into separate partial for easier CE => EE --- app/views/shared/issuable/_nav.html.haml | 5 +---- app/views/shared/issuable/nav_links/_all.html.haml | 6 ++++++ 2 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 app/views/shared/issuable/nav_links/_all.html.haml diff --git a/app/views/shared/issuable/_nav.html.haml b/app/views/shared/issuable/_nav.html.haml index 3f03cc7a275..6d8a4668cec 100644 --- a/app/views/shared/issuable/_nav.html.haml +++ b/app/views/shared/issuable/_nav.html.haml @@ -1,6 +1,5 @@ - type = local_assigns.fetch(:type, :issues) - page_context_word = type.to_s.humanize(capitalize: false) -- issuables = @issues || @merge_requests %ul.nav-links.issues-state-filters %li{ class: active_when(params[:state] == 'opened') }> @@ -20,6 +19,4 @@ = link_to page_filter_path(state: 'closed', label: true), id: 'state-closed', title: 'Filter by issues that are currently closed.', data: { state: 'closed' } do #{issuables_state_counter_text(type, :closed)} - %li{ class: active_when(params[:state] == 'all') }> - = link_to page_filter_path(state: 'all', label: true), id: 'state-all', title: "Show all #{page_context_word}.", data: { state: 'all' } do - #{issuables_state_counter_text(type, :all)} + = render 'shared/issuable/nav_links/all', page_context_word: page_context_word, counter: issuables_state_counter_text(type, :all) diff --git a/app/views/shared/issuable/nav_links/_all.html.haml b/app/views/shared/issuable/nav_links/_all.html.haml new file mode 100644 index 00000000000..d7ad7090a45 --- /dev/null +++ b/app/views/shared/issuable/nav_links/_all.html.haml @@ -0,0 +1,6 @@ +- page_context_word = local_assigns.fetch(:page_context_word) +- counter = local_assigns.fetch(:counter) + +%li{ class: active_when(params[:state] == 'all') }> + = link_to page_filter_path(state: 'all', label: true), id: 'state-all', title: "Show all #{page_context_word}.", data: { state: 'all' } do + #{counter} From 4b66bdfa1af8fbef5d2af94a62e2522806cc7250 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 5 Dec 2017 12:00:02 +0000 Subject: [PATCH 10/10] Second iteration of Move Kubernetes from service to Cluster page --- .../javascripts/clusters/clusters_bundle.js | 15 + app/assets/stylesheets/pages/clusters.scss | 6 + .../projects/clusters/gcp_controller.rb | 75 ++++ .../projects/clusters/user_controller.rb | 39 +++ .../projects/clusters_controller.rb | 96 ++---- app/models/clusters/cluster.rb | 6 +- app/models/clusters/platforms/kubernetes.rb | 14 + app/services/clusters/create_service.rb | 2 +- app/validators/cluster_name_validator.rb | 10 +- .../layouts/nav/sidebar/_project.html.haml | 16 +- .../clusters/_advanced_settings.html.haml | 13 +- app/views/projects/clusters/_banner.html.haml | 21 ++ .../projects/clusters/_dropdown.html.haml | 12 + .../projects/clusters/_enabled.html.haml | 16 + app/views/projects/clusters/_form.html.haml | 35 -- .../projects/clusters/gcp/_form.html.haml | 32 ++ .../clusters/{ => gcp}/_header.html.haml | 4 +- .../projects/clusters/gcp/_show.html.haml | 40 +++ .../clusters/{ => gcp}/login.html.haml | 3 +- app/views/projects/clusters/gcp/new.html.haml | 10 + app/views/projects/clusters/new.html.haml | 17 +- app/views/projects/clusters/new_gcp.html.haml | 10 - app/views/projects/clusters/show.html.haml | 56 +-- .../projects/clusters/user/_form.html.haml | 25 ++ .../projects/clusters/user/_header.html.haml | 5 + .../projects/clusters/user/_show.html.haml | 29 ++ .../projects/clusters/user/new.html.haml | 11 + .../35616-move-k8-to-cluster-page.yml | 5 + config/routes/project.rb | 12 +- .../projects/clusters/gcp_controller_spec.rb | 185 ++++++++++ .../projects/clusters/user_controller_spec.rb | 87 +++++ .../projects/clusters_controller_spec.rb | 319 +++++------------- .../clusters/{cluster.rb => clusters.rb} | 15 +- .../projects/clusters/applications_spec.rb | 107 ++++++ spec/features/projects/clusters/gcp_spec.rb | 136 ++++++++ spec/features/projects/clusters/user_spec.rb | 101 ++++++ spec/features/projects/clusters_spec.rb | 191 +---------- .../clusters/clusters_bundle_spec.js | 16 +- 38 files changed, 1160 insertions(+), 632 deletions(-) create mode 100644 app/controllers/projects/clusters/gcp_controller.rb create mode 100644 app/controllers/projects/clusters/user_controller.rb create mode 100644 app/views/projects/clusters/_banner.html.haml create mode 100644 app/views/projects/clusters/_dropdown.html.haml create mode 100644 app/views/projects/clusters/_enabled.html.haml delete mode 100644 app/views/projects/clusters/_form.html.haml create mode 100644 app/views/projects/clusters/gcp/_form.html.haml rename app/views/projects/clusters/{ => gcp}/_header.html.haml (92%) create mode 100644 app/views/projects/clusters/gcp/_show.html.haml rename app/views/projects/clusters/{ => gcp}/login.html.haml (79%) create mode 100644 app/views/projects/clusters/gcp/new.html.haml delete mode 100644 app/views/projects/clusters/new_gcp.html.haml create mode 100644 app/views/projects/clusters/user/_form.html.haml create mode 100644 app/views/projects/clusters/user/_header.html.haml create mode 100644 app/views/projects/clusters/user/_show.html.haml create mode 100644 app/views/projects/clusters/user/new.html.haml create mode 100644 changelogs/unreleased/35616-move-k8-to-cluster-page.yml create mode 100644 spec/controllers/projects/clusters/gcp_controller_spec.rb create mode 100644 spec/controllers/projects/clusters/user_controller_spec.rb rename spec/factories/clusters/{cluster.rb => clusters.rb} (56%) create mode 100644 spec/features/projects/clusters/applications_spec.rb create mode 100644 spec/features/projects/clusters/gcp_spec.rb create mode 100644 spec/features/projects/clusters/user_spec.rb diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index dc443475952..cdb5c430aa9 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -48,6 +48,7 @@ export default class Clusters { this.toggle = this.toggle.bind(this); this.installApplication = this.installApplication.bind(this); + this.showToken = this.showToken.bind(this); this.toggleButton = document.querySelector('.js-toggle-cluster'); this.toggleInput = document.querySelector('.js-toggle-input'); @@ -56,6 +57,8 @@ export default class Clusters { this.creatingContainer = document.querySelector('.js-cluster-creating'); this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason'); this.successApplicationContainer = document.querySelector('.js-cluster-application-notice'); + this.showTokenButton = document.querySelector('.js-show-cluster-token'); + this.tokenField = document.querySelector('.js-cluster-token'); initSettingsPanels(); this.initApplications(); @@ -97,11 +100,13 @@ export default class Clusters { addListeners() { this.toggleButton.addEventListener('click', this.toggle); + if (this.showTokenButton) this.showTokenButton.addEventListener('click', this.showToken); eventHub.$on('installApplication', this.installApplication); } removeListeners() { this.toggleButton.removeEventListener('click', this.toggle); + if (this.showTokenButton) this.showTokenButton.removeEventListener('click', this.showToken); eventHub.$off('installApplication', this.installApplication); } @@ -149,6 +154,16 @@ export default class Clusters { this.toggleInput.setAttribute('value', this.toggleButton.classList.contains('checked').toString()); } + showToken() { + const type = this.tokenField.getAttribute('type'); + + if (type === 'password') { + this.tokenField.setAttribute('type', 'text'); + } else { + this.tokenField.setAttribute('type', 'password'); + } + } + hideAll() { this.errorContainer.classList.add('hidden'); this.successContainer.classList.add('hidden'); diff --git a/app/assets/stylesheets/pages/clusters.scss b/app/assets/stylesheets/pages/clusters.scss index e5b9e1f2de6..83e211d6086 100644 --- a/app/assets/stylesheets/pages/clusters.scss +++ b/app/assets/stylesheets/pages/clusters.scss @@ -8,3 +8,9 @@ // Wait for the Vue to kick-in and render the applications block min-height: 302px; } + +.clusters-dropdown-menu { + max-width: 100%; +} + +@include new-style-dropdown('.clusters-dropdown '); diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb new file mode 100644 index 00000000000..b64f7a2a6bd --- /dev/null +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -0,0 +1,75 @@ +class Projects::Clusters::GcpController < Projects::ApplicationController + before_action :authorize_read_cluster! + before_action :authorize_google_api, except: [:login] + before_action :authorize_create_cluster!, only: [:new, :create] + + def login + begin + state = generate_session_key_redirect(gcp_new_namespace_project_clusters_path.to_s) + + @authorize_url = GoogleApi::CloudPlatform::Client.new( + nil, callback_google_api_auth_url, + state: state).authorize_url + rescue GoogleApi::Auth::ConfigMissingError + # no-op + end + end + + def new + @cluster = ::Clusters::Cluster.new.tap do |cluster| + cluster.build_provider_gcp + end + end + + def create + @cluster = ::Clusters::CreateService + .new(project, current_user, create_params) + .execute(token_in_session) + + if @cluster.persisted? + redirect_to project_cluster_path(project, @cluster) + else + render :new + end + end + + private + + def create_params + params.require(:cluster).permit( + :enabled, + :name, + provider_gcp_attributes: [ + :gcp_project_id, + :zone, + :num_nodes, + :machine_type + ]).merge( + provider_type: :gcp, + platform_type: :kubernetes + ) + end + + def authorize_google_api + unless GoogleApi::CloudPlatform::Client.new(token_in_session, nil) + .validate_token(expires_at_in_session) + redirect_to action: 'login' + end + end + + def token_in_session + @token_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_token] + end + + def expires_at_in_session + @expires_at_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] + end + + def generate_session_key_redirect(uri) + GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key| + session[key] = uri + end + end +end diff --git a/app/controllers/projects/clusters/user_controller.rb b/app/controllers/projects/clusters/user_controller.rb new file mode 100644 index 00000000000..d7678512073 --- /dev/null +++ b/app/controllers/projects/clusters/user_controller.rb @@ -0,0 +1,39 @@ +class Projects::Clusters::UserController < Projects::ApplicationController + before_action :authorize_read_cluster! + before_action :authorize_create_cluster!, only: [:new, :create] + + def new + @cluster = ::Clusters::Cluster.new.tap do |cluster| + cluster.build_platform_kubernetes + end + end + + def create + @cluster = ::Clusters::CreateService + .new(project, current_user, create_params) + .execute + + if @cluster.persisted? + redirect_to project_cluster_path(project, @cluster) + else + render :new + end + end + + private + + def create_params + params.require(:cluster).permit( + :enabled, + :name, + platform_kubernetes_attributes: [ + :namespace, + :api_url, + :token, + :ca_cert + ]).merge( + provider_type: :user, + platform_type: :kubernetes + ) + end +end diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 9a56c9de858..d18b6d4b78c 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -1,11 +1,12 @@ class Projects::ClustersController < Projects::ApplicationController - before_action :cluster, except: [:login, :index, :new, :new_gcp, :create] + before_action :cluster, except: [:index, :new] before_action :authorize_read_cluster! - before_action :authorize_create_cluster!, only: [:new, :new_gcp, :create] - before_action :authorize_google_api, only: [:new_gcp, :create] + before_action :authorize_create_cluster!, only: [:new] before_action :authorize_update_cluster!, only: [:update] before_action :authorize_admin_cluster!, only: [:destroy] + STATUS_POLLING_INTERVAL = 10_000 + def index if project.cluster redirect_to project_cluster_path(project, project.cluster) @@ -14,43 +15,13 @@ class Projects::ClustersController < Projects::ApplicationController end end - def login - begin - state = generate_session_key_redirect(providers_gcp_new_namespace_project_clusters_url.to_s) - - @authorize_url = GoogleApi::CloudPlatform::Client.new( - nil, callback_google_api_auth_url, - state: state).authorize_url - rescue GoogleApi::Auth::ConfigMissingError - # no-op - end - end - def new end - def new_gcp - @cluster = Clusters::Cluster.new.tap do |cluster| - cluster.build_provider_gcp - end - end - - def create - @cluster = Clusters::CreateService - .new(project, current_user, create_params) - .execute(token_in_session) - - if @cluster.persisted? - redirect_to project_cluster_path(project, @cluster) - else - render :new_gcp - end - end - def status respond_to do |format| format.json do - Gitlab::PollingInterval.set_header(response, interval: 10_000) + Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL) render json: ClusterSerializer .new(project: @project, current_user: @current_user) @@ -88,46 +59,29 @@ class Projects::ClustersController < Projects::ApplicationController private def cluster - @cluster ||= project.cluster.present(current_user: current_user) - end - - def create_params - params.require(:cluster).permit( - :enabled, - :name, - :provider_type, - provider_gcp_attributes: [ - :gcp_project_id, - :zone, - :num_nodes, - :machine_type - ]) + @cluster ||= project.clusters.find(params[:id]) + .present(current_user: current_user) end def update_params - params.require(:cluster).permit(:enabled) - end - - def authorize_google_api - unless GoogleApi::CloudPlatform::Client.new(token_in_session, nil) - .validate_token(expires_at_in_session) - redirect_to action: 'login' - end - end - - def token_in_session - @token_in_session ||= - session[GoogleApi::CloudPlatform::Client.session_key_for_token] - end - - def expires_at_in_session - @expires_at_in_session ||= - session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] - end - - def generate_session_key_redirect(uri) - GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key| - session[key] = uri + if cluster.managed? + params.require(:cluster).permit( + :enabled, + platform_kubernetes_attributes: [ + :namespace + ] + ) + else + params.require(:cluster).permit( + :enabled, + :name, + platform_kubernetes_attributes: [ + :api_url, + :token, + :ca_cert, + :namespace + ] + ) end end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 6d7fb4b7dbf..45beced1427 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -17,7 +17,7 @@ module Clusters # we force autosave to happen when we save `Cluster` model has_one :provider_gcp, class_name: 'Clusters::Providers::Gcp', autosave: true - has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes' + has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes', autosave: true has_one :application_helm, class_name: 'Clusters::Applications::Helm' has_one :application_ingress, class_name: 'Clusters::Applications::Ingress' @@ -70,6 +70,10 @@ module Clusters return platform_kubernetes if kubernetes? end + def managed? + !user? + end + def first_project return @first_project if defined?(@first_project) diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 7ab670cf1ef..9160a169452 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -34,12 +34,15 @@ module Clusters validates :api_url, url: true, presence: true validates :token, presence: true + validate :prevent_modification, on: :update + after_save :clear_reactive_cache! alias_attribute :ca_pem, :ca_cert delegate :project, to: :cluster, allow_nil: true delegate :enabled?, to: :cluster, allow_nil: true + delegate :managed?, to: :cluster, allow_nil: true alias_method :active?, :enabled? @@ -173,6 +176,17 @@ module Clusters def enforce_namespace_to_lower_case self.namespace = self.namespace&.downcase end + + def prevent_modification + return unless managed? + + if api_url_changed? || token_changed? || ca_pem_changed? + errors.add(:base, "cannot modify managed cluster") + return false + end + + true + end end end end diff --git a/app/services/clusters/create_service.rb b/app/services/clusters/create_service.rb index 1d407739b21..7b697f6d807 100644 --- a/app/services/clusters/create_service.rb +++ b/app/services/clusters/create_service.rb @@ -2,7 +2,7 @@ module Clusters class CreateService < BaseService attr_reader :access_token - def execute(access_token) + def execute(access_token = nil) @access_token = access_token create_cluster.tap do |cluster| diff --git a/app/validators/cluster_name_validator.rb b/app/validators/cluster_name_validator.rb index 13ec342f399..e7d32550176 100644 --- a/app/validators/cluster_name_validator.rb +++ b/app/validators/cluster_name_validator.rb @@ -3,11 +3,7 @@ # Custom validator for ClusterName. class ClusterNameValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - if record.user? - unless value.present? - record.errors.add(attribute, " has to be present") - end - elsif record.gcp? + if record.managed? if record.persisted? && record.name_changed? record.errors.add(attribute, " can not be changed because it's synchronized with provider") end @@ -19,6 +15,10 @@ class ClusterNameValidator < ActiveModel::EachValidator unless value =~ Gitlab::Regex.kubernetes_namespace_regex record.errors.add(attribute, Gitlab::Regex.kubernetes_namespace_regex_message) end + else + unless value.present? + record.errors.add(attribute, " has to be present") + end end end end diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 2ce960df13c..8b2d2a5c74d 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -146,7 +146,7 @@ = number_with_delimiter(@project.open_merge_requests_count) - if project_nav_tab? :pipelines - = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts, :clusters]) do + = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts, :clusters, :user, :gcp]) do = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines' do .nav-icon-container = sprite_icon('pipeline') @@ -154,7 +154,7 @@ CI / CD %ul.sidebar-sub-level-items - = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts], html_options: { class: "fly-out-top-item" } ) do + = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts, :clusters, :user, :gcp], html_options: { class: "fly-out-top-item" } ) do = link_to project_pipelines_path(@project) do %strong.fly-out-top-item-name #{ _('CI / CD') } @@ -183,18 +183,18 @@ %span Environments + - if project_nav_tab? :clusters + = nav_link(controller: [:clusters, :user, :gcp]) do + = link_to project_clusters_path(@project), title: 'Cluster', class: 'shortcuts-cluster' do + %span + Cluster + - if @project.feature_available?(:builds, current_user) && !@project.empty_repo? = nav_link(path: 'pipelines#charts') do = link_to charts_project_pipelines_path(@project), title: 'Charts', class: 'shortcuts-pipelines-charts' do %span Charts - - if project_nav_tab? :clusters - = nav_link(controller: :clusters) do - = link_to project_clusters_path(@project), title: 'Cluster', class: 'shortcuts-cluster' do - %span - Cluster - - if project_nav_tab? :wiki = nav_link(controller: :wikis) do = link_to get_project_wiki_path(@project), class: 'shortcuts-wiki' do diff --git a/app/views/projects/clusters/_advanced_settings.html.haml b/app/views/projects/clusters/_advanced_settings.html.haml index 97532f1e2bd..2b3095eb94b 100644 --- a/app/views/projects/clusters/_advanced_settings.html.haml +++ b/app/views/projects/clusters/_advanced_settings.html.haml @@ -1,10 +1,11 @@ - if can?(current_user, :admin_cluster, @cluster) - .append-bottom-20 - %label.append-bottom-10 - = s_('ClusterIntegration|Google Container Engine') - %p - - link_gke = link_to(s_('ClusterIntegration|Google Container Engine'), @cluster.gke_cluster_url, target: '_blank', rel: 'noopener noreferrer') - = s_('ClusterIntegration|Manage your cluster by visiting %{link_gke}').html_safe % { link_gke: link_gke } + - if @cluster.managed? + .append-bottom-20 + %label.append-bottom-10 + = s_('ClusterIntegration|Google Container Engine') + %p + - link_gke = link_to(s_('ClusterIntegration|Google Container Engine'), @cluster.gke_cluster_url, target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Manage your cluster by visiting %{link_gke}').html_safe % { link_gke: link_gke } .well.form-group %label.text-danger diff --git a/app/views/projects/clusters/_banner.html.haml b/app/views/projects/clusters/_banner.html.haml new file mode 100644 index 00000000000..a1cc66eac92 --- /dev/null +++ b/app/views/projects/clusters/_banner.html.haml @@ -0,0 +1,21 @@ +%h4= s_('ClusterIntegration|Enable cluster integration') +.settings-content + + .hidden.js-cluster-error.alert.alert-danger.alert-block.append-bottom-10{ role: 'alert' } + = s_('ClusterIntegration|Something went wrong while creating your cluster on Google Container Engine') + %p.js-error-reason + + .hidden.js-cluster-creating.alert.alert-info.alert-block.append-bottom-10{ role: 'alert' } + = s_('ClusterIntegration|Cluster is being created on Google Container Engine...') + + .hidden.js-cluster-success.alert.alert-success.alert-block.append-bottom-10{ role: 'alert' } + = s_('ClusterIntegration|Cluster was successfully created on Google Container Engine. Refresh the page to see cluster\'s details') + + %p + - if @cluster.enabled? + - if can?(current_user, :update_cluster, @cluster) + = s_('ClusterIntegration|Cluster integration is enabled for this project. Disabling this integration will not affect your cluster, it will only temporarily turn off GitLab\'s connection to it.') + - else + = s_('ClusterIntegration|Cluster integration is enabled for this project.') + - else + = s_('ClusterIntegration|Cluster integration is disabled for this project.') diff --git a/app/views/projects/clusters/_dropdown.html.haml b/app/views/projects/clusters/_dropdown.html.haml new file mode 100644 index 00000000000..39188c7ca27 --- /dev/null +++ b/app/views/projects/clusters/_dropdown.html.haml @@ -0,0 +1,12 @@ +%h4.prepend-top-0= s_('ClusterIntegration|Choose how to set up cluster integration') + +.dropdown.clusters-dropdown + %button.dropdown-menu-toggle.dropdown-menu-full-width{ type: 'button', data: { toggle: 'dropdown' }, 'aria-haspopup': true, 'aria-expanded': false } + %span.dropdown-toggle-text + = dropdown_text + = icon('chevron-down') + %ul.dropdown-menu.clusters-dropdown-menu.dropdown-menu-full-width + %li + = link_to(s_('ClusterIntegration|Create cluster on Google Container Engine'), gcp_new_namespace_project_clusters_path(@project.namespace, @project)) + %li + = link_to(s_('ClusterIntegration|Add an existing cluster'), user_new_namespace_project_clusters_path(@project.namespace, @project)) diff --git a/app/views/projects/clusters/_enabled.html.haml b/app/views/projects/clusters/_enabled.html.haml new file mode 100644 index 00000000000..f4d261df8f5 --- /dev/null +++ b/app/views/projects/clusters/_enabled.html.haml @@ -0,0 +1,16 @@ += form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| + = form_errors(@cluster) + .form-group.append-bottom-20 + %label.append-bottom-10 + = field.hidden_field :enabled, { class: 'js-toggle-input'} + + %button{ type: 'button', + class: "js-toggle-cluster project-feature-toggle #{'checked' unless !@cluster.enabled?} #{'disabled' unless can?(current_user, :update_cluster, @cluster)}", + 'aria-label': s_('ClusterIntegration|Toggle Cluster'), + disabled: !can?(current_user, :update_cluster, @cluster), + data: { 'enabled-text': 'Enabled', 'disabled-text': 'Disabled' } } + + - if can?(current_user, :update_cluster, @cluster) + .form-group + = field.submit _('Save'), class: 'btn btn-success' + diff --git a/app/views/projects/clusters/_form.html.haml b/app/views/projects/clusters/_form.html.haml deleted file mode 100644 index 1f8ae463d0f..00000000000 --- a/app/views/projects/clusters/_form.html.haml +++ /dev/null @@ -1,35 +0,0 @@ -.row - .col-sm-8.col-sm-offset-4 - %p - - link_to_help_page = link_to(s_('ClusterIntegration|help page'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer') - = s_('ClusterIntegration|Read our %{link_to_help_page} on cluster integration.').html_safe % { link_to_help_page: link_to_help_page} - - = form_for @cluster, url: namespace_project_clusters_path(@project.namespace, @project, @cluster), as: :cluster do |field| - = field.hidden_field :provider_type, value: :gcp - = form_errors(@cluster) - .form-group - = field.label :name, s_('ClusterIntegration|Cluster name') - = field.text_field :name, class: 'form-control' - - = field.fields_for :provider_gcp, @cluster.provider_gcp do |provider_gcp_field| - .form-group - = provider_gcp_field.label :gcp_project_id, s_('ClusterIntegration|Google Cloud Platform project ID') - = link_to(s_('ClusterIntegration|See your projects'), 'https://console.cloud.google.com/home/dashboard', target: '_blank', rel: 'noopener noreferrer') - = provider_gcp_field.text_field :gcp_project_id, class: 'form-control' - - .form-group - = provider_gcp_field.label :zone, s_('ClusterIntegration|Zone') - = link_to(s_('ClusterIntegration|See zones'), 'https://cloud.google.com/compute/docs/regions-zones/regions-zones', target: '_blank', rel: 'noopener noreferrer') - = provider_gcp_field.text_field :zone, class: 'form-control', placeholder: 'us-central1-a' - - .form-group - = provider_gcp_field.label :num_nodes, s_('ClusterIntegration|Number of nodes') - = provider_gcp_field.text_field :num_nodes, class: 'form-control', placeholder: '3' - - .form-group - = provider_gcp_field.label :machine_type, s_('ClusterIntegration|Machine type') - = link_to(s_('ClusterIntegration|See machine types'), 'https://cloud.google.com/compute/docs/machine-types', target: '_blank', rel: 'noopener noreferrer') - = provider_gcp_field.text_field :machine_type, class: 'form-control', placeholder: 'n1-standard-2' - - .form-group - = field.submit s_('ClusterIntegration|Create cluster'), class: 'btn btn-save' diff --git a/app/views/projects/clusters/gcp/_form.html.haml b/app/views/projects/clusters/gcp/_form.html.haml new file mode 100644 index 00000000000..0f6bae97571 --- /dev/null +++ b/app/views/projects/clusters/gcp/_form.html.haml @@ -0,0 +1,32 @@ +%p + - link_to_help_page = link_to(s_('ClusterIntegration|help page'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Read our %{link_to_help_page} on cluster integration.').html_safe % { link_to_help_page: link_to_help_page} + += form_for @cluster, html: { class: 'prepend-top-20' }, url: gcp_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field| + = form_errors(@cluster) + .form-group + = field.label :name, s_('ClusterIntegration|Cluster name') + = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name') + + = field.fields_for :provider_gcp, @cluster.provider_gcp do |provider_gcp_field| + .form-group + = provider_gcp_field.label :gcp_project_id, s_('ClusterIntegration|Google Cloud Platform project ID') + = link_to(s_('ClusterIntegration|See your projects'), 'https://console.cloud.google.com/home/dashboard', target: '_blank', rel: 'noopener noreferrer') + = provider_gcp_field.text_field :gcp_project_id, class: 'form-control', placeholder: s_('ClusterIntegration|Project ID') + + .form-group + = provider_gcp_field.label :zone, s_('ClusterIntegration|Zone') + = link_to(s_('ClusterIntegration|See zones'), 'https://cloud.google.com/compute/docs/regions-zones/regions-zones', target: '_blank', rel: 'noopener noreferrer') + = provider_gcp_field.text_field :zone, class: 'form-control', placeholder: 'us-central1-a' + + .form-group + = provider_gcp_field.label :num_nodes, s_('ClusterIntegration|Number of nodes') + = provider_gcp_field.text_field :num_nodes, class: 'form-control', placeholder: '3' + + .form-group + = provider_gcp_field.label :machine_type, s_('ClusterIntegration|Machine type') + = link_to(s_('ClusterIntegration|See machine types'), 'https://cloud.google.com/compute/docs/machine-types', target: '_blank', rel: 'noopener noreferrer') + = provider_gcp_field.text_field :machine_type, class: 'form-control', placeholder: 'n1-standard-4' + + .form-group + = field.submit s_('ClusterIntegration|Create cluster'), class: 'btn btn-success' diff --git a/app/views/projects/clusters/_header.html.haml b/app/views/projects/clusters/gcp/_header.html.haml similarity index 92% rename from app/views/projects/clusters/_header.html.haml rename to app/views/projects/clusters/gcp/_header.html.haml index beb798e7154..cddb53c2688 100644 --- a/app/views/projects/clusters/_header.html.haml +++ b/app/views/projects/clusters/gcp/_header.html.haml @@ -1,5 +1,5 @@ -%h4.prepend-top-0 - = s_('ClusterIntegration|Create new cluster on Google Container Engine') +%h4.prepend-top-20 + = s_('ClusterIntegration|Enter the details for your cluster') %p = s_('ClusterIntegration|Please make sure that your Google account meets the following requirements:') %ul diff --git a/app/views/projects/clusters/gcp/_show.html.haml b/app/views/projects/clusters/gcp/_show.html.haml new file mode 100644 index 00000000000..3fa9f69708a --- /dev/null +++ b/app/views/projects/clusters/gcp/_show.html.haml @@ -0,0 +1,40 @@ +.form-group + %label.append-bottom-10{ for: 'cluster-name' } + = s_('ClusterIntegration|Cluster name') + .input-group + %input.form-control.cluster-name.js-select-on-focus{ value: @cluster.name, readonly: true } + %span.input-group-btn + = clipboard_button(text: @cluster.name, title: s_('ClusterIntegration|Copy cluster name'), class: 'btn-default') + += form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| + = form_errors(@cluster) + = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| + .form-group + = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL') + .input-group + = platform_kubernetes_field.text_field :api_url, class: 'form-control js-select-on-focus', placeholder: s_('ClusterIntegration|API URL'), readonly: true + %span.input-group-btn + = clipboard_button(text: @cluster.platform_kubernetes.api_url, title: s_('ClusterIntegration|Copy API URL'), class: 'btn-default') + + .form-group + = platform_kubernetes_field.label :ca_cert, s_('ClusterIntegration|CA Certificate') + .input-group + = platform_kubernetes_field.text_area :ca_cert, class: 'form-control js-select-on-focus', placeholder: s_('ClusterIntegration|Certificate Authority bundle (PEM format)'), readonly: true + %span.input-group-addon.clipboard-addon + = clipboard_button(text: @cluster.platform_kubernetes.ca_cert, title: s_('ClusterIntegration|Copy CA Certificate'), class: 'btn-blank') + + .form-group + = platform_kubernetes_field.label :token, s_('ClusterIntegration|Token') + .input-group + = platform_kubernetes_field.text_field :token, class: 'form-control js-cluster-token js-select-on-focus', type: 'password', placeholder: s_('ClusterIntegration|Token'), readonly: true + %span.input-group-btn + %button.btn.btn-default.js-show-cluster-token{ type: 'button' } + = s_('ClusterIntegration|Show') + = clipboard_button(text: @cluster.platform_kubernetes.token, title: s_('ClusterIntegration|Copy Token'), class: 'btn-default') + + .form-group + = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)') + = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') + + .form-group + = field.submit s_('ClusterIntegration|Save changes'), class: 'btn btn-success' diff --git a/app/views/projects/clusters/login.html.haml b/app/views/projects/clusters/gcp/login.html.haml similarity index 79% rename from app/views/projects/clusters/login.html.haml rename to app/views/projects/clusters/gcp/login.html.haml index fde030b500b..790ba61fd86 100644 --- a/app/views/projects/clusters/login.html.haml +++ b/app/views/projects/clusters/gcp/login.html.haml @@ -3,8 +3,9 @@ .row.prepend-top-default .col-sm-4 - = render 'sidebar' + = render 'projects/clusters/sidebar' .col-sm-8 + = render 'projects/clusters/dropdown', dropdown_text: s_('ClusterIntegration|Create cluster on Google Container Engine') = render 'header' .row .col-sm-8.col-sm-offset-4.signin-with-google diff --git a/app/views/projects/clusters/gcp/new.html.haml b/app/views/projects/clusters/gcp/new.html.haml new file mode 100644 index 00000000000..9a79480c82f --- /dev/null +++ b/app/views/projects/clusters/gcp/new.html.haml @@ -0,0 +1,10 @@ +- breadcrumb_title "Cluster" +- page_title _("New Cluster") + +.row.prepend-top-default + .col-sm-4 + = render 'projects/clusters/sidebar' + .col-sm-8 + = render 'projects/clusters/dropdown', dropdown_text: s_('ClusterIntegration|Create cluster on Google Container Engine') + = render 'header' + = render 'form' diff --git a/app/views/projects/clusters/new.html.haml b/app/views/projects/clusters/new.html.haml index 665120c7e49..2e5bc34f64a 100644 --- a/app/views/projects/clusters/new.html.haml +++ b/app/views/projects/clusters/new.html.haml @@ -5,16 +5,9 @@ .col-sm-4 = render 'sidebar' .col-sm-8 - - if @project.deployment_platform&.active? - %h4.prepend-top-0= s_('ClusterIntegration|Cluster management') + %h4.prepend-top-0= s_('ClusterIntegration|Choose how to set up cluster integration') - %p= s_('ClusterIntegration|A cluster has been set up on this project through the Kubernetes integration page') - = link_to s_('ClusterIntegration|Manage Kubernetes integration'), edit_project_service_path(@project, :kubernetes), class: 'btn append-bottom-20' - - - else - %h4.prepend-top-0= s_('ClusterIntegration|Choose how to set up cluster integration') - - %p= s_('ClusterIntegration|Create a new cluster on Google Container Engine right from GitLab') - = link_to s_('ClusterIntegration|Create on GKE'), providers_gcp_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' - %p= s_('ClusterIntegration|Enter the details for an existing Kubernetes cluster') - = link_to s_('ClusterIntegration|Add an existing cluster'), edit_project_service_path(@project, :kubernetes), class: 'btn append-bottom-20' + %p= s_('ClusterIntegration|Create a new cluster on Google Engine right from GitLab') + = link_to s_('ClusterIntegration|Create on GKE'), gcp_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' + %p= s_('ClusterIntegration|Enter the details for an existing Kubernetes cluster') + = link_to s_('ClusterIntegration|Add an existing cluster'), user_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' diff --git a/app/views/projects/clusters/new_gcp.html.haml b/app/views/projects/clusters/new_gcp.html.haml deleted file mode 100644 index 48e6b6ae8e8..00000000000 --- a/app/views/projects/clusters/new_gcp.html.haml +++ /dev/null @@ -1,10 +0,0 @@ -- breadcrumb_title "Cluster" -- page_title _("New Cluster") - -.row.prepend-top-default - .col-sm-4 - = render 'sidebar' - .col-sm-8 - = render 'header' - -= render 'form' diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index b7671f5e3c4..d23efe4d9aa 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -13,52 +13,16 @@ cluster_status_reason: @cluster.status_reason, help_path: help_page_path('user/project/clusters/index.md', anchor: 'installing-applications') } } - .js-cluster-application-notice .flash-container %section.settings.no-animate.expanded - %h4= s_('ClusterIntegration|Enable cluster integration') - .settings-content - - .hidden.js-cluster-error.alert.alert-danger.alert-block.append-bottom-10{ role: 'alert' } - = s_('ClusterIntegration|Something went wrong while creating your cluster on Google Container Engine') - %p.js-error-reason - - .hidden.js-cluster-creating.alert.alert-info.alert-block.append-bottom-10{ role: 'alert' } - = s_('ClusterIntegration|Cluster is being created on Google Container Engine...') - - .hidden.js-cluster-success.alert.alert-success.alert-block.append-bottom-10{ role: 'alert' } - = s_('ClusterIntegration|Cluster was successfully created on Google Container Engine') - - %p - - if @cluster.enabled? - - if can?(current_user, :update_cluster, @cluster) - = s_('ClusterIntegration|Cluster integration is enabled for this project. Disabling this integration will not affect your cluster, it will only temporarily turn off GitLab\'s connection to it.') - - else - = s_('ClusterIntegration|Cluster integration is enabled for this project.') - - else - = s_('ClusterIntegration|Cluster integration is disabled for this project.') - - = form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| - = form_errors(@cluster) - .form-group.append-bottom-20 - %label.append-bottom-10 - = field.hidden_field :enabled, { class: 'js-toggle-input'} - - %button{ type: 'button', - class: "js-toggle-cluster project-feature-toggle #{'checked' unless !@cluster.enabled?} #{'disabled' unless can?(current_user, :update_cluster, @cluster)}", - 'aria-label': s_('ClusterIntegration|Toggle Cluster'), - disabled: !can?(current_user, :update_cluster, @cluster), - data: { 'enabled-text': 'Enabled', 'disabled-text': 'Disabled' } } - - - if can?(current_user, :update_cluster, @cluster) - .form-group - = field.submit _('Save'), class: 'btn btn-success' + = render 'banner' + = render 'enabled' .cluster-applications-table#js-cluster-applications - %section.settings#js-cluster-details + %section.settings#js-cluster-details{ class: ('expanded' if expanded) } .settings-header %h4= s_('ClusterIntegration|Cluster details') %button.btn.js-settings-toggle @@ -66,20 +30,16 @@ %p= s_('ClusterIntegration|See and edit the details for your cluster') .settings-content - - .form_group.append-bottom-20 - %label.append-bottom-10{ for: 'cluster-name' } - = s_('ClusterIntegration|Cluster name') - .input-group - %input.form-control.cluster-name{ value: @cluster.name, disabled: true } - %span.input-group-addon.clipboard-addon - = clipboard_button(text: @cluster.name, title: s_('ClusterIntegration|Copy cluster name')) + - if @cluster.managed? + = render 'projects/clusters/gcp/show' + - else + = render 'projects/clusters/user/show' %section.settings.no-animate#js-cluster-advanced-settings{ class: ('expanded' if expanded) } .settings-header %h4= _('Advanced settings') %button.btn.js-settings-toggle = expanded ? 'Collapse' : 'Expand' - %p= s_('ClusterIntegration|Manage Cluster integration on your GitLab project') + %p= s_('ClusterIntegration|Manage cluster integration on your GitLab project') .settings-content = render 'advanced_settings' diff --git a/app/views/projects/clusters/user/_form.html.haml b/app/views/projects/clusters/user/_form.html.haml new file mode 100644 index 00000000000..4a9bd5186c6 --- /dev/null +++ b/app/views/projects/clusters/user/_form.html.haml @@ -0,0 +1,25 @@ += form_for @cluster, url: user_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field| + = form_errors(@cluster) + .form-group + = field.label :name, s_('ClusterIntegration|Cluster name') + = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name') + + = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| + .form-group + = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL') + = platform_kubernetes_field.text_field :api_url, class: 'form-control', placeholder: s_('ClusterIntegration|API URL') + + .form-group + = platform_kubernetes_field.label :ca_cert, s_('ClusterIntegration|CA Certificate') + = platform_kubernetes_field.text_area :ca_cert, class: 'form-control', placeholder: s_('ClusterIntegration|Certificate Authority bundle (PEM format)') + + .form-group + = platform_kubernetes_field.label :token, s_('ClusterIntegration|Token') + = platform_kubernetes_field.text_field :token, class: 'form-control', placeholder: s_('ClusterIntegration|Service token'), autocomplete: 'off' + + .form-group + = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)') + = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') + + .form-group + = field.submit s_('ClusterIntegration|Add cluster'), class: 'btn btn-success' diff --git a/app/views/projects/clusters/user/_header.html.haml b/app/views/projects/clusters/user/_header.html.haml new file mode 100644 index 00000000000..06ac210a06d --- /dev/null +++ b/app/views/projects/clusters/user/_header.html.haml @@ -0,0 +1,5 @@ +%h4.prepend-top-20 + = s_('ClusterIntegration|Enter the details for your cluster') +%p + - link_to_help_page = link_to(s_('ClusterIntegration|documentation'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Please enter access information for your cluster. If you need help, you can read our %{link_to_help_page} on clusters').html_safe % { link_to_help_page: link_to_help_page } diff --git a/app/views/projects/clusters/user/_show.html.haml b/app/views/projects/clusters/user/_show.html.haml new file mode 100644 index 00000000000..5931e0b7f17 --- /dev/null +++ b/app/views/projects/clusters/user/_show.html.haml @@ -0,0 +1,29 @@ += form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| + = form_errors(@cluster) + .form-group + = field.label :name, s_('ClusterIntegration|Cluster name') + = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name') + + = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| + .form-group + = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL') + = platform_kubernetes_field.text_field :api_url, class: 'form-control', placeholder: s_('ClusterIntegration|API URL') + + .form-group + = platform_kubernetes_field.label :ca_cert, s_('ClusterIntegration|CA Certificate') + = platform_kubernetes_field.text_area :ca_cert, class: 'form-control', placeholder: s_('ClusterIntegration|Certificate Authority bundle (PEM format)') + + .form-group + = platform_kubernetes_field.label :token, s_('ClusterIntegration|Token') + .input-group + = platform_kubernetes_field.text_field :token, class: 'form-control js-cluster-token', type: 'password', placeholder: s_('ClusterIntegration|Token'), autocomplete: 'off' + %span.input-group-addon.clipboard-addon + %button.js-show-cluster-token.btn-blank{ type: 'button' } + = s_('ClusterIntegration|Show') + + .form-group + = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)') + = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') + + .form-group + = field.submit s_('ClusterIntegration|Save changes'), class: 'btn btn-success' diff --git a/app/views/projects/clusters/user/new.html.haml b/app/views/projects/clusters/user/new.html.haml new file mode 100644 index 00000000000..68f38f83453 --- /dev/null +++ b/app/views/projects/clusters/user/new.html.haml @@ -0,0 +1,11 @@ +- breadcrumb_title "Cluster" +- page_title _("New Cluster") + +.row.prepend-top-default + .col-sm-4 + = render 'projects/clusters/sidebar' + .col-sm-8 + = render 'projects/clusters/dropdown', dropdown_text: s_('ClusterIntegration|Add an existing cluster') + = render 'header' + .prepend-top-20 + = render 'form' diff --git a/changelogs/unreleased/35616-move-k8-to-cluster-page.yml b/changelogs/unreleased/35616-move-k8-to-cluster-page.yml new file mode 100644 index 00000000000..032a39608ce --- /dev/null +++ b/changelogs/unreleased/35616-move-k8-to-cluster-page.yml @@ -0,0 +1,5 @@ +--- +title: Create a new form to add Existing Kubernetes Cluster +merge_request: 14805 +author: +type: added diff --git a/config/routes/project.rb b/config/routes/project.rb index bdafaba3ab3..45474034822 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -183,10 +183,16 @@ constraints(ProjectUrlConstrainer.new) do end end - resources :clusters, except: [:edit] do + resources :clusters, except: [:edit, :create] do collection do - get :login - get '/providers/gcp/new', action: :new_gcp + scope :providers do + get '/user/new', to: 'clusters/user#new' + post '/user', to: 'clusters/user#create' + + get '/gcp/new', to: 'clusters/gcp#new' + get '/gcp/login', to: 'clusters/gcp#login' + post '/gcp', to: 'clusters/gcp#create' + end end member do diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb new file mode 100644 index 00000000000..bb5ef7bb365 --- /dev/null +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -0,0 +1,185 @@ +require 'spec_helper' + +describe Projects::Clusters::GcpController do + include AccessMatchersForController + include GoogleApi::CloudPlatformHelpers + + set(:project) { create(:project) } + + describe 'GET login' do + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when omniauth has been configured' do + let(:key) { 'secret-key' } + + let(:session_key_for_redirect_uri) do + GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key) + end + + before do + allow(SecureRandom).to receive(:hex).and_return(key) + end + + it 'has authorize_url' do + go + + expect(assigns(:authorize_url)).to include(key) + expect(session[session_key_for_redirect_uri]).to eq(gcp_new_project_clusters_path(project)) + end + end + + context 'when omniauth has not configured' do + before do + stub_omniauth_setting(providers: []) + end + + it 'does not have authorize_url' do + go + + expect(assigns(:authorize_url)).to be_nil + end + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + get :login, namespace_id: project.namespace, project_id: project + end + end + + describe 'GET new' do + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when access token is valid' do + before do + stub_google_api_validate_token + end + + it 'has new object' do + go + + expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) + end + end + + context 'when access token is expired' do + before do + stub_google_api_expired_token + end + + it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } + end + + context 'when access token is not stored in session' do + it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + get :new, namespace_id: project.namespace, project_id: project + end + end + + describe 'POST create' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_gcp_attributes: { + gcp_project_id: '111' + } + } + } + end + + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when access token is valid' do + before do + stub_google_api_validate_token + end + + context 'when creates a cluster on gke' do + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { go }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Gcp.count } + expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + expect(project.cluster).to be_gcp + expect(project.cluster).to be_kubernetes + end + end + end + + context 'when access token is expired' do + before do + stub_google_api_expired_token + end + + it 'redirects to login page' do + expect(go).to redirect_to(gcp_login_project_clusters_path(project)) + end + end + + context 'when access token is not stored in session' do + it 'redirects to login page' do + expect(go).to redirect_to(gcp_login_project_clusters_path(project)) + end + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + post :create, params.merge(namespace_id: project.namespace, project_id: project) + end + end +end diff --git a/spec/controllers/projects/clusters/user_controller_spec.rb b/spec/controllers/projects/clusters/user_controller_spec.rb new file mode 100644 index 00000000000..22005e0dc66 --- /dev/null +++ b/spec/controllers/projects/clusters/user_controller_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +describe Projects::Clusters::UserController do + include AccessMatchersForController + + set(:project) { create(:project) } + + describe 'GET new' do + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + it 'has new object' do + go + + expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + get :new, namespace_id: project.namespace, project_id: project + end + end + + describe 'POST create' do + let(:params) do + { + cluster: { + name: 'new-cluster', + platform_kubernetes_attributes: { + api_url: 'http://my-url', + token: 'test', + namespace: 'aaa' + } + } + } + end + + describe 'functionality' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + 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.cluster)) + end + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + post :create, params.merge(namespace_id: project.namespace, project_id: project) + end + end +end diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index ca2bcb2b5ae..66e67652dad 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -4,6 +4,8 @@ describe Projects::ClustersController do include AccessMatchersForController include GoogleApi::CloudPlatformHelpers + set(:project) { create(:project) } + describe 'GET index' do describe 'functionality' do let(:user) { create(:user) } @@ -14,22 +16,18 @@ describe Projects::ClustersController do end context 'when project has a cluster' do - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } + let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } it { expect(go).to redirect_to(project_cluster_path(project, project.cluster)) } end context 'when project does not have a cluster' do - let(:project) { create(:project) } - it { expect(go).to redirect_to(new_project_cluster_path(project)) } end end describe 'security' do - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(project) } @@ -46,198 +44,8 @@ describe Projects::ClustersController do end end - describe 'GET login' do - let(:project) { create(:project) } - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - context 'when omniauth has been configured' do - let(:key) { 'secere-key' } - - let(:session_key_for_redirect_uri) do - GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key) - end - - before do - allow(SecureRandom).to receive(:hex).and_return(key) - end - - it 'has authorize_url' do - go - - expect(assigns(:authorize_url)).to include(key) - expect(session[session_key_for_redirect_uri]).to eq(providers_gcp_new_project_clusters_url(project)) - end - end - - context 'when omniauth has not configured' do - before do - stub_omniauth_setting(providers: []) - end - - it 'does not have authorize_url' do - go - - expect(assigns(:authorize_url)).to be_nil - end - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - get :login, namespace_id: project.namespace, project_id: project - end - end - - shared_examples 'requires to login' do - it 'redirects to create a cluster' do - subject - - expect(response).to redirect_to(login_project_clusters_path(project)) - end - end - - describe 'GET new_gcp' do - let(:project) { create(:project) } - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - context 'when access token is valid' do - before do - stub_google_api_validate_token - end - - it 'has new object' do - go - - expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) - end - end - - context 'when access token is expired' do - before do - stub_google_api_expired_token - end - - it { expect(go).to redirect_to(login_project_clusters_path(project)) } - end - - context 'when access token is not stored in session' do - it { expect(go).to redirect_to(login_project_clusters_path(project)) } - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - get :new_gcp, namespace_id: project.namespace, project_id: project - end - end - - describe 'POST create' do - let(:project) { create(:project) } - - let(:params) do - { - cluster: { - name: 'new-cluster', - provider_type: :gcp, - provider_gcp_attributes: { - gcp_project_id: '111' - } - } - } - end - - describe 'functionality' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - context 'when access token is valid' do - before do - stub_google_api_validate_token - end - - context 'when creates a cluster on gke' do - it 'creates a new cluster' do - expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } - expect(response).to redirect_to(project_cluster_path(project, project.cluster)) - end - end - end - - context 'when access token is expired' do - before do - stub_google_api_expired_token - end - - it 'redirects to login page' do - expect(go).to redirect_to(login_project_clusters_path(project)) - end - end - - context 'when access token is not stored in session' do - it 'redirects to login page' do - expect(go).to redirect_to(login_project_clusters_path(project)) - end - end - end - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - post :create, params.merge(namespace_id: project.namespace, project_id: project) - end - end - describe 'GET status' do - let(:cluster) { create(:cluster, :project, :providing_by_gcp) } - let(:project) { cluster.project } + let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } describe 'functionality' do let(:user) { create(:user) } @@ -275,8 +83,7 @@ describe Projects::ClustersController do end describe 'GET show' do - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } describe 'functionality' do let(:user) { create(:user) } @@ -313,10 +120,8 @@ describe Projects::ClustersController do end describe 'PUT update' do - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - - describe 'functionality' do + context 'when cluster is provided by GCP' do + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } let(:user) { create(:user) } before do @@ -324,10 +129,16 @@ describe Projects::ClustersController do sign_in(user) end - context 'when update enabled' do + context 'when changing parameters' do let(:params) do { - cluster: { enabled: false } + cluster: { + enabled: false, + name: 'my-new-cluster-name', + platform_kubernetes_attributes: { + namespace: 'my-namespace' + } + } } end @@ -340,8 +151,14 @@ describe Projects::ClustersController do expect(cluster.enabled).to be_falsey end + it "does not change cluster name" do + go + + expect(cluster.name).to eq('test-cluster') + end + context 'when cluster is being created' do - let(:cluster) { create(:cluster, :project, :providing_by_gcp) } + let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } it "rejects changes" do go @@ -354,11 +171,46 @@ describe Projects::ClustersController do end end + context 'when cluster is provided by user' do + let(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'when changing parameters' do + let(:params) do + { + cluster: { + enabled: false, + name: 'my-new-cluster-name', + platform_kubernetes_attributes: { + namespace: 'my-namespace' + } + } + } + end + + it "updates and redirects back to show page" do + go + + cluster.reload + expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + expect(flash[:notice]).to eq('Cluster was successfully updated.') + expect(cluster.enabled).to be_falsey + expect(cluster.name).to eq('my-new-cluster-name') + expect(cluster.platform_kubernetes.namespace).to eq('my-namespace') + end + end + end + describe 'security' do + set(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + let(:params) do - { - cluster: { enabled: false } - } + { cluster: { enabled: false } } end it { expect { go }.to be_allowed_for(:admin) } @@ -378,10 +230,7 @@ describe Projects::ClustersController do end end - describe 'delete update' do - let(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - + describe 'DELETE destroy' do describe 'functionality' do let(:user) { create(:user) } @@ -390,31 +239,37 @@ describe Projects::ClustersController do sign_in(user) end - it "destroys and redirects back to clusters list" do - expect { go } - .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Platforms::Kubernetes.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(-1) + context 'when cluster is provided by GCP' do + context 'when cluster is created' do + let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - expect(response).to redirect_to(project_clusters_path(project)) - expect(flash[:notice]).to eq('Cluster integration was successfully removed.') - end + it "destroys and redirects back to clusters list" do + expect { go } + .to change { Clusters::Cluster.count }.by(-1) + .and change { Clusters::Platforms::Kubernetes.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) - context 'when cluster is being created' do - let(:cluster) { create(:cluster, :project, :providing_by_gcp) } + expect(response).to redirect_to(project_clusters_path(project)) + expect(flash[:notice]).to eq('Cluster integration was successfully removed.') + end + end - it "destroys and redirects back to clusters list" do - expect { go } - .to change { Clusters::Cluster.count }.by(-1) - .and change { Clusters::Providers::Gcp.count }.by(-1) + context 'when cluster is being created' do + let!(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } - expect(response).to redirect_to(project_clusters_path(project)) - expect(flash[:notice]).to eq('Cluster integration was successfully removed.') + it "destroys and redirects back to clusters list" do + expect { go } + .to change { Clusters::Cluster.count }.by(-1) + .and change { Clusters::Providers::Gcp.count }.by(-1) + + expect(response).to redirect_to(project_clusters_path(project)) + expect(flash[:notice]).to eq('Cluster integration was successfully removed.') + end end end - context 'when provider is user' do - let(:cluster) { create(:cluster, :project, :provided_by_user) } + context 'when cluster is provided by user' do + let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } it "destroys and redirects back to clusters list" do expect { go } @@ -429,6 +284,8 @@ describe Projects::ClustersController do end describe 'security' do + set(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(project) } it { expect { go }.to be_allowed_for(:master).of(project) } diff --git a/spec/factories/clusters/cluster.rb b/spec/factories/clusters/clusters.rb similarity index 56% rename from spec/factories/clusters/cluster.rb rename to spec/factories/clusters/clusters.rb index c4261178f2d..81866845a20 100644 --- a/spec/factories/clusters/cluster.rb +++ b/spec/factories/clusters/clusters.rb @@ -13,27 +13,20 @@ FactoryGirl.define do provider_type :user platform_type :kubernetes - platform_kubernetes do - create(:cluster_platform_kubernetes, :configured) - end + platform_kubernetes factory: [:cluster_platform_kubernetes, :configured] end trait :provided_by_gcp do provider_type :gcp platform_type :kubernetes - before(:create) do |cluster, evaluator| - cluster.platform_kubernetes = build(:cluster_platform_kubernetes, :configured) - cluster.provider_gcp = build(:cluster_provider_gcp, :created) - end + provider_gcp factory: [:cluster_provider_gcp, :created] + platform_kubernetes factory: [:cluster_platform_kubernetes, :configured] end trait :providing_by_gcp do provider_type :gcp - - provider_gcp do - create(:cluster_provider_gcp, :creating) - end + provider_gcp factory: [:cluster_provider_gcp, :creating] end end end diff --git a/spec/features/projects/clusters/applications_spec.rb b/spec/features/projects/clusters/applications_spec.rb new file mode 100644 index 00000000000..b34cd061ec6 --- /dev/null +++ b/spec/features/projects/clusters/applications_spec.rb @@ -0,0 +1,107 @@ +require 'spec_helper' + +feature 'Clusters Applications', :js do + include GoogleApi::CloudPlatformHelpers + + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + describe 'Installing applications' do + before do + visit project_cluster_path(project, cluster) + end + + context 'when cluster is being created' do + let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project])} + + scenario 'user is unable to install applications' do + page.within('.js-cluster-application-row-helm') do + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button').text).to eq('Install') + end + end + end + + context 'when cluster is created' do + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project])} + + scenario 'user can install applications' do + page.within('.js-cluster-application-row-helm') do + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to be_nil + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') + end + end + + context 'when user installs Helm' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async).and_return(nil) + + page.within('.js-cluster-application-row-helm') do + page.find(:css, '.js-cluster-application-install-button').click + end + end + + it 'he sees status transition' do + page.within('.js-cluster-application-row-helm') do + # FE sends request and gets the response, then the buttons is "Install" + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') + + Clusters::Cluster.last.application_helm.make_installing! + + # FE starts polling and update the buttons to "Installing" + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installing') + + Clusters::Cluster.last.application_helm.make_installed! + + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installed') + end + + expect(page).to have_content('Helm Tiller was successfully installed on your cluster') + end + end + + context 'when user installs Ingress' do + context 'when user installs application: Ingress' do + before do + allow(ClusterInstallAppWorker).to receive(:perform_async).and_return(nil) + + create(:cluster_applications_helm, :installed, cluster: cluster) + + page.within('.js-cluster-application-row-ingress') do + page.find(:css, '.js-cluster-application-install-button').click + end + end + + it 'he sees status transition' do + page.within('.js-cluster-application-row-ingress') do + # FE sends request and gets the response, then the buttons is "Install" + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') + + Clusters::Cluster.last.application_ingress.make_installing! + + # FE starts polling and update the buttons to "Installing" + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installing') + + Clusters::Cluster.last.application_ingress.make_installed! + + expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') + expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installed') + end + + expect(page).to have_content('Ingress was successfully installed on your cluster') + end + end + end + end + end +end diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb new file mode 100644 index 00000000000..8a0da669147 --- /dev/null +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -0,0 +1,136 @@ +require 'spec_helper' + +feature 'Gcp Cluster', :js do + include GoogleApi::CloudPlatformHelpers + + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + gitlab_sign_in(user) + allow(Projects::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 } + end + + context 'when user has signed with Google' do + before do + allow_any_instance_of(Projects::Clusters::GcpController) + .to receive(:token_in_session).and_return('token') + allow_any_instance_of(Projects::Clusters::GcpController) + .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) + end + + context 'when user does not have a cluster and visits cluster index page' do + before do + visit project_clusters_path(project) + + click_link 'Create on GKE' + end + + context 'when user filled form with valid parameters' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_create) do + OpenStruct.new( + self_link: 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123', + status: 'RUNNING' + ) + end + + allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) + + fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123' + fill_in 'cluster_name', with: 'dev-cluster' + click_button 'Create cluster' + end + + it 'user sees a cluster details page and creation status' do + expect(page).to have_content('Cluster is being created on Google Container Engine...') + + Clusters::Cluster.last.provider.make_created! + + expect(page).to have_content('Cluster was successfully created on Google Container Engine') + end + + it 'user sees a error if something worng during creation' do + expect(page).to have_content('Cluster is being created on Google Container Engine...') + + Clusters::Cluster.last.provider.make_errored!('Something wrong!') + + expect(page).to have_content('Something wrong!') + end + end + + context 'when user filled form with invalid parameters' do + before do + click_button 'Create cluster' + end + + it 'user sees a validation error' do + expect(page).to have_css('#error_explanation') + end + end + end + + context 'when user does have a cluster and visits cluster page' do + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + + before do + visit project_cluster_path(project, cluster) + end + + it 'user sees a cluster details page' do + expect(page).to have_button('Save') + expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) + end + + context 'when user disables the cluster' do + before do + page.find(:css, '.js-toggle-cluster').click + click_button 'Save' + end + + it 'user sees the successful message' do + expect(page).to have_content('Cluster was successfully updated.') + end + end + + context 'when user changes cluster parameters' do + before do + fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' + click_button 'Save changes' + end + + it 'user sees the successful message' do + expect(page).to have_content('Cluster was successfully updated.') + expect(cluster.reload.platform_kubernetes.namespace).to eq('my-namespace') + end + end + + context 'when user destroy the cluster' do + before do + page.accept_confirm do + click_link 'Remove integration' + end + end + + it 'user sees creation form with the successful message' do + expect(page).to have_content('Cluster integration was successfully removed.') + expect(page).to have_link('Create on GKE') + end + end + end + end + + context 'when user has not signed with Google' do + before do + visit project_clusters_path(project) + + click_link 'Create on GKE' + end + + it 'user sees a login page' do + expect(page).to have_css('.signin-with-google') + end + end +end diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb new file mode 100644 index 00000000000..e97ba88f2f4 --- /dev/null +++ b/spec/features/projects/clusters/user_spec.rb @@ -0,0 +1,101 @@ +require 'spec_helper' + +feature 'User Cluster', :js do + include GoogleApi::CloudPlatformHelpers + + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + gitlab_sign_in(user) + allow(Projects::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 } + end + + context 'when user does not have a cluster and visits cluster index page' do + before do + visit project_clusters_path(project) + + click_link 'Add an existing cluster' + end + + context 'when user filled form with valid parameters' do + before do + 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' + click_button 'Add cluster' + end + + it 'user sees a cluster details page' do + expect(page).to have_content('Enable 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') + end + end + + context 'when user filled form with invalid parameters' do + before do + click_button 'Add cluster' + end + + it 'user sees a validation error' do + expect(page).to have_css('#error_explanation') + end + end + end + + context 'when user does have a cluster and visits cluster page' do + let(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } + + before do + visit project_cluster_path(project, cluster) + end + + it 'user sees a cluster details page' do + expect(page).to have_button('Save') + end + + context 'when user disables the cluster' do + before do + page.find(:css, '.js-toggle-cluster').click + fill_in 'cluster_name', with: 'dev-cluster' + click_button 'Save' + end + + it 'user sees the successful message' do + expect(page).to have_content('Cluster was successfully updated.') + end + end + + context 'when user changes cluster parameters' do + before do + fill_in 'cluster_name', with: 'my-dev-cluster' + fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' + click_button 'Save changes' + end + + it 'user sees the successful message' do + expect(page).to have_content('Cluster was successfully updated.') + expect(cluster.reload.name).to eq('my-dev-cluster') + expect(cluster.reload.platform_kubernetes.namespace).to eq('my-namespace') + end + end + + context 'when user destroy the cluster' do + before do + page.accept_confirm do + click_link 'Remove integration' + end + end + + it 'user sees creation form with the successful message' do + expect(page).to have_content('Cluster integration was successfully removed.') + expect(page).to have_link('Add an existing cluster') + end + end + end +end diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index 197e6df4997..4243c4fd266 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -3,204 +3,23 @@ require 'spec_helper' feature 'Clusters', :js do include GoogleApi::CloudPlatformHelpers - let!(:project) { create(:project, :repository) } - let!(:user) { create(:user) } + let(:project) { create(:project) } + let(:user) { create(:user) } before do project.add_master(user) gitlab_sign_in(user) end - context 'when user has signed in Google' do - before do - allow_any_instance_of(Projects::ClustersController) - .to receive(:token_in_session).and_return('token') - allow_any_instance_of(Projects::ClustersController) - .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) - end - - context 'when user does not have a cluster and visits cluster index page' do - before do - visit project_clusters_path(project) - - click_link 'Create on GKE' - end - - it 'user sees a new page' do - expect(page).to have_button('Create cluster') - end - - context 'when user filled form with valid parameters' do - before do - double.tap do |dbl| - allow(dbl).to receive(:status).and_return('RUNNING') - allow(dbl).to receive(:self_link) - .and_return('projects/gcp-project-12345/zones/us-central1-a/operations/ope-123') - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_clusters_create).and_return(dbl) - end - - allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) - - fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123' - fill_in 'cluster_name', with: 'dev-cluster' - click_button 'Create cluster' - end - - it 'user sees a cluster details page and creation status' do - expect(page).to have_content('Cluster is being created on Google Container Engine...') - - # Application Installation buttons - page.within('.js-cluster-application-row-helm') do - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - expect(page.find(:css, '.js-cluster-application-install-button').text).to eq('Install') - end - - Clusters::Cluster.last.provider.make_created! - - expect(page).to have_content('Cluster was successfully created on Google Container Engine') - end - - it 'user sees a error if something worng during creation' do - expect(page).to have_content('Cluster is being created on Google Container Engine...') - - Clusters::Cluster.last.provider.make_errored!('Something wrong!') - - expect(page).to have_content('Something wrong!') - end - end - - context 'when user filled form with invalid parameters' do - before do - click_button 'Create cluster' - end - - it 'user sees a validation error' do - expect(page).to have_css('#error_explanation') - end - end - end - - context 'when user has a cluster and visits cluster index page' do - let!(:cluster) { create(:cluster, :project, :provided_by_gcp) } - let(:project) { cluster.project } - - before do - visit project_clusters_path(project) - end - - it 'user sees an cluster details page' do - expect(page).to have_button('Save') - expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) - - # Application Installation buttons - page.within('.js-cluster-application-row-helm') do - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to be_nil - expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') - end - end - - context 'when user installs application: Helm Tiller' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async).and_return(nil) - - page.within('.js-cluster-application-row-helm') do - page.find(:css, '.js-cluster-application-install-button').click - end - end - - it 'user sees status transition' do - page.within('.js-cluster-application-row-helm') do - # FE sends request and gets the response, then the buttons is "Install" - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') - - Clusters::Cluster.last.application_helm.make_installing! - - # FE starts polling and update the buttons to "Installing" - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installing') - - Clusters::Cluster.last.application_helm.make_installed! - - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installed') - end - - expect(page).to have_content('Helm Tiller was successfully installed on your cluster') - end - end - - context 'when user installs application: Ingress' do - before do - allow(ClusterInstallAppWorker).to receive(:perform_async).and_return(nil) - # Helm Tiller needs to be installed before you can install Ingress - create(:cluster_applications_helm, :installed, cluster: cluster) - - visit project_clusters_path(project) - - page.within('.js-cluster-application-row-ingress') do - page.find(:css, '.js-cluster-application-install-button').click - end - end - - it 'user sees status transition' do - page.within('.js-cluster-application-row-ingress') do - # FE sends request and gets the response, then the buttons is "Install" - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Install') - - Clusters::Cluster.last.application_ingress.make_installing! - - # FE starts polling and update the buttons to "Installing" - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installing') - - Clusters::Cluster.last.application_ingress.make_installed! - - expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') - expect(page.find(:css, '.js-cluster-application-install-button')).to have_content('Installed') - end - - expect(page).to have_content('Ingress was successfully installed on your cluster') - end - end - - context 'when user disables the cluster' do - before do - page.find(:css, '.js-toggle-cluster').click - click_button 'Save' - end - - it 'user sees the succeccful message' do - expect(page).to have_content('Cluster was successfully updated.') - end - end - - context 'when user destory the cluster' do - before do - page.accept_confirm do - click_link 'Remove integration' - end - end - - it 'user sees creation form with the succeccful message' do - expect(page).to have_content('Cluster integration was successfully removed.') - expect(page).to have_link('Create on GKE') - end - end - end - end - - context 'when user has not signed in Google' do + context 'when user does not have a cluster and visits cluster index page' do before do visit project_clusters_path(project) click_link 'Create on GKE' end - it 'user sees a login page' do - expect(page).to have_css('.signin-with-google') + it 'user sees a new page' do + expect(page).to have_button('Create cluster') end end end diff --git a/spec/javascripts/clusters/clusters_bundle_spec.js b/spec/javascripts/clusters/clusters_bundle_spec.js index 027e8001053..6d6e71cc215 100644 --- a/spec/javascripts/clusters/clusters_bundle_spec.js +++ b/spec/javascripts/clusters/clusters_bundle_spec.js @@ -36,6 +36,20 @@ describe('Clusters', () => { }); }); + describe('showToken', () => { + it('should update tye field type', () => { + cluster.showTokenButton.click(); + expect( + cluster.tokenField.getAttribute('type'), + ).toEqual('text'); + + cluster.showTokenButton.click(); + expect( + cluster.tokenField.getAttribute('type'), + ).toEqual('password'); + }); + }); + describe('checkForNewInstalls', () => { const INITIAL_APP_MAP = { helm: { status: null, title: 'Helm Tiller' }, @@ -113,7 +127,7 @@ describe('Clusters', () => { }); describe('when cluster is created', () => { - it('should show the success container', () => { + it('should show the success container and fresh the page', () => { cluster.updateContainer(null, 'created'); expect(