diff options
-rw-r--r-- | app/models/ci/pipeline.rb | 9 | ||||
-rw-r--r-- | app/models/project.rb | 14 | ||||
-rw-r--r-- | app/views/projects/pipelines_settings/_show.html.haml | 6 | ||||
-rw-r--r-- | doc/api/projects.md | 6 | ||||
-rw-r--r-- | doc/user/project/pipelines/settings.md | 2 | ||||
-rw-r--r-- | spec/models/ci/pipeline_spec.rb | 26 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 24 |
7 files changed, 49 insertions, 38 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 57bf5a8a4c5..12986e5781e 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -343,10 +343,11 @@ module Ci end def ci_yaml_file_path - return '.gitlab-ci.yml' if project.ci_config_file.blank? - return project.ci_config_file if File.extname(project.ci_config_file.to_s) == '.yml' - - File.join(project.ci_config_file || '', '.gitlab-ci.yml') + if project.ci_config_file.blank? + '.gitlab-ci.yml' + else + project.ci_config_file + end end def environments diff --git a/app/models/project.rb b/app/models/project.rb index 507dffde18b..5374aca7701 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -187,7 +187,7 @@ class Project < ActiveRecord::Base validates :creator, presence: true, on: :create validates :description, length: { maximum: 2000 }, allow_blank: true validates :ci_config_file, - format: { without: /\.{2}/.freeze, + format: { without: /\.{2}/, message: 'cannot include directory traversal.' }, length: { maximum: 255 }, allow_blank: true @@ -222,7 +222,6 @@ class Project < ActiveRecord::Base add_authentication_token_field :runners_token before_save :ensure_runners_token - before_validation :clean_ci_config_file mount_uploader :avatar, AvatarUploader has_many :uploads, as: :model, dependent: :destroy @@ -527,6 +526,11 @@ class Project < ActiveRecord::Base import_data&.destroy end + def ci_config_file=(value) + # Strip all leading slashes so that //foo -> foo + super(value&.sub(%r{\A/+}, '')) + end + def import_url=(value) return super(value) unless Gitlab::UrlSanitizer.valid?(value) @@ -1484,10 +1488,4 @@ class Project < ActiveRecord::Base raise ex 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 diff --git a/app/views/projects/pipelines_settings/_show.html.haml b/app/views/projects/pipelines_settings/_show.html.haml index 2c2f0341e2a..4b3efd12e08 100644 --- a/app/views/projects/pipelines_settings/_show.html.haml +++ b/app/views/projects/pipelines_settings/_show.html.haml @@ -47,11 +47,11 @@ %hr .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' %p.help-block - Optionally specify the location of your CI config file, e.g. my/path or my/path/.my-config.yml. - Default is to use '.gitlab-ci.yml' in the repository root. + The path to CI config file. Default to <code>.gitlab-ci.yml</code> + = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'custom-ci-config-file'), target: '_blank' %hr .form-group diff --git a/doc/api/projects.md b/doc/api/projects.md index ea349ae8f68..7565f18e907 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -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 | | `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 | -| `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 @@ -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 | | `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 | -| `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 @@ -418,7 +418,7 @@ Parameters: | `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 | | `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 diff --git a/doc/user/project/pipelines/settings.md b/doc/user/project/pipelines/settings.md index 702b3453a0e..7274fe816cc 100644 --- a/doc/user/project/pipelines/settings.md +++ b/doc/user/project/pipelines/settings.md @@ -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, 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. diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 5ed02031708..8d4d87def5e 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -748,47 +748,35 @@ describe Ci::Pipeline, models: true do end end - describe 'yaml config file resolution' do - let(:project) { FactoryGirl.build(:project) } + describe '#ci_yaml_file_path' do + let(:project) { create(:empty_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' } - expect(pipeline.ci_yaml_file_path).to eq('custom/path/.gitlab-ci.yml') + expect(pipeline.ci_yaml_file_path).to eq('custom/path') 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 } expect(pipeline.ci_yaml_file_path).to eq('.gitlab-ci.yml') 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) { '' } expect(pipeline.ci_yaml_file_path).to eq('.gitlab-ci.yml') 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 allow(project).to receive(:ci_config_file) { 'custom' } pipeline.ci_yaml_file 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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fb39357659c..349f9c3d7eb 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -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(: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) } @@ -1491,6 +1493,28 @@ describe Project, models: true do 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 let(:project) { create(:empty_project, import_url: generate(:url)) } |