From ecefe090460687a078e3d1aacf621fd5bff07fb5 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 31 Aug 2018 16:58:13 +0100 Subject: Render link to branch only when branch still exists --- app/views/projects/pipelines/_info.html.haml | 6 +++++- changelogs/unreleased/42611-removed-branch-link.yml | 5 +++++ spec/features/projects/pipelines/pipeline_spec.rb | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/42611-removed-branch-link.yml diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index ccb83148ded..57c5f64ee8d 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -13,7 +13,11 @@ = pluralize @pipeline.total_size, "job" - if @pipeline.ref from - = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" + - if @project.repository.branch_exists?(@pipeline.ref) + = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" + - else + %span.ref-name + = @pipeline.ref - if @pipeline.duration in = time_interval_in_words(@pipeline.duration) diff --git a/changelogs/unreleased/42611-removed-branch-link.yml b/changelogs/unreleased/42611-removed-branch-link.yml new file mode 100644 index 00000000000..03a206871b4 --- /dev/null +++ b/changelogs/unreleased/42611-removed-branch-link.yml @@ -0,0 +1,5 @@ +--- +title: Only render link to branch when branch still exists in pipeline page +merge_request: +author: +type: fixed diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 603503a531c..4d659cb988e 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -63,6 +63,11 @@ describe 'Pipeline', :js do expect(page).to have_css('#js-tab-pipeline.active') end + it 'shows link to the pipeline ref' do + expect(page).to have_link(pipeline.ref) + end + + it_behaves_like 'showing user status' do let(:user_with_status) { pipeline.user } @@ -208,6 +213,18 @@ describe 'Pipeline', :js do it { expect(page).not_to have_content('Cancel running') } end end + + context 'with deleted branch' do + before do + DeleteBranchService.new(@project, @user).execute(pipeline.ref) + end + + it 'does not render link to the pipeline ref' do + expect(page).not_to have_link(pipeline.ref) + expect(page).to have_content(pipeline.ref) + end + end + end context 'when user does not have access to read jobs' do -- cgit v1.2.1 From 2906e09f3ab8df4a5f2baf88663fd9cf459554d1 Mon Sep 17 00:00:00 2001 From: Ryan Hefner Date: Thu, 13 Sep 2018 16:27:57 +0200 Subject: Add dbname to MySQL migration GRANT command --- lib/gitlab/database/migration_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 7f012312819..f1466513d45 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -937,7 +937,7 @@ database (#{dbname}) using a super user and running: For MySQL you instead need to run: - GRANT ALL PRIVILEGES ON *.* TO #{user}@'%' + GRANT ALL PRIVILEGES ON #{dbname}.* TO #{user}@'%' Both queries will grant the user super user permissions, ensuring you don't run into similar problems in the future (e.g. when new tables are created). -- cgit v1.2.1 From 08a38b71b7035f477a9ecc273ff8bd0301eb4a21 Mon Sep 17 00:00:00 2001 From: Valerio Baldisserotto Date: Wed, 3 Oct 2018 09:15:12 +0000 Subject: Change path where to create the config.json file --- doc/ci/docker/using_kaniko.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/ci/docker/using_kaniko.md b/doc/ci/docker/using_kaniko.md index 7d4f28e1f47..66f0d429165 100644 --- a/doc/ci/docker/using_kaniko.md +++ b/doc/ci/docker/using_kaniko.md @@ -39,7 +39,7 @@ few important details: In the following example, kaniko is used to build a Docker image and then push it to [GitLab Container Registry](../../user/project/container_registry.md). The job will run only when a tag is pushed. A `config.json` file is created under -`/root/.docker` with the needed GitLab Container Registry credentials taken from the +`/kaniko/.docker` with the needed GitLab Container Registry credentials taken from the [environment variables](../variables/README.md#predefined-variables-environment-variables) GitLab CI/CD provides. In the last step, kaniko uses the `Dockerfile` under the root directory of the project, builds the Docker image and pushes it to the @@ -52,8 +52,7 @@ build: name: gcr.io/kaniko-project/executor:debug entrypoint: [""] script: - - mkdir -p /root/.docker - - echo "{\"auths\":{\"$CI_REGISTRY\":{\"username\":\"$CI_REGISTRY_USER\",\"password\":\"$CI_REGISTRY_PASSWORD\"}}}" > /root/.docker/config.json + - echo "{\"auths\":{\"$CI_REGISTRY\":{\"username\":\"$CI_REGISTRY_USER\",\"password\":\"$CI_REGISTRY_PASSWORD\"}}}" > /kaniko/.docker/config.json - /kaniko/executor --context $CI_PROJECT_DIR --dockerfile $CI_PROJECT_DIR/Dockerfile --destination $CI_REGISTRY_IMAGE:$CI_COMMIT_TAG only: - tags -- cgit v1.2.1 From 0d583e5e8a36231eef614305208ea67ab91a5b62 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 8 Oct 2018 16:55:13 +0100 Subject: Creates ref_exists? method for Pipeline class --- app/models/ci/pipeline.rb | 4 ++++ app/views/projects/pipelines/_info.html.haml | 2 +- spec/features/projects/pipelines/pipeline_spec.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 17024e8a0af..c53b14bd406 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -268,6 +268,10 @@ module Ci stage unless stage.statuses_count.zero? end + def ref_exists? + project.repository.ref_exists?(self.ref) + end + ## # TODO We do not completely switch to persisted stages because of # race conditions with setting statuses gitlab-ce#23257. diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index 57c5f64ee8d..ba7f542f68e 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -13,7 +13,7 @@ = pluralize @pipeline.total_size, "job" - if @pipeline.ref from - - if @project.repository.branch_exists?(@pipeline.ref) + - if @pipeline.ref_exists? = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name" - else %span.ref-name diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index 8cdefebd4ce..a79dbe7f877 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -244,7 +244,7 @@ describe 'Pipeline', :js do context 'with deleted branch' do before do - DeleteBranchService.new(@project, @user).execute(pipeline.ref) + allow(pipeline).to receive(:ref_exists?).and_return(false) end it 'does not render link to the pipeline ref' do -- cgit v1.2.1 From a1e267dc7534acc425ac510a7446d4a83db7c0b6 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 9 Oct 2018 17:20:29 +0000 Subject: Document the role of the maintainer --- doc/development/code_review.md | 69 ++++++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 16 deletions(-) diff --git a/doc/development/code_review.md b/doc/development/code_review.md index edf0b6f46df..6833154b2be 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -4,31 +4,25 @@ There are a few rules to get your merge request accepted: -1. Your merge request should only be **merged by a [maintainer][team]**. - 1. If your merge request includes only backend changes [^1], it must be +1. Your merge request can only be **merged by a [maintainer][team]**. + 1. If your merge request includes backend changes [^1], it must be **approved by a [backend maintainer][projects]**. - 1. If your merge request includes only frontend changes [^1], it must be + 1. If your merge request includes frontend changes [^1], it must be **approved by a [frontend maintainer][projects]**. - 1. If your merge request includes UX changes [^1], it must - be **approved by a [UX team member][team]**. + 1. If your merge request includes UX changes [^1], it must be + **approved by a [UX team member][team]**. 1. If your merge request includes adding a new JavaScript library [^1], it must be **approved by a [frontend lead][team]**. 1. If your merge request includes adding a new UI/UX paradigm [^1], it must be **approved by a [UX lead][team]**. - 1. If your merge request includes frontend and backend changes [^1], it must - be **approved by a [frontend and a backend maintainer][projects]**. - 1. If your merge request includes UX and frontend changes [^1], it must - be **approved by a [UX team member and a frontend maintainer][team]**. - 1. If your merge request includes UX, frontend and backend changes [^1], it must - be **approved by a [UX team member, a frontend and a backend maintainer][team]**. - 1. If your merge request includes a new dependency or a filesystem change, it must - be *approved by a [Distribution team member][team]*. See how to work with the [Distribution team for more details.](https://about.gitlab.com/handbook/engineering/dev-backend/distribution/) -1. To lower the amount of merge requests maintainers need to review, you can + 1. If your merge request includes a new dependency or a filesystem change, it must be + **approved by a [Distribution team member][team]**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/dev-backend/distribution/) for more details. + 1. If more than one of the items above apply to your merge request, it must be + **approved by all listed people**. The last of the people to approve can then merge it. +1. To lower the amount of merge requests maintainers need to review, you are encouraged ask or assign any [reviewers][projects] for a first review. 1. If you need some guidance (e.g. it's your first merge request), feel free to ask one of the [Merge request coaches][team]. - 1. It is recommended that you assign a maintainer that is from a different team than your own. - This ensures that all code across GitLab is consistent and can be easily understood by all contributors. 1. Keep in mind that maintainers are also going to perform a final code review. The ideal scenario is that the reviewer has already addressed any concerns @@ -37,6 +31,49 @@ There are a few rules to get your merge request accepted: For more guidance, see [CONTRIBUTING.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md). +### The role of the maintainer + +Maintainers are responsible for the overall health, quality, and consistency of +the GitLab codebase, across domains and product areas. Consequently, their reviews +will focus primarily on things like overall architecture, code organization, +separation of concerns, tests, DRYness, consistency, readability, etc. + +Their job is explicitly _not_ to review the solution itself. By the time a merge +request makes it to a maintainer, they should be able to assume that it actually +solves the problem it was meant to solve, that it does so in the most appropriate +way, that it satisfies all requirements, and that there are no remaining bugs, +logical problems, or uncovered edge cases. + +The responsibility to find the best solution and implement it lies with the +merge request author, and they should be confident of the chosen solution, +implementation, and everything else that makes up the merge request, before +they ask a maintainer for final review, approval, and merge. + +To reach this level of confidence, an author is expected to involve other people +in the investigation and implementation processes as appropriate. They may want +to reach out to domain experts to discuss different solutions or get an +implementation reviewed, to product managers and UX designers to clear up +confusion or verify that the end result matches what they had in mind, or to +database specialists to get input on the data model or specific queries. + +They are also strongly encouraged to get their code reviewed by any other developer +as soon as there is any code to review, to get a second opinion on the chosen +solution and implementation and an extra pair of eyes looking for bugs, +logic problems, or uncovered edge cases, and to ease the job of the maintainer. + +Of course, a maintainer will also make note of any issues with the solution or +implementation they may find, but in general will assume that the author is the +expert on the issue at hand, and that they made their choices with good reason. + +Since a maintainer's job does not depend on their domain-specific knowledge beyond +knowledge of the overall GitLab codebase, they can review merge requests from any +team and in any product area. + +Authors are recommended to assign merge requests to maintainers from other teams +than their own, to ensure that all code across GitLab is consistent and can be +easily understood by all contributors, from both inside and outside the company, +without requiring team-specific expertise. + ## Best practices This guide contains advice and best practices for performing code review, and -- cgit v1.2.1 From eb0ded1d909fe5bb53294dc8f9b48a2fdee45fb9 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 10 Oct 2018 10:48:14 +0000 Subject: Rewrite guidance on getting your merge request reviewed, approved, and merged --- doc/development/code_review.md | 68 ++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 6833154b2be..d18bb51eb5d 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -1,14 +1,31 @@ # Code Review Guidelines +This guide contains advice and best practices for performing code review, and +having your code reviewed. + +All merge requests for GitLab CE and EE, whether written by a GitLab team member +or a volunteer contributor, must go through a code review process to ensure the +code is effective, understandable, and maintainable. + ## Getting your merge request reviewed, approved, and merged -There are a few rules to get your merge request accepted: +You are strongly encouraged to get your code **reviewed** by a +[reviewer](https://about.gitlab.com/handbook/engineering/#reviewer) as soon as +there is any code to review, to get a second opinion on the chosen solution and +implementation, and an extra pair of eyes looking for bugs, logic problems, or +uncovered edge cases. The reviewer can be from a different team, but it is often +useful to pick someone who knows the domain well. + +If you need some guidance (e.g. it's your first merge request), feel free to ask +one of the [Merge request coaches][team]. + +Depending on the areas your merge request touches, it must be **approved** by one +or more [maintainers](https://about.gitlab.com/handbook/engineering/#maintainer): -1. Your merge request can only be **merged by a [maintainer][team]**. 1. If your merge request includes backend changes [^1], it must be - **approved by a [backend maintainer][projects]**. + **approved by a [backend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_backend)**. 1. If your merge request includes frontend changes [^1], it must be - **approved by a [frontend maintainer][projects]**. + **approved by a [frontend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_frontend)**. 1. If your merge request includes UX changes [^1], it must be **approved by a [UX team member][team]**. 1. If your merge request includes adding a new JavaScript library [^1], it must be @@ -17,19 +34,14 @@ There are a few rules to get your merge request accepted: **approved by a [UX lead][team]**. 1. If your merge request includes a new dependency or a filesystem change, it must be **approved by a [Distribution team member][team]**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/dev-backend/distribution/) for more details. - 1. If more than one of the items above apply to your merge request, it must be - **approved by all listed people**. The last of the people to approve can then merge it. -1. To lower the amount of merge requests maintainers need to review, you are encouraged - ask or assign any [reviewers][projects] for a first review. - 1. If you need some guidance (e.g. it's your first merge request), feel free - to ask one of the [Merge request coaches][team]. - -1. Keep in mind that maintainers are also going to perform a final code review. - The ideal scenario is that the reviewer has already addressed any concerns - the maintainer would have found, and the maintainer only has to perform the - merge, but be prepared for further review comments. - -For more guidance, see [CONTRIBUTING.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md). + +Getting your merge request **merged** also requires a maintainer. If it requires +more than one approval, the last maintainer to review and approve it will also merge it. + +Keep in mind that maintainers are also going to perform a final code review. +The ideal scenario is that the reviewer has already identified any concerns +the maintainer would have found, and the maintainer only has to perform the +merge, but be prepared for further review comments. ### The role of the maintainer @@ -53,13 +65,9 @@ To reach this level of confidence, an author is expected to involve other people in the investigation and implementation processes as appropriate. They may want to reach out to domain experts to discuss different solutions or get an implementation reviewed, to product managers and UX designers to clear up -confusion or verify that the end result matches what they had in mind, or to -database specialists to get input on the data model or specific queries. - -They are also strongly encouraged to get their code reviewed by any other developer -as soon as there is any code to review, to get a second opinion on the chosen -solution and implementation and an extra pair of eyes looking for bugs, -logic problems, or uncovered edge cases, and to ease the job of the maintainer. +confusion or verify that the end result matches what they had in mind, to +database specialists to get input on the data model or specific queries, +or to any other developer to get a code review. Of course, a maintainer will also make note of any issues with the solution or implementation they may find, but in general will assume that the author is the @@ -76,18 +84,6 @@ without requiring team-specific expertise. ## Best practices -This guide contains advice and best practices for performing code review, and -having your code reviewed. - -All merge requests for GitLab CE and EE, whether written by a GitLab team member -or a volunteer contributor, must go through a code review process to ensure the -code is effective, understandable, and maintainable. - -Any developer can, and is encouraged to, perform code review on merge requests -of colleagues and contributors. However, the final decision to accept a merge -request is up to one the project's maintainers, denoted on the -[engineering projects][projects]. - ### Everyone - Accept that many programming decisions are opinions. Discuss tradeoffs, which -- cgit v1.2.1 From 0a4f62d278dd33f6e921084fbdb2f7419b3b0061 Mon Sep 17 00:00:00 2001 From: Mark Lapierre Date: Wed, 10 Oct 2018 14:47:27 +0000 Subject: Update Test plan.md Add ~"test plan" label and update attributes --- .gitlab/issue_templates/Test plan.md | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.gitlab/issue_templates/Test plan.md b/.gitlab/issue_templates/Test plan.md index 580fab206b3..db8e30c419c 100644 --- a/.gitlab/issue_templates/Test plan.md +++ b/.gitlab/issue_templates/Test plan.md @@ -38,22 +38,22 @@ test plan](https://testing.googleblog.com/2011/09/10-minute-test-plan.html) and [this wiki page from an open-source tool that implements the ACC model](https://code.google.com/archive/p/test-analytics/wikis/AccExplained.wiki). --> -| | Simple | Secure | Responsive | Obvious | Stable | -|------------|:------:|:------:|:----------:|:-------:|:------:| -| Admin | | | | | | -| Groups | | | | | | -| Project | | | | | | -| Repository | | | | | | -| Issues | | | | | | -| MRs | | | | | | -| CI/CD | | | | | | -| Ops | | | | | | -| Registry | | | | | | -| Wiki | | | | | | -| Snippets | | | | | | -| Settings | | | | | | -| Tracking | | | | | | -| API | | | | | | +| | Secure | Responsive | Intuitive | Reliable | +|------------|:------:|:----------:|:---------:|:--------:| +| Admin | | | | | +| Groups | | | | | +| Project | | | | | +| Repository | | | | | +| Issues | | | | | +| MRs | | | | | +| CI/CD | | | | | +| Ops | | | | | +| Registry | | | | | +| Wiki | | | | | +| Snippets | | | | | +| Settings | | | | | +| Tracking | | | | | +| API | | | | | ## Capabilities @@ -65,7 +65,7 @@ more complex features could involve multiple or even all. Example (from https://gitlab.com/gitlab-org/gitlab-ce/issues/50353): * Respository is - * Simple + * Intuitive * It's easy to select the desired file template * It doesn't require unnecessary actions to save the change * It's easy to undo the change after selecting a template @@ -93,4 +93,4 @@ When adding new automated tests, please keep [testing levels](https://docs.gitla in mind. --> -/label ~Quality \ No newline at end of file +/label ~Quality ~"test plan" \ No newline at end of file -- cgit v1.2.1 From 4df3d07854b4787c1758972d40ee04cdd9e0ef04 Mon Sep 17 00:00:00 2001 From: Evan Read Date: Thu, 11 Oct 2018 16:13:37 +1000 Subject: Remove broken link - And minor linting fixes. --- doc/administration/job_artifacts.md | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/doc/administration/job_artifacts.md b/doc/administration/job_artifacts.md index 757865ea2c5..a1ea78b64bd 100644 --- a/doc/administration/job_artifacts.md +++ b/doc/administration/job_artifacts.md @@ -1,12 +1,10 @@ # Jobs artifacts administration ->**Notes:** ->- Introduced in GitLab 8.2 and GitLab Runner 0.7.0. ->- Starting with GitLab 8.4 and GitLab Runner 1.0, the artifacts archive format - changed to `ZIP`. ->- Starting with GitLab 8.17, builds are renamed to jobs. ->- This is the administration documentation. For the user guide see - [pipelines/job_artifacts](../user/project/pipelines/job_artifacts.md). +> **Notes:** +> - Introduced in GitLab 8.2 and GitLab Runner 0.7.0. +> - Starting with GitLab 8.4 and GitLab Runner 1.0, the artifacts archive format changed to `ZIP`. +> - Starting with GitLab 8.17, builds are renamed to jobs. +> - This is the administration documentation. For the user guide see [pipelines/job_artifacts](../user/project/pipelines/job_artifacts.md). Artifacts is a list of files and directories which are attached to a job after it completes successfully. This feature is enabled by default in all @@ -98,7 +96,7 @@ _The artifacts are stored by default in If you don't want to use the local disk where GitLab is installed to store the artifacts, you can use an object storage like AWS S3 instead. This configuration relies on valid AWS credentials to be configured already. -Use an [Object storage option][os] like AWS S3 to store job artifacts. +Use an object storage option like AWS S3 to store job artifacts. ### Object Storage Settings @@ -315,4 +313,3 @@ memory and disk I/O. [reconfigure gitlab]: restart_gitlab.md#omnibus-gitlab-reconfigure "How to reconfigure Omnibus GitLab" [restart gitlab]: restart_gitlab.md#installations-from-source "How to restart GitLab" [gitlab workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse "GitLab Workhorse repository" -[os]: https://docs.gitlab.com/administration/job_artifacts.html#using-object-storage -- cgit v1.2.1 From 3421f1d124ecf34c620d75488c22fa3fab602721 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 11 Oct 2018 19:52:42 +0900 Subject: Expose id and name attributes to Applications API --- lib/api/entities.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 5a4b85f98cf..f5e33000a9f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1412,7 +1412,9 @@ module API end class Application < Grape::Entity + expose :id expose :uid, as: :application_id + expose :name, as: :application_name expose :redirect_uri, as: :callback_url end -- cgit v1.2.1 From 6dd4ae0d87fd9a30ab9ce36b5127be36929f5692 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 11 Oct 2018 19:54:15 +0900 Subject: Support GET /applications and DELETE /applications/:id endpoints #52559 --- doc/api/applications.md | 51 ++++++++++++++++++++++++++++++++-- lib/api/applications.rb | 17 ++++++++++++ spec/requests/api/applications_spec.rb | 38 +++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 2 deletions(-) diff --git a/doc/api/applications.md b/doc/api/applications.md index 6d244594b71..d74a3cdf5c1 100644 --- a/doc/api/applications.md +++ b/doc/api/applications.md @@ -4,12 +4,12 @@ [ce-8160]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8160 +Only admin user can use the Applications API. + ## Create a application Create a application by posting a JSON payload. -User must be admin to do that. - Returns `200` if the request succeeds. ``` @@ -30,8 +30,55 @@ Example response: ```json { + "id":1, "application_id": "5832fc6e14300a0d962240a8144466eef4ee93ef0d218477e55f11cf12fc3737", + "application_name": "MyApplication", "secret": "ee1dd64b6adc89cf7e2c23099301ccc2c61b441064e9324d963c46902a85ec34", "callback_url": "http://redirect.uri" } ``` + +## List all applications + +List all registered applications. + +``` +GET /applications +``` + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/applications +``` + +Example response: + +```json +[ + { + "id":1, + "application_id": "5832fc6e14300a0d962240a8144466eef4ee93ef0d218477e55f11cf12fc3737", + "application_name": "MyApplication", + "callback_url": "http://redirect.uri" + } +] +``` + +> Note: the `secret` value will not be exposed by this API. + +## Delete an application + +Delete a specific application. + +Returns `204` if the request succeeds. + +``` +DELETE /applications/:id +``` + +Parameters: + +- `id` (required) - The id of the application (not the application_id) + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/applications/:id +``` diff --git a/lib/api/applications.rb b/lib/api/applications.rb index f29cd7fc003..1c940af95d7 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -24,6 +24,23 @@ module API render_validation_error! application end end + + desc 'Get applications' do + success Entities::ApplicationWithSecret + end + get do + applications = Doorkeeper::Application.all + present applications, with: Entities::Application + end + + # rubocop: disable CodeReuse/ActiveRecord + desc 'Delete an application' + delete ':id' do + Doorkeeper::Application.find_by(id: params[:id]).destroy + + status 204 + end + # rubocop: enable CodeReuse/ActiveRecord end end end diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index f56bc932f40..02dfbfa8fd7 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -5,6 +5,7 @@ describe API::Applications, :api do let(:admin_user) { create(:user, admin: true) } let(:user) { create(:user, admin: false) } + let(:application) { create(:application, name: 'application_name', redirect_uri: 'http://application.url', scopes: '') } describe 'POST /applications' do context 'authenticated and authorized user' do @@ -83,4 +84,41 @@ describe API::Applications, :api do end end end + + describe 'GET /applications' do + context 'authenticated and authorized user' do + it 'can list application' do + get api('/applications') + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to be_a(Array) + end + end + + context 'non-authenticated user' do + it 'cannot list application' do + get api('/applications') + + expect(response).to have_http_status 401 + end + end + end + + describe 'DELETE /applications/:id' do + context 'authenticated and authorized user' do + it 'can delete an application' do + delete api("/applications/#{application.id}") + + expect(response).to have_gitlab_http_status(204) + end + end + + context 'non-authenticated user' do + it 'cannot delete an application' do + delete api("/applications/#{application.id}") + + expect(response).to have_http_status 401 + end + end + end end -- cgit v1.2.1 From 33c88f5e5192bec231656e4253263178d8004e63 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 11 Oct 2018 21:12:25 +0900 Subject: Fix tests --- spec/requests/api/applications_spec.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 02dfbfa8fd7..f95b40fff17 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -88,7 +88,7 @@ describe API::Applications, :api do describe 'GET /applications' do context 'authenticated and authorized user' do it 'can list application' do - get api('/applications') + get api('/applications', admin_user) expect(response).to have_gitlab_http_status(200) expect(json_response).to be_a(Array) @@ -97,7 +97,7 @@ describe API::Applications, :api do context 'non-authenticated user' do it 'cannot list application' do - get api('/applications') + get api('/applications', user) expect(response).to have_http_status 401 end @@ -107,15 +107,17 @@ describe API::Applications, :api do describe 'DELETE /applications/:id' do context 'authenticated and authorized user' do it 'can delete an application' do - delete api("/applications/#{application.id}") - + expect do + delete api("/applications/#{application.id}", admin_user) + end.to change { Doorkeeper::Application.count }.by -1 + expect(response).to have_gitlab_http_status(204) end end context 'non-authenticated user' do it 'cannot delete an application' do - delete api("/applications/#{application.id}") + delete api("/applications/#{application.id}", user) expect(response).to have_http_status 401 end -- cgit v1.2.1 From f1645bf7e722096f570a706d37c3379f07a55a68 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 11 Oct 2018 22:35:17 +0900 Subject: Fix unauthorized user tests and add non-authenticated user tests --- spec/requests/api/applications_spec.rb | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index f95b40fff17..9240dd94990 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -95,6 +95,14 @@ describe API::Applications, :api do end end + context 'authorized user without authorization' do + it 'cannot list application' do + get api('/applications', user) + + expect(response).to have_http_status 403 + end + end + context 'non-authenticated user' do it 'cannot list application' do get api('/applications', user) @@ -109,16 +117,24 @@ describe API::Applications, :api do it 'can delete an application' do expect do delete api("/applications/#{application.id}", admin_user) - end.to change { Doorkeeper::Application.count }.by -1 - + end.to change { Doorkeeper::Application.count }.by(-1) + expect(response).to have_gitlab_http_status(204) end end - context 'non-authenticated user' do + context 'authorized user without authorization' do it 'cannot delete an application' do delete api("/applications/#{application.id}", user) + expect(response).to have_http_status 403 + end + end + + context 'non-authenticated user' do + it 'cannot delete an application' do + delete api("/applications/#{application.id}") + expect(response).to have_http_status 401 end end -- cgit v1.2.1 From 20bbac427ab2aea423d3f377c4319687520475a4 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 11 Oct 2018 23:32:44 +0900 Subject: Hit the database --- spec/requests/api/applications_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 9240dd94990..77b4c5ecbba 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -5,7 +5,7 @@ describe API::Applications, :api do let(:admin_user) { create(:user, admin: true) } let(:user) { create(:user, admin: false) } - let(:application) { create(:application, name: 'application_name', redirect_uri: 'http://application.url', scopes: '') } + let!(:application) { create(:application, name: 'application_name', redirect_uri: 'http://application.url', scopes: '') } describe 'POST /applications' do context 'authenticated and authorized user' do @@ -107,7 +107,7 @@ describe API::Applications, :api do it 'cannot list application' do get api('/applications', user) - expect(response).to have_http_status 401 + expect(response).to have_http_status 403 end end end -- cgit v1.2.1 From 9fcf657889729e23587ec12a1c97c8d0cd43d992 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Fri, 12 Oct 2018 00:25:52 +0900 Subject: Differentiate test application values --- spec/requests/api/applications_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 77b4c5ecbba..5c383317b7e 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -5,7 +5,7 @@ describe API::Applications, :api do let(:admin_user) { create(:user, admin: true) } let(:user) { create(:user, admin: false) } - let!(:application) { create(:application, name: 'application_name', redirect_uri: 'http://application.url', scopes: '') } + let!(:application) { create(:application, name: 'another_application', redirect_uri: 'http://other_application.url', scopes: '') } describe 'POST /applications' do context 'authenticated and authorized user' do -- cgit v1.2.1 From 9e3804a843a9146e7bd77eb6abfe10417be7a549 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Fri, 12 Oct 2018 09:28:40 +0900 Subject: Non-authenticated user test should not have user passed in the API call !22296 --- spec/requests/api/applications_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 5c383317b7e..3e732ed478e 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -105,9 +105,9 @@ describe API::Applications, :api do context 'non-authenticated user' do it 'cannot list application' do - get api('/applications', user) + get api('/applications') - expect(response).to have_http_status 403 + expect(response).to have_http_status 401 end end end -- cgit v1.2.1 From 2122727468a42bf8b5b4c0e7cb18b528cf4ea4ab Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Fri, 12 Oct 2018 09:31:22 +0900 Subject: Add changelog !22296 --- changelogs/unreleased/52559-applications-api-get-delete.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/52559-applications-api-get-delete.yml diff --git a/changelogs/unreleased/52559-applications-api-get-delete.yml b/changelogs/unreleased/52559-applications-api-get-delete.yml new file mode 100644 index 00000000000..19f98a2bb56 --- /dev/null +++ b/changelogs/unreleased/52559-applications-api-get-delete.yml @@ -0,0 +1,5 @@ +--- +title: Add Applications API endpoints for listing and deleting entries. +merge_request: 22296 +author: Jean-Baptiste Vasseur +type: added -- cgit v1.2.1 From 23d70f6281f7c099ff7ce5b9abe40d7afd5ab668 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Fri, 12 Oct 2018 09:40:11 +0900 Subject: Improve call for retrieving all applications !22296 --- lib/api/applications.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/applications.rb b/lib/api/applications.rb index 1c940af95d7..8d83684e3a9 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -29,7 +29,7 @@ module API success Entities::ApplicationWithSecret end get do - applications = Doorkeeper::Application.all + applications = Doorkeeper::Application.where("owner_id IS NULL") present applications, with: Entities::Application end -- cgit v1.2.1 From 1dbbd0b9b01050ef929780474153dfc57a57efdc Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Fri, 12 Oct 2018 09:41:36 +0900 Subject: disable CodeReuse/ActiveRecord --- lib/api/applications.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/api/applications.rb b/lib/api/applications.rb index 8d83684e3a9..048e7bc81f9 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -25,6 +25,7 @@ module API end end + # rubocop: disable CodeReuse/ActiveRecord desc 'Get applications' do success Entities::ApplicationWithSecret end @@ -32,6 +33,7 @@ module API applications = Doorkeeper::Application.where("owner_id IS NULL") present applications, with: Entities::Application end + # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord desc 'Delete an application' -- cgit v1.2.1 From abf7c10b67cfd7827370b25f1e4c64a3f615670b Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Fri, 12 Oct 2018 18:33:58 +0900 Subject: Do not return secret from GET /applications !22296 --- lib/api/applications.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/applications.rb b/lib/api/applications.rb index 048e7bc81f9..9d0f7a093d5 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -27,7 +27,7 @@ module API # rubocop: disable CodeReuse/ActiveRecord desc 'Get applications' do - success Entities::ApplicationWithSecret + success Entities::Application end get do applications = Doorkeeper::Application.where("owner_id IS NULL") -- cgit v1.2.1 From 15168c443364900237e16e17a4c5b8cf7f97a1b4 Mon Sep 17 00:00:00 2001 From: Michael Hahnle Date: Sat, 13 Oct 2018 09:59:31 +0000 Subject: Request to be a German proofreader --- doc/development/i18n/proofreader.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index 5e13c725e15..bf9fa1ed9f0 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -20,6 +20,7 @@ are very appreciative of the work done by translators and proofreaders! - French - Davy Defaud - [GitLab](https://gitlab.com/DevDef), [Crowdin](https://crowdin.com/profile/DevDef) - German + - Michael Hahnle - [GitLab](https://gitlab.com/mhah), [Crowdin](https://crowdin.com/profile/mhah) - Indonesian - Ahmad Naufal Mukhtar - [GitLab](https://gitlab.com/anaufalm), [Crowdin](https://crowdin.com/profile/anaufalm) - Italian -- cgit v1.2.1 From 27326ac62c9bef892ab0dcc255e6abc74573983d Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 13 Oct 2018 16:15:46 -0700 Subject: Associate Rakefile with Ruby icon in diffs Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/52654 --- .../javascripts/vue_shared/components/file_icon/file_icon_map.js | 5 ++--- changelogs/unreleased/sh-associate-rakefile-ruby.yml | 5 +++++ 2 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/sh-associate-rakefile-ruby.yml diff --git a/app/assets/javascripts/vue_shared/components/file_icon/file_icon_map.js b/app/assets/javascripts/vue_shared/components/file_icon/file_icon_map.js index 9bca1993ccc..bffaa096210 100644 --- a/app/assets/javascripts/vue_shared/components/file_icon/file_icon_map.js +++ b/app/assets/javascripts/vue_shared/components/file_icon/file_icon_map.js @@ -549,6 +549,7 @@ const fileNameIcons = { jenkinsfile: 'jenkins', 'firebase.json': 'firebase', '.firebaserc': 'firebase', + Rakefile: 'ruby', 'rollup.config.js': 'rollup', 'rollup.config.ts': 'rollup', 'rollup-config.js': 'rollup', @@ -583,7 +584,5 @@ const fileNameIcons = { }; export default function getIconForFile(name) { - return fileNameIcons[name] || - fileExtensionIcons[name ? name.split('.').pop() : ''] || - ''; + return fileNameIcons[name] || fileExtensionIcons[name ? name.split('.').pop() : ''] || ''; } diff --git a/changelogs/unreleased/sh-associate-rakefile-ruby.yml b/changelogs/unreleased/sh-associate-rakefile-ruby.yml new file mode 100644 index 00000000000..3e3fcb8d860 --- /dev/null +++ b/changelogs/unreleased/sh-associate-rakefile-ruby.yml @@ -0,0 +1,5 @@ +--- +title: Associate Rakefile with Ruby icon in diffs +merge_request: +author: +type: other -- cgit v1.2.1 From c47aea75766fe3165cc9530fc9ccb5a4ac615e46 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Mon, 15 Oct 2018 20:07:56 +0900 Subject: Use have_gitlab_http_status following best practices !22296 --- spec/requests/api/applications_spec.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 3e732ed478e..270e12bf201 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -16,7 +16,7 @@ describe API::Applications, :api do application = Doorkeeper::Application.find_by(name: 'application_name', redirect_uri: 'http://application.url') - expect(response).to have_http_status 201 + expect(response).to have_gitlab_http_status(201) expect(json_response).to be_a Hash expect(json_response['application_id']).to eq application.uid expect(json_response['secret']).to eq application.secret @@ -28,7 +28,7 @@ describe API::Applications, :api do post api('/applications', admin_user), name: 'application_name', redirect_uri: 'wrong_url_format', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['message']['redirect_uri'][0]).to eq('must be an absolute URI.') end @@ -38,7 +38,7 @@ describe API::Applications, :api do post api('/applications', admin_user), redirect_uri: 'http://application.url', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['error']).to eq('name is missing') end @@ -48,7 +48,7 @@ describe API::Applications, :api do post api('/applications', admin_user), name: 'application_name', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['error']).to eq('redirect_uri is missing') end @@ -58,7 +58,7 @@ describe API::Applications, :api do post api('/applications', admin_user), name: 'application_name', redirect_uri: 'http://application.url' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 400 + expect(response).to have_gitlab_http_status(400) expect(json_response).to be_a Hash expect(json_response['error']).to eq('scopes is missing') end @@ -70,7 +70,7 @@ describe API::Applications, :api do post api('/applications', user), name: 'application_name', redirect_uri: 'http://application.url', scopes: '' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 403 + expect(response).to have_gitlab_http_status(403) end end @@ -80,7 +80,7 @@ describe API::Applications, :api do post api('/applications'), name: 'application_name', redirect_uri: 'http://application.url' end.not_to change { Doorkeeper::Application.count } - expect(response).to have_http_status 401 + expect(response).to have_gitlab_http_status(401) end end end @@ -99,7 +99,7 @@ describe API::Applications, :api do it 'cannot list application' do get api('/applications', user) - expect(response).to have_http_status 403 + expect(response).to have_gitlab_http_status(403) end end @@ -107,7 +107,7 @@ describe API::Applications, :api do it 'cannot list application' do get api('/applications') - expect(response).to have_http_status 401 + expect(response).to have_gitlab_http_status(401) end end end @@ -127,7 +127,7 @@ describe API::Applications, :api do it 'cannot delete an application' do delete api("/applications/#{application.id}", user) - expect(response).to have_http_status 403 + expect(response).to have_gitlab_http_status(403) end end @@ -135,7 +135,7 @@ describe API::Applications, :api do it 'cannot delete an application' do delete api("/applications/#{application.id}") - expect(response).to have_http_status 401 + expect(response).to have_gitlab_http_status(401) end end end -- cgit v1.2.1 From 1ae9aefe5572cc62256f28e16163015f075b6c59 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Mon, 15 Oct 2018 23:03:08 +0900 Subject: Use application finder for Doorkeeper Applications --- app/finders/applications_finder.rb | 26 ++++++++++++++++++++++++++ lib/api/applications.rb | 9 +++------ 2 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 app/finders/applications_finder.rb diff --git a/app/finders/applications_finder.rb b/app/finders/applications_finder.rb new file mode 100644 index 00000000000..14bd35105d2 --- /dev/null +++ b/app/finders/applications_finder.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class ApplicationsFinder + attr_reader :params + + def initialize(params = {}) + @params = params + end + + # rubocop: disable CodeReuse/ActiveRecord + def execute + applications = Doorkeeper::Application.where("owner_id IS NULL") + by_id(applications) + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + # rubocop: disable CodeReuse/ActiveRecord + def by_id(applications) + return applications unless params[:id] + + Doorkeeper::Application.find_by(id: params[:id]) + end + # rubocop: enable CodeReuse/ActiveRecord +end diff --git a/lib/api/applications.rb b/lib/api/applications.rb index 9d0f7a093d5..92717e04543 100644 --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -25,24 +25,21 @@ module API end end - # rubocop: disable CodeReuse/ActiveRecord desc 'Get applications' do success Entities::Application end get do - applications = Doorkeeper::Application.where("owner_id IS NULL") + applications = ApplicationsFinder.new.execute present applications, with: Entities::Application end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord desc 'Delete an application' delete ':id' do - Doorkeeper::Application.find_by(id: params[:id]).destroy + application = ApplicationsFinder.new(params).execute + application.destroy status 204 end - # rubocop: enable CodeReuse/ActiveRecord end end end -- cgit v1.2.1 From ea0e7dc5c0cacd32d3329718ea71e7e5a867e0f4 Mon Sep 17 00:00:00 2001 From: benjamin Date: Tue, 16 Oct 2018 18:14:26 +0800 Subject: change generating url functions for external url --- app/views/admin/groups/show.html.haml | 2 +- app/views/admin/projects/show.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index 0c683f86252..21c1260e982 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -119,7 +119,7 @@ = _("%{group_name} group members").html_safe % { group_name: @group.name } %span.badge.badge-pill= @group.members.size .float-right - = link_to icon('pencil-square-o', text: _('Manage access')), polymorphic_url([@group, :members]), class: "btn btn-sm" + = link_to icon('pencil-square-o', text: _('Manage access')), group_group_members_path(@group), class: "btn btn-sm" %ul.content-list.group-users-list.content-list.members-list = render partial: 'shared/members/member', collection: @members, as: :member, locals: { show_controls: false } .card-footer diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index fefb4c7455d..03cce4745aa 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -183,7 +183,7 @@ project members %span.badge.badge-pill= @project.users.size .float-right - = link_to icon('pencil-square-o', text: 'Manage access'), polymorphic_url([@project, :members]), class: "btn btn-sm" + = link_to icon('pencil-square-o', text: 'Manage access'), project_project_members_path(@project), class: "btn btn-sm" %ul.content-list.project_members.members-list = render partial: 'shared/members/member', collection: @project_members, as: :member, locals: { show_controls: false } .card-footer -- cgit v1.2.1 From d5ef55976a0166712a692ee1785412d7f7901273 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 16 Oct 2018 10:39:01 +0100 Subject: When to create follow-up technical debt issues --- doc/development/contributing/issue_workflow.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index c0a635db12f..d302065347e 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -315,6 +315,28 @@ for a release by the appropriate person. Make sure to mention the merge request that the ~"technical debt" issue or ~"UX debt" issue is associated with in the description of the issue. +## Technical debt in follow-up issues + +It's common to discover technical debt during development of a new feature. In +the spirit of "minimum viable change", resolution is often deferred to a +follow-up issue. However, this has limited value unless a commitment to address +the debt is made. As technical debt reduces development velocity, it's important +to keep it under control. + +Before accepting resolution of technical debt in a follow-up issue, maintainers +should check that that fix is not trivial, and that the contributor (or their +team) can commit to scheduling the issue within the next 3 releases. + +Trivial fixes can - and should - be addressed within the same MR. + +If a commitment to address the issue in the foreseeable future cannot be found, +the maintainer must make a value judgement on whether the problem deserves an +issue at all. If the commitment is lacking because the issue is neither trivial +nor valuable, then perhaps no issue needs to be made after all. If a commitment +is lacking because technical debt is being unfairly neglected, then maintainers +should generally insist on resolution of the issue upfront, to protect +development velocity. + ## Stewardship For issues related to the open source stewardship of GitLab, -- cgit v1.2.1 From 15cd91c71a57a0b84af620181a64b26d5aec8237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Mon, 15 Oct 2018 12:17:21 +0200 Subject: Make all legacy security reports to use raw format - This introduces and uses `:raw` format for all legacy reports, the ones that do not have yet proper parsers on Backend - Raw format is needed to make Frontend be able to parse reports, without the need of decompressing, - This also extends fixtures to seed security reports with database, even though parser code is part of EE --- app/models/ci/job_artifact.rb | 17 +- app/presenters/ci/build_runner_presenter.rb | 10 +- changelogs/unreleased/use-raw-file-format.yml | 5 + db/fixtures/development/14_pipelines.rb | 75 +- .../ci/build/artifacts/adapters/gzip_stream.rb | 48 ++ .../ci/build/artifacts/adapters/raw_stream.rb | 27 + lib/gitlab/ci/build/artifacts/gzip_file_adapter.rb | 46 - spec/factories/ci/job_artifacts.rb | 4 +- spec/fixtures/security-reports/feature-branch.zip | Bin 0 -> 7163 bytes .../gl-container-scanning-report.json | 18 + .../feature-branch/gl-dast-report.json | 40 + .../gl-dependency-scanning-report.json | 46 + .../gl-license-management-report.json | 242 ++++++ .../feature-branch/gl-sast-report.json | 944 +++++++++++++++++++++ spec/fixtures/security-reports/master.zip | Bin 0 -> 6710 bytes .../master/gl-container-scanning-report.json | 18 + .../security-reports/master/gl-dast-report.json | 40 + .../master/gl-dependency-scanning-report.json | 35 + .../master/gl-license-management-report.json | 150 ++++ .../security-reports/master/gl-sast-report.json | 944 +++++++++++++++++++++ .../build/artifacts/adapters/gzip_stream_spec.rb | 56 ++ .../ci/build/artifacts/adapters/raw_stream_spec.rb | 47 + .../ci/build/artifacts/gzip_file_adapter_spec.rb | 56 -- spec/models/ci/job_artifact_spec.rb | 8 + spec/presenters/ci/build_runner_presenter_spec.rb | 32 +- 25 files changed, 2777 insertions(+), 131 deletions(-) create mode 100644 changelogs/unreleased/use-raw-file-format.yml create mode 100644 lib/gitlab/ci/build/artifacts/adapters/gzip_stream.rb create mode 100644 lib/gitlab/ci/build/artifacts/adapters/raw_stream.rb delete mode 100644 lib/gitlab/ci/build/artifacts/gzip_file_adapter.rb create mode 100644 spec/fixtures/security-reports/feature-branch.zip create mode 100644 spec/fixtures/security-reports/feature-branch/gl-container-scanning-report.json create mode 100644 spec/fixtures/security-reports/feature-branch/gl-dast-report.json create mode 100644 spec/fixtures/security-reports/feature-branch/gl-dependency-scanning-report.json create mode 100644 spec/fixtures/security-reports/feature-branch/gl-license-management-report.json create mode 100644 spec/fixtures/security-reports/feature-branch/gl-sast-report.json create mode 100644 spec/fixtures/security-reports/master.zip create mode 100644 spec/fixtures/security-reports/master/gl-container-scanning-report.json create mode 100644 spec/fixtures/security-reports/master/gl-dast-report.json create mode 100644 spec/fixtures/security-reports/master/gl-dependency-scanning-report.json create mode 100644 spec/fixtures/security-reports/master/gl-license-management-report.json create mode 100644 spec/fixtures/security-reports/master/gl-sast-report.json create mode 100644 spec/lib/gitlab/ci/build/artifacts/adapters/gzip_stream_spec.rb create mode 100644 spec/lib/gitlab/ci/build/artifacts/adapters/raw_stream_spec.rb delete mode 100644 spec/lib/gitlab/ci/build/artifacts/gzip_file_adapter_spec.rb diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index cb73fc74bb6..2b28b702b05 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -27,11 +27,15 @@ module Ci metadata: :gzip, trace: :raw, junit: :gzip, - codequality: :gzip, - sast: :gzip, - dependency_scanning: :gzip, - container_scanning: :gzip, - dast: :gzip + + # All these file formats use `raw` as we need to store them uncompressed + # for Frontend to fetch the files and do analysis + # When they will be only used by backend, they can be `gzipped`. + codequality: :raw, + sast: :raw, + dependency_scanning: :raw, + container_scanning: :raw, + dast: :raw }.freeze belongs_to :project @@ -100,7 +104,8 @@ module Ci } FILE_FORMAT_ADAPTERS = { - gzip: Gitlab::Ci::Build::Artifacts::GzipFileAdapter + gzip: Gitlab::Ci::Build::Artifacts::Adapters::GzipStream, + raw: Gitlab::Ci::Build::Artifacts::Adapters::RawStream }.freeze def valid_file_format? diff --git a/app/presenters/ci/build_runner_presenter.rb b/app/presenters/ci/build_runner_presenter.rb index 880218e2727..300f85e1e9d 100644 --- a/app/presenters/ci/build_runner_presenter.rb +++ b/app/presenters/ci/build_runner_presenter.rb @@ -30,12 +30,12 @@ module Ci def create_reports(reports, expire_in:) return unless reports&.any? - reports.map do |k, v| + reports.map do |report_type, report_paths| { - artifact_type: k.to_sym, - artifact_format: :gzip, - name: ::Ci::JobArtifact::DEFAULT_FILE_NAMES[k.to_sym], - paths: v, + artifact_type: report_type.to_sym, + artifact_format: ::Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(report_type.to_sym), + name: ::Ci::JobArtifact::DEFAULT_FILE_NAMES.fetch(report_type.to_sym), + paths: report_paths, when: 'always', expire_in: expire_in } diff --git a/changelogs/unreleased/use-raw-file-format.yml b/changelogs/unreleased/use-raw-file-format.yml new file mode 100644 index 00000000000..d86db51fea4 --- /dev/null +++ b/changelogs/unreleased/use-raw-file-format.yml @@ -0,0 +1,5 @@ +--- +title: Make all legacy security reports to use raw format +merge_request: +author: +type: changed diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb index 5535c4a14e5..5af77c49913 100644 --- a/db/fixtures/development/14_pipelines.rb +++ b/db/fixtures/development/14_pipelines.rb @@ -1,7 +1,7 @@ require './spec/support/sidekiq' class Gitlab::Seeder::Pipelines - STAGES = %w[build test deploy notify] + STAGES = %w[build test security deploy notify] BUILDS = [ # build stage { name: 'build:linux', stage: 'build', status: :success, @@ -31,6 +31,16 @@ class Gitlab::Seeder::Pipelines { name: 'spinach:osx', stage: 'test', status: :failed, allow_failure: true, queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, + # security stage + { name: 'dast', stage: 'security', status: :success, + queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, + { name: 'sast', stage: 'security', status: :success, + queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, + { name: 'dependency_scanning', stage: 'security', status: :success, + queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, + { name: 'container_scanning', stage: 'security', status: :success, + queued_at: 8.hour.ago, started_at: 8.hour.ago, finished_at: 7.hour.ago }, + # deploy stage { name: 'staging', stage: 'deploy', environment: 'staging', status_event: :success, options: { environment: { action: 'start', on_stop: 'stop staging' } }, @@ -108,6 +118,11 @@ class Gitlab::Seeder::Pipelines setup_artifacts(build) setup_test_reports(build) + if build.ref == build.project.default_branch + setup_security_reports_file(build) + else + setup_security_reports_legacy_archive(build) + end setup_build_log(build) build.project.environments. @@ -143,6 +158,55 @@ class Gitlab::Seeder::Pipelines end end + def setup_security_reports_file(build) + return unless build.stage == "security" + + # we have two sources: master and feature-branch + branch_name = build.ref == build.project.default_branch ? + 'master' : 'feature-branch' + + artifacts_cache_file(security_reports_path(branch_name, build.name)) do |file| + build.job_artifacts.build( + project: build.project, + file_type: build.name, + file_format: :raw, + file: file) + end + end + + def setup_security_reports_legacy_archive(build) + return unless build.stage == "security" + + # we have two sources: master and feature-branch + branch_name = build.ref == build.project.default_branch ? + 'master' : 'feature-branch' + + artifacts_cache_file(security_reports_archive_path(branch_name)) do |file| + build.job_artifacts.build( + project: build.project, + file_type: :archive, + file_format: :zip, + file: file) + end + + # assign dummy metadata + artifacts_cache_file(artifacts_metadata_path) do |file| + build.job_artifacts.build( + project: build.project, + file_type: :metadata, + file_format: :gzip, + file: file) + end + + build.options = { + artifacts: { + paths: [ + Ci::JobArtifact::DEFAULT_FILE_NAMES.fetch(build.name.to_sym) + ] + } + } + end + def setup_build_log(build) if %w(running success failed).include?(build.status) build.trace.set(FFaker::Lorem.paragraphs(6).join("\n\n")) @@ -190,6 +254,15 @@ class Gitlab::Seeder::Pipelines Rails.root + 'spec/fixtures/junit/junit.xml.gz' end + def security_reports_archive_path(branch) + Rails.root.join('spec', 'fixtures', 'security-reports', branch + '.zip') + end + + def security_reports_path(branch, name) + file_name = Ci::JobArtifact::DEFAULT_FILE_NAMES.fetch(name.to_sym) + Rails.root.join('spec', 'fixtures', 'security-reports', branch, file_name) + end + def artifacts_cache_file(file_path) file = Tempfile.new("artifacts") file.close diff --git a/lib/gitlab/ci/build/artifacts/adapters/gzip_stream.rb b/lib/gitlab/ci/build/artifacts/adapters/gzip_stream.rb new file mode 100644 index 00000000000..ee3647f24fd --- /dev/null +++ b/lib/gitlab/ci/build/artifacts/adapters/gzip_stream.rb @@ -0,0 +1,48 @@ +module Gitlab + module Ci + module Build + module Artifacts + module Adapters + class GzipStream + attr_reader :stream + + InvalidStreamError = Class.new(StandardError) + + def initialize(stream) + raise InvalidStreamError, "Stream is required" unless stream + + @stream = stream + end + + def each_blob + stream.seek(0) + + until stream.eof? + gzip(stream) do |gz| + yield gz.read, gz.orig_name + unused = gz.unused&.length.to_i + # pos has already reached to EOF at the moment + # We rewind the pos to the top of unused files + # to read next gzip stream, to support multistream archives + # https://golang.org/src/compress/gzip/gunzip.go#L117 + stream.seek(-unused, IO::SEEK_CUR) + end + end + end + + private + + def gzip(stream, &block) + gz = Zlib::GzipReader.new(stream) + yield(gz) + rescue Zlib::Error => e + raise InvalidStreamError, e.message + ensure + gz&.finish + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/build/artifacts/adapters/raw_stream.rb b/lib/gitlab/ci/build/artifacts/adapters/raw_stream.rb new file mode 100644 index 00000000000..fa6842cf36a --- /dev/null +++ b/lib/gitlab/ci/build/artifacts/adapters/raw_stream.rb @@ -0,0 +1,27 @@ +module Gitlab + module Ci + module Build + module Artifacts + module Adapters + class RawStream + attr_reader :stream + + InvalidStreamError = Class.new(StandardError) + + def initialize(stream) + raise InvalidStreamError, "Stream is required" unless stream + + @stream = stream + end + + def each_blob + stream.seek(0) + + yield(stream.read, 'raw') unless stream.eof? + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/build/artifacts/gzip_file_adapter.rb b/lib/gitlab/ci/build/artifacts/gzip_file_adapter.rb deleted file mode 100644 index 65f65cdce08..00000000000 --- a/lib/gitlab/ci/build/artifacts/gzip_file_adapter.rb +++ /dev/null @@ -1,46 +0,0 @@ -module Gitlab - module Ci - module Build - module Artifacts - class GzipFileAdapter - attr_reader :stream - - InvalidStreamError = Class.new(StandardError) - - def initialize(stream) - raise InvalidStreamError, "Stream is required" unless stream - - @stream = stream - end - - def each_blob - stream.seek(0) - - until stream.eof? - gzip(stream) do |gz| - yield gz.read, gz.orig_name - unused = gz.unused&.length.to_i - # pos has already reached to EOF at the moment - # We rewind the pos to the top of unused files - # to read next gzip stream, to support multistream archives - # https://golang.org/src/compress/gzip/gunzip.go#L117 - stream.seek(-unused, IO::SEEK_CUR) - end - end - end - - private - - def gzip(stream, &block) - gz = Zlib::GzipReader.new(stream) - yield(gz) - rescue Zlib::Error => e - raise InvalidStreamError, e.message - ensure - gz&.finish - end - end - end - end - end -end diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb index e0aeadeecd7..2c76c22ba69 100644 --- a/spec/factories/ci/job_artifacts.rb +++ b/spec/factories/ci/job_artifacts.rb @@ -119,11 +119,11 @@ FactoryBot.define do trait :codequality do file_type :codequality - file_format :gzip + file_format :raw after(:build) do |artifact, evaluator| artifact.file = fixture_file_upload( - Rails.root.join('spec/fixtures/codequality/codequality.json.gz'), 'application/x-gzip') + Rails.root.join('spec/fixtures/codequality/codequality.json'), 'application/json') end end diff --git a/spec/fixtures/security-reports/feature-branch.zip b/spec/fixtures/security-reports/feature-branch.zip new file mode 100644 index 00000000000..730ce3dc5f8 Binary files /dev/null and b/spec/fixtures/security-reports/feature-branch.zip differ diff --git a/spec/fixtures/security-reports/feature-branch/gl-container-scanning-report.json b/spec/fixtures/security-reports/feature-branch/gl-container-scanning-report.json new file mode 100644 index 00000000000..9840382df6f --- /dev/null +++ b/spec/fixtures/security-reports/feature-branch/gl-container-scanning-report.json @@ -0,0 +1,18 @@ +{ + "image": "registry.gitlab.com/bikebilly/auto-devops-10-6/feature-branch:e7315ba964febb11bac8f5cd6ec433db8a3a1583", + "unapproved": [ + "CVE-2017-15650" + ], + "vulnerabilities": [ + { + "featurename": "musl", + "featureversion": "1.1.14-r15", + "vulnerability": "CVE-2017-15650", + "namespace": "alpine:v3.4", + "description": "", + "link": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15650", + "severity": "Medium", + "fixedby": "1.1.14-r16" + } + ] +} diff --git a/spec/fixtures/security-reports/feature-branch/gl-dast-report.json b/spec/fixtures/security-reports/feature-branch/gl-dast-report.json new file mode 100644 index 00000000000..3a308bf047e --- /dev/null +++ b/spec/fixtures/security-reports/feature-branch/gl-dast-report.json @@ -0,0 +1,40 @@ +{ + "site": { + "alerts": [ + { + "sourceid": "3", + "wascid": "15", + "cweid": "16", + "reference": "

http://msdn.microsoft.com/en-us/library/ie/gg622941%28v=vs.85%29.aspx

https://www.owasp.org/index.php/List_of_useful_HTTP_headers

", + "otherinfo": "

This issue still applies to error type pages (401, 403, 500, etc) as those pages are often still affected by injection issues, in which case there is still concern for browsers sniffing pages away from their actual content type.

At \"High\" threshold this scanner will not alert on client or server error responses.

", + "solution": "

Ensure that the application/web server sets the Content-Type header appropriately, and that it sets the X-Content-Type-Options header to 'nosniff' for all web pages.

If possible, ensure that the end user uses a standards-compliant and modern web browser that does not perform MIME-sniffing at all, or that can be directed by the web application/web server to not perform MIME-sniffing.

", + "count": "2", + "pluginid": "10021", + "alert": "X-Content-Type-Options Header Missing", + "name": "X-Content-Type-Options Header Missing", + "riskcode": "1", + "confidence": "2", + "riskdesc": "Low (Medium)", + "desc": "

The Anti-MIME-Sniffing header X-Content-Type-Options was not set to 'nosniff'. This allows older versions of Internet Explorer and Chrome to perform MIME-sniffing on the response body, potentially causing the response body to be interpreted and displayed as a content type other than the declared content type. Current (early 2014) and legacy versions of Firefox will use the declared content type (if one is set), rather than performing MIME-sniffing.

", + "instances": [ + { + "param": "X-Content-Type-Options", + "method": "GET", + "uri": "http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io" + }, + { + "param": "X-Content-Type-Options", + "method": "GET", + "uri": "http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io/" + } + ] + } + ], + "@ssl": "false", + "@port": "80", + "@host": "bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io", + "@name": "http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io" + }, + "@generated": "Fri, 13 Apr 2018 09:22:01", + "@version": "2.7.0" +} diff --git a/spec/fixtures/security-reports/feature-branch/gl-dependency-scanning-report.json b/spec/fixtures/security-reports/feature-branch/gl-dependency-scanning-report.json new file mode 100644 index 00000000000..4b47e259c0f --- /dev/null +++ b/spec/fixtures/security-reports/feature-branch/gl-dependency-scanning-report.json @@ -0,0 +1,46 @@ +[ + { + "priority": "Unknown", + "file": "pom.xml", + "cve": "CVE-2012-4387", + "url": "http://struts.apache.org/docs/s2-011.html", + "message": "Long parameter name DoS for org.apache.struts/struts2-core", + "tools": [ + "gemnasium" + ], + "tool": "gemnasium" + }, + { + "priority": "Unknown", + "file": "pom.xml", + "cve": "CVE-2013-1966", + "url": "http://struts.apache.org/docs/s2-014.html", + "message": "Remote command execution due to flaw in the includeParams attribute of URL and Anchor tags for org.apache.struts/struts2-core", + "tools": [ + "gemnasium" + ], + "tool": "gemnasium" + }, + { + "priority": "Unknown", + "file": "pom.xml", + "cve": "CVE-2013-2115", + "url": "http://struts.apache.org/docs/s2-014.html", + "message": "Remote command execution due to flaw in the includeParams attribute of URL and Anchor tags for org.apache.struts/struts2-core", + "tools": [ + "gemnasium" + ], + "tool": "gemnasium" + }, + { + "priority": "Unknown", + "file": "pom.xml", + "cve": "CVE-2013-2134", + "url": "http://struts.apache.org/docs/s2-015.html", + "message": "Arbitrary OGNL code execution via unsanitized wildcard matching for org.apache.struts/struts2-core", + "tools": [ + "gemnasium" + ], + "tool": "gemnasium" + } +] diff --git a/spec/fixtures/security-reports/feature-branch/gl-license-management-report.json b/spec/fixtures/security-reports/feature-branch/gl-license-management-report.json new file mode 100644 index 00000000000..c1d20fa02fa --- /dev/null +++ b/spec/fixtures/security-reports/feature-branch/gl-license-management-report.json @@ -0,0 +1,242 @@ +{ + "licenses": [ + { + "count": 13, + "name": "MIT" + }, + { + "count": 2, + "name": "New BSD" + }, + { + "count": 1, + "name": "LGPL" + } + ], + "dependencies": [ + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "bundler", + "url": "http://bundler.io", + "description": "The best way to manage your application's dependencies", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "concurrent-ruby", + "url": "http://www.concurrent-ruby.com", + "description": "Modern concurrency tools for Ruby. Inspired by Erlang, Clojure, Scala, Haskell, F#, C#, Java, and classic concurrency patterns.", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "connection_pool", + "url": "https://github.com/mperham/connection_pool", + "description": "Generic connection pool for Ruby", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "mini_portile2", + "url": "http://github.com/flavorjones/mini_portile", + "description": "Simplistic port-like solution for developers", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "mustermann", + "url": "https://github.com/sinatra/mustermann", + "description": "Your personal string matching expert.", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "nokogiri", + "url": "http://nokogiri.org", + "description": "Nokogiri (鋸) is an HTML, XML, SAX, and Reader parser", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "New BSD", + "url": "http://opensource.org/licenses/BSD-3-Clause" + }, + "dependency": { + "name": "pg", + "url": "https://bitbucket.org/ged/ruby-pg", + "description": "Pg is the Ruby interface to the {PostgreSQL RDBMS}[http://www.postgresql.org/]", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "New BSD", + "url": "http://opensource.org/licenses/BSD-3-Clause" + }, + "dependency": { + "name": "puma", + "url": "http://puma.io", + "description": "Puma is a simple, fast, threaded, and highly concurrent HTTP 1.1 server for Ruby/Rack applications", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "rack", + "url": "https://rack.github.io/", + "description": "a modular Ruby webserver interface", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "rack-protection", + "url": "http://github.com/sinatra/sinatra/tree/master/rack-protection", + "description": "Protect against typical web attacks, works with all Rack apps, including Rails.", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "redis", + "url": "https://github.com/redis/redis-rb", + "description": "A Ruby client library for Redis", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "LGPL", + "url": "http://www.gnu.org/licenses/lgpl.txt" + }, + "dependency": { + "name": "sidekiq", + "url": "http://sidekiq.org", + "description": "Simple, efficient background processing for Ruby", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "sinatra", + "url": "http://www.sinatrarb.com/", + "description": "Classy web-development dressed in a DSL", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "slim", + "url": "http://slim-lang.com/", + "description": "Slim is a template language.", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "temple", + "url": "https://github.com/judofyr/temple", + "description": "Template compilation framework in Ruby", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "tilt", + "url": "http://github.com/rtomayko/tilt/", + "description": "Generic interface to multiple Ruby template engines", + "pathes": [ + "." + ] + } + } + ] +} diff --git a/spec/fixtures/security-reports/feature-branch/gl-sast-report.json b/spec/fixtures/security-reports/feature-branch/gl-sast-report.json new file mode 100644 index 00000000000..a85b9be8b5f --- /dev/null +++ b/spec/fixtures/security-reports/feature-branch/gl-sast-report.json @@ -0,0 +1,944 @@ +[ + { + "category": "sast", + "message": "Probable insecure usage of temp file/directory.", + "cve": "python/hardcoded/hardcoded-tmp.py:52865813c884a507be1f152d654245af34aba8a391626d01f1ab6d3f52ec8779:B108", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-tmp.py", + "start_line": 1, + "end_line": 1 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B108", + "value": "B108", + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html" + } + ], + "priority": "Medium", + "file": "python/hardcoded/hardcoded-tmp.py", + "line": 1, + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html", + "tool": "bandit" + }, + { + "category": "sast", + "name": "Predictable pseudorandom number generator", + "message": "Predictable pseudorandom number generator", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:47:PREDICTABLE_RANDOM", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 47, + "end_line": 47, + "class": "com.gitlab.security_products.tests.App", + "method": "generateSecretToken2" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-PREDICTABLE_RANDOM", + "value": "PREDICTABLE_RANDOM", + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 47, + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "name": "Predictable pseudorandom number generator", + "message": "Predictable pseudorandom number generator", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:41:PREDICTABLE_RANDOM", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 41, + "end_line": 41, + "class": "com.gitlab.security_products.tests.App", + "method": "generateSecretToken1" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-PREDICTABLE_RANDOM", + "value": "PREDICTABLE_RANDOM", + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 41, + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:cb203b465dffb0cb3a8e8bd8910b84b93b0a5995a938e4b903dbb0cd6ffa1254:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 11, + "end_line": 11 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 11, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:a7173c43ae66bd07466632d819d450e0071e02dbf782763640d1092981f9631b:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 12, + "end_line": 12 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 12, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:017017b77deb0b8369b6065947833eeea752a92ec8a700db590fece3e934cf0d:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 13, + "end_line": 13 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 13, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:45fc8c53aea7b84f06bc4e590cc667678d6073c4c8a1d471177ca2146fb22db2:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 14, + "end_line": 14 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 14, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Pickle library appears to be in use, possible security issue.", + "cve": "python/imports/imports-aliases.py:5f200d47291e7bbd8352db23019b85453ca048dd98ea0c291260fa7d009963a4:B301", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 15, + "end_line": 15 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B301", + "value": "B301" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 15, + "tool": "bandit" + }, + { + "category": "sast", + "name": "ECB mode is insecure", + "message": "ECB mode is insecure", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:29:ECB_MODE", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 29, + "end_line": 29, + "class": "com.gitlab.security_products.tests.App", + "method": "insecureCypher" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-ECB_MODE", + "value": "ECB_MODE", + "url": "https://find-sec-bugs.github.io/bugs.htm#ECB_MODE" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 29, + "url": "https://find-sec-bugs.github.io/bugs.htm#ECB_MODE", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "name": "Cipher with no integrity", + "message": "Cipher with no integrity", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:29:CIPHER_INTEGRITY", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 29, + "end_line": 29, + "class": "com.gitlab.security_products.tests.App", + "method": "insecureCypher" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-CIPHER_INTEGRITY", + "value": "CIPHER_INTEGRITY", + "url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 29, + "url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "message": "Probable insecure usage of temp file/directory.", + "cve": "python/hardcoded/hardcoded-tmp.py:63dd4d626855555b816985d82c4614a790462a0a3ada89dc58eb97f9c50f3077:B108", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-tmp.py", + "start_line": 14, + "end_line": 14 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B108", + "value": "B108", + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html" + } + ], + "priority": "Medium", + "file": "python/hardcoded/hardcoded-tmp.py", + "line": 14, + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Probable insecure usage of temp file/directory.", + "cve": "python/hardcoded/hardcoded-tmp.py:4ad6d4c40a8c263fc265f3384724014e0a4f8dd6200af83e51ff120420038031:B108", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-tmp.py", + "start_line": 10, + "end_line": 10 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B108", + "value": "B108", + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html" + } + ], + "priority": "Medium", + "file": "python/hardcoded/hardcoded-tmp.py", + "line": 10, + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with Popen module.", + "cve": "python/imports/imports-aliases.py:2c3e1fa1e54c3c6646e8bcfaee2518153c6799b77587ff8d9a7b0631f6d34785:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 1, + "end_line": 1 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 1, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with pickle module.", + "cve": "python/imports/imports.py:af58d07f6ad519ef5287fcae65bf1a6999448a1a3a8bc1ac2a11daa80d0b96bf:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports.py", + "start_line": 2, + "end_line": 2 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports.py", + "line": 2, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with subprocess module.", + "cve": "python/imports/imports.py:8de9bc98029d212db530785a5f6780cfa663548746ff228ab8fa96c5bb82f089:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports.py", + "start_line": 4, + "end_line": 4 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports.py", + "line": 4, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'blerg'", + "cve": "python/hardcoded/hardcoded-passwords.py:97c30f1d76d2a88913e3ce9ae74087874d740f87de8af697a9c455f01119f633:B106", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 22, + "end_line": 22 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B106", + "value": "B106", + "url": "https://docs.openstack.org/bandit/latest/plugins/b106_hardcoded_password_funcarg.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 22, + "url": "https://docs.openstack.org/bandit/latest/plugins/b106_hardcoded_password_funcarg.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'root'", + "cve": "python/hardcoded/hardcoded-passwords.py:7431c73a0bc16d94ece2a2e75ef38f302574d42c37ac0c3c38ad0b3bf8a59f10:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 5, + "end_line": 5 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 5, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: ''", + "cve": "python/hardcoded/hardcoded-passwords.py:d2d1857c27caedd49c57bfbcdc23afcc92bd66a22701fcdc632869aab4ca73ee:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 9, + "end_line": 9 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 9, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'ajklawejrkl42348swfgkg'", + "cve": "python/hardcoded/hardcoded-passwords.py:fb3866215a61393a5c9c32a3b60e2058171a23219c353f722cbd3567acab21d2:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 13, + "end_line": 13 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 13, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'blerg'", + "cve": "python/hardcoded/hardcoded-passwords.py:63c62a8b7e1e5224439bd26b28030585ac48741e28ca64561a6071080c560a5f:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 23, + "end_line": 23 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 23, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'blerg'", + "cve": "python/hardcoded/hardcoded-passwords.py:4311b06d08df8fa58229b341c531da8e1a31ec4520597bdff920cd5c098d86f9:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 24, + "end_line": 24 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 24, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with subprocess module.", + "cve": "python/imports/imports-function.py:5858400c2f39047787702de44d03361ef8d954c9d14bd54ee1c2bef9e6a7df93:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-function.py", + "start_line": 4, + "end_line": 4 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-function.py", + "line": 4, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with pickle module.", + "cve": "python/imports/imports-function.py:dbda3cf4190279d30e0aad7dd137eca11272b0b225e8af4e8bf39682da67d956:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-function.py", + "start_line": 2, + "end_line": 2 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports-function.py", + "line": 2, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with Popen module.", + "cve": "python/imports/imports-from.py:eb8a0db9cd1a8c1ab39a77e6025021b1261cc2a0b026b2f4a11fca4e0636d8dd:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-from.py", + "start_line": 7, + "end_line": 7 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-from.py", + "line": 7, + "tool": "bandit" + }, + { + "category": "sast", + "message": "subprocess call with shell=True seems safe, but may be changed in the future, consider rewriting without shell", + "cve": "python/imports/imports-aliases.py:f99f9721e27537fbcb6699a4cf39c6740d6234d2c6f06cfc2d9ea977313c483d:B602", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 9, + "end_line": 9 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B602", + "value": "B602", + "url": "https://docs.openstack.org/bandit/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 9, + "url": "https://docs.openstack.org/bandit/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with subprocess module.", + "cve": "python/imports/imports-from.py:332a12ab1146698f614a905ce6a6a5401497a12281aef200e80522711c69dcf4:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-from.py", + "start_line": 6, + "end_line": 6 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-from.py", + "line": 6, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with Popen module.", + "cve": "python/imports/imports-from.py:0a48de4a3d5348853a03666cb574697e3982998355e7a095a798bd02a5947276:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-from.py", + "start_line": 1, + "end_line": 2 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-from.py", + "line": 1, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with pickle module.", + "cve": "python/imports/imports-aliases.py:51b71661dff994bde3529639a727a678c8f5c4c96f00d300913f6d5be1bbdf26:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 7, + "end_line": 8 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 7, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with loads module.", + "cve": "python/imports/imports-aliases.py:6ff02aeb3149c01ab68484d794a94f58d5d3e3bb0d58557ef4153644ea68ea54:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 6, + "end_line": 6 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 6, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120)", + "cve": "c/subdir/utils.c:b466873101951fe96e1332f6728eb7010acbbd5dfc3b65d7d53571d091a06d9e:CWE-119!/CWE-120", + "confidence": "Low", + "solution": "Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "c/subdir/utils.c", + "start_line": 4 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-119", + "value": "119", + "url": "https://cwe.mitre.org/data/definitions/119.html" + }, + { + "type": "cwe", + "name": "CWE-120", + "value": "120", + "url": "https://cwe.mitre.org/data/definitions/120.html" + } + ], + "file": "c/subdir/utils.c", + "line": 4, + "url": "https://cwe.mitre.org/data/definitions/119.html", + "tool": "flawfinder" + }, + { + "category": "sast", + "message": "Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362)", + "cve": "c/subdir/utils.c:bab681140fcc8fc3085b6bba74081b44ea145c1c98b5e70cf19ace2417d30770:CWE-362", + "confidence": "Low", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "c/subdir/utils.c", + "start_line": 8 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-362", + "value": "362", + "url": "https://cwe.mitre.org/data/definitions/362.html" + } + ], + "file": "c/subdir/utils.c", + "line": 8, + "url": "https://cwe.mitre.org/data/definitions/362.html", + "tool": "flawfinder" + }, + { + "category": "sast", + "message": "Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120)", + "cve": "cplusplus/src/hello.cpp:c8c6dd0afdae6814194cf0930b719f757ab7b379cf8f261e7f4f9f2f323a818a:CWE-119!/CWE-120", + "confidence": "Low", + "solution": "Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "cplusplus/src/hello.cpp", + "start_line": 6 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-119", + "value": "119", + "url": "https://cwe.mitre.org/data/definitions/119.html" + }, + { + "type": "cwe", + "name": "CWE-120", + "value": "120", + "url": "https://cwe.mitre.org/data/definitions/120.html" + } + ], + "file": "cplusplus/src/hello.cpp", + "line": 6, + "url": "https://cwe.mitre.org/data/definitions/119.html", + "tool": "flawfinder" + }, + { + "category": "sast", + "message": "Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120)", + "cve": "cplusplus/src/hello.cpp:331c04062c4fe0c7c486f66f59e82ad146ab33cdd76ae757ca41f392d568cbd0:CWE-120", + "confidence": "Low", + "solution": "Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused)", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "cplusplus/src/hello.cpp", + "start_line": 7 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-120", + "value": "120", + "url": "https://cwe.mitre.org/data/definitions/120.html" + } + ], + "file": "cplusplus/src/hello.cpp", + "line": 7, + "url": "https://cwe.mitre.org/data/definitions/120.html", + "tool": "flawfinder" + } +] diff --git a/spec/fixtures/security-reports/master.zip b/spec/fixtures/security-reports/master.zip new file mode 100644 index 00000000000..4684aecb738 Binary files /dev/null and b/spec/fixtures/security-reports/master.zip differ diff --git a/spec/fixtures/security-reports/master/gl-container-scanning-report.json b/spec/fixtures/security-reports/master/gl-container-scanning-report.json new file mode 100644 index 00000000000..500c19e3abb --- /dev/null +++ b/spec/fixtures/security-reports/master/gl-container-scanning-report.json @@ -0,0 +1,18 @@ +{ + "image": "registry.gitlab.com/bikebilly/auto-devops-10-6/feature-branch:e7315ba964febb11bac8f5cd6ec433db8a3a1583", + "unapproved": [ + "CVE-2017-15651" + ], + "vulnerabilities": [ + { + "featurename": "musl", + "featureversion": "1.1.14-r15", + "vulnerability": "CVE-2017-15651", + "namespace": "alpine:v3.4", + "description": "", + "link": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-15651", + "severity": "Medium", + "fixedby": "1.1.14-r16" + } + ] +} diff --git a/spec/fixtures/security-reports/master/gl-dast-report.json b/spec/fixtures/security-reports/master/gl-dast-report.json new file mode 100644 index 00000000000..3a308bf047e --- /dev/null +++ b/spec/fixtures/security-reports/master/gl-dast-report.json @@ -0,0 +1,40 @@ +{ + "site": { + "alerts": [ + { + "sourceid": "3", + "wascid": "15", + "cweid": "16", + "reference": "

http://msdn.microsoft.com/en-us/library/ie/gg622941%28v=vs.85%29.aspx

https://www.owasp.org/index.php/List_of_useful_HTTP_headers

", + "otherinfo": "

This issue still applies to error type pages (401, 403, 500, etc) as those pages are often still affected by injection issues, in which case there is still concern for browsers sniffing pages away from their actual content type.

At \"High\" threshold this scanner will not alert on client or server error responses.

", + "solution": "

Ensure that the application/web server sets the Content-Type header appropriately, and that it sets the X-Content-Type-Options header to 'nosniff' for all web pages.

If possible, ensure that the end user uses a standards-compliant and modern web browser that does not perform MIME-sniffing at all, or that can be directed by the web application/web server to not perform MIME-sniffing.

", + "count": "2", + "pluginid": "10021", + "alert": "X-Content-Type-Options Header Missing", + "name": "X-Content-Type-Options Header Missing", + "riskcode": "1", + "confidence": "2", + "riskdesc": "Low (Medium)", + "desc": "

The Anti-MIME-Sniffing header X-Content-Type-Options was not set to 'nosniff'. This allows older versions of Internet Explorer and Chrome to perform MIME-sniffing on the response body, potentially causing the response body to be interpreted and displayed as a content type other than the declared content type. Current (early 2014) and legacy versions of Firefox will use the declared content type (if one is set), rather than performing MIME-sniffing.

", + "instances": [ + { + "param": "X-Content-Type-Options", + "method": "GET", + "uri": "http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io" + }, + { + "param": "X-Content-Type-Options", + "method": "GET", + "uri": "http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io/" + } + ] + } + ], + "@ssl": "false", + "@port": "80", + "@host": "bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io", + "@name": "http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io" + }, + "@generated": "Fri, 13 Apr 2018 09:22:01", + "@version": "2.7.0" +} diff --git a/spec/fixtures/security-reports/master/gl-dependency-scanning-report.json b/spec/fixtures/security-reports/master/gl-dependency-scanning-report.json new file mode 100644 index 00000000000..b4e4e8e7dd5 --- /dev/null +++ b/spec/fixtures/security-reports/master/gl-dependency-scanning-report.json @@ -0,0 +1,35 @@ +[ + { + "priority": "Unknown", + "file": "pom.xml", + "cve": "CVE-2012-4386", + "url": "http://struts.apache.org/docs/s2-010.html", + "message": "CSRF protection bypass for org.apache.struts/struts2-core", + "tools": [ + "gemnasium" + ], + "tool": "gemnasium" + }, + { + "priority": "Unknown", + "file": "pom.xml", + "cve": "CVE-2012-4387", + "url": "http://struts.apache.org/docs/s2-011.html", + "message": "Long parameter name DoS for org.apache.struts/struts2-core", + "tools": [ + "gemnasium" + ], + "tool": "gemnasium" + }, + { + "priority": "Unknown", + "file": "pom.xml", + "cve": "CVE-2013-1966", + "url": "http://struts.apache.org/docs/s2-014.html", + "message": "Remote command execution due to flaw in the includeParams attribute of URL and Anchor tags for org.apache.struts/struts2-core", + "tools": [ + "gemnasium" + ], + "tool": "gemnasium" + } +] diff --git a/spec/fixtures/security-reports/master/gl-license-management-report.json b/spec/fixtures/security-reports/master/gl-license-management-report.json new file mode 100644 index 00000000000..fe91e4fb7ee --- /dev/null +++ b/spec/fixtures/security-reports/master/gl-license-management-report.json @@ -0,0 +1,150 @@ +{ + "licenses": [ + { + "count": 10, + "name": "MIT" + } + ], + "dependencies": [ + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "mini_portile2", + "url": "http://github.com/flavorjones/mini_portile", + "description": "Simplistic port-like solution for developers", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "mustermann", + "url": "https://github.com/sinatra/mustermann", + "description": "Your personal string matching expert.", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "nokogiri", + "url": "http://nokogiri.org", + "description": "Nokogiri (鋸) is an HTML, XML, SAX, and Reader parser", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "rack", + "url": "https://rack.github.io/", + "description": "a modular Ruby webserver interface", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "rack-protection", + "url": "http://github.com/sinatra/sinatra/tree/master/rack-protection", + "description": "Protect against typical web attacks, works with all Rack apps, including Rails.", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "redis", + "url": "https://github.com/redis/redis-rb", + "description": "A Ruby client library for Redis", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "sinatra", + "url": "http://www.sinatrarb.com/", + "description": "Classy web-development dressed in a DSL", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "slim", + "url": "http://slim-lang.com/", + "description": "Slim is a template language.", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "temple", + "url": "https://github.com/judofyr/temple", + "description": "Template compilation framework in Ruby", + "pathes": [ + "." + ] + } + }, + { + "license": { + "name": "MIT", + "url": "http://opensource.org/licenses/mit-license" + }, + "dependency": { + "name": "tilt", + "url": "http://github.com/rtomayko/tilt/", + "description": "Generic interface to multiple Ruby template engines", + "pathes": [ + "." + ] + } + } + ] +} diff --git a/spec/fixtures/security-reports/master/gl-sast-report.json b/spec/fixtures/security-reports/master/gl-sast-report.json new file mode 100644 index 00000000000..a85b9be8b5f --- /dev/null +++ b/spec/fixtures/security-reports/master/gl-sast-report.json @@ -0,0 +1,944 @@ +[ + { + "category": "sast", + "message": "Probable insecure usage of temp file/directory.", + "cve": "python/hardcoded/hardcoded-tmp.py:52865813c884a507be1f152d654245af34aba8a391626d01f1ab6d3f52ec8779:B108", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-tmp.py", + "start_line": 1, + "end_line": 1 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B108", + "value": "B108", + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html" + } + ], + "priority": "Medium", + "file": "python/hardcoded/hardcoded-tmp.py", + "line": 1, + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html", + "tool": "bandit" + }, + { + "category": "sast", + "name": "Predictable pseudorandom number generator", + "message": "Predictable pseudorandom number generator", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:47:PREDICTABLE_RANDOM", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 47, + "end_line": 47, + "class": "com.gitlab.security_products.tests.App", + "method": "generateSecretToken2" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-PREDICTABLE_RANDOM", + "value": "PREDICTABLE_RANDOM", + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 47, + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "name": "Predictable pseudorandom number generator", + "message": "Predictable pseudorandom number generator", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:41:PREDICTABLE_RANDOM", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 41, + "end_line": 41, + "class": "com.gitlab.security_products.tests.App", + "method": "generateSecretToken1" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-PREDICTABLE_RANDOM", + "value": "PREDICTABLE_RANDOM", + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 41, + "url": "https://find-sec-bugs.github.io/bugs.htm#PREDICTABLE_RANDOM", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:cb203b465dffb0cb3a8e8bd8910b84b93b0a5995a938e4b903dbb0cd6ffa1254:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 11, + "end_line": 11 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 11, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:a7173c43ae66bd07466632d819d450e0071e02dbf782763640d1092981f9631b:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 12, + "end_line": 12 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 12, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:017017b77deb0b8369b6065947833eeea752a92ec8a700db590fece3e934cf0d:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 13, + "end_line": 13 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 13, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Use of insecure MD2, MD4, or MD5 hash function.", + "cve": "python/imports/imports-aliases.py:45fc8c53aea7b84f06bc4e590cc667678d6073c4c8a1d471177ca2146fb22db2:B303", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 14, + "end_line": 14 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B303", + "value": "B303" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 14, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Pickle library appears to be in use, possible security issue.", + "cve": "python/imports/imports-aliases.py:5f200d47291e7bbd8352db23019b85453ca048dd98ea0c291260fa7d009963a4:B301", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 15, + "end_line": 15 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B301", + "value": "B301" + } + ], + "priority": "Medium", + "file": "python/imports/imports-aliases.py", + "line": 15, + "tool": "bandit" + }, + { + "category": "sast", + "name": "ECB mode is insecure", + "message": "ECB mode is insecure", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:29:ECB_MODE", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 29, + "end_line": 29, + "class": "com.gitlab.security_products.tests.App", + "method": "insecureCypher" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-ECB_MODE", + "value": "ECB_MODE", + "url": "https://find-sec-bugs.github.io/bugs.htm#ECB_MODE" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 29, + "url": "https://find-sec-bugs.github.io/bugs.htm#ECB_MODE", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "name": "Cipher with no integrity", + "message": "Cipher with no integrity", + "cve": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy:29:CIPHER_INTEGRITY", + "severity": "Medium", + "confidence": "High", + "scanner": { + "id": "find_sec_bugs", + "name": "Find Security Bugs" + }, + "location": { + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "start_line": 29, + "end_line": 29, + "class": "com.gitlab.security_products.tests.App", + "method": "insecureCypher" + }, + "identifiers": [ + { + "type": "find_sec_bugs_type", + "name": "Find Security Bugs-CIPHER_INTEGRITY", + "value": "CIPHER_INTEGRITY", + "url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY" + } + ], + "priority": "Medium", + "file": "groovy/src/main/java/com/gitlab/security_products/tests/App.groovy", + "line": 29, + "url": "https://find-sec-bugs.github.io/bugs.htm#CIPHER_INTEGRITY", + "tool": "find_sec_bugs" + }, + { + "category": "sast", + "message": "Probable insecure usage of temp file/directory.", + "cve": "python/hardcoded/hardcoded-tmp.py:63dd4d626855555b816985d82c4614a790462a0a3ada89dc58eb97f9c50f3077:B108", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-tmp.py", + "start_line": 14, + "end_line": 14 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B108", + "value": "B108", + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html" + } + ], + "priority": "Medium", + "file": "python/hardcoded/hardcoded-tmp.py", + "line": 14, + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Probable insecure usage of temp file/directory.", + "cve": "python/hardcoded/hardcoded-tmp.py:4ad6d4c40a8c263fc265f3384724014e0a4f8dd6200af83e51ff120420038031:B108", + "severity": "Medium", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-tmp.py", + "start_line": 10, + "end_line": 10 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B108", + "value": "B108", + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html" + } + ], + "priority": "Medium", + "file": "python/hardcoded/hardcoded-tmp.py", + "line": 10, + "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with Popen module.", + "cve": "python/imports/imports-aliases.py:2c3e1fa1e54c3c6646e8bcfaee2518153c6799b77587ff8d9a7b0631f6d34785:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 1, + "end_line": 1 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 1, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with pickle module.", + "cve": "python/imports/imports.py:af58d07f6ad519ef5287fcae65bf1a6999448a1a3a8bc1ac2a11daa80d0b96bf:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports.py", + "start_line": 2, + "end_line": 2 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports.py", + "line": 2, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with subprocess module.", + "cve": "python/imports/imports.py:8de9bc98029d212db530785a5f6780cfa663548746ff228ab8fa96c5bb82f089:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports.py", + "start_line": 4, + "end_line": 4 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports.py", + "line": 4, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'blerg'", + "cve": "python/hardcoded/hardcoded-passwords.py:97c30f1d76d2a88913e3ce9ae74087874d740f87de8af697a9c455f01119f633:B106", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 22, + "end_line": 22 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B106", + "value": "B106", + "url": "https://docs.openstack.org/bandit/latest/plugins/b106_hardcoded_password_funcarg.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 22, + "url": "https://docs.openstack.org/bandit/latest/plugins/b106_hardcoded_password_funcarg.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'root'", + "cve": "python/hardcoded/hardcoded-passwords.py:7431c73a0bc16d94ece2a2e75ef38f302574d42c37ac0c3c38ad0b3bf8a59f10:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 5, + "end_line": 5 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 5, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: ''", + "cve": "python/hardcoded/hardcoded-passwords.py:d2d1857c27caedd49c57bfbcdc23afcc92bd66a22701fcdc632869aab4ca73ee:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 9, + "end_line": 9 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 9, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'ajklawejrkl42348swfgkg'", + "cve": "python/hardcoded/hardcoded-passwords.py:fb3866215a61393a5c9c32a3b60e2058171a23219c353f722cbd3567acab21d2:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 13, + "end_line": 13 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 13, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'blerg'", + "cve": "python/hardcoded/hardcoded-passwords.py:63c62a8b7e1e5224439bd26b28030585ac48741e28ca64561a6071080c560a5f:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 23, + "end_line": 23 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 23, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Possible hardcoded password: 'blerg'", + "cve": "python/hardcoded/hardcoded-passwords.py:4311b06d08df8fa58229b341c531da8e1a31ec4520597bdff920cd5c098d86f9:B105", + "severity": "Low", + "confidence": "Medium", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/hardcoded/hardcoded-passwords.py", + "start_line": 24, + "end_line": 24 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B105", + "value": "B105", + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html" + } + ], + "priority": "Low", + "file": "python/hardcoded/hardcoded-passwords.py", + "line": 24, + "url": "https://docs.openstack.org/bandit/latest/plugins/b105_hardcoded_password_string.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with subprocess module.", + "cve": "python/imports/imports-function.py:5858400c2f39047787702de44d03361ef8d954c9d14bd54ee1c2bef9e6a7df93:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-function.py", + "start_line": 4, + "end_line": 4 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-function.py", + "line": 4, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with pickle module.", + "cve": "python/imports/imports-function.py:dbda3cf4190279d30e0aad7dd137eca11272b0b225e8af4e8bf39682da67d956:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-function.py", + "start_line": 2, + "end_line": 2 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports-function.py", + "line": 2, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with Popen module.", + "cve": "python/imports/imports-from.py:eb8a0db9cd1a8c1ab39a77e6025021b1261cc2a0b026b2f4a11fca4e0636d8dd:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-from.py", + "start_line": 7, + "end_line": 7 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-from.py", + "line": 7, + "tool": "bandit" + }, + { + "category": "sast", + "message": "subprocess call with shell=True seems safe, but may be changed in the future, consider rewriting without shell", + "cve": "python/imports/imports-aliases.py:f99f9721e27537fbcb6699a4cf39c6740d6234d2c6f06cfc2d9ea977313c483d:B602", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 9, + "end_line": 9 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B602", + "value": "B602", + "url": "https://docs.openstack.org/bandit/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 9, + "url": "https://docs.openstack.org/bandit/latest/plugins/b602_subprocess_popen_with_shell_equals_true.html", + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with subprocess module.", + "cve": "python/imports/imports-from.py:332a12ab1146698f614a905ce6a6a5401497a12281aef200e80522711c69dcf4:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-from.py", + "start_line": 6, + "end_line": 6 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-from.py", + "line": 6, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with Popen module.", + "cve": "python/imports/imports-from.py:0a48de4a3d5348853a03666cb574697e3982998355e7a095a798bd02a5947276:B404", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-from.py", + "start_line": 1, + "end_line": 2 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B404", + "value": "B404" + } + ], + "priority": "Low", + "file": "python/imports/imports-from.py", + "line": 1, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with pickle module.", + "cve": "python/imports/imports-aliases.py:51b71661dff994bde3529639a727a678c8f5c4c96f00d300913f6d5be1bbdf26:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 7, + "end_line": 8 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 7, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Consider possible security implications associated with loads module.", + "cve": "python/imports/imports-aliases.py:6ff02aeb3149c01ab68484d794a94f58d5d3e3bb0d58557ef4153644ea68ea54:B403", + "severity": "Low", + "confidence": "High", + "scanner": { + "id": "bandit", + "name": "Bandit" + }, + "location": { + "file": "python/imports/imports-aliases.py", + "start_line": 6, + "end_line": 6 + }, + "identifiers": [ + { + "type": "bandit_test_id", + "name": "Bandit Test ID B403", + "value": "B403" + } + ], + "priority": "Low", + "file": "python/imports/imports-aliases.py", + "line": 6, + "tool": "bandit" + }, + { + "category": "sast", + "message": "Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120)", + "cve": "c/subdir/utils.c:b466873101951fe96e1332f6728eb7010acbbd5dfc3b65d7d53571d091a06d9e:CWE-119!/CWE-120", + "confidence": "Low", + "solution": "Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "c/subdir/utils.c", + "start_line": 4 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-119", + "value": "119", + "url": "https://cwe.mitre.org/data/definitions/119.html" + }, + { + "type": "cwe", + "name": "CWE-120", + "value": "120", + "url": "https://cwe.mitre.org/data/definitions/120.html" + } + ], + "file": "c/subdir/utils.c", + "line": 4, + "url": "https://cwe.mitre.org/data/definitions/119.html", + "tool": "flawfinder" + }, + { + "category": "sast", + "message": "Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362)", + "cve": "c/subdir/utils.c:bab681140fcc8fc3085b6bba74081b44ea145c1c98b5e70cf19ace2417d30770:CWE-362", + "confidence": "Low", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "c/subdir/utils.c", + "start_line": 8 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-362", + "value": "362", + "url": "https://cwe.mitre.org/data/definitions/362.html" + } + ], + "file": "c/subdir/utils.c", + "line": 8, + "url": "https://cwe.mitre.org/data/definitions/362.html", + "tool": "flawfinder" + }, + { + "category": "sast", + "message": "Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120)", + "cve": "cplusplus/src/hello.cpp:c8c6dd0afdae6814194cf0930b719f757ab7b379cf8f261e7f4f9f2f323a818a:CWE-119!/CWE-120", + "confidence": "Low", + "solution": "Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "cplusplus/src/hello.cpp", + "start_line": 6 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-119", + "value": "119", + "url": "https://cwe.mitre.org/data/definitions/119.html" + }, + { + "type": "cwe", + "name": "CWE-120", + "value": "120", + "url": "https://cwe.mitre.org/data/definitions/120.html" + } + ], + "file": "cplusplus/src/hello.cpp", + "line": 6, + "url": "https://cwe.mitre.org/data/definitions/119.html", + "tool": "flawfinder" + }, + { + "category": "sast", + "message": "Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120)", + "cve": "cplusplus/src/hello.cpp:331c04062c4fe0c7c486f66f59e82ad146ab33cdd76ae757ca41f392d568cbd0:CWE-120", + "confidence": "Low", + "solution": "Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused)", + "scanner": { + "id": "flawfinder", + "name": "Flawfinder" + }, + "location": { + "file": "cplusplus/src/hello.cpp", + "start_line": 7 + }, + "identifiers": [ + { + "type": "cwe", + "name": "CWE-120", + "value": "120", + "url": "https://cwe.mitre.org/data/definitions/120.html" + } + ], + "file": "cplusplus/src/hello.cpp", + "line": 7, + "url": "https://cwe.mitre.org/data/definitions/120.html", + "tool": "flawfinder" + } +] diff --git a/spec/lib/gitlab/ci/build/artifacts/adapters/gzip_stream_spec.rb b/spec/lib/gitlab/ci/build/artifacts/adapters/gzip_stream_spec.rb new file mode 100644 index 00000000000..987c6b37aaa --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/adapters/gzip_stream_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Adapters::GzipStream do + describe '#initialize' do + context 'when stream is passed' do + let(:stream) { File.open(expand_fixture_path('junit/junit.xml.gz'), 'rb') } + + it 'initialized' do + expect { described_class.new(stream) }.not_to raise_error + end + end + + context 'when stream is not passed' do + let(:stream) { nil } + + it 'raises an error' do + expect { described_class.new(stream) }.to raise_error(described_class::InvalidStreamError) + end + end + end + + describe '#each_blob' do + let(:adapter) { described_class.new(stream) } + + context 'when stream is gzip file' do + context 'when gzip file contains one file' do + let(:stream) { File.open(expand_fixture_path('junit/junit.xml.gz'), 'rb') } + + it 'iterates content and file_name' do + expect { |b| adapter.each_blob(&b) } + .to yield_with_args(fixture_file('junit/junit.xml'), 'rspec.xml') + end + end + + context 'when gzip file contains three files' do + let(:stream) { File.open(expand_fixture_path('junit/junit_with_three_testsuites.xml.gz'), 'rb') } + + it 'iterates content and file_name' do + expect { |b| adapter.each_blob(&b) } + .to yield_successive_args( + [fixture_file('junit/junit_with_three_testsuites_1.xml'), 'rspec-3.xml'], + [fixture_file('junit/junit_with_three_testsuites_2.xml'), 'rspec-1.xml'], + [fixture_file('junit/junit_with_three_testsuites_3.xml'), 'rspec-2.xml']) + end + end + end + + context 'when stream is zip file' do + let(:stream) { File.open(expand_fixture_path('ci_build_artifacts.zip'), 'rb') } + + it 'raises an error' do + expect { |b| adapter.each_blob(&b) }.to raise_error(described_class::InvalidStreamError) + end + end + end +end diff --git a/spec/lib/gitlab/ci/build/artifacts/adapters/raw_stream_spec.rb b/spec/lib/gitlab/ci/build/artifacts/adapters/raw_stream_spec.rb new file mode 100644 index 00000000000..ec2dd724b45 --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/adapters/raw_stream_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Adapters::RawStream do + describe '#initialize' do + context 'when stream is passed' do + let(:stream) { File.open(expand_fixture_path('junit/junit.xml'), 'rb') } + + it 'initialized' do + expect { described_class.new(stream) }.not_to raise_error + end + end + + context 'when stream is not passed' do + let(:stream) { nil } + + it 'raises an error' do + expect { described_class.new(stream) }.to raise_error(described_class::InvalidStreamError) + end + end + end + + describe '#each_blob' do + let(:adapter) { described_class.new(stream) } + + context 'when file is not empty' do + let(:stream) { File.open(expand_fixture_path('junit/junit.xml'), 'rb') } + + it 'iterates content' do + expect { |b| adapter.each_blob(&b) } + .to yield_with_args(fixture_file('junit/junit.xml'), 'raw') + end + end + + context 'when file is empty' do + let(:stream) { Tempfile.new } + + after do + stream.unlink + end + + it 'does not iterate content' do + expect { |b| adapter.each_blob(&b) } + .not_to yield_control + end + end + end +end diff --git a/spec/lib/gitlab/ci/build/artifacts/gzip_file_adapter_spec.rb b/spec/lib/gitlab/ci/build/artifacts/gzip_file_adapter_spec.rb deleted file mode 100644 index 384329dda18..00000000000 --- a/spec/lib/gitlab/ci/build/artifacts/gzip_file_adapter_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Ci::Build::Artifacts::GzipFileAdapter do - describe '#initialize' do - context 'when stream is passed' do - let(:stream) { File.open(expand_fixture_path('junit/junit.xml.gz'), 'rb') } - - it 'initialized' do - expect { described_class.new(stream) }.not_to raise_error - end - end - - context 'when stream is not passed' do - let(:stream) { nil } - - it 'raises an error' do - expect { described_class.new(stream) }.to raise_error(described_class::InvalidStreamError) - end - end - end - - describe '#each_blob' do - let(:adapter) { described_class.new(stream) } - - context 'when stream is gzip file' do - context 'when gzip file contains one file' do - let(:stream) { File.open(expand_fixture_path('junit/junit.xml.gz'), 'rb') } - - it 'iterates content and file_name' do - expect { |b| adapter.each_blob(&b) } - .to yield_with_args(fixture_file('junit/junit.xml'), 'rspec.xml') - end - end - - context 'when gzip file contains three files' do - let(:stream) { File.open(expand_fixture_path('junit/junit_with_three_testsuites.xml.gz'), 'rb') } - - it 'iterates content and file_name' do - expect { |b| adapter.each_blob(&b) } - .to yield_successive_args( - [fixture_file('junit/junit_with_three_testsuites_1.xml'), 'rspec-3.xml'], - [fixture_file('junit/junit_with_three_testsuites_2.xml'), 'rspec-1.xml'], - [fixture_file('junit/junit_with_three_testsuites_3.xml'), 'rspec-2.xml']) - end - end - end - - context 'when stream is zip file' do - let(:stream) { File.open(expand_fixture_path('ci_build_artifacts.zip'), 'rb') } - - it 'raises an error' do - expect { |b| adapter.each_blob(&b) }.to raise_error(described_class::InvalidStreamError) - end - end - end -end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 85fad77a242..fb5bec4108a 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -194,6 +194,14 @@ describe Ci::JobArtifact do end end + context 'when file format is raw' do + let(:artifact) { build(:ci_job_artifact, :codequality, file_format: :raw) } + + it 'iterates blob once' do + expect { |b| artifact.each_blob(&b) }.to yield_control.once + end + end + context 'when there are no adapters for the file format' do let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) } diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb index a42d1f3d399..170e0ac5717 100644 --- a/spec/presenters/ci/build_runner_presenter_spec.rb +++ b/spec/presenters/ci/build_runner_presenter_spec.rb @@ -40,21 +40,23 @@ describe Ci::BuildRunnerPresenter do context "with reports" do Ci::JobArtifact::DEFAULT_FILE_NAMES.each do |file_type, filename| - let(:report) { { "#{file_type}": [filename] } } - let(:build) { create(:ci_build, options: { artifacts: { reports: report } } ) } - - let(:report_expectation) do - { - name: filename, - artifact_type: :"#{file_type}", - artifact_format: :gzip, - paths: [filename], - when: 'always' - } - end - - it 'presents correct hash' do - expect(presenter.artifacts.first).to include(report_expectation) + context file_type.to_s do + let(:report) { { "#{file_type}": [filename] } } + let(:build) { create(:ci_build, options: { artifacts: { reports: report } } ) } + + let(:report_expectation) do + { + name: filename, + artifact_type: :"#{file_type}", + artifact_format: Ci::JobArtifact::TYPE_AND_FORMAT_PAIRS.fetch(file_type), + paths: [filename], + when: 'always' + } + end + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(report_expectation) + end end end end -- cgit v1.2.1 From 7acc6340c1e8191f1df0cbc50f4cc4f2291e698b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 16 Oct 2018 15:04:32 +0200 Subject: Move external CI config files into Ci class context --- lib/gitlab/ci/config/external/file/base.rb | 29 ++++ lib/gitlab/ci/config/external/file/local.rb | 34 ++++ lib/gitlab/ci/config/external/file/remote.rb | 30 ++++ lib/gitlab/ci/config/external/mapper.rb | 32 ++++ lib/gitlab/ci/config/external/processor.rb | 52 ++++++ lib/gitlab/ci/external/file/base.rb | 29 ---- lib/gitlab/ci/external/file/local.rb | 34 ---- lib/gitlab/ci/external/file/remote.rb | 30 ---- lib/gitlab/ci/external/mapper.rb | 32 ---- lib/gitlab/ci/external/processor.rb | 52 ------ .../gitlab/ci/config/external/file/local_spec.rb | 78 +++++++++ .../gitlab/ci/config/external/file/remote_spec.rb | 114 +++++++++++++ spec/lib/gitlab/ci/config/external/mapper_spec.rb | 96 +++++++++++ .../gitlab/ci/config/external/processor_spec.rb | 182 +++++++++++++++++++++ spec/lib/gitlab/ci/external/file/local_spec.rb | 78 --------- spec/lib/gitlab/ci/external/file/remote_spec.rb | 114 ------------- spec/lib/gitlab/ci/external/mapper_spec.rb | 96 ----------- spec/lib/gitlab/ci/external/processor_spec.rb | 182 --------------------- 18 files changed, 647 insertions(+), 647 deletions(-) create mode 100644 lib/gitlab/ci/config/external/file/base.rb create mode 100644 lib/gitlab/ci/config/external/file/local.rb create mode 100644 lib/gitlab/ci/config/external/file/remote.rb create mode 100644 lib/gitlab/ci/config/external/mapper.rb create mode 100644 lib/gitlab/ci/config/external/processor.rb delete mode 100644 lib/gitlab/ci/external/file/base.rb delete mode 100644 lib/gitlab/ci/external/file/local.rb delete mode 100644 lib/gitlab/ci/external/file/remote.rb delete mode 100644 lib/gitlab/ci/external/mapper.rb delete mode 100644 lib/gitlab/ci/external/processor.rb create mode 100644 spec/lib/gitlab/ci/config/external/file/local_spec.rb create mode 100644 spec/lib/gitlab/ci/config/external/file/remote_spec.rb create mode 100644 spec/lib/gitlab/ci/config/external/mapper_spec.rb create mode 100644 spec/lib/gitlab/ci/config/external/processor_spec.rb delete mode 100644 spec/lib/gitlab/ci/external/file/local_spec.rb delete mode 100644 spec/lib/gitlab/ci/external/file/remote_spec.rb delete mode 100644 spec/lib/gitlab/ci/external/mapper_spec.rb delete mode 100644 spec/lib/gitlab/ci/external/processor_spec.rb diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb new file mode 100644 index 00000000000..f4da07b0b02 --- /dev/null +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module External + module File + class Base + YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze + + def initialize(location, opts = {}) + @location = location + end + + def valid? + location.match(YAML_WHITELIST_EXTENSION) && content + end + + def content + raise NotImplementedError, 'content must be implemented and return a string or nil' + end + + def error_message + raise NotImplementedError, 'error_message must be implemented and return a string' + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb new file mode 100644 index 00000000000..1aa7f687507 --- /dev/null +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module External + module File + class Local < Base + attr_reader :location, :project, :sha + + def initialize(location, opts = {}) + super + + @project = opts.fetch(:project) + @sha = opts.fetch(:sha) + end + + def content + @content ||= fetch_local_content + end + + def error_message + "Local file '#{location}' is not valid." + end + + private + + def fetch_local_content + project.repository.blob_data_at(sha, location) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb new file mode 100644 index 00000000000..59bb3e8999e --- /dev/null +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module External + module File + class Remote < Base + include Gitlab::Utils::StrongMemoize + attr_reader :location + + def content + return @content if defined?(@content) + + @content = strong_memoize(:content) do + begin + Gitlab::HTTP.get(location) + rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError + nil + end + end + end + + def error_message + "Remote file '#{location}' is not valid." + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb new file mode 100644 index 00000000000..58bd6a19acf --- /dev/null +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module External + class Mapper + def initialize(values, project, sha) + @locations = Array(values.fetch(:include, [])) + @project = project + @sha = sha + end + + def process + locations.map { |location| build_external_file(location) } + end + + private + + attr_reader :locations, :project, :sha + + def build_external_file(location) + if ::Gitlab::UrlSanitizer.valid?(location) + Gitlab::Ci::External::File::Remote.new(location) + else + options = { project: project, sha: sha } + Gitlab::Ci::External::File::Local.new(location, options) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb new file mode 100644 index 00000000000..76cf3ce89f9 --- /dev/null +++ b/lib/gitlab/ci/config/external/processor.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module External + class Processor + FileError = Class.new(StandardError) + + def initialize(values, project, sha) + @values = values + @external_files = Gitlab::Ci::External::Mapper.new(values, project, sha).process + @content = {} + end + + def perform + return values if external_files.empty? + + external_files.each do |external_file| + validate_external_file(external_file) + @content.deep_merge!(content_of(external_file)) + end + + append_inline_content + remove_include_keyword + end + + private + + attr_reader :values, :external_files, :content + + def validate_external_file(external_file) + unless external_file.valid? + raise FileError, external_file.error_message + end + end + + def content_of(external_file) + Gitlab::Ci::Config::Loader.new(external_file.content).load! + end + + def append_inline_content + @content.deep_merge!(@values) + end + + def remove_include_keyword + content.delete(:include) + content + end + end + end + end +end diff --git a/lib/gitlab/ci/external/file/base.rb b/lib/gitlab/ci/external/file/base.rb deleted file mode 100644 index f4da07b0b02..00000000000 --- a/lib/gitlab/ci/external/file/base.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - module File - class Base - YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze - - def initialize(location, opts = {}) - @location = location - end - - def valid? - location.match(YAML_WHITELIST_EXTENSION) && content - end - - def content - raise NotImplementedError, 'content must be implemented and return a string or nil' - end - - def error_message - raise NotImplementedError, 'error_message must be implemented and return a string' - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/file/local.rb b/lib/gitlab/ci/external/file/local.rb deleted file mode 100644 index 1aa7f687507..00000000000 --- a/lib/gitlab/ci/external/file/local.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - module File - class Local < Base - attr_reader :location, :project, :sha - - def initialize(location, opts = {}) - super - - @project = opts.fetch(:project) - @sha = opts.fetch(:sha) - end - - def content - @content ||= fetch_local_content - end - - def error_message - "Local file '#{location}' is not valid." - end - - private - - def fetch_local_content - project.repository.blob_data_at(sha, location) - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/file/remote.rb b/lib/gitlab/ci/external/file/remote.rb deleted file mode 100644 index 59bb3e8999e..00000000000 --- a/lib/gitlab/ci/external/file/remote.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - module File - class Remote < Base - include Gitlab::Utils::StrongMemoize - attr_reader :location - - def content - return @content if defined?(@content) - - @content = strong_memoize(:content) do - begin - Gitlab::HTTP.get(location) - rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError - nil - end - end - end - - def error_message - "Remote file '#{location}' is not valid." - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/mapper.rb b/lib/gitlab/ci/external/mapper.rb deleted file mode 100644 index 58bd6a19acf..00000000000 --- a/lib/gitlab/ci/external/mapper.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - class Mapper - def initialize(values, project, sha) - @locations = Array(values.fetch(:include, [])) - @project = project - @sha = sha - end - - def process - locations.map { |location| build_external_file(location) } - end - - private - - attr_reader :locations, :project, :sha - - def build_external_file(location) - if ::Gitlab::UrlSanitizer.valid?(location) - Gitlab::Ci::External::File::Remote.new(location) - else - options = { project: project, sha: sha } - Gitlab::Ci::External::File::Local.new(location, options) - end - end - end - end - end -end diff --git a/lib/gitlab/ci/external/processor.rb b/lib/gitlab/ci/external/processor.rb deleted file mode 100644 index 76cf3ce89f9..00000000000 --- a/lib/gitlab/ci/external/processor.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Ci - module External - class Processor - FileError = Class.new(StandardError) - - def initialize(values, project, sha) - @values = values - @external_files = Gitlab::Ci::External::Mapper.new(values, project, sha).process - @content = {} - end - - def perform - return values if external_files.empty? - - external_files.each do |external_file| - validate_external_file(external_file) - @content.deep_merge!(content_of(external_file)) - end - - append_inline_content - remove_include_keyword - end - - private - - attr_reader :values, :external_files, :content - - def validate_external_file(external_file) - unless external_file.valid? - raise FileError, external_file.error_message - end - end - - def content_of(external_file) - Gitlab::Ci::Config::Loader.new(external_file.content).load! - end - - def append_inline_content - @content.deep_merge!(@values) - end - - def remove_include_keyword - content.delete(:include) - content - end - end - end - end -end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb new file mode 100644 index 00000000000..73bb4ccf468 --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::External::File::Local do + let(:project) { create(:project, :repository) } + let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } + + describe '#valid?' do + context 'when is a valid local path' do + let(:location) { '/lib/gitlab/ci/templates/existent-file.yml' } + + before do + allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return("image: 'ruby2:2'") + end + + it 'should return true' do + expect(local_file.valid?).to be_truthy + end + end + + context 'when is not a valid local path' do + let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } + + it 'should return false' do + expect(local_file.valid?).to be_falsy + end + end + + context 'when is not a yaml file' do + let(:location) { '/config/application.rb' } + + it 'should return false' do + expect(local_file.valid?).to be_falsy + end + end + end + + describe '#content' do + context 'with a a valid file' do + let(:local_file_content) do + <<~HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + end + let(:location) { '/lib/gitlab/ci/templates/existent-file.yml' } + + before do + allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return(local_file_content) + end + + it 'should return the content of the file' do + expect(local_file.content).to eq(local_file_content) + end + end + + context 'with an invalid file' do + let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } + + it 'should be nil' do + expect(local_file.content).to be_nil + end + end + end + + describe '#error_message' do + let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } + + it 'should return an error message' do + expect(local_file.error_message).to eq("Local file '#{location}' is not valid.") + end + end +end diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb new file mode 100644 index 00000000000..b1819c8960b --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::External::File::Remote do + let(:remote_file) { described_class.new(location) } + let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:remote_file_content) do + <<~HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + end + + describe "#valid?" do + context 'when is a valid remote url' do + before do + WebMock.stub_request(:get, location).to_return(body: remote_file_content) + end + + it 'should return true' do + expect(remote_file.valid?).to be_truthy + end + end + + context 'with an irregular url' do + let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it 'should return false' do + expect(remote_file.valid?).to be_falsy + end + end + + context 'with a timeout' do + before do + allow(Gitlab::HTTP).to receive(:get).and_raise(Timeout::Error) + end + + it 'should be falsy' do + expect(remote_file.valid?).to be_falsy + end + end + + context 'when is not a yaml file' do + let(:location) { 'https://asdasdasdaj48ggerexample.com' } + + it 'should be falsy' do + expect(remote_file.valid?).to be_falsy + end + end + + context 'with an internal url' do + let(:location) { 'http://localhost:8080' } + + it 'should be falsy' do + expect(remote_file.valid?).to be_falsy + end + end + end + + describe "#content" do + context 'with a valid remote file' do + before do + WebMock.stub_request(:get, location).to_return(body: remote_file_content) + end + + it 'should return the content of the file' do + expect(remote_file.content).to eql(remote_file_content) + end + end + + context 'with a timeout' do + before do + allow(Gitlab::HTTP).to receive(:get).and_raise(Timeout::Error) + end + + it 'should be falsy' do + expect(remote_file.content).to be_falsy + end + end + + context 'with an invalid remote url' do + let(:location) { 'https://asdasdasdaj48ggerexample.com' } + + before do + WebMock.stub_request(:get, location).to_raise(SocketError.new('Some HTTP error')) + end + + it 'should be nil' do + expect(remote_file.content).to be_nil + end + end + + context 'with an internal url' do + let(:location) { 'http://localhost:8080' } + + it 'should be nil' do + expect(remote_file.content).to be_nil + end + end + end + + describe "#error_message" do + let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + + it 'should return an error message' do + expect(remote_file.error_message).to eq("Remote file '#{location}' is not valid.") + end + end +end diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb new file mode 100644 index 00000000000..d925d6af73d --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::External::Mapper do + let(:project) { create(:project, :repository) } + let(:file_content) do + <<~HEREDOC + image: 'ruby:2.2' + HEREDOC + end + + describe '#process' do + subject { described_class.new(values, project, '123456').process } + + context "when 'include' keyword is defined as string" do + context 'when the string is a local file' do + let(:values) do + { + include: '/lib/gitlab/ci/templates/non-existent-file.yml', + image: 'ruby:2.2' + } + end + + it 'returns an array' do + expect(subject).to be_an(Array) + end + + it 'returns File instances' do + expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Local) + end + end + + context 'when the string is a remote file' do + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:values) do + { + include: remote_url, + image: 'ruby:2.2' + } + end + + before do + WebMock.stub_request(:get, remote_url).to_return(body: file_content) + end + + it 'returns an array' do + expect(subject).to be_an(Array) + end + + it 'returns File instances' do + expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Remote) + end + end + end + + context "when 'include' is defined as an array" do + let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:values) do + { + include: + [ + remote_url, + '/lib/gitlab/ci/templates/template.yml' + ], + image: 'ruby:2.2' + } + end + + before do + WebMock.stub_request(:get, remote_url).to_return(body: file_content) + end + + it 'returns an array' do + expect(subject).to be_an(Array) + end + + it 'returns Files instances' do + expect(subject).to all(respond_to(:valid?)) + expect(subject).to all(respond_to(:content)) + end + end + + context "when 'include' is not defined" do + let(:values) do + { + image: 'ruby:2.2' + } + end + + it 'returns an empty array' do + expect(subject).to be_empty + end + end + end +end diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb new file mode 100644 index 00000000000..3c7394f53d2 --- /dev/null +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -0,0 +1,182 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Ci::External::Processor do + let(:project) { create(:project, :repository) } + let(:processor) { described_class.new(values, project, '12345') } + + describe "#perform" do + context 'when no external files defined' do + let(:values) { { image: 'ruby:2.2' } } + + it 'should return the same values' do + expect(processor.perform).to eq(values) + end + end + + context 'when an invalid local file is defined' do + let(:values) { { include: '/lib/gitlab/ci/templates/non-existent-file.yml', image: 'ruby:2.2' } } + + it 'should raise an error' do + expect { processor.perform }.to raise_error( + described_class::FileError, + "Local file '/lib/gitlab/ci/templates/non-existent-file.yml' is not valid." + ) + end + end + + context 'when an invalid remote file is defined' do + let(:remote_file) { 'http://doesntexist.com/.gitlab-ci-1.yml' } + let(:values) { { include: remote_file, image: 'ruby:2.2' } } + + before do + WebMock.stub_request(:get, remote_file).to_raise(SocketError.new('Some HTTP error')) + end + + it 'should raise an error' do + expect { processor.perform }.to raise_error( + described_class::FileError, + "Remote file '#{remote_file}' is not valid." + ) + end + end + + context 'with a valid remote external file is defined' do + let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:values) { { include: remote_file, image: 'ruby:2.2' } } + let(:external_file_content) do + <<-HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + + rspec: + script: + - bundle exec rspec + + rubocop: + script: + - bundle exec rubocop + HEREDOC + end + + before do + WebMock.stub_request(:get, remote_file).to_return(body: external_file_content) + end + + it 'should append the file to the values' do + output = processor.perform + expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop]) + end + + it "should remove the 'include' keyword" do + expect(processor.perform[:include]).to be_nil + end + end + + context 'with a valid local external file is defined' do + let(:values) { { include: '/lib/gitlab/ci/templates/template.yml', image: 'ruby:2.2' } } + let(:local_file_content) do + <<-HEREDOC + before_script: + - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs + - ruby -v + - which ruby + - gem install bundler --no-ri --no-rdoc + - bundle install --jobs $(nproc) "${FLAGS[@]}" + HEREDOC + end + + before do + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + end + + it 'should append the file to the values' do + output = processor.perform + expect(output.keys).to match_array([:image, :before_script]) + end + + it "should remove the 'include' keyword" do + expect(processor.perform[:include]).to be_nil + end + end + + context 'with multiple external files are defined' do + let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:external_files) do + [ + '/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml', + remote_file + ] + end + let(:values) do + { + include: external_files, + image: 'ruby:2.2' + } + end + + let(:remote_file_content) do + <<-HEREDOC + stages: + - build + - review + - cleanup + HEREDOC + end + + before do + local_file_content = File.read(Rails.root.join('spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml')) + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) + end + + it 'should append the files to the values' do + expect(processor.perform.keys).to match_array([:image, :stages, :before_script, :rspec]) + end + + it "should remove the 'include' keyword" do + expect(processor.perform[:include]).to be_nil + end + end + + context 'when external files are defined but not valid' do + let(:values) { { include: '/lib/gitlab/ci/templates/template.yml', image: 'ruby:2.2' } } + + let(:local_file_content) { 'invalid content file ////' } + + before do + allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + end + + it 'should raise an error' do + expect { processor.perform }.to raise_error(Gitlab::Ci::Config::Loader::FormatError) + end + end + + context "when both external files and values defined the same key" do + let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } + let(:values) do + { + include: remote_file, + image: 'ruby:2.2' + } + end + + let(:remote_file_content) do + <<~HEREDOC + image: php:5-fpm-alpine + HEREDOC + end + + it 'should take precedence' do + WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) + expect(processor.perform[:image]).to eq('ruby:2.2') + end + end + end +end diff --git a/spec/lib/gitlab/ci/external/file/local_spec.rb b/spec/lib/gitlab/ci/external/file/local_spec.rb deleted file mode 100644 index 73bb4ccf468..00000000000 --- a/spec/lib/gitlab/ci/external/file/local_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Ci::External::File::Local do - let(:project) { create(:project, :repository) } - let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } - - describe '#valid?' do - context 'when is a valid local path' do - let(:location) { '/lib/gitlab/ci/templates/existent-file.yml' } - - before do - allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return("image: 'ruby2:2'") - end - - it 'should return true' do - expect(local_file.valid?).to be_truthy - end - end - - context 'when is not a valid local path' do - let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } - - it 'should return false' do - expect(local_file.valid?).to be_falsy - end - end - - context 'when is not a yaml file' do - let(:location) { '/config/application.rb' } - - it 'should return false' do - expect(local_file.valid?).to be_falsy - end - end - end - - describe '#content' do - context 'with a a valid file' do - let(:local_file_content) do - <<~HEREDOC - before_script: - - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - ruby -v - - which ruby - - gem install bundler --no-ri --no-rdoc - - bundle install --jobs $(nproc) "${FLAGS[@]}" - HEREDOC - end - let(:location) { '/lib/gitlab/ci/templates/existent-file.yml' } - - before do - allow_any_instance_of(described_class).to receive(:fetch_local_content).and_return(local_file_content) - end - - it 'should return the content of the file' do - expect(local_file.content).to eq(local_file_content) - end - end - - context 'with an invalid file' do - let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } - - it 'should be nil' do - expect(local_file.content).to be_nil - end - end - end - - describe '#error_message' do - let(:location) { '/lib/gitlab/ci/templates/non-existent-file.yml' } - - it 'should return an error message' do - expect(local_file.error_message).to eq("Local file '#{location}' is not valid.") - end - end -end diff --git a/spec/lib/gitlab/ci/external/file/remote_spec.rb b/spec/lib/gitlab/ci/external/file/remote_spec.rb deleted file mode 100644 index b1819c8960b..00000000000 --- a/spec/lib/gitlab/ci/external/file/remote_spec.rb +++ /dev/null @@ -1,114 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Ci::External::File::Remote do - let(:remote_file) { described_class.new(location) } - let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:remote_file_content) do - <<~HEREDOC - before_script: - - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - ruby -v - - which ruby - - gem install bundler --no-ri --no-rdoc - - bundle install --jobs $(nproc) "${FLAGS[@]}" - HEREDOC - end - - describe "#valid?" do - context 'when is a valid remote url' do - before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content) - end - - it 'should return true' do - expect(remote_file.valid?).to be_truthy - end - end - - context 'with an irregular url' do - let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - it 'should return false' do - expect(remote_file.valid?).to be_falsy - end - end - - context 'with a timeout' do - before do - allow(Gitlab::HTTP).to receive(:get).and_raise(Timeout::Error) - end - - it 'should be falsy' do - expect(remote_file.valid?).to be_falsy - end - end - - context 'when is not a yaml file' do - let(:location) { 'https://asdasdasdaj48ggerexample.com' } - - it 'should be falsy' do - expect(remote_file.valid?).to be_falsy - end - end - - context 'with an internal url' do - let(:location) { 'http://localhost:8080' } - - it 'should be falsy' do - expect(remote_file.valid?).to be_falsy - end - end - end - - describe "#content" do - context 'with a valid remote file' do - before do - WebMock.stub_request(:get, location).to_return(body: remote_file_content) - end - - it 'should return the content of the file' do - expect(remote_file.content).to eql(remote_file_content) - end - end - - context 'with a timeout' do - before do - allow(Gitlab::HTTP).to receive(:get).and_raise(Timeout::Error) - end - - it 'should be falsy' do - expect(remote_file.content).to be_falsy - end - end - - context 'with an invalid remote url' do - let(:location) { 'https://asdasdasdaj48ggerexample.com' } - - before do - WebMock.stub_request(:get, location).to_raise(SocketError.new('Some HTTP error')) - end - - it 'should be nil' do - expect(remote_file.content).to be_nil - end - end - - context 'with an internal url' do - let(:location) { 'http://localhost:8080' } - - it 'should be nil' do - expect(remote_file.content).to be_nil - end - end - end - - describe "#error_message" do - let(:location) { 'not-valid://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - - it 'should return an error message' do - expect(remote_file.error_message).to eq("Remote file '#{location}' is not valid.") - end - end -end diff --git a/spec/lib/gitlab/ci/external/mapper_spec.rb b/spec/lib/gitlab/ci/external/mapper_spec.rb deleted file mode 100644 index d925d6af73d..00000000000 --- a/spec/lib/gitlab/ci/external/mapper_spec.rb +++ /dev/null @@ -1,96 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Ci::External::Mapper do - let(:project) { create(:project, :repository) } - let(:file_content) do - <<~HEREDOC - image: 'ruby:2.2' - HEREDOC - end - - describe '#process' do - subject { described_class.new(values, project, '123456').process } - - context "when 'include' keyword is defined as string" do - context 'when the string is a local file' do - let(:values) do - { - include: '/lib/gitlab/ci/templates/non-existent-file.yml', - image: 'ruby:2.2' - } - end - - it 'returns an array' do - expect(subject).to be_an(Array) - end - - it 'returns File instances' do - expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Local) - end - end - - context 'when the string is a remote file' do - let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:values) do - { - include: remote_url, - image: 'ruby:2.2' - } - end - - before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) - end - - it 'returns an array' do - expect(subject).to be_an(Array) - end - - it 'returns File instances' do - expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Remote) - end - end - end - - context "when 'include' is defined as an array" do - let(:remote_url) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:values) do - { - include: - [ - remote_url, - '/lib/gitlab/ci/templates/template.yml' - ], - image: 'ruby:2.2' - } - end - - before do - WebMock.stub_request(:get, remote_url).to_return(body: file_content) - end - - it 'returns an array' do - expect(subject).to be_an(Array) - end - - it 'returns Files instances' do - expect(subject).to all(respond_to(:valid?)) - expect(subject).to all(respond_to(:content)) - end - end - - context "when 'include' is not defined" do - let(:values) do - { - image: 'ruby:2.2' - } - end - - it 'returns an empty array' do - expect(subject).to be_empty - end - end - end -end diff --git a/spec/lib/gitlab/ci/external/processor_spec.rb b/spec/lib/gitlab/ci/external/processor_spec.rb deleted file mode 100644 index 3c7394f53d2..00000000000 --- a/spec/lib/gitlab/ci/external/processor_spec.rb +++ /dev/null @@ -1,182 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Ci::External::Processor do - let(:project) { create(:project, :repository) } - let(:processor) { described_class.new(values, project, '12345') } - - describe "#perform" do - context 'when no external files defined' do - let(:values) { { image: 'ruby:2.2' } } - - it 'should return the same values' do - expect(processor.perform).to eq(values) - end - end - - context 'when an invalid local file is defined' do - let(:values) { { include: '/lib/gitlab/ci/templates/non-existent-file.yml', image: 'ruby:2.2' } } - - it 'should raise an error' do - expect { processor.perform }.to raise_error( - described_class::FileError, - "Local file '/lib/gitlab/ci/templates/non-existent-file.yml' is not valid." - ) - end - end - - context 'when an invalid remote file is defined' do - let(:remote_file) { 'http://doesntexist.com/.gitlab-ci-1.yml' } - let(:values) { { include: remote_file, image: 'ruby:2.2' } } - - before do - WebMock.stub_request(:get, remote_file).to_raise(SocketError.new('Some HTTP error')) - end - - it 'should raise an error' do - expect { processor.perform }.to raise_error( - described_class::FileError, - "Remote file '#{remote_file}' is not valid." - ) - end - end - - context 'with a valid remote external file is defined' do - let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:values) { { include: remote_file, image: 'ruby:2.2' } } - let(:external_file_content) do - <<-HEREDOC - before_script: - - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - ruby -v - - which ruby - - gem install bundler --no-ri --no-rdoc - - bundle install --jobs $(nproc) "${FLAGS[@]}" - - rspec: - script: - - bundle exec rspec - - rubocop: - script: - - bundle exec rubocop - HEREDOC - end - - before do - WebMock.stub_request(:get, remote_file).to_return(body: external_file_content) - end - - it 'should append the file to the values' do - output = processor.perform - expect(output.keys).to match_array([:image, :before_script, :rspec, :rubocop]) - end - - it "should remove the 'include' keyword" do - expect(processor.perform[:include]).to be_nil - end - end - - context 'with a valid local external file is defined' do - let(:values) { { include: '/lib/gitlab/ci/templates/template.yml', image: 'ruby:2.2' } } - let(:local_file_content) do - <<-HEREDOC - before_script: - - apt-get update -qq && apt-get install -y -qq sqlite3 libsqlite3-dev nodejs - - ruby -v - - which ruby - - gem install bundler --no-ri --no-rdoc - - bundle install --jobs $(nproc) "${FLAGS[@]}" - HEREDOC - end - - before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) - end - - it 'should append the file to the values' do - output = processor.perform - expect(output.keys).to match_array([:image, :before_script]) - end - - it "should remove the 'include' keyword" do - expect(processor.perform[:include]).to be_nil - end - end - - context 'with multiple external files are defined' do - let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:external_files) do - [ - '/spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml', - remote_file - ] - end - let(:values) do - { - include: external_files, - image: 'ruby:2.2' - } - end - - let(:remote_file_content) do - <<-HEREDOC - stages: - - build - - review - - cleanup - HEREDOC - end - - before do - local_file_content = File.read(Rails.root.join('spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml')) - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) - WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) - end - - it 'should append the files to the values' do - expect(processor.perform.keys).to match_array([:image, :stages, :before_script, :rspec]) - end - - it "should remove the 'include' keyword" do - expect(processor.perform[:include]).to be_nil - end - end - - context 'when external files are defined but not valid' do - let(:values) { { include: '/lib/gitlab/ci/templates/template.yml', image: 'ruby:2.2' } } - - let(:local_file_content) { 'invalid content file ////' } - - before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) - end - - it 'should raise an error' do - expect { processor.perform }.to raise_error(Gitlab::Ci::Config::Loader::FormatError) - end - end - - context "when both external files and values defined the same key" do - let(:remote_file) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } - let(:values) do - { - include: remote_file, - image: 'ruby:2.2' - } - end - - let(:remote_file_content) do - <<~HEREDOC - image: php:5-fpm-alpine - HEREDOC - end - - it 'should take precedence' do - WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) - expect(processor.perform[:image]).to eq('ruby:2.2') - end - end - end -end -- cgit v1.2.1 From 5f502c3a82827ebd76e9585e5caa015753016c6a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 16 Oct 2018 15:09:36 +0200 Subject: Move external CI config class into proper namespace --- lib/gitlab/ci/config.rb | 4 +- lib/gitlab/ci/config/external/file/base.rb | 32 +++++----- lib/gitlab/ci/config/external/file/local.rb | 38 ++++++------ lib/gitlab/ci/config/external/file/remote.rb | 32 +++++----- lib/gitlab/ci/config/external/mapper.rb | 37 ++++++------ lib/gitlab/ci/config/external/processor.rb | 68 +++++++++++----------- .../gitlab/ci/config/external/file/local_spec.rb | 2 +- .../gitlab/ci/config/external/file/remote_spec.rb | 2 +- spec/lib/gitlab/ci/config/external/mapper_spec.rb | 8 ++- .../gitlab/ci/config/external/processor_spec.rb | 13 +++-- spec/lib/gitlab/ci/config_spec.rb | 4 +- 11 files changed, 127 insertions(+), 113 deletions(-) diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index fe98d25af29..7b3c5c12c61 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -15,7 +15,7 @@ module Gitlab @global.compose! rescue Loader::FormatError, Extendable::ExtensionError => e raise Config::ConfigError, e.message - rescue ::Gitlab::Ci::External::Processor::FileError => e + rescue External::Processor::FileError => e raise ::Gitlab::Ci::YamlProcessor::ValidationError, e.message end @@ -81,7 +81,7 @@ module Gitlab def process_external_files(config, project, opts) sha = opts.fetch(:sha) { project.repository.root_ref_sha } - ::Gitlab::Ci::External::Processor.new(config, project, sha).perform + Config::External::Processor.new(config, project, sha).perform end end end diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index f4da07b0b02..dc96c97b129 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -2,25 +2,27 @@ module Gitlab module Ci - module External - module File - class Base - YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze + class Config + module External + module File + class Base + YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze - def initialize(location, opts = {}) - @location = location - end + def initialize(location, opts = {}) + @location = location + end - def valid? - location.match(YAML_WHITELIST_EXTENSION) && content - end + def valid? + location.match(YAML_WHITELIST_EXTENSION) && content + end - def content - raise NotImplementedError, 'content must be implemented and return a string or nil' - end + def content + raise NotImplementedError, 'content must be implemented and return a string or nil' + end - def error_message - raise NotImplementedError, 'error_message must be implemented and return a string' + def error_message + raise NotImplementedError, 'error_message must be implemented and return a string' + end end end end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index 1aa7f687507..8c0ffd36449 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -2,30 +2,32 @@ module Gitlab module Ci - module External - module File - class Local < Base - attr_reader :location, :project, :sha + class Config + module External + module File + class Local < Base + attr_reader :location, :project, :sha - def initialize(location, opts = {}) - super + def initialize(location, opts = {}) + super - @project = opts.fetch(:project) - @sha = opts.fetch(:sha) - end + @project = opts.fetch(:project) + @sha = opts.fetch(:sha) + end - def content - @content ||= fetch_local_content - end + def content + @content ||= fetch_local_content + end - def error_message - "Local file '#{location}' is not valid." - end + def error_message + "Local file '#{location}' is not valid." + end - private + private - def fetch_local_content - project.repository.blob_data_at(sha, location) + def fetch_local_content + project.repository.blob_data_at(sha, location) + end end end end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb index 59bb3e8999e..a19c7639b5a 100644 --- a/lib/gitlab/ci/config/external/file/remote.rb +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -2,26 +2,28 @@ module Gitlab module Ci - module External - module File - class Remote < Base - include Gitlab::Utils::StrongMemoize - attr_reader :location + class Config + module External + module File + class Remote < Base + include Gitlab::Utils::StrongMemoize + attr_reader :location - def content - return @content if defined?(@content) + def content + return @content if defined?(@content) - @content = strong_memoize(:content) do - begin - Gitlab::HTTP.get(location) - rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError - nil + @content = strong_memoize(:content) do + begin + Gitlab::HTTP.get(location) + rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError + nil + end end end - end - def error_message - "Remote file '#{location}' is not valid." + def error_message + "Remote file '#{location}' is not valid." + end end end end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index 58bd6a19acf..def3563e505 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -2,28 +2,29 @@ module Gitlab module Ci - module External - class Mapper - def initialize(values, project, sha) - @locations = Array(values.fetch(:include, [])) - @project = project - @sha = sha - end + class Config + module External + class Mapper + def initialize(values, project, sha) + @locations = Array(values.fetch(:include, [])) + @project = project + @sha = sha + end - def process - locations.map { |location| build_external_file(location) } - end + def process + locations.map { |location| build_external_file(location) } + end - private + private - attr_reader :locations, :project, :sha + attr_reader :locations, :project, :sha - def build_external_file(location) - if ::Gitlab::UrlSanitizer.valid?(location) - Gitlab::Ci::External::File::Remote.new(location) - else - options = { project: project, sha: sha } - Gitlab::Ci::External::File::Local.new(location, options) + def build_external_file(location) + if ::Gitlab::UrlSanitizer.valid?(location) + External::File::Remote.new(location) + else + External::File::Local.new(location, project: project, sha: sha) + end end end end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb index 76cf3ce89f9..f3b20085cd6 100644 --- a/lib/gitlab/ci/config/external/processor.rb +++ b/lib/gitlab/ci/config/external/processor.rb @@ -2,49 +2,51 @@ module Gitlab module Ci - module External - class Processor - FileError = Class.new(StandardError) - - def initialize(values, project, sha) - @values = values - @external_files = Gitlab::Ci::External::Mapper.new(values, project, sha).process - @content = {} - end + class Config + module External + class Processor + FileError = Class.new(StandardError) + + def initialize(values, project, sha) + @values = values + @external_files = External::Mapper.new(values, project, sha).process + @content = {} + end - def perform - return values if external_files.empty? + def perform + return values if external_files.empty? - external_files.each do |external_file| - validate_external_file(external_file) - @content.deep_merge!(content_of(external_file)) - end + external_files.each do |external_file| + validate_external_file(external_file) + @content.deep_merge!(content_of(external_file)) + end - append_inline_content - remove_include_keyword - end + append_inline_content + remove_include_keyword + end - private + private - attr_reader :values, :external_files, :content + attr_reader :values, :external_files, :content - def validate_external_file(external_file) - unless external_file.valid? - raise FileError, external_file.error_message + def validate_external_file(external_file) + unless external_file.valid? + raise FileError, external_file.error_message + end end - end - def content_of(external_file) - Gitlab::Ci::Config::Loader.new(external_file.content).load! - end + def content_of(external_file) + Config::Loader.new(external_file.content).load! + end - def append_inline_content - @content.deep_merge!(@values) - end + def append_inline_content + @content.deep_merge!(@values) + end - def remove_include_keyword - content.delete(:include) - content + def remove_include_keyword + content.delete(:include) + content + end end end end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 73bb4ccf468..5c0e2eee71f 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::File::Local do +describe Gitlab::Ci::Config::External::File::Local do let(:project) { create(:project, :repository) } let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index b1819c8960b..72853797bcd 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::File::Remote do +describe Gitlab::Ci::Config::External::File::Remote do let(:remote_file) { described_class.new(location) } let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:remote_file_content) do diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index d925d6af73d..5b236fe99f1 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::Mapper do +describe Gitlab::Ci::Config::External::Mapper do let(:project) { create(:project, :repository) } let(:file_content) do <<~HEREDOC @@ -27,7 +27,8 @@ describe Gitlab::Ci::External::Mapper do end it 'returns File instances' do - expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Local) + expect(subject.first) + .to be_an_instance_of(Gitlab::Ci::Config::External::File::Local) end end @@ -49,7 +50,8 @@ describe Gitlab::Ci::External::Mapper do end it 'returns File instances' do - expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Remote) + expect(subject.first) + .to be_an_instance_of(Gitlab::Ci::Config::External::File::Remote) end end end diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 3c7394f53d2..0196e11628d 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::Processor do +describe Gitlab::Ci::Config::External::Processor do let(:project) { create(:project, :repository) } let(:processor) { described_class.new(values, project, '12345') } @@ -92,7 +92,8 @@ describe Gitlab::Ci::External::Processor do end before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) + .to receive(:fetch_local_content).and_return(local_file_content) end it 'should append the file to the values' do @@ -131,7 +132,10 @@ describe Gitlab::Ci::External::Processor do before do local_file_content = File.read(Rails.root.join('spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml')) - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + + allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) + .to receive(:fetch_local_content).and_return(local_file_content) + WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) end @@ -150,7 +154,8 @@ describe Gitlab::Ci::External::Processor do let(:local_file_content) { 'invalid content file ////' } before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) + .to receive(:fetch_local_content).and_return(local_file_content) end it 'should raise an error' do diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index b43aca8a354..7a749a2ef6d 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -1,6 +1,4 @@ -require 'fast_spec_helper' - -require_dependency 'active_model' +require 'spec_helper' describe Gitlab::Ci::Config do let(:config) do -- cgit v1.2.1 From eb315ef9c3ce5907a92a10d223fda0bf165f9ef7 Mon Sep 17 00:00:00 2001 From: Mark Lapierre Date: Fri, 3 Aug 2018 14:32:12 -0400 Subject: Update database changes MR template Updates the database changes MR template to match the default template. Items from the general checklist were removed and a reminder was added to keep the description up-to-date. --- .../merge_request_templates/Database changes.md | 28 ++++++++++++++-------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/.gitlab/merge_request_templates/Database changes.md b/.gitlab/merge_request_templates/Database changes.md index e636ec313df..354393b60e0 100644 --- a/.gitlab/merge_request_templates/Database changes.md +++ b/.gitlab/merge_request_templates/Database changes.md @@ -1,8 +1,23 @@ -Add a description of your merge request here. Merge requests without an adequate -description will not be reviewed until one is added. +## What does this MR do? + + + +Add a description of your merge request here. ## Database checklist +- [ ] Conforms to the [database guides](https://docs.gitlab.com/ee/development/README.html#databases-guides) + When adding migrations: - [ ] Updated `db/schema.rb` @@ -35,16 +50,9 @@ When removing columns, tables, indexes or other structures: - [ ] [Changelog entry](https://docs.gitlab.com/ee/development/changelog.html) added, if necessary - [ ] [Documentation created/updated](https://docs.gitlab.com/ee/development/documentation/index.html#contributing-to-docs) -- [ ] [API support added](https://docs.gitlab.com/ee/development/api_styleguide.html) - [ ] [Tests added for this feature/bug](https://docs.gitlab.com/ee/development/testing_guide/index.html) -- Conforms to the [code review guidelines](https://docs.gitlab.com/ee/development/code_review.html) - - [ ] Has been reviewed by a Backend [maintainer](https://about.gitlab.com/handbook/engineering/#maintainer) - - [ ] Has been reviewed by a Database [specialist](https://about.gitlab.com/team/structure/#specialist) +- [ ] Conforms to the [code review guidelines](https://docs.gitlab.com/ee/development/code_review.html) - [ ] Conforms to the [merge request performance guidelines](https://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html) - [ ] Conforms to the [style guides](https://gitlab.com/gitlab-org/gitlab-ee/blob/master/CONTRIBUTING.md#style-guides) -- [ ] If you have multiple commits, please combine them into a few logically organized commits by [squashing them](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) -- [ ] [Internationalization required/considered](https://docs.gitlab.com/ee/development/i18n/index.html) -- [ ] For a paid feature, have we considered GitLab.com plans, how it works for groups, and is there a design for promoting it to users who aren't on the correct plan? -- [ ] [End-to-end tests](https://docs.gitlab.com/ee/development/testing_guide/end_to_end_tests.html#testing-code-in-merge-requests) pass (`package-and-qa` manual pipeline job) /label ~database -- cgit v1.2.1 From 9442e6f2d53d38b1c5dc4733b11b40b391e2a79f Mon Sep 17 00:00:00 2001 From: Daniel Gruesso Date: Tue, 16 Oct 2018 16:24:23 +0000 Subject: Link to Nurtch doc site. --- doc/user/project/clusters/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 3ec17806490..48004471f0a 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -215,7 +215,7 @@ twice, which can lead to confusion during deployments. | [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) | 10.2+ | Ingress can provide load balancing, SSL termination, and name-based virtual hosting. It acts as a web proxy for your applications and is useful if you want to use [Auto DevOps] or deploy your own web apps. | [stable/nginx-ingress](https://github.com/helm/charts/tree/master/stable/nginx-ingress) | | [Prometheus](https://prometheus.io/docs/introduction/overview/) | 10.4+ | Prometheus is an open-source monitoring and alerting system useful to supervise your deployed applications. | [stable/prometheus](https://github.com/helm/charts/tree/master/stable/prometheus) | | [GitLab Runner](https://docs.gitlab.com/runner/) | 10.6+ | GitLab Runner is the open source project that is used to run your jobs and send the results back to GitLab. It is used in conjunction with [GitLab CI/CD](https://about.gitlab.com/features/gitlab-ci-cd/), the open-source continuous integration service included with GitLab that coordinates the jobs. When installing the GitLab Runner via the applications, it will run in **privileged mode** by default. Make sure you read the [security implications](#security-implications) before doing so. | [runner/gitlab-runner](https://gitlab.com/charts/gitlab-runner) | -| [JupyterHub](http://jupyter.org/) | 11.0+ | [JupyterHub](https://jupyterhub.readthedocs.io/en/stable/) is a multi-user service for managing notebooks across a team. [Jupyter Notebooks](https://jupyter-notebook.readthedocs.io/en/latest/) provide a web-based interactive programming environment used for data analysis, visualization, and machine learning. We use [this](https://gitlab.com/gitlab-org/jupyterhub-user-image/blob/master/Dockerfile) custom Jupyter image that installs additional useful packages on top of the base Jupyter. You will also see ready-to-use DevOps Runbooks built with [Rubix](https://github.com/amit1rrr/rubix). **Note**: Authentication will be enabled for any user of the GitLab server via OAuth2. HTTPS will be supported in a future release. | [jupyter/jupyterhub](https://jupyterhub.github.io/helm-chart/) | +| [JupyterHub](http://jupyter.org/) | 11.0+ | [JupyterHub](https://jupyterhub.readthedocs.io/en/stable/) is a multi-user service for managing notebooks across a team. [Jupyter Notebooks](https://jupyter-notebook.readthedocs.io/en/latest/) provide a web-based interactive programming environment used for data analysis, visualization, and machine learning. We use [this](https://gitlab.com/gitlab-org/jupyterhub-user-image/blob/master/Dockerfile) custom Jupyter image that installs additional useful packages on top of the base Jupyter. You will also see ready-to-use DevOps Runbooks built with Nurtch's [Rubix library](https://github.com/amit1rrr/rubix). More information on creating executable runbooks can be found at [Nurtch Documentation](http://docs.nurtch.com/en/latest). **Note**: Authentication will be enabled for any user of the GitLab server via OAuth2. HTTPS will be supported in a future release. | [jupyter/jupyterhub](https://jupyterhub.github.io/helm-chart/) | ## Getting the external IP address -- cgit v1.2.1 From 9628fafa2d8df0008fe62637bad5616a8ca47d22 Mon Sep 17 00:00:00 2001 From: Artur Pomadowski Date: Tue, 16 Oct 2018 20:32:12 +0000 Subject: Fix param typo in notes.md --- doc/api/notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/notes.md b/doc/api/notes.md index 44940bdd9e5..9f6740ad86a 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -98,7 +98,7 @@ POST /projects/:id/issues/:issue_iid/notes Parameters: - `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) -- `issue_id` (required) - The IID of an issue +- `issue_iid` (required) - The IID of an issue - `body` (required) - The content of a note - `created_at` (optional) - Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) -- cgit v1.2.1 From c3c22cb210dd1545b1a4ef500643480836245bae Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Tue, 16 Oct 2018 21:05:04 +1300 Subject: Move shared examples into the only file where used This will make it easier to perform in place adjustments, etc for future MRs involving group clusters. --- spec/services/clusters/create_service_spec.rb | 103 +++++++++++++++++---- .../services/clusters/create_service_shared.rb | 59 ------------ 2 files changed, 87 insertions(+), 75 deletions(-) delete mode 100644 spec/support/services/clusters/create_service_shared.rb diff --git a/spec/services/clusters/create_service_spec.rb b/spec/services/clusters/create_service_spec.rb index 3959295c13e..5373ad7f4b2 100644 --- a/spec/services/clusters/create_service_spec.rb +++ b/spec/services/clusters/create_service_spec.rb @@ -2,33 +2,104 @@ require 'spec_helper' describe Clusters::CreateService do let(:access_token) { 'xxx' } - let(:project) { create(:project) } let(:user) { create(:user) } + let(:service) { described_class.new(user, params) } - subject { described_class.new(user, params).execute(project: project, access_token: access_token) } + describe '#execute' do + before do + allow(ClusterProvisionWorker).to receive(:perform_async) + end - context 'when provider is gcp' do - context 'when project has no clusters' do - context 'when correct params' do - include_context 'valid cluster create params' + shared_context 'valid cluster create params' do + let(:params) do + { + name: 'test-cluster', + provider_type: :gcp, + provider_gcp_attributes: { + gcp_project_id: 'gcp-project', + zone: 'us-central1-a', + num_nodes: 1, + machine_type: 'machine_type-a', + legacy_abac: 'true' + } + } + end + end - include_examples 'create cluster service success' + shared_context 'invalid cluster create params' do + let(:params) do + { + name: 'test-cluster', + provider_type: :gcp, + provider_gcp_attributes: { + gcp_project_id: '!!!!!!!', + zone: 'us-central1-a', + num_nodes: 1, + machine_type: 'machine_type-a' + } + } end + end - context 'when invalid params' do - include_context 'invalid cluster create params' + shared_examples 'create cluster service success' do + it 'creates a cluster object and performs a worker' do + expect(ClusterProvisionWorker).to receive(:perform_async) - include_examples 'create cluster service error' + expect { subject } + .to change { Clusters::Cluster.count }.by(1) + .and change { Clusters::Providers::Gcp.count }.by(1) + + expect(subject.name).to eq('test-cluster') + expect(subject.user).to eq(user) + expect(subject.provider.gcp_project_id).to eq('gcp-project') + expect(subject.provider.zone).to eq('us-central1-a') + expect(subject.provider.num_nodes).to eq(1) + expect(subject.provider.machine_type).to eq('machine_type-a') + expect(subject.provider.access_token).to eq(access_token) + expect(subject.provider).to be_legacy_abac + expect(subject.platform).to be_nil end end - context 'when project has a cluster' do - include_context 'valid cluster create params' - let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) } - - it 'does not create a cluster' do + shared_examples 'create cluster service error' do + it 'returns an error' do expect(ClusterProvisionWorker).not_to receive(:perform_async) - expect { subject }.to raise_error(ArgumentError).and change { Clusters::Cluster.count }.by(0) + expect { subject }.to change { Clusters::Cluster.count }.by(0) + expect(subject.errors[:"provider_gcp.gcp_project_id"]).to be_present + end + end + + context 'create cluster for project' do + let(:project) { create(:project) } + + subject { service.execute(project: project, access_token: access_token) } + + context 'when project has no clusters' do + context 'when correct params' do + include_context 'valid cluster create params' + + include_examples 'create cluster service success' + + it 'associates project to the cluster' do + expect(subject.project).to eq(project) + end + end + + context 'when invalid params' do + include_context 'invalid cluster create params' + + include_examples 'create cluster service error' + end + end + + context 'when project has a cluster' do + include_context 'valid cluster create params' + let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) } + + it 'does not create a cluster' do + expect(ClusterProvisionWorker).not_to receive(:perform_async) + expect { subject }.to raise_error(ArgumentError).and change { Clusters::Cluster.count }.by(0) + end end end end diff --git a/spec/support/services/clusters/create_service_shared.rb b/spec/support/services/clusters/create_service_shared.rb deleted file mode 100644 index b0bf942aa09..00000000000 --- a/spec/support/services/clusters/create_service_shared.rb +++ /dev/null @@ -1,59 +0,0 @@ -shared_context 'valid cluster create params' do - let(:params) do - { - name: 'test-cluster', - provider_type: :gcp, - provider_gcp_attributes: { - gcp_project_id: 'gcp-project', - zone: 'us-central1-a', - num_nodes: 1, - machine_type: 'machine_type-a', - legacy_abac: 'true' - } - } - end -end - -shared_context 'invalid cluster create params' do - let(:params) do - { - name: 'test-cluster', - provider_type: :gcp, - provider_gcp_attributes: { - gcp_project_id: '!!!!!!!', - zone: 'us-central1-a', - num_nodes: 1, - machine_type: 'machine_type-a' - } - } - end -end - -shared_examples 'create cluster service success' do - it 'creates a cluster object and performs a worker' do - expect(ClusterProvisionWorker).to receive(:perform_async) - - expect { subject } - .to change { Clusters::Cluster.count }.by(1) - .and change { Clusters::Providers::Gcp.count }.by(1) - - expect(subject.name).to eq('test-cluster') - expect(subject.user).to eq(user) - expect(subject.project).to eq(project) - expect(subject.provider.gcp_project_id).to eq('gcp-project') - expect(subject.provider.zone).to eq('us-central1-a') - expect(subject.provider.num_nodes).to eq(1) - expect(subject.provider.machine_type).to eq('machine_type-a') - expect(subject.provider.access_token).to eq(access_token) - expect(subject.provider).to be_legacy_abac - expect(subject.platform).to be_nil - end -end - -shared_examples 'create cluster service error' do - it 'returns an error' do - expect(ClusterProvisionWorker).not_to receive(:perform_async) - expect { subject }.to change { Clusters::Cluster.count }.by(0) - expect(subject.errors[:"provider_gcp.gcp_project_id"]).to be_present - end -end -- cgit v1.2.1 From 90056ed25b547537b02b29715e6153d3aab4cc79 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 16 Oct 2018 22:31:27 +0000 Subject: Clarify responsibilities of MR author and maintainer based on feedback. --- doc/development/code_review.md | 91 +++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/doc/development/code_review.md b/doc/development/code_review.md index d18bb51eb5d..fac31fe8e8a 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -14,7 +14,8 @@ You are strongly encouraged to get your code **reviewed** by a there is any code to review, to get a second opinion on the chosen solution and implementation, and an extra pair of eyes looking for bugs, logic problems, or uncovered edge cases. The reviewer can be from a different team, but it is often -useful to pick someone who knows the domain well. +useful to pick someone who knows the domain well. You can read more about the +importance of involving reviewer(s) in the section on the responsibility of the author below. If you need some guidance (e.g. it's your first merge request), feel free to ask one of the [Merge request coaches][team]. @@ -38,49 +39,59 @@ or more [maintainers](https://about.gitlab.com/handbook/engineering/#maintainer) Getting your merge request **merged** also requires a maintainer. If it requires more than one approval, the last maintainer to review and approve it will also merge it. -Keep in mind that maintainers are also going to perform a final code review. -The ideal scenario is that the reviewer has already identified any concerns -the maintainer would have found, and the maintainer only has to perform the -merge, but be prepared for further review comments. +As described in the section on the responsibility of the maintainer below, you +are recommended to get your merge request approved and merged by maintainer(s) +from other teams than your own. -### The role of the maintainer +### The responsibility of the merge request author -Maintainers are responsible for the overall health, quality, and consistency of -the GitLab codebase, across domains and product areas. Consequently, their reviews -will focus primarily on things like overall architecture, code organization, -separation of concerns, tests, DRYness, consistency, readability, etc. +The responsibility to find the best solution and implement it lies with the +merge request author. -Their job is explicitly _not_ to review the solution itself. By the time a merge -request makes it to a maintainer, they should be able to assume that it actually -solves the problem it was meant to solve, that it does so in the most appropriate -way, that it satisfies all requirements, and that there are no remaining bugs, -logical problems, or uncovered edge cases. +Before assigning a merge request to maintainer for approval and merge, they +should be confident that it actually solves the problem it was meant to solve, +that it does so in the most appropriate way, that it satisfies all requirements, +and that there are no remaining bugs, logical problems, or uncovered edge cases. +The merge request should also have a completed task list in its description and +a passing CI pipeline to avoid unnecessary back and forth. -The responsibility to find the best solution and implement it lies with the -merge request author, and they should be confident of the chosen solution, -implementation, and everything else that makes up the merge request, before -they ask a maintainer for final review, approval, and merge. - -To reach this level of confidence, an author is expected to involve other people -in the investigation and implementation processes as appropriate. They may want -to reach out to domain experts to discuss different solutions or get an -implementation reviewed, to product managers and UX designers to clear up -confusion or verify that the end result matches what they had in mind, to -database specialists to get input on the data model or specific queries, -or to any other developer to get a code review. - -Of course, a maintainer will also make note of any issues with the solution or -implementation they may find, but in general will assume that the author is the -expert on the issue at hand, and that they made their choices with good reason. - -Since a maintainer's job does not depend on their domain-specific knowledge beyond -knowledge of the overall GitLab codebase, they can review merge requests from any -team and in any product area. - -Authors are recommended to assign merge requests to maintainers from other teams -than their own, to ensure that all code across GitLab is consistent and can be -easily understood by all contributors, from both inside and outside the company, -without requiring team-specific expertise. +To reach the required level of confidence in their solution, an author is expected +to involve other people in the investigation and implementation processes as +appropriate: + +They are encouraged to reach out to domain experts to discuss different solutions +or get an implementation reviewed, to product managers and UX designers to clear +up confusion or verify that the end result matches what they had in mind, to +database specialists to get input on the data model or specific queries, or to +any other developer to get an in-depth review of the solution. + +### The responsibility of the maintainer + +Maintainers are responsible for the overall health, quality, and consistency of +the GitLab codebase, across domains and product areas. + +Consequently, their reviews will focus primarily on things like overall +architecture, code organization, separation of concerns, tests, DRYness, +consistency, and readability. + +Since a maintainer's job only depends on their knowledge of the overall GitLab +codebase, and not that of any specific domain, they can review, approve and merge +merge requests from any team and in any product area. + +In fact, authors are recommended to get their merge requests merged by maintainers +from other teams than their own, to ensure that all code across GitLab is consistent +and can be easily understood by all contributors, from both inside and outside the +company, without requiring team-specific expertise. + +Maintainers will do their best to also review the specifics of the chosen solution +before merging, but as they are not necessarily domain experts, they may be poorly +placed to do so without an unreasonable investment of time. In those cases, they +will defer to the judgment of the author and earlier reviewers and involved domain +experts, in favor of focusing on their primary responsibilities. + +If a developer who happens to also be a maintainer was involved in a merge request +as a domain expert and/or reviewer, it is recommended that they are not also picked +as the maintainer to ultimately approve and merge it. ## Best practices -- cgit v1.2.1 From d1fe07a2e5aeaa29d191eeab130335876fd07f85 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 17 Oct 2018 11:48:41 +0100 Subject: Fixes broken borders in reports section MR widget --- .../reports/components/grouped_test_reports_app.vue | 2 +- .../vue_merge_request_widget/mr_widget_options.vue | 11 ++++++----- changelogs/unreleased/51386-broken-border-reports.yml | 5 +++++ 3 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/51386-broken-border-reports.yml diff --git a/app/assets/javascripts/reports/components/grouped_test_reports_app.vue b/app/assets/javascripts/reports/components/grouped_test_reports_app.vue index fb8c6402d02..b373d83a44b 100644 --- a/app/assets/javascripts/reports/components/grouped_test_reports_app.vue +++ b/app/assets/javascripts/reports/components/grouped_test_reports_app.vue @@ -82,7 +82,7 @@ :loading-text="groupedSummaryText" :error-text="groupedSummaryText" :has-issues="reports.length > 0" - class="mr-widget-border-top grouped-security-reports mr-report" + class="mr-widget-section grouped-security-reports mr-report" >
-
+ +
Date: Wed, 17 Oct 2018 12:46:10 +0000 Subject: Should be `start_in` not `start_key` --- doc/ci/yaml/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 24d60a0cdcc..424e1af7ba3 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -681,7 +681,7 @@ Delayed job are for executing scripts after a certain period. This is useful if you want to avoid jobs entering `pending` state immediately. You can set the period with `start_in` key. The value of `start_in` key is an elapsed time in seconds, unless a unit is -provided. `start_key` must be less than or equal to one hour. Examples of valid values include: +provided. `start_in` key must be less than or equal to one hour. Examples of valid values include: - `10 seconds` - `30 minutes` -- cgit v1.2.1 From 49d7c3b3461442a60d06a26c8cc971205afa3642 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 17 Oct 2018 15:16:28 +0100 Subject: Removes extra empty lines --- spec/features/projects/pipelines/pipeline_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index a79dbe7f877..5734c7e355e 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -72,7 +72,6 @@ describe 'Pipeline', :js do expect(page).to have_link(pipeline.ref) end - it_behaves_like 'showing user status' do let(:user_with_status) { pipeline.user } @@ -252,7 +251,6 @@ describe 'Pipeline', :js do expect(page).to have_content(pipeline.ref) end end - end context 'when user does not have access to read jobs' do -- cgit v1.2.1 From 6b1820b5694e920323ada00cf36612f6343aceae Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 17 Oct 2018 17:29:25 +0100 Subject: Amend the tech debt in follow-ups policy --- doc/development/contributing/issue_workflow.md | 50 +++++++++++++++++--------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index d302065347e..ad7887b496d 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -319,23 +319,39 @@ Make sure to mention the merge request that the ~"technical debt" issue or It's common to discover technical debt during development of a new feature. In the spirit of "minimum viable change", resolution is often deferred to a -follow-up issue. However, this has limited value unless a commitment to address -the debt is made. As technical debt reduces development velocity, it's important -to keep it under control. - -Before accepting resolution of technical debt in a follow-up issue, maintainers -should check that that fix is not trivial, and that the contributor (or their -team) can commit to scheduling the issue within the next 3 releases. - -Trivial fixes can - and should - be addressed within the same MR. - -If a commitment to address the issue in the foreseeable future cannot be found, -the maintainer must make a value judgement on whether the problem deserves an -issue at all. If the commitment is lacking because the issue is neither trivial -nor valuable, then perhaps no issue needs to be made after all. If a commitment -is lacking because technical debt is being unfairly neglected, then maintainers -should generally insist on resolution of the issue upfront, to protect -development velocity. +follow-up issue. However, this cannot be used as an excuse to merge poor-quality +code that would otherwise not pass review, or to overlook trivial matters that +don't deserve the be scheduled independently, and would be best resolved in the +original merge request - or not tracked at all! + +The overheads of scheduling, and rate of change in the GitLab codebase, mean +that the cost of a trivial technical debt issue can quickly exceed the value of +tracking it. This generally means we should resolve these in the original merge +request - or simply not create a follow-up issue at all. + +For example, a typo in a comment that is being copied between files is worth +fixing in the same MR, but not worth creating a follow-up issue for. Renaming a +method that is used in many places to make its intent slightly clearer may be +worth fixing, but it should not happen in the same MR, and is generally not +worth the overhead of having an issue of its own. These issues would invariably +be labelled `~P4 ~S4` if we were to create them. + +More severe technical debt can have implications for development velocity. If +it isn't addressed in a timely manner, the codebase becomes needlessly difficult +to change, new features become difficult to add, and regressions abound. + +Discoveries of this kind of technical debt should be treated seriously, and +while resolution in a follow-up issue may be appropriate, maintainers should +generally obtain a scheduling commitment from the author of the original MR, or +the engineering or product manager for the relevant area. This may take the form +of appropriate Priority / Severity labels on the issue, or an explicit milestone +and assignee. + +The maintainer must always agree before an outstanding discussion is resolved in +this manner, and will be the one to create the issue. The title and description +should be of the same quality as those created +[in the usual manner](#technical-and-ux-debt) - in particular, the issue title +**must not** begin with "Follow-up ...". ## Stewardship -- cgit v1.2.1 From 290e458113c2c5fff6bd265c0b3c16d6ac31fdda Mon Sep 17 00:00:00 2001 From: Jasper Maes Date: Wed, 17 Oct 2018 18:46:24 +0200 Subject: Rails5: fix deployment model spec --- changelogs/unreleased/rails5-fix-deployment-spec.yml | 5 +++++ spec/models/deployment_spec.rb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/rails5-fix-deployment-spec.yml diff --git a/changelogs/unreleased/rails5-fix-deployment-spec.yml b/changelogs/unreleased/rails5-fix-deployment-spec.yml new file mode 100644 index 00000000000..9e53c617a54 --- /dev/null +++ b/changelogs/unreleased/rails5-fix-deployment-spec.yml @@ -0,0 +1,5 @@ +--- +title: 'Rails5: fix deployment model spec' +merge_request: 22428 +author: Jasper Maes +type: other diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 182070781dd..b8364e0cf88 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -57,7 +57,7 @@ describe Deployment do last_deployments = described_class.last_for_environment([staging, production, testing]) expect(last_deployments.size).to eq(2) - expect(last_deployments).to eq(deployments.last(2)) + expect(last_deployments).to match_array(deployments.last(2)) end end end -- cgit v1.2.1 From 0c13bffbdc121513ef8d586b0e912cfcd7ee3169 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Wed, 17 Oct 2018 18:06:49 +0100 Subject: Maintainers should be involved in follow-up issues --- doc/development/contributing/issue_workflow.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index ad7887b496d..dac2948dbca 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -351,7 +351,8 @@ The maintainer must always agree before an outstanding discussion is resolved in this manner, and will be the one to create the issue. The title and description should be of the same quality as those created [in the usual manner](#technical-and-ux-debt) - in particular, the issue title -**must not** begin with "Follow-up ...". +**must not** begin with `Follow-up`! The creating maintainer should also expect +to be involved in some capacity when work begins on the follow-up issue. ## Stewardship -- cgit v1.2.1 From 2a631de547b230e52daf591cbbf31e0b1e953d45 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 17 Oct 2018 17:38:45 +0000 Subject: Strongly recommend involving a domain expert, especially when in doubt. --- doc/development/code_review.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 4d3a817e78b..3fe79943fdc 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -13,8 +13,8 @@ You are strongly encouraged to get your code **reviewed** by a [reviewer](https://about.gitlab.com/handbook/engineering/#reviewer) as soon as there is any code to review, to get a second opinion on the chosen solution and implementation, and an extra pair of eyes looking for bugs, logic problems, or -uncovered edge cases. The reviewer can be from a different team, but it is often -useful to pick someone who knows the domain well. You can read more about the +uncovered edge cases. The reviewer can be from a different team, but it is +recommended to pick someone who knows the domain well. You can read more about the importance of involving reviewer(s) in the section on the responsibility of the author below. If you need some guidance (e.g. it's your first merge request), feel free to ask @@ -48,7 +48,7 @@ from other teams than your own. The responsibility to find the best solution and implement it lies with the merge request author. -Before assigning a merge request to maintainer for approval and merge, they +Before assigning a merge request to a maintainer for approval and merge, they should be confident that it actually solves the problem it was meant to solve, that it does so in the most appropriate way, that it satisfies all requirements, and that there are no remaining bugs, logical problems, or uncovered edge cases. @@ -57,7 +57,7 @@ a passing CI pipeline to avoid unnecessary back and forth. To reach the required level of confidence in their solution, an author is expected to involve other people in the investigation and implementation processes as -appropriate: +appropriate. They are encouraged to reach out to domain experts to discuss different solutions or get an implementation reviewed, to product managers and UX designers to clear @@ -65,6 +65,10 @@ up confusion or verify that the end result matches what they had in mind, to database specialists to get input on the data model or specific queries, or to any other developer to get an in-depth review of the solution. +If an author is unsure if a merge request needs a domain expert's opinion, that's +usually a pretty good sign that it does, since without it the required level of +confidence in their solution will not have been reached. + ### The responsibility of the maintainer Maintainers are responsible for the overall health, quality, and consistency of -- cgit v1.2.1 From c57ac000067b73640033a359267dbacf57e7d2d0 Mon Sep 17 00:00:00 2001 From: Mark Lapierre Date: Tue, 25 Sep 2018 13:33:48 -0400 Subject: Add support for pushing and viewing files The MR below adds a test for the code owners feature. This adds the part of those changes specific to CE - the ability to add and view files in a project. https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/7368 --- app/views/projects/tree/_tree_content.html.haml | 2 +- qa/qa/factory/repository/push.rb | 12 ++++++++++++ qa/qa/page/project/show.rb | 10 ++++++++++ qa/spec/factory/repository/push_spec.rb | 26 +++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 qa/spec/factory/repository/push_spec.rb diff --git a/app/views/projects/tree/_tree_content.html.haml b/app/views/projects/tree/_tree_content.html.haml index 587aeafa82f..5e0523f0b96 100644 --- a/app/views/projects/tree/_tree_content.html.haml +++ b/app/views/projects/tree/_tree_content.html.haml @@ -1,6 +1,6 @@ .tree-content-holder.js-tree-content{ 'data-logs-path': @logs_path } .table-holder - %table.table#tree-slider{ class: "table_#{@hex_path} tree-table" } + %table.table#tree-slider{ class: "table_#{@hex_path} tree-table qa-file-tree" } %thead %tr %th= s_('ProjectFileTree|Name') diff --git a/qa/qa/factory/repository/push.rb b/qa/qa/factory/repository/push.rb index 6c5088f1da5..703c78daa99 100644 --- a/qa/qa/factory/repository/push.rb +++ b/qa/qa/factory/repository/push.rb @@ -30,6 +30,14 @@ module QA @directory = dir end + def files=(files) + if !files.is_a?(Array) || files.empty? + raise ArgumentError, "Please provide an array of hashes e.g.: [{name: 'file1', content: 'foo'}]" + end + + @files = files + end + def fabricate! Git::Repository.perform do |repository| if ssh_key @@ -63,6 +71,10 @@ module QA @directory.each_child do |f| repository.add_file(f.basename, f.read) if f.file? end + elsif @files + @files.each do |f| + repository.add_file(f[:name], f[:content]) + end else repository.add_file(file_name, file_content) end diff --git a/qa/qa/page/project/show.rb b/qa/qa/page/project/show.rb index fcc4bb79c10..d6dddf03ffb 100644 --- a/qa/qa/page/project/show.rb +++ b/qa/qa/page/project/show.rb @@ -42,6 +42,10 @@ module QA element :web_ide_button end + view 'app/views/projects/tree/_tree_content.html.haml' do + element :file_tree + end + def project_name find('.qa-project-name').text end @@ -51,6 +55,12 @@ module QA click_element :new_file_option end + def go_to_file(filename) + within_element(:file_tree) do + click_on filename + end + end + def switch_to_branch(branch_name) find_element(:branches_select).click diff --git a/qa/spec/factory/repository/push_spec.rb b/qa/spec/factory/repository/push_spec.rb new file mode 100644 index 00000000000..2eb6c008248 --- /dev/null +++ b/qa/spec/factory/repository/push_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +describe QA::Factory::Repository::Push do + describe '.files=' do + let(:files) do + [ + { + name: 'file.txt', + content: 'foo' + } + ] + end + + it 'raises an error if files is not an array' do + expect { subject.files = '' }.to raise_error(ArgumentError) + end + + it 'raises an error if files is an empty array' do + expect { subject.files = [] }.to raise_error(ArgumentError) + end + + it 'does not raise if files is an array' do + expect { subject.files = files }.not_to raise_error + end + end +end -- cgit v1.2.1 From 3d82f20d1bae1ba4f67a87d66828d65c7cef651d Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 17 Oct 2018 13:02:40 -0700 Subject: Strip whitespace around GitHub personal access tokens Some browsers insert a trailing whitespace after pasting the token into the field. This should help reduce confusion. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/46588 --- app/controllers/import/github_controller.rb | 2 +- changelogs/unreleased/sh-strip-github-pat-whitespace.yml | 5 +++++ .../githubish_import_controller_shared_examples.rb | 12 ++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-strip-github-pat-whitespace.yml diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index 1dfa814cdd5..e3eec5a020d 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -20,7 +20,7 @@ class Import::GithubController < Import::BaseController end def personal_access_token - session[access_token_key] = params[:personal_access_token] + session[access_token_key] = params[:personal_access_token]&.strip redirect_to status_import_url end diff --git a/changelogs/unreleased/sh-strip-github-pat-whitespace.yml b/changelogs/unreleased/sh-strip-github-pat-whitespace.yml new file mode 100644 index 00000000000..ea26f57e8f0 --- /dev/null +++ b/changelogs/unreleased/sh-strip-github-pat-whitespace.yml @@ -0,0 +1,5 @@ +--- +title: Strip whitespace around GitHub personal access tokens +merge_request: 22432 +author: +type: fixed diff --git a/spec/support/controllers/githubish_import_controller_shared_examples.rb b/spec/support/controllers/githubish_import_controller_shared_examples.rb index 1c1b68c12a2..140490f2dc2 100644 --- a/spec/support/controllers/githubish_import_controller_shared_examples.rb +++ b/spec/support/controllers/githubish_import_controller_shared_examples.rb @@ -22,6 +22,18 @@ shared_examples 'a GitHub-ish import controller: POST personal_access_token' do expect(session[:"#{provider}_access_token"]).to eq(token) expect(controller).to redirect_to(status_import_url) end + + it "strips access token with spaces" do + token = 'asdfasdf9876' + + allow_any_instance_of(Gitlab::LegacyGithubImport::Client) + .to receive(:user).and_return(true) + + post :personal_access_token, personal_access_token: " #{token} " + + expect(session[:"#{provider}_access_token"]).to eq(token) + expect(controller).to redirect_to(status_import_url) + end end shared_examples 'a GitHub-ish import controller: GET new' do -- cgit v1.2.1 From 8a20d242f331a84ceff980c0615df1aa6fa3b257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 17 Oct 2018 17:45:20 -0300 Subject: [ci skip] Update Gitaly installation documentation Now the rake task takes the path to the default storage directory. --- doc/install/installation.md | 4 ++-- doc/update/patch_versions.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/install/installation.md b/doc/install/installation.md index 9e2e58657f1..1210ac58499 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -460,11 +460,11 @@ GitLab-Pages uses [GNU Make](https://www.gnu.org/software/make/). This step is o ### Install Gitaly # Fetch Gitaly source with Git and compile with Go - sudo -u git -H bundle exec rake "gitlab:gitaly:install[/home/git/gitaly]" RAILS_ENV=production + sudo -u git -H bundle exec rake "gitlab:gitaly:install[/home/git/gitaly,/home/git/repositories]" RAILS_ENV=production You can specify a different Git repository by providing it as an extra parameter: - sudo -u git -H bundle exec rake "gitlab:gitaly:install[/home/git/gitaly,https://example.com/gitaly.git]" RAILS_ENV=production + sudo -u git -H bundle exec rake "gitlab:gitaly:install[/home/git/gitaly,/home/git/repositories,https://example.com/gitaly.git]" RAILS_ENV=production Next, make sure gitaly configured: diff --git a/doc/update/patch_versions.md b/doc/update/patch_versions.md index a4f17746b69..2e8380aa5d8 100644 --- a/doc/update/patch_versions.md +++ b/doc/update/patch_versions.md @@ -83,7 +83,7 @@ sudo -u git -H bundle exec rake "gitlab:workhorse:install[/home/git/gitlab-workh ```bash cd /home/git/gitlab -sudo -u git -H bundle exec rake "gitlab:gitaly:install[/home/git/gitaly]" RAILS_ENV=production +sudo -u git -H bundle exec rake "gitlab:gitaly:install[/home/git/gitaly,/home/git/repositories]" RAILS_ENV=production ``` ### 6. Update gitlab-shell to the corresponding version -- cgit v1.2.1 From 6683aa8b1073eb56c99343dafab11b2e65375a04 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Wed, 17 Oct 2018 17:39:12 -0500 Subject: Fix uninitialized constant HamlLint::Linter Running multiple `rake` commands with `spring', such as `spring rake db:rollback` could cause a `NameError: uninitialized constant HamlLint::Linter` --- lib/haml_lint/inline_javascript.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/haml_lint/inline_javascript.rb b/lib/haml_lint/inline_javascript.rb index 5ecd6169ecf..2e98227a05e 100644 --- a/lib/haml_lint/inline_javascript.rb +++ b/lib/haml_lint/inline_javascript.rb @@ -2,9 +2,9 @@ # frozen_string_literal: true unless Rails.env.production? - require 'haml_lint/haml_visitor' - require 'haml_lint/linter' - require 'haml_lint/linter_registry' + require_dependency 'haml_lint/haml_visitor' + require_dependency 'haml_lint/linter' + require_dependency 'haml_lint/linter_registry' module HamlLint class Linter::InlineJavaScript < Linter -- cgit v1.2.1 From 84b69a95bbe110cb688b7b6b6f20a53ff3819c89 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 18 Oct 2018 09:33:46 +0900 Subject: Test ApplicationsFinder !22296 --- spec/finders/applications_finder_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/finders/applications_finder_spec.rb diff --git a/spec/finders/applications_finder_spec.rb b/spec/finders/applications_finder_spec.rb new file mode 100644 index 00000000000..a75eb9c9ca5 --- /dev/null +++ b/spec/finders/applications_finder_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ApplicationsFinder do + let(:application1) { create(:application, name: 'some_application', owner: nil, redirect_uri: 'http://some_application.url', scopes: '') } + let(:application2) { create(:application, name: 'another_application', owner: nil, redirect_uri: 'http://other_application.url', scopes: '') } + + describe '#execute' do + it 'returns an array of applications' do + found = described_class.new.execute + + expect(found).to match_array([application1,application2]) + end + it 'returns the application by id' do + params = { id: application1.id } + found = described_class.new(params).execute + + expect(found).to match(application1) + end + end +end -- cgit v1.2.1 From deeeffe09dfd331bc6068cea23346a35ef46ff45 Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 18 Oct 2018 09:37:45 +0900 Subject: Source cleaning !22296 --- app/finders/applications_finder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/finders/applications_finder.rb b/app/finders/applications_finder.rb index 14bd35105d2..88b1d17cf19 100644 --- a/app/finders/applications_finder.rb +++ b/app/finders/applications_finder.rb @@ -9,7 +9,7 @@ class ApplicationsFinder # rubocop: disable CodeReuse/ActiveRecord def execute - applications = Doorkeeper::Application.where("owner_id IS NULL") + applications = Doorkeeper::Application.where(owner_id: nil) by_id(applications) end # rubocop: enable CodeReuse/ActiveRecord -- cgit v1.2.1 From 6da97336522dfba8a02485f765be433ec4f4781a Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 18 Oct 2018 09:43:35 +0900 Subject: Use ApplicationsFinder !22296 --- app/controllers/admin/applications_controller.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index 00d2cc01192..2fcd38607be 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -6,11 +6,9 @@ class Admin::ApplicationsController < Admin::ApplicationController before_action :set_application, only: [:show, :edit, :update, :destroy] before_action :load_scopes, only: [:new, :create, :edit, :update] - # rubocop: disable CodeReuse/ActiveRecord def index - @applications = Doorkeeper::Application.where("owner_id IS NULL") + @applications = ApplicationsFinder.new.execute end - # rubocop: enable CodeReuse/ActiveRecord def show end @@ -49,11 +47,9 @@ class Admin::ApplicationsController < Admin::ApplicationController private - # rubocop: disable CodeReuse/ActiveRecord def set_application - @application = Doorkeeper::Application.where("owner_id IS NULL").find(params[:id]) + @application = ApplicationsFinder.new({ id: params[:id] }).execute end - # rubocop: enable CodeReuse/ActiveRecord # Only allow a trusted parameter "white list" through. def application_params -- cgit v1.2.1 From fbd372114de4930e94266fbfee2268643e72930f Mon Sep 17 00:00:00 2001 From: JB Vasseur Date: Thu, 18 Oct 2018 10:08:23 +0900 Subject: Code styling --- spec/finders/applications_finder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/finders/applications_finder_spec.rb b/spec/finders/applications_finder_spec.rb index a75eb9c9ca5..14d6b35cc27 100644 --- a/spec/finders/applications_finder_spec.rb +++ b/spec/finders/applications_finder_spec.rb @@ -10,7 +10,7 @@ describe ApplicationsFinder do it 'returns an array of applications' do found = described_class.new.execute - expect(found).to match_array([application1,application2]) + expect(found).to match_array([application1, application2]) end it 'returns the application by id' do params = { id: application1.id } -- cgit v1.2.1 From f262b8f320c2f6760cc978ef946c7c2b5b440a58 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 10 Oct 2018 16:01:04 +0900 Subject: Improve specs for Ci::ProcessBuildService --- spec/services/ci/process_build_service_spec.rb | 251 +++++++++---------------- 1 file changed, 90 insertions(+), 161 deletions(-) diff --git a/spec/services/ci/process_build_service_spec.rb b/spec/services/ci/process_build_service_spec.rb index 9f47439dc4a..3c730c07457 100644 --- a/spec/services/ci/process_build_service_spec.rb +++ b/spec/services/ci/process_build_service_spec.rb @@ -2,141 +2,40 @@ require 'spec_helper' describe Ci::ProcessBuildService, '#execute' do + VALID_STATUSES_WHEN_ON_SUCCESS = %w[success skipped].freeze + VALID_STATUSES_WHEN_ON_FAILURE = %w[failed].freeze + VALID_STATUSES_WHEN_ALWAYS = %w[success failed skipped].freeze + VALID_STATUSES_WHEN_MANUAL = %w[success skipped].freeze + VALID_STATUSES_WHEN_DELAYED = %w[success skipped].freeze + SKIPPABLE_STATUES = %w[created pending].freeze + let(:user) { create(:user) } let(:project) { create(:project) } - subject { described_class.new(project, user).execute(build, current_status) } + subject { described_class.new(project, user).execute(build, status_for_prior_stages) } before do project.add_maintainer(user) end - shared_examples_for 'Enqueuing properly' do |valid_statuses_for_when| - valid_statuses_for_when.each do |status_for_prior_stages| - context "when status for prior stages is #{status_for_prior_stages}" do - let(:current_status) { status_for_prior_stages } - - %w[created skipped manual scheduled].each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, when: when_option, user: user, project: project) } - - it 'enqueues the build' do - expect { subject }.to change { build.status }.to('pending') - end - end - end - - %w[pending running success failed canceled].each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, when: when_option, user: user, project: project) } - - it 'does not change the build status' do - expect { subject }.not_to change { build.status } - end - end - end - end - end - - (HasStatus::AVAILABLE_STATUSES - valid_statuses_for_when).each do |status_for_prior_stages| - let(:current_status) { status_for_prior_stages } - - context "when status for prior stages is #{status_for_prior_stages}" do - %w[created pending].each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, when: when_option, user: user, project: project) } - - it 'skips the build' do - expect { subject }.to change { build.status }.to('skipped') - end - end - end - - (HasStatus::AVAILABLE_STATUSES - %w[created pending]).each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, when: when_option, user: user, project: project) } - - it 'does not change build status' do - expect { subject }.not_to change { build.status } - end - end - end - end - end - end - - shared_examples_for 'Actionizing properly' do |valid_statuses_for_when| - valid_statuses_for_when.each do |status_for_prior_stages| - context "when status for prior stages is #{status_for_prior_stages}" do - let(:current_status) { status_for_prior_stages } - - %w[created].each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :actionable, user: user, project: project) } - - it 'enqueues the build' do - expect { subject }.to change { build.status }.to('manual') - end - end - end - - %w[manual skipped pending running success failed canceled scheduled].each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :actionable, user: user, project: project) } - - it 'does not change the build status' do - expect { subject }.not_to change { build.status } - end - end - end - end - end - - (HasStatus::AVAILABLE_STATUSES - valid_statuses_for_when).each do |status_for_prior_stages| - let(:current_status) { status_for_prior_stages } - - context "when status for prior stages is #{status_for_prior_stages}" do - %w[created pending].each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :actionable, user: user, project: project) } - - it 'skips the build' do - expect { subject }.to change { build.status }.to('skipped') - end - end - end - - (HasStatus::AVAILABLE_STATUSES - %w[created pending]).each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :actionable, user: user, project: project) } - - it 'does not change build status' do - expect { subject }.not_to change { build.status } - end - end - end - end - end - end - - shared_examples_for 'Scheduling properly' do |valid_statuses_for_when| - valid_statuses_for_when.each do |status_for_prior_stages| - context "when status for prior stages is #{status_for_prior_stages}" do - let(:current_status) { status_for_prior_stages } + shared_examples_for 'Change the build status' do |when_option: nil, current_statuses: nil, from_statuses:, to_status:, factory_options: nil| + current_statuses.each do |current_status| + context "when status for prior stages is #{current_status}" do + let(:status_for_prior_stages) { current_status } - %w[created].each do |status| + from_statuses.each do |status| context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :schedulable, user: user, project: project) } + let(:build) { create(:ci_build, status.to_sym, *factory_options, when: when_option, user: user, project: project) } - it 'enqueues the build' do - expect { subject }.to change { build.status }.to('scheduled') + it 'changes the build status' do + expect { subject }.to change { build.status }.to(to_status) end end end - %w[manual skipped pending running success failed canceled scheduled].each do |status| + (HasStatus::AVAILABLE_STATUSES - from_statuses).each do |status| context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :schedulable, user: user, project: project) } + let(:build) { create(:ci_build, status.to_sym, *factory_options, when: when_option, user: user, project: project) } it 'does not change the build status' do expect { subject }.not_to change { build.status } @@ -145,61 +44,67 @@ describe Ci::ProcessBuildService, '#execute' do end end end - - (HasStatus::AVAILABLE_STATUSES - valid_statuses_for_when).each do |status_for_prior_stages| - let(:current_status) { status_for_prior_stages } - - context "when status for prior stages is #{status_for_prior_stages}" do - %w[created pending].each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :schedulable, user: user, project: project) } - - it 'skips the build' do - expect { subject }.to change { build.status }.to('skipped') - end - end - end - - (HasStatus::AVAILABLE_STATUSES - %w[created pending]).each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, :schedulable, user: user, project: project) } - - it 'does not change build status' do - expect { subject }.not_to change { build.status } - end - end - end - end - end end context 'when build has on_success option' do - let(:when_option) { :on_success } - - it_behaves_like 'Enqueuing properly', %w[success skipped] + it_behaves_like 'Change the build status', + when_option: :on_success, + current_statuses: VALID_STATUSES_WHEN_ON_SUCCESS, + from_statuses: %w[created skipped manual scheduled], + to_status: 'pending' + + it_behaves_like 'Change the build status', + when_option: :on_success, + current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), + from_statuses: SKIPPABLE_STATUES, + to_status: 'skipped' end context 'when build has on_failure option' do - let(:when_option) { :on_failure } - - it_behaves_like 'Enqueuing properly', %w[failed] + it_behaves_like 'Change the build status', + when_option: :on_failure, + current_statuses: VALID_STATUSES_WHEN_ON_FAILURE, + from_statuses: %w[created skipped manual scheduled], + to_status: 'pending' + + it_behaves_like 'Change the build status', + when_option: :on_failure, + current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_FAILURE), + from_statuses: SKIPPABLE_STATUES, + to_status: 'skipped' end context 'when build has always option' do - let(:when_option) { :always } - - it_behaves_like 'Enqueuing properly', %w[success failed skipped] + it_behaves_like 'Change the build status', + when_option: :always, + current_statuses: VALID_STATUSES_WHEN_ALWAYS, + from_statuses: %w[created skipped manual scheduled], + to_status: 'pending' + + it_behaves_like 'Change the build status', + when_option: :always, + current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ALWAYS), + from_statuses: SKIPPABLE_STATUES, + to_status: 'skipped' end context 'when build has manual option' do - let(:when_option) { :manual } - - it_behaves_like 'Actionizing properly', %w[success skipped] + it_behaves_like 'Change the build status', + when_option: :manual, + current_statuses: VALID_STATUSES_WHEN_MANUAL, + from_statuses: %w[created], + to_status: 'manual', + factory_options: %i[actionable] + + it_behaves_like 'Change the build status', + when_option: :manual, + current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), + from_statuses: SKIPPABLE_STATUES, + to_status: 'skipped', + factory_options: %i[actionable] end context 'when build has delayed option' do - let(:when_option) { :delayed } - before do allow(Ci::BuildScheduleWorker).to receive(:perform_at) { } end @@ -209,7 +114,19 @@ describe Ci::ProcessBuildService, '#execute' do stub_feature_flags(ci_enable_scheduled_build: true) end - it_behaves_like 'Scheduling properly', %w[success skipped] + it_behaves_like 'Change the build status', + when_option: :delayed, + current_statuses: VALID_STATUSES_WHEN_DELAYED, + from_statuses: %w[created], + to_status: 'scheduled', + factory_options: %i[schedulable] + + it_behaves_like 'Change the build status', + when_option: :delayed, + current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), + from_statuses: SKIPPABLE_STATUES, + to_status: 'skipped', + factory_options: %i[schedulable] end context 'when ci_enable_scheduled_build is enabled' do @@ -217,7 +134,19 @@ describe Ci::ProcessBuildService, '#execute' do stub_feature_flags(ci_enable_scheduled_build: false) end - it_behaves_like 'Actionizing properly', %w[success skipped] + it_behaves_like 'Change the build status', + when_option: :delayed, + current_statuses: VALID_STATUSES_WHEN_DELAYED, + from_statuses: %w[created], + to_status: 'manual', + factory_options: %i[schedulable] + + it_behaves_like 'Change the build status', + when_option: :delayed, + current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), + from_statuses: SKIPPABLE_STATUES, + to_status: 'skipped', + factory_options: %i[schedulable] end end end -- cgit v1.2.1 From 995d2d13a3e3aea2cd68ebf4a4017a597b0428f2 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 18 Oct 2018 15:07:27 +0900 Subject: Simplify the spec --- spec/services/ci/process_build_service_spec.rb | 201 ++++++++++++------------- 1 file changed, 97 insertions(+), 104 deletions(-) diff --git a/spec/services/ci/process_build_service_spec.rb b/spec/services/ci/process_build_service_spec.rb index 3c730c07457..9a53b32394d 100644 --- a/spec/services/ci/process_build_service_spec.rb +++ b/spec/services/ci/process_build_service_spec.rb @@ -2,106 +2,93 @@ require 'spec_helper' describe Ci::ProcessBuildService, '#execute' do - VALID_STATUSES_WHEN_ON_SUCCESS = %w[success skipped].freeze - VALID_STATUSES_WHEN_ON_FAILURE = %w[failed].freeze - VALID_STATUSES_WHEN_ALWAYS = %w[success failed skipped].freeze - VALID_STATUSES_WHEN_MANUAL = %w[success skipped].freeze - VALID_STATUSES_WHEN_DELAYED = %w[success skipped].freeze - SKIPPABLE_STATUES = %w[created pending].freeze - let(:user) { create(:user) } let(:project) { create(:project) } - subject { described_class.new(project, user).execute(build, status_for_prior_stages) } + subject { described_class.new(project, user).execute(build, current_status) } before do project.add_maintainer(user) end - shared_examples_for 'Change the build status' do |when_option: nil, current_statuses: nil, from_statuses:, to_status:, factory_options: nil| - current_statuses.each do |current_status| - context "when status for prior stages is #{current_status}" do - let(:status_for_prior_stages) { current_status } + context 'when build has on_success option' do + let(:build) { create(:ci_build, :created, when: :on_success, user: user, project: project) } - from_statuses.each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, *factory_options, when: when_option, user: user, project: project) } + context 'when current status is success' do + let(:current_status) { 'success' } - it 'changes the build status' do - expect { subject }.to change { build.status }.to(to_status) - end - end - end + it 'changes the build status' do + expect { subject }.to change { build.status }.to('pending') + end + end - (HasStatus::AVAILABLE_STATUSES - from_statuses).each do |status| - context "when build status is #{status}" do - let(:build) { create(:ci_build, status.to_sym, *factory_options, when: when_option, user: user, project: project) } + context 'when current status is failed' do + let(:current_status) { 'failed' } - it 'does not change the build status' do - expect { subject }.not_to change { build.status } - end - end - end + it 'does not change the build status' do + expect { subject }.to change { build.status }.to('skipped') end end end - context 'when build has on_success option' do - it_behaves_like 'Change the build status', - when_option: :on_success, - current_statuses: VALID_STATUSES_WHEN_ON_SUCCESS, - from_statuses: %w[created skipped manual scheduled], - to_status: 'pending' - - it_behaves_like 'Change the build status', - when_option: :on_success, - current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), - from_statuses: SKIPPABLE_STATUES, - to_status: 'skipped' - end - context 'when build has on_failure option' do - it_behaves_like 'Change the build status', - when_option: :on_failure, - current_statuses: VALID_STATUSES_WHEN_ON_FAILURE, - from_statuses: %w[created skipped manual scheduled], - to_status: 'pending' - - it_behaves_like 'Change the build status', - when_option: :on_failure, - current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_FAILURE), - from_statuses: SKIPPABLE_STATUES, - to_status: 'skipped' + let(:build) { create(:ci_build, :created, when: :on_failure, user: user, project: project) } + + context 'when current status is success' do + let(:current_status) { 'success' } + + it 'changes the build status' do + expect { subject }.to change { build.status }.to('skipped') + end + end + + context 'when current status is failed' do + let(:current_status) { 'failed' } + + it 'does not change the build status' do + expect { subject }.to change { build.status }.to('pending') + end + end end context 'when build has always option' do - it_behaves_like 'Change the build status', - when_option: :always, - current_statuses: VALID_STATUSES_WHEN_ALWAYS, - from_statuses: %w[created skipped manual scheduled], - to_status: 'pending' - - it_behaves_like 'Change the build status', - when_option: :always, - current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ALWAYS), - from_statuses: SKIPPABLE_STATUES, - to_status: 'skipped' + let(:build) { create(:ci_build, :created, when: :always, user: user, project: project) } + + context 'when current status is success' do + let(:current_status) { 'success' } + + it 'changes the build status' do + expect { subject }.to change { build.status }.to('pending') + end + end + + context 'when current status is failed' do + let(:current_status) { 'failed' } + + it 'does not change the build status' do + expect { subject }.to change { build.status }.to('pending') + end + end end context 'when build has manual option' do - it_behaves_like 'Change the build status', - when_option: :manual, - current_statuses: VALID_STATUSES_WHEN_MANUAL, - from_statuses: %w[created], - to_status: 'manual', - factory_options: %i[actionable] - - it_behaves_like 'Change the build status', - when_option: :manual, - current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), - from_statuses: SKIPPABLE_STATUES, - to_status: 'skipped', - factory_options: %i[actionable] + let(:build) { create(:ci_build, :created, :actionable, user: user, project: project) } + + context 'when current status is success' do + let(:current_status) { 'success' } + + it 'changes the build status' do + expect { subject }.to change { build.status }.to('manual') + end + end + + context 'when current status is failed' do + let(:current_status) { 'failed' } + + it 'does not change the build status' do + expect { subject }.to change { build.status }.to('skipped') + end + end end context 'when build has delayed option' do @@ -109,44 +96,50 @@ describe Ci::ProcessBuildService, '#execute' do allow(Ci::BuildScheduleWorker).to receive(:perform_at) { } end + let(:build) { create(:ci_build, :created, :schedulable, user: user, project: project) } + context 'when ci_enable_scheduled_build is enabled' do before do stub_feature_flags(ci_enable_scheduled_build: true) end - it_behaves_like 'Change the build status', - when_option: :delayed, - current_statuses: VALID_STATUSES_WHEN_DELAYED, - from_statuses: %w[created], - to_status: 'scheduled', - factory_options: %i[schedulable] - - it_behaves_like 'Change the build status', - when_option: :delayed, - current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), - from_statuses: SKIPPABLE_STATUES, - to_status: 'skipped', - factory_options: %i[schedulable] + context 'when current status is success' do + let(:current_status) { 'success' } + + it 'changes the build status' do + expect { subject }.to change { build.status }.to('scheduled') + end + end + + context 'when current status is failed' do + let(:current_status) { 'failed' } + + it 'does not change the build status' do + expect { subject }.to change { build.status }.to('skipped') + end + end end - context 'when ci_enable_scheduled_build is enabled' do + context 'when ci_enable_scheduled_build is disabled' do before do stub_feature_flags(ci_enable_scheduled_build: false) end - it_behaves_like 'Change the build status', - when_option: :delayed, - current_statuses: VALID_STATUSES_WHEN_DELAYED, - from_statuses: %w[created], - to_status: 'manual', - factory_options: %i[schedulable] - - it_behaves_like 'Change the build status', - when_option: :delayed, - current_statuses: (HasStatus::AVAILABLE_STATUSES - VALID_STATUSES_WHEN_ON_SUCCESS), - from_statuses: SKIPPABLE_STATUES, - to_status: 'skipped', - factory_options: %i[schedulable] + context 'when current status is success' do + let(:current_status) { 'success' } + + it 'changes the build status' do + expect { subject }.to change { build.status }.to('manual') + end + end + + context 'when current status is failed' do + let(:current_status) { 'failed' } + + it 'does not change the build status' do + expect { subject }.to change { build.status }.to('skipped') + end + end end end end -- cgit v1.2.1 From 23526470ddb2a51575dd7c280b89d21bef6df619 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 10 Oct 2018 14:27:16 +0900 Subject: Drop `allow_overflow` option in `TimeHelper.duration_in_numbers` --- app/helpers/time_helper.rb | 16 ++++++------ app/views/projects/ci/builds/_build.html.haml | 2 +- lib/gitlab/ci/status/build/scheduled.rb | 2 +- spec/helpers/time_helper_spec.rb | 35 +++++++-------------------- 4 files changed, 18 insertions(+), 37 deletions(-) diff --git a/app/helpers/time_helper.rb b/app/helpers/time_helper.rb index 3e6a301b77d..719c351242c 100644 --- a/app/helpers/time_helper.rb +++ b/app/helpers/time_helper.rb @@ -21,17 +21,15 @@ module TimeHelper "#{from.to_s(:short)} - #{to.to_s(:short)}" end - def duration_in_numbers(duration_in_seconds, allow_overflow = false) - if allow_overflow - seconds = duration_in_seconds % 1.minute - minutes = (duration_in_seconds / 1.minute) % (1.hour / 1.minute) - hours = duration_in_seconds / 1.hour + def duration_in_numbers(duration_in_seconds) + seconds = duration_in_seconds % 1.minute + minutes = (duration_in_seconds / 1.minute) % (1.hour / 1.minute) + hours = duration_in_seconds / 1.hour - "%02d:%02d:%02d" % [hours, minutes, seconds] + if hours == 0 + "%02d:%02d" % [minutes, seconds] else - time_format = duration_in_seconds < 1.hour ? "%M:%S" : "%H:%M:%S" - - Time.at(duration_in_seconds).utc.strftime(time_format) + "%02d:%02d:%02d" % [hours, minutes, seconds] end end end diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 95828626bd9..203d46cb1a6 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -108,7 +108,7 @@ .btn.btn-default.has-tooltip{ disabled: true, title: job.scheduled_at } = sprite_icon('planning') - = duration_in_numbers(job.execute_in, true) + = duration_in_numbers(job.execute_in) - confirmation_message = s_("DelayedJobs|Are you sure you want to run %{job_name} immediately? This job will run automatically after it's timer finishes.") % { job_name: job.name } = link_to play_project_job_path(job.project, job, return_to: request.original_url), method: :post, diff --git a/lib/gitlab/ci/status/build/scheduled.rb b/lib/gitlab/ci/status/build/scheduled.rb index eebb3f761c5..84cd58283d2 100644 --- a/lib/gitlab/ci/status/build/scheduled.rb +++ b/lib/gitlab/ci/status/build/scheduled.rb @@ -29,7 +29,7 @@ module Gitlab def execute_in remaining_seconds = [0, subject.scheduled_at - Time.now].max - duration_in_numbers(remaining_seconds, true) + duration_in_numbers(remaining_seconds) end end end diff --git a/spec/helpers/time_helper_spec.rb b/spec/helpers/time_helper_spec.rb index cc310766433..8bf378549fe 100644 --- a/spec/helpers/time_helper_spec.rb +++ b/spec/helpers/time_helper_spec.rb @@ -22,34 +22,17 @@ describe TimeHelper do describe "#duration_in_numbers" do using RSpec::Parameterized::TableSyntax - context "without passing allow_overflow" do - where(:duration, :formatted_string) do - 0 | "00:00" - 1.second | "00:01" - 42.seconds | "00:42" - 2.minutes + 1.second | "02:01" - 3.hours + 2.minutes + 1.second | "03:02:01" - 30.hours | "06:00:00" - end - - with_them do - it { expect(duration_in_numbers(duration)).to eq formatted_string } - end + where(:duration, :formatted_string) do + 0 | "00:00" + 1.second | "00:01" + 42.seconds | "00:42" + 2.minutes + 1.second | "02:01" + 3.hours + 2.minutes + 1.second | "03:02:01" + 30.hours | "30:00:00" end - context "with allow_overflow = true" do - where(:duration, :formatted_string) do - 0 | "00:00:00" - 1.second | "00:00:01" - 42.seconds | "00:00:42" - 2.minutes + 1.second | "00:02:01" - 3.hours + 2.minutes + 1.second | "03:02:01" - 30.hours | "30:00:00" - end - - with_them do - it { expect(duration_in_numbers(duration, true)).to eq formatted_string } - end + with_them do + it { expect(duration_in_numbers(duration)).to eq formatted_string } end end end -- cgit v1.2.1 From 8fe57d24a766dcff112fafed6932a9fc76274a42 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 10 Oct 2018 14:28:02 +0900 Subject: Add changelog --- .../unreleased/drop-allow_overflow-option-duration_in_numbers.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/drop-allow_overflow-option-duration_in_numbers.yml diff --git a/changelogs/unreleased/drop-allow_overflow-option-duration_in_numbers.yml b/changelogs/unreleased/drop-allow_overflow-option-duration_in_numbers.yml new file mode 100644 index 00000000000..4bece6459a0 --- /dev/null +++ b/changelogs/unreleased/drop-allow_overflow-option-duration_in_numbers.yml @@ -0,0 +1,5 @@ +--- +title: Drop `allow_overflow` option in `TimeHelper.duration_in_numbers` +merge_request: 52284 +author: +type: changed -- cgit v1.2.1 From fb685031d8c60ca1d8f607c569cd519875e873c9 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 12 Oct 2018 10:57:08 +0900 Subject: Fix spec --- spec/lib/gitlab/ci/status/build/scheduled_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/ci/status/build/scheduled_spec.rb b/spec/lib/gitlab/ci/status/build/scheduled_spec.rb index f98183d6d18..4a52b3ab8de 100644 --- a/spec/lib/gitlab/ci/status/build/scheduled_spec.rb +++ b/spec/lib/gitlab/ci/status/build/scheduled_spec.rb @@ -26,9 +26,9 @@ describe Gitlab::Ci::Status::Build::Scheduled do context 'when scheduled_at is expired' do let(:build) { create(:ci_build, :expired_scheduled, project: project) } - it 'shows 00:00:00' do + it 'shows 00:00' do Timecop.freeze do - expect(subject.status_tooltip).to include('00:00:00') + expect(subject.status_tooltip).to include('00:00') end end end -- cgit v1.2.1 From 105a8c177f7c2bfdd670180fe57d424d2c2de152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 18 Oct 2018 10:00:42 +0200 Subject: [QA] Fix the 'clone using deploy key' tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/assets/javascripts/jobs/components/job_app.vue | 6 +++--- app/assets/javascripts/jobs/components/job_log.vue | 2 +- .../vue_shared/components/ci_badge_link.vue | 2 +- qa/qa/page/project/job/show.rb | 25 +++++++++++++++------- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue index fa35b87ef2b..ba14aaeed2c 100644 --- a/app/assets/javascripts/jobs/components/job_app.vue +++ b/app/assets/javascripts/jobs/components/job_app.vue @@ -165,7 +165,7 @@