From 1b524071f455668888831b4bccde7736dd1fb227 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 1 Apr 2014 11:59:09 +0200 Subject: [PATCH 01/10] Allow to handle settings collections --- Gemfile | 1 + Gemfile.lock | 3 + lib/travis/api/app.rb | 7 +- lib/travis/api/app/endpoint/repos.rb | 9 - .../api/app/endpoint/setting_endpoint.rb | 94 +++++++++ lib/travis/api/app/middleware/rewrite.rb | 5 +- lib/travis/api/app/responders/json.rb | 8 +- lib/travis/api/serializer.rb | 23 +++ lib/travis/api/v2/http.rb | 1 + lib/travis/api/v2/http/error.rb | 1 + lib/travis/api/v2/http/validation_error.rb | 39 ++++ spec/integration/settings_endpoint_spec.rb | 178 ++++++++++++++++++ spec/spec_helper.rb | 5 + 13 files changed, 362 insertions(+), 12 deletions(-) create mode 100644 lib/travis/api/app/endpoint/setting_endpoint.rb create mode 100644 lib/travis/api/serializer.rb create mode 100644 lib/travis/api/v2/http/error.rb create mode 100644 lib/travis/api/v2/http/validation_error.rb create mode 100644 spec/integration/settings_endpoint_spec.rb diff --git a/Gemfile b/Gemfile index 58bc4852..69604289 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,7 @@ gem 'travis-sidekiqs', github: 'travis-ci/travis-sidekiqs', require: nil, ref: ' gem 'sinatra' gem 'sinatra-contrib', require: nil #github: 'sinatra/sinatra-contrib', require: nil +gem 'active_model_serializers' gem 'unicorn' gem 'sentry-raven', github: 'getsentry/raven-ruby' gem 'yard-sinatra', github: 'rkh/yard-sinatra' diff --git a/Gemfile.lock b/Gemfile.lock index 3dfced19..ea2e6e73 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -96,6 +96,8 @@ GEM rack-cache (~> 1.2) rack-test (~> 0.6.1) sprockets (~> 2.2.1) + active_model_serializers (0.8.1) + activemodel (>= 3.0) activemodel (3.2.17) activesupport (= 3.2.17) builder (~> 3.0.0) @@ -279,6 +281,7 @@ PLATFORMS ruby DEPENDENCIES + active_model_serializers bunny (~> 0.8.0) dalli database_cleaner (~> 0.8.0) diff --git a/lib/travis/api/app.rb b/lib/travis/api/app.rb index c15a6146..80fa6e26 100644 --- a/lib/travis/api/app.rb +++ b/lib/travis/api/app.rb @@ -18,6 +18,7 @@ require 'sidekiq' require 'metriks/reporter/logger' require 'travis/support/log_subscriber/active_record_metrics' require 'fileutils' +require 'travis/api/serializer' # Rack class implementing the HTTP API. # Instances respond to #call. @@ -113,7 +114,11 @@ module Travis::Api use Travis::Api::App::Middleware::Metriks use Travis::Api::App::Middleware::Rewrite - Endpoint.subclasses.each { |e| map(e.prefix) { run(e.new) } } + Endpoint.subclasses.each do |e| + next if e == SettingsEndpoint # TODO: add something like abstract? method to check if + # class should be registered + map(e.prefix) { run(e.new) } + end end end diff --git a/lib/travis/api/app/endpoint/repos.rb b/lib/travis/api/app/endpoint/repos.rb index e0d01cbe..9dafd733 100644 --- a/lib/travis/api/app/endpoint/repos.rb +++ b/lib/travis/api/app/endpoint/repos.rb @@ -41,15 +41,6 @@ class Travis::Api::App respond_with service(:find_repo, params.merge(schema: 'cc')) end - get '/:id/settings/ssh_keys' do - settings = service(:find_repo_settings, params).run - if settings - respond_with({ settings: settings.obfuscated }, version: :v2) - else - status 404 - end - end - # Get settings for a given repository # get '/:id/settings' do diff --git a/lib/travis/api/app/endpoint/setting_endpoint.rb b/lib/travis/api/app/endpoint/setting_endpoint.rb new file mode 100644 index 00000000..44bc047f --- /dev/null +++ b/lib/travis/api/app/endpoint/setting_endpoint.rb @@ -0,0 +1,94 @@ +require 'travis/api/app' + +class Travis::Api::App + class SettingsEndpoint < Endpoint + set(:prefix) { "/settings/" << name[/[^:]+$/].underscore } + + class << self + # This method checks if class based on a given name exists or creates + # a new SettingsEndpoint subclass, which will be then used as an endpoint + def subclass(name) + class_name = name.to_s.camelize + if Travis::Api::App.const_defined?(class_name) + Travis::Api::App.const_get(class_name) + else + klass = create_settings_class(name) + Travis::Api::App.const_set(class_name, klass) + klass + end + end + + def create_settings_class(name) + klass = Class.new(self) do + define_method(:name) { name } + get("/", scope: :private) do index end + get("/:id", scope: :private) do show end + post("/", scope: :private) do create end + patch("/:id", scope: :private) do update end + delete("/:id", scope: :private) do destroy end + end + end + end + + # Rails style methods for easy overriding + def index + respond_with(collection, type: name, verson: :v2) + end + + def show + respond_with(record, type: singular_name, verson: :v2) + end + + def update + record.update(params[singular_name]) + if record.valid? + repo_settings.save + respond_with(record, type: singular_name, version: :v2) + else + status 422 + respond_with(record, type: :validation_error, version: :v2) + end + end + + def create + record = collection.create(params[singular_name]) + if record.valid? + repo_settings.save + respond_with(record, type: singular_name, version: :v2) + else + status 422 + respond_with(record, type: :validation_error, version: :v2) + end + end + + def destroy + record = collection.destroy(params[:id]) || record_not_found + repo_settings.save + respond_with(record, type: singular_name, verson: :v2) + end + + def singular_name + name.to_s.singularize + end + + def collection + @collection ||= repo_settings.send(name) + end + + # This method can't be called "settings" because it clashes with + # Sinatra's method + def repo_settings + @settings ||= begin + service(:find_repo_settings, id: params['repository_id'].to_i).run + end || halt(404, error: "Couldn't find repository") + end + + def record + collection.find(params[:id]) || record_not_found + end + + def record_not_found + halt(404, { error: "Could not find a requested setting" }) + end + end +end diff --git a/lib/travis/api/app/middleware/rewrite.rb b/lib/travis/api/app/middleware/rewrite.rb index 893feb34..0c7b2e22 100644 --- a/lib/travis/api/app/middleware/rewrite.rb +++ b/lib/travis/api/app/middleware/rewrite.rb @@ -5,6 +5,7 @@ class Travis::Api::App class Rewrite < Middleware FORMAT = %r(\.(json|xml|png|txt|atom|svg)$) V1_REPO_URL = %r(^(/[^/]+/[^/]+(?:/builds(?:/[\d]+)?|/cc)?)$) + SETTINGS_URL = %r(^/settings) helpers :accept @@ -37,7 +38,9 @@ class Travis::Api::App end def redirect_v1_named_repo_path - force_redirect("/repositories#{$1}.#{env['travis.format']}") if request.path =~ V1_REPO_URL + if request.path =~ V1_REPO_URL && request.path !~ SETTINGS_URL + force_redirect("/repositories#{$1}.#{env['travis.format']}") + end end def force_redirect(path) diff --git a/lib/travis/api/app/responders/json.rb b/lib/travis/api/app/responders/json.rb index 012b2be8..3bc3bb8d 100644 --- a/lib/travis/api/app/responders/json.rb +++ b/lib/travis/api/app/responders/json.rb @@ -27,7 +27,13 @@ class Travis::Api::App end def result - builder ? builder.new(resource, params).data : basic_type_resource + if builder + p = params + p[:root] = options[:type] if options[:type] + builder.new(resource, p).data + else + basic_type_resource + end end def builder diff --git a/lib/travis/api/serializer.rb b/lib/travis/api/serializer.rb new file mode 100644 index 00000000..a34ecdff --- /dev/null +++ b/lib/travis/api/serializer.rb @@ -0,0 +1,23 @@ +require 'active_model_serializers' + +module Travis + module Api + class Serializer < ActiveModel::Serializer + def data + as_json + end + end + + class ArraySerializer < ActiveModel::ArraySerializer + def data + as_json + end + + def initialize(resource, options) + options[:each_serializer] ||= Travis::Api::V2::Http.const_get(options[:root].to_s.singularize.camelize) + super(resource, options) + end + end + end +end + diff --git a/lib/travis/api/v2/http.rb b/lib/travis/api/v2/http.rb index 2746d130..6034ef07 100644 --- a/lib/travis/api/v2/http.rb +++ b/lib/travis/api/v2/http.rb @@ -21,6 +21,7 @@ module Travis require 'travis/api/v2/http/request' require 'travis/api/v2/http/ssl_key' require 'travis/api/v2/http/user' + require 'travis/api/v2/http/validation_error' end end end diff --git a/lib/travis/api/v2/http/error.rb b/lib/travis/api/v2/http/error.rb new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/lib/travis/api/v2/http/error.rb @@ -0,0 +1 @@ + diff --git a/lib/travis/api/v2/http/validation_error.rb b/lib/travis/api/v2/http/validation_error.rb new file mode 100644 index 00000000..dc5cfe33 --- /dev/null +++ b/lib/travis/api/v2/http/validation_error.rb @@ -0,0 +1,39 @@ +module Travis + module Api + module V2 + module Http + class ValidationError + attr_reader :resource + + def initialize(resource, options = {}) + @resource = resource + end + + def data + response = { + message: 'Validation failed' + } + resource.errors.to_hash.each do |name, errors| + response['errors'] ||= [] + errors.each do |error_code| + response['errors'] << { field: name, code: code(error_code) } + end + end + + response + end + + def code(error_code) + case error_code + when :blank + 'missing_field' + else + error_code.to_s + end + end + end + end + end + end +end + diff --git a/spec/integration/settings_endpoint_spec.rb b/spec/integration/settings_endpoint_spec.rb new file mode 100644 index 00000000..f7ae0dc7 --- /dev/null +++ b/spec/integration/settings_endpoint_spec.rb @@ -0,0 +1,178 @@ +require 'spec_helper' + +describe Travis::Api::App::SettingsEndpoint do + let(:repo) { Repository.by_slug('svenfuchs/minimal').first } + let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json' } } + + before do + model_class = Class.new(Repository::Settings::Model) do + field :name + field :secret, encrypted: true + + validates :name, presence: true + validates :secret, presence: true + end + collection_class = Class.new(Repository::Settings::Collection) do + model model_class + end + Repository::Settings.class_eval do + register :items, collection_class + end + serializer_class = Class.new(Travis::Api::Serializer) do + attributes :id, :name + end + Travis::Api::V2::Http.const_set(:Item, serializer_class) + Travis::Api::V2::Http.const_set(:Items, Travis::Api::ArraySerializer) + + add_settings_endpoint :items + end + + after do + Travis::Api::App.send :remove_const, :Items + Travis::Api::V2::Http.send :remove_const, :Items + Travis::Api::V2::Http.send :remove_const, :Item + end + + describe 'with authenticated user' do + let(:user) { User.where(login: 'svenfuchs').first } + let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } + let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json', 'HTTP_AUTHORIZATION' => "token #{token}" } } + + before { user.permissions.create!(:repository_id => repo.id, :admin => true, :push => true) } + + describe 'GET /items/:id' do + it 'returns an item' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + response = get '/settings/items/' + item.id, { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'an item' + json['item']['id'].should == item.id + json['item'].should_not have_key('secret') + end + + it 'returns 404 if item can\'t be found' do + response = get '/settings/items/123', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['error'].should == "Could not find a requested setting" + end + end + + describe 'GET /items' do + it 'returns items list' do + settings = repo.settings + settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + response = get '/settings/items', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['items'].should have(1).items + item = json['items'].first + item['name'].should == 'an item' + item['id'].should_not be_nil + item.should_not have_key('secret') + end + end + + describe 'POST /items' do + it 'creates a new item' do + params = { repository_id: repo.id, item: { name: 'foo', secret: 'TEH SECRET' } } + response = post '/settings/items', params, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'foo' + json['item']['id'].should_not be_nil + json['item'].should_not have_key('secret') + + item = repo.reload.settings.items.first + item.id.should_not be_nil + item.name.should == 'foo' + item.secret.decrypt.should == 'TEH SECRET' + end + + it 'returns error message if item is invalid' do + params = { repository_id: repo.id } + response = post '/settings/items', params, headers + response.status.should == 422 + + json = JSON.parse(response.body) + json['message'].should == 'Validation failed' + json['errors'].should == [{ + 'field' => 'name', + 'code' => 'missing_field' + }, { + 'field' => 'secret', + 'code' => 'missing_field' + }] + + repo.reload.settings.items.length.should == 0 + end + end + + describe 'PATCH /items/:id' do + it 'should update an item' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + params = { repository_id: repo.id, item: { name: 'a new name', secret: 'a new secret' } } + response = patch '/settings/items/' + item.id, params, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'a new name' + json['item']['id'].should == item.id + json['item'].should_not have_key('secret') + + updated_item = repo.reload.settings.items.find(item.id) + updated_item.id.should == item.id + updated_item.name.should == 'a new name' + updated_item.secret.decrypt.should == 'a new secret' + end + + it 'returns an error message if item is invalid' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + params = { repository_id: repo.id, item: { name: '' } } + response = patch '/settings/items/' + item.id, params, headers + response.status.should == 422 + + json = JSON.parse(response.body) + json['message'].should == 'Validation failed' + json['errors'].should == [{ + 'field' => 'name', + 'code' => 'missing_field' + }] + + updated_item = repo.reload.settings.items.find(item.id) + updated_item.id.should == item.id + updated_item.name.should == 'an item' + updated_item.secret.decrypt.should == 'TEH SECRET' + end + end + + describe 'DELETE /items/:id' do + it 'should delete an item' do + settings = repo.settings + item = settings.items.create(name: 'an item', secret: 'TEH SECRET') + settings.save + + params = { repository_id: repo.id } + response = delete '/settings/items/' + item.id, params, headers + json = JSON.parse(response.body) + json['item']['name'].should == 'an item' + json['item']['id'].should == item.id + json['item'].should_not have_key('secret') + + repo.reload.settings.items.should have(0).items + end + + it 'returns 404 if item can\'t be found' do + response = delete '/settings/items/123', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['error'].should == "Could not find a requested setting" + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 43db91da..197cb126 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,6 +26,11 @@ module TestHelpers @custom_endpoints ||= [] end + def add_settings_endpoint(name) + Travis::Api::App::SettingsEndpoint.subclass(name) + set_app Travis::Api::App.new + end + def add_endpoint(prefix, &block) endpoint = Sinatra.new(Travis::Api::App::Endpoint, &block) endpoint.set(prefix: prefix) From e9cdef1c9b56d44fbd2bd4fe2e4f2bb7775d7015 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 1 Apr 2014 13:05:25 +0200 Subject: [PATCH 02/10] Add /settings/ssh_keys API --- lib/travis/api/app.rb | 2 ++ lib/travis/api/v2/http.rb | 4 +++ lib/travis/api/v2/http/ssh_key.rb | 11 ++++++ lib/travis/api/v2/http/ssh_keys.rb | 2 ++ spec/integration/v2/settings/ssh_keys_spec.rb | 34 +++++++++++++++++++ 5 files changed, 53 insertions(+) create mode 100644 lib/travis/api/v2/http/ssh_key.rb create mode 100644 lib/travis/api/v2/http/ssh_keys.rb create mode 100644 spec/integration/v2/settings/ssh_keys_spec.rb diff --git a/lib/travis/api/app.rb b/lib/travis/api/app.rb index 80fa6e26..e8b290f4 100644 --- a/lib/travis/api/app.rb +++ b/lib/travis/api/app.rb @@ -114,6 +114,8 @@ module Travis::Api use Travis::Api::App::Middleware::Metriks use Travis::Api::App::Middleware::Rewrite + SettingsEndpoint.subclass :ssh_keys + Endpoint.subclasses.each do |e| next if e == SettingsEndpoint # TODO: add something like abstract? method to check if # class should be registered diff --git a/lib/travis/api/v2/http.rb b/lib/travis/api/v2/http.rb index 6034ef07..ca1384cf 100644 --- a/lib/travis/api/v2/http.rb +++ b/lib/travis/api/v2/http.rb @@ -1,3 +1,5 @@ +require 'travis/api/serializer' + module Travis module Api module V2 @@ -20,6 +22,8 @@ module Travis require 'travis/api/v2/http/requests' require 'travis/api/v2/http/request' require 'travis/api/v2/http/ssl_key' + require 'travis/api/v2/http/ssh_key' + require 'travis/api/v2/http/ssh_keys' require 'travis/api/v2/http/user' require 'travis/api/v2/http/validation_error' end diff --git a/lib/travis/api/v2/http/ssh_key.rb b/lib/travis/api/v2/http/ssh_key.rb new file mode 100644 index 00000000..a785f365 --- /dev/null +++ b/lib/travis/api/v2/http/ssh_key.rb @@ -0,0 +1,11 @@ +module Travis + module Api + module V2 + module Http + class SshKey < Travis::Api::Serializer + attributes :id, :name + end + end + end + end +end diff --git a/lib/travis/api/v2/http/ssh_keys.rb b/lib/travis/api/v2/http/ssh_keys.rb new file mode 100644 index 00000000..d93829be --- /dev/null +++ b/lib/travis/api/v2/http/ssh_keys.rb @@ -0,0 +1,2 @@ +class Travis::Api::V2::Http::SshKeys < Travis::Api::ArraySerializer +end diff --git a/spec/integration/v2/settings/ssh_keys_spec.rb b/spec/integration/v2/settings/ssh_keys_spec.rb new file mode 100644 index 00000000..84566da5 --- /dev/null +++ b/spec/integration/v2/settings/ssh_keys_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe Travis::Api::App::SettingsEndpoint do + let(:repo) { Repository.by_slug('svenfuchs/minimal').first } + let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json' } } + + describe 'with authenticated user' do + let(:user) { User.where(login: 'svenfuchs').first } + let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } + let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json', 'HTTP_AUTHORIZATION' => "token #{token}" } } + + before { user.permissions.create!(:repository_id => repo.id, :admin => true, :push => true) } + + describe 'GET /items/:id' do + it 'returns an item' do + settings = repo.settings + record = settings.ssh_keys.create(name: 'key for my repo', content: 'the key') + settings.save + + response = get '/settings/ssh_keys/' + record.id, { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['ssh_key']['name'].should == 'key for my repo' + json['ssh_key']['id'].should == record.id + json['ssh_key'].should_not have_key('secret') + end + + it 'returns 404 if ssh_key can\'t be found' do + response = get '/settings/ssh_key/123', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['error'].should == "Could not find a requested setting" + end + end + end +end From 5b0af649597cd7e76e6c8bdda8c5d1ac75094c5c Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 1 Apr 2014 15:34:21 +0200 Subject: [PATCH 03/10] Use travis-core from ps-settings-encryption branch --- Gemfile | 4 ++-- Gemfile.lock | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 69604289..637f9792 100644 --- a/Gemfile +++ b/Gemfile @@ -3,11 +3,12 @@ ruby '2.0.0' source 'https://rubygems.org' gemspec -gem 'travis-core', github: 'travis-ci/travis-core' +gem 'travis-core', github: 'travis-ci/travis-core', branch: 'ps-settings-encryption' gem 'travis-support', github: 'travis-ci/travis-support' gem 'travis-sidekiqs', github: 'travis-ci/travis-sidekiqs', require: nil, ref: 'cde9741' gem 'sinatra' gem 'sinatra-contrib', require: nil #github: 'sinatra/sinatra-contrib', require: nil +gem 'debugger' gem 'active_model_serializers' gem 'unicorn' @@ -32,7 +33,6 @@ end group :development do gem 'foreman' gem 'rerun' - # gem 'debugger' gem 'rb-fsevent', '~> 0.9.1' end diff --git a/Gemfile.lock b/Gemfile.lock index ea2e6e73..cdda0d5b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,12 +23,14 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: dd94b9d19ac4cc33f383f740194ba1b3c6a73d3a + revision: bb1b9584dfd970fb6ab1d6b6b3efbbacc51e1970 + branch: ps-settings-encryption specs: travis-core (0.0.1) actionmailer (~> 3.2.12) activerecord (~> 3.2.12) coder (~> 0.4.0) + coercible (~> 1.0.0) data_migrations (~> 0.0.1) gh hashr (~> 0.0.19) @@ -121,6 +123,9 @@ GEM timers (>= 1.0.0) coder (0.4.0) coderay (1.1.0) + coercible (1.0.0) + descendants_tracker (~> 0.0.1) + columnize (0.3.6) connection_pool (0.9.3) daemons (1.1.9) dalli (2.6.4) @@ -128,6 +133,14 @@ GEM activerecord rake database_cleaner (0.8.0) + debugger (1.6.2) + columnize (>= 0.3.1) + debugger-linecache (~> 1.2.0) + debugger-ruby_core_source (~> 1.2.3) + debugger-linecache (1.2.0) + debugger-ruby_core_source (1.2.3) + descendants_tracker (0.0.4) + thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.2.5) dotenv (0.9.0) erubis (2.7.0) @@ -264,6 +277,8 @@ GEM eventmachine (>= 1.0.0) rack (>= 1.0.0) thor (0.14.6) + thread_safe (0.3.1) + atomic (>= 1.1.7, < 2) tilt (1.4.1) timers (1.1.0) treetop (1.4.15) @@ -285,6 +300,7 @@ DEPENDENCIES bunny (~> 0.8.0) dalli database_cleaner (~> 0.8.0) + debugger factory_girl (~> 2.4.0) foreman gh From 87d3cbb96badc18bed1c3b2eecd1a53dd1541806 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 1 Apr 2014 16:25:41 +0200 Subject: [PATCH 04/10] Remove debugger from Gemfile --- Gemfile | 1 - Gemfile.lock | 8 -------- 2 files changed, 9 deletions(-) diff --git a/Gemfile b/Gemfile index 637f9792..ef8eb92e 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,6 @@ gem 'travis-support', github: 'travis-ci/travis-support' gem 'travis-sidekiqs', github: 'travis-ci/travis-sidekiqs', require: nil, ref: 'cde9741' gem 'sinatra' gem 'sinatra-contrib', require: nil #github: 'sinatra/sinatra-contrib', require: nil -gem 'debugger' gem 'active_model_serializers' gem 'unicorn' diff --git a/Gemfile.lock b/Gemfile.lock index cdda0d5b..3686fc50 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -125,7 +125,6 @@ GEM coderay (1.1.0) coercible (1.0.0) descendants_tracker (~> 0.0.1) - columnize (0.3.6) connection_pool (0.9.3) daemons (1.1.9) dalli (2.6.4) @@ -133,12 +132,6 @@ GEM activerecord rake database_cleaner (0.8.0) - debugger (1.6.2) - columnize (>= 0.3.1) - debugger-linecache (~> 1.2.0) - debugger-ruby_core_source (~> 1.2.3) - debugger-linecache (1.2.0) - debugger-ruby_core_source (1.2.3) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) diff-lcs (1.2.5) @@ -300,7 +293,6 @@ DEPENDENCIES bunny (~> 0.8.0) dalli database_cleaner (~> 0.8.0) - debugger factory_girl (~> 2.4.0) foreman gh From 11e814055e492a3bc3b85fa44ca2e504d3c296bb Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 1 Apr 2014 16:52:33 +0200 Subject: [PATCH 05/10] Fix specs --- spec/integration/v2/repositories_spec.rb | 11 ++++------- spec/integration/v2/settings/ssh_keys_spec.rb | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/spec/integration/v2/repositories_spec.rb b/spec/integration/v2/repositories_spec.rb index 30f2d100..166f4b86 100644 --- a/spec/integration/v2/repositories_spec.rb +++ b/spec/integration/v2/repositories_spec.rb @@ -25,21 +25,18 @@ describe 'Repos' do end it 'allows to update settings' do - json = { 'settings' => { 'a-new-setting' => 'value' } }.to_json + json = { 'settings' => { 'build_pushes' => false } }.to_json response = patch "repos/#{repo.id}/settings", json, headers - repo.reload.settings['a-new-setting'].should == 'value' + repo.reload.settings['build_pushes'].should == false body = JSON.parse(response.body) - body['settings']['a-new-setting'].should == 'value' + body['settings']['build_pushes'].should == false end it 'allows to get settings' do - repo.settings.replace('foo' => { 'type' => 'password', 'value' => 'abc123' }) - repo.save - response = get "repos/#{repo.id}/settings", {}, headers - settings = Repository::Settings.defaults.deep_merge({ 'foo' => { 'type' => 'password', 'value' => '∗∗∗∗∗∗' } }) + settings = Repository::Settings.defaults JSON.parse(response.body).should == { 'settings' => settings } end end diff --git a/spec/integration/v2/settings/ssh_keys_spec.rb b/spec/integration/v2/settings/ssh_keys_spec.rb index 84566da5..e78cf117 100644 --- a/spec/integration/v2/settings/ssh_keys_spec.rb +++ b/spec/integration/v2/settings/ssh_keys_spec.rb @@ -25,7 +25,7 @@ describe Travis::Api::App::SettingsEndpoint do end it 'returns 404 if ssh_key can\'t be found' do - response = get '/settings/ssh_key/123', { repository_id: repo.id }, headers + response = get '/settings/ssh_keys/123', { repository_id: repo.id }, headers json = JSON.parse(response.body) json['error'].should == "Could not find a requested setting" end From 54fb58a941c242d5654f3f93db0a5df503adef84 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 1 Apr 2014 16:58:36 +0200 Subject: [PATCH 06/10] Special case for settings in rewrite is not needed --- lib/travis/api/app/middleware/rewrite.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/travis/api/app/middleware/rewrite.rb b/lib/travis/api/app/middleware/rewrite.rb index 0c7b2e22..34346e37 100644 --- a/lib/travis/api/app/middleware/rewrite.rb +++ b/lib/travis/api/app/middleware/rewrite.rb @@ -5,7 +5,6 @@ class Travis::Api::App class Rewrite < Middleware FORMAT = %r(\.(json|xml|png|txt|atom|svg)$) V1_REPO_URL = %r(^(/[^/]+/[^/]+(?:/builds(?:/[\d]+)?|/cc)?)$) - SETTINGS_URL = %r(^/settings) helpers :accept @@ -38,7 +37,7 @@ class Travis::Api::App end def redirect_v1_named_repo_path - if request.path =~ V1_REPO_URL && request.path !~ SETTINGS_URL + if request.path =~ V1_REPO_URL force_redirect("/repositories#{$1}.#{env['travis.format']}") end end From 3bc94a995363d39df01587fcef77731e89580e82 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Wed, 9 Apr 2014 21:35:39 +0200 Subject: [PATCH 07/10] Bump travis-core --- Gemfile.lock | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 3686fc50..86683817 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,7 +23,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: bb1b9584dfd970fb6ab1d6b6b3efbbacc51e1970 + revision: e9e0b02da4c63a2cc0846275df1a819944e9311a branch: ps-settings-encryption specs: travis-core (0.0.1) @@ -111,9 +111,9 @@ GEM activesupport (3.2.17) i18n (~> 0.6, >= 0.6.4) multi_json (~> 1.0) - addressable (2.3.5) + addressable (2.3.6) arel (3.0.3) - atomic (1.1.15) + atomic (1.1.16) avl_tree (1.1.3) backports (2.8.2) builder (3.0.4) @@ -179,7 +179,7 @@ GEM mime-types (1.25.1) mocha (0.14.0) metaclass (~> 0.0.1) - multi_json (1.9.0) + multi_json (1.9.2) multipart-post (2.0.0) net-http-persistent (2.9.4) net-http-pipeline (1.0.1) @@ -201,7 +201,7 @@ GEM rack (>= 0.4) rack-protection (1.5.1) rack - rack-ssl (1.3.3) + rack-ssl (1.3.4) rack rack-test (0.6.2) rack (>= 1.0) @@ -270,8 +270,7 @@ GEM eventmachine (>= 1.0.0) rack (>= 1.0.0) thor (0.14.6) - thread_safe (0.3.1) - atomic (>= 1.1.7, < 2) + thread_safe (0.3.3) tilt (1.4.1) timers (1.1.0) treetop (1.4.15) From f107d4676ec869551ca52848f83ace19751de1b9 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Wed, 9 Apr 2014 21:36:16 +0200 Subject: [PATCH 08/10] Params in POST and PATCH requests should be fetched from request body This also adds missing specs for ssh_keys endpoint --- lib/travis/api/app.rb | 2 +- .../api/app/endpoint/setting_endpoint.rb | 10 +- spec/integration/settings_endpoint_spec.rb | 15 ++- spec/integration/v2/settings/ssh_keys_spec.rb | 116 +++++++++++++++++- 4 files changed, 127 insertions(+), 16 deletions(-) diff --git a/lib/travis/api/app.rb b/lib/travis/api/app.rb index e8b290f4..434e1d8e 100644 --- a/lib/travis/api/app.rb +++ b/lib/travis/api/app.rb @@ -18,7 +18,7 @@ require 'sidekiq' require 'metriks/reporter/logger' require 'travis/support/log_subscriber/active_record_metrics' require 'fileutils' -require 'travis/api/serializer' +require 'travis/api/v2/http' # Rack class implementing the HTTP API. # Instances respond to #call. diff --git a/lib/travis/api/app/endpoint/setting_endpoint.rb b/lib/travis/api/app/endpoint/setting_endpoint.rb index 44bc047f..b7c47b76 100644 --- a/lib/travis/api/app/endpoint/setting_endpoint.rb +++ b/lib/travis/api/app/endpoint/setting_endpoint.rb @@ -32,15 +32,15 @@ class Travis::Api::App # Rails style methods for easy overriding def index - respond_with(collection, type: name, verson: :v2) + respond_with(collection, type: name, version: :v2) end def show - respond_with(record, type: singular_name, verson: :v2) + respond_with(record, type: singular_name, version: :v2) end def update - record.update(params[singular_name]) + record.update(JSON.parse(request.body.read)[singular_name]) if record.valid? repo_settings.save respond_with(record, type: singular_name, version: :v2) @@ -51,7 +51,7 @@ class Travis::Api::App end def create - record = collection.create(params[singular_name]) + record = collection.create(JSON.parse(request.body.read)[singular_name]) if record.valid? repo_settings.save respond_with(record, type: singular_name, version: :v2) @@ -64,7 +64,7 @@ class Travis::Api::App def destroy record = collection.destroy(params[:id]) || record_not_found repo_settings.save - respond_with(record, type: singular_name, verson: :v2) + respond_with(record, type: singular_name, version: :v2) end def singular_name diff --git a/spec/integration/settings_endpoint_spec.rb b/spec/integration/settings_endpoint_spec.rb index f7ae0dc7..1df946b7 100644 --- a/spec/integration/settings_endpoint_spec.rb +++ b/spec/integration/settings_endpoint_spec.rb @@ -78,8 +78,8 @@ describe Travis::Api::App::SettingsEndpoint do describe 'POST /items' do it 'creates a new item' do - params = { repository_id: repo.id, item: { name: 'foo', secret: 'TEH SECRET' } } - response = post '/settings/items', params, headers + body = { item: { name: 'foo', secret: 'TEH SECRET' } }.to_json + response = post "/settings/items?repository_id=#{repo.id}", body, headers json = JSON.parse(response.body) json['item']['name'].should == 'foo' json['item']['id'].should_not be_nil @@ -92,8 +92,7 @@ describe Travis::Api::App::SettingsEndpoint do end it 'returns error message if item is invalid' do - params = { repository_id: repo.id } - response = post '/settings/items', params, headers + response = post "/settings/items?repository_id=#{repo.id}", '{}', headers response.status.should == 422 json = JSON.parse(response.body) @@ -116,8 +115,8 @@ describe Travis::Api::App::SettingsEndpoint do item = settings.items.create(name: 'an item', secret: 'TEH SECRET') settings.save - params = { repository_id: repo.id, item: { name: 'a new name', secret: 'a new secret' } } - response = patch '/settings/items/' + item.id, params, headers + body = { item: { name: 'a new name', secret: 'a new secret' } }.to_json + response = patch "/settings/items/#{item.id}?repository_id=#{repo.id}", body, headers json = JSON.parse(response.body) json['item']['name'].should == 'a new name' json['item']['id'].should == item.id @@ -134,8 +133,8 @@ describe Travis::Api::App::SettingsEndpoint do item = settings.items.create(name: 'an item', secret: 'TEH SECRET') settings.save - params = { repository_id: repo.id, item: { name: '' } } - response = patch '/settings/items/' + item.id, params, headers + body = { item: { name: '' } }.to_json + response = patch "/settings/items/#{item.id}?repository_id=#{repo.id}", body, headers response.status.should == 422 json = JSON.parse(response.body) diff --git a/spec/integration/v2/settings/ssh_keys_spec.rb b/spec/integration/v2/settings/ssh_keys_spec.rb index e78cf117..f13be3d0 100644 --- a/spec/integration/v2/settings/ssh_keys_spec.rb +++ b/spec/integration/v2/settings/ssh_keys_spec.rb @@ -11,7 +11,7 @@ describe Travis::Api::App::SettingsEndpoint do before { user.permissions.create!(:repository_id => repo.id, :admin => true, :push => true) } - describe 'GET /items/:id' do + describe 'GET /ssh_keys/:id' do it 'returns an item' do settings = repo.settings record = settings.ssh_keys.create(name: 'key for my repo', content: 'the key') @@ -21,7 +21,7 @@ describe Travis::Api::App::SettingsEndpoint do json = JSON.parse(response.body) json['ssh_key']['name'].should == 'key for my repo' json['ssh_key']['id'].should == record.id - json['ssh_key'].should_not have_key('secret') + json['ssh_key'].should_not have_key('content') end it 'returns 404 if ssh_key can\'t be found' do @@ -30,5 +30,117 @@ describe Travis::Api::App::SettingsEndpoint do json['error'].should == "Could not find a requested setting" end end + + describe 'GET /settings/ssh_keys' do + it 'returns a list of ssh_keys' do + settings = repo.settings + record = settings.ssh_keys.create(name: 'key for my repo', content: 'the key') + settings.save + + response = get '/settings/ssh_keys', { repository_id: repo.id }, headers + response.should be_successful + + json = JSON.parse(response.body) + key = json['ssh_keys'].first + key['name'].should == 'key for my repo' + key['id'].should == record.id + key.should_not have_key('content') + end + end + + describe 'POST /settings/ssh_keys' do + it 'creates a new key' do + body = { ssh_key: { name: 'foo', content: 'content' } }.to_json + response = post "/settings/ssh_keys?repository_id=#{repo.id}", body, headers + json = JSON.parse(response.body) + json['ssh_key']['name'].should == 'foo' + json['ssh_key']['id'].should_not be_nil + json['ssh_key'].should_not have_key('content') + + ssh_key = repo.reload.settings.ssh_keys.first + ssh_key.id.should_not be_nil + ssh_key.name.should == 'foo' + ssh_key.content.decrypt.should == 'content' + end + + it 'returns error message if a key is invalid' do + response = post "/settings/ssh_keys?repository_id=#{repo.id}", '{}', headers + response.status.should == 422 + + json = JSON.parse(response.body) + json['message'].should == 'Validation failed' + json['errors'].should == [{ + 'field' => 'name', + 'code' => 'missing_field' + }] + + repo.reload.settings.ssh_keys.length.should == 0 + end + end + + describe 'PATCH /settings/ssh_keys/:id' do + it 'should update a key' do + settings = repo.settings + ssh_key = settings.ssh_keys.create(name: 'foo', content: 'content') + settings.save + + body = { ssh_key: { name: 'bar', content: 'a new content' } }.to_json + response = patch "/settings/ssh_keys/#{ssh_key.id}?repository_id=#{repo.id}", body, headers + json = JSON.parse(response.body) + json['ssh_key']['name'].should == 'bar' + json['ssh_key']['id'].should == ssh_key.id + json['ssh_key'].should_not have_key('content') + + updated_ssh_key = repo.reload.settings.ssh_keys.find(ssh_key.id) + updated_ssh_key.id.should == ssh_key.id + updated_ssh_key.name.should == 'bar' + updated_ssh_key.content.decrypt.should == 'a new content' + end + + it 'returns an error message if ssh_key is invalid' do + settings = repo.settings + ssh_key = settings.ssh_keys.create(name: 'foo', content: 'content') + settings.save + + body = { ssh_key: { name: '' } }.to_json + response = patch "/settings/ssh_keys/#{ssh_key.id}?repository_id=#{repo.id}", body, headers + response.status.should == 422 + + json = JSON.parse(response.body) + json['message'].should == 'Validation failed' + json['errors'].should == [{ + 'field' => 'name', + 'code' => 'missing_field' + }] + + updated_ssh_key = repo.reload.settings.ssh_keys.find(ssh_key.id) + updated_ssh_key.id.should == ssh_key.id + updated_ssh_key.name.should == 'foo' + updated_ssh_key.content.decrypt.should == 'content' + end + end + + describe 'DELETE /ssh_keys/:id' do + it 'should delete an ssh_key' do + settings = repo.settings + ssh_key = settings.ssh_keys.create(name: 'foo', content: 'content') + settings.save + + params = { repository_id: repo.id } + response = delete '/settings/ssh_keys/' + ssh_key.id, params, headers + json = JSON.parse(response.body) + json['ssh_key']['name'].should == 'foo' + json['ssh_key']['id'].should == ssh_key.id + json['ssh_key'].should_not have_key('content') + + repo.reload.settings.ssh_keys.should have(0).ssh_keys + end + + it 'returns 404 if ssh_key can\'t be found' do + response = delete '/settings/ssh_keys/123', { repository_id: repo.id }, headers + json = JSON.parse(response.body) + json['error'].should == "Could not find a requested setting" + end + end end end From 3994b82f78e54733405263b4e04db1ee4345d023 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 15 Apr 2014 10:53:59 +0200 Subject: [PATCH 09/10] Bump travis-core and use master branch --- Gemfile | 2 +- Gemfile.lock | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index ef8eb92e..2267514f 100644 --- a/Gemfile +++ b/Gemfile @@ -3,7 +3,7 @@ ruby '2.0.0' source 'https://rubygems.org' gemspec -gem 'travis-core', github: 'travis-ci/travis-core', branch: 'ps-settings-encryption' +gem 'travis-core', github: 'travis-ci/travis-core' gem 'travis-support', github: 'travis-ci/travis-support' gem 'travis-sidekiqs', github: 'travis-ci/travis-sidekiqs', require: nil, ref: 'cde9741' gem 'sinatra' diff --git a/Gemfile.lock b/Gemfile.lock index 86683817..31b00d76 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,8 +23,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: e9e0b02da4c63a2cc0846275df1a819944e9311a - branch: ps-settings-encryption + revision: d2a19ee99f543eb2db376390847b682caa960b7f specs: travis-core (0.0.1) actionmailer (~> 3.2.12) From 17fd6201b895255b9de55b2422d12f1b2b263879 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Tue, 15 Apr 2014 10:58:08 +0200 Subject: [PATCH 10/10] Don't error out on settings We didn't have scope: :private specified on settings endpoint which resulted in errors (services check permissions of current user anyway, but it will error out if there is no user available). --- lib/travis/api/app/endpoint/repos.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/travis/api/app/endpoint/repos.rb b/lib/travis/api/app/endpoint/repos.rb index 9dafd733..b6be002c 100644 --- a/lib/travis/api/app/endpoint/repos.rb +++ b/lib/travis/api/app/endpoint/repos.rb @@ -43,7 +43,7 @@ class Travis::Api::App # Get settings for a given repository # - get '/:id/settings' do + get '/:id/settings', scope: :private do settings = service(:find_repo_settings, params).run if settings respond_with({ settings: settings.obfuscated }, version: :v2) @@ -52,7 +52,7 @@ class Travis::Api::App end end - patch '/:id/settings' do + patch '/:id/settings', scope: :private do payload = JSON.parse request.body.read if payload['settings'].blank? || !payload['settings'].is_a?(Hash)