From 17ba052f5c9d7c390b350469d15ffc674a943b07 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 30 Jun 2017 15:23:46 +0800
Subject: [PATCH] Update wordings, allow only full path, add tests
---
app/models/ci/pipeline.rb | 9 ++++---
app/models/project.rb | 14 +++++-----
.../pipelines_settings/_show.html.haml | 6 ++---
doc/api/projects.md | 6 ++---
doc/user/project/pipelines/settings.md | 2 +-
spec/models/ci/pipeline_spec.rb | 26 +++++--------------
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 57bf5a8a4c..12986e5781 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 507dffde18..5374aca770 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 2c2f0341e2..4b3efd12e0 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 ea349ae8f6..7565f18e90 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 702b3453a0..7274fe816c 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 5ed0203170..8d4d87def5 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 fb39357659..349f9c3d7e 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)) }
--
2.30.9