Commit 17ba052f authored by Lin Jen-Shin's avatar Lin Jen-Shin

Update wordings, allow only full path, add tests

parent 62fdbbee
...@@ -343,10 +343,11 @@ module Ci ...@@ -343,10 +343,11 @@ module Ci
end end
def ci_yaml_file_path def ci_yaml_file_path
return '.gitlab-ci.yml' if project.ci_config_file.blank? if project.ci_config_file.blank?
return project.ci_config_file if File.extname(project.ci_config_file.to_s) == '.yml' '.gitlab-ci.yml'
else
File.join(project.ci_config_file || '', '.gitlab-ci.yml') project.ci_config_file
end
end end
def environments def environments
......
...@@ -187,7 +187,7 @@ class Project < ActiveRecord::Base ...@@ -187,7 +187,7 @@ class Project < ActiveRecord::Base
validates :creator, presence: true, on: :create validates :creator, presence: true, on: :create
validates :description, length: { maximum: 2000 }, allow_blank: true validates :description, length: { maximum: 2000 }, allow_blank: true
validates :ci_config_file, validates :ci_config_file,
format: { without: /\.{2}/.freeze, format: { without: /\.{2}/,
message: 'cannot include directory traversal.' }, message: 'cannot include directory traversal.' },
length: { maximum: 255 }, length: { maximum: 255 },
allow_blank: true allow_blank: true
...@@ -222,7 +222,6 @@ class Project < ActiveRecord::Base ...@@ -222,7 +222,6 @@ class Project < ActiveRecord::Base
add_authentication_token_field :runners_token add_authentication_token_field :runners_token
before_save :ensure_runners_token before_save :ensure_runners_token
before_validation :clean_ci_config_file
mount_uploader :avatar, AvatarUploader mount_uploader :avatar, AvatarUploader
has_many :uploads, as: :model, dependent: :destroy has_many :uploads, as: :model, dependent: :destroy
...@@ -527,6 +526,11 @@ class Project < ActiveRecord::Base ...@@ -527,6 +526,11 @@ class Project < ActiveRecord::Base
import_data&.destroy import_data&.destroy
end end
def ci_config_file=(value)
# Strip all leading slashes so that //foo -> foo
super(value&.sub(%r{\A/+}, ''))
end
def import_url=(value) def import_url=(value)
return super(value) unless Gitlab::UrlSanitizer.valid?(value) return super(value) unless Gitlab::UrlSanitizer.valid?(value)
...@@ -1484,10 +1488,4 @@ class Project < ActiveRecord::Base ...@@ -1484,10 +1488,4 @@ class Project < ActiveRecord::Base
raise ex raise ex
end end
def clean_ci_config_file
return unless self.ci_config_file
# Cleanup path removing leading/trailing slashes
self.ci_config_file = ci_config_file.gsub(/^\/+|\/+$/, '')
end
end end
...@@ -47,11 +47,11 @@ ...@@ -47,11 +47,11 @@
%hr %hr
.form-group .form-group
= f.label :ci_config_file, 'Custom CI Config File', class: 'label-light' = f.label :ci_config_file, 'Custom CI config file', class: 'label-light'
= f.text_field :ci_config_file, class: 'form-control', placeholder: '.gitlab-ci.yml' = f.text_field :ci_config_file, class: 'form-control', placeholder: '.gitlab-ci.yml'
%p.help-block %p.help-block
Optionally specify the location of your CI config file, e.g. my/path or my/path/.my-config.yml. The path to CI config file. Default to <code>.gitlab-ci.yml</code>
Default is to use '.gitlab-ci.yml' in the repository root. = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'custom-ci-config-file'), target: '_blank'
%hr %hr
.form-group .form-group
......
...@@ -346,7 +346,7 @@ Parameters: ...@@ -346,7 +346,7 @@ Parameters:
| `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project |
| `avatar` | mixed | no | Image file for avatar of the project | | `avatar` | mixed | no | Image file for avatar of the project |
| `printing_merge_request_link_enabled` | boolean | no | Show link to create/view merge request when pushing from the command line | | `printing_merge_request_link_enabled` | boolean | no | Show link to create/view merge request when pushing from the command line |
| `ci_config_file` | boolean | no | The relative path to the CI config file (E.g. my/path or my/path/.gitlab-ci.yml or my/path/my-config.yml) | | `ci_config_file` | boolean | no | The path to CI config file |
### Create project for user ### Create project for user
...@@ -383,7 +383,7 @@ Parameters: ...@@ -383,7 +383,7 @@ Parameters:
| `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project |
| `avatar` | mixed | no | Image file for avatar of the project | | `avatar` | mixed | no | Image file for avatar of the project |
| `printing_merge_request_link_enabled` | boolean | no | Show link to create/view merge request when pushing from the command line | | `printing_merge_request_link_enabled` | boolean | no | Show link to create/view merge request when pushing from the command line |
| `ci_config_file` | boolean | no | The relative path to the CI config file (E.g. my/path or my/path/.gitlab-ci.yml or my/path/my-config.yml) | | `ci_config_file` | boolean | no | The path to CI config file |
### Edit project ### Edit project
...@@ -418,7 +418,7 @@ Parameters: ...@@ -418,7 +418,7 @@ Parameters:
| `request_access_enabled` | boolean | no | Allow users to request member access | | `request_access_enabled` | boolean | no | Allow users to request member access |
| `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project |
| `avatar` | mixed | no | Image file for avatar of the project | | `avatar` | mixed | no | Image file for avatar of the project |
| `ci_config_file` | boolean | no | The relative path to the CI config file (E.g. my/path or my/path/.gitlab-ci.yml or my/path/my-config.yml) | | `ci_config_file` | boolean | no | The path to CI config file |
### Fork project ### Fork project
......
...@@ -27,7 +27,7 @@ The default value is 60 minutes. Decrease the time limit if you want to impose ...@@ -27,7 +27,7 @@ The default value is 60 minutes. Decrease the time limit if you want to impose
a hard limit on your jobs' running time or increase it otherwise. In any case, a hard limit on your jobs' running time or increase it otherwise. In any case,
if the job surpasses the threshold, it is marked as failed. if the job surpasses the threshold, it is marked as failed.
## Custom CI Config File ## Custom CI config file
> - [Introduced][ce-12509] in GitLab 9.4. > - [Introduced][ce-12509] in GitLab 9.4.
......
...@@ -748,47 +748,35 @@ describe Ci::Pipeline, models: true do ...@@ -748,47 +748,35 @@ describe Ci::Pipeline, models: true do
end end
end end
describe 'yaml config file resolution' do describe '#ci_yaml_file_path' do
let(:project) { FactoryGirl.build(:project) } let(:project) { create(:empty_project) }
let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:pipeline) { create(:ci_empty_pipeline, project: project) }
it 'uses custom ci config file path when present' do it 'returns the path from project' do
allow(project).to receive(:ci_config_file) { 'custom/path' } allow(project).to receive(:ci_config_file) { 'custom/path' }
expect(pipeline.ci_yaml_file_path).to eq('custom/path/.gitlab-ci.yml') expect(pipeline.ci_yaml_file_path).to eq('custom/path')
end end
it 'uses root when custom path is nil' do it 'returns default when custom path is nil' do
allow(project).to receive(:ci_config_file) { nil } allow(project).to receive(:ci_config_file) { nil }
expect(pipeline.ci_yaml_file_path).to eq('.gitlab-ci.yml') expect(pipeline.ci_yaml_file_path).to eq('.gitlab-ci.yml')
end end
it 'uses root when custom path is empty' do it 'returns default when custom path is empty' do
allow(project).to receive(:ci_config_file) { '' } allow(project).to receive(:ci_config_file) { '' }
expect(pipeline.ci_yaml_file_path).to eq('.gitlab-ci.yml') expect(pipeline.ci_yaml_file_path).to eq('.gitlab-ci.yml')
end end
it 'allows custom filename' do
allow(project).to receive(:ci_config_file) { 'custom/path/.my-config.yml' }
expect(pipeline.ci_yaml_file_path).to eq('custom/path/.my-config.yml')
end
it 'custom filename must be yml' do
allow(project).to receive(:ci_config_file) { 'custom/path/.my-config.cnf' }
expect(pipeline.ci_yaml_file_path).to eq('custom/path/.my-config.cnf/.gitlab-ci.yml')
end
it 'reports error if the file is not found' do it 'reports error if the file is not found' do
allow(project).to receive(:ci_config_file) { 'custom' } allow(project).to receive(:ci_config_file) { 'custom' }
pipeline.ci_yaml_file pipeline.ci_yaml_file
expect(pipeline.yaml_errors) expect(pipeline.yaml_errors)
.to eq('Failed to load CI/CD config file at custom/.gitlab-ci.yml') .to eq('Failed to load CI/CD config file at custom')
end end
end end
......
...@@ -144,6 +144,8 @@ describe Project, models: true do ...@@ -144,6 +144,8 @@ describe Project, models: true do
it { is_expected.to validate_length_of(:description).is_at_most(2000) } it { is_expected.to validate_length_of(:description).is_at_most(2000) }
it { is_expected.to validate_length_of(:ci_config_file).is_at_most(255) } it { is_expected.to validate_length_of(:ci_config_file).is_at_most(255) }
it { is_expected.to allow_value('').for(:ci_config_file) }
it { is_expected.not_to allow_value('test/../foo').for(:ci_config_file) }
it { is_expected.to validate_presence_of(:creator) } it { is_expected.to validate_presence_of(:creator) }
...@@ -1491,6 +1493,28 @@ describe Project, models: true do ...@@ -1491,6 +1493,28 @@ describe Project, models: true do
end end
end end
describe '#ci_config_file=' do
let(:project) { create(:empty_project) }
it 'sets nil' do
project.update!(ci_config_file: nil)
expect(project.ci_config_file).to be_nil
end
it 'sets a string' do
project.update!(ci_config_file: 'foo/.gitlab_ci.yml')
expect(project.ci_config_file).to eq('foo/.gitlab_ci.yml')
end
it 'sets a string but remove all leading slashes' do
project.update!(ci_config_file: '///foo//.gitlab_ci.yml')
expect(project.ci_config_file).to eq('foo//.gitlab_ci.yml')
end
end
describe 'Project import job' do describe 'Project import job' do
let(:project) { create(:empty_project, import_url: generate(:url)) } let(:project) { create(:empty_project, import_url: generate(:url)) }
......
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