Commit 0b447a99 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'fix-trailing-slash-in-bamboo_url' into 'master'

Fix a bug with trailing slash in bamboo_url

Also, improve specs for BambooService

Similar to !3679.

See merge request !3680
parents 60206023 acf911ee
...@@ -28,6 +28,7 @@ v 8.7.0 (unreleased) ...@@ -28,6 +28,7 @@ v 8.7.0 (unreleased)
- Allow SAML to handle external users based on user's information !3530 - Allow SAML to handle external users based on user's information !3530
- Allow Omniauth providers to be marked as `external` !3657 - Allow Omniauth providers to be marked as `external` !3657
- Add endpoints to archive or unarchive a project !3372 - 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 - Add links to CI setup documentation from project settings and builds pages
- Handle nil descriptions in Slack issue messages (Stan Hu) - 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) - API: Expose open_issues_count, closed_issues_count, open_merge_requests_count for labels (Robert Schilling)
......
...@@ -82,17 +82,17 @@ class BambooService < CiService ...@@ -82,17 +82,17 @@ class BambooService < CiService
end end
def build_info(sha) 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? if username.blank? && password.blank?
@response = HTTParty.get(parsed_url.to_s, verify: false) @response = HTTParty.get(url, verify: false)
else else
get_url = "#{url}&os_authType=basic" url << '&os_authType=basic'
auth = { auth = {
username: username, username: username,
password: password, password: password
} }
@response = HTTParty.get(get_url, verify: false, basic_auth: auth) @response = HTTParty.get(url, verify: false, basic_auth: auth)
end end
end end
...@@ -101,11 +101,11 @@ class BambooService < CiService ...@@ -101,11 +101,11 @@ class BambooService < CiService
if @response.code != 200 || @response['results']['results']['size'] == '0' if @response.code != 200 || @response['results']['results']['size'] == '0'
# If actual build link can't be determined, send user to build summary page. # 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 else
# If actual build link is available, go to build result page. # If actual build link is available, go to build result page.
result_key = @response['results']['results']['result']['planResultKey']['key'] result_key = @response['results']['results']['result']['planResultKey']['key']
"#{bamboo_url}/browse/#{result_key}" URI.join(bamboo_url, "/browse/#{result_key}").to_s
end end
end end
...@@ -134,7 +134,7 @@ class BambooService < CiService ...@@ -134,7 +134,7 @@ class BambooService < CiService
return unless supported_events.include?(data[:object_kind]) return unless supported_events.include?(data[:object_kind])
# Bamboo requires a GET and does not take any data. # Bamboo requires a GET and does not take any data.
self.class.get("#{bamboo_url}/updateAndBuild.action?buildKey=#{build_key}", url = URI.join(bamboo_url, "/updateAndBuild.action?buildKey=#{build_key}").to_s
verify: false) self.class.get(url, verify: false)
end end
end end
...@@ -21,74 +21,232 @@ ...@@ -21,74 +21,232 @@
require 'spec_helper' require 'spec_helper'
describe BambooService, models: true do describe BambooService, models: true do
describe "Associations" do describe 'Associations' do
it { is_expected.to belong_to :project } it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe "Execute" do describe 'Validations' do
let(:user) { create(:user) } describe '#bamboo_url' do
let(:project) { create(:project) } it 'does not validate the presence of bamboo_url if service is not active' do
bamboo_service = service
bamboo_service.active = false
context "when a password was previously set" do expect(bamboo_service).not_to validate_presence_of(:bamboo_url)
before do end
@bamboo_service = BambooService.create(
project: create(:project), it 'validates the presence of bamboo_url if service is active' do
properties: { bamboo_service = service
bamboo_url: 'http://gitlab.com', bamboo_service.active = true
username: 'mic',
password: "password" 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 end
it "reset password if url changed" do it 'validates the presence of build_key if service is active' do
@bamboo_service.bamboo_url = 'http://gitlab1.com' bamboo_service = service
@bamboo_service.save bamboo_service.active = true
expect(@bamboo_service.password).to be_nil
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 end
it "does not reset password if username changed" do it 'does not validate the presence of username if username is nil' do
@bamboo_service.username = "some_name" bamboo_service = service
@bamboo_service.save bamboo_service.active = true
expect(@bamboo_service.password).to eq("password") 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
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 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 '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
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 end
it "does not reset password if new url is set together with password, even if it's the same password" do 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 = service
@bamboo_service.password = 'password'
@bamboo_service.save bamboo_service.bamboo_url = 'http://gitlab_edited.com'
expect(@bamboo_service.password).to eq("password") bamboo_service.password = 'password'
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com") bamboo_service.save
expect(bamboo_service.password).to eq('password')
expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com')
end
end end
it "should reset password if url changed, even if setter called multiple times" do it 'saves password if new url is set together with password when no password was previously set' do
@bamboo_service.bamboo_url = 'http://gitlab1.com' bamboo_service = service
@bamboo_service.bamboo_url = 'http://gitlab1.com' bamboo_service.password = nil
@bamboo_service.save
expect(@bamboo_service.password).to be_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 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 end
context "when no password was previously set" do it 'sets commit status to "success" when build state contains Success' do
before do stub_request(build_state: 'YAY Success!')
@bamboo_service = BambooService.create(
project: create(:project), 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: { properties: {
bamboo_url: 'http://gitlab.com', bamboo_url: bamboo_url,
username: 'mic' username: 'mic',
password: 'password',
build_key: 'foo'
} }
) )
end end
it "saves password if new url is set together with password" do def stub_request(status: 200, body: nil, build_state: 'success')
@bamboo_service.bamboo_url = 'http://gitlab_edited.com' bamboo_full_url = 'http://mic:password@gitlab.com/rest/api/latest/result?label=123&os_authType=basic'
@bamboo_service.password = 'password' body ||= %Q({"results":{"results":{"result":{"buildState":"#{build_state}"}}}})
@bamboo_service.save
expect(@bamboo_service.password).to eq("password")
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
end
end WebMock.stub_request(:get, bamboo_full_url).to_return(
status: status,
headers: { 'Content-Type' => 'application/json' },
body: body
)
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