Commit 2f7b2057 authored by Rémy Coutable's avatar Rémy Coutable

Fix broken URI joining for `teamcity_url` with suffixes

If one had configured a `teamcity_url` like http://foo.bar/teamcity in
the previous implementation the plugin directed it's request i.e. to
http://foo.bar/httpAuth/... instead of http://foo.bar/teamcity/httpAuth/...

`URI.join` only works correctly, if the prefix URL has
  - at least one or more  trailing '/'
  - the appended parts are _not_ prefixed with '/'

The current implementation should work with all sorts of TeamCity base
URLs.
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 17c32ee8
...@@ -39,6 +39,7 @@ v 8.9.0 (unreleased) ...@@ -39,6 +39,7 @@ v 8.9.0 (unreleased)
- Fix issues filter when ordering by milestone - Fix issues filter when ordering by milestone
- Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3 - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3
- Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid) - Bamboo Service: Fix missing credentials & URL handling when base URL contains a path (Benjamin Schmid)
- TeamCity Service: Fix URL handling when base URL contains a path
- Todos will display target state if issuable target is 'Closed' or 'Merged' - Todos will display target state if issuable target is 'Closed' or 'Merged'
- Fix bug when sorting issues by milestone due date and filtering by two or more labels - Fix bug when sorting issues by milestone due date and filtering by two or more labels
- Add support for using Yubikeys (U2F) for two-factor authentication - Add support for using Yubikeys (U2F) for two-factor authentication
......
class TeamcityService < CiService class TeamcityService < CiService
include HTTParty
prop_accessor :teamcity_url, :build_type, :username, :password prop_accessor :teamcity_url, :build_type, :username, :password
validates :teamcity_url, presence: true, url: true, if: :activated? validates :teamcity_url, presence: true, url: true, if: :activated?
...@@ -64,15 +62,7 @@ class TeamcityService < CiService ...@@ -64,15 +62,7 @@ class TeamcityService < CiService
end end
def build_info(sha) def build_info(sha)
url = URI.join( @response = get_path("httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}")
teamcity_url,
"/httpAuth/app/rest/builds/branch:unspecified:any,number:#{sha}"
).to_s
auth = {
username: username,
password: password
}
@response = HTTParty.get(url, verify: false, basic_auth: auth)
end end
def build_page(sha, ref) def build_page(sha, ref)
...@@ -81,14 +71,11 @@ class TeamcityService < CiService ...@@ -81,14 +71,11 @@ class TeamcityService < CiService
if @response.code != 200 if @response.code != 200
# If actual build link can't be determined, # If actual build link can't be determined,
# send user to build summary page. # send user to build summary page.
URI.join(teamcity_url, "/viewLog.html?buildTypeId=#{build_type}").to_s build_url("viewLog.html?buildTypeId=#{build_type}")
else else
# If actual build link is available, go to build result page. # If actual build link is available, go to build result page.
built_id = @response['build']['id'] built_id = @response['build']['id']
URI.join( build_url("viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}")
teamcity_url,
"/viewLog.html?buildId=#{built_id}&buildTypeId=#{build_type}"
).to_s
end end
end end
...@@ -123,8 +110,8 @@ class TeamcityService < CiService ...@@ -123,8 +110,8 @@ class TeamcityService < CiService
branch = Gitlab::Git.ref_name(data[:ref]) branch = Gitlab::Git.ref_name(data[:ref])
self.class.post( HTTParty.post(
URI.join(teamcity_url, '/httpAuth/app/rest/buildQueue').to_s, build_url('httpAuth/app/rest/buildQueue'),
body: "<build branchName=\"#{branch}\">"\ body: "<build branchName=\"#{branch}\">"\
"<buildType id=\"#{build_type}\"/>"\ "<buildType id=\"#{build_type}\"/>"\
'</build>', '</build>',
...@@ -132,4 +119,18 @@ class TeamcityService < CiService ...@@ -132,4 +119,18 @@ class TeamcityService < CiService
basic_auth: auth basic_auth: auth
) )
end end
private
def build_url(path)
URI.join("#{teamcity_url}/", path).to_s
end
def get_path(path)
HTTParty.get(build_url(path), verify: false,
basic_auth: {
username: username,
password: password
})
end
end end
...@@ -126,19 +126,19 @@ describe TeamcityService, models: true do ...@@ -126,19 +126,19 @@ describe TeamcityService, models: true do
it 'returns a specific URL when status is 500' do it 'returns a specific URL when status is 500' do
stub_request(status: 500) stub_request(status: 500)
expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildTypeId=foo') expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildTypeId=foo')
end end
it 'returns a build URL when teamcity_url has no trailing slash' do it 'returns a build URL when teamcity_url has no trailing slash' do
stub_request(body: %Q({"build":{"id":"666"}})) stub_request(body: %Q({"build":{"id":"666"}}))
expect(service(teamcity_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo') expect(service(teamcity_url: 'http://gitlab.com/teamcity').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
end end
it 'returns a build URL when teamcity_url has a trailing slash' do it 'returns a build URL when teamcity_url has a trailing slash' do
stub_request(body: %Q({"build":{"id":"666"}})) stub_request(body: %Q({"build":{"id":"666"}}))
expect(service(teamcity_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo') expect(service(teamcity_url: 'http://gitlab.com/teamcity/').build_page('123', 'unused')).to eq('http://gitlab.com/teamcity/viewLog.html?buildId=666&buildTypeId=foo')
end end
end end
...@@ -180,7 +180,7 @@ describe TeamcityService, models: true do ...@@ -180,7 +180,7 @@ describe TeamcityService, models: true do
end end
end end
def service(teamcity_url: 'http://gitlab.com') def service(teamcity_url: 'http://gitlab.com/teamcity')
described_class.create( described_class.create(
project: create(:empty_project), project: create(:empty_project),
properties: { properties: {
...@@ -193,7 +193,7 @@ describe TeamcityService, models: true do ...@@ -193,7 +193,7 @@ describe TeamcityService, models: true do
end end
def stub_request(status: 200, body: nil, build_status: 'success') def stub_request(status: 200, body: nil, build_status: 'success')
teamcity_full_url = 'http://mic:password@gitlab.com/httpAuth/app/rest/builds/branch:unspecified:any,number:123' teamcity_full_url = 'http://mic:password@gitlab.com/teamcity/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
body ||= %Q({"build":{"status":"#{build_status}","id":"666"}}) body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
WebMock.stub_request(:get, teamcity_full_url).to_return( WebMock.stub_request(:get, teamcity_full_url).to_return(
......
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