diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-24 09:11:46 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-11-24 09:11:46 +0000 |
commit | 8883c54feaf1ca2d5a859ecd634043b36dfb8832 (patch) | |
tree | 22cc323511d7fde2e3f3c50ba13b50817c5a7bb1 | |
parent | 555532c942a339983ab09730be3f1b72eaec38d1 (diff) | |
download | gitlab-ce-8883c54feaf1ca2d5a859ecd634043b36dfb8832.tar.gz |
Add latest changes from gitlab-org/gitlab@master
28 files changed, 682 insertions, 347 deletions
diff --git a/.gitlab/ci/build-images.gitlab-ci.yml b/.gitlab/ci/build-images.gitlab-ci.yml index a60a5f6040c..09ffc87f087 100644 --- a/.gitlab/ci/build-images.gitlab-ci.yml +++ b/.gitlab/ci/build-images.gitlab-ci.yml @@ -23,9 +23,6 @@ build-qa-image: script: - run_timed_command "scripts/build_qa_image" -# This image is used by: -# - The `CNG` pipelines (via the `review-build-cng` job): https://gitlab.com/gitlab-org/build/CNG/-/blob/cfc67136d711e1c8c409bf8e57427a644393da2f/.gitlab-ci.yml#L335 -# - The `omnibus-gitlab` pipelines (via the `e2e:package-and-test` job): https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/dfd1ad475868fc84e91ab7b5706aa03e46dc3a86/.gitlab-ci.yml#L130 build-assets-image: extends: - .base-image-build @@ -33,7 +30,11 @@ build-assets-image: stage: build-images needs: ["compile-production-assets"] script: - # TODO: Change the image tag to be the MD5 of assets files and skip image building if the image exists - # We'll also need to pass GITLAB_ASSETS_TAG to the trigerred omnibus-gitlab pipeline similarly to how we do it for trigerred CNG pipelines - # https://gitlab.com/gitlab-org/gitlab/issues/208389 + - skopeo login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY - run_timed_command "scripts/build_assets_image" + artifacts: + expire_in: 7 days + paths: + # The `cached-assets-hash.txt` file is used in `review-build-cng-env` (`.gitlab/ci/review-apps/main.gitlab-ci.yml`) + # to pass the assets image tag to the CNG downstream pipeline. + - cached-assets-hash.txt diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 6be77fe52c8..4d120de277a 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -28,6 +28,7 @@ fi fi - assets_compile_script + - echo -n "${GITLAB_ASSETS_HASH}" > "cached-assets-hash.txt" compile-production-assets: extends: @@ -43,6 +44,7 @@ compile-production-assets: # These assets are used in multiple locations: # - in `build-assets-image` job to create assets image for packaging systems # - GitLab UI for integration tests: https://gitlab.com/gitlab-org/gitlab-ui/-/blob/e88493b3c855aea30bf60baee692a64606b0eb1e/.storybook/preview-head.pug#L1 + - cached-assets-hash.txt - public/assets/ - "${WEBPACK_COMPILE_LOG_PATH}" when: always @@ -73,9 +75,6 @@ update-assets-compile-production-cache: - .assets-compile-cache-push - .shared:rules:update-cache stage: prepare - script: - - !reference [compile-production-assets, script] - - echo -n "${GITLAB_ASSETS_HASH}" > "cached-assets-hash.txt" artifacts: {} # This job's purpose is only to update the cache. update-assets-compile-test-cache: diff --git a/.gitlab/ci/package-and-test/main.gitlab-ci.yml b/.gitlab/ci/package-and-test/main.gitlab-ci.yml index 34c95d7637d..be210fac5a8 100644 --- a/.gitlab/ci/package-and-test/main.gitlab-ci.yml +++ b/.gitlab/ci/package-and-test/main.gitlab-ci.yml @@ -38,23 +38,6 @@ stages: extends: - .gitlab-qa-install -.omnibus-env: - variables: - BUILD_ENV: build.env - script: - - | - SECURITY_SOURCES=$([[ ! "$CI_PROJECT_NAMESPACE" =~ ^gitlab-org\/security ]] || echo "true") - echo "SECURITY_SOURCES=${SECURITY_SOURCES:-false}" > $BUILD_ENV - echo "OMNIBUS_GITLAB_CACHE_UPDATE=${OMNIBUS_GITLAB_CACHE_UPDATE:-false}" >> $BUILD_ENV - for version_file in *_VERSION; do echo "$version_file=$(cat $version_file)" >> $BUILD_ENV; done - echo "OMNIBUS_GITLAB_RUBY3_BUILD=${OMNIBUS_GITLAB_RUBY3_BUILD:-false}" >> $BUILD_ENV - echo "OMNIBUS_GITLAB_CACHE_EDITION=${OMNIBUS_GITLAB_CACHE_EDITION:-GITLAB}" >> $BUILD_ENV - echo "Built environment file for omnibus build:" - cat $BUILD_ENV - artifacts: - reports: - dotenv: $BUILD_ENV - .update-script: script: - export QA_COMMAND="bundle exec gitlab-qa Test::Omnibus::UpdateFromPrevious $RELEASE $GITLAB_VERSION $UPDATE_TYPE -- $QA_RSPEC_TAGS $RSPEC_REPORT_OPTS" @@ -108,9 +91,42 @@ dont-interrupt-me: trigger-omnibus-env: extends: - - .omnibus-env - .rules:omnibus-build stage: .pre + needs: + # We need this job because we need its `cached-assets-hash.txt` artifact, so that we can pass the assets image tag to the downstream omnibus-gitlab pipeline. + - pipeline: $PARENT_PIPELINE_ID + job: build-assets-image + variables: + BUILD_ENV: build.env + before_script: + - | + # This is duplicating the function from `scripts/utils.sh` since `.gitlab/ci/package-and-test/main.gitlab-ci.yml` can be included in other projects. + function assets_image_tag() { + local cache_assets_hash_file="cached-assets-hash.txt" + + if [[ -n "${CI_COMMIT_TAG}" ]]; then + echo -n "${CI_COMMIT_REF_NAME}" + elif [[ -f "${cache_assets_hash_file}" ]]; then + echo -n "assets-hash-$(cat ${cache_assets_hash_file} | cut -c1-10)" + else + echo -n "${CI_COMMIT_SHA}" + fi + } + script: + - | + SECURITY_SOURCES=$([[ ! "$CI_PROJECT_NAMESPACE" =~ ^gitlab-org\/security ]] || echo "true") + echo "SECURITY_SOURCES=${SECURITY_SOURCES:-false}" > $BUILD_ENV + echo "OMNIBUS_GITLAB_CACHE_UPDATE=${OMNIBUS_GITLAB_CACHE_UPDATE:-false}" >> $BUILD_ENV + for version_file in *_VERSION; do echo "$version_file=$(cat $version_file)" >> $BUILD_ENV; done + echo "OMNIBUS_GITLAB_RUBY3_BUILD=${OMNIBUS_GITLAB_RUBY3_BUILD:-false}" >> $BUILD_ENV + echo "OMNIBUS_GITLAB_CACHE_EDITION=${OMNIBUS_GITLAB_CACHE_EDITION:-GITLAB}" >> $BUILD_ENV + echo "GITLAB_ASSETS_TAG=$(assets_image_tag)" >> $BUILD_ENV + echo "Built environment file for omnibus build:" + cat $BUILD_ENV + artifacts: + reports: + dotenv: $BUILD_ENV trigger-omnibus: extends: .rules:omnibus-build @@ -128,6 +144,7 @@ trigger-omnibus: GITLAB_SHELL_VERSION: $GITLAB_SHELL_VERSION GITLAB_WORKHORSE_VERSION: $GITLAB_WORKHORSE_VERSION GITLAB_VERSION: $CI_COMMIT_SHA + GITLAB_ASSETS_TAG: $GITLAB_ASSETS_TAG IMAGE_TAG: $CI_COMMIT_SHA TOP_UPSTREAM_SOURCE_PROJECT: $CI_PROJECT_PATH SECURITY_SOURCES: $SECURITY_SOURCES diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index 8740a5fe17d..f6668d7864e 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -74,6 +74,8 @@ e2e:package-and-test: - build-qa-image - e2e-test-pipeline-generate variables: + # This is needed by `trigger-omnibus-env` (`.gitlab/ci/package-and-test/main.gitlab-ci.yml`). + PARENT_PIPELINE_ID: $CI_PIPELINE_ID SKIP_MESSAGE: Skipping package-and-test due to mr containing only quarantine changes! RELEASE: "${REGISTRY_HOST}/${REGISTRY_GROUP}/build/omnibus-gitlab-mirror/gitlab-ee:${CI_COMMIT_SHA}" GITLAB_QA_IMAGE: "${CI_REGISTRY_IMAGE}/gitlab-ee-qa:${CI_COMMIT_SHA}" diff --git a/.gitlab/ci/review-apps/main.gitlab-ci.yml b/.gitlab/ci/review-apps/main.gitlab-ci.yml index 8971d328179..27e2014baf1 100644 --- a/.gitlab/ci/review-apps/main.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/main.gitlab-ci.yml @@ -34,19 +34,25 @@ review-build-cng-env: - .review:rules:review-build-cng image: ${REGISTRY_HOST}/${REGISTRY_GROUP}/gitlab-build-images/debian-${DEBIAN_VERSION}-ruby-${RUBY_VERSION}:bundler-2.3 stage: prepare - needs: [] + needs: + # We need this job because we need its `cached-assets-hash.txt` artifact, so that we can pass the assets image tag to the downstream CNG pipeline. + - pipeline: $PARENT_PIPELINE_ID + job: build-assets-image + variables: + BUILD_ENV: build.env before_script: - source ./scripts/utils.sh - install_gitlab_gem script: - - ruby -r./scripts/trigger-build.rb -e "puts Trigger.variables_for_env_file(Trigger::CNG.new.variables)" > build.env + - 'ruby -r./scripts/trigger-build.rb -e "puts Trigger.variables_for_env_file(Trigger::CNG.new.variables)" > $BUILD_ENV' + - echo "GITLAB_ASSETS_TAG=$(assets_image_tag)" >> $BUILD_ENV - ruby -e 'puts "FULL_RUBY_VERSION=#{RUBY_VERSION}"' >> build.env - - cat build.env + - cat $BUILD_ENV artifacts: reports: - dotenv: build.env + dotenv: $BUILD_ENV paths: - - build.env + - $BUILD_ENV expire_in: 7 days when: always diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 3865fe0a6ed..be9ad5128f3 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -62,6 +62,8 @@ start-review-app-pipeline: # They need to be explicitly passed on to the child pipeline. # https://docs.gitlab.com/ee/ci/pipelines/multi_project_pipelines.html#pass-cicd-variables-to-a-downstream-pipeline-by-using-the-variables-keyword variables: + # This is needed by `review-build-cng-env` (`.gitlab/ci/review-apps/main.gitlab-ci.yml`). + PARENT_PIPELINE_ID: $CI_PIPELINE_ID SCHEDULE_TYPE: $SCHEDULE_TYPE DAST_RUN: $DAST_RUN SKIP_MESSAGE: Skipping review-app due to mr containing only quarantine changes! diff --git a/Dockerfile.assets b/Dockerfile.assets index 403d16cc4ab..ba69a614e88 100644 --- a/Dockerfile.assets +++ b/Dockerfile.assets @@ -1,4 +1,4 @@ # Simple container to store assets for later use FROM scratch -ADD public/assets /assets/ +COPY public/assets /assets/ CMD /bin/true diff --git a/doc/ci/pipelines/settings.md b/doc/ci/pipelines/settings.md index a7f3cfb59d3..e99644fb6ee 100644 --- a/doc/ci/pipelines/settings.md +++ b/doc/ci/pipelines/settings.md @@ -284,9 +284,8 @@ when merging a merge request would cause the project's test coverage to decline. Follow these steps to enable the `Coverage-Check` MR approval rule: 1. Set up a [`coverage`](../yaml/index.md#coverage) regular expression for all jobs you want to include in the overall coverage value. -1. Go to your project and select **Settings > General**. -1. Expand **Merge request approvals**. -1. Select **Enable** next to the `Coverage-Check` approval rule. +1. Go to your project and select **Settings > Merge requests**. +1. Under **Merge request approvals**, select **Enable** next to the `Coverage-Check` approval rule. 1. Select the **Target branch**. 1. Set the number of **Approvals required** to greater than zero. 1. Select the users or groups to provide approval. diff --git a/doc/development/database/efficient_in_operator_queries.md b/doc/development/database/efficient_in_operator_queries.md index fb7ff3c1cc2..0f65dcfef9e 100644 --- a/doc/development/database/efficient_in_operator_queries.md +++ b/doc/development/database/efficient_in_operator_queries.md @@ -160,7 +160,7 @@ The technique can only optimize `IN` queries that satisfy the following requirem in the following order: `column_for_the_in_query`, `order by column 1`, and `order by column 2`. - The columns in the `ORDER BY` clause are distinct - (the combination of the columns uniquely identifies one particular column in the table). + (the combination of the columns uniquely identifies one particular row in the table). WARNING: This technique does not improve the performance of the `COUNT(*)` queries. diff --git a/doc/development/directory_structure.md b/doc/development/directory_structure.md index 27384fa559f..34ee86d9ee5 100644 --- a/doc/development/directory_structure.md +++ b/doc/development/directory_structure.md @@ -1,94 +1,11 @@ --- -stage: none -group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +redirect_to: 'software_design.md' +remove_date: '2023-01-24' --- -# Backend directory structure +This document was moved to [another location](software_design.md) -## Use namespaces to define bounded contexts - -A healthy application is divided into macro and sub components that represent the contexts at play, -whether they are related to business domain or infrastructure code. - -As GitLab code has so many features and components it's hard to see what contexts are involved. -We should expect any class to be defined inside a module/namespace that represents the contexts where it operates. - -When we namespace classes inside their domain: - -- Similar terminology becomes unambiguous as the domain clarifies the meaning: - For example, `MergeRequests::Diff` and `Notes::Diff`. -- Top-level namespaces could be associated to one or more groups identified as domain experts. -- We can better identify the interactions and coupling between components. - For example, several classes inside `MergeRequests::` domain interact more with `Ci::` - domain and less with `ImportExport::`. - -```ruby -# bad -class MyClass -end - -# good -module MyDomain - class MyClass - end -end -``` - -### About namespace naming - -A good guideline for naming a top-level namespace (bounded context) is to use the related -[feature category](https://gitlab.com/gitlab-com/www-gitlab-com/-/blob/master/data/categories.yml). For example, `Continuous Integration` feature category maps to `Ci::` namespace. - -Alternatively a new class could be added to `Projects::` or `Groups::` if it's either: - -- Strictly related to one of these domains. For example `Projects::Alias`. -- A new component that does not have yet a more specific domain. In this case, when - a more explicit domain does emerge we would need to move the class to a more specific - namespace. - -Do not use the [stage or group name](https://about.gitlab.com/handbook/product/categories/#devops-stages) -since a feature category could be reassigned to a different group in the future. - -```ruby -# bad -module Create - class Commit - end -end - -# good -module Repositories - class Commit - end -end -``` - -On the other hand, a feature category may sometimes be too granular. Features tend to be -treated differently according to Product and Marketing, while they may share a lot of -domain models and behavior under the hood. In this case, having too many bounded contexts -could make them shallow and more coupled with other contexts. - -Bounded contexts (or top-level namespaces) can be seen as macro-components in the overall app. -Good bounded contexts should be [deep](https://medium.com/@nakabonne/depth-of-module-f62dac3c2fdb) -so consider having nested namespaces to further break down complex parts of the domain. -For example, `Ci::Config::`. - -For example, instead of having separate and granular bounded contexts like: `ContainerScanning::`, -`ContainerHostSecurity::`, `ContainerNetworkSecurity::`, we could have: - -```ruby -module ContainerSecurity - module HostSecurity - end - - module NetworkSecurity - end - - module Scanning - end -end -``` - -If classes that are defined into a namespace have a lot in common with classes in other namespaces, -chances are that these two namespaces are part of the same bounded context. +<!-- This redirect file can be deleted after <2023-01-24>. --> +<!-- Redirects that point to other docs in the same project expire in three months. --> +<!-- Redirects that point to docs in a different project or site (link is not relative and starts with `https:`) expire in one year. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/redirects.html --> diff --git a/doc/development/feature_development.md b/doc/development/feature_development.md index 2f6a5e25107..d839dcbdf77 100644 --- a/doc/development/feature_development.md +++ b/doc/development/feature_development.md @@ -19,7 +19,7 @@ Consult these topics for information on contributing to specific GitLab features ### General -- [Directory structure](directory_structure.md) +- [Software design guides](software_design.md) - [GitLab EventStore](event_store.md) to publish/subscribe to domain events - [GitLab utilities](utilities.md) - [Newlines style guide](newlines_styleguide.md) diff --git a/doc/development/i18n/externalization.md b/doc/development/i18n/externalization.md index 91e2efcb2a3..2269a28e496 100644 --- a/doc/development/i18n/externalization.md +++ b/doc/development/i18n/externalization.md @@ -284,9 +284,7 @@ expect(wrapper.text()).toEqual(MSG_ALERT_SETTINGS_FORM_ERROR); ### Dynamic translations -Sometimes there are dynamic translations that the parser can't find when running -`bin/rake gettext:find`. For these scenarios you can use the [`N_` method](https://github.com/grosser/gettext_i18n_rails/blob/c09e38d481e0899ca7d3fc01786834fa8e7aab97/Readme.md#unfound-translations-with-rake-gettextfind). -There's also an alternative method to [translate messages from validation errors](https://github.com/grosser/gettext_i18n_rails/blob/c09e38d481e0899ca7d3fc01786834fa8e7aab97/Readme.md#option-a). +For more details you can see how we [keep translations dynamic](#keep-translations-dynamic). ## Working with special content @@ -764,6 +762,10 @@ class MyPresenter end ``` +Sometimes there are dynamic translations that the parser can't find when running +`bin/rake gettext:find`. For these scenarios you can use the [`N_` method](https://github.com/grosser/gettext_i18n_rails/blob/c09e38d481e0899ca7d3fc01786834fa8e7aab97/Readme.md#unfound-translations-with-rake-gettextfind). +There's also an alternative method to [translate messages from validation errors](https://github.com/grosser/gettext_i18n_rails/blob/c09e38d481e0899ca7d3fc01786834fa8e7aab97/Readme.md#option-a). + ### Splitting sentences Never split a sentence, as it assumes the sentence's grammar and structure is the same in all diff --git a/doc/development/reusing_abstractions.md b/doc/development/reusing_abstractions.md index 58d1e20394c..e3f523fc6a7 100644 --- a/doc/development/reusing_abstractions.md +++ b/doc/development/reusing_abstractions.md @@ -198,7 +198,7 @@ Several base classes implement the service classes convention. You may consider - `BaseGroupService` for services scoped to groups. Classes that are not service objects should be -[created elsewhere](directory_structure.md#use-namespaces-to-define-bounded-contexts), +[created elsewhere](software_design.md#use-namespaces-to-define-bounded-contexts), such as in `lib`. #### ServiceResponse diff --git a/doc/development/software_design.md b/doc/development/software_design.md new file mode 100644 index 00000000000..03cbbb13d9f --- /dev/null +++ b/doc/development/software_design.md @@ -0,0 +1,141 @@ +--- +stage: none ++group: Engineering Productivity +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + +# Software design guides + +## Use ubiquitous language instead of CRUD terminology + +The code should use the same [ubiquitous language](https://about.gitlab.com/handbook/communication/#ubiquitous-language) +as used in the product and user documentation. Failure to use ubiquitous language correctly +can be a major cause of confusion for contributors and customers when there is constant translation +or use of multiple terms. +This also goes against our [communication strategy](https://about.gitlab.com/handbook/communication/#mecefu-terms). + +In the example below, [CRUD](https://en.wikipedia.org/wiki/Create,_read,_update_and_delete) +terminology introduces ambiguity. The name says we are creating an `epic_issues` +association record, but we are adding an existing issue to an epic. The name `epic_issues`, +used from Rails convention, leaks to higher abstractions such as service objects. +The code speaks the framework jargon rather than ubiquitous language. + +```ruby +# Bad +EpicIssues::CreateService +``` + +Using ubiquitous language makes the code clear and doesn't introduce any +cognitive load to a reader trying to translate the framework jargon. + +```ruby +# Good +Epic::AddExistingIssueService +``` + +You can use CRUD when representing simple concepts that are not ambiguous, +like creating a project, and when matching the existing ubiquitous language. + +```ruby +# OK: Matches the product language. +Projects::CreateService +``` + +New classes and database tables should use ubiquitous language. In this case the model name +and table name follow the Rails convention. + +Existing classes that don't follow ubiquitous language should be renamed, when possible. +Some low level abstractions such as the database tables don't need to be renamed. +For example, use `self.table_name=` when the model name diverges from the table name. + +We can allow exceptions only when renaming is challenging. For example, when the naming is used +for STI, exposed to the user, or if it would be a breaking change. + +## Use namespaces to define bounded contexts + +A healthy application is divided into macro and sub components that represent the contexts at play, +whether they are related to business domain or infrastructure code. + +As GitLab code has so many features and components it's hard to see what contexts are involved. +We should expect any class to be defined inside a module/namespace that represents the contexts where it operates. + +When we namespace classes inside their domain: + +- Similar terminology becomes unambiguous as the domain clarifies the meaning: + For example, `MergeRequests::Diff` and `Notes::Diff`. +- Top-level namespaces could be associated to one or more groups identified as domain experts. +- We can better identify the interactions and coupling between components. + For example, several classes inside `MergeRequests::` domain interact more with `Ci::` + domain and less with `ImportExport::`. + +A good guideline for naming a top-level namespace (bounded context) is to use the related +[feature category](https://gitlab.com/gitlab-com/www-gitlab-com/-/blob/master/data/categories.yml). +For example, `Continuous Integration` feature category maps to `Ci::` namespace. + +```ruby +# bad +class JobArtifact +end + +# good +module Ci + class JobArtifact + end +end +``` + +Projects and Groups are generally container concepts because they identify tenants. +They allow features to exist at the project or group level, like repositories or runners, +but do not nest such features under `Projects::` or `Groups::`. + +`Projects::` and `Groups::` namespaces should be used only for concepts that are strictly related to them: +for example `Project::CreateService` or `Groups::TransferService`. + +For controllers we allow `app/controllers/projects` and `app/controllers/groups` to be exceptions. +We use this convention to indicate the scope of a given web endpoint. + +Do not use the [stage or group name](https://about.gitlab.com/handbook/product/categories/#devops-stages) +because a feature category could be reassigned to a different group in the future. + +```ruby +# bad +module Create + class Commit + end +end + +# good +module Repositories + class Commit + end +end +``` + +On the other hand, a feature category may sometimes be too granular. Features tend to be +treated differently according to Product and Marketing, while they may share a lot of +domain models and behavior under the hood. In this case, having too many bounded contexts +could make them shallow and more coupled with other contexts. + +Bounded contexts (or top-level namespaces) can be seen as macro-components in the overall app. +Good bounded contexts should be [deep](https://medium.com/@nakabonne/depth-of-module-f62dac3c2fdb) +so consider having nested namespaces to further break down complex parts of the domain. +For example, `Ci::Config::`. + +For example, instead of having separate and granular bounded contexts like: `ContainerScanning::`, +`ContainerHostSecurity::`, `ContainerNetworkSecurity::`, we could have: + +```ruby +module ContainerSecurity + module HostSecurity + end + + module NetworkSecurity + end + + module Scanning + end +end +``` + +If classes that are defined into a namespace have a lot in common with classes in other namespaces, +chances are that these two namespaces are part of the same bounded context. diff --git a/doc/subscriptions/img/add-license.png b/doc/subscriptions/img/add-license.png Binary files differnew file mode 100644 index 00000000000..3efb318971c --- /dev/null +++ b/doc/subscriptions/img/add-license.png diff --git a/doc/subscriptions/index.md b/doc/subscriptions/index.md index aab4c87ce5b..98ce40c9e9b 100644 --- a/doc/subscriptions/index.md +++ b/doc/subscriptions/index.md @@ -155,9 +155,6 @@ For qualifying open source projects, the [GitLab for Open Source Program](https: #### Meeting GitLab for Open Source Program requirements -NOTE: -GitLab for Open Source Program benefits apply to an entire GitLab namespace. To qualify for the GitLab for Open Source Program, all projects in an applicant's namespace must meet program requirements. Applicants submit materials related to one project in the applying namespace, and the open source program team uses that project to verify eligibility of the entire namespace. - To meet GitLab for Open Source Program requirements, first add an OSI-approved open source license to all projects in your namespace. To add a license to a project: @@ -165,8 +162,13 @@ To add a license to a project: 1. On the top bar, select **Main menu > Projects** and find your project. 1. On the overview page, select **Add LICENSE**. If the license you want is not available as a license template, manually copy the entire, unaltered [text of your chosen license](https://opensource.org/licenses/alphabetical) into the `LICENSE` file. Note that GitLab defaults to **All rights reserved** if users do not perform this action. +![Add license](img/add-license.png) + Applicants must add the correct license to each project in their respective groups or namespaces. When you're sure you're using OSI-approved licenses for your projects, you can take your screenshots. +NOTE: +GitLab for Open Source Program benefits apply to an entire GitLab namespace. To qualify for the GitLab for Open Source Program, all projects in an applicant's namespace must meet program requirements. Applicants submit materials related to one project in the applying namespace, and the open source program team uses that project to verify eligibility of the entire namespace. + #### Verification for Open Source Program Next, take screenshots of your project to confirm that project's eligibility. You must upload three screenshots: diff --git a/doc/user/project/integrations/slack.md b/doc/user/project/integrations/slack.md index d34c558ebbc..2ded5799c23 100644 --- a/doc/user/project/integrations/slack.md +++ b/doc/user/project/integrations/slack.md @@ -36,10 +36,6 @@ to control GitLab from Slack. Slash commands are configured separately. - *To send messages to channels,* enter the Slack channel names, separated by commas. - *To send direct messages,* use the Member ID found in the user's Slack profile. - - NOTE: - Usernames and private channels are not supported. - 1. In **Webhook**, enter the webhook URL you copied in the [Slack configuration](#configure-slack) step. 1. Optional. In **Username**, enter the username of the Slack bot that sends diff --git a/lib/api/api.rb b/lib/api/api.rb index 00fac054547..ae830e13b91 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -226,6 +226,7 @@ module API mount ::API::Lint mount ::API::Markdown mount ::API::MergeRequestApprovals + mount ::API::MergeRequests mount ::API::MergeRequestDiffs mount ::API::Metadata mount ::API::Metrics::Dashboard::Annotations @@ -304,7 +305,6 @@ module API mount ::API::Labels mount ::API::MavenPackages mount ::API::Members - mount ::API::MergeRequests mount ::API::Notes mount ::API::NotificationSettings mount ::API::NpmInstancePackages diff --git a/lib/api/entities/issuable_time_stats.rb b/lib/api/entities/issuable_time_stats.rb index f93b4651b1f..717d2282441 100644 --- a/lib/api/entities/issuable_time_stats.rb +++ b/lib/api/entities/issuable_time_stats.rb @@ -7,12 +7,12 @@ module API Gitlab::TimeTrackingFormatter.output(time_spent) end - expose :time_estimate - expose :total_time_spent - expose :human_time_estimate + expose :time_estimate, documentation: { type: 'integer', example: 12600 } + expose :total_time_spent, documentation: { type: 'integer', example: 3600 } + expose :human_time_estimate, documentation: { type: 'string', example: '3h 30m' } with_options(format_with: :time_tracking_formatter) do - expose :total_time_spent, as: :human_total_time_spent + expose :total_time_spent, as: :human_total_time_spent, documentation: { type: 'string', example: '1h' } end # rubocop: disable CodeReuse/ActiveRecord diff --git a/lib/api/helpers/merge_requests_helpers.rb b/lib/api/helpers/merge_requests_helpers.rb index eed9fa30d3c..ee3bb49c97f 100644 --- a/lib/api/helpers/merge_requests_helpers.rb +++ b/lib/api/helpers/merge_requests_helpers.rb @@ -11,100 +11,107 @@ module API params :ee_approval_params do end - params :merge_requests_negatable_params do - optional :author_id, type: Integer, desc: 'Return merge requests which are authored by the user with the given ID' - optional :author_username, type: String, desc: 'Return merge requests which are authored by the user with the given username' + params :merge_requests_negatable_params do |options| + optional :author_id, type: Integer, + desc: "#{options[:prefix]}Returns merge requests created by the given user `id`. Mutually exclusive with `author_username`. Combine with `scope=all` or `scope=assigned_to_me`." + optional :author_username, type: String, + desc: "#{options[:prefix]}Returns merge requests created by the given `username`. Mutually exclusive with `author_id`." mutually_exclusive :author_id, :author_username - - optional :assignee_id, - types: [Integer, String], - integer_none_any: true, - desc: 'Return merge requests which are assigned to the user with the given ID' - optional :assignee_username, - type: Array[String], - check_assignees_count: true, - coerce_with: Validations::Validators::CheckAssigneesCount.coerce, - desc: 'Return merge requests which are assigned to the user with the given username' + optional :assignee_id, types: [Integer, String], + integer_none_any: true, + desc: "#{options[:prefix]}Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee." + optional :assignee_username, type: Array[String], + check_assignees_count: true, + coerce_with: Validations::Validators::CheckAssigneesCount.coerce, + desc: "#{options[:prefix]}Returns merge requests created by the given `username`. Mutually exclusive with `author_id`.", + documentation: { is_array: true } mutually_exclusive :assignee_id, :assignee_username - optional :reviewer_username, - type: String, - desc: 'Return merge requests which have the user as a reviewer with the given username' - - optional :labels, - type: Array[String], - coerce_with: Validations::Types::CommaSeparatedToArray.coerce, - desc: 'Comma-separated list of label names' - optional :milestone, type: String, desc: 'Return merge requests for a specific milestone' - optional :my_reaction_emoji, type: String, desc: 'Return issues reacted by the authenticated user by the given emoji' + optional :reviewer_username, type: String, + desc: "#{options[:prefix]}Returns merge requests which have the user as a reviewer with the given `username`. `None` returns merge requests with no reviewers. `Any` returns merge requests with any reviewer. Mutually exclusive with `reviewer_id`. Introduced in GitLab 13.8." + optional :labels, type: Array[String], + coerce_with: Validations::Types::CommaSeparatedToArray.coerce, + desc: "#{options[:prefix]}Returns merge requests matching a comma-separated list of labels. `None` lists all merge requests with no labels. `Any` lists all merge requests with at least one label. Predefined names are case-insensitive.", + documentation: { is_array: true } + optional :milestone, type: String, + desc: "#{options[:prefix]}Returns merge requests for a specific milestone. `None` returns merge requests with no milestone. `Any` returns merge requests that have an assigned milestone." + optional :my_reaction_emoji, type: String, + desc: "#{options[:prefix]}Returns merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction." end params :merge_requests_base_params do - use :merge_requests_negatable_params - optional :reviewer_id, - types: [Integer, String], - integer_none_any: true, - desc: 'Return merge requests which have the user as a reviewer with the given ID' + use :merge_requests_negatable_params, prefix: '' + + optional :reviewer_id, types: [Integer, String], + integer_none_any: true, + desc: 'Returns merge requests which have the user as a reviewer with the given user `id`. `None` returns merge requests with no reviewers. `Any` returns merge requests with any reviewer. Mutually exclusive with `reviewer_username`.' mutually_exclusive :reviewer_id, :reviewer_username - optional :state, - type: String, - values: %w[opened closed locked merged all], - default: 'all', - desc: 'Return opened, closed, locked, merged, or all merge requests' - optional :order_by, - type: String, - values: Helpers::MergeRequestsHelpers.sort_options, - default: 'created_at', - desc: "Return merge requests ordered by #{Helpers::MergeRequestsHelpers.sort_options_help} fields." - optional :sort, - type: String, - values: %w[asc desc], - default: 'desc', - desc: 'Return merge requests sorted in `asc` or `desc` order.' - optional :with_labels_details, type: Boolean, desc: 'Return titles of labels and other details', default: false - optional :with_merge_status_recheck, type: Boolean, desc: 'Request that stale merge statuses be rechecked asynchronously', default: false - optional :created_after, type: DateTime, desc: 'Return merge requests created after the specified time' - optional :created_before, type: DateTime, desc: 'Return merge requests created before the specified time' - optional :updated_after, type: DateTime, desc: 'Return merge requests updated after the specified time' - optional :updated_before, type: DateTime, desc: 'Return merge requests updated before the specified time' - optional :view, - type: String, - values: %w[simple], - desc: 'If simple, returns the `iid`, URL, title, description, and basic state of merge request' - - optional :scope, - type: String, - values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all], - desc: 'Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`' - optional :source_branch, type: String, desc: 'Return merge requests with the given source branch' - optional :source_project_id, type: Integer, desc: 'Return merge requests with the given source project id' - optional :target_branch, type: String, desc: 'Return merge requests with the given target branch' - optional :search, - type: String, - desc: 'Search merge requests for text present in the title, description, or any combination of these' - optional :in, type: String, desc: '`title`, `description`, or a string joining them with comma' - optional :wip, type: String, values: %w[yes no], desc: 'Search merge requests for WIP in the title' - optional :not, type: Hash, desc: 'Parameters to negate' do - use :merge_requests_negatable_params - optional :reviewer_id, - types: Integer, - desc: 'Return merge requests which have the user as a reviewer with the given ID' + optional :state, type: String, + values: %w[opened closed locked merged all], + default: 'all', + desc: 'Returns `all` merge requests or just those that are `opened`, `closed`, `locked`, or `merged`.' + optional :order_by, type: String, + values: Helpers::MergeRequestsHelpers.sort_options, + default: 'created_at', + desc: "Returns merge requests ordered by #{Helpers::MergeRequestsHelpers.sort_options_help} fields. Introduced in GitLab 14.8." + optional :sort, type: String, + values: %w[asc desc], + default: 'desc', + desc: 'Returns merge requests sorted in `asc` or `desc` order.' + optional :with_labels_details, type: Boolean, + default: false, + desc: 'If `true`, response returns more details for each label in labels field: `:name`,`:color`, `:description`, `:description_html`, `:text_color`' + optional :with_merge_status_recheck, type: Boolean, + default: false, + desc: 'If `true`, this projection requests (but does not guarantee) that the `merge_status` field be recalculated asynchronously. Introduced in GitLab 13.0.' + optional :created_after, type: DateTime, + desc: 'Returns merge requests created on or after the given time. Expected in ISO 8601 format.', + documentation: { example: '2019-03-15T08:00:00Z' } + optional :created_before, type: DateTime, + desc: 'Returns merge requests created on or before the given time. Expected in ISO 8601 format.', + documentation: { example: '2019-03-15T08:00:00Z' } + optional :updated_after, type: DateTime, + desc: 'Returns merge requests updated on or after the given time. Expected in ISO 8601 format.', + documentation: { example: '2019-03-15T08:00:00Z' } + optional :updated_before, type: DateTime, + desc: 'Returns merge requests updated on or before the given time. Expected in ISO 8601 format.', + documentation: { example: '2019-03-15T08:00:00Z' } + optional :view, type: String, + values: %w[simple], + desc: 'If simple, returns the `iid`, URL, title, description, and basic state of merge request' + optional :scope, type: String, + values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all], + desc: 'Returns merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`' + optional :source_branch, type: String, desc: 'Returns merge requests with the given source branch' + optional :source_project_id, type: Integer, desc: 'Returns merge requests with the given source project id' + optional :target_branch, type: String, desc: 'Returns merge requests with the given target branch' + optional :search, type: String, + desc: 'Search merge requests against their `title` and `description`.' + optional :in, type: String, + desc: 'Modify the scope of the search attribute. `title`, `description`, or a string joining them with comma.', + documentation: { example: 'title,description' } + optional :wip, type: String, + values: %w[yes no], + desc: 'Filter merge requests against their `wip` status. `yes` to return only draft merge requests, `no` to return non-draft merge requests.' + optional :not, type: Hash, desc: 'Returns merge requests that do not match the parameters supplied' do + use :merge_requests_negatable_params, prefix: '`<Negated>` ' + + optional :reviewer_id, types: Integer, + desc: '`<Negated>` Returns merge requests which have the user as a reviewer with the given user `id`. `None` returns merge requests with no reviewers. `Any` returns merge requests with any reviewer. Mutually exclusive with `reviewer_username`.' mutually_exclusive :reviewer_id, :reviewer_username end - - optional :deployed_before, - 'Return merge requests deployed before the given date/time' - optional :deployed_after, - 'Return merge requests deployed after the given date/time' - optional :environment, - 'Returns merge requests deployed to the given environment' + optional :deployed_before, desc: 'Returns merge requests deployed before the given date/time. Expected in ISO 8601 format.', + documentation: { example: '2019-03-15T08:00:00Z' } + optional :deployed_after, desc: 'Returns merge requests deployed after the given date/time. Expected in ISO 8601 format', + documentation: { example: '2019-03-15T08:00:00Z' } + optional :environment, desc: 'Returns merge requests deployed to the given environment', + documentation: { example: '2019-03-15T08:00:00Z' } end params :optional_scope_param do - optional :scope, - type: String, - values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all], - default: 'created_by_me', - desc: 'Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`' + optional :scope, type: String, + values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all], + default: 'created_by_me', + desc: 'Returns merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`' end def handle_merge_request_errors!(merge_request) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index bb2861aa221..de52f3405e3 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -134,7 +134,13 @@ module API resource :merge_requests do desc 'List merge requests' do + detail 'Get all merge requests the authenticated user has access to. By default it returns only merge requests created by the current user. To get all merge requests, use parameter `scope=all`.' success Entities::MergeRequestBasic + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 422, message: 'Unprocessable entity' } + ] + tags %w[merge_requests] end params do use :merge_requests_params @@ -150,16 +156,24 @@ module API end params do - requires :id, type: String, desc: 'The ID of a group' + requires :id, type: String, desc: 'The ID or URL-encoded path of the group owned by the authenticated user.' end resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do - desc 'Get a list of group merge requests' do + desc 'List group merge requests' do + detail 'Get all merge requests for this group and its subgroups.' success Entities::MergeRequestBasic + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' }, + { code: 422, message: 'Unprocessable entity' } + ] + tags %w[merge_requests] end params do use :merge_requests_params - optional :non_archived, type: Boolean, desc: 'Return merge requests from non archived projects', - default: true + optional :non_archived, type: Boolean, + default: true, + desc: 'Returns merge requests from non archived projects only.' end get ":id/merge_requests", feature_category: :code_review, urgency: :low do validate_anonymous_search_access! if declared_params[:search].present? @@ -170,36 +184,62 @@ module API end params do - requires :id, types: [String, Integer], desc: 'The ID or URL-encoded path of the project' + requires :id, types: [String, Integer], desc: 'The ID or URL-encoded path of the project.' end resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do include TimeTrackingEndpoints helpers do params :optional_params do - optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request' - optional :assignee_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Comma-separated list of assignee ids' - optional :reviewer_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Comma-separated list of reviewer ids' - optional :description, type: String, desc: 'The description of the merge request' - optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' - optional :add_labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' - optional :remove_labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' - optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request' - optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' - optional :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch' + optional :assignee_id, type: Integer, desc: 'Assignee user ID.' + optional :assignee_ids, type: Array[Integer], + coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, + desc: 'The IDs of the users to assign the merge request to, as a comma-separated list. Set to 0 or provide an empty value to unassign all assignees.', + documentation: { is_array: true } + optional :reviewer_ids, type: Array[Integer], + coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, + desc: 'The IDs of the users to review the merge request, as a comma-separated list. Set to 0 or provide an empty value to unassign all reviewers.', + documentation: { is_array: true } + optional :description, type: String, desc: 'Description of the merge request. Limited to 1,048,576 characters.' + optional :labels, type: Array[String], + coerce_with: Validations::Types::CommaSeparatedToArray.coerce, + desc: 'Comma-separated label names for a merge request. Set to an empty string to unassign all labels.', + documentation: { is_array: true } + optional :add_labels, type: Array[String], + coerce_with: Validations::Types::CommaSeparatedToArray.coerce, + desc: 'Comma-separated label names to add to a merge request.', + documentation: { is_array: true } + optional :remove_labels, type: Array[String], + coerce_with: Validations::Types::CommaSeparatedToArray.coerce, + desc: 'Comma-separated label names to remove from a merge request.', + documentation: { is_array: true } + optional :milestone_id, type: Integer, desc: 'The global ID of a milestone to assign the merge reques to.' + optional :remove_source_branch, type: Boolean, desc: 'Flag indicating if a merge request should remove the source branch when merging.' + optional :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch.' optional :allow_maintainer_to_push, type: Boolean, as: :allow_collaboration, desc: '[deprecated] See allow_collaboration' - optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge' + optional :squash, type: Grape::API::Boolean, desc: 'Squash commits into a single commit when merging.' use :optional_params_ee end end - desc 'List merge requests' do + desc 'List project merge requests' do + detail 'Get all merge requests for this project.' success Entities::MergeRequestBasic + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' }, + { code: 422, message: 'Unprocessable entity' } + ] + tags %w[merge_requests] end params do use :merge_requests_params - optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IID array of merge requests' + + optional :iids, type: Array[Integer], + coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, + desc: 'Returns the request having the given `iid`.', + documentation: { is_array: true } end get ":id/merge_requests", feature_category: :code_review, urgency: :low do authorize! :read_merge_request, user_project @@ -226,15 +266,24 @@ module API **options end - desc 'Create a merge request' do + desc 'Create merge request' do + detail 'Create a new merge request.' + failure [ + { code: 400, message: 'Bad request' }, + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' }, + { code: 409, message: 'Conflict' }, + { code: 422, message: 'Unprocessable entity' } + ] success Entities::MergeRequest + tags %w[merge_requests] end params do - requires :title, type: String, desc: 'The title of the merge request' - requires :source_branch, type: String, desc: 'The source branch' - requires :target_branch, type: String, desc: 'The target branch' + requires :title, type: String, desc: 'The title of the merge request.' + requires :source_branch, type: String, desc: 'The source branch.' + requires :target_branch, type: String, desc: 'The target branch.' optional :target_project_id, type: Integer, - desc: 'The target project of the merge request defaults to the :id of the project' + desc: 'The target project of the merge request defaults to the :id of the project.' use :optional_params end post ":id/merge_requests", feature_category: :code_review, urgency: :low do @@ -253,9 +302,17 @@ module API present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project end - desc 'Delete a merge request' + desc 'Delete a merge request' do + detail 'Only for administrators and project owners. Deletes the merge request in question. ' + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' }, + { code: 412, message: 'Precondition failed' } + ] + tags %w[merge_requests] + end params do - requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request' + requires :merge_request_iid, type: Integer, desc: 'The internal ID of the merge request.' end delete ":id/merge_requests/:merge_request_iid", feature_category: :code_review, urgency: :low do merge_request = find_project_merge_request(params[:merge_request_iid]) @@ -268,13 +325,19 @@ module API end params do - requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request' - optional :render_html, type: Boolean, desc: 'Returns the description and title rendered HTML' - optional :include_diverged_commits_count, type: Boolean, desc: 'Returns the commits count behind the target branch' - optional :include_rebase_in_progress, type: Boolean, desc: 'Returns whether a rebase operation is ongoing ' + requires :merge_request_iid, type: Integer, desc: 'The internal ID of the merge request.' + optional :render_html, type: Boolean, desc: 'If `true`, response includes rendered HTML for title and description.' + optional :include_diverged_commits_count, type: Boolean, desc: 'If `true`, response includes the commits behind the target branch.' + optional :include_rebase_in_progress, type: Boolean, desc: 'If `true`, response includes whether a rebase operation is in progress.' end - desc 'Get a single merge request' do + desc 'Get single merge request' do + detail 'Shows information about a single merge request. Note: the `changes_count` value in the response is a string, not an integer. This is because when an merge request has too many changes to display and store, it is capped at 1,000. In that case, the API returns the string `"1000+"` for the changes count.' + success Entities::MergeRequest + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end get ':id/merge_requests/:merge_request_iid', feature_category: :code_review, urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -289,8 +352,13 @@ module API include_rebase_in_progress: params[:include_rebase_in_progress] end - desc 'Get the participants of a merge request' do + desc 'Get single merge request participants' do + detail 'Get a list of merge request participants.' success Entities::UserBasic + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end get ':id/merge_requests/:merge_request_iid/participants', feature_category: :code_review, urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -300,8 +368,13 @@ module API present paginate(participants), with: Entities::UserBasic end - desc 'Get the reviewers of a merge request' do + desc 'Get single merge request reviewers' do + detail 'Get a list of merge request reviewers.' success Entities::MergeRequestReviewer + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end get ':id/merge_requests/:merge_request_iid/reviewers', feature_category: :code_review, urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -311,8 +384,13 @@ module API present paginate(reviewers), with: Entities::MergeRequestReviewer end - desc 'Get the commits of a merge request' do + desc 'Get single merge request commits' do + detail 'Get a list of merge request commits.' success Entities::Commit + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end get ':id/merge_requests/:merge_request_iid/commits', feature_category: :code_review, urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -324,8 +402,13 @@ module API present commits, with: Entities::Commit end - desc 'Get the context commits of a merge request' do + desc 'List merge request context commits' do + detail 'Get a list of merge request context commits.' success Entities::Commit + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end get ':id/merge_requests/:merge_request_iid/context_commits', feature_category: :code_review, urgency: :high do merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -336,10 +419,20 @@ module API end params do - requires :commits, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, allow_blank: false, desc: 'List of context commits sha' - end - desc 'create context commits of merge request' do + requires :commits, type: Array[String], + coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, + allow_blank: false, + desc: 'The context commits’ SHA.', + documentation: { is_array: true } + end + desc 'Create merge request context commits' do + detail 'Create a list of merge request context commits.' success Entities::Commit + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end post ':id/merge_requests/:merge_request_iid/context_commits', feature_category: :code_review do commit_ids = params[:commits] @@ -363,9 +456,21 @@ module API end params do - requires :commits, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, allow_blank: false, desc: 'List of context commits sha' + requires :commits, type: Array[String], + coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, + allow_blank: false, + desc: 'The context commits’ SHA.', + documentation: { is_array: true } + end + desc 'Delete merge request context commits' do + detail 'Delete a list of merge request context commits.' + failure [ + { code: 400, message: 'Bad request' }, + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end - desc 'remove context commits of merge request' delete ':id/merge_requests/:merge_request_iid/context_commits', feature_category: :code_review do commit_ids = params[:commits] merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -382,8 +487,13 @@ module API status 204 end - desc 'Show the merge request changes' do + desc 'Get single merge request changes' do + detail 'Shows information about the merge request including its files and changes.' success Entities::MergeRequestChanges + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end get ':id/merge_requests/:merge_request_iid/changes', feature_category: :code_review, urgency: :low do merge_request = find_merge_request_with_access(params[:merge_request_iid]) @@ -395,17 +505,28 @@ module API access_raw_diffs: to_boolean(params.fetch(:access_raw_diffs, false)) end - desc 'Get the merge request pipelines' do + desc 'Get single merge request pipelines' do + detail 'Get a list of merge request pipelines.' success Entities::Ci::PipelineBasic + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end get ':id/merge_requests/:merge_request_iid/pipelines', urgency: :low, feature_category: :continuous_integration do pipelines = merge_request_pipelines_with_access - present paginate(pipelines), with: Entities::Ci::PipelineBasic end - desc 'Create a pipeline for merge request' do + desc 'Create merge request pipeline' do + detail 'Create a new pipeline for a merge request. A pipeline created via this endpoint doesn’t run a regular branch/tag pipeline. It requires `.gitlab-ci.yml` to be configured with `only: [merge_requests]` to create jobs.' success ::API::Entities::Ci::Pipeline + failure [ + { code: 400, message: 'Bad request' }, + { code: 404, message: 'Not found' }, + { code: 405, message: 'Method not allowed' } + ] + tags %w[merge_requests] end post ':id/merge_requests/:merge_request_iid/pipelines', urgency: :low, feature_category: :continuous_integration do pipeline = ::MergeRequests::CreatePipelineService @@ -423,15 +544,25 @@ module API end end - desc 'Update a merge request' do + desc 'Update merge request' do + detail 'Updates an existing merge request. You can change the target branch, title, or even close the merge request.' success Entities::MergeRequest + failure [ + { code: 400, message: 'Bad request' }, + { code: 404, message: 'Not found' }, + { code: 409, message: 'Conflict' }, + { code: 422, message: 'Unprocessable entity' } + ] + tags %w[merge_requests] end params do - optional :title, type: String, allow_blank: false, desc: 'The title of the merge request' - optional :target_branch, type: String, allow_blank: false, desc: 'The target branch' - optional :state_event, type: String, values: %w[close reopen], - desc: 'Status of the merge request' - optional :discussion_locked, type: Boolean, desc: 'Whether the MR discussion is locked' + optional :title, type: String, allow_blank: false, desc: 'The title of the merge request.' + optional :target_branch, type: String, allow_blank: false, desc: 'The target branch.' + optional :state_event, type: String, + values: %w[close reopen], + desc: 'New state (close/reopen).' + optional :discussion_locked, type: Boolean, + desc: 'Flag indicating if the merge request’s discussion is locked. If the discussion is locked only project members can add, edit or resolve comments.' use :optional_params at_least_one_of(*::API::MergeRequests.update_params_at_least_one_of) @@ -456,17 +587,27 @@ module API end desc 'Merge a merge request' do + detail 'Accept and merge changes submitted with the merge request using this API.' success Entities::MergeRequest + failure [ + { code: 400, message: 'Bad request' }, + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' }, + { code: 405, message: 'Method not allowed' }, + { code: 409, message: 'Conflict' }, + { code: 422, message: 'Unprocessable entity' } + ] + tags %w[merge_requests] end params do - optional :merge_commit_message, type: String, desc: 'Custom merge commit message' - optional :squash_commit_message, type: String, desc: 'Custom squash commit message' + optional :merge_commit_message, type: String, desc: 'Custom merge commit message.' + optional :squash_commit_message, type: String, desc: 'Custom squash commit message.' optional :should_remove_source_branch, type: Boolean, - desc: 'When true, the source branch will be deleted if possible' + desc: 'If `true`, removes the source branch.' optional :merge_when_pipeline_succeeds, type: Boolean, - desc: 'When true, this merge request will be merged when the pipeline succeeds' - optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch' - optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge' + desc: 'If `true`, the merge request is merged when the pipeline succeeds.' + optional :sha, type: String, desc: 'If present, then this SHA must match the HEAD of the source branch, otherwise the merge fails.' + optional :squash, type: Grape::API::Boolean, desc: 'If `true`, the commits are squashed into a single commit on merge.' end put ':id/merge_requests/:merge_request_iid/merge', feature_category: :code_review, urgency: :low do Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/4796') @@ -512,7 +653,13 @@ module API end end - desc 'Returns the up to date merge-ref HEAD commit' + desc 'Returns the up to date merge-ref HEAD commit' do + detail 'Returns the up to date merge-ref HEAD commit' + failure [ + { code: 400, message: 'Bad request' } + ] + tags %w[merge_requests] + end get ':id/merge_requests/:merge_request_iid/merge_ref', feature_category: :code_review do merge_request = find_project_merge_request(params[:merge_request_iid]) @@ -525,8 +672,16 @@ module API end end - desc 'Cancel merge if "Merge When Pipeline Succeeds" is enabled' do + desc 'Cancel Merge When Pipeline Succeeds' do + detail 'Cancel merge if "Merge When Pipeline Succeeds" is enabled' success Entities::MergeRequest + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' }, + { code: 405, message: 'Method not allowed' }, + { code: 406, message: 'Not acceptable' } + ] + tags %w[merge_requests] end post ':id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds', feature_category: :code_review do merge_request = find_project_merge_request(params[:merge_request_iid]) @@ -536,11 +691,17 @@ module API AutoMergeService.new(merge_request.target_project, current_user).cancel(merge_request) end - desc 'Rebase the merge request against its target branch' do - detail 'This feature was added in GitLab 11.6' + desc 'Rebase a merge request' do + detail 'Automatically rebase the `source_branch` of the merge request against its `target_branch`. This feature was added in GitLab 11.6' + failure [ + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not found' }, + { code: 409, message: 'Conflict' } + ] + tags %w[merge_requests] end params do - optional :skip_ci, type: Boolean, desc: 'Do not create CI pipeline' + optional :skip_ci, type: Boolean, desc: 'Set to true to skip creating a CI pipeline.' end put ':id/merge_requests/:merge_request_iid/rebase', feature_category: :code_review, urgency: :low do merge_request = find_project_merge_request(params[:merge_request_iid]) @@ -555,8 +716,13 @@ module API render_api_error!(e.message, 409) end - desc 'Remove merge request approvals' do - detail 'This feature was added in GitLab 15.4' + desc 'Reset approvals of a merge request' do + detail 'Clear all approvals of merge request. This feature was added in GitLab 15.4' + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end put ':id/merge_requests/:merge_request_iid/reset_approvals', feature_category: :code_review, urgency: :low do merge_request = find_project_merge_request(params[:merge_request_iid]) @@ -568,8 +734,13 @@ module API status :accepted end - desc 'List issues that will be closed on merge' do + desc 'List issues that close on merge' do + detail 'Get all the issues that would be closed by merging the provided merge request.' success Entities::MRNote + failure [ + { code: 404, message: 'Not found' } + ] + tags %w[merge_requests] end params do use :pagination diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb index b8323304957..20f4b2988eb 100644 --- a/lib/api/time_tracking_endpoints.rb +++ b/lib/api/time_tracking_endpoints.rb @@ -54,10 +54,18 @@ module API issuable_collection_name = issuable_name.pluralize issuable_key = "#{issuable_name}_iid".to_sym - desc "Set a time estimate for a project #{issuable_name}" + desc "Set a time estimate for a #{issuable_name}" do + detail " Sets an estimated time of work for this #{issuable_name}." + success Entities::IssuableTimeStats + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags [issuable_collection_name] + end params do - requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" - requires :duration, type: String, desc: 'The duration to be parsed' + requires issuable_key, type: Integer, desc: "The internal ID of the #{issuable_name}." + requires :duration, type: String, desc: 'The duration in human format.', documentation: { example: '3h30m' } end post ":id/#{issuable_collection_name}/:#{issuable_key}/time_estimate" do authorize! admin_issuable_key, load_issuable @@ -66,9 +74,17 @@ module API update_issuable(time_estimate: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration))) end - desc "Reset the time estimate for a project #{issuable_name}" + desc "Reset the time estimate for a project #{issuable_name}" do + detail "Resets the estimated time for this #{issuable_name} to 0 seconds." + success Entities::IssuableTimeStats + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags [issuable_collection_name] + end params do - requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + requires issuable_key, type: Integer, desc: "The internal ID of the #{issuable_name}." end post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_time_estimate" do authorize! admin_issuable_key, load_issuable @@ -77,10 +93,18 @@ module API update_issuable(time_estimate: 0) end - desc "Add spent time for a project #{issuable_name}" + desc "Add spent time for a #{issuable_name}" do + detail "Adds spent time for this #{issuable_name}." + success Entities::IssuableTimeStats + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags [issuable_collection_name] + end params do - requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" - requires :duration, type: String, desc: 'The duration to be parsed' + requires issuable_key, type: Integer, desc: "The internal ID of the #{issuable_name}." + requires :duration, type: String, desc: 'The duration in human format.' end post ":id/#{issuable_collection_name}/:#{issuable_key}/add_spent_time" do authorize! admin_issuable_key, load_issuable @@ -97,9 +121,17 @@ module API update_issuable(update_params) end - desc "Reset spent time for a project #{issuable_name}" + desc "Reset spent time for a #{issuable_name}" do + detail "Resets the total spent time for this #{issuable_name} to 0 seconds." + success Entities::IssuableTimeStats + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags [issuable_collection_name] + end params do - requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + requires issuable_key, type: Integer, desc: "The internal ID of the #{issuable_name}" end post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_spent_time" do authorize! admin_issuable_key, load_issuable @@ -108,9 +140,17 @@ module API update_issuable(spend_time: { duration: :reset, user_id: current_user.id }) end - desc "Show time stats for a project #{issuable_name}" + desc "Get time tracking stats" do + detail "Get time tracking stats" + success Entities::IssuableTimeStats + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + tags [issuable_collection_name] + end params do - requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}" + requires issuable_key, type: Integer, desc: "The internal ID of the #{issuable_name}" end get ":id/#{issuable_collection_name}/:#{issuable_key}/time_stats" do authorize! read_issuable_key, load_issuable diff --git a/qa/Gemfile b/qa/Gemfile index 2eb3bf7e4f5..c9498085196 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -21,7 +21,7 @@ gem 'rotp', '~> 6.2.1' gem 'parallel', '~> 1.22', '>= 1.22.1' gem 'rainbow', '~> 3.1.1' gem 'rspec-parameterized', '~> 0.5.2' -gem 'octokit', '~> 6.0.0' +gem 'octokit', '~> 6.0.1' gem "faraday-retry", "~> 2.0" gem 'webdrivers', '~> 5.2' gem 'zeitwerk', '~> 2.4' diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index e7578c2c080..1742f6086a3 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -181,7 +181,7 @@ GEM nokogiri (1.13.9) mini_portile2 (~> 2.8.0) racc (~> 1.4) - octokit (6.0.0) + octokit (6.0.1) faraday (>= 1, < 3) sawyer (~> 0.9) oj (3.13.23) @@ -315,7 +315,7 @@ DEPENDENCIES influxdb-client (~> 2.8) knapsack (~> 4.0) nokogiri (~> 1.13, >= 1.13.9) - octokit (~> 6.0.0) + octokit (~> 6.0.1) parallel (~> 1.22, >= 1.22.1) parallel_tests (~> 4.0) pry-byebug (~> 3.10.1) diff --git a/scripts/build_assets_image b/scripts/build_assets_image index 8aa6526061a..ee8623c826e 100755 --- a/scripts/build_assets_image +++ b/scripts/build_assets_image @@ -1,36 +1,75 @@ +#!/bin/sh + +set -e + +# This script builds an image that contains assets, that's then used by: +# - The `CNG` downstream pipelines (triggered from `gitlab-org/gitlab` via the `review-build-cng` job): +# https://gitlab.com/gitlab-org/gitlab/-/blob/c34e0834b01cd45c1f69a01b5e38dd6bc505f903/.gitlab/ci/review-apps/main.gitlab-ci.yml#L69. +# - The `omnibus-gitlab` downstream pipelines (triggered from `gitlab-org/gitlab` via the `e2e:package-and-test` job): +# https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/dfd1ad475868fc84e91ab7b5706aa03e46dc3a86/.gitlab-ci.yml#L130. +# - The `gitlab-org/charts/gitlab` `master` pipelines via `gitlab-org/build/CNG`, +# which pull `registry.gitlab.com/gitlab-org/gitlab/gitlab-assets-ee:master` +# - The `omnibus-gitlab` and CNG `master`/stable-branch pipelines, for both gitlab.com and dev.gitlab.org, +# which pull `registry.gitlab.com/gitlab-org/gitlab/gitlab-assets-ee:${CI_COMMIT_REF_SLUG}`. +# - The `omnibus-gitlab` tag pipelines, for both gitlab.com and dev.gitlab.org, +# which pull `registry.gitlab.com/gitlab-org/gitlab/gitlab-assets-ee:${CI_COMMIT_REF_SLUG}`. +# - The CNG tag pipelines, for both gitlab.com and dev.gitlab.org, +# which pull `registry.gitlab.com/gitlab-org/gitlab/gitlab-assets-ee:${CI_COMMIT_REF_NAME}`. +# - The auto-deploy pipelines, which pull `registry.gitlab.com/gitlab-org/gitlab/gitlab-assets-ee:${CI_COMMIT_SHA}`. + +. scripts/utils.sh + # Exit early if we don't want to build the image -if [[ "${BUILD_ASSETS_IMAGE}" != "true" ]] +if [ "${BUILD_ASSETS_IMAGE}" != "true" ] then exit 0 fi # Generate the image name based on the project this is being run in ASSETS_IMAGE_NAME="gitlab-assets-ce" + # `dev.gitlab-org` still has gitlab-ee. -if [[ "${CI_PROJECT_NAME}" == "gitlab" ]] || [[ "${CI_PROJECT_NAME}" == "gitlab-ee" ]] -then +if [ "${CI_PROJECT_NAME}" = "gitlab" ] || [ "${CI_PROJECT_NAME}" = "gitlab-ee" ]; then ASSETS_IMAGE_NAME="gitlab-assets-ee" fi -ASSETS_IMAGE_PATH=${CI_REGISTRY}/${CI_PROJECT_PATH}/${ASSETS_IMAGE_NAME} +ASSETS_IMAGE_PATH="${CI_REGISTRY}/${CI_PROJECT_PATH}/${ASSETS_IMAGE_NAME}" -mkdir -p assets_container.build/public -cp -r public/assets assets_container.build/public/ -cp Dockerfile.assets assets_container.build/ +# Used in MR pipelines +COMMIT_ASSETS_HASH_DESTINATION="${ASSETS_IMAGE_PATH}:$(assets_image_tag)" +# Used by other projects's master pipelines +COMMIT_REF_SLUG_DESTINATION="${ASSETS_IMAGE_PATH}:${CI_COMMIT_REF_SLUG}" +# Used by auto-deploy pipelines: https://gitlab.com/gitlab-org/release/docs/blob/master/general/deploy/auto-deploy.md +COMMIT_SHA_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_SHA} +# Used for CNG tag pipelines +COMMIT_REF_NAME_DESTINATION="${ASSETS_IMAGE_PATH}:${CI_COMMIT_REF_NAME}" -COMMIT_REF_SLUG_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_REF_SLUG} +if skopeo inspect "docker://${COMMIT_ASSETS_HASH_DESTINATION}" > /dev/null; then + echosuccess "Image ${COMMIT_ASSETS_HASH_DESTINATION} already exists, no need to rebuild it." -COMMIT_SHA_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_SHA} -COMMIT_REF_NAME_DESTINATION=${ASSETS_IMAGE_PATH}:${CI_COMMIT_REF_NAME} + skopeo copy "docker://${COMMIT_ASSETS_HASH_DESTINATION}" "docker://${COMMIT_REF_SLUG_DESTINATION}" + skopeo copy "docker://${COMMIT_ASSETS_HASH_DESTINATION}" "docker://${COMMIT_SHA_DESTINATION}" -DESTINATIONS="--destination=$COMMIT_REF_SLUG_DESTINATION --destination=$COMMIT_SHA_DESTINATION" + if [ -n "${CI_COMMIT_TAG}" ]; then + skopeo copy "docker://${COMMIT_ASSETS_HASH_DESTINATION}" "docker://${COMMIT_REF_NAME_DESTINATION}" + fi +else + echoinfo "Image ${COMMIT_ASSETS_HASH_DESTINATION} doesn't exist, we'll need to build it." -# Also tag the image with GitLab version, if running on a tag pipeline, so -# other projects can simply use that instead of computing the slug. -if [ -n "$CI_COMMIT_TAG" ]; then - DESTINATIONS="$DESTINATIONS --destination=$COMMIT_REF_NAME_DESTINATION" -fi + DESTINATIONS="--destination=${COMMIT_ASSETS_HASH_DESTINATION} --destination=${COMMIT_REF_SLUG_DESTINATION} --destination=${COMMIT_SHA_DESTINATION}" -echo "building assets image for destinations: $DESTINATIONS" + if [ -n "${CI_COMMIT_TAG}" ]; then + DESTINATIONS="$DESTINATIONS --destination=${COMMIT_REF_NAME_DESTINATION}" + fi -/kaniko/executor --context=assets_container.build --dockerfile=assets_container.build/Dockerfile.assets $DESTINATIONS + mkdir -p assets_container.build/public + cp -r public/assets assets_container.build/public/ + cp Dockerfile.assets assets_container.build/ + + echo "Building assets image for destinations: ${DESTINATIONS}" + + /kaniko/executor \ + --context="assets_container.build" \ + --dockerfile="assets_container.build/Dockerfile.assets" \ + ${DESTINATIONS} +fi diff --git a/scripts/trigger-build.rb b/scripts/trigger-build.rb index 897ca9f473e..8dfab8dd2eb 100755 --- a/scripts/trigger-build.rb +++ b/scripts/trigger-build.rb @@ -160,6 +160,8 @@ module Trigger end class CNG < Base + ASSETS_HASH = "cached-assets-hash.txt" + def variables # Delete variables that aren't useful when using native triggers. super.tap do |hash| @@ -187,7 +189,6 @@ module Trigger "TRIGGER_BRANCH" => ref, "GITLAB_VERSION" => ENV['CI_COMMIT_SHA'], "GITLAB_TAG" => ENV['CI_COMMIT_TAG'], # Always set a value, even an empty string, so that the downstream pipeline can correctly check it. - "GITLAB_ASSETS_TAG" => ENV['CI_COMMIT_TAG'] ? ENV['CI_COMMIT_REF_NAME'] : ENV['CI_COMMIT_SHA'], "FORCE_RAILS_IMAGE_BUILDS" => 'true', "CE_PIPELINE" => Trigger.ee? ? nil : "true", # Always set a value, even an empty string, so that the downstream pipeline can correctly check it. "EE_PIPELINE" => Trigger.ee? ? "true" : nil # Always set a value, even an empty string, so that the downstream pipeline can correctly check it. diff --git a/scripts/utils.sh b/scripts/utils.sh index 50ca7f558f6..b9b40d3dbd5 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -15,9 +15,11 @@ function retry() { function test_url() { local url="${1}" + local curl_args="${2}" local status + local cmd="curl ${curl_args} --output /dev/null -L -s -w ''%{http_code}'' \"${url}\"" - status=$(curl --output /dev/null -L -s -w ''%{http_code}'' "${url}") + status=$(eval "${cmd}") if [[ $status == "200" ]]; then return 0 @@ -203,3 +205,16 @@ function danger_as_local() { # We need to base SHA to help danger determine the base commit for this shallow clone. bundle exec danger dry_run --fail-on-errors=true --verbose --base="${CI_MERGE_REQUEST_DIFF_BASE_SHA}" --head="${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA:-$CI_COMMIT_SHA}" --dangerfile="${DANGER_DANGERFILE:-Dangerfile}" } + +# We're inlining this function in `.gitlab/ci/package-and-test/main.gitlab-ci.yml` so make sure to reflect any changes there +function assets_image_tag() { + local cache_assets_hash_file="cached-assets-hash.txt" + + if [[ -n "${CI_COMMIT_TAG}" ]]; then + echo -n "${CI_COMMIT_REF_NAME}" + elif [[ -f "${cache_assets_hash_file}" ]]; then + echo -n "assets-hash-$(cat ${cache_assets_hash_file} | cut -c1-10)" + else + echo -n "${CI_COMMIT_SHA}" + fi +} diff --git a/spec/scripts/trigger-build_spec.rb b/spec/scripts/trigger-build_spec.rb index ac8e3c7797c..52682387e20 100644 --- a/spec/scripts/trigger-build_spec.rb +++ b/spec/scripts/trigger-build_spec.rb @@ -318,28 +318,6 @@ RSpec.describe Trigger do end end - describe "GITLAB_ASSETS_TAG" do - context 'when CI_COMMIT_TAG is set' do - before do - stub_env('CI_COMMIT_TAG', 'v1.0') - end - - it 'sets GITLAB_ASSETS_TAG to CI_COMMIT_REF_NAME' do - expect(subject.variables['GITLAB_ASSETS_TAG']).to eq(env['CI_COMMIT_REF_NAME']) - end - end - - context 'when CI_COMMIT_TAG is nil' do - before do - stub_env('CI_COMMIT_TAG', nil) - end - - it 'sets GITLAB_ASSETS_TAG to CI_COMMIT_SHA' do - expect(subject.variables['GITLAB_ASSETS_TAG']).to eq(env['CI_COMMIT_SHA']) - end - end - end - describe "CE_PIPELINE" do context 'when Trigger.ee? is true' do before do |