From 90e3a4919fd977c1615a5270fd49146140159f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Alc=C3=A1ntara?= Date: Mon, 9 Sep 2019 20:27:51 +0000 Subject: [PATCH] Create new feature flagged UI for cloud providers - Create HAML UI select a cloud provider to create a cluster. - Add query param to :new cluster view to display a specific cluster provider form depending on the value of the provider query param. - Update unit tests and e2e tests to reflect these changes --- .../clusters/clusters_controller.rb | 13 ++++-- app/presenters/clusterable_presenter.rb | 4 +- .../instance_clusterable_presenter.rb | 4 +- .../_cloud_provider_button.html.haml | 8 ++++ .../_cloud_provider_selector.html.haml | 11 +++++ .../clusters/clusters/eks/_index.html.haml | 1 + app/views/clusters/clusters/new.html.haml | 44 ++++++++++++------- locale/gitlab.pot | 19 +++++--- .../admin/clusters_controller_spec.rb | 19 +++++++- .../groups/clusters_controller_spec.rb | 19 +++++++- .../projects/clusters_controller_spec.rb | 23 +++++++++- spec/features/projects/clusters/gcp_spec.rb | 3 ++ spec/features/projects/clusters_spec.rb | 24 ++++++++++ 13 files changed, 158 insertions(+), 34 deletions(-) create mode 100644 app/views/clusters/clusters/cloud_providers/_cloud_provider_button.html.haml create mode 100644 app/views/clusters/clusters/cloud_providers/_cloud_provider_selector.html.haml create mode 100644 app/views/clusters/clusters/eks/_index.html.haml diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index ec8077d18e3..bcd771dafcf 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -35,6 +35,12 @@ class Clusters::ClustersController < Clusters::BaseController end def new + return unless Feature.enabled?(:create_eks_clusters) + + @gke_selected = params[:provider] == 'gke' + @eks_selected = params[:provider] == 'eks' + + return redirect_to @authorize_url if @gke_selected && @authorize_url && !@valid_gcp_token end # Overridding ActionController::Metal#status is NOT a good idea @@ -99,7 +105,7 @@ class Clusters::ClustersController < Clusters::BaseController validate_gcp_token user_cluster - render :new, locals: { active_tab: 'gcp' } + render :new, locals: { active_tab: 'create' } end end @@ -116,7 +122,7 @@ class Clusters::ClustersController < Clusters::BaseController validate_gcp_token gcp_cluster - render :new, locals: { active_tab: 'user' } + render :new, locals: { active_tab: 'add' } end end @@ -189,7 +195,8 @@ class Clusters::ClustersController < Clusters::BaseController end def generate_gcp_authorize_url - state = generate_session_key_redirect(clusterable.new_path.to_s) + params = Feature.enabled?(:create_eks_clusters) ? { provider: :gke } : {} + state = generate_session_key_redirect(clusterable.new_path(params).to_s) @authorize_url = GoogleApi::CloudPlatform::Client.new( nil, callback_google_api_auth_url, diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb index d1bf0344b66..49c64b31fc7 100644 --- a/app/presenters/clusterable_presenter.rb +++ b/app/presenters/clusterable_presenter.rb @@ -25,8 +25,8 @@ class ClusterablePresenter < Gitlab::View::Presenter::Delegated polymorphic_path([clusterable, :clusters]) end - def new_path - new_polymorphic_path([clusterable, :cluster]) + def new_path(options = {}) + new_polymorphic_path([clusterable, :cluster], options) end def create_user_clusters_path diff --git a/app/presenters/instance_clusterable_presenter.rb b/app/presenters/instance_clusterable_presenter.rb index f8bbe5216f1..cce400ad2a1 100644 --- a/app/presenters/instance_clusterable_presenter.rb +++ b/app/presenters/instance_clusterable_presenter.rb @@ -18,8 +18,8 @@ class InstanceClusterablePresenter < ClusterablePresenter end override :new_path - def new_path - new_admin_cluster_path + def new_path(options = {}) + new_admin_cluster_path(options) end override :cluster_status_cluster_path diff --git a/app/views/clusters/clusters/cloud_providers/_cloud_provider_button.html.haml b/app/views/clusters/clusters/cloud_providers/_cloud_provider_button.html.haml new file mode 100644 index 00000000000..f707c6585ec --- /dev/null +++ b/app/views/clusters/clusters/cloud_providers/_cloud_provider_button.html.haml @@ -0,0 +1,8 @@ +- provider = local_assigns.fetch(:provider) +- logo_path = local_assigns.fetch(:logo_path) +- label = local_assigns.fetch(:label) + += link_to clusterable.new_path(provider: provider), class: 'btn gl-button btn-outline flex-fill d-inline-flex flex-column mr-3 justify-content-center align-items-center' do + = image_tag logo_path, alt: label, class: 'gl-w-13 gl-h-13' + %span + = label diff --git a/app/views/clusters/clusters/cloud_providers/_cloud_provider_selector.html.haml b/app/views/clusters/clusters/cloud_providers/_cloud_provider_selector.html.haml new file mode 100644 index 00000000000..24506205243 --- /dev/null +++ b/app/views/clusters/clusters/cloud_providers/_cloud_provider_selector.html.haml @@ -0,0 +1,11 @@ +- gke_label = s_('ClusterIntegration|Google GKE') +- eks_label = s_('ClusterIntegration|Amazon EKS') +- create_cluster_label = s_('ClusterIntegration|Create cluster on') +.d-flex.flex-column + %h5 + = create_cluster_label + .d-flex + = render partial: 'clusters/clusters/cloud_providers/cloud_provider_button', + locals: { provider: 'gke', label: gke_label, logo_path: '' } + = render partial: 'clusters/clusters/cloud_providers/cloud_provider_button', + locals: { provider: 'eks', label: eks_label, logo_path: '' } diff --git a/app/views/clusters/clusters/eks/_index.html.haml b/app/views/clusters/clusters/eks/_index.html.haml new file mode 100644 index 00000000000..ca8e9ba527a --- /dev/null +++ b/app/views/clusters/clusters/eks/_index.html.haml @@ -0,0 +1 @@ +.js-create-eks-cluster-form-container diff --git a/app/views/clusters/clusters/new.html.haml b/app/views/clusters/clusters/new.html.haml index 6a8af23e5e8..fb182d99ff0 100644 --- a/app/views/clusters/clusters/new.html.haml +++ b/app/views/clusters/clusters/new.html.haml @@ -1,6 +1,8 @@ - breadcrumb_title _('Kubernetes') - page_title _('Kubernetes Cluster') -- active_tab = local_assigns.fetch(:active_tab, 'gcp') +- create_eks_enabled = Feature.enabled?(:create_eks_clusters) +- active_tab = local_assigns.fetch(:active_tab, 'create') +- link_end = ''.html_safe = javascript_include_tag 'https://apis.google.com/js/api.js' = render_gcp_signup_offer @@ -11,26 +13,36 @@ .col-md-9.js-toggle-container %ul.nav-links.nav-tabs.gitlab-tabs.nav{ role: 'tablist' } %li.nav-item{ role: 'presentation' } - %a.nav-link{ href: '#create-gcp-cluster-pane', id: 'create-gcp-cluster-tab', class: active_when(active_tab == 'gcp'), data: { toggle: 'tab' }, role: 'tab' } + %a.nav-link{ href: '#create-cluster-pane', id: 'create-cluster-tab', class: active_when(active_tab == 'create'), data: { toggle: 'tab' }, role: 'tab' } %span Create new Cluster on GKE %li.nav-item{ role: 'presentation' } - %a.nav-link{ href: '#add-user-cluster-pane', id: 'add-user-cluster-tab', class: active_when(active_tab == 'user'), data: { toggle: 'tab' }, role: 'tab' } + %a.nav-link{ href: '#add-cluster-pane', id: 'add-cluster-tab', class: active_when(active_tab == 'add'), data: { toggle: 'tab' }, role: 'tab' } %span Add existing cluster .tab-content.gitlab-tab-content - .tab-pane{ id: 'create-gcp-cluster-pane', class: active_when(active_tab == 'gcp'), role: 'tabpanel' } - = render 'clusters/clusters/gcp/header' - - if @valid_gcp_token - = render 'clusters/clusters/gcp/form' - - elsif @authorize_url - .signin-with-google - = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), @authorize_url) - = _('or') - = link_to('create a new Google account', 'https://accounts.google.com/SignUpWithoutGmail?service=cloudconsole&continue=https%3A%2F%2Fconsole.cloud.google.com%2Ffreetrial%3Futm_campaign%3D2018_cpanel%26utm_source%3Dgitlab%26utm_medium%3Dreferral', target: '_blank', rel: 'noopener noreferrer') - - else - - link = link_to(s_('ClusterIntegration|properly configured'), help_page_path("integration/google"), target: '_blank', rel: 'noopener noreferrer') - = s_('Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service.').html_safe % { link_to_documentation: link } + - if create_eks_enabled + .tab-pane{ id: 'create-cluster-pane', class: active_when(active_tab == 'create'), role: 'tabpanel' } + - if @gke_selected && @valid_gcp_token + = render 'clusters/clusters/gcp/header' + = render 'clusters/clusters/gcp/form' + - elsif @eks_selected + = render 'clusters/clusters/eks/index' + - else + = render 'clusters/clusters/cloud_providers/cloud_provider_selector' + - else + .tab-pane{ id: 'create-cluster-pane', class: active_when(active_tab == 'create'), role: 'tabpanel' } + = render 'clusters/clusters/gcp/header' + - if @valid_gcp_token + = render 'clusters/clusters/gcp/form' + - elsif @authorize_url + .signin-with-google + - create_account_link = ''.html_safe % { url: 'https://accounts.google.com/SignUpWithoutGmail?service=cloudconsole&continue=https%3A%2F%2Fconsole.cloud.google.com%2Ffreetrial%3Futm_campaign%3D2018_cpanel%26utm_source%3Dgitlab%26utm_medium%3Dreferral' } + = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px', alt: _('Sign in with Google')), @authorize_url) + = s_('or %{link_start}create a new Google account%{link_end}').html_safe % { link_start: create_account_link, link_end: link_end } + - else + - documentation_link_start = ''.html_safe % { url: help_page_path("integration/google") } + = s_('Google authentication is not %{link_start}property configured%{link_end}. Ask your GitLab administrator if you want to use this service.').html_safe % { link_start: documentation_link_start, link_end: link_end } - .tab-pane{ id: 'add-user-cluster-pane', class: active_when(active_tab == 'user'), role: 'tabpanel' } + .tab-pane{ id: 'add-cluster-pane', class: active_when(active_tab == 'add'), role: 'tabpanel' } = render 'clusters/clusters/user/header' = render 'clusters/clusters/user/form' diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a77d70a5700..9ca3b357f77 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2530,6 +2530,9 @@ msgstr "" msgid "ClusterIntegration|Alternatively" msgstr "" +msgid "ClusterIntegration|Amazon EKS" +msgstr "" + msgid "ClusterIntegration|An error occurred when trying to contact the Google Cloud API. Please try again later." msgstr "" @@ -2608,6 +2611,9 @@ msgstr "" msgid "ClusterIntegration|Create Kubernetes cluster" msgstr "" +msgid "ClusterIntegration|Create cluster on" +msgstr "" + msgid "ClusterIntegration|Did you know?" msgstr "" @@ -2659,6 +2665,9 @@ msgstr "" msgid "ClusterIntegration|Google Cloud Platform project" msgstr "" +msgid "ClusterIntegration|Google GKE" +msgstr "" + msgid "ClusterIntegration|Google Kubernetes Engine" msgstr "" @@ -2989,9 +2998,6 @@ msgstr "" msgid "ClusterIntegration|pricing" msgstr "" -msgid "ClusterIntegration|properly configured" -msgstr "" - msgid "ClusterIntegration|sign up" msgstr "" @@ -5518,7 +5524,7 @@ msgstr "" msgid "Google Takeout" msgstr "" -msgid "Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service." +msgid "Google authentication is not %{link_start}property configured%{link_end}. Ask your GitLab administrator if you want to use this service." msgstr "" msgid "Got it" @@ -10565,6 +10571,9 @@ msgstr "" msgid "Sign in via 2FA code" msgstr "" +msgid "Sign in with Google" +msgstr "" + msgid "Sign out" msgstr "" @@ -14180,7 +14189,7 @@ msgstr "" msgid "nounSeries|%{item}, and %{lastItem}" msgstr "" -msgid "or" +msgid "or %{link_start}create a new Google account%{link_end}" msgstr "" msgid "out of %d total test" diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index e5501535875..afc059d7561 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -73,8 +73,8 @@ describe Admin::ClustersController do end describe 'GET #new' do - def get_new - get :new + def get_new(provider: 'gke') + get :new, params: { provider: provider } end describe 'functionality for new cluster' do @@ -85,6 +85,7 @@ describe Admin::ClustersController do end before do + stub_feature_flags(create_eks_clusters: false) allow(SecureRandom).to receive(:hex).and_return(key) end @@ -94,6 +95,20 @@ describe Admin::ClustersController do expect(assigns(:authorize_url)).to include(key) expect(session[session_key_for_redirect_uri]).to eq(new_admin_cluster_path) end + + context 'when create_eks_clusters feature flag is enabled' do + before do + stub_feature_flags(create_eks_clusters: true) + end + + context 'when selected provider is gke and no valid gcp token exists' do + it 'redirects to gcp authorize_url' do + get_new + + expect(response).to redirect_to(assigns(:authorize_url)) + end + end + end end context 'when omniauth has not configured' do diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 09677b42887..5a3ba51d4df 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -85,8 +85,8 @@ describe Groups::ClustersController do end describe 'GET new' do - def go - get :new, params: { group_id: group } + def go(provider: 'gke') + get :new, params: { group_id: group, provider: provider } end describe 'functionality for new cluster' do @@ -97,6 +97,7 @@ describe Groups::ClustersController do end before do + stub_feature_flags(create_eks_clusters: false) allow(SecureRandom).to receive(:hex).and_return(key) end @@ -106,6 +107,20 @@ describe Groups::ClustersController do expect(assigns(:authorize_url)).to include(key) expect(session[session_key_for_redirect_uri]).to eq(new_group_cluster_path(group)) end + + context 'when create_eks_clusters feature flag is enabled' do + before do + stub_feature_flags(create_eks_clusters: true) + end + + context 'when selected provider is gke and no valid gcp token exists' do + it 'redirects to gcp authorize_url' do + go + + expect(response).to redirect_to(assigns(:authorize_url)) + end + end + end end context 'when omniauth has not configured' do diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 35cbab57037..8ac72df5d20 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -79,8 +79,12 @@ describe Projects::ClustersController do end describe 'GET new' do - def go - get :new, params: { namespace_id: project.namespace, project_id: project } + def go(provider: 'gke') + get :new, params: { + namespace_id: project.namespace, + project_id: project, + provider: provider + } end describe 'functionality for new cluster' do @@ -91,6 +95,7 @@ describe Projects::ClustersController do end before do + stub_feature_flags(create_eks_clusters: false) allow(SecureRandom).to receive(:hex).and_return(key) end @@ -100,6 +105,20 @@ describe Projects::ClustersController do expect(assigns(:authorize_url)).to include(key) expect(session[session_key_for_redirect_uri]).to eq(new_project_cluster_path(project)) end + + context 'when create_eks_clusters feature flag is enabled' do + before do + stub_feature_flags(create_eks_clusters: true) + end + + context 'when selected provider is gke and no valid gcp token exists' do + it 'redirects to gcp authorize_url' do + go + + expect(response).to redirect_to(assigns(:authorize_url)) + end + end + end end context 'when omniauth has not configured' do diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 820ce48e52c..a11237db508 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -18,6 +18,8 @@ describe 'Gcp Cluster', :js do let(:project_id) { 'test-project-1234' } before do + stub_feature_flags(create_eks_clusters: false) + allow_any_instance_of(Projects::ClustersController) .to receive(:token_in_session).and_return('token') allow_any_instance_of(Projects::ClustersController) @@ -147,6 +149,7 @@ describe 'Gcp Cluster', :js do context 'when user has not signed with Google' do before do + stub_feature_flags(create_eks_clusters: false) visit project_clusters_path(project) click_link 'Add Kubernetes cluster' diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index ce382c19fc1..d1cd19dff2d 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -51,6 +51,7 @@ describe 'Clusters', :js do context 'when user has not signed in Google' do before do + stub_feature_flags(create_eks_clusters: false) visit project_clusters_path(project) click_link 'Add Kubernetes cluster' @@ -62,4 +63,27 @@ describe 'Clusters', :js do expect(page).to have_link('Google account') end end + + context 'when create_eks_clusters feature flag is enabled' do + before do + stub_feature_flags(create_eks_clusters: true) + end + + context 'when user access create cluster page' do + before do + visit project_clusters_path(project) + + click_link 'Add Kubernetes cluster' + click_link 'Create new Cluster on GKE' + end + + it 'user sees a link to create a GKE cluster' do + expect(page).to have_link('Google GKE') + end + + it 'user sees a link to create an EKS cluster' do + expect(page).to have_link('Amazon EKS') + end + end + end end