From 4b41b57abf3ad9c2e0e81b3804cb01af6f879349 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 15 Jul 2022 06:09:57 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/assets/javascripts/issues/index.js | 2 + .../components/resource_links_block.vue | 89 +++++ .../javascripts/linked_resources/constants.js | 5 + .../javascripts/linked_resources/index.js | 28 ++ .../set_status_modal_wrapper.vue | 163 ++++---- .../work_item_links/work_item_links.vue | 42 ++- .../work_item_links/work_item_links_form.vue | 77 +++- .../graphql/project_work_items.query.graphql | 5 +- .../graphql/update_work_item.mutation.graphql | 1 + .../graphql/work_item.fragment.graphql | 7 + app/assets/stylesheets/framework/header.scss | 3 +- app/views/profiles/show.html.haml | 14 +- .../issues/_linked_resources.html.haml | 4 + .../issue_type/_details_content.html.haml | 1 + .../counts_all/20210216181918_releases.yml | 2 +- doc/development/feature_flags/index.md | 5 + doc/development/geo.md | 5 + doc/development/geo/proxying.md | 356 ++++++++++++++++++ doc/user/application_security/sast/index.md | 8 +- lib/gitlab/git.rb | 2 + lib/gitlab/gitaly_client.rb | 16 + lib/gitlab/gitaly_client/operation_service.rb | 25 +- lib/gitlab/gitaly_client/ref_service.rb | 11 + locale/gitlab.pot | 20 +- scripts/pipeline_test_report_builder.rb | 2 +- .../resource_links_block_spec.js.snap | 70 ++++ .../components/resource_links_block_spec.js | 35 ++ .../set_status_modal_wrapper_spec.js | 22 +- .../work_item_links_form_spec.js | 24 +- .../work_item_links/work_item_links_spec.js | 30 +- spec/frontend/work_items/mock_data.js | 32 +- .../gitaly_client/operation_service_spec.rb | 13 - .../gitlab/gitaly_client/ref_service_spec.rb | 43 ++- spec/lib/gitlab/gitaly_client_spec.rb | 40 ++ spec/spec_helper.rb | 1 + .../support/helpers/detailed_error_helpers.rb | 19 + workhorse/internal/log/logging.go | 8 - workhorse/internal/log/logging_test.go | 11 - workhorse/internal/upstream/upstream.go | 5 - 39 files changed, 1031 insertions(+), 215 deletions(-) create mode 100644 app/assets/javascripts/linked_resources/components/resource_links_block.vue create mode 100644 app/assets/javascripts/linked_resources/constants.js create mode 100644 app/assets/javascripts/linked_resources/index.js create mode 100644 app/views/projects/issues/_linked_resources.html.haml create mode 100644 doc/development/geo/proxying.md create mode 100644 spec/frontend/issuable/linked_resources/components/__snapshots__/resource_links_block_spec.js.snap create mode 100644 spec/frontend/issuable/linked_resources/components/resource_links_block_spec.js create mode 100644 spec/support/helpers/detailed_error_helpers.rb diff --git a/app/assets/javascripts/issues/index.js b/app/assets/javascripts/issues/index.js index 67c6c723dcc..380bb5f5346 100644 --- a/app/assets/javascripts/issues/index.js +++ b/app/assets/javascripts/issues/index.js @@ -23,6 +23,7 @@ import initNotesApp from '~/notes'; import { store } from '~/notes/stores'; import ZenMode from '~/zen_mode'; import initAwardsApp from '~/emoji/awards_app'; +import initLinkedResources from '~/linked_resources'; import FilteredSearchServiceDesk from './filtered_search_service_desk'; export function initFilteredSearchServiceDesk() { @@ -59,6 +60,7 @@ export function initShow() { if (issueType === IssueType.Incident) { initIncidentApp({ ...issuableData, issuableId: el.dataset.issuableId }); initHeaderActions(store, IssueType.Incident); + initLinkedResources(); initRelatedIssues(IssueType.Incident); } else { initIssueApp(issuableData, store); diff --git a/app/assets/javascripts/linked_resources/components/resource_links_block.vue b/app/assets/javascripts/linked_resources/components/resource_links_block.vue new file mode 100644 index 00000000000..3bfee61df15 --- /dev/null +++ b/app/assets/javascripts/linked_resources/components/resource_links_block.vue @@ -0,0 +1,89 @@ + + + diff --git a/app/assets/javascripts/linked_resources/constants.js b/app/assets/javascripts/linked_resources/constants.js new file mode 100644 index 00000000000..358de326830 --- /dev/null +++ b/app/assets/javascripts/linked_resources/constants.js @@ -0,0 +1,5 @@ +import { __ } from '~/locale'; + +export const LINKED_RESOURCES_HEADER_TEXT = __('Linked resources'); +export const LINKED_RESOURCES_HELP_TEXT = __('Read more about linked resources'); +export const LINKED_RESOURCES_ADD_BUTTON_TEXT = __('Add a resource link'); diff --git a/app/assets/javascripts/linked_resources/index.js b/app/assets/javascripts/linked_resources/index.js new file mode 100644 index 00000000000..4ac9ca31a84 --- /dev/null +++ b/app/assets/javascripts/linked_resources/index.js @@ -0,0 +1,28 @@ +import Vue from 'vue'; +import { parseBoolean } from '~/lib/utils/common_utils'; +import ResourceLinksBlock from './components/resource_links_block.vue'; + +export default function initLinkedResources() { + const linkedResourcesRootElement = document.querySelector('.js-linked-resources-root'); + + if (linkedResourcesRootElement) { + const { issuableId, canAddResourceLinks, helpPath } = linkedResourcesRootElement.dataset; + + // eslint-disable-next-line no-new + new Vue({ + el: linkedResourcesRootElement, + name: 'LinkedResourcesRoot', + components: { + resourceLinksBlock: ResourceLinksBlock, + }, + render: (createElement) => + createElement('resource-links-block', { + props: { + issuableId, + helpPath, + canAddResourceLinks: parseBoolean(canAddResourceLinks), + }, + }), + }); + } +} diff --git a/app/assets/javascripts/set_status_modal/set_status_modal_wrapper.vue b/app/assets/javascripts/set_status_modal/set_status_modal_wrapper.vue index eb0931c6fe2..579316f481c 100644 --- a/app/assets/javascripts/set_status_modal/set_status_modal_wrapper.vue +++ b/app/assets/javascripts/set_status_modal/set_status_modal_wrapper.vue @@ -1,10 +1,13 @@ - - {{ s__('WorkItem|Add') }} + + {{ s__('WorkItem|Add task') }} - + {{ s__('WorkItem|Cancel') }} diff --git a/app/assets/javascripts/work_items/graphql/project_work_items.query.graphql b/app/assets/javascripts/work_items/graphql/project_work_items.query.graphql index 173a29be6a9..7d38d203b84 100644 --- a/app/assets/javascripts/work_items/graphql/project_work_items.query.graphql +++ b/app/assets/javascripts/work_items/graphql/project_work_items.query.graphql @@ -1,11 +1,12 @@ -query projectWorkItems($searchTerm: String, $projectPath: ID!) { +query projectWorkItems($searchTerm: String, $projectPath: ID!, $types: [IssueType!]) { workspace: project(fullPath: $projectPath) { id - workItems(search: $searchTerm) { + workItems(search: $searchTerm, types: $types) { edges { node { id title + state } } } diff --git a/app/assets/javascripts/work_items/graphql/update_work_item.mutation.graphql b/app/assets/javascripts/work_items/graphql/update_work_item.mutation.graphql index c0b6e856411..25eb8099251 100644 --- a/app/assets/javascripts/work_items/graphql/update_work_item.mutation.graphql +++ b/app/assets/javascripts/work_items/graphql/update_work_item.mutation.graphql @@ -5,5 +5,6 @@ mutation workItemUpdate($input: WorkItemUpdateInput!) { workItem { ...WorkItem } + errors } } diff --git a/app/assets/javascripts/work_items/graphql/work_item.fragment.graphql b/app/assets/javascripts/work_items/graphql/work_item.fragment.graphql index 78ed8d43c77..5f64eda96aa 100644 --- a/app/assets/javascripts/work_items/graphql/work_item.fragment.graphql +++ b/app/assets/javascripts/work_items/graphql/work_item.fragment.graphql @@ -35,6 +35,13 @@ fragment WorkItem on WorkItem { iid title } + children { + edges { + node { + id + } + } + } } } } diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index ced62926218..37f92d3cf3d 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -506,8 +506,7 @@ max-width: unset; } - .no-emoji-placeholder, - .clear-user-status { + .no-emoji-placeholder { svg { fill: var(--gray-500, $gray-500); } diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 343a6a39b24..dda1640968e 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -44,20 +44,18 @@ %p= s_("Profiles|This emoji and message will appear on your profile and throughout the interface.") .col-lg-8 = f.fields_for :status, @user.status do |status_form| - - emoji_button = button_tag type: :button, - class: 'js-toggle-emoji-menu emoji-menu-toggle-button btn gl-button btn-default has-tooltip', - title: s_("Profiles|Add status emoji") do + - emoji_button = render Pajamas::ButtonComponent.new(button_options: { title: s_("Profiles|Add status emoji"), + class: 'js-toggle-emoji-menu emoji-menu-toggle-button has-tooltip' } ) do - if custom_emoji = emoji_icon(@user.status.emoji, class: 'gl-mr-0!') %span#js-no-emoji-placeholder.no-emoji-placeholder{ class: ('hidden' if custom_emoji) } = sprite_icon('slight-smile', css_class: 'award-control-icon-neutral') = sprite_icon('smiley', css_class: 'award-control-icon-positive') = sprite_icon('smile', css_class: 'award-control-icon-super-positive') - - reset_message_button = button_tag type: :button, - id: 'js-clear-user-status-button', - class: 'clear-user-status btn gl-button btn-default has-tooltip', - title: s_("Profiles|Clear status") do - = sprite_icon("close") + - reset_message_button = render Pajamas::ButtonComponent.new(icon: 'close', + button_options: { id: 'js-clear-user-status-button', + class: 'has-tooltip', + title: s_("Profiles|Clear status") } ) = status_form.hidden_field :emoji, id: 'js-status-emoji-field' .form-group.gl-form-group diff --git a/app/views/projects/issues/_linked_resources.html.haml b/app/views/projects/issues/_linked_resources.html.haml new file mode 100644 index 00000000000..6b3ea46c066 --- /dev/null +++ b/app/views/projects/issues/_linked_resources.html.haml @@ -0,0 +1,4 @@ +- if Feature.enabled?(:incident_resource_links_widget, @project) && can?(current_user, :read_issuable_resource_link, @issue) + .js-linked-resources-root{ data: { issuable_id: @issue.id, + can_add_resource_links: "#{can?(current_user, :admin_issuable_resource_link, @issue)}", + help_path: help_page_path('user/project/issues/related_issues')} } diff --git a/app/views/shared/issue_type/_details_content.html.haml b/app/views/shared/issue_type/_details_content.html.haml index 7c5b3fd4b3c..39e7d196965 100644 --- a/app/views/shared/issue_type/_details_content.html.haml +++ b/app/views/shared/issue_type/_details_content.html.haml @@ -18,6 +18,7 @@ = render 'projects/issues/design_management' = render_if_exists 'projects/issues/work_item_links' + = render_if_exists 'projects/issues/linked_resources' = render_if_exists 'projects/issues/related_issues' #js-related-merge-requests{ data: { endpoint: expose_path(api_v4_projects_issues_related_merge_requests_path(id: @project.id, issue_iid: issuable.iid)), project_namespace: @project.namespace.path, project_path: @project.path } } diff --git a/config/metrics/counts_all/20210216181918_releases.yml b/config/metrics/counts_all/20210216181918_releases.yml index 77d12712978..aff67e4e6e5 100644 --- a/config/metrics/counts_all/20210216181918_releases.yml +++ b/config/metrics/counts_all/20210216181918_releases.yml @@ -3,7 +3,7 @@ data_category: optional key_path: counts.releases description: Count of releases product_section: ops -product_stage: releases +product_stage: release product_group: release product_category: release_orchestration value_type: number diff --git a/doc/development/feature_flags/index.md b/doc/development/feature_flags/index.md index 31ab6ca3c32..140d5f826cf 100644 --- a/doc/development/feature_flags/index.md +++ b/doc/development/feature_flags/index.md @@ -236,6 +236,11 @@ command. For example: /chatops run feature list --staging ``` +## Toggle a feature flag + +See [rolling out changes](controls.md#rolling-out-changes) for more information about toggling +feature flags. + ## Delete a feature flag See [cleaning up feature flags](controls.md#cleaning-up) for more information about diff --git a/doc/development/geo.md b/doc/development/geo.md index d385ea0492f..9e9bd85ecd8 100644 --- a/doc/development/geo.md +++ b/doc/development/geo.md @@ -469,6 +469,11 @@ clone, and compares the hash with the value the **primary** site calculated. If there is a mismatch, Geo will mark this as a mismatch and the administrator can see this in the [Geo Admin Area](../user/admin_area/geo_sites.md). +## Geo proxying + +Geo secondaries can proxy web requests to the primary. +Read more on the [Geo proxying (development) page](geo/proxying.md). + ## Glossary ### Primary site diff --git a/doc/development/geo/proxying.md b/doc/development/geo/proxying.md new file mode 100644 index 00000000000..41c7f426c6f --- /dev/null +++ b/doc/development/geo/proxying.md @@ -0,0 +1,356 @@ +--- +stage: Systems +group: Geo +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Geo proxying + +With Geo proxying, secondaries now proxy web requests through Workhorse to the primary, so users navigating to the +secondary see a read-write UI, and are able to do all operations that they can do on the primary. + +## Request life cycle + +### Top-level view + +The proxying interaction can be explained at a high level through the following diagram: + +```mermaid +sequenceDiagram +actor client +participant secondary +participant primary + +client->>secondary: GET /explore +secondary-->>primary: GET /explore (proxied) +primary-->>secondary: HTTP/1.1 200 OK [..] +secondary->>client: HTTP/1.1 200 OK [..] +``` + +### Proxy detection mechanism + +To know whether or not it should proxy requests to the primary, and the URL of the primary (as it is stored in +the database), Workhorse polls the internal API when Geo is enabled. When proxying should be enabled, the internal +API responds with the primary URL and JWT-signed data that is passed on to the primary for every request. + +```mermaid +sequenceDiagram + participant W as Workhorse (secondary) + participant API as Internal Rails API + W->API: GET /api/v4/geo/proxy (internal) + loop Poll every 10 seconds + API-->W: {geo_proxy_primary_url, geo_proxy_extra_data}, update config + end +``` + +### In-depth request flow and local data acceleration compared with proxying + +Detailing implementation, Workhorse on the secondary (requested) site decides whether to proxy the data or not. If it +can "accelerate" the data type (that is, can serve locally to save a roundtrip request), it returns the data +immediately. Otherwise, traffic is sent to the primary's internal URL, served by Workhorse on the primary exactly +as a direct request would. The response is then be proxied back to the user through the secondary Workhorse in the +same connection. + +```mermaid +flowchart LR + A[Client]--->W1["Workhorse (secondary)"] + W1 --> W1C[Serve data locally?] + W1C -- "Yes" ----> W1 + W1C -- "No (proxy)" ----> W2["Workhorse (primary)"] + W2 --> W1 ----> A +``` + +## Sign-in + +### Requests proxied to the primary requiring authorization + +```mermaid +sequenceDiagram +autoNumber +participant Client +participant Secondary +participant Primary + +Client->>Secondary: `/group/project` request +Secondary->>Primary: proxy /group/project +opt primary not signed in +Primary-->>Secondary: 302 redirect +Secondary-->>Client: proxy 302 redirect +Client->>Secondary: /users/sign_in +Secondary->>Primary: proxy /users/sign_in +Note right of Primary: authentication happens, POST to same URL etc +Primary-->>Secondary: 302 redirect +Secondary-->>Client: proxy 302 redirect +Client->>Secondary: /group/project +Secondary->>Primary: proxy /group/project +end +Primary-->>Secondary: /group/project logged in response (session on primary created) +Secondary-->>Client: proxy full response +``` + +### Requests requiring a user session on the secondary + +At the moment, this flow only applies to Project Replication Details and Design Replication Details in the Geo Admin +Area. For more context, see +[View replication data on the primary site](../../administration/geo/index.md#view-replication-data-on-the-primary-site). + +```mermaid +sequenceDiagram +autoNumber +participant Client +participant Secondary +participant Primary + +Client->>Secondary: `admin/geo/replication/projects` request +opt secondary not signed in +Secondary-->>Client: 302 redirect +Client->>Secondary: /users/auth/geo/sign_in +Secondary-->>Client: 302 redirect +Client->>Secondary: /oauth/geo/auth/geo/sign_in +Secondary-->>Client: 302 redirect +Client->>Secondary: /oauth/authorize +Secondary->>Primary: proxy /oauth/authorize +opt primary not signed in +Primary-->>Secondary: 302 redirect +Secondary-->>Client: proxy 302 redirect +Client->>Secondary: /users/sign_in +Secondary->>Primary: proxy /users/sign_in +Note right of Primary: authentication happens, POST to same URL etc +end +Primary-->>Secondary: 302 redirect +Secondary-->>Client: proxy 302 redirect +Client->>Secondary: /oauth/geo/callback +Secondary-->>Client: 302 redirect +Client->>Secondary: admin/geo/replication/projects +end +Secondary-->>Client: admin/geo/replication/projects logged in response (session on both primary and secondary) +``` + +## Git pull + +For historical reasons, the `push_from_secondary` path is used to forward a Git pull. There is [an issue proposing to +rename this route](https://gitlab.com/gitlab-org/gitlab/-/issues/292690) to avoid confusion. + +### Git pull over HTTP(s) + +#### Accelerated repositories + +When a repository exists on the secondary and we detect is up to date with the primary, we serve it directly instead of +proxying. + +```mermaid +sequenceDiagram +participant C as Git client +participant Wsec as "Workhorse (secondary)" +participant Rsec as "Rails (secondary)" +participant Gsec as "Gitaly (secondary)" +C->>Wsec: GET /foo/bar.git/info/refs/?service=git-upload-pack +Wsec->>Rsec: +note over Rsec: decide that the repo is synced and up to date +Rsec-->>Wsec: 401 Unauthorized +Wsec-->>C: +C->>Wsec: GET /foo/bar.git/info/refs/?service=git-upload-pack +Wsec->>Rsec: +Rsec-->>Wsec: Render Workhorse OK +Wsec-->>C: 200 OK +C->>Wsec: POST /foo/bar.git/git-upload-pack +Wsec->>Rsec: GitHttpController#git_receive_pack +Rsec-->>Wsec: Render Workhorse OK +Wsec->>Gsec: Workhorse gets the connection details from Rails, connects to Gitaly: SmartHTTP Service, UploadPack RPC (check the proto for details) +Gsec-->>Wsec: Return a stream of Proto messages +Wsec-->>C: Pipe messages to the Git client +``` + +#### Proxied repositories + +If a requested repository isn't synced, or we detect is not up to date, the request will be proxied to the primary, in +order to get the latest version of the changes. + +```mermaid +sequenceDiagram +participant C as Git client +participant Wsec as "Workhorse (secondary)" +participant Rsec as "Rails (secondary)" +participant W as "Workhorse (primary)" +participant R as "Rails (primary)" +participant G as "Gitaly (primary)" +C->>Wsec: GET /foo/bar.git/info/refs/?service=git-upload-pack +Wsec->>Rsec: +note over Rsec: decide that the repo is out of date +Rsec-->>Wsec: 302 Redirect to /-/push_from_secondary/2/foo/bar.git/info/refs?service=git-upload-pack +Wsec-->>C: +C->>Wsec: GET /-/push_from_secondary/2/foo/bar.git/info/refs/?service=git-upload-pack +Wsec->>W: +W->>R: +R-->>W: 401 Unauthorized +W-->>Wsec: +Wsec-->>C: +C->>Wsec: GET /-/push_from_secondary/2/foo/bar.git/info/refs/?service=git-upload-pack +note over W: proxied +Wsec->>W: +W->>R: +R-->>W: Render Workhorse OK +W-->>Wsec: +Wsec-->>C: +C->>Wsec: POST /-/push_from_secondary/2/foo/bar.git/git-upload-pack +Wsec->>W: +W->>R: GitHttpController#git_receive_pack +R-->>W: Render Workhorse OK +W->>G: Workhorse gets the connection details from Rails, connects to Gitaly: SmartHTTP Service, UploadPack RPC (check the proto for details) +G-->>W: Return a stream of Proto messages +W-->>Wsec: Pipe messages to the Git client +Wsec-->>C: Return piped messages from Git +``` + +### Git pull over SSH + +As SSH operations go through GitLab Shell instead of Workhorse, they are not proxied through the mechanism used for +Workhorse requests. With SSH operations, they are proxied as Git HTTP requests to the primary site by the secondary +Rails internal API. + +#### Accelerated repositories + +When a repository exists on the secondary and we detect is up to date with the primary, we serve it directly instead of +proxying. + +```mermaid +sequenceDiagram +participant C as Git client +participant S as GitLab Shell (secondary) +participant I as Internal API (secondary Rails) +participant G as Gitaly (secondary) +C->>S: git pull +S->>I: SSH key validation (api/v4/internal/authorized_keys?key=..) +I-->>S: HTTP/1.1 200 OK +S->>G: InfoRefs:UploadPack RPC +G-->>S: stream Git response back +S-->>C: stream Git response back +C-->>S: stream Git data to push +S->>G: UploadPack RPC +G-->>S: stream Git response back +S-->>C: stream Git response back +``` + +#### Proxied repositories + +If a requested repository isn't synced, or we detect is not up to date, the request will be proxied to the primary, in +order to get the latest version of the changes. + +```mermaid +sequenceDiagram +participant C as Git client +participant S as GitLab Shell (secondary) +participant I as Internal API (secondary Rails) +participant P as Primary API +C->>S: git pull +S->>I: SSH key validation (api/v4/internal/authorized_keys?key=..) +I-->>S: HTTP/1.1 300 (custom action status) with {endpoint, msg, primary_repo} +S->>I: POST /api/v4/geo/proxy_git_ssh/info_refs_upload_pack +I->>P: POST $PRIMARY/foo/bar.git/info/refs/?service=git-upload-pack +P-->>I: HTTP/1.1 200 OK +I-->>S: +S-->>C: return Git response from primary +C-->>S: stream Git data to push +S->>I: POST /api/v4/geo/proxy_git_ssh/upload_pack +I->>P: POST $PRIMARY/foo/bar.git/git-upload-pack +P-->>I: HTTP/1.1 200 OK +I-->>S: +S-->>C: return Git response from primary +``` + +## Git push + +### Unified URLs + +With unified URLs, a push will redirect to a local path formatted as `/-/push_from_secondary/$SECONDARY_ID/*`. Further +requests through this path will be proxied to the primary, which will handle the push. + +#### Git push over SSH + +As SSH operations go through GitLab Shell instead of Workhorse, they are not proxied through the mechanism used for +Workhorse requests. With SSH operations, they are proxied as Git HTTP requests to the primary site by the secondary +Rails internal API. + +```mermaid +sequenceDiagram +participant C as Git client +participant S as GitLab Shell (secondary) +participant I as Internal API (secondary Rails) +participant P as Primary API +C->>S: git push +S->>I: SSH key validation (api/v4/internal/authorized_keys?key=..) +I-->>S: HTTP/1.1 300 (custom action status) with {endpoint, msg, primary_repo} +S->>I: POST /api/v4/geo/proxy_git_ssh/info_refs_receive_pack +I->>P: POST $PRIMARY/foo/bar.git/info/refs/?service=git-receive-pack +P-->>I: HTTP/1.1 200 OK +I-->>S: +S-->>C: return Git response from primary +C-->>S: stream Git data to push +S->>I: POST /api/v4/geo/proxy_git_ssh/receive_pack +I->>P: POST $PRIMARY/foo/bar.git/git-receive-pack +P-->>I: HTTP/1.1 200 OK +I-->>S: +S-->>C: return Git response from primary +``` + +#### Git push over HTTP(s) + +```mermaid +sequenceDiagram +participant C as Git client +participant Wsec as Workhorse (secondary) +participant W as Workhorse (primary) +participant R as Rails (primary) +participant G as Gitaly (primary) +C->>Wsec: GET /foo/bar.git/info/refs/?service=git-receive-pack +Wsec->>C: 302 Redirect to /-/push_from_secondary/2/foo/bar.git/info/refs?service=git-receive-pack +C->>Wsec: GET /-/push_from_secondary/2/foo/bar.git/info/refs/?service=git-receive-pack +Wsec->>W: +W->>R: +R-->>W: 401 Unauthorized +W-->>Wsec: +Wsec-->>C: +C->>Wsec: GET /-/push_from_secondary/2/foo/bar.git/info/refs/?service=git-receive-pack +Wsec->>W: +W->>R: +R-->>W: Render Workhorse OK +W-->>Wsec: +Wsec-->>C: +C->>Wsec: POST /-/push_from_secondary/2/foo/bar.git/git-receive-pack +Wsec->>W: +W->>R: GitHttpController:git_receive_pack +R-->>W: Render Workhorse OK +W->>G: Get connection details from Rails and connects to SmartHTTP Service, ReceivePack RPC +G-->>W: Return a stream of Proto messages +W-->>Wsec: Pipe messages to the Git client +Wsec-->>C: Return piped messages from Git +``` + +### Git push over HTTP with Separate URLs + +With separate URLs, the secondary will redirect to a URL formatted like `$PRIMARY/-/push_from_secondary/$SECONDARY_ID/*`. + +```mermaid +sequenceDiagram +participant Wsec as Workhorse (secondary) +participant C as Git client +participant W as Workhorse (primary) +participant R as Rails (primary) +participant G as Gitaly (primary) +C->>Wsec: GET $SECONDARY/foo/bar.git/info/refs/?service=git-receive-pack +Wsec->>C: 302 Redirect to $PRIMARY/-/push_from_secondary/2/foo/bar.git/info/refs?service=git-receive-pack +C->>W: GET $PRIMARY/-/push_from_secondary/2/foo/bar.git/info/refs/?service=git-receive-pack +W->>R: +R-->>W: 401 Unauthorized +W-->>C: +C->>W: GET /-/push_from_secondary/2/foo/bar.git/info/refs/?service=git-receive-pack +W->>R: +R-->>W: Render Workhorse OK +W-->>C: +C->>W: POST /-/push_from_secondary/2/foo/bar.git/git-receive-pack +W->>R: GitHttpController:git_receive_pack +R-->>W: Render Workhorse OK +W->>G: Get connection details from Rails and connects to SmartHTTP Service, ReceivePack RPC +G-->>W: Return a stream of Proto messages +W-->>C: Pipe messages to the Git client +``` diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index f43b4f5358c..92dc795afe5 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -147,8 +147,8 @@ The default scanner images are build off a base Alpine image for size and mainta > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/6479) in GitLab 14.10. -GitLab offers [Red Hat UBI](https://www.redhat.com/en/blog/introducing-red-hat-universal-base-image) -versions of the images that are FIPS-enabled. To use the FIPS-enabled images, you can either: +GitLab offers an image version, based on the [Red Hat UBI](https://www.redhat.com/en/blog/introducing-red-hat-universal-base-image) base image, +that uses a FIPS 140-validated cryptographic module. To use the FIPS-enabled image, you can either: - Set the `SAST_IMAGE_SUFFIX` to `-fips`. - Add the `-fips` extension to the default image name. @@ -163,6 +163,10 @@ include: - template: Security/SAST.gitlab-ci.yml ``` +A FIPS-compliant image is only available for the Semgrep-based analyzer. + +To use SAST in a FIPS-compliant manner, you must [exclude other analyzers from running](analyzers.md#customize-analyzers). + ### Making SAST analyzers available to all GitLab tiers All open source (OSS) analyzers have been moved to the GitLab Free tier as of GitLab 13.3. diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 505d0b8d728..882bd57eb1d 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -18,6 +18,8 @@ module Gitlab UnknownRef = Class.new(BaseError) CommandTimedOut = Class.new(CommandError) InvalidPageToken = Class.new(BaseError) + InvalidRefFormatError = Class.new(BaseError) + ReferencesLockedError = Class.new(BaseError) class << self include Gitlab::EncodingHelper diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index dd480b90ee5..996534f4194 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -485,6 +485,22 @@ module Gitlab stack_counter.select { |_, v| v == max }.keys end + + def self.decode_detailed_error(err) + # details could have more than one in theory, but we only have one to worry about for now. + detailed_error = err.to_rpc_status&.details&.first + + return unless detailed_error.present? + + prefix = %r{type\.googleapis\.com\/gitaly\.(?.+)} + error_type = prefix.match(detailed_error.type_url)[:error_type] + + Gitaly.const_get(error_type, false).decode(detailed_error.value) + rescue NameError, NoMethodError + # Error Class might not be known to ruby yet + nil + end + private_class_method :max_stacks end end diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index ac40f041ae7..35d3ddf5d7f 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -102,7 +102,7 @@ module Gitlab raise Gitlab::Git::PreReceiveError, pre_receive_error end rescue GRPC::BadStatus => e - detailed_error = decode_detailed_error(e) + detailed_error = GitalyClient.decode_detailed_error(e) case detailed_error&.error when :custom_hook @@ -166,7 +166,7 @@ module Gitlab Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) rescue GRPC::BadStatus => e - detailed_error = decode_detailed_error(e) + detailed_error = GitalyClient.decode_detailed_error(e) case detailed_error&.error when :access_check @@ -277,7 +277,7 @@ module Gitlab rebase_sha rescue GRPC::BadStatus => e - detailed_error = decode_detailed_error(e) + detailed_error = GitalyClient.decode_detailed_error(e) case detailed_error&.error when :access_check @@ -314,7 +314,7 @@ module Gitlab response.squash_sha rescue GRPC::BadStatus => e - detailed_error = decode_detailed_error(e) + detailed_error = GitalyClient.decode_detailed_error(e) case detailed_error&.error when :resolve_revision, :rebase_conflict @@ -474,7 +474,7 @@ module Gitlab handle_cherry_pick_or_revert_response(response) rescue GRPC::BadStatus => e - detailed_error = decode_detailed_error(e) + detailed_error = GitalyClient.decode_detailed_error(e) case detailed_error&.error when :access_check @@ -538,21 +538,6 @@ module Gitlab raise ArgumentError, "Unknown action '#{action[:action]}'" end - def decode_detailed_error(err) - # details could have more than one in theory, but we only have one to worry about for now. - detailed_error = err.to_rpc_status&.details&.first - - return unless detailed_error.present? - - prefix = %r{type\.googleapis\.com\/gitaly\.(?.+)} - error_type = prefix.match(detailed_error.type_url)[:error_type] - - Gitaly.const_get(error_type, false).decode(detailed_error.value) - rescue NameError, NoMethodError - # Error Class might not be known to ruby yet - nil - end - def custom_hook_error_message(custom_hook_error) # Custom hooks may return messages via either stdout or stderr which have a specific prefix. If # that prefix is present we'll want to print the hook's output, otherwise we'll want to print the diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index c064811b1e7..31e1406356f 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -132,6 +132,17 @@ module Gitlab response = GitalyClient.call(@repository.storage, :ref_service, :delete_refs, request, timeout: GitalyClient.medium_timeout) raise Gitlab::Git::Repository::GitError, response.git_error if response.git_error.present? + rescue GRPC::BadStatus => e + detailed_error = GitalyClient.decode_detailed_error(e) + + case detailed_error&.error + when :invalid_format + raise Gitlab::Git::InvalidRefFormatError, "references have an invalid format: #{detailed_error.invalid_format.refs.join(",")}" + when :references_locked + raise Gitlab::Git::ReferencesLockedError + else + raise e + end end # Limit: 0 implies no limit, thus all tag names will be returned diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 720366ecdf0..462fb2a420f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2147,6 +2147,9 @@ msgstr "" msgid "Add a related issue" msgstr "" +msgid "Add a resource link" +msgstr "" + msgid "Add a suffix to Service Desk email address. %{linkStart}Learn more.%{linkEnd}" msgstr "" @@ -23453,6 +23456,9 @@ msgstr "" msgid "Linked issues" msgstr "" +msgid "Linked resources" +msgstr "" + msgid "LinkedIn" msgstr "" @@ -31753,6 +31759,9 @@ msgstr "" msgid "Read more about GitLab at %{link_to_promo}." msgstr "" +msgid "Read more about linked resources" +msgstr "" + msgid "Read more about project permissions %{help_link_open}here%{help_link_close}" msgstr "" @@ -43828,10 +43837,7 @@ msgstr "" msgid "Work in progress Limit" msgstr "" -msgid "WorkItem|Add" -msgstr "" - -msgid "WorkItem|Add a child" +msgid "WorkItem|Add a task" msgstr "" msgid "WorkItem|Add assignee" @@ -43840,6 +43846,9 @@ msgstr "" msgid "WorkItem|Add assignees" msgstr "" +msgid "WorkItem|Add task" +msgstr "" + msgid "WorkItem|Are you sure you want to cancel editing?" msgstr "" @@ -43896,6 +43905,9 @@ msgstr "" msgid "WorkItem|Something went wrong when fetching work item types. Please try again" msgstr "" +msgid "WorkItem|Something went wrong when trying to add a child. Please try again." +msgstr "" + msgid "WorkItem|Something went wrong while updating the work item. Please try again." msgstr "" diff --git a/scripts/pipeline_test_report_builder.rb b/scripts/pipeline_test_report_builder.rb index 5299dba3f97..649b68427ea 100755 --- a/scripts/pipeline_test_report_builder.rb +++ b/scripts/pipeline_test_report_builder.rb @@ -73,7 +73,7 @@ class PipelineTestReportBuilder def test_report_for_build(pipeline, build_id) fetch("#{pipeline['web_url']}/tests/suite.json?build_ids[]=#{build_id}") rescue Net::HTTPServerException => e - raise e unless e.response.code == 404 + raise e unless e.response.code.to_i == 404 puts "Artifacts not found. They may have expired. Skipping this build." end diff --git a/spec/frontend/issuable/linked_resources/components/__snapshots__/resource_links_block_spec.js.snap b/spec/frontend/issuable/linked_resources/components/__snapshots__/resource_links_block_spec.js.snap new file mode 100644 index 00000000000..24586744ad6 --- /dev/null +++ b/spec/frontend/issuable/linked_resources/components/__snapshots__/resource_links_block_spec.js.snap @@ -0,0 +1,70 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ResourceLinksBlock with defaults renders correct component 1`] = ` + +`; diff --git a/spec/frontend/issuable/linked_resources/components/resource_links_block_spec.js b/spec/frontend/issuable/linked_resources/components/resource_links_block_spec.js new file mode 100644 index 00000000000..c17ca1a3287 --- /dev/null +++ b/spec/frontend/issuable/linked_resources/components/resource_links_block_spec.js @@ -0,0 +1,35 @@ +import { GlButton } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import ResourceLinksBlock from '~/linked_resources/components/resource_links_block.vue'; + +describe('ResourceLinksBlock', () => { + let wrapper; + + const findResourceLinkAddButton = () => wrapper.find(GlButton); + const helpPath = '/help/user/project/issues/linked_resources'; + + describe('with defaults', () => { + it('renders correct component', () => { + wrapper = shallowMount(ResourceLinksBlock, { + propsData: { + helpPath, + canAddResourceLinks: true, + }, + }); + + expect(wrapper.element).toMatchSnapshot(); + }); + }); + + describe('with canAddResourceLinks=false', () => { + it('does not show the add button', () => { + wrapper = shallowMount(ResourceLinksBlock, { + propsData: { + canAddResourceLinks: false, + }, + }); + + expect(findResourceLinkAddButton().exists()).toBe(false); + }); + }); +}); diff --git a/spec/frontend/set_status_modal/set_status_modal_wrapper_spec.js b/spec/frontend/set_status_modal/set_status_modal_wrapper_spec.js index 0b672cbc93e..e3b5478290a 100644 --- a/spec/frontend/set_status_modal/set_status_modal_wrapper_spec.js +++ b/spec/frontend/set_status_modal/set_status_modal_wrapper_spec.js @@ -1,10 +1,11 @@ import { GlModal, GlFormCheckbox } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import { nextTick } from 'vue'; import { initEmojiMock, clearEmojiMock } from 'helpers/emoji'; import * as UserApi from '~/api/user_api'; import EmojiPicker from '~/emoji/components/picker.vue'; import createFlash from '~/flash'; +import stubChildren from 'helpers/stub_children'; import SetStatusModalWrapper, { AVAILABILITY_STATUS, } from '~/set_status_modal/set_status_modal_wrapper.vue'; @@ -26,12 +27,23 @@ describe('SetStatusModalWrapper', () => { defaultEmoji, }; + const EmojiPickerStub = { + props: EmojiPicker.props, + template: '
', + }; + const createComponent = (props = {}) => { - return shallowMount(SetStatusModalWrapper, { + return mount(SetStatusModalWrapper, { propsData: { ...defaultProps, ...props, }, + stubs: { + ...stubChildren(SetStatusModalWrapper), + GlFormInput: false, + GlFormInputGroup: false, + EmojiPicker: EmojiPickerStub, + }, mocks: { $toast, }, @@ -43,7 +55,7 @@ describe('SetStatusModalWrapper', () => { const findClearStatusButton = () => wrapper.find('.js-clear-user-status-button'); const findAvailabilityCheckbox = () => wrapper.find(GlFormCheckbox); const findClearStatusAtMessage = () => wrapper.find('[data-testid="clear-status-at-message"]'); - const getEmojiPicker = () => wrapper.findComponent(EmojiPicker); + const getEmojiPicker = () => wrapper.findComponent(EmojiPickerStub); const initModal = async ({ mockOnUpdateSuccess = true, mockOnUpdateFailure = true } = {}) => { const modal = findModal(); @@ -88,7 +100,7 @@ describe('SetStatusModalWrapper', () => { }); it('has a clear status button', () => { - expect(findClearStatusButton().isVisible()).toBe(true); + expect(findClearStatusButton().exists()).toBe(true); }); it('displays the clear status at dropdown', () => { @@ -125,7 +137,7 @@ describe('SetStatusModalWrapper', () => { }); it('hides the clear status button', () => { - expect(findClearStatusButton().isVisible()).toBe(false); + expect(findClearStatusButton().exists()).toBe(false); }); }); diff --git a/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js b/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js index f784f10ba28..93bf7286aa7 100644 --- a/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js +++ b/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js @@ -6,19 +6,23 @@ import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import WorkItemLinksForm from '~/work_items/components/work_item_links/work_item_links_form.vue'; import projectWorkItemsQuery from '~/work_items/graphql/project_work_items.query.graphql'; -import { availableWorkItemsResponse } from '../../mock_data'; +import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; +import { availableWorkItemsResponse, updateWorkItemMutationResponse } from '../../mock_data'; Vue.use(VueApollo); describe('WorkItemLinksForm', () => { let wrapper; - const createComponent = async ({ response = availableWorkItemsResponse } = {}) => { + const updateMutationResolver = jest.fn().mockResolvedValue(updateWorkItemMutationResponse); + + const createComponent = async ({ listResponse = availableWorkItemsResponse } = {}) => { wrapper = shallowMountExtended(WorkItemLinksForm, { apolloProvider: createMockApollo([ - [projectWorkItemsQuery, jest.fn().mockResolvedValue(response)], + [projectWorkItemsQuery, jest.fn().mockResolvedValue(listResponse)], + [updateWorkItemMutation, updateMutationResolver], ]), - propsData: { issuableId: 1 }, + propsData: { issuableGid: 'gid://gitlab/WorkItem/1' }, provide: { projectPath: 'project/path', }, @@ -29,6 +33,7 @@ describe('WorkItemLinksForm', () => { const findForm = () => wrapper.findComponent(GlForm); const findCombobox = () => wrapper.findComponent(GlFormCombobox); + const findAddChildButton = () => wrapper.findByTestId('add-child-button'); beforeEach(async () => { await createComponent(); @@ -43,7 +48,18 @@ describe('WorkItemLinksForm', () => { }); it('passes available work items as prop when typing in combobox', async () => { + findCombobox().vm.$emit('input', 'Task'); + await waitForPromises(); + expect(findCombobox().exists()).toBe(true); expect(findCombobox().props('tokenList').length).toBe(2); }); + + it('selects and add child', async () => { + findCombobox().vm.$emit('input', availableWorkItemsResponse.data.workspace.workItems.edges[0]); + + findAddChildButton().vm.$emit('click'); + await waitForPromises(); + expect(updateMutationResolver).toHaveBeenCalled(); + }); }); diff --git a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js index 774e9198992..2ec9b1ec0ac 100644 --- a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js +++ b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js @@ -51,6 +51,20 @@ describe('WorkItemLinks', () => { expect(findLinksBody().exists()).toBe(false); }); + describe('add link form', () => { + it('displays form on click add button and hides form on cancel', async () => { + findToggleAddFormButton().vm.$emit('click'); + await nextTick(); + + expect(findAddLinksForm().exists()).toBe(true); + + findAddLinksForm().vm.$emit('cancel'); + await nextTick(); + + expect(findAddLinksForm().exists()).toBe(false); + }); + }); + describe('when no child links', () => { beforeEach(async () => { await createComponent({ response: workItemHierarchyEmptyResponse }); @@ -59,22 +73,6 @@ describe('WorkItemLinks', () => { it('displays empty state if there are no children', () => { expect(findEmptyState().exists()).toBe(true); }); - - describe('add link form', () => { - it('displays form on click add button and hides form on cancel', async () => { - expect(findEmptyState().exists()).toBe(true); - - findToggleAddFormButton().vm.$emit('click'); - await nextTick(); - - expect(findAddLinksForm().exists()).toBe(true); - - findAddLinksForm().vm.$emit('cancel'); - await nextTick(); - - expect(findAddLinksForm().exists()).toBe(false); - }); - }); }); it('renders all hierarchy widget children', () => { diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js index 1eec58f5ebc..3df30971aef 100644 --- a/spec/frontend/work_items/mock_data.js +++ b/spec/frontend/work_items/mock_data.js @@ -58,6 +58,15 @@ export const workItemQueryResponse = { iid: '5', title: 'Parent title', }, + children: { + edges: [ + { + node: { + id: 'gid://gitlab/WorkItem/444', + }, + }, + ], + }, }, ], }, @@ -83,7 +92,17 @@ export const updateWorkItemMutationResponse = { deleteWorkItem: false, updateWorkItem: false, }, - widgets: [], + widgets: [ + { + children: { + edges: [ + { + node: 'gid://gitlab/WorkItem/444', + }, + ], + }, + }, + ], }, }, }, @@ -136,6 +155,15 @@ export const workItemResponseFactory = ({ iid: '5', title: 'Parent title', }, + children: { + edges: [ + { + node: { + id: 'gid://gitlab/WorkItem/444', + }, + }, + ], + }, }, ], }, @@ -378,12 +406,14 @@ export const availableWorkItemsResponse = { node: { id: 'gid://gitlab/WorkItem/458', title: 'Task 1', + state: 'OPEN', }, }, { node: { id: 'gid://gitlab/WorkItem/459', title: 'Task 2', + state: 'OPEN', }, }, ], diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 5172a9389f9..e04895d975f 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -2,9 +2,6 @@ require 'spec_helper' -require 'google/rpc/status_pb' -require 'google/protobuf/well_known_types' - RSpec.describe Gitlab::GitalyClient::OperationService do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository) } @@ -816,14 +813,4 @@ RSpec.describe Gitlab::GitalyClient::OperationService do end end end - - def new_detailed_error(error_code, error_message, details) - status_error = Google::Rpc::Status.new( - code: error_code, - message: error_message, - details: [Google::Protobuf::Any.pack(details)] - ) - - GRPC::BadStatus.new(error_code, error_message, { "grpc-status-details-bin" => Google::Rpc::Status.encode(status_error) }) - end end diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb index 2e37c98a591..c794e3ca9ae 100644 --- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb @@ -258,13 +258,54 @@ RSpec.describe Gitlab::GitalyClient::RefService do describe '#delete_refs' do let(:prefixes) { %w(refs/heads refs/keep-around) } + subject(:delete_refs) { client.delete_refs(except_with_prefixes: prefixes) } + it 'sends a delete_refs message' do expect_any_instance_of(Gitaly::RefService::Stub) .to receive(:delete_refs) .with(gitaly_request_with_params(except_with_prefix: prefixes), kind_of(Hash)) .and_return(double('delete_refs_response', git_error: "")) - client.delete_refs(except_with_prefixes: prefixes) + delete_refs + end + + context 'with a references locked error' do + let(:references_locked_error) do + new_detailed_error( + GRPC::Core::StatusCodes::FAILED_PRECONDITION, + "error message", + Gitaly::DeleteRefsError.new(references_locked: Gitaly::ReferencesLockedError.new)) + end + + it 'raises ReferencesLockedError' do + expect_any_instance_of(Gitaly::RefService::Stub).to receive(:delete_refs) + .with(gitaly_request_with_params(except_with_prefix: prefixes), kind_of(Hash)) + .and_raise(references_locked_error) + + expect { delete_refs }.to raise_error(Gitlab::Git::ReferencesLockedError) + end + end + + context 'with a invalid format error' do + let(:invalid_refs) {['\invali.\d/1', '\.invali/d/2']} + let(:invalid_reference_format_error) do + new_detailed_error( + GRPC::Core::StatusCodes::INVALID_ARGUMENT, + "error message", + Gitaly::DeleteRefsError.new(invalid_format: Gitaly::InvalidRefFormatError.new(refs: invalid_refs))) + end + + it 'raises InvalidRefFormatError' do + expect_any_instance_of(Gitaly::RefService::Stub) + .to receive(:delete_refs) + .with(gitaly_request_with_params(except_with_prefix: prefixes), kind_of(Hash)) + .and_raise(invalid_reference_format_error) + + expect { delete_refs }.to raise_error do |error| + expect(error).to be_a(Gitlab::Git::InvalidRefFormatError) + expect(error.message).to eq("references have an invalid format: #{invalid_refs.join(",")}") + end + end end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index f0735cfc2f3..a3840ca843f 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -545,4 +545,44 @@ RSpec.describe Gitlab::GitalyClient do end end end + + describe '.decode_detailed_error' do + let(:detailed_error) do + new_detailed_error(GRPC::Core::StatusCodes::INVALID_ARGUMENT, + "error message", + Gitaly::InvalidRefFormatError.new) + end + + let(:error_without_details) do + error_code = GRPC::Core::StatusCodes::INVALID_ARGUMENT + error_message = "error message" + + status_error = Google::Rpc::Status.new( + code: error_code, + message: error_message, + details: nil + ) + + GRPC::BadStatus.new( + error_code, + error_message, + { "grpc-status-details-bin" => Google::Rpc::Status.encode(status_error) }) + end + + context 'decodes a structured error' do + using RSpec::Parameterized::TableSyntax + + where(:error, :result) do + detailed_error | Gitaly::InvalidRefFormatError.new + error_without_details | nil + StandardError.new | nil + end + + with_them do + it 'returns correct detailed error' do + expect(described_class.decode_detailed_error(error)).to eq(result) + end + end + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 607c45eef45..56027aa02c8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -204,6 +204,7 @@ RSpec.configure do |config| config.include SnowplowHelpers config.include RenderedHelpers config.include RSpec::Benchmark::Matchers, type: :benchmark + config.include DetailedErrorHelpers include StubFeatureFlags include StubSnowplow diff --git a/spec/support/helpers/detailed_error_helpers.rb b/spec/support/helpers/detailed_error_helpers.rb new file mode 100644 index 00000000000..2da53a6bffd --- /dev/null +++ b/spec/support/helpers/detailed_error_helpers.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'google/rpc/status_pb' +require 'google/protobuf/well_known_types' + +module DetailedErrorHelpers + def new_detailed_error(error_code, error_message, details) + status_error = Google::Rpc::Status.new( + code: error_code, + message: error_message, + details: [Google::Protobuf::Any.pack(details)] + ) + + GRPC::BadStatus.new( + error_code, + error_message, + { "grpc-status-details-bin" => Google::Rpc::Status.encode(status_error) }) + end +end diff --git a/workhorse/internal/log/logging.go b/workhorse/internal/log/logging.go index ae7164db920..80c09c1bf02 100644 --- a/workhorse/internal/log/logging.go +++ b/workhorse/internal/log/logging.go @@ -67,14 +67,6 @@ func (b *Builder) WithError(err error) *Builder { return b } -func Debug(args ...interface{}) { - NewBuilder().Debug(args...) -} - -func (b *Builder) Debug(args ...interface{}) { - b.entry.Debug(args...) -} - func Info(args ...interface{}) { NewBuilder().Info(args...) } diff --git a/workhorse/internal/log/logging_test.go b/workhorse/internal/log/logging_test.go index 9daf282daf4..1cb6438532e 100644 --- a/workhorse/internal/log/logging_test.go +++ b/workhorse/internal/log/logging_test.go @@ -7,7 +7,6 @@ import ( "net/http/httptest" "testing" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) @@ -15,7 +14,6 @@ func captureLogs(b *Builder, testFn func()) string { buf := &bytes.Buffer{} logger := b.entry.Logger - logger.SetLevel(logrus.DebugLevel) oldOut := logger.Out logger.Out = buf defer func() { @@ -27,15 +25,6 @@ func captureLogs(b *Builder, testFn func()) string { return buf.String() } -func TestLogDebug(t *testing.T) { - b := NewBuilder() - logLine := captureLogs(b, func() { - b.Debug("an observation") - }) - - require.Regexp(t, `level=debug msg="an observation"`, logLine) -} - func TestLogInfo(t *testing.T) { b := NewBuilder() logLine := captureLogs(b, func() { diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index f836e32f06c..43b470b568f 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -23,7 +23,6 @@ import ( apipkg "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/config" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper" - "gitlab.com/gitlab-org/gitlab/workhorse/internal/log" proxypkg "gitlab.com/gitlab-org/gitlab/workhorse/internal/proxy" "gitlab.com/gitlab-org/gitlab/workhorse/internal/rejectmethods" "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload" @@ -183,20 +182,16 @@ func (u *upstream) findGeoProxyRoute(cleanedPath string, r *http.Request) *route defer u.mu.RUnlock() if u.geoProxyBackend.String() == "" { - log.WithRequest(r).Debug("Geo Proxy: Not a Geo proxy") return nil } // Some routes are safe to serve from this GitLab instance for _, ro := range u.geoLocalRoutes { if ro.isMatch(cleanedPath, r) { - log.WithRequest(r).Debug("Geo Proxy: Handle this request locally") return &ro } } - log.WithRequest(r).WithFields(log.Fields{"geoProxyBackend": u.geoProxyBackend}).Debug("Geo Proxy: Forward this request") - if cleanedPath == "/-/cable" { return &u.geoProxyCableRoute }