Commit f1f2262b authored by Dylan Griffith's avatar Dylan Griffith

Merge branch '216785-terraform-plan-developer-access' into 'master'

Allow developer access to read Terraform state

See merge request gitlab-org/gitlab!33573
parents c2823db8 ae342e6f
...@@ -341,6 +341,7 @@ class ProjectPolicy < BasePolicy ...@@ -341,6 +341,7 @@ class ProjectPolicy < BasePolicy
enable :update_alert_management_alert enable :update_alert_management_alert
enable :create_design enable :create_design
enable :destroy_design enable :destroy_design
enable :read_terraform_state
end end
rule { can?(:developer_access) & user_confirmed? }.policy do rule { can?(:developer_access) & user_confirmed? }.policy do
......
...@@ -5,26 +5,17 @@ module Terraform ...@@ -5,26 +5,17 @@ module Terraform
include Gitlab::OptimisticLocking include Gitlab::OptimisticLocking
StateLockedError = Class.new(StandardError) StateLockedError = Class.new(StandardError)
UnauthorizedError = Class.new(StandardError)
# rubocop: disable CodeReuse/ActiveRecord
def find_with_lock def find_with_lock
raise ArgumentError unless params[:name].present? retrieve_with_lock(find_only: true) do |state|
yield state if block_given?
state = Terraform::State.find_by(project: project, name: params[:name]) end
raise ActiveRecord::RecordNotFound.new("Couldn't find state") unless state
retry_optimistic_lock(state) { |state| yield state } if state && block_given?
state
end
# rubocop: enable CodeReuse/ActiveRecord
def create_or_find!
raise ArgumentError unless params[:name].present?
Terraform::State.create_or_find_by(project: project, name: params[:name])
end end
def handle_with_lock def handle_with_lock
raise UnauthorizedError unless can_modify_state?
retrieve_with_lock do |state| retrieve_with_lock do |state|
raise StateLockedError unless lock_matches?(state) raise StateLockedError unless lock_matches?(state)
...@@ -36,6 +27,7 @@ module Terraform ...@@ -36,6 +27,7 @@ module Terraform
def lock! def lock!
raise ArgumentError if params[:lock_id].blank? raise ArgumentError if params[:lock_id].blank?
raise UnauthorizedError unless can_modify_state?
retrieve_with_lock do |state| retrieve_with_lock do |state|
raise StateLockedError if state.locked? raise StateLockedError if state.locked?
...@@ -49,6 +41,8 @@ module Terraform ...@@ -49,6 +41,8 @@ module Terraform
end end
def unlock! def unlock!
raise UnauthorizedError unless can_modify_state?
retrieve_with_lock do |state| retrieve_with_lock do |state|
# force-unlock does not pass ID, so we ignore it if it is missing # force-unlock does not pass ID, so we ignore it if it is missing
raise StateLockedError unless params[:lock_id].nil? || lock_matches?(state) raise StateLockedError unless params[:lock_id].nil? || lock_matches?(state)
...@@ -63,8 +57,21 @@ module Terraform ...@@ -63,8 +57,21 @@ module Terraform
private private
def retrieve_with_lock def retrieve_with_lock(find_only: false)
create_or_find!.tap { |state| retry_optimistic_lock(state) { |state| yield state } } create_or_find!(find_only: find_only).tap { |state| retry_optimistic_lock(state) { |state| yield state } }
end
def create_or_find!(find_only:)
raise ArgumentError unless params[:name].present?
find_params = { project: project, name: params[:name] }
if find_only
Terraform::State.find_by(find_params) || # rubocop: disable CodeReuse/ActiveRecord
raise(ActiveRecord::RecordNotFound.new("Couldn't find state"))
else
Terraform::State.create_or_find_by(find_params)
end
end end
def lock_matches?(state) def lock_matches?(state)
...@@ -73,5 +80,9 @@ module Terraform ...@@ -73,5 +80,9 @@ module Terraform
ActiveSupport::SecurityUtils ActiveSupport::SecurityUtils
.secure_compare(state.lock_xid.to_s, params[:lock_id].to_s) .secure_compare(state.lock_xid.to_s, params[:lock_id].to_s)
end end
def can_modify_state?
current_user.can?(:admin_terraform_state, project)
end
end end
end end
---
title: Allow developer role read-only access to Terraform state
merge_request: 33573
author:
type: added
...@@ -36,6 +36,14 @@ To get started with a GitLab-managed Terraform State, there are two different op ...@@ -36,6 +36,14 @@ To get started with a GitLab-managed Terraform State, there are two different op
- [Use a local machine](#get-started-using-local-development). - [Use a local machine](#get-started-using-local-development).
- [Use GitLab CI](#get-started-using-gitlab-ci). - [Use GitLab CI](#get-started-using-gitlab-ci).
## Permissions for using Terraform
In GitLab version 13.1, [Maintainer access](../permissions.md) was required to use a
GitLab managed Terraform state backend. In GitLab versions 13.2 and greater,
[Maintainer access](../permissions.md) is required to lock, unlock and write to the state
(using `terraform apply`), while [Developer access](../permissions.md) is required to read
the state (using `terraform plan -lock=false`).
## Get started using local development ## Get started using local development
If you plan to only run `terraform plan` and `terraform apply` commands from your If you plan to only run `terraform plan` and `terraform apply` commands from your
...@@ -54,8 +62,7 @@ local machine, this is a simple way to get started: ...@@ -54,8 +62,7 @@ local machine, this is a simple way to get started:
``` ```
1. Create a [Personal Access Token](../profile/personal_access_tokens.md) with 1. Create a [Personal Access Token](../profile/personal_access_tokens.md) with
the `api` scope. The Terraform backend is restricted to users with the `api` scope.
[Maintainer access](../permissions.md) to the repository.
1. On your local machine, run `terraform init`, passing in the following options, 1. On your local machine, run `terraform init`, passing in the following options,
replacing `<YOUR-PROJECT-NAME>`, `<YOUR-PROJECT-ID>`, `<YOUR-USERNAME>` and replacing `<YOUR-PROJECT-NAME>`, `<YOUR-PROJECT-ID>`, `<YOUR-USERNAME>` and
...@@ -89,10 +96,6 @@ Next, [configure the backend](#configure-the-backend). ...@@ -89,10 +96,6 @@ Next, [configure the backend](#configure-the-backend).
After executing the `terraform init` command, you must configure the Terraform backend After executing the `terraform init` command, you must configure the Terraform backend
and the CI YAML file: and the CI YAML file:
CAUTION: **Important:**
The Terraform backend is restricted to users with [Maintainer access](../permissions.md)
to the repository.
1. In your Terraform project, define the [HTTP backend](https://www.terraform.io/docs/backends/types/http.html) 1. In your Terraform project, define the [HTTP backend](https://www.terraform.io/docs/backends/types/http.html)
by adding the following code block in a `.tf` file (such as `backend.tf`) to by adding the following code block in a `.tf` file (such as `backend.tf`) to
define the remote backend: define the remote backend:
......
...@@ -142,6 +142,8 @@ The following table depicts the various user permission levels in a project. ...@@ -142,6 +142,8 @@ The following table depicts the various user permission levels in a project.
| Manage clusters | | | | ✓ | ✓ | | Manage clusters | | | | ✓ | ✓ |
| Manage Project Operations | | | | ✓ | ✓ | | Manage Project Operations | | | | ✓ | ✓ |
| View Pods logs | | | | ✓ | ✓ | | View Pods logs | | | | ✓ | ✓ |
| Read Terraform state | | | ✓ | ✓ | ✓ |
| Manage Terraform state | | | | ✓ | ✓ |
| Manage license policy **(ULTIMATE)** | | | | ✓ | ✓ | | Manage license policy **(ULTIMATE)** | | | | ✓ | ✓ |
| Edit comments (posted by any user) | | | | ✓ | ✓ | | Edit comments (posted by any user) | | | | ✓ | ✓ |
| Manage Error Tracking | | | | ✓ | ✓ | | Manage Error Tracking | | | | ✓ | ✓ |
......
...@@ -11,7 +11,7 @@ module API ...@@ -11,7 +11,7 @@ module API
before do before do
authenticate! authenticate!
authorize! :admin_terraform_state, user_project authorize! :read_terraform_state, user_project
end end
params do params do
...@@ -46,6 +46,8 @@ module API ...@@ -46,6 +46,8 @@ module API
desc 'Add a new terraform state or update an existing one' desc 'Add a new terraform state or update an existing one'
route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
post do post do
authorize! :admin_terraform_state, user_project
data = request.body.read data = request.body.read
no_content! if data.empty? no_content! if data.empty?
...@@ -59,6 +61,8 @@ module API ...@@ -59,6 +61,8 @@ module API
desc 'Delete a terraform state of a certain name' desc 'Delete a terraform state of a certain name'
route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth
delete do delete do
authorize! :admin_terraform_state, user_project
remote_state_handler.handle_with_lock do |state| remote_state_handler.handle_with_lock do |state|
state.destroy! state.destroy!
status :ok status :ok
...@@ -77,6 +81,8 @@ module API ...@@ -77,6 +81,8 @@ module API
requires :Path, type: String, desc: 'Terraform path' requires :Path, type: String, desc: 'Terraform path'
end end
post '/lock' do post '/lock' do
authorize! :admin_terraform_state, user_project
status_code = :ok status_code = :ok
lock_info = { lock_info = {
'Operation' => params[:Operation], 'Operation' => params[:Operation],
...@@ -108,6 +114,8 @@ module API ...@@ -108,6 +114,8 @@ module API
optional :ID, type: String, limit: 255, desc: 'Terraform state lock ID' optional :ID, type: String, limit: 255, desc: 'Terraform state lock ID'
end end
delete '/lock' do delete '/lock' do
authorize! :admin_terraform_state, user_project
remote_state_handler.unlock! remote_state_handler.unlock!
status :ok status :ok
rescue ::Terraform::RemoteStateHandler::StateLockedError rescue ::Terraform::RemoteStateHandler::StateLockedError
......
...@@ -9,5 +9,11 @@ FactoryBot.define do ...@@ -9,5 +9,11 @@ FactoryBot.define do
trait :with_file do trait :with_file do
file { fixture_file_upload('spec/fixtures/terraform/terraform.tfstate', 'application/json') } file { fixture_file_upload('spec/fixtures/terraform/terraform.tfstate', 'application/json') }
end end
trait :locked do
sequence(:lock_xid) { |n| "lock-#{n}" }
locked_at { Time.current }
locked_by_user { create(:user) }
end
end end
end end
...@@ -46,6 +46,7 @@ RSpec.describe ProjectPolicy do ...@@ -46,6 +46,7 @@ RSpec.describe ProjectPolicy do
resolve_note create_container_image update_container_image destroy_container_image daily_statistics resolve_note create_container_image update_container_image destroy_container_image daily_statistics
create_environment update_environment create_deployment update_deployment create_release update_release create_environment update_environment create_deployment update_deployment create_release update_release
create_metrics_dashboard_annotation delete_metrics_dashboard_annotation update_metrics_dashboard_annotation create_metrics_dashboard_annotation delete_metrics_dashboard_annotation update_metrics_dashboard_annotation
read_terraform_state
] ]
end end
......
...@@ -59,10 +59,11 @@ RSpec.describe API::Terraform::State do ...@@ -59,10 +59,11 @@ RSpec.describe API::Terraform::State do
context 'with developer permissions' do context 'with developer permissions' do
let(:current_user) { developer } let(:current_user) { developer }
it 'returns forbidden if the user cannot access the state' do it 'returns terraform state belonging to a project of given state name' do
request request
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:ok)
expect(response.body).to eq(state.file.read)
end end
end end
end end
...@@ -94,10 +95,11 @@ RSpec.describe API::Terraform::State do ...@@ -94,10 +95,11 @@ RSpec.describe API::Terraform::State do
context 'with developer permissions' do context 'with developer permissions' do
let(:job) { create(:ci_build, project: project, user: developer) } let(:job) { create(:ci_build, project: project, user: developer) }
it 'returns forbidden if the user cannot access the state' do it 'returns terraform state belonging to a project of given state name' do
request request
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:ok)
expect(response.body).to eq(state.file.read)
end end
end end
end end
...@@ -235,9 +237,43 @@ RSpec.describe API::Terraform::State do ...@@ -235,9 +237,43 @@ RSpec.describe API::Terraform::State do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end
context 'state is already locked' do
before do
state.update!(lock_xid: 'locked', locked_by_user: current_user)
end
it 'returns an error' do
request
expect(response).to have_gitlab_http_status(:conflict)
end
end
context 'user does not have permission to lock the state' do
let(:current_user) { developer }
it 'returns an error' do
request
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end end
describe 'DELETE /projects/:id/terraform/state/:name/lock' do describe 'DELETE /projects/:id/terraform/state/:name/lock' do
let(:params) do
{
ID: lock_id,
Version: '0.1',
Operation: 'OperationTypePlan',
Info: '',
Who: "#{current_user.username}",
Created: Time.now.utc.iso8601(6),
Path: ''
}
end
before do before do
state.lock_xid = '123-456' state.lock_xid = '123-456'
state.save! state.save!
...@@ -246,7 +282,7 @@ RSpec.describe API::Terraform::State do ...@@ -246,7 +282,7 @@ RSpec.describe API::Terraform::State do
subject(:request) { delete api("#{state_path}/lock"), headers: auth_header, params: params } subject(:request) { delete api("#{state_path}/lock"), headers: auth_header, params: params }
context 'with the correct lock id' do context 'with the correct lock id' do
let(:params) { { ID: '123-456' } } let(:lock_id) { '123-456' }
it 'removes the terraform state lock' do it 'removes the terraform state lock' do
request request
...@@ -266,7 +302,7 @@ RSpec.describe API::Terraform::State do ...@@ -266,7 +302,7 @@ RSpec.describe API::Terraform::State do
end end
context 'with an incorrect lock id' do context 'with an incorrect lock id' do
let(:params) { { ID: '456-789' } } let(:lock_id) { '456-789' }
it 'returns an error' do it 'returns an error' do
request request
...@@ -276,7 +312,7 @@ RSpec.describe API::Terraform::State do ...@@ -276,7 +312,7 @@ RSpec.describe API::Terraform::State do
end end
context 'with a longer than 255 character lock id' do context 'with a longer than 255 character lock id' do
let(:params) { { ID: '0' * 256 } } let(:lock_id) { '0' * 256 }
it 'returns an error' do it 'returns an error' do
request request
...@@ -284,5 +320,16 @@ RSpec.describe API::Terraform::State do ...@@ -284,5 +320,16 @@ RSpec.describe API::Terraform::State do
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
end end
end end
context 'user does not have permission to unlock the state' do
let(:lock_id) { '123-456' }
let(:current_user) { developer }
it 'returns an error' do
request
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end end
end end
...@@ -4,7 +4,10 @@ require 'spec_helper' ...@@ -4,7 +4,10 @@ require 'spec_helper'
RSpec.describe Terraform::RemoteStateHandler do RSpec.describe Terraform::RemoteStateHandler do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) } let_it_be(:developer) { create(:user, developer_projects: [project]) }
let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) }
let_it_be(:user) { maintainer }
describe '#find_with_lock' do describe '#find_with_lock' do
context 'without a state name' do context 'without a state name' do
...@@ -34,33 +37,6 @@ RSpec.describe Terraform::RemoteStateHandler do ...@@ -34,33 +37,6 @@ RSpec.describe Terraform::RemoteStateHandler do
end end
end end
describe '#create_or_find!' do
it 'requires passing a state name' do
handler = described_class.new(project, user)
expect { handler.create_or_find! }.to raise_error(ArgumentError)
end
it 'allows to create states with same name in different projects' do
project_b = create(:project)
state_a = described_class.new(project, user, name: 'my-state').create_or_find!
state_b = described_class.new(project_b, user, name: 'my-state').create_or_find!
expect(state_a).to be_persisted
expect(state_b).to be_persisted
expect(state_a.id).not_to eq state_b.id
end
it 'loads the same state upon subsequent call in the project scope' do
state_a = described_class.new(project, user, name: 'my-state').create_or_find!
state_b = described_class.new(project, user, name: 'my-state').create_or_find!
expect(state_a).to be_persisted
expect(state_a.id).to eq state_b.id
end
end
context 'when state locking is not being used' do context 'when state locking is not being used' do
subject { described_class.new(project, user, name: 'my-state') } subject { described_class.new(project, user, name: 'my-state') }
...@@ -74,7 +50,7 @@ RSpec.describe Terraform::RemoteStateHandler do ...@@ -74,7 +50,7 @@ RSpec.describe Terraform::RemoteStateHandler do
end end
it 'returns the state object itself' do it 'returns the state object itself' do
state = subject.create_or_find! state = subject.handle_with_lock
expect(state.name).to eq 'my-state' expect(state.name).to eq 'my-state'
end end
...@@ -89,10 +65,9 @@ RSpec.describe Terraform::RemoteStateHandler do ...@@ -89,10 +65,9 @@ RSpec.describe Terraform::RemoteStateHandler do
context 'when using locking' do context 'when using locking' do
describe '#handle_with_lock' do describe '#handle_with_lock' do
it 'handles a locked state using exclusive read lock' do subject(:handler) { described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc') }
handler = described_class
.new(project, user, name: 'new-state', lock_id: 'abc-abc')
it 'handles a locked state using exclusive read lock' do
handler.lock! handler.lock!
state = handler.handle_with_lock do |state| state = handler.handle_with_lock do |state|
...@@ -101,20 +76,35 @@ RSpec.describe Terraform::RemoteStateHandler do ...@@ -101,20 +76,35 @@ RSpec.describe Terraform::RemoteStateHandler do
expect(state.name).to eq 'new-name' expect(state.name).to eq 'new-name'
end end
end
it 'raises exception if lock has not been acquired before' do it 'raises exception if lock has not been acquired before' do
handler = described_class expect { handler.handle_with_lock }
.new(project, user, name: 'new-state', lock_id: 'abc-abc') .to raise_error(described_class::StateLockedError)
end
context 'user does not have permission to modify state' do
let(:user) { developer }
expect { handler.handle_with_lock } it 'raises an exception' do
.to raise_error(described_class::StateLockedError) expect { handler.handle_with_lock }
.to raise_error(described_class::UnauthorizedError)
end
end
end end
describe '#lock!' do describe '#lock!' do
it 'allows to lock state if it does not exist yet' do let(:lock_id) { 'abc-abc' }
handler = described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc')
subject(:handler) do
described_class.new(
project,
user,
name: 'new-state',
lock_id: lock_id
)
end
it 'allows to lock state if it does not exist yet' do
state = handler.lock! state = handler.lock!
expect(state).to be_persisted expect(state).to be_persisted
...@@ -122,22 +112,61 @@ RSpec.describe Terraform::RemoteStateHandler do ...@@ -122,22 +112,61 @@ RSpec.describe Terraform::RemoteStateHandler do
end end
it 'allows to lock state if it exists and is not locked' do it 'allows to lock state if it exists and is not locked' do
state = described_class.new(project, user, name: 'new-state').create_or_find! state = create(:terraform_state, project: project, name: 'new-state')
handler = described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc')
handler.lock! handler.lock!
expect(state.reload.lock_xid).to eq 'abc-abc' expect(state.reload.lock_xid).to eq lock_id
expect(state).to be_locked expect(state).to be_locked
end end
it 'raises an exception when trying to unlocked state locked by someone else' do it 'raises an exception when trying to unlocked state locked by someone else' do
described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc').lock! described_class.new(project, user, name: 'new-state', lock_id: '12a-23f').lock!
handler = described_class.new(project, user, name: 'new-state', lock_id: '12a-23f')
expect { handler.lock! }.to raise_error(described_class::StateLockedError) expect { handler.lock! }.to raise_error(described_class::StateLockedError)
end end
end end
describe '#unlock!' do
let(:lock_id) { 'abc-abc' }
subject(:handler) do
described_class.new(
project,
user,
name: 'new-state',
lock_id: lock_id
)
end
before do
create(:terraform_state, :locked, project: project, name: 'new-state', lock_xid: 'abc-abc')
end
it 'unlocks the state' do
state = handler.unlock!
expect(state.lock_xid).to be_nil
end
context 'with no lock ID (force-unlock)' do
let(:lock_id) { }
it 'unlocks the state' do
state = handler.unlock!
expect(state.lock_xid).to be_nil
end
end
context 'with different lock ID' do
let(:lock_id) { 'other' }
it 'raises an exception' do
expect { handler.unlock! }
.to raise_error(described_class::StateLockedError)
end
end
end
end end
end end
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