diff options
78 files changed, 706 insertions, 280 deletions
diff --git a/.gitignore b/.gitignore index 24391f51483..30cb231e83f 100644 --- a/.gitignore +++ b/.gitignore @@ -101,3 +101,5 @@ apollo.config.js /tmp/matching_tests.txt ee/changelogs/unreleased-ee /sitespeed-result +tags.lock +tags.temp diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 6d74952b7a1..ff7be75265b 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,7 +1,7 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import { escape } from 'lodash'; -import { GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; +import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { sprintf } from '~/locale'; import { deprecatedCreateFlash as createFlash } from '~/flash'; @@ -19,6 +19,7 @@ export default { components: { DiffFileHeader, DiffContent, + GlButton, GlLoadingIcon, }, directives: { @@ -267,16 +268,21 @@ export default { <div v-safe-html="errorMessage" class="nothing-here-block"></div> </div> <template v-else> - <div v-show="showWarning" class="nothing-here-block diff-collapsed"> - {{ $options.i18n.collapsed }} - <a - class="click-to-expand" - data-testid="toggle-link" - href="#" + <div + v-show="showWarning" + class="collapsed-file-warning gl-p-7 gl-bg-orange-50 gl-text-center gl-rounded-bottom-left-base gl-rounded-bottom-right-base" + > + <p class="gl-mb-8"> + {{ $options.i18n.autoCollapsed }} + </p> + <gl-button + data-testid="expand-button" + category="secondary" + variant="warning" @click.prevent="handleToggle" > {{ $options.i18n.expand }} - </a> + </gl-button> </div> <diff-content v-show="showContent" diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 361f8e1a6da..4ec24d452bf 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -13,6 +13,6 @@ export const DIFF_FILE = { ), fork: __('Fork'), cancel: __('Cancel'), - collapsed: __('This diff is collapsed.'), - expand: __('Click to expand it.'), + autoCollapsed: __('Files with large changes are collapsed by default.'), + expand: __('Expand file'), }; diff --git a/app/assets/javascripts/groups/constants.js b/app/assets/javascripts/groups/constants.js index c538934a37d..e2722d780dc 100644 --- a/app/assets/javascripts/groups/constants.js +++ b/app/assets/javascripts/groups/constants.js @@ -31,14 +31,16 @@ export const GROUP_VISIBILITY_TYPE = { 'Public - The group and any public projects can be viewed without any authentication.', ), internal: __( - 'Internal - The group and any internal projects can be viewed by any logged in user.', + 'Internal - The group and any internal projects can be viewed by any logged in user except external users.', ), private: __('Private - The group and its projects can only be viewed by members.'), }; export const PROJECT_VISIBILITY_TYPE = { public: __('Public - The project can be accessed without any authentication.'), - internal: __('Internal - The project can be accessed by any logged in user.'), + internal: __( + 'Internal - The project can be accessed by any logged in user except external users.', + ), private: __( 'Private - Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.', ), diff --git a/app/assets/javascripts/snippets/components/snippet_header.vue b/app/assets/javascripts/snippets/components/snippet_header.vue index 951ef25e3ef..686ca5fcab1 100644 --- a/app/assets/javascripts/snippets/components/snippet_header.vue +++ b/app/assets/javascripts/snippets/components/snippet_header.vue @@ -120,7 +120,7 @@ export default { ? __('The snippet is visible only to project members.') : __('The snippet is visible only to me.'); case 'internal': - return __('The snippet is visible to any logged in user.'); + return __('The snippet is visible to any logged in user except external users.'); default: return __('The snippet can be accessed without any authentication.'); } diff --git a/app/assets/javascripts/snippets/constants.js b/app/assets/javascripts/snippets/constants.js index e75922df15f..2a9ecbc27dc 100644 --- a/app/assets/javascripts/snippets/constants.js +++ b/app/assets/javascripts/snippets/constants.js @@ -14,7 +14,7 @@ export const SNIPPET_VISIBILITY = { [SNIPPET_VISIBILITY_INTERNAL]: { label: __('Internal'), icon: 'shield', - description: __('The snippet is visible to any logged in user.'), + description: __('The snippet is visible to any logged in user except external users.'), }, [SNIPPET_VISIBILITY_PUBLIC]: { label: __('Public'), diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index 2f5043f9ffa..b1f2ffb8503 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -30,6 +30,8 @@ class GitlabSchema < GraphQL::Schema default_max_page_size 100 + lazy_resolve ::Gitlab::Graphql::Lazy, :force + class << self def multiplex(queries, **kwargs) kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context]) diff --git a/app/graphql/types/release_links_type.rb b/app/graphql/types/release_links_type.rb index b00fcf2ceb4..da2ef4665f2 100644 --- a/app/graphql/types/release_links_type.rb +++ b/app/graphql/types/release_links_type.rb @@ -15,22 +15,22 @@ module Types field :edit_url, GraphQL::STRING_TYPE, null: true, description: "HTTP URL of the release's edit page", authorize: :update_release - field :open_merge_requests_url, GraphQL::STRING_TYPE, null: true, + field :opened_merge_requests_url, GraphQL::STRING_TYPE, null: true, description: 'HTTP URL of the merge request page, filtered by this release and `state=open`' field :merged_merge_requests_url, GraphQL::STRING_TYPE, null: true, description: 'HTTP URL of the merge request page , filtered by this release and `state=merged`' field :closed_merge_requests_url, GraphQL::STRING_TYPE, null: true, description: 'HTTP URL of the merge request page , filtered by this release and `state=closed`' - field :open_issues_url, GraphQL::STRING_TYPE, null: true, + field :opened_issues_url, GraphQL::STRING_TYPE, null: true, description: 'HTTP URL of the issues page, filtered by this release and `state=open`' field :closed_issues_url, GraphQL::STRING_TYPE, null: true, description: 'HTTP URL of the issues page, filtered by this release and `state=closed`' - field :merge_requests_url, GraphQL::STRING_TYPE, null: true, method: :open_merge_requests_url, + field :merge_requests_url, GraphQL::STRING_TYPE, null: true, method: :opened_merge_requests_url, description: 'HTTP URL of the merge request page filtered by this release', - deprecated: { reason: 'Use `open_merge_requests_url`', milestone: '13.6' } - field :issues_url, GraphQL::STRING_TYPE, null: true, method: :open_issues_url, + deprecated: { reason: 'Use `openedMergeRequestsUrl`', milestone: '13.6' } + field :issues_url, GraphQL::STRING_TYPE, null: true, method: :opened_issues_url, description: 'HTTP URL of the issues page filtered by this release', - deprecated: { reason: 'Use `open_issues_url`', milestone: '13.6' } + deprecated: { reason: 'Use `openedIssuesUrl`', milestone: '13.6' } end end diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index a7b9e17c898..896dcdd2caf 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -31,7 +31,7 @@ module VisibilityLevelHelper when Gitlab::VisibilityLevel::PRIVATE _("Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.") when Gitlab::VisibilityLevel::INTERNAL - _("The project can be accessed by any logged in user.") + _("The project can be accessed by any logged in user except external users.") when Gitlab::VisibilityLevel::PUBLIC _("The project can be accessed without any authentication.") end @@ -42,7 +42,7 @@ module VisibilityLevelHelper when Gitlab::VisibilityLevel::PRIVATE _("The group and its projects can only be viewed by members.") when Gitlab::VisibilityLevel::INTERNAL - _("The group and any internal projects can be viewed by any logged in user.") + _("The group and any internal projects can be viewed by any logged in user except external users.") when Gitlab::VisibilityLevel::PUBLIC _("The group and any public projects can be viewed without any authentication.") end diff --git a/app/presenters/release_presenter.rb b/app/presenters/release_presenter.rb index 1a89a76b4dc..2e85ab2bb94 100644 --- a/app/presenters/release_presenter.rb +++ b/app/presenters/release_presenter.rb @@ -23,7 +23,7 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated project_release_url(project, release) end - def open_merge_requests_url + def opened_merge_requests_url return unless release_mr_issue_urls_available? project_merge_requests_url(project, params_for_issues_and_mrs) @@ -41,7 +41,7 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated project_merge_requests_url(project, params_for_issues_and_mrs(state: 'closed')) end - def open_issues_url + def opened_issues_url return unless release_mr_issue_urls_available? project_issues_url(project, params_for_issues_and_mrs) diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index 4793e685163..94eff552fb9 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -72,7 +72,7 @@ %li = _("For public projects, anyone can view pipelines and access job details (output logs and artifacts)") %li - = _("For internal projects, any logged in user can view pipelines and access job details (output logs and artifacts)") + = _("For internal projects, any logged in user except external users can view pipelines and access job details (output logs and artifacts)") %li = _("For private projects, any member (guest or higher) can view pipelines and access job details (output logs and artifacts)") %p diff --git a/app/workers/ci/delete_objects_worker.rb b/app/workers/ci/delete_objects_worker.rb index e34be33b438..1a886d0efeb 100644 --- a/app/workers/ci/delete_objects_worker.rb +++ b/app/workers/ci/delete_objects_worker.rb @@ -14,7 +14,7 @@ module Ci def remaining_work_count(*args) @remaining_work_count ||= service - .remaining_batches_count(max_batch_count: remaining_capacity) + .remaining_batches_count(max_batch_count: max_running_jobs) end def max_running_jobs diff --git a/bin/feature-flag b/bin/feature-flag index 0de9b90681f..613ddc1d8cb 100755 --- a/bin/feature-flag +++ b/bin/feature-flag @@ -104,7 +104,7 @@ class FeatureFlagOptionParser end # Name is a first name - options.name = argv.first + options.name = argv.first.downcase.gsub(/-/, '_') options end diff --git a/changelogs/unreleased/46087.yml b/changelogs/unreleased/46087.yml new file mode 100644 index 00000000000..3cec8195d6a --- /dev/null +++ b/changelogs/unreleased/46087.yml @@ -0,0 +1,5 @@ +--- +title: Clarify that external users cannot access all internal projects, groups, and snippets +merge_request: 46087 +author: Ben Bodenmiller (@bbodenmiller) +type: other diff --git a/changelogs/unreleased/feature-collapsed-diff-files-larger-ui.yml b/changelogs/unreleased/feature-collapsed-diff-files-larger-ui.yml new file mode 100644 index 00000000000..1057c18522b --- /dev/null +++ b/changelogs/unreleased/feature-collapsed-diff-files-larger-ui.yml @@ -0,0 +1,5 @@ +--- +title: Expand Diff File collapsed UI to be significantly more obvious +merge_request: 46286 +author: +type: changed diff --git a/config/feature_flags/development/ci_new_artifact_file_reader.yml b/config/feature_flags/development/ci_new_artifact_file_reader.yml new file mode 100644 index 00000000000..ccd36558b1d --- /dev/null +++ b/config/feature_flags/development/ci_new_artifact_file_reader.yml @@ -0,0 +1,8 @@ +--- +name: ci_new_artifact_file_reader +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46552 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/273755 +milestone: '13.6' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/config/feature_flags/development/graphql_lazy_authorization.yml b/config/feature_flags/development/graphql_lazy_authorization.yml new file mode 100644 index 00000000000..8277c2666ea --- /dev/null +++ b/config/feature_flags/development/graphql_lazy_authorization.yml @@ -0,0 +1,7 @@ +--- +name: graphql_lazy_authorization +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45263 +rollout_issue_url: +type: development +group: group::plan +default_enabled: false diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index f8c50a31d72..fd36324f363 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -17273,14 +17273,14 @@ type ReleaseLinks { editUrl: String """ - HTTP URL of the issues page filtered by this release. Deprecated in 13.6: Use `open_issues_url` + HTTP URL of the issues page filtered by this release. Deprecated in 13.6: Use `openedIssuesUrl` """ - issuesUrl: String @deprecated(reason: "Use `open_issues_url`. Deprecated in 13.6") + issuesUrl: String @deprecated(reason: "Use `openedIssuesUrl`. Deprecated in 13.6") """ - HTTP URL of the merge request page filtered by this release. Deprecated in 13.6: Use `open_merge_requests_url` + HTTP URL of the merge request page filtered by this release. Deprecated in 13.6: Use `openedMergeRequestsUrl` """ - mergeRequestsUrl: String @deprecated(reason: "Use `open_merge_requests_url`. Deprecated in 13.6") + mergeRequestsUrl: String @deprecated(reason: "Use `openedMergeRequestsUrl`. Deprecated in 13.6") """ HTTP URL of the merge request page , filtered by this release and `state=merged` @@ -17290,12 +17290,12 @@ type ReleaseLinks { """ HTTP URL of the issues page, filtered by this release and `state=open` """ - openIssuesUrl: String + openedIssuesUrl: String """ HTTP URL of the merge request page, filtered by this release and `state=open` """ - openMergeRequestsUrl: String + openedMergeRequestsUrl: String """ HTTP URL of the release diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 577c92dd66b..8181bfcf727 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -49821,7 +49821,7 @@ }, { "name": "issuesUrl", - "description": "HTTP URL of the issues page filtered by this release. Deprecated in 13.6: Use `open_issues_url`", + "description": "HTTP URL of the issues page filtered by this release. Deprecated in 13.6: Use `openedIssuesUrl`", "args": [ ], @@ -49831,11 +49831,11 @@ "ofType": null }, "isDeprecated": true, - "deprecationReason": "Use `open_issues_url`. Deprecated in 13.6" + "deprecationReason": "Use `openedIssuesUrl`. Deprecated in 13.6" }, { "name": "mergeRequestsUrl", - "description": "HTTP URL of the merge request page filtered by this release. Deprecated in 13.6: Use `open_merge_requests_url`", + "description": "HTTP URL of the merge request page filtered by this release. Deprecated in 13.6: Use `openedMergeRequestsUrl`", "args": [ ], @@ -49845,7 +49845,7 @@ "ofType": null }, "isDeprecated": true, - "deprecationReason": "Use `open_merge_requests_url`. Deprecated in 13.6" + "deprecationReason": "Use `openedMergeRequestsUrl`. Deprecated in 13.6" }, { "name": "mergedMergeRequestsUrl", @@ -49862,7 +49862,7 @@ "deprecationReason": null }, { - "name": "openIssuesUrl", + "name": "openedIssuesUrl", "description": "HTTP URL of the issues page, filtered by this release and `state=open`", "args": [ @@ -49876,7 +49876,7 @@ "deprecationReason": null }, { - "name": "openMergeRequestsUrl", + "name": "openedMergeRequestsUrl", "description": "HTTP URL of the merge request page, filtered by this release and `state=open`", "args": [ diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index d1ed9ebb2ce..e820b953d49 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2304,11 +2304,11 @@ Evidence for a release. | `closedIssuesUrl` | String | HTTP URL of the issues page, filtered by this release and `state=closed` | | `closedMergeRequestsUrl` | String | HTTP URL of the merge request page , filtered by this release and `state=closed` | | `editUrl` | String | HTTP URL of the release's edit page | -| `issuesUrl` **{warning-solid}** | String | **Deprecated:** Use `open_issues_url`. Deprecated in 13.6 | -| `mergeRequestsUrl` **{warning-solid}** | String | **Deprecated:** Use `open_merge_requests_url`. Deprecated in 13.6 | +| `issuesUrl` **{warning-solid}** | String | **Deprecated:** Use `openedIssuesUrl`. Deprecated in 13.6 | +| `mergeRequestsUrl` **{warning-solid}** | String | **Deprecated:** Use `openedMergeRequestsUrl`. Deprecated in 13.6 | | `mergedMergeRequestsUrl` | String | HTTP URL of the merge request page , filtered by this release and `state=merged` | -| `openIssuesUrl` | String | HTTP URL of the issues page, filtered by this release and `state=open` | -| `openMergeRequestsUrl` | String | HTTP URL of the merge request page, filtered by this release and `state=open` | +| `openedIssuesUrl` | String | HTTP URL of the issues page, filtered by this release and `state=open` | +| `openedMergeRequestsUrl` | String | HTTP URL of the merge request page, filtered by this release and `state=open` | | `selfUrl` | String | HTTP URL of the release | ### ReleaseSource diff --git a/doc/api/project_snippets.md b/doc/api/project_snippets.md index cc8bb20b003..7955050e716 100644 --- a/doc/api/project_snippets.md +++ b/doc/api/project_snippets.md @@ -17,7 +17,7 @@ Constants for snippet visibility levels are: | visibility | Description | | ---------- | ----------- | | `private` | The snippet is visible only the snippet creator | -| `internal` | The snippet is visible for any logged in user | +| `internal` | The snippet is visible for any logged in user except [external users](../user/permissions.md#external-users) | | `public` | The snippet can be accessed without any authentication | NOTE: **Note:** diff --git a/doc/api/projects.md b/doc/api/projects.md index f6ed905cda1..07b14db75eb 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -16,7 +16,7 @@ Values for the project visibility level are: - `private`: Project access must be granted explicitly for each user. - `internal`: - The project can be cloned by any logged in user. + The project can be cloned by any logged in user except [external users](../user/permissions.md#external-users). - `public`: The project can be accessed without any authentication. diff --git a/doc/api/snippets.md b/doc/api/snippets.md index 431d745ac84..c2812de5dd7 100644 --- a/doc/api/snippets.md +++ b/doc/api/snippets.md @@ -21,7 +21,7 @@ Valid values for snippet visibility levels are: | Visibility | Description | |:-----------|:----------------------------------------------------| | `private` | Snippet is visible only to the snippet creator. | -| `internal` | Snippet is visible for any logged in user. | +| `internal` | Snippet is visible for any logged in user except [external users](../user/permissions.md#external-users). | | `public` | Snippet can be accessed without any authentication. | ## List all snippets for a user diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index 7f76c64796b..d1ca3431826 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -302,6 +302,58 @@ build: - docker run my-docker-image /script/to/run/tests ``` +#### Enable registry mirror for `docker:dind` service + +When the Docker daemon starts inside of the service container, it uses +the default configuration. You may want to configure a [registry +mirror](https://docs.docker.com/registry/recipes/mirror/) for +performance improvements and ensuring you don't reach DockerHub rate limits. + +##### Inside `.gitlab-ci.yml` + +You can append extra CLI flags to the `dind` service to set the registry +mirror: + +```yaml +services: + - name: docker:19.03.13-dind + command: ["--registry-mirror", "https://registry-mirror.example.com"] # Specify the registry mirror to use. +``` + +##### Docker executor inside GitLab Runner configuration + +If you are an administrator of GitLab Runner and you always want to use +the mirror for every `dind` service, update the +[configuration](https://docs.gitlab.com/runner/configuration/advanced-configuration.html) +to specify a [volume +mount](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#volumes-in-the-runnersdocker-section). + +Given we have a file `/opt/docker/daemon.json` with the following +content: + +```json +{ + "registry-mirrors": [ + "https://registry-mirror.example.com" + ] +} +``` + +Update the `config.toml` for GitLab Runner to mount the file to +`/etc/docker/daemon.json`. This would mount the file for **every** +container that is created by GitLab Runner. The configuration will be +picked up by the `dind` service. + +```toml +[[runners]] + ... + executor = "docker" + [runners.docker] + image = "alpine:3.12" + privileged = true + volumes = ["/opt/docker/daemon.json:/etc/docker/daemon.json:ro"] +``` + ### Use Docker socket binding The third approach is to bind-mount `/var/run/docker.sock` into the diff --git a/doc/ci/pipelines/settings.md b/doc/ci/pipelines/settings.md index 143a5346e88..6619f1b5f02 100644 --- a/doc/ci/pipelines/settings.md +++ b/doc/ci/pipelines/settings.md @@ -183,7 +183,7 @@ Job logs and artifacts are [not visible for guest users and non-project members] If **Public pipelines** is enabled (default): - For **public** projects, anyone can view the pipelines and related features. -- For **internal** projects, any logged in user can view the pipelines +- For **internal** projects, any logged in user except [external users](../../user/permissions.md#external-users) can view the pipelines and related features. - For **private** projects, any project member (guest or higher) can view the pipelines and related features. @@ -192,7 +192,7 @@ If **Public pipelines** is disabled: - For **public** projects, anyone can view the pipelines, but only members (reporter or higher) can access the related features. -- For **internal** projects, any logged in user can view the pipelines. +- For **internal** projects, any logged in user except [external users](../../user/permissions.md#external-users) can view the pipelines. However, only members (reporter or higher) can access the job related features. - For **private** projects, only project members (reporter or higher) can view the pipelines or access the related features. diff --git a/doc/public_access/public_access.md b/doc/public_access/public_access.md index 848065de001..6babaf8080b 100644 --- a/doc/public_access/public_access.md +++ b/doc/public_access/public_access.md @@ -21,12 +21,12 @@ on the repository. ### Internal projects -Internal projects can be cloned by any logged in user. +Internal projects can be cloned by any logged in user except [external users](../user/permissions.md#external-users). They will also be listed in the public access directory (`/public`), but only for logged in users. -Any logged in user will have [Guest permissions](../user/permissions.md) +Any logged in user except [external users](../user/permissions.md#external-users) will have [Guest permissions](../user/permissions.md) on the repository. NOTE: **Note:** diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 470bc5c67da..a6f1447f551 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -314,6 +314,10 @@ External users: - Can only access public projects and projects to which they are explicitly granted access, thus hiding all other internal or private ones from them (like being logged out). +- Can only access public groups and groups to which they are explicitly granted access, + thus hiding all other internal or private ones from them (like being + logged out). +- Can only access public snippets. Access can be granted by adding the user as member to the project or group. Like usual users, they receive a role in the project or group with all diff --git a/lib/api/composer_packages.rb b/lib/api/composer_packages.rb index 1becbd668a3..0ac5cc45ccf 100644 --- a/lib/api/composer_packages.rb +++ b/lib/api/composer_packages.rb @@ -9,6 +9,8 @@ module API include ::API::Helpers::Packages::BasicAuthHelpers::Constants include ::Gitlab::Utils::StrongMemoize + feature_category :package_registry + content_type :json, 'application/json' default_format :json diff --git a/lib/api/conan_package_endpoints.rb b/lib/api/conan_package_endpoints.rb index 9b6867a328b..188a42f26f8 100644 --- a/lib/api/conan_package_endpoints.rb +++ b/lib/api/conan_package_endpoints.rb @@ -29,6 +29,8 @@ module API CONAN_FILES = (Gitlab::Regex::Packages::CONAN_RECIPE_FILES + Gitlab::Regex::Packages::CONAN_PACKAGE_FILES).freeze included do + feature_category :package_registry + helpers ::API::Helpers::PackagesManagerClientsHelpers helpers ::API::Helpers::Packages::Conan::ApiHelpers helpers ::API::Helpers::RelatedResourcesHelpers diff --git a/lib/api/debian_package_endpoints.rb b/lib/api/debian_package_endpoints.rb index 168b3ca7a4f..c95c75b7e5c 100644 --- a/lib/api/debian_package_endpoints.rb +++ b/lib/api/debian_package_endpoints.rb @@ -26,6 +26,8 @@ module API }.freeze included do + feature_category :package_registry + helpers ::API::Helpers::PackagesHelpers helpers ::API::Helpers::Packages::BasicAuthHelpers diff --git a/lib/api/entities/release.rb b/lib/api/entities/release.rb index 84c89b9f510..ac43617914a 100644 --- a/lib/api/entities/release.rb +++ b/lib/api/entities/release.rb @@ -30,8 +30,8 @@ module API expose :evidences, using: Entities::Releases::Evidence, expose_nil: false, if: ->(_, _) { can_download_code? } expose :_links do expose :self_url, as: :self, expose_nil: false - expose :open_merge_requests_url, as: :merge_requests_url, expose_nil: false - expose :open_issues_url, as: :issues_url, expose_nil: false + expose :opened_merge_requests_url, as: :merge_requests_url, expose_nil: false + expose :opened_issues_url, as: :issues_url, expose_nil: false expose :edit_url, expose_nil: false end diff --git a/lib/api/generic_packages.rb b/lib/api/generic_packages.rb index a0c33ab65b9..3e1dd044c8d 100644 --- a/lib/api/generic_packages.rb +++ b/lib/api/generic_packages.rb @@ -7,6 +7,8 @@ module API file_name: API::NO_SLASH_URL_PART_REGEX }.freeze + feature_category :package_registry + before do require_packages_enabled! authenticate! diff --git a/lib/api/go_proxy.rb b/lib/api/go_proxy.rb index 85724287c56..8fb4c561c40 100755 --- a/lib/api/go_proxy.rb +++ b/lib/api/go_proxy.rb @@ -4,6 +4,8 @@ module API helpers Gitlab::Golang helpers ::API::Helpers::PackagesHelpers + feature_category :package_registry + # basic semver, except case encoded (A => !a) MODULE_VERSION_REGEX = /v(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-([-.!a-z0-9]+))?(?:\+([-.!a-z0-9]+))?/.freeze diff --git a/lib/api/group_packages.rb b/lib/api/group_packages.rb index 5b6290df0dd..31b28c3990f 100644 --- a/lib/api/group_packages.rb +++ b/lib/api/group_packages.rb @@ -8,6 +8,8 @@ module API authorize_packages_access!(user_group) end + feature_category :package_registry + helpers ::API::Helpers::PackagesHelpers params do diff --git a/lib/api/helpers/resource_label_events_helpers.rb b/lib/api/helpers/resource_label_events_helpers.rb index 423bd4e704b..ad2733baffc 100644 --- a/lib/api/helpers/resource_label_events_helpers.rb +++ b/lib/api/helpers/resource_label_events_helpers.rb @@ -3,10 +3,13 @@ module API module Helpers module ResourceLabelEventsHelpers - def self.eventable_types + def self.feature_category_per_eventable_type # This is a method instead of a constant, allowing EE to more easily # extend it. - [Issue, MergeRequest] + { + Issue => :issue_tracking, + MergeRequest => :code_review + } end end end diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index a3e2fa84c32..414ef98ccdb 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -5,6 +5,8 @@ module API file_name: API::NO_SLASH_URL_PART_REGEX }.freeze + feature_category :package_registry + content_type :md5, 'text/plain' content_type :sha1, 'text/plain' content_type :binary, 'application/octet-stream' diff --git a/lib/api/notification_settings.rb b/lib/api/notification_settings.rb index bad3f5ead7a..7d28394e034 100644 --- a/lib/api/notification_settings.rb +++ b/lib/api/notification_settings.rb @@ -5,6 +5,8 @@ module API class NotificationSettings < ::API::Base before { authenticate! } + feature_category :users + helpers ::API::Helpers::MembersHelpers resource :notification_settings do diff --git a/lib/api/npm_packages.rb b/lib/api/npm_packages.rb index 1443b28c1ee..e0c20fe3cc4 100644 --- a/lib/api/npm_packages.rb +++ b/lib/api/npm_packages.rb @@ -4,6 +4,8 @@ module API helpers ::API::Helpers::PackagesHelpers helpers ::API::Helpers::Packages::DependencyProxyHelpers + feature_category :package_registry + NPM_ENDPOINT_REQUIREMENTS = { package_name: API::NO_SLASH_URL_PART_REGEX }.freeze diff --git a/lib/api/nuget_packages.rb b/lib/api/nuget_packages.rb index 0f2c956a9df..65a85f3c930 100644 --- a/lib/api/nuget_packages.rb +++ b/lib/api/nuget_packages.rb @@ -10,6 +10,8 @@ module API helpers ::API::Helpers::PackagesManagerClientsHelpers helpers ::API::Helpers::Packages::BasicAuthHelpers + feature_category :package_registry + POSITIVE_INTEGER_REGEX = %r{\A[1-9]\d*\z}.freeze NON_NEGATIVE_INTEGER_REGEX = %r{\A0|[1-9]\d*\z}.freeze diff --git a/lib/api/package_files.rb b/lib/api/package_files.rb index c1fc9a6e4d8..4a33f3e8af2 100644 --- a/lib/api/package_files.rb +++ b/lib/api/package_files.rb @@ -8,6 +8,8 @@ module API authorize_packages_access!(user_project) end + feature_category :package_registry + helpers ::API::Helpers::PackagesHelpers params do diff --git a/lib/api/pages.rb b/lib/api/pages.rb index 813307c498f..5f695f3853d 100644 --- a/lib/api/pages.rb +++ b/lib/api/pages.rb @@ -2,6 +2,8 @@ module API class Pages < ::API::Base + feature_category :pages + before do require_pages_config_enabled! authenticated_with_can_read_all_resources! diff --git a/lib/api/pages_domains.rb b/lib/api/pages_domains.rb index 00c51298c45..2e7f8475509 100644 --- a/lib/api/pages_domains.rb +++ b/lib/api/pages_domains.rb @@ -4,6 +4,8 @@ module API class PagesDomains < ::API::Base include PaginationParams + feature_category :pages + PAGES_DOMAINS_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(domain: API::NO_SLASH_URL_PART_REGEX) before do diff --git a/lib/api/project_clusters.rb b/lib/api/project_clusters.rb index 46ccb4ba1a0..cfb0c5fd705 100644 --- a/lib/api/project_clusters.rb +++ b/lib/api/project_clusters.rb @@ -6,6 +6,8 @@ module API before { authenticate! } + feature_category :kubernetes_management + params do requires :id, type: String, desc: 'The ID of the project' end diff --git a/lib/api/project_container_repositories.rb b/lib/api/project_container_repositories.rb index d565531d372..3125de88de5 100644 --- a/lib/api/project_container_repositories.rb +++ b/lib/api/project_container_repositories.rb @@ -10,6 +10,8 @@ module API before { authorize_read_container_images! } + feature_category :package_registry + params do requires :id, type: String, desc: 'The ID of a project' end diff --git a/lib/api/project_events.rb b/lib/api/project_events.rb index 3765473bc0e..69b47f9420d 100644 --- a/lib/api/project_events.rb +++ b/lib/api/project_events.rb @@ -6,6 +6,8 @@ module API include APIGuard helpers ::API::Helpers::EventsHelpers + feature_category :users + params do requires :id, type: String, desc: 'The ID of a project' end diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb index 184f89200ab..76b3dea723a 100644 --- a/lib/api/project_export.rb +++ b/lib/api/project_export.rb @@ -4,6 +4,8 @@ module API class ProjectExport < ::API::Base helpers Helpers::RateLimiter + feature_category :importers + before do not_found! unless Gitlab::CurrentSettings.project_export_enabled? authorize_admin_project diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index bc2d8c816a8..91bcc7e0257 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -7,6 +7,8 @@ module API before { authenticate! } before { authorize_admin_project } + feature_category :integrations + helpers do params :project_hook_properties do requires :url, type: String, desc: "The URL to send the request to" diff --git a/lib/api/project_import.rb b/lib/api/project_import.rb index 5c4e1d73ee1..15b06cea385 100644 --- a/lib/api/project_import.rb +++ b/lib/api/project_import.rb @@ -8,6 +8,8 @@ module API helpers Helpers::FileUploadHelpers helpers Helpers::RateLimiter + feature_category :importers + helpers do def import_params declared_params(include_missing: false) diff --git a/lib/api/project_milestones.rb b/lib/api/project_milestones.rb index a81118f44bd..8675de33923 100644 --- a/lib/api/project_milestones.rb +++ b/lib/api/project_milestones.rb @@ -7,6 +7,8 @@ module API before { authenticate! } + feature_category :issue_tracking + params do requires :id, type: String, desc: 'The ID of a project' end diff --git a/lib/api/project_packages.rb b/lib/api/project_packages.rb index b8d97b1243a..56e94333433 100644 --- a/lib/api/project_packages.rb +++ b/lib/api/project_packages.rb @@ -8,6 +8,8 @@ module API authorize_packages_access!(user_project) end + feature_category :package_registry + helpers ::API::Helpers::PackagesHelpers params do diff --git a/lib/api/project_repository_storage_moves.rb b/lib/api/project_repository_storage_moves.rb index 38eb74663d3..fe6de3ea385 100644 --- a/lib/api/project_repository_storage_moves.rb +++ b/lib/api/project_repository_storage_moves.rb @@ -6,6 +6,8 @@ module API before { authenticated_as_admin! } + feature_category :gitaly + resource :project_repository_storage_moves do desc 'Get a list of all project repository storage moves' do detail 'This feature was introduced in GitLab 13.0.' diff --git a/lib/api/projects.rb b/lib/api/projects.rb index ecee76ae60c..2012c348cd1 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -11,6 +11,8 @@ module API before { authenticate_non_get! } + feature_category :projects, ['/projects/:id/custom_attributes', '/projects/:id/custom_attributes/:key'] + helpers do # EE::API::Projects would override this method def apply_filters(projects) @@ -150,7 +152,7 @@ module API use :statistics_params use :with_custom_attributes end - get ":user_id/projects" do + get ":user_id/projects", feature_category: :projects do user = find_user(params[:user_id]) not_found!('User') unless user @@ -167,7 +169,7 @@ module API use :collection_params use :statistics_params end - get ":user_id/starred_projects" do + get ":user_id/starred_projects", feature_category: :projects do user = find_user(params[:user_id]) not_found!('User') unless user @@ -187,7 +189,7 @@ module API use :statistics_params use :with_custom_attributes end - get do + get feature_category: :projects do present_projects load_projects end @@ -234,7 +236,7 @@ module API use :create_params end # rubocop: disable CodeReuse/ActiveRecord - post "user/:user_id" do + post "user/:user_id", feature_category: :projects do Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab/issues/21139') authenticated_as_admin! user = User.find_by(id: params.delete(:user_id)) @@ -270,7 +272,7 @@ module API optional :license, type: Boolean, default: false, desc: 'Include project license data' end - get ":id" do + get ":id", feature_category: :projects do options = { with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, current_user: current_user, @@ -294,7 +296,7 @@ module API optional :path, type: String, desc: 'The path that will be assigned to the fork' optional :name, type: String, desc: 'The name that will be assigned to the fork' end - post ':id/fork' do + post ':id/fork', feature_category: :source_code_management do Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42284') not_found! unless can?(current_user, :fork_project, user_project) @@ -332,14 +334,14 @@ module API use :collection_params use :with_custom_attributes end - get ':id/forks' do + get ':id/forks', feature_category: :source_code_management do forks = ForkProjectsFinder.new(user_project, params: project_finder_params, current_user: current_user).execute present_projects forks, request_scope: user_project end desc 'Check pages access of this project' - get ':id/pages_access' do + get ':id/pages_access', feature_category: :pages do authorize! :read_pages_content, user_project unless user_project.public_pages? status 200 end @@ -357,7 +359,7 @@ module API at_least_one_of(*Helpers::ProjectsHelpers.update_params_at_least_one_of) end - put ':id' do + put ':id', feature_category: :projects do authorize_admin_project attrs = declared_params(include_missing: false) authorize! :rename_project, user_project if attrs[:name].present? @@ -381,7 +383,7 @@ module API desc 'Archive a project' do success Entities::Project end - post ':id/archive' do + post ':id/archive', feature_category: :projects do authorize!(:archive_project, user_project) ::Projects::UpdateService.new(user_project, current_user, archived: true).execute @@ -392,7 +394,7 @@ module API desc 'Unarchive a project' do success Entities::Project end - post ':id/unarchive' do + post ':id/unarchive', feature_category: :projects do authorize!(:archive_project, user_project) ::Projects::UpdateService.new(user_project, current_user, archived: false).execute @@ -403,7 +405,7 @@ module API desc 'Star a project' do success Entities::Project end - post ':id/star' do + post ':id/star', feature_category: :projects do if current_user.starred?(user_project) not_modified! else @@ -417,7 +419,7 @@ module API desc 'Unstar a project' do success Entities::Project end - post ':id/unstar' do + post ':id/unstar', feature_category: :projects do if current_user.starred?(user_project) current_user.toggle_star(user_project) user_project.reset @@ -435,21 +437,21 @@ module API optional :search, type: String, desc: 'Return list of users matching the search criteria' use :pagination end - get ':id/starrers' do + get ':id/starrers', feature_category: :projects do starrers = UsersStarProjectsFinder.new(user_project, params, current_user: current_user).execute present paginate(starrers), with: Entities::UserStarsProject end desc 'Get languages in project repository' - get ':id/languages' do + get ':id/languages', feature_category: :source_code_management do ::Projects::RepositoryLanguagesService .new(user_project, current_user) .execute.map { |lang| [lang.name, lang.share] }.to_h end desc 'Delete a project' - delete ":id" do + delete ":id", feature_category: :projects do authorize! :remove_project, user_project delete_project(user_project) @@ -459,7 +461,7 @@ module API params do requires :forked_from_id, type: String, desc: 'The ID of the project it was forked from' end - post ":id/fork/:forked_from_id" do + post ":id/fork/:forked_from_id", feature_category: :source_code_management do authorize! :admin_project, user_project fork_from_project = find_project!(params[:forked_from_id]) @@ -478,7 +480,7 @@ module API end desc 'Remove a forked_from relationship' - delete ":id/fork" do + delete ":id/fork", feature_category: :source_code_management do authorize! :remove_fork_project, user_project result = destroy_conditionally!(user_project) do @@ -496,7 +498,7 @@ module API requires :group_access, type: Integer, values: Gitlab::Access.values, as: :link_group_access, desc: 'The group access level' optional :expires_at, type: Date, desc: 'Share expiration date' end - post ":id/share" do + post ":id/share", feature_category: :authentication_and_authorization do authorize! :admin_project, user_project group = Group.find_by_id(params[:group_id]) @@ -518,7 +520,7 @@ module API requires :group_id, type: Integer, desc: 'The ID of the group' end # rubocop: disable CodeReuse/ActiveRecord - delete ":id/share/:group_id" do + delete ":id/share/:group_id", feature_category: :authentication_and_authorization do authorize! :admin_project, user_project link = user_project.project_group_links.find_by(group_id: params[:group_id]) @@ -535,7 +537,7 @@ module API # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 requires :file, type: File, desc: 'The file to be uploaded' # rubocop:disable Scalability/FileUploads end - post ":id/uploads" do + post ":id/uploads", feature_category: :not_owned do upload = UploadService.new(user_project, params[:file]).execute present upload, with: Entities::ProjectUpload @@ -549,7 +551,7 @@ module API optional :skip_users, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Filter out users with the specified IDs' use :pagination end - get ':id/users' do + get ':id/users', feature_category: :authentication_and_authorization do users = DeclarativePolicy.subject_scope { user_project.team.users } users = users.search(params[:search]) if params[:search].present? users = users.where_not_in(params[:skip_users]) if params[:skip_users].present? @@ -560,7 +562,7 @@ module API desc 'Start the housekeeping task for a project' do detail 'This feature was introduced in GitLab 9.0.' end - post ':id/housekeeping' do + post ':id/housekeeping', feature_category: :source_code_management do authorize_admin_project begin @@ -574,7 +576,7 @@ module API params do requires :namespace, type: String, desc: 'The ID or path of the new namespace' end - put ":id/transfer" do + put ":id/transfer", feature_category: :projects do authorize! :change_namespace, user_project namespace = find_namespace!(params[:namespace]) diff --git a/lib/api/pypi_packages.rb b/lib/api/pypi_packages.rb index 5622bc6e42d..7104fb8d999 100644 --- a/lib/api/pypi_packages.rb +++ b/lib/api/pypi_packages.rb @@ -12,6 +12,8 @@ module API helpers ::API::Helpers::Packages::BasicAuthHelpers include ::API::Helpers::Packages::BasicAuthHelpers::Constants + feature_category :package_registry + default_format :json rescue_from ArgumentError do |e| diff --git a/lib/api/resource_label_events.rb b/lib/api/resource_label_events.rb index d3a219f0810..33589f6c393 100644 --- a/lib/api/resource_label_events.rb +++ b/lib/api/resource_label_events.rb @@ -7,7 +7,7 @@ module API before { authenticate! } - Helpers::ResourceLabelEventsHelpers.eventable_types.each do |eventable_type| + Helpers::ResourceLabelEventsHelpers.feature_category_per_eventable_type.each do |eventable_type, feature_category| parent_type = eventable_type.parent_class.to_s.underscore eventables_str = eventable_type.to_s.underscore.pluralize @@ -24,7 +24,7 @@ module API use :pagination end - get ":id/#{eventables_str}/:eventable_id/resource_label_events" do + get ":id/#{eventables_str}/:eventable_id/resource_label_events", feature_category: feature_category do eventable = find_noteable(eventable_type, params[:eventable_id]) events = eventable.resource_label_events.inc_relations @@ -40,7 +40,7 @@ module API requires :event_id, type: String, desc: 'The ID of a resource label event' requires :eventable_id, types: [Integer, String], desc: 'The ID of the eventable' end - get ":id/#{eventables_str}/:eventable_id/resource_label_events/:event_id" do + get ":id/#{eventables_str}/:eventable_id/resource_label_events/:event_id", feature_category: feature_category do eventable = find_noteable(eventable_type, params[:eventable_id]) event = eventable.resource_label_events.find(params[:event_id]) diff --git a/lib/api/resource_milestone_events.rb b/lib/api/resource_milestone_events.rb index 21411f68dd5..aeedd7ad109 100644 --- a/lib/api/resource_milestone_events.rb +++ b/lib/api/resource_milestone_events.rb @@ -7,7 +7,10 @@ module API before { authenticate! } - [Issue, MergeRequest].each do |eventable_type| + { + Issue => :issue_tracking, + MergeRequest => :code_review + }.each do |eventable_type, feature_category| parent_type = eventable_type.parent_class.to_s.underscore eventables_str = eventable_type.to_s.underscore.pluralize @@ -23,7 +26,7 @@ module API use :pagination end - get ":id/#{eventables_str}/:eventable_id/resource_milestone_events" do + get ":id/#{eventables_str}/:eventable_id/resource_milestone_events", feature_category: feature_category do eventable = find_noteable(eventable_type, params[:eventable_id]) events = ResourceMilestoneEventFinder.new(current_user, eventable).execute @@ -38,7 +41,7 @@ module API requires :event_id, type: String, desc: 'The ID of a resource milestone event' requires :eventable_id, types: [Integer, String], desc: 'The ID of the eventable' end - get ":id/#{eventables_str}/:eventable_id/resource_milestone_events/:event_id" do + get ":id/#{eventables_str}/:eventable_id/resource_milestone_events/:event_id", feature_category: feature_category do eventable = find_noteable(eventable_type, params[:eventable_id]) event = eventable.resource_milestone_events.find(params[:event_id]) diff --git a/lib/api/resource_state_events.rb b/lib/api/resource_state_events.rb index 9bfda39be90..3460aa2c00e 100644 --- a/lib/api/resource_state_events.rb +++ b/lib/api/resource_state_events.rb @@ -7,7 +7,10 @@ module API before { authenticate! } - [Issue, MergeRequest].each do |eventable_class| + { + Issue => :issue_tracking, + MergeRequest => :code_review + }.each do |eventable_class, feature_category| eventable_name = eventable_class.to_s.underscore params do @@ -22,7 +25,7 @@ module API use :pagination end - get ":id/#{eventable_name.pluralize}/:eventable_iid/resource_state_events" do + get ":id/#{eventable_name.pluralize}/:eventable_iid/resource_state_events", feature_category: feature_category do eventable = find_noteable(eventable_class, params[:eventable_iid]) events = ResourceStateEventFinder.new(current_user, eventable).execute @@ -37,7 +40,7 @@ module API requires :eventable_iid, types: Integer, desc: "The IID of the #{eventable_name}" requires :event_id, type: Integer, desc: 'The ID of a resource state event' end - get ":id/#{eventable_name.pluralize}/:eventable_iid/resource_state_events/:event_id" do + get ":id/#{eventable_name.pluralize}/:eventable_iid/resource_state_events/:event_id", feature_category: feature_category do eventable = find_noteable(eventable_class, params[:eventable_iid]) event = ResourceStateEventFinder.new(current_user, eventable).find(params[:event_id]) diff --git a/lib/gitlab/ci/artifact_file_reader.rb b/lib/gitlab/ci/artifact_file_reader.rb index b0fad026ec5..d576953c1a0 100644 --- a/lib/gitlab/ci/artifact_file_reader.rb +++ b/lib/gitlab/ci/artifact_file_reader.rb @@ -45,6 +45,14 @@ module Gitlab end def read_zip_file!(file_path) + if ::Feature.enabled?(:ci_new_artifact_file_reader, job.project, default_enabled: false) + read_with_new_artifact_file_reader(file_path) + else + read_with_legacy_artifact_file_reader(file_path) + end + end + + def read_with_new_artifact_file_reader(file_path) job.artifacts_file.use_open_file do |file| zip_file = Zip::File.new(file, false, true) entry = zip_file.find_entry(file_path) @@ -61,6 +69,25 @@ module Gitlab end end + def read_with_legacy_artifact_file_reader(file_path) + job.artifacts_file.use_file do |archive_path| + Zip::File.open(archive_path) do |zip_file| + entry = zip_file.find_entry(file_path) + unless entry + raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!" + end + + if entry.name_is_directory? + raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!" + end + + zip_file.get_input_stream(entry) do |is| + is.read + end + end + end + end + def max_archive_size_in_mb ActiveSupport::NumberHelper.number_to_human_size(MAX_ARCHIVE_SIZE) end diff --git a/lib/gitlab/graphql/authorize/authorize_field_service.rb b/lib/gitlab/graphql/authorize/authorize_field_service.rb index cbf3e7b8429..e8db619f88a 100644 --- a/lib/gitlab/graphql/authorize/authorize_field_service.rb +++ b/lib/gitlab/graphql/authorize/authorize_field_service.rb @@ -46,6 +46,8 @@ module Gitlab # Returns any authorize metadata from @field def field_authorizations + return [] if @field.metadata[:authorize] == true + Array.wrap(@field.metadata[:authorize]) end @@ -54,7 +56,7 @@ module Gitlab # The field is a built-in/scalar type, or a list of scalars # authorize using the parent's object parent_typed_object.object - elsif @field.connection? || resolved_type.is_a?(Array) + elsif @field.connection? || @field.type.list? || resolved_type.is_a?(Array) # The field is a connection or a list of non-built-in types, we'll # authorize each element when rendering nil @@ -75,16 +77,25 @@ module Gitlab # no need to do anything elsif authorizing_object # Authorizing fields representing scalars, or a simple field with an object - resolved_type if allowed_access?(current_user, authorizing_object) + ::Gitlab::Graphql::Lazy.with_value(authorizing_object) do |object| + resolved_type if allowed_access?(current_user, object) + end elsif @field.connection? - # A connection with pagination, modify the visible nodes on the - # connection type in place - resolved_type.object.edge_nodes.to_a.keep_if { |node| allowed_access?(current_user, node) } - resolved_type - elsif resolved_type.is_a? Array + ::Gitlab::Graphql::Lazy.with_value(resolved_type) do |type| + # A connection with pagination, modify the visible nodes on the + # connection type in place + nodes = to_nodes(type) + nodes.keep_if { |node| allowed_access?(current_user, node) } if nodes + type + end + elsif @field.type.list? || resolved_type.is_a?(Array) # A simple list of rendered types each object being an object to authorize - resolved_type.select do |single_object_type| - allowed_access?(current_user, realized(single_object_type).object) + ::Gitlab::Graphql::Lazy.with_value(resolved_type) do |items| + items.select do |single_object_type| + object_type = realized(single_object_type) + object = object_type.try(:object) || object_type + allowed_access?(current_user, object) + end end else raise "Can't authorize #{@field}" @@ -93,18 +104,23 @@ module Gitlab # Ensure that we are dealing with realized objects, not delayed promises def realized(thing) - case thing - when BatchLoader::GraphQL - thing.sync - when GraphQL::Execution::Lazy - thing.value # part of the private api, but we need to unwrap it here. + ::Gitlab::Graphql::Lazy.force(thing) + end + + # Try to get the connection + # can be at type.object or at type + def to_nodes(type) + if type.respond_to?(:nodes) + type.nodes + elsif type.respond_to?(:object) + to_nodes(type.object) else - thing + nil end end def allowed_access?(current_user, object) - object = object.sync if object.respond_to?(:sync) + object = realized(object) authorizations.all? do |ability| Ability.allowed?(current_user, ability, object) diff --git a/lib/gitlab/graphql/lazy.rb b/lib/gitlab/graphql/lazy.rb index a7f7610a041..b25e6b33a76 100644 --- a/lib/gitlab/graphql/lazy.rb +++ b/lib/gitlab/graphql/lazy.rb @@ -3,17 +3,45 @@ module Gitlab module Graphql class Lazy + include Gitlab::Utils::StrongMemoize + + def initialize(&block) + @proc = block + end + + def force + strong_memoize(:force) { self.class.force(@proc.call) } + end + + def then(&block) + self.class.new { yield force } + end + # Force evaluation of a (possibly) lazy value def self.force(value) case value + when ::Gitlab::Graphql::Lazy + value.force when ::BatchLoader::GraphQL value.sync + when ::GraphQL::Execution::Lazy + value.value # part of the private api, but we can force this as well when ::Concurrent::Promise - value.execute.value + value.execute if value.state == :unscheduled + + value.value # value.value(10.seconds) else value end end + + def self.with_value(unforced, &block) + if Feature.enabled?(:graphql_lazy_authorization) + self.new { unforced }.then(&block) + else + block.call(unforced) + end + end end end end diff --git a/lib/gitlab/metrics/requests_rack_middleware.rb b/lib/gitlab/metrics/requests_rack_middleware.rb index 8b2aa4d29d7..a541b27508b 100644 --- a/lib/gitlab/metrics/requests_rack_middleware.rb +++ b/lib/gitlab/metrics/requests_rack_middleware.rb @@ -18,6 +18,15 @@ module Gitlab FEATURE_CATEGORY_HEADER = 'X-Gitlab-Feature-Category' FEATURE_CATEGORY_DEFAULT = 'unknown' + # These were the top 5 categories at a point in time, chosen as a + # reasonable default. If we initialize every category we'll end up + # with an explosion in unused metric combinations, but we want the + # most common ones to be always present. + FEATURE_CATEGORIES_TO_INITIALIZE = ['authentication_and_authorization', + 'code_review', 'continuous_integration', + 'not_owned', 'source_code_management', + FEATURE_CATEGORY_DEFAULT].freeze + def initialize(app) @app = app end @@ -46,16 +55,10 @@ module Gitlab # # For example `rate(http_requests_total{status="500"}[1m])` would return # no data until the first 500 error would occur. - - # The list of feature categories is currently not needed by the application - # anywhere else. So no need to keep these in memory forever. - # Doing it here, means we're only reading the file on boot. - feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:strip).uniq << FEATURE_CATEGORY_DEFAULT - HTTP_METHODS.each do |method, statuses| http_request_duration_seconds.get({ method: method }) - statuses.product(feature_categories) do |status, feature_category| + statuses.product(FEATURE_CATEGORIES_TO_INITIALIZE) do |status, feature_category| http_requests_total.get({ method: method, status: status, feature_category: feature_category }) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d0a533b60bb..b41dbc2b0c6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10884,6 +10884,9 @@ msgstr "" msgid "Expand approvers" msgstr "" +msgid "Expand file" +msgstr "" + msgid "Expand milestones" msgstr "" @@ -11585,6 +11588,9 @@ msgstr "" msgid "Files breadcrumb" msgstr "" +msgid "Files with large changes are collapsed by default." +msgstr "" + msgid "Files, directories, and submodules in the path %{path} for commit reference %{ref}" msgstr "" @@ -11783,7 +11789,7 @@ msgstr "" msgid "For help setting up the Service Desk for your instance, please contact an administrator." msgstr "" -msgid "For internal projects, any logged in user can view pipelines and access job details (output logs and artifacts)" +msgid "For internal projects, any logged in user except external users can view pipelines and access job details (output logs and artifacts)" msgstr "" msgid "For more info, read the documentation." @@ -14418,10 +14424,10 @@ msgstr "" msgid "Internal" msgstr "" -msgid "Internal - The group and any internal projects can be viewed by any logged in user." +msgid "Internal - The group and any internal projects can be viewed by any logged in user except external users." msgstr "" -msgid "Internal - The project can be accessed by any logged in user." +msgid "Internal - The project can be accessed by any logged in user except external users." msgstr "" msgid "Internal URL (optional)" @@ -26477,7 +26483,7 @@ msgstr "" msgid "The global settings require you to enable Two-Factor Authentication for your account." msgstr "" -msgid "The group and any internal projects can be viewed by any logged in user." +msgid "The group and any internal projects can be viewed by any logged in user except external users." msgstr "" msgid "The group and any public projects can be viewed without any authentication." @@ -26588,7 +26594,7 @@ msgstr "" msgid "The private key to use when a client certificate is provided. This value is encrypted at rest." msgstr "" -msgid "The project can be accessed by any logged in user." +msgid "The project can be accessed by any logged in user except external users." msgstr "" msgid "The project can be accessed by any user who is logged in." @@ -26663,7 +26669,7 @@ msgstr "" msgid "The snippet is visible only to project members." msgstr "" -msgid "The snippet is visible to any logged in user." +msgid "The snippet is visible to any logged in user except external users." msgstr "" msgid "The specified tab is invalid, please select another" diff --git a/spec/bin/feature_flag_spec.rb b/spec/bin/feature_flag_spec.rb index 8e0bed5b769..710b1606923 100644 --- a/spec/bin/feature_flag_spec.rb +++ b/spec/bin/feature_flag_spec.rb @@ -13,7 +13,7 @@ RSpec.describe 'bin/feature-flag' do let(:options) { FeatureFlagOptionParser.parse(argv) } let(:creator) { described_class.new(options) } let(:existing_flags) do - { 'existing-feature-flag' => File.join('config', 'feature_flags', 'development', 'existing-feature-flag.yml') } + { 'existing_feature_flag' => File.join('config', 'feature_flags', 'development', 'existing_feature_flag.yml') } end before do @@ -32,12 +32,12 @@ RSpec.describe 'bin/feature-flag' do it 'properly creates a feature flag' do expect(File).to receive(:write).with( - File.join('config', 'feature_flags', 'development', 'feature-flag-name.yml'), + File.join('config', 'feature_flags', 'development', 'feature_flag_name.yml'), anything) expect do subject - end.to output(/name: feature-flag-name/).to_stdout + end.to output(/name: feature_flag_name/).to_stdout end context 'when running on master' do diff --git a/spec/features/merge_request/user_expands_diff_spec.rb b/spec/features/merge_request/user_expands_diff_spec.rb index 0cdc87de761..08a92d5bc21 100644 --- a/spec/features/merge_request/user_expands_diff_spec.rb +++ b/spec/features/merge_request/user_expands_diff_spec.rb @@ -15,11 +15,11 @@ RSpec.describe 'User expands diff', :js do it 'allows user to expand diff' do page.within find('[id="19763941ab80e8c09871c0a425f0560d9053bcb3"]') do - click_link 'Click to expand it.' + find('[data-testid="expand-button"]').click wait_for_requests - expect(page).not_to have_content('Click to expand it.') + expect(page).not_to have_content('Expand file') expect(page).to have_selector('.code') end end diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js index 4a41b5b4f98..86b5d497a07 100644 --- a/spec/frontend/diffs/components/diff_file_spec.js +++ b/spec/frontend/diffs/components/diff_file_spec.js @@ -90,7 +90,7 @@ function createComponent({ file }) { const findDiffHeader = wrapper => wrapper.find(DiffFileHeaderComponent); const findDiffContentArea = wrapper => wrapper.find('[data-testid="content-area"]'); const findLoader = wrapper => wrapper.find('[data-testid="loader-icon"]'); -const findToggleLinks = wrapper => wrapper.findAll('[data-testid="toggle-link"]'); +const findToggleButton = wrapper => wrapper.find('[data-testid="expand-button"]'); const toggleFile = wrapper => findDiffHeader(wrapper).vm.$emit('toggleFile'); const isDisplayNone = element => element.style.display === 'none'; @@ -187,9 +187,11 @@ describe('DiffFile', () => { makeFileAutomaticallyCollapsed(store); }); - it('should show the collapsed file warning with expansion link', () => { - expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed'); - expect(findToggleLinks(wrapper).length).toEqual(1); + it('should show the collapsed file warning with expansion button', () => { + expect(findDiffContentArea(wrapper).html()).toContain( + 'Files with large changes are collapsed by default.', + ); + expect(findToggleButton(wrapper).exists()).toBe(true); }); it('should style the component so that it `.has-body` for layout purposes', () => { @@ -292,8 +294,10 @@ describe('DiffFile', () => { await wrapper.vm.$nextTick(); - expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed'); - expect(findToggleLinks(wrapper).length).toEqual(1); + expect(findDiffContentArea(wrapper).html()).toContain( + 'Files with large changes are collapsed by default.', + ); + expect(findToggleButton(wrapper).exists()).toBe(true); }); it.each` diff --git a/spec/frontend/groups/mock_data.js b/spec/frontend/groups/mock_data.js index 380dda9f7b1..603cb27deec 100644 --- a/spec/frontend/groups/mock_data.js +++ b/spec/frontend/groups/mock_data.js @@ -7,13 +7,14 @@ export const ITEM_TYPE = { export const GROUP_VISIBILITY_TYPE = { public: 'Public - The group and any public projects can be viewed without any authentication.', - internal: 'Internal - The group and any internal projects can be viewed by any logged in user.', + internal: + 'Internal - The group and any internal projects can be viewed by any logged in user except external users.', private: 'Private - The group and its projects can only be viewed by members.', }; export const PROJECT_VISIBILITY_TYPE = { public: 'Public - The project can be accessed without any authentication.', - internal: 'Internal - The project can be accessed by any logged in user.', + internal: 'Internal - The project can be accessed by any logged in user except external users.', private: 'Private - Project access must be granted explicitly to each user. If this project is part of a group, access will be granted to members of the group.', }; diff --git a/spec/graphql/types/release_links_type_spec.rb b/spec/graphql/types/release_links_type_spec.rb index e46dc0ddffe..d3494e0e86c 100644 --- a/spec/graphql/types/release_links_type_spec.rb +++ b/spec/graphql/types/release_links_type_spec.rb @@ -8,10 +8,10 @@ RSpec.describe GitlabSchema.types['ReleaseLinks'] do it 'has the expected fields' do expected_fields = %w[ selfUrl - openMergeRequestsUrl + openedMergeRequestsUrl mergedMergeRequestsUrl closedMergeRequestsUrl - openIssuesUrl + openedIssuesUrl closedIssuesUrl editUrl mergeRequestsUrl diff --git a/spec/lib/api/every_api_endpoint_spec.rb b/spec/lib/api/every_api_endpoint_spec.rb index ac424a511c0..2f5c646c61f 100644 --- a/spec/lib/api/every_api_endpoint_spec.rb +++ b/spec/lib/api/every_api_endpoint_spec.rb @@ -37,7 +37,15 @@ RSpec.describe 'Every API endpoint' do ::API::Lint, ::API::Markdown, ::API::Members, ::API::MergeRequestDiffs, ::API::MergeRequests, ::API::MergeRequestApprovals, ::API::Metrics::Dashboard::Annotations, ::API::Metrics::UserStarredDashboards, ::API::Namespaces, ::API::Notes, - ::API::Discussions + ::API::Discussions, ::API::ResourceLabelEvents, ::API::ResourceMilestoneEvents, + ::API::ResourceStateEvents, ::API::NotificationSettings, ::API::ProjectPackages, + ::API::GroupPackages, ::API::PackageFiles, ::API::NugetPackages, ::API::PypiPackages, + ::API::ComposerPackages, ::API::ConanProjectPackages, ::API::ConanInstancePackages, + ::API::DebianGroupPackages, ::API::DebianProjectPackages, ::API::MavenPackages, + ::API::NpmPackages, ::API::GenericPackages, ::API::GoProxy, ::API::Pages, + ::API::PagesDomains, ::API::ProjectClusters, ::API::ProjectContainerRepositories, + ::API::ProjectEvents, ::API::ProjectExport, ::API::ProjectImport, ::API::ProjectHooks, + ::API::ProjectMilestones, ::API::ProjectRepositoryStorageMoves, ::API::Projects ] next unless completed_classes.include?(klass) diff --git a/spec/lib/gitlab/badge/coverage/report_spec.rb b/spec/lib/gitlab/badge/coverage/report_spec.rb index 4a9508712a4..ebe6b8603b1 100644 --- a/spec/lib/gitlab/badge/coverage/report_spec.rb +++ b/spec/lib/gitlab/badge/coverage/report_spec.rb @@ -3,13 +3,22 @@ require 'spec_helper' RSpec.describe Gitlab::Badge::Coverage::Report do - let(:project) { create(:project, :repository) } - let(:job_name) { nil } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) } + + let_it_be(:builds) do + [ + create(:ci_build, :success, pipeline: pipeline, coverage: 40, name: 'first'), + create(:ci_build, :success, pipeline: pipeline, coverage: 60) + ] + end let(:badge) do described_class.new(project, 'master', opts: { job: job_name }) end + let(:job_name) { nil } + describe '#entity' do it 'describes a coverage' do expect(badge.entity).to eq 'coverage' @@ -28,81 +37,47 @@ RSpec.describe Gitlab::Badge::Coverage::Report do end end - shared_examples 'unknown coverage report' do - context 'particular job specified' do - let(:job_name) { '' } - - it 'returns nil' do - expect(badge.status).to be_nil - end + describe '#status' do + before do + allow(badge).to receive(:pipeline).and_return(pipeline) end - context 'particular job not specified' do - let(:job_name) { nil } + context 'with no pipeline' do + let(:pipeline) { nil } it 'returns nil' do expect(badge.status).to be_nil end end - end - - context 'when latest successful pipeline exists' do - before do - create_pipeline do |pipeline| - create(:ci_build, :success, pipeline: pipeline, name: 'first', coverage: 40) - create(:ci_build, :success, pipeline: pipeline, coverage: 60) - end - - create_pipeline do |pipeline| - create(:ci_build, :failed, pipeline: pipeline, coverage: 10) - end - end - - context 'when particular job specified' do - let(:job_name) { 'first' } - it 'returns coverage for the particular job' do - expect(badge.status).to eq 40 + context 'with no job specified' do + it 'returns the pipeline coverage value' do + expect(badge.status).to eq(50.00) end end - context 'when particular job not specified' do - let(:job_name) { '' } - - it 'returns arithemetic mean for the pipeline' do - expect(badge.status).to eq 50 - end - end - end + context 'with a blank job name' do + let(:job_name) { ' ' } - context 'when only failed pipeline exists' do - before do - create_pipeline do |pipeline| - create(:ci_build, :failed, pipeline: pipeline, coverage: 10) + it 'returns the pipeline coverage value' do + expect(badge.status).to eq(50.00) end end - it_behaves_like 'unknown coverage report' + context 'with an unmatching job name specified' do + let(:job_name) { 'incorrect name' } - context 'particular job specified' do - let(:job_name) { 'nonexistent' } - - it 'retruns nil' do + it 'returns nil' do expect(badge.status).to be_nil end end - end - context 'pipeline does not exist' do - it_behaves_like 'unknown coverage report' - end - - def create_pipeline - opts = { project: project, sha: project.commit.id, ref: 'master' } + context 'with a matching job name specified' do + let(:job_name) { 'first' } - create(:ci_pipeline, opts).tap do |pipeline| - yield pipeline - ::Ci::ProcessPipelineService.new(pipeline).execute + it 'returns the pipeline coverage value' do + expect(badge.status).to eq(40.00) + end end end end diff --git a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb index e982f0eb015..83a37655ea9 100644 --- a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb +++ b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb @@ -18,6 +18,17 @@ RSpec.describe Gitlab::Ci::ArtifactFileReader do expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') end + context 'when FF ci_new_artifact_file_reader is disabled' do + before do + stub_feature_flags(ci_new_artifact_file_reader: false) + end + + it 'returns the content at the path' do + is_expected.to be_present + expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') + end + end + context 'when path does not exist' do let(:path) { 'file/does/not/exist.txt' } let(:expected_error) do diff --git a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb index 7576523ce52..fca08ebf48b 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb @@ -27,13 +27,17 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do end end + def resolve + service.authorized_resolve[type_instance, {}, context] + end + subject(:service) { described_class.new(field) } describe '#authorized_resolve' do let_it_be(:current_user) { build(:user) } let_it_be(:presented_object) { 'presented object' } let_it_be(:query_type) { GraphQL::ObjectType.new } - let_it_be(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)} + let_it_be(:schema) { GitlabSchema } let_it_be(:query) { GraphQL::Query.new(schema, document: nil, context: {}, variables: {}) } let_it_be(:context) { GraphQL::Query::Context.new(query: query, values: { current_user: current_user }, object: nil) } @@ -41,125 +45,211 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeFieldService do let(:type_instance) { type_class.authorized_new(presented_object, context) } let(:field) { type_class.fields['testField'].to_graphql } - subject(:resolved) { service.authorized_resolve.call(type_instance, {}, context) } + subject(:resolved) { resolve&.force } - context 'scalar types' do - shared_examples 'checking permissions on the presented object' do - it 'checks the abilities on the object being presented and returns the value' do - expected_permissions.each do |permission| - spy_ability_check_for(permission, presented_object, passed: true) - end + context 'reading the field of a lazy value' do + let(:ability) { :read_field } + let(:presented_object) { lazy_upcase('a') } + let(:type_class) { type_with_field(GraphQL::STRING_TYPE, ability) } - expect(resolved).to eq('Resolved value') + let(:upcaser) do + Module.new do + def self.upcase(strs) + strs.map(&:upcase) + end end + end - it 'returns nil if the value was not authorized' do - allow(Ability).to receive(:allowed?).and_return false - - expect(resolved).to be_nil + def lazy_upcase(str) + ::BatchLoader::GraphQL.for(str).batch do |strs, found| + strs.zip(upcaser.upcase(strs)).each { |s, us| found[s, us] } end end - context 'when the field is a built-in scalar type' do - let(:type_class) { type_with_field(GraphQL::STRING_TYPE, :read_field) } - let(:expected_permissions) { [:read_field] } + it 'does not run authorizations until we force the resolved value' do + expect(Ability).not_to receive(:allowed?) - it_behaves_like 'checking permissions on the presented object' + expect(resolve).to respond_to(:force) end - context 'when the field is a list of scalar types' do - let(:type_class) { type_with_field([GraphQL::STRING_TYPE], :read_field) } - let(:expected_permissions) { [:read_field] } + it 'runs authorizations when we force the resolved value' do + spy_ability_check_for(ability, 'A') - it_behaves_like 'checking permissions on the presented object' + expect(resolved).to eq('Resolved value') end - context 'when the field is sub-classed scalar type' do - let(:type_class) { type_with_field(Types::TimeType, :read_field) } - let(:expected_permissions) { [:read_field] } + it 'redacts values that fail the permissions check' do + spy_ability_check_for(ability, 'A', passed: false) - it_behaves_like 'checking permissions on the presented object' + expect(resolved).to be_nil end - context 'when the field is a list of sub-classed scalar types' do - let(:type_class) { type_with_field([Types::TimeType], :read_field) } - let(:expected_permissions) { [:read_field] } + context 'we batch two calls' do + def resolve(value) + instance = type_class.authorized_new(lazy_upcase(value), context) + service.authorized_resolve[instance, {}, context] + end - it_behaves_like 'checking permissions on the presented object' - end - end + it 'batches resolution, but authorizes each object separately' do + expect(upcaser).to receive(:upcase).once.and_call_original + spy_ability_check_for(:read_field, 'A', passed: true) + spy_ability_check_for(:read_field, 'B', passed: false) + spy_ability_check_for(:read_field, 'C', passed: true) - context 'when the field is a connection' do - context 'when it resolves to nil' do - let(:type_class) { type_with_field(Types::QueryType.connection_type, :read_field, nil) } + a = resolve('a') + b = resolve('b') + c = resolve('c') - it 'does not fail when authorizing' do - expect(resolved).to be_nil + expect(a.force).to be_present + expect(b.force).to be_nil + expect(c.force).to be_present end end end - context 'when the field is a specific type' do - let(:custom_type) { type(:read_type) } - let(:object_in_field) { double('presented in field') } + shared_examples 'authorizing fields' do + context 'scalar types' do + shared_examples 'checking permissions on the presented object' do + it 'checks the abilities on the object being presented and returns the value' do + expected_permissions.each do |permission| + spy_ability_check_for(permission, presented_object, passed: true) + end - let(:type_class) { type_with_field(custom_type, :read_field, object_in_field) } - let(:type_instance) { type_class.authorized_new(object_in_field, context) } + expect(resolved).to eq('Resolved value') + end - subject(:resolved) { service.authorized_resolve.call(type_instance, {}, context) } + it 'returns nil if the value was not authorized' do + allow(Ability).to receive(:allowed?).and_return false - it 'checks both field & type permissions' do - spy_ability_check_for(:read_field, object_in_field, passed: true) - spy_ability_check_for(:read_type, object_in_field, passed: true) + expect(resolved).to be_nil + end + end - expect(resolved).to eq(object_in_field) - end + context 'when the field is a built-in scalar type' do + let(:type_class) { type_with_field(GraphQL::STRING_TYPE, :read_field) } + let(:expected_permissions) { [:read_field] } - it 'returns nil if viewing was not allowed' do - spy_ability_check_for(:read_field, object_in_field, passed: false) - spy_ability_check_for(:read_type, object_in_field, passed: true) + it_behaves_like 'checking permissions on the presented object' + end - expect(resolved).to be_nil + context 'when the field is a list of scalar types' do + let(:type_class) { type_with_field([GraphQL::STRING_TYPE], :read_field) } + let(:expected_permissions) { [:read_field] } + + it_behaves_like 'checking permissions on the presented object' + end + + context 'when the field is sub-classed scalar type' do + let(:type_class) { type_with_field(Types::TimeType, :read_field) } + let(:expected_permissions) { [:read_field] } + + it_behaves_like 'checking permissions on the presented object' + end + + context 'when the field is a list of sub-classed scalar types' do + let(:type_class) { type_with_field([Types::TimeType], :read_field) } + let(:expected_permissions) { [:read_field] } + + it_behaves_like 'checking permissions on the presented object' + end end - context 'when the field is not nullable' do - let(:type_class) { type_with_field(custom_type, :read_field, object_in_field, null: false) } + context 'when the field is a connection' do + context 'when it resolves to nil' do + let(:type_class) { type_with_field(Types::QueryType.connection_type, :read_field, nil) } - it 'returns nil when viewing is not allowed' do - spy_ability_check_for(:read_type, object_in_field, passed: false) + it 'does not fail when authorizing' do + expect(resolved).to be_nil + end + end - expect(resolved).to be_nil + context 'when it returns values' do + let(:objects) { [1, 2, 3] } + let(:field_type) { type([:read_object]).connection_type } + let(:type_class) { type_with_field(field_type, [], objects) } + + it 'filters out unauthorized values' do + spy_ability_check_for(:read_object, 1, passed: true) + spy_ability_check_for(:read_object, 2, passed: false) + spy_ability_check_for(:read_object, 3, passed: true) + + expect(resolved.nodes).to eq [1, 3] + end end end - context 'when the field is a list' do - let(:object_1) { double('presented in field 1') } - let(:object_2) { double('presented in field 2') } - let(:presented_types) { [double(object: object_1), double(object: object_2)] } + context 'when the field is a specific type' do + let(:custom_type) { type(:read_type) } + let(:object_in_field) { double('presented in field') } + + let(:type_class) { type_with_field(custom_type, :read_field, object_in_field) } + let(:type_instance) { type_class.authorized_new(object_in_field, context) } - let(:type_class) { type_with_field([custom_type], :read_field, presented_types) } - let(:type_instance) { type_class.authorized_new(presented_types, context) } + it 'checks both field & type permissions' do + spy_ability_check_for(:read_field, object_in_field, passed: true) + spy_ability_check_for(:read_type, object_in_field, passed: true) - it 'checks all permissions' do - allow(Ability).to receive(:allowed?) { true } + expect(resolved).to eq(object_in_field) + end - spy_ability_check_for(:read_field, object_1, passed: true) - spy_ability_check_for(:read_type, object_1, passed: true) - spy_ability_check_for(:read_field, object_2, passed: true) - spy_ability_check_for(:read_type, object_2, passed: true) + it 'returns nil if viewing was not allowed' do + spy_ability_check_for(:read_field, object_in_field, passed: false) + spy_ability_check_for(:read_type, object_in_field, passed: true) - expect(resolved).to eq(presented_types) + expect(resolved).to be_nil + end + + context 'when the field is not nullable' do + let(:type_class) { type_with_field(custom_type, :read_field, object_in_field, null: false) } + + it 'returns nil when viewing is not allowed' do + spy_ability_check_for(:read_type, object_in_field, passed: false) + + expect(resolved).to be_nil + end end - it 'filters out objects that the user cannot see' do - allow(Ability).to receive(:allowed?) { true } + context 'when the field is a list' do + let(:object_1) { double('presented in field 1') } + let(:object_2) { double('presented in field 2') } + let(:presented_types) { [double(object: object_1), double(object: object_2)] } - spy_ability_check_for(:read_type, object_1, passed: false) + let(:type_class) { type_with_field([custom_type], :read_field, presented_types) } + let(:type_instance) { type_class.authorized_new(presented_types, context) } - expect(resolved.map(&:object)).to contain_exactly(object_2) + it 'checks all permissions' do + allow(Ability).to receive(:allowed?) { true } + + spy_ability_check_for(:read_field, object_1, passed: true) + spy_ability_check_for(:read_type, object_1, passed: true) + spy_ability_check_for(:read_field, object_2, passed: true) + spy_ability_check_for(:read_type, object_2, passed: true) + + expect(resolved).to eq(presented_types) + end + + it 'filters out objects that the user cannot see' do + allow(Ability).to receive(:allowed?) { true } + + spy_ability_check_for(:read_type, object_1, passed: false) + + expect(resolved).to contain_exactly(have_attributes(object: object_2)) + end end end end + + it_behaves_like 'authorizing fields' + + context 'the graphql_lazy_authorization feature flag is disabled' do + before do + stub_feature_flags(graphql_lazy_authorization: false) + end + + subject(:resolved) { resolve } + + it_behaves_like 'authorizing fields' + end end private diff --git a/spec/lib/gitlab/graphql/lazy_spec.rb b/spec/lib/gitlab/graphql/lazy_spec.rb new file mode 100644 index 00000000000..795978ab0a4 --- /dev/null +++ b/spec/lib/gitlab/graphql/lazy_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Graphql::Lazy do + def load(key) + BatchLoader.for(key).batch do |keys, loader| + keys.each { |x| loader.call(x, x * x) } + end + end + + let(:value) { double(x: 1) } + + describe '#force' do + subject { described_class.new { value.x } } + + it 'can extract the value' do + expect(subject.force).to be 1 + end + + it 'can derive new lazy values' do + expect(subject.then { |x| x + 2 }.force).to be 3 + end + + it 'only evaluates once' do + expect(value).to receive(:x).once + + expect(subject.force).to eq(subject.force) + end + + it 'deals with nested laziness' do + expect(described_class.new { load(10) }.force).to eq(100) + expect(described_class.new { described_class.new { 5 } }.force).to eq 5 + end + end + + describe '.with_value' do + let(:inner) { described_class.new { value.x } } + + subject { described_class.with_value(inner) { |x| x.to_s } } + + it 'defers the application of a block to a value' do + expect(value).not_to receive(:x) + + expect(subject).to be_an_instance_of(described_class) + end + + it 'evaluates to the application of the block to the value' do + expect(value).to receive(:x).once + + expect(subject.force).to eq(inner.force.to_s) + end + end + + describe '.force' do + context 'when given a plain value' do + subject { described_class.force(1) } + + it 'unwraps the value' do + expect(subject).to be 1 + end + end + + context 'when given a wrapped lazy value' do + subject { described_class.force(described_class.new { 2 }) } + + it 'unwraps the value' do + expect(subject).to be 2 + end + end + + context 'when the value is from a batchloader' do + subject { described_class.force(load(3)) } + + it 'syncs the value' do + expect(subject).to be 9 + end + end + + context 'when the value is a GraphQL lazy' do + subject { described_class.force(GitlabSchema.after_lazy(load(3)) { |x| x + 1 } ) } + + it 'forces the evaluation' do + expect(subject).to be 10 + end + end + + context 'when the value is a promise' do + subject { described_class.force(::Concurrent::Promise.new { 4 }) } + + it 'executes the promise and waits for the value' do + expect(subject).to be 4 + end + end + end +end diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 88c889b4055..1f7daaa308d 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -129,12 +129,11 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do describe '.initialize_metrics', :prometheus do it "sets labels for http_requests_total" do - feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:strip) << described_class::FEATURE_CATEGORY_DEFAULT expected_labels = [] described_class::HTTP_METHODS.each do |method, statuses| statuses.each do |status| - feature_categories.each do |feature_category| + described_class::FEATURE_CATEGORIES_TO_INITIALIZE.each do |feature_category| expected_labels << { method: method.to_s, status: status.to_s, feature_category: feature_category.to_s } end end @@ -152,6 +151,13 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class.http_request_duration_seconds.values.keys).to include(*expected_labels) end + + it 'has every label in config/feature_categories.yml' do + defaults = [described_class::FEATURE_CATEGORY_DEFAULT, 'not_owned'] + feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:strip) + defaults + + expect(described_class::FEATURE_CATEGORIES_TO_INITIALIZE).to all(be_in(feature_categories)) + end end end end diff --git a/spec/presenters/release_presenter_spec.rb b/spec/presenters/release_presenter_spec.rb index 63c35bcf3ac..b518584569b 100644 --- a/spec/presenters/release_presenter_spec.rb +++ b/spec/presenters/release_presenter_spec.rb @@ -64,8 +64,8 @@ RSpec.describe ReleasePresenter do end end - describe '#open_merge_requests_url' do - subject { presenter.open_merge_requests_url } + describe '#opened_merge_requests_url' do + subject { presenter.opened_merge_requests_url } it 'returns merge requests url with state=open' do is_expected.to eq(project_merge_requests_url(project, opened_url_params)) @@ -112,8 +112,8 @@ RSpec.describe ReleasePresenter do end end - describe '#open_issues_url' do - subject { presenter.open_issues_url } + describe '#opened_issues_url' do + subject { presenter.opened_issues_url } it 'returns issues url with state=open' do is_expected.to eq(project_issues_url(project, opened_url_params)) diff --git a/spec/requests/api/graphql/project/release_spec.rb b/spec/requests/api/graphql/project/release_spec.rb index ebb588981d6..ade7cc177be 100644 --- a/spec/requests/api/graphql/project/release_spec.rb +++ b/spec/requests/api/graphql/project/release_spec.rb @@ -184,10 +184,10 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let(:release_fields) do query_graphql_field(:links, nil, %{ selfUrl - openMergeRequestsUrl + openedMergeRequestsUrl mergedMergeRequestsUrl closedMergeRequestsUrl - openIssuesUrl + openedIssuesUrl closedIssuesUrl mergeRequestsUrl issuesUrl @@ -199,10 +199,10 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do expect(data).to eq( 'selfUrl' => project_release_url(project, release), - 'openMergeRequestsUrl' => project_merge_requests_url(project, opened_url_params), + 'openedMergeRequestsUrl' => project_merge_requests_url(project, opened_url_params), 'mergedMergeRequestsUrl' => project_merge_requests_url(project, merged_url_params), 'closedMergeRequestsUrl' => project_merge_requests_url(project, closed_url_params), - 'openIssuesUrl' => project_issues_url(project, opened_url_params), + 'openedIssuesUrl' => project_issues_url(project, opened_url_params), 'closedIssuesUrl' => project_issues_url(project, closed_url_params), 'mergeRequestsUrl' => project_merge_requests_url(project, opened_url_params), 'issuesUrl' => project_issues_url(project, opened_url_params) diff --git a/spec/requests/api/graphql/project/releases_spec.rb b/spec/requests/api/graphql/project/releases_spec.rb index 46ba1cdd1ea..9288dab3f2e 100644 --- a/spec/requests/api/graphql/project/releases_spec.rb +++ b/spec/requests/api/graphql/project/releases_spec.rb @@ -42,10 +42,10 @@ RSpec.describe 'Query.project(fullPath).releases()' do } links { selfUrl - openMergeRequestsUrl + openedMergeRequestsUrl mergedMergeRequestsUrl closedMergeRequestsUrl - openIssuesUrl + openedIssuesUrl closedIssuesUrl mergeRequestsUrl issuesUrl @@ -111,10 +111,10 @@ RSpec.describe 'Query.project(fullPath).releases()' do }, 'links' => { 'selfUrl' => project_release_url(project, release), - 'openMergeRequestsUrl' => project_merge_requests_url(project, opened_url_params), + 'openedMergeRequestsUrl' => project_merge_requests_url(project, opened_url_params), 'mergedMergeRequestsUrl' => project_merge_requests_url(project, merged_url_params), 'closedMergeRequestsUrl' => project_merge_requests_url(project, closed_url_params), - 'openIssuesUrl' => project_issues_url(project, opened_url_params), + 'openedIssuesUrl' => project_issues_url(project, opened_url_params), 'closedIssuesUrl' => project_issues_url(project, closed_url_params), 'mergeRequestsUrl' => project_merge_requests_url(project, opened_url_params), 'issuesUrl' => project_issues_url(project, opened_url_params) diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 01f8bcd242c..1844e12ed7a 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -478,6 +478,8 @@ module GraphqlHelpers use Gitlab::Graphql::Authorize use Gitlab::Graphql::Pagination::Connections + lazy_resolve ::Gitlab::Graphql::Lazy, :force + query(query_type) end diff --git a/spec/support/shared_examples/graphql/label_fields.rb b/spec/support/shared_examples/graphql/label_fields.rb index b1bfb395bc6..caf5dae409a 100644 --- a/spec/support/shared_examples/graphql/label_fields.rb +++ b/spec/support/shared_examples/graphql/label_fields.rb @@ -106,13 +106,11 @@ RSpec.shared_examples 'querying a GraphQL type with labels' do end it 'batches queries for labels by title' do - pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/217767') - multi_selection = query_for(label_b, label_c) single_selection = query_for(label_d) expect { run_query(multi_selection) } - .to issue_same_number_of_queries_as { run_query(single_selection) } + .to issue_same_number_of_queries_as { run_query(single_selection) }.ignoring_cached_queries end end diff --git a/spec/workers/ci/delete_objects_worker_spec.rb b/spec/workers/ci/delete_objects_worker_spec.rb index 6cb8e0cba37..5ff3b1d724a 100644 --- a/spec/workers/ci/delete_objects_worker_spec.rb +++ b/spec/workers/ci/delete_objects_worker_spec.rb @@ -9,9 +9,14 @@ RSpec.describe Ci::DeleteObjectsWorker do describe '#perform' do it 'executes a service' do + allow(worker).to receive(:max_running_jobs).and_return(25) + expect_next_instance_of(Ci::DeleteObjectsService) do |instance| expect(instance).to receive(:execute) - expect(instance).to receive(:remaining_batches_count).once.and_call_original + expect(instance).to receive(:remaining_batches_count) + .with(max_batch_count: 25) + .once + .and_call_original end worker.perform |