summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG1
-rw-r--r--app/models/project_services/bamboo_service.rb20
-rw-r--r--spec/models/project_services/bamboo_service_spec.rb262
3 files changed, 221 insertions, 62 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 382318a203c..8646d99b64d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -18,6 +18,7 @@ v 8.7.0 (unreleased)
- API: Expose `subscribed` for issues and merge requests (Robert Schilling)
- Allow SAML to handle external users based on user's information !3530
- Add endpoints to archive or unarchive a project !3372
+ - Fix a bug whith trailing slash in bamboo_url
- Add links to CI setup documentation from project settings and builds pages
- Handle nil descriptions in Slack issue messages (Stan Hu)
- API: Expose open_issues_count, closed_issues_count, open_merge_requests_count for labels (Robert Schilling)
diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb
index 9e7f642180e..060062aaf7a 100644
--- a/app/models/project_services/bamboo_service.rb
+++ b/app/models/project_services/bamboo_service.rb
@@ -82,17 +82,17 @@ class BambooService < CiService
end
def build_info(sha)
- url = URI.parse("#{bamboo_url}/rest/api/latest/result?label=#{sha}")
+ url = URI.join(bamboo_url, "/rest/api/latest/result?label=#{sha}").to_s
if username.blank? && password.blank?
- @response = HTTParty.get(parsed_url.to_s, verify: false)
+ @response = HTTParty.get(url, verify: false)
else
- get_url = "#{url}&os_authType=basic"
+ url << '&os_authType=basic'
auth = {
- username: username,
- password: password,
+ username: username,
+ password: password
}
- @response = HTTParty.get(get_url, verify: false, basic_auth: auth)
+ @response = HTTParty.get(url, verify: false, basic_auth: auth)
end
end
@@ -101,11 +101,11 @@ class BambooService < CiService
if @response.code != 200 || @response['results']['results']['size'] == '0'
# If actual build link can't be determined, send user to build summary page.
- "#{bamboo_url}/browse/#{build_key}"
+ URI.join(bamboo_url, "/browse/#{build_key}").to_s
else
# If actual build link is available, go to build result page.
result_key = @response['results']['results']['result']['planResultKey']['key']
- "#{bamboo_url}/browse/#{result_key}"
+ URI.join(bamboo_url, "/browse/#{result_key}").to_s
end
end
@@ -134,7 +134,7 @@ class BambooService < CiService
return unless supported_events.include?(data[:object_kind])
# Bamboo requires a GET and does not take any data.
- self.class.get("#{bamboo_url}/updateAndBuild.action?buildKey=#{build_key}",
- verify: false)
+ url = URI.join(bamboo_url, "/updateAndBuild.action?buildKey=#{build_key}").to_s
+ self.class.get(url, verify: false)
end
end
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index c34b2487ecf..31b2c90122d 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -21,74 +21,232 @@
require 'spec_helper'
describe BambooService, models: true do
- describe "Associations" do
+ describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
end
- describe "Execute" do
- let(:user) { create(:user) }
- let(:project) { create(:project) }
-
- context "when a password was previously set" do
- before do
- @bamboo_service = BambooService.create(
- project: create(:project),
- properties: {
- bamboo_url: 'http://gitlab.com',
- username: 'mic',
- password: "password"
- }
- )
+ describe 'Validations' do
+ describe '#bamboo_url' do
+ it 'does not validate the presence of bamboo_url if service is not active' do
+ bamboo_service = service
+ bamboo_service.active = false
+
+ expect(bamboo_service).not_to validate_presence_of(:bamboo_url)
+ end
+
+ it 'validates the presence of bamboo_url if service is active' do
+ bamboo_service = service
+ bamboo_service.active = true
+
+ expect(bamboo_service).to validate_presence_of(:bamboo_url)
+ end
+ end
+
+ describe '#build_key' do
+ it 'does not validate the presence of build_key if service is not active' do
+ bamboo_service = service
+ bamboo_service.active = false
+
+ expect(bamboo_service).not_to validate_presence_of(:build_key)
end
-
- it "reset password if url changed" do
- @bamboo_service.bamboo_url = 'http://gitlab1.com'
- @bamboo_service.save
- expect(@bamboo_service.password).to be_nil
+
+ it 'validates the presence of build_key if service is active' do
+ bamboo_service = service
+ bamboo_service.active = true
+
+ expect(bamboo_service).to validate_presence_of(:build_key)
+ end
+ end
+
+ describe '#username' do
+ it 'does not validate the presence of username if service is not active' do
+ bamboo_service = service
+ bamboo_service.active = false
+
+ expect(bamboo_service).not_to validate_presence_of(:username)
+ end
+
+ it 'does not validate the presence of username if username is nil' do
+ bamboo_service = service
+ bamboo_service.active = true
+ bamboo_service.password = nil
+
+ expect(bamboo_service).not_to validate_presence_of(:username)
+ end
+
+ it 'validates the presence of username if service is active and username is present' do
+ bamboo_service = service
+ bamboo_service.active = true
+ bamboo_service.password = 'secret'
+
+ expect(bamboo_service).to validate_presence_of(:username)
end
-
- it "does not reset password if username changed" do
- @bamboo_service.username = "some_name"
- @bamboo_service.save
- expect(@bamboo_service.password).to eq("password")
+ end
+
+ describe '#password' do
+ it 'does not validate the presence of password if service is not active' do
+ bamboo_service = service
+ bamboo_service.active = false
+
+ expect(bamboo_service).not_to validate_presence_of(:password)
end
- it "does not reset password if new url is set together with password, even if it's the same password" do
- @bamboo_service.bamboo_url = 'http://gitlab_edited.com'
- @bamboo_service.password = 'password'
- @bamboo_service.save
- expect(@bamboo_service.password).to eq("password")
- expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
+ it 'does not validate the presence of password if username is nil' do
+ bamboo_service = service
+ bamboo_service.active = true
+ bamboo_service.username = nil
+
+ expect(bamboo_service).not_to validate_presence_of(:password)
end
- it "should reset password if url changed, even if setter called multiple times" do
- @bamboo_service.bamboo_url = 'http://gitlab1.com'
- @bamboo_service.bamboo_url = 'http://gitlab1.com'
- @bamboo_service.save
- expect(@bamboo_service.password).to be_nil
+ it 'validates the presence of password if service is active and username is present' do
+ bamboo_service = service
+ bamboo_service.active = true
+ bamboo_service.username = 'john'
+
+ expect(bamboo_service).to validate_presence_of(:password)
end
end
-
- context "when no password was previously set" do
- before do
- @bamboo_service = BambooService.create(
- project: create(:project),
- properties: {
- bamboo_url: 'http://gitlab.com',
- username: 'mic'
- }
- )
+ end
+
+ describe 'Callbacks' do
+ describe 'before_update :reset_password' do
+ context 'when a password was previously set' do
+ it 'resets password if url changed' do
+ bamboo_service = service
+
+ bamboo_service.bamboo_url = 'http://gitlab1.com'
+ bamboo_service.save
+
+ expect(bamboo_service.password).to be_nil
+ end
+
+ it 'does not reset password if username changed' do
+ bamboo_service = service
+
+ bamboo_service.username = 'some_name'
+ bamboo_service.save
+
+ expect(bamboo_service.password).to eq('password')
+ end
+
+ it "does not reset password if new url is set together with password, even if it's the same password" do
+ bamboo_service = service
+
+ bamboo_service.bamboo_url = 'http://gitlab_edited.com'
+ bamboo_service.password = 'password'
+ bamboo_service.save
+
+ expect(bamboo_service.password).to eq('password')
+ expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com')
+ end
end
- it "saves password if new url is set together with password" do
- @bamboo_service.bamboo_url = 'http://gitlab_edited.com'
- @bamboo_service.password = 'password'
- @bamboo_service.save
- expect(@bamboo_service.password).to eq("password")
- expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
+ it 'saves password if new url is set together with password when no password was previously set' do
+ bamboo_service = service
+ bamboo_service.password = nil
+
+ bamboo_service.bamboo_url = 'http://gitlab_edited.com'
+ bamboo_service.password = 'password'
+ bamboo_service.save
+
+ expect(bamboo_service.password).to eq('password')
+ expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com')
end
+ end
+ end
+
+ describe '#build_page' do
+ it 'returns a specific URL when status is 500' do
+ stub_request(status: 500)
+
+ expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
+ end
+
+ it 'returns a specific URL when response has no results' do
+ stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
+
+ expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
+ end
+
+ it 'returns a build URL when bamboo_url has no trailing slash' do
+ stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+
+ expect(service(bamboo_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/browse/42')
+ end
+
+ it 'returns a build URL when bamboo_url has a trailing slash' do
+ stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+
+ expect(service(bamboo_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/browse/42')
+ end
+ end
+
+ describe '#commit_status' do
+ it 'sets commit status to :error when status is 500' do
+ stub_request(status: 500)
+
+ expect(service.commit_status('123', 'unused')).to eq(:error)
+ end
+
+ it 'sets commit status to "pending" when status is 404' do
+ stub_request(status: 404)
+
+ expect(service.commit_status('123', 'unused')).to eq('pending')
+ end
+
+ it 'sets commit status to "pending" when response has no results' do
+ stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
+
+ expect(service.commit_status('123', 'unused')).to eq('pending')
+ end
+
+ it 'sets commit status to "success" when build state contains Success' do
+ stub_request(build_state: 'YAY Success!')
+ expect(service.commit_status('123', 'unused')).to eq('success')
end
+
+ it 'sets commit status to "failed" when build state contains Failed' do
+ stub_request(build_state: 'NO Failed!')
+
+ expect(service.commit_status('123', 'unused')).to eq('failed')
+ end
+
+ it 'sets commit status to "pending" when build state contains Pending' do
+ stub_request(build_state: 'NO Pending!')
+
+ expect(service.commit_status('123', 'unused')).to eq('pending')
+ end
+
+ it 'sets commit status to :error when build state is unknown' do
+ stub_request(build_state: 'FOO BAR!')
+
+ expect(service.commit_status('123', 'unused')).to eq(:error)
+ end
+ end
+
+ def service(bamboo_url: 'http://gitlab.com')
+ described_class.create(
+ project: build_stubbed(:empty_project),
+ properties: {
+ bamboo_url: bamboo_url,
+ username: 'mic',
+ password: 'password',
+ build_key: 'foo'
+ }
+ )
+ end
+
+ def stub_request(status: 200, body: nil, build_state: 'success')
+ bamboo_full_url = 'http://mic:password@gitlab.com/rest/api/latest/result?label=123&os_authType=basic'
+ body ||= %Q({"results":{"results":{"result":{"buildState":"#{build_state}"}}}})
+
+ WebMock.stub_request(:get, bamboo_full_url).to_return(
+ status: status,
+ headers: { 'Content-Type' => 'application/json' },
+ body: body
+ )
end
end