Commit f62b414a authored by Sri's avatar Sri Committed by Bob Van Landuyt

Refactor Service Accounts creation

- Use refs (branches / tags) to determine protected
  state of service account CI vars. This achieves
  consistency / backward compatibility with original
  AWS five minute production prototype

- Frontend table component's `tableFields` is not
  reactive, thus it is now a top-level property

- Use `permitted_params` for the `create` request

- Update existing variable, do not destroy and then
  recreate
parent 27ca9b47
<script>
import { GlButton, GlFormGroup, GlFormSelect } from '@gitlab/ui';
import { __ } from '~/locale';
import { __, s__ } from '~/locale';
export default {
components: { GlButton, GlFormGroup, GlFormSelect },
props: {
availableRegions: { required: true, type: Array },
environments: { required: true, type: Array },
refs: { required: true, type: Array },
cancelPath: { required: true, type: String },
},
i18n: {
title: __('Configure region for environment'),
gcpRegionLabel: __('Region'),
gcpRegionDescription: __('List of suitable GCP locations'),
environmentLabel: __('Environment'),
environmentDescription: __('List of environments for this project'),
refsLabel: s__('GoogleCloud|Refs'),
refsDescription: s__('GoogleCloud|Configured region is linked to the selected branch or tag'),
submitLabel: __('Configure region'),
cancelLabel: __('Cancel'),
},
......@@ -28,14 +28,14 @@ export default {
</header>
<gl-form-group
label-for="environment"
:label="$options.i18n.environmentLabel"
:description="$options.i18n.environmentDescription"
label-for="ref"
:label="$options.i18n.refsLabel"
:description="$options.i18n.refsDescription"
>
<gl-form-select id="environment" name="environment" required>
<gl-form-select id="ref" name="ref" required>
<option value="*">{{ __('All') }}</option>
<option v-for="environment in environments" :key="environment.id" :value="environment.name">
{{ environment.name }}
<option v-for="ref in refs" :key="ref" :value="ref">
{{ ref }}
</option>
</gl-form-select>
</gl-form-group>
......
<script>
import { GlButton, GlFormGroup, GlFormSelect, GlFormCheckbox } from '@gitlab/ui';
import { __ } from '~/locale';
import { GlButton, GlFormCheckbox, GlFormGroup, GlFormSelect } from '@gitlab/ui';
import { s__ } from '~/locale';
export default {
ALL_REFS: '*',
components: { GlButton, GlFormGroup, GlFormSelect, GlFormCheckbox },
props: {
gcpProjects: { required: true, type: Array },
environments: { required: true, type: Array },
refs: { required: true, type: Array },
cancelPath: { required: true, type: String },
},
i18n: {
title: __('Create service account'),
gcpProjectLabel: __('Google Cloud project'),
gcpProjectDescription: __(
'New service account is generated for the selected Google Cloud project',
title: s__('GoogleCloud|Create service account'),
gcpProjectLabel: s__('GoogleCloud|Google Cloud project'),
gcpProjectDescription: s__(
'GoogleCloud|New service account is generated for the selected Google Cloud project',
),
environmentLabel: __('Environment'),
environmentDescription: __('Generated service account is linked to the selected environment'),
submitLabel: __('Create service account'),
cancelLabel: __('Cancel'),
checkboxLabel: __(
'I understand the responsibilities involved with managing service account keys',
refsLabel: s__('GoogleCloud|Refs'),
refsDescription: s__(
'GoogleCloud|Generated service account is linked to the selected branch or tag',
),
submitLabel: s__('GoogleCloud|Create service account'),
cancelLabel: s__('GoogleCloud|Cancel'),
checkboxLabel: s__(
'GoogleCloud|I understand the responsibilities involved with managing service account keys',
),
},
};
......@@ -47,18 +50,14 @@ export default {
</gl-form-select>
</gl-form-group>
<gl-form-group
label-for="environment"
:label="$options.i18n.environmentLabel"
:description="$options.i18n.environmentDescription"
label-for="ref"
:label="$options.i18n.refsLabel"
:description="$options.i18n.refsDescription"
>
<gl-form-select id="environment" name="environment" required>
<option value="*">{{ __('All') }}</option>
<option
v-for="environment in environments"
:key="environment.name"
:value="environment.name"
>
{{ environment.name }}
<gl-form-select id="ref" name="ref" required>
<option :value="$options.ALL_REFS">{{ __('All') }}</option>
<option v-for="ref in refs" :key="ref" :value="ref">
{{ ref }}
</option>
</gl-form-select>
</gl-form-group>
......
......@@ -18,16 +18,12 @@ export default {
required: true,
},
},
data() {
return {
tableFields: [
{ key: 'environment', label: __('Environment'), sortable: true },
{ key: 'gcp_project', label: __('Google Cloud Project'), sortable: true },
{ key: 'service_account_exists', label: __('Service Account'), sortable: true },
{ key: 'service_account_key_exists', label: __('Service Account Key'), sortable: true },
],
};
},
tableFields: [
{ key: 'ref', label: __('Environment'), sortable: true },
{ key: 'gcp_project', label: __('Google Cloud Project'), sortable: true },
{ key: 'service_account_exists', label: __('Service Account'), sortable: true },
{ key: 'service_account_key_exists', label: __('Service Account Key'), sortable: true },
],
i18n: {
createServiceAccount: __('Create service account'),
found: __(''),
......@@ -62,7 +58,7 @@ export default {
<h2 class="gl-font-size-h2">{{ $options.i18n.serviceAccountsTitle }}</h2>
<p>{{ $options.i18n.serviceAccountsDescription }}</p>
<gl-table :items="list" :fields="tableFields">
<gl-table :items="list" :fields="$options.tableFields">
<template #cell(service_account_exists)="{ value }">
{{ value ? $options.i18n.found : $options.i18n.notFound }}
</template>
......
......@@ -8,18 +8,22 @@ class Projects::GoogleCloud::GcpRegionsController < Projects::GoogleCloud::BaseC
def index
@google_cloud_path = project_google_cloud_index_path(project)
params = { per_page: 50 }
branches = BranchesFinder.new(project.repository, params).execute(gitaly_pagination: true)
tags = TagsFinder.new(project.repository, params).execute(gitaly_pagination: true)
refs = (branches + tags).map(&:name)
@js_data = {
screen: 'gcp_regions_form',
availableRegions: AVAILABLE_REGIONS,
environments: project.environments,
refs: refs,
cancelPath: project_google_cloud_index_path(project)
}.to_json
end
def create
permitted_params = params.permit(:environment, :gcp_region)
permitted_params = params.permit(:ref, :gcp_region)
GoogleCloud::GcpRegionAddOrReplaceService.new(project).execute(permitted_params[:environment], permitted_params[:gcp_region])
GoogleCloud::GcpRegionAddOrReplaceService.new(project).execute(permitted_params[:ref], permitted_params[:gcp_region])
redirect_to project_google_cloud_index_path(project), notice: _('GCP region configured')
end
......
......@@ -12,10 +12,14 @@ class Projects::GoogleCloud::ServiceAccountsController < Projects::GoogleCloud::
@js_data = { screen: 'no_gcp_projects' }.to_json
render status: :unauthorized, template: 'projects/google_cloud/errors/no_gcp_projects'
else
params = { per_page: 50 }
branches = BranchesFinder.new(project.repository, params).execute(gitaly_pagination: true)
tags = TagsFinder.new(project.repository, params).execute(gitaly_pagination: true)
refs = (branches + tags).map(&:name)
@js_data = {
screen: 'service_accounts_form',
gcpProjects: gcp_projects,
environments: project.environments,
refs: refs,
cancelPath: project_google_cloud_index_path(project)
}.to_json
end
......@@ -24,12 +28,14 @@ class Projects::GoogleCloud::ServiceAccountsController < Projects::GoogleCloud::
end
def create
permitted_params = params.permit(:gcp_project, :ref)
response = GoogleCloud::CreateServiceAccountsService.new(
project,
current_user,
google_oauth2_token: token_in_session,
gcp_project_id: params[:gcp_project],
environment_name: params[:environment]
gcp_project_id: permitted_params[:gcp_project],
environment_name: permitted_params[:ref]
).execute
redirect_to project_google_cloud_index_path(project), notice: response.message
......
......@@ -12,7 +12,7 @@ module GoogleCloud
service_account.project_id,
service_account.to_json,
service_account_key.to_json,
environment_protected?
ProtectedBranch.protected?(project, environment_name) || ProtectedTag.protected?(project, environment_name)
)
ServiceResponse.success(message: _('Service account generated successfully'), payload: {
......@@ -50,11 +50,6 @@ module GoogleCloud
def service_account_desc
"GitLab generated service account for project '#{project.name}' and environment '#{environment_name}'"
end
# Overridden in EE
def environment_protected?
false
end
end
end
......
......@@ -14,12 +14,12 @@ module GoogleCloud
#
# This method looks up GitLab project's CI vars
# and returns Google Cloud Service Accounts combinations
# aligning GitLab project and environment to GCP projects
# aligning GitLab project and ref to GCP projects
def find_for_project
group_vars_by_environment.map do |environment_scope, value|
group_vars_by_ref.map do |environment_scope, value|
{
environment: environment_scope,
ref: environment_scope,
gcp_project: value['GCP_PROJECT_ID'],
service_account_exists: value['GCP_SERVICE_ACCOUNT'].present?,
service_account_key_exists: value['GCP_SERVICE_ACCOUNT_KEY'].present?
......@@ -27,21 +27,21 @@ module GoogleCloud
end
end
def add_for_project(environment, gcp_project_id, service_account, service_account_key, is_protected)
def add_for_project(ref, gcp_project_id, service_account, service_account_key, is_protected)
project_var_create_or_replace(
environment,
ref,
'GCP_PROJECT_ID',
gcp_project_id,
is_protected
)
project_var_create_or_replace(
environment,
ref,
'GCP_SERVICE_ACCOUNT',
service_account,
is_protected
)
project_var_create_or_replace(
environment,
ref,
'GCP_SERVICE_ACCOUNT_KEY',
service_account_key,
is_protected
......@@ -50,7 +50,7 @@ module GoogleCloud
private
def group_vars_by_environment
def group_vars_by_ref
filtered_vars = project.variables.filter { |variable| GCP_KEYS.include? variable.key }
filtered_vars.each_with_object({}) do |variable, grouped|
grouped[variable.environment_scope] ||= {}
......@@ -59,10 +59,19 @@ module GoogleCloud
end
def project_var_create_or_replace(environment_scope, key, value, is_protected)
params = { key: key, filter: { environment_scope: environment_scope } }
existing_variable = ::Ci::VariablesFinder.new(project, params).execute.first
existing_variable.destroy if existing_variable
project.variables.create!(key: key, value: value, environment_scope: environment_scope, protected: is_protected)
change_params = { variable_params: { key: key, value: value, environment_scope: environment_scope, protected: is_protected } }
filter_params = { key: key, filter: { environment_scope: environment_scope } }
existing_variable = ::Ci::VariablesFinder.new(project, filter_params).execute.first
if existing_variable
change_params[:action] = :update
change_params[:variable] = existing_variable
else
change_params[:action] = :create
end
::Ci::ChangeVariableService.new(container: project, current_user: current_user, params: change_params).execute
end
end
end
# frozen_string_literal: true
module EE
module GoogleCloud
module CreateServiceAccountsService
extend ::Gitlab::Utils::Override
include ::Gitlab::Utils::StrongMemoize
private
def existing_environments
strong_memoize(:environment) do
Environments::EnvironmentsFinder.new(project, current_user, name: environment_name).execute
end
end
def environment
strong_memoize(:environment) do
existing_environments.first
end
end
override :environment_protected?
def environment_protected?
environment ? environment.protected? : false
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe GoogleCloud::CreateServiceAccountsService do
let(:google_oauth2_token) { 'mock-token' }
let(:gcp_project_id) { 'mock-gcp-project-id' }
describe '#execute' do
before do
stub_licensed_features(protected_environments: true)
mock_google_oauth2_creds = Struct.new(:app_id, :app_secret)
.new('mock-app-id', 'mock-app-secret')
allow(Gitlab::Auth::OAuth::Provider)
.to receive(:config_for)
.with('google_oauth2')
.and_return(mock_google_oauth2_creds)
expect_next_instance_of(GoogleApi::CloudPlatform::Client) do |client|
mock_service_account = Struct.new(:project_id, :unique_id, :email)
.new('mock-project-id', 'mock-unique-id', 'mock-email')
expect(client)
.to receive(:create_service_account)
.and_return(mock_service_account)
expect(client)
.to receive(:create_service_account_key)
.and_return('mock-key')
expect(client)
.to receive(:grant_service_account_roles)
end
end
context 'when environment is new' do
it 'creates unprotected vars' do
project = create(:project)
response = described_class.new(
project,
nil,
google_oauth2_token: google_oauth2_token,
gcp_project_id: gcp_project_id,
environment_name: '*'
).execute
expect(response.status).to eq(:success)
expect(response.message).to eq('Service account generated successfully')
expect(project.variables.count).to eq(3)
expect(project.variables.first.protected).to eq(false)
expect(project.variables.second.protected).to eq(false)
expect(project.variables.third.protected).to eq(false)
end
end
context 'when environment is not protected' do
it 'creates unprotected vars' do
project = create(:project)
environment = create(:environment, project: project, name: 'staging')
response = described_class.new(
project,
nil,
google_oauth2_token: google_oauth2_token,
gcp_project_id: gcp_project_id,
environment_name: environment.name
).execute
expect(response.status).to eq(:success)
expect(response.message).to eq('Service account generated successfully')
expect(project.variables.count).to eq(3)
expect(project.variables.first.protected).to eq(false)
expect(project.variables.second.protected).to eq(false)
expect(project.variables.third.protected).to eq(false)
end
end
context 'when environment is protected', :aggregate_failures do
it 'creates protected vars' do
project = create(:project)
environment = create(:environment, project: project, name: 'production')
create(:protected_environment, project: project, name: environment.name)
response = described_class.new(
project,
nil,
google_oauth2_token: google_oauth2_token,
gcp_project_id: gcp_project_id,
environment_name: environment.name
).execute
expect(response.status).to eq(:success)
expect(response.message).to eq('Service account generated successfully')
expect(project.variables.count).to eq(3)
expect(project.variables.first.protected).to eq(true)
expect(project.variables.second.protected).to eq(true)
expect(project.variables.third.protected).to eq(true)
end
end
end
end
......@@ -15979,9 +15979,6 @@ msgstr ""
msgid "Generate site and private keys at"
msgstr ""
msgid "Generated service account is linked to the selected environment"
msgstr ""
msgid "Generic"
msgstr ""
......@@ -16999,9 +16996,6 @@ msgstr ""
msgid "Google Cloud authorizations required"
msgstr ""
msgid "Google Cloud project"
msgstr ""
msgid "Google Cloud project misconfigured"
msgstr ""
......@@ -17011,6 +17005,30 @@ msgstr ""
msgid "Google authentication is not %{link_start}properly configured%{link_end}. Ask your GitLab administrator if you want to use this service."
msgstr ""
msgid "GoogleCloud|Cancel"
msgstr ""
msgid "GoogleCloud|Configured region is linked to the selected branch or tag"
msgstr ""
msgid "GoogleCloud|Create service account"
msgstr ""
msgid "GoogleCloud|Generated service account is linked to the selected branch or tag"
msgstr ""
msgid "GoogleCloud|Google Cloud project"
msgstr ""
msgid "GoogleCloud|I understand the responsibilities involved with managing service account keys"
msgstr ""
msgid "GoogleCloud|New service account is generated for the selected Google Cloud project"
msgstr ""
msgid "GoogleCloud|Refs"
msgstr ""
msgid "Got it"
msgstr ""
......@@ -18209,9 +18227,6 @@ msgstr ""
msgid "I forgot my password"
msgstr ""
msgid "I understand the responsibilities involved with managing service account keys"
msgstr ""
msgid "I want to explore GitLab to see if it’s worth switching to"
msgstr ""
......@@ -22038,9 +22053,6 @@ msgstr ""
msgid "List of all merge commits"
msgstr ""
msgid "List of environments for this project"
msgstr ""
msgid "List of suitable GCP locations"
msgstr ""
......@@ -24498,9 +24510,6 @@ msgstr ""
msgid "New schedule"
msgstr ""
msgid "New service account is generated for the selected Google Cloud project"
msgstr ""
msgid "New snippet"
msgstr ""
......
......@@ -17,7 +17,7 @@ const SCREEN_COMPONENTS = {
};
const SERVICE_ACCOUNTS_FORM_PROPS = {
gcpProjects: [1, 2, 3],
environments: [4, 5, 6],
refs: [4, 5, 6],
cancelPath: '',
};
const HOME_PROPS = {
......
......@@ -10,7 +10,7 @@ describe('GcpRegionsForm component', () => {
const findAllFormSelects = () => wrapper.findAllComponents(GlFormSelect);
const findAllButtons = () => wrapper.findAllComponents(GlButton);
const propsData = { availableRegions: [], environments: [], cancelPath: '#cancel-url' };
const propsData = { availableRegions: [], refs: [], cancelPath: '#cancel-url' };
beforeEach(() => {
wrapper = shallowMount(GcpRegionsForm, { propsData });
......@@ -34,12 +34,12 @@ describe('GcpRegionsForm component', () => {
expect(select.exists()).toBe(true);
});
it('contains Environments form group', () => {
it('contains Refs form group', () => {
const formGroup = findAllFormGroups().at(1);
expect(formGroup.exists()).toBe(true);
});
it('contains Environments dropdown', () => {
it('contains Refs dropdown', () => {
const select = findAllFormSelects().at(1);
expect(select.exists()).toBe(true);
});
......
......@@ -11,7 +11,7 @@ describe('ServiceAccountsForm component', () => {
const findAllButtons = () => wrapper.findAllComponents(GlButton);
const findCheckbox = () => wrapper.findComponent(GlFormCheckbox);
const propsData = { gcpProjects: [], environments: [], cancelPath: '#cancel-url' };
const propsData = { gcpProjects: [], refs: [], cancelPath: '#cancel-url' };
beforeEach(() => {
wrapper = shallowMount(ServiceAccountsForm, { propsData, stubs: { GlFormCheckbox } });
......
......@@ -3,7 +3,8 @@
require 'spec_helper'
RSpec.describe Projects::GoogleCloud::GcpRegionsController do
let_it_be(:project) { create(:project, :public) }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:repository) { project.repository }
RSpec.shared_examples "should be not found" do
it 'returns not found' do
......
......@@ -93,6 +93,14 @@ RSpec.describe Projects::GoogleCloud::ServiceAccountsController do
it 'returns success on GET' do
authorized_members.each do |authorized_member|
allow_next_instance_of(BranchesFinder) do |branches_finder|
allow(branches_finder).to receive(:execute).and_return([])
end
allow_next_instance_of(TagsFinder) do |branches_finder|
allow(branches_finder).to receive(:execute).and_return([])
end
sign_in(authorized_member)
get url
......@@ -105,7 +113,7 @@ RSpec.describe Projects::GoogleCloud::ServiceAccountsController do
authorized_members.each do |authorized_member|
sign_in(authorized_member)
post url, params: { gcp_project: 'prj1', environment: 'env1' }
post url, params: { gcp_project: 'prj1', ref: 'env1' }
expect(response).to redirect_to(project_google_cloud_index_path(project))
end
......
......@@ -26,6 +26,8 @@ RSpec.describe GoogleCloud::CreateServiceAccountsService do
end
it 'creates unprotected vars', :aggregate_failures do
allow(ProtectedBranch).to receive(:protected?).and_return(false)
project = create(:project)
service = described_class.new(
......@@ -45,5 +47,28 @@ RSpec.describe GoogleCloud::CreateServiceAccountsService do
expect(project.variables.second.protected).to eq(false)
expect(project.variables.third.protected).to eq(false)
end
it 'creates protected vars', :aggregate_failures do
allow(ProtectedBranch).to receive(:protected?).and_return(true)
project = create(:project)
service = described_class.new(
project,
nil,
google_oauth2_token: 'mock-token',
gcp_project_id: 'mock-gcp-project-id',
environment_name: '*'
)
response = service.execute
expect(response.status).to eq(:success)
expect(response.message).to eq('Service account generated successfully')
expect(project.variables.count).to eq(3)
expect(project.variables.first.protected).to eq(true)
expect(project.variables.second.protected).to eq(true)
expect(project.variables.third.protected).to eq(true)
end
end
end
......@@ -37,17 +37,17 @@ RSpec.describe GoogleCloud::ServiceAccountsService do
aggregate_failures 'testing list of service accounts' do
expect(list.length).to eq(3)
expect(list.first[:environment]).to eq('*')
expect(list.first[:ref]).to eq('*')
expect(list.first[:gcp_project]).to eq('prj1')
expect(list.first[:service_account_exists]).to eq(false)
expect(list.first[:service_account_key_exists]).to eq(true)
expect(list.second[:environment]).to eq('staging')
expect(list.second[:ref]).to eq('staging')
expect(list.second[:gcp_project]).to eq('prj2')
expect(list.second[:service_account_exists]).to eq(true)
expect(list.second[:service_account_key_exists]).to eq(false)
expect(list.third[:environment]).to eq('production')
expect(list.third[:ref]).to eq('production')
expect(list.third[:gcp_project]).to eq('prj3')
expect(list.third[:service_account_exists]).to eq(true)
expect(list.third[:service_account_key_exists]).to eq(true)
......@@ -68,12 +68,12 @@ RSpec.describe GoogleCloud::ServiceAccountsService do
aggregate_failures 'testing list of service accounts' do
expect(list.length).to eq(2)
expect(list.first[:environment]).to eq('env_1')
expect(list.first[:ref]).to eq('env_1')
expect(list.first[:gcp_project]).to eq('gcp_prj_id_1')
expect(list.first[:service_account_exists]).to eq(true)
expect(list.first[:service_account_key_exists]).to eq(true)
expect(list.second[:environment]).to eq('env_2')
expect(list.second[:ref]).to eq('env_2')
expect(list.second[:gcp_project]).to eq('gcp_prj_id_2')
expect(list.second[:service_account_exists]).to eq(true)
expect(list.second[:service_account_key_exists]).to eq(true)
......@@ -89,12 +89,12 @@ RSpec.describe GoogleCloud::ServiceAccountsService do
expect(list.length).to eq(2)
# asserting that the first service account is replaced
expect(list.first[:environment]).to eq('env_1')
expect(list.first[:ref]).to eq('env_1')
expect(list.first[:gcp_project]).to eq('new_project')
expect(list.first[:service_account_exists]).to eq(true)
expect(list.first[:service_account_key_exists]).to eq(true)
expect(list.second[:environment]).to eq('env_2')
expect(list.second[:ref]).to eq('env_2')
expect(list.second[:gcp_project]).to eq('gcp_prj_id_2')
expect(list.second[:service_account_exists]).to eq(true)
expect(list.second[:service_account_key_exists]).to eq(true)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment