From 3a52eefc27143af8a2b3838a159c52484ca4bc8b Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 2 Dec 2021 15:10:48 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .eslintrc.yml | 2 +- .gitignore | 1 - .gitlab/ci/rules.gitlab-ci.yml | 1 - .gitlab/ci/static-analysis.gitlab-ci.yml | 24 +++- .rubocop_todo/style/open_struct_use.yml | 1 - .../create_http_integration.mutation.graphql | 3 +- .../destroy_http_integration.mutation.graphql | 3 +- .../reset_http_token.mutation.graphql | 3 +- .../update_http_integration.mutation.graphql | 3 +- .../board_list_create.mutation.graphql | 3 +- .../board_list_update.mutation.graphql | 3 +- .../boards/graphql/board_lists.query.graphql | 6 +- .../mutations/upload_design.mutation.graphql | 3 +- .../graphql/queries/get_design.query.graphql | 3 +- .../user_availability.fragment.graphql | 3 +- .../incident_fields.fragment.graphql | 3 +- .../pipeline_editor/pipeline_editor_app.vue | 33 +++-- .../runner/graphql/get_runner.query.graphql | 3 +- .../graphql/runner_update.mutation.graphql | 3 +- .../queries/epic/epic_children.query.graphql | 23 +++- app/models/packages/conan/metadatum.rb | 32 ++++- app/views/projects/show.html.haml | 1 - app/views/projects/tracings/show.html.haml | 10 +- config/apollo.config.js | 10 ++ .../new_route_ci_minutes_purchase.yml | 2 +- ...ges_conan_allow_empty_username_channel.yml | 8 ++ config/initializers/grape_validators.rb | 3 + .../geo/secondary_proxy/index.md | 43 +++++-- doc/administration/geo/setup/index.md | 1 + doc/development/fe_guide/graphql.md | 113 +++++++++++++++++- doc/user/packages/conan_repository/index.md | 24 ++++ lib/api/concerns/packages/conan_endpoints.rb | 16 ++- lib/api/helpers/packages/conan/api_helpers.rb | 15 +++ .../validators/packages_conan_user_channel.rb | 32 +++++ lib/gitlab/regex.rb | 13 +- locale/gitlab.pot | 18 +-- spec/dependencies/omniauth_saml_spec.rb | 2 +- .../packages_conan_user_channel_spec.rb | 42 +++++++ spec/lib/gitlab/regex_spec.rb | 44 ++++--- spec/models/packages/conan/metadatum_spec.rb | 55 +++++++++ .../api/conan_project_packages_spec.rb | 5 +- spec/spec_helper.rb | 4 + .../api/conan_packages_shared_examples.rb | 66 ++++++++++ workhorse/internal/upstream/upstream.go | 4 +- workhorse/internal/upstream/upstream_test.go | 4 + 45 files changed, 572 insertions(+), 122 deletions(-) create mode 100644 config/apollo.config.js create mode 100644 config/feature_flags/development/packages_conan_allow_empty_username_channel.yml create mode 100644 lib/api/validations/validators/packages_conan_user_channel.rb create mode 100644 spec/lib/api/validations/validators/packages_conan_user_channel_spec.rb diff --git a/.eslintrc.yml b/.eslintrc.yml index 7e87db0c72a..0f73a9c5105 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -138,4 +138,4 @@ overrides: #'@graphql-eslint/known-type-names': error '@graphql-eslint/no-anonymous-operations': error '@graphql-eslint/unique-operation-name': error - # '@graphql-eslint/require-id-when-available': error + '@graphql-eslint/require-id-when-available': error diff --git a/.gitignore b/.gitignore index bff82967fc6..0a7808601ea 100644 --- a/.gitignore +++ b/.gitignore @@ -95,7 +95,6 @@ jsdoc/ webpack-dev-server.json /.nvimrc .solargraph.yml -apollo.config.js /tmp/matching_foss_tests.txt /tmp/matching_tests.txt ee/changelogs/unreleased-ee diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index c9854fad2b6..0d6c81c9624 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -1369,7 +1369,6 @@ - <<: *if-merge-request-labels-skip-undercoverage allow_failure: true - <<: *if-merge-request-labels-run-all-rspec - - <<: *if-merge-request-approved - <<: *if-merge-request changes: *backend-patterns diff --git a/.gitlab/ci/static-analysis.gitlab-ci.yml b/.gitlab/ci/static-analysis.gitlab-ci.yml index ebd223c4171..b2c54f30865 100644 --- a/.gitlab/ci/static-analysis.gitlab-ci.yml +++ b/.gitlab/ci/static-analysis.gitlab-ci.yml @@ -10,6 +10,7 @@ # Disable warnings in browserslist which can break on backports # https://github.com/browserslist/browserslist/blob/a287ec6/node.js#L367-L384 BROWSERSLIST_IGNORE_OLD_DATA: "true" + GRAPHQL_SCHEMA_APOLLO_FILE: "tmp/tests/graphql/gitlab_schema_apollo.graphql" update-static-analysis-cache: extends: @@ -47,17 +48,34 @@ static-verification-with-database: variables: SETUP_DB: "true" +generate-apollo-graphl-schema: + extends: + - .static-analysis-base + - .frontend:rules:default-frontend-jobs + image: + name: registry.gitlab.com/gitlab-org/gitlab-build-images:apollo + entrypoint: [""] + needs: ['graphql-schema-dump'] + variables: + USE_BUNDLE_INSTALL: "false" + script: + - apollo client:download-schema --config=config/apollo.config.js ${GRAPHQL_SCHEMA_APOLLO_FILE} + artifacts: + name: graphql-schema-apollo + paths: + - "${GRAPHQL_SCHEMA_APOLLO_FILE}" + eslint: extends: - .static-analysis-base - .yarn-cache - - .static-analysis:rules:ee - needs: [] + - .frontend:rules:default-frontend-jobs + needs: ['generate-apollo-graphl-schema'] variables: USE_BUNDLE_INSTALL: "false" script: - run_timed_command "retry yarn install --frozen-lockfile" - - run_timed_command "yarn run lint:eslint:all" + - run_timed_command "yarn run lint:eslint:all --parser-options=schema:${GRAPHQL_SCHEMA_APOLLO_FILE}" eslint as-if-foss: extends: diff --git a/.rubocop_todo/style/open_struct_use.yml b/.rubocop_todo/style/open_struct_use.yml index c9dbf7c2cc8..1a5c8167d6c 100644 --- a/.rubocop_todo/style/open_struct_use.yml +++ b/.rubocop_todo/style/open_struct_use.yml @@ -18,7 +18,6 @@ Style/OpenStructUse: - spec/controllers/import/fogbugz_controller_spec.rb - spec/controllers/import/gitlab_controller_spec.rb - spec/controllers/projects/clusters_controller_spec.rb - - spec/dependencies/omniauth_saml_spec.rb - spec/factories/go_module_versions.rb - spec/factories/wiki_pages.rb - spec/features/projects/clusters_spec.rb diff --git a/app/assets/javascripts/alerts_settings/graphql/mutations/create_http_integration.mutation.graphql b/app/assets/javascripts/alerts_settings/graphql/mutations/create_http_integration.mutation.graphql index 172add06785..d4f4f244759 100644 --- a/app/assets/javascripts/alerts_settings/graphql/mutations/create_http_integration.mutation.graphql +++ b/app/assets/javascripts/alerts_settings/graphql/mutations/create_http_integration.mutation.graphql @@ -4,8 +4,7 @@ mutation createHttpIntegration($projectPath: ID!, $name: String!, $active: Boole httpIntegrationCreate(input: { projectPath: $projectPath, name: $name, active: $active }) { errors # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available integration { ...HttpIntegrationItem } diff --git a/app/assets/javascripts/alerts_settings/graphql/mutations/destroy_http_integration.mutation.graphql b/app/assets/javascripts/alerts_settings/graphql/mutations/destroy_http_integration.mutation.graphql index f1d357b229a..caa258e0848 100644 --- a/app/assets/javascripts/alerts_settings/graphql/mutations/destroy_http_integration.mutation.graphql +++ b/app/assets/javascripts/alerts_settings/graphql/mutations/destroy_http_integration.mutation.graphql @@ -4,8 +4,7 @@ mutation destroyHttpIntegration($id: ID!) { httpIntegrationDestroy(input: { id: $id }) { errors # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available integration { ...HttpIntegrationItem } diff --git a/app/assets/javascripts/alerts_settings/graphql/mutations/reset_http_token.mutation.graphql b/app/assets/javascripts/alerts_settings/graphql/mutations/reset_http_token.mutation.graphql index e541fe49968..2f30f9abb5c 100644 --- a/app/assets/javascripts/alerts_settings/graphql/mutations/reset_http_token.mutation.graphql +++ b/app/assets/javascripts/alerts_settings/graphql/mutations/reset_http_token.mutation.graphql @@ -4,8 +4,7 @@ mutation resetHttpIntegrationToken($id: ID!) { httpIntegrationResetToken(input: { id: $id }) { errors # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available integration { ...HttpIntegrationItem } diff --git a/app/assets/javascripts/alerts_settings/graphql/mutations/update_http_integration.mutation.graphql b/app/assets/javascripts/alerts_settings/graphql/mutations/update_http_integration.mutation.graphql index e9942c1720d..2cf56613673 100644 --- a/app/assets/javascripts/alerts_settings/graphql/mutations/update_http_integration.mutation.graphql +++ b/app/assets/javascripts/alerts_settings/graphql/mutations/update_http_integration.mutation.graphql @@ -4,8 +4,7 @@ mutation updateHttpIntegration($id: ID!, $name: String!, $active: Boolean!) { httpIntegrationUpdate(input: { id: $id, name: $name, active: $active }) { errors # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available integration { ...HttpIntegrationItem } diff --git a/app/assets/javascripts/boards/graphql/board_list_create.mutation.graphql b/app/assets/javascripts/boards/graphql/board_list_create.mutation.graphql index d662c33e592..81cc7b4d246 100644 --- a/app/assets/javascripts/boards/graphql/board_list_create.mutation.graphql +++ b/app/assets/javascripts/boards/graphql/board_list_create.mutation.graphql @@ -3,8 +3,7 @@ mutation createBoardList($boardId: BoardID!, $backlog: Boolean, $labelId: LabelID) { boardListCreate(input: { boardId: $boardId, backlog: $backlog, labelId: $labelId }) { # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available list { ...BoardListFragment } diff --git a/app/assets/javascripts/boards/graphql/board_list_update.mutation.graphql b/app/assets/javascripts/boards/graphql/board_list_update.mutation.graphql index e083decdb35..7ea0e2f915a 100644 --- a/app/assets/javascripts/boards/graphql/board_list_update.mutation.graphql +++ b/app/assets/javascripts/boards/graphql/board_list_update.mutation.graphql @@ -3,8 +3,7 @@ mutation UpdateBoardList($listId: ID!, $position: Int, $collapsed: Boolean) { updateBoardList(input: { listId: $listId, position: $position, collapsed: $collapsed }) { # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available list { ...BoardListFragment } diff --git a/app/assets/javascripts/boards/graphql/board_lists.query.graphql b/app/assets/javascripts/boards/graphql/board_lists.query.graphql index b1f0b86b3bf..e6e98864aad 100644 --- a/app/assets/javascripts/boards/graphql/board_lists.query.graphql +++ b/app/assets/javascripts/boards/graphql/board_lists.query.graphql @@ -14,8 +14,7 @@ query BoardLists( hideBacklogList lists(issueFilters: $filters) { # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available nodes { ...BoardListFragment } @@ -29,8 +28,7 @@ query BoardLists( hideBacklogList lists(issueFilters: $filters) { # We have ID in a deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available nodes { ...BoardListFragment } diff --git a/app/assets/javascripts/design_management/graphql/mutations/upload_design.mutation.graphql b/app/assets/javascripts/design_management/graphql/mutations/upload_design.mutation.graphql index 24ec1816974..34d683ac1ee 100644 --- a/app/assets/javascripts/design_management/graphql/mutations/upload_design.mutation.graphql +++ b/app/assets/javascripts/design_management/graphql/mutations/upload_design.mutation.graphql @@ -3,8 +3,7 @@ mutation uploadDesign($files: [Upload!]!, $projectPath: ID!, $iid: ID!) { designManagementUpload(input: { projectPath: $projectPath, iid: $iid, files: $files }) { - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available designs { ...DesignItem versions { diff --git a/app/assets/javascripts/design_management/graphql/queries/get_design.query.graphql b/app/assets/javascripts/design_management/graphql/queries/get_design.query.graphql index a638f80d095..a5394457f73 100644 --- a/app/assets/javascripts/design_management/graphql/queries/get_design.query.graphql +++ b/app/assets/javascripts/design_management/graphql/queries/get_design.query.graphql @@ -13,8 +13,7 @@ query getDesign( id designCollection { designs(atVersion: $atVersion, filenames: $filenames) { - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available nodes { ...DesignItem issue { diff --git a/app/assets/javascripts/graphql_shared/fragments/user_availability.fragment.graphql b/app/assets/javascripts/graphql_shared/fragments/user_availability.fragment.graphql index bb3b4f0e9f8..429993b37bf 100644 --- a/app/assets/javascripts/graphql_shared/fragments/user_availability.fragment.graphql +++ b/app/assets/javascripts/graphql_shared/fragments/user_availability.fragment.graphql @@ -1,5 +1,4 @@ -# TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled -# # eslint-disable-next-line @graphql-eslint/require-id-when-available +# eslint-disable-next-line @graphql-eslint/require-id-when-available fragment UserAvailability on User { status { availability diff --git a/app/assets/javascripts/incidents/graphql/fragments/incident_fields.fragment.graphql b/app/assets/javascripts/incidents/graphql/fragments/incident_fields.fragment.graphql index ac826fe26de..faa68d37088 100644 --- a/app/assets/javascripts/incidents/graphql/fragments/incident_fields.fragment.graphql +++ b/app/assets/javascripts/incidents/graphql/fragments/incident_fields.fragment.graphql @@ -1,5 +1,4 @@ -# TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled -# # eslint-disable-next-line @graphql-eslint/require-id-when-available +# eslint-disable-next-line @graphql-eslint/require-id-when-available fragment IncidentFields on Issue { severity } diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue index db21bc936bf..563defdc2c1 100644 --- a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue +++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue @@ -222,21 +222,18 @@ export default { }, }, i18n: { - tabEdit: s__('Pipelines|Edit'), - tabGraph: s__('Pipelines|Visualize'), - tabLint: s__('Pipelines|Lint'), - }, - resetModal: { - actionPrimary: { - text: __('Reset file'), + resetModal: { + actionPrimary: { + text: __('Reset file'), + }, + actionCancel: { + text: __('Cancel'), + }, + body: s__( + 'Pipeline Editor|Are you sure you want to reset the file to its last committed version?', + ), + title: __('Discard changes'), }, - actionCancel: { - text: __('Cancel'), - }, - body: s__( - 'Pipeline Editor|Are you sure you want to reset the file to its last committed version?', - ), - title: __('Discard changes'), }, watch: { isEmpty(flag) { @@ -364,12 +361,12 @@ export default { - {{ $options.resetModal.body }} + {{ $options.i18n.resetModal.body }} diff --git a/app/assets/javascripts/runner/graphql/get_runner.query.graphql b/app/assets/javascripts/runner/graphql/get_runner.query.graphql index 95b3603c1c6..59c55eae060 100644 --- a/app/assets/javascripts/runner/graphql/get_runner.query.graphql +++ b/app/assets/javascripts/runner/graphql/get_runner.query.graphql @@ -2,8 +2,7 @@ query getRunner($id: CiRunnerID!) { # We have an id in deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available runner(id: $id) { ...RunnerDetails } diff --git a/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql b/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql index a493d1a6f68..8d1b75828be 100644 --- a/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql +++ b/app/assets/javascripts/runner/graphql/runner_update.mutation.graphql @@ -6,8 +6,7 @@ mutation runnerUpdate($input: RunnerUpdateInput!) { runnerUpdate(input: $input) { # We have an id in deep nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available runner { ...RunnerDetails } diff --git a/app/graphql/queries/epic/epic_children.query.graphql b/app/graphql/queries/epic/epic_children.query.graphql index 9e76af48152..c35abcabe44 100644 --- a/app/graphql/queries/epic/epic_children.query.graphql +++ b/app/graphql/queries/epic/epic_children.query.graphql @@ -45,6 +45,16 @@ fragment EpicNode on Epic { confidential hasChildren hasIssues + labels { + __typename + nodes { + __typename + color + description + textColor + title + } + } group { __typename id @@ -72,8 +82,7 @@ query childItems( edges { __typename # We have an id in deeply nested fragment - # TODO: Uncomment next line when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75220 is merged and the rule is enabled - # # eslint-disable-next-line @graphql-eslint/require-id-when-available + # eslint-disable-next-line @graphql-eslint/require-id-when-available node { __typename ...EpicNode @@ -127,6 +136,16 @@ query childItems( dueDate } healthStatus + labels { + __typename + nodes { + __typename + color + description + textColor + title + } + } } } pageInfo { diff --git a/app/models/packages/conan/metadatum.rb b/app/models/packages/conan/metadatum.rb index 7ec2641177a..ad4b499c1e5 100644 --- a/app/models/packages/conan/metadatum.rb +++ b/app/models/packages/conan/metadatum.rb @@ -1,19 +1,26 @@ # frozen_string_literal: true class Packages::Conan::Metadatum < ApplicationRecord + NONE_VALUE = '_' + belongs_to :package, -> { where(package_type: :conan) }, inverse_of: :conan_metadatum validates :package, presence: true validates :package_username, - presence: true, - format: { with: Gitlab::Regex.conan_recipe_component_regex } + :package_channel, + presence: true, + format: { with: Gitlab::Regex.conan_recipe_component_regex }, + if: -> { Feature.disabled?(:packages_conan_allow_empty_username_channel) } - validates :package_channel, - presence: true, - format: { with: Gitlab::Regex.conan_recipe_component_regex } + validates :package_username, + :package_channel, + presence: true, + format: { with: Gitlab::Regex.conan_recipe_user_channel_regex }, + if: -> { Feature.enabled?(:packages_conan_allow_empty_username_channel) } validate :conan_package_type + validate :username_channel_none_values, if: -> { Feature.enabled?(:packages_conan_allow_empty_username_channel) } def recipe "#{package.name}/#{package.version}@#{package_username}/#{package_channel}" @@ -31,6 +38,15 @@ class Packages::Conan::Metadatum < ApplicationRecord package_username.tr('+', '/') end + def self.validate_username_and_channel(username, channel) + return if (username != NONE_VALUE && channel != NONE_VALUE) || + (username == NONE_VALUE && channel == NONE_VALUE) + + none_field = username == NONE_VALUE ? :username : :channel + + yield(none_field) + end + private def conan_package_type @@ -38,4 +54,10 @@ class Packages::Conan::Metadatum < ApplicationRecord errors.add(:base, _('Package type must be Conan')) end end + + def username_channel_none_values + self.class.validate_username_and_channel(package_username, package_channel) do |none_field| + errors.add("package_#{none_field}".to_sym, _("can't be solely blank")) + end + end end diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index e515f1e7320..4757f50739b 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -19,7 +19,6 @@ = render "archived_notice", project: @project = render_if_exists "projects/marked_for_deletion_notice", project: @project = render_if_exists "projects/ancestor_group_marked_for_deletion_notice", project: @project -= render_if_exists 'projects/sast_entry_points', project: @project - view_path = @project.default_view diff --git a/app/views/projects/tracings/show.html.haml b/app/views/projects/tracings/show.html.haml index 21c1d02d92e..813908e5a57 100644 --- a/app/views/projects/tracings/show.html.haml +++ b/app/views/projects/tracings/show.html.haml @@ -3,10 +3,12 @@ - if @project.tracing_external_url.present? %h3.page-title= _('Tracing') - .gl-alert.gl-alert-info.alert.flex-alert - = sprite_icon('information-o', css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title') - .alert-message - = _("Your password isn't required to view this page. If a password or any other personal details are requested, please contact your administrator to report abuse.") + .gl-alert.gl-alert-info.gl-mb-5 + .gl-alert-container + = sprite_icon('information-o', css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title') + .gl-alert-content + .gl-alert-body + = _("Your password isn't required to view this page. If a password or any other personal details are requested, please contact your administrator to report abuse.") - jaeger_link = link_to('Jaeger tracing', 'https://www.jaegertracing.io/', target: "_blank", rel: "noreferrer") %p.light= _("GitLab uses %{jaeger_link} to monitor distributed systems.").html_safe % { jaeger_link: jaeger_link } diff --git a/config/apollo.config.js b/config/apollo.config.js new file mode 100644 index 00000000000..cc847f43286 --- /dev/null +++ b/config/apollo.config.js @@ -0,0 +1,10 @@ +module.exports = { + client: { + service: { + name: 'gitlab', + localSchemaFile: './tmp/tests/graphql/gitlab_schema.graphql', + }, + includes: ['../{ee/,jh/,}app/assets/javascripts/**/*.{js,graphql}'], + excludes: ['../{ee/,jh/,}spec/{frontend,frontend_integration}/**/*'], + }, +}; diff --git a/config/feature_flags/development/new_route_ci_minutes_purchase.yml b/config/feature_flags/development/new_route_ci_minutes_purchase.yml index c34fb14a9f0..06fbfab255c 100644 --- a/config/feature_flags/development/new_route_ci_minutes_purchase.yml +++ b/config/feature_flags/development/new_route_ci_minutes_purchase.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/322582 milestone: '13.10' type: development group: group::purchase -default_enabled: false +default_enabled: true diff --git a/config/feature_flags/development/packages_conan_allow_empty_username_channel.yml b/config/feature_flags/development/packages_conan_allow_empty_username_channel.yml new file mode 100644 index 00000000000..15a38dcf1b6 --- /dev/null +++ b/config/feature_flags/development/packages_conan_allow_empty_username_channel.yml @@ -0,0 +1,8 @@ +--- +name: packages_conan_allow_empty_username_channel +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75106 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346006 +milestone: '14.5' +type: development +group: group::package +default_enabled: false diff --git a/config/initializers/grape_validators.rb b/config/initializers/grape_validators.rb index 1492894e1fa..402da1c9abe 100644 --- a/config/initializers/grape_validators.rb +++ b/config/initializers/grape_validators.rb @@ -11,3 +11,6 @@ Grape::Validations.register_validator(:untrusted_regexp, ::API::Validations::Val Grape::Validations.register_validator(:email_or_email_list, ::API::Validations::Validators::EmailOrEmailList) Grape::Validations.register_validator(:iteration_id, ::API::Validations::Validators::IntegerOrCustomValue) Grape::Validations.register_validator(:project_portable, ::API::Validations::Validators::ProjectPortable) +# TODO Delete this validator along with the packages_conan_allow_empty_username_channel feature flag +# # https://gitlab.com/gitlab-org/gitlab/-/issues/346006 +Grape::Validations.register_validator('packages_conan_user_channel', ::API::Validations::Validators::PackagesConanUserChannel) diff --git a/doc/administration/geo/secondary_proxy/index.md b/doc/administration/geo/secondary_proxy/index.md index 2b8c0d1e6fa..209c12a1b7e 100644 --- a/doc/administration/geo/secondary_proxy/index.md +++ b/doc/administration/geo/secondary_proxy/index.md @@ -7,11 +7,14 @@ type: howto # Geo proxying for secondary sites **(PREMIUM SELF)** -> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5914) in GitLab 14.4 [with a flag](../../feature_flags.md) named `geo_secondary_proxy`. Disabled by default. +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5914) in GitLab 14.4 [with a flag](../../feature_flags.md) named `geo_secondary_proxy`. Disabled by default. +> - [Enabled by default for unified URLs](https://gitlab.com/gitlab-org/gitlab/-/issues/325732) in GitLab 14.6. +> - [Disabled by default for different URLs](https://gitlab.com/gitlab-org/gitlab/-/issues/325732) in GitLab 14.6 [with a flag](../../feature_flags.md) named `geo_secondary_proxy_separate_urls`. FLAG: -On self-managed GitLab, by default this feature is not available. See below to [Set up a unified URL for Geo sites](#set-up-a-unified-url-for-geo-sites). -The feature is not ready for production use. +On self-managed GitLab, this feature is only available by default for Geo sites using a unified URL. See below to +[set up a unified URL for Geo sites](#set-up-a-unified-url-for-geo-sites). +The feature is not ready for production use with separate URLs. Use Geo proxying to: @@ -65,8 +68,10 @@ a single URL used by all Geo sites, including the primary. In the Geo administration page of the **primary** site, edit each Geo secondary that is using the secondary proxying and set the `URL` field to the single URL. Make sure the primary site is also using this URL. + +## Disable Geo proxying -### Enable secondary proxying +You can disable the secondary proxying on each Geo site, separately, by following these steps: 1. SSH into each application node (serving user traffic directly) on your secondary Geo site and add the following environment variable: @@ -77,7 +82,7 @@ a single URL used by all Geo sites, including the primary. ```ruby gitlab_workhorse['env'] = { - "GEO_SECONDARY_PROXY" => "1" + "GEO_SECONDARY_PROXY" => "0" } ``` @@ -87,18 +92,34 @@ a single URL used by all Geo sites, including the primary. gitlab-ctl reconfigure ``` -1. SSH into one node running Rails on your primary Geo site and enable the Geo secondary proxy feature flag: - - ```shell - sudo gitlab-rails runner "Feature.enable(:geo_secondary_proxy)" - ``` - ## Enable Geo proxying with Separate URLs The ability to use proxying with separate URLs is still in development. You can follow the ["Geo secondary proxying with separate URLs" epic](https://gitlab.com/groups/gitlab-org/-/epics/6865) for progress. +To try out this feature, enable the `geo_secondary_proxy_separate_urls` feature flag. +SSH into one node running Rails on your primary Geo site and run: + +```shell +sudo gitlab-rails runner "Feature.enable(:geo_secondary_proxy_separate_urls)" +``` + +## Limitations + +The asynchronous Geo replication can cause unexpected issues when secondary proxying is used, for accelerated +data types that may be replicated to the Geo secondaries with a delay. + +For example, we found a potential issue where +[Replication lag introduces read-your-own-write inconsistencies](https://gitlab.com/gitlab-org/gitlab/-/issues/345267). +If the replication lag is high enough, this can result in Git reads receiving stale data when hitting a secondary. + +Non-Rails requests are not proxied, so other services may need to use a separate, non-unified URL to ensure requests +are always sent to the primary. These services include: + +- GitLab Container Registry - [can be configured to use a separate domain](../../packages/container_registry.md#configure-container-registry-under-its-own-domain). +- GitLab Pages - should always use a separate domain, as part of [the prerequisites for running GitLab Pages](../../pages/index.md#prerequisites). + ## Features accelerated by secondary Geo sites Most HTTP traffic sent to a secondary Geo site can be proxied to the primary Geo site. With this architecture, diff --git a/doc/administration/geo/setup/index.md b/doc/administration/geo/setup/index.md index 84dff69ebe7..7d365f73101 100644 --- a/doc/administration/geo/setup/index.md +++ b/doc/administration/geo/setup/index.md @@ -26,6 +26,7 @@ If you installed GitLab using the Omnibus packages (highly recommended): 1. [Configure GitLab](../replication/configuration.md) to set the **primary** and **secondary** site(s). 1. Optional: [Configure a secondary LDAP server](../../auth/ldap/index.md) for the **secondary** site(s). See [notes on LDAP](../index.md#ldap). 1. Follow the [Using a Geo Site](../replication/usage.md) guide. +1. [Configure Geo secondary proxying](../secondary_proxy/index.md) to use a single, unified URL for all Geo sites. This step is recommended to accelerate most read requests while transparently proxying writes to the primary Geo site. ## Post-installation documentation diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index 44d43a32803..0f91065fead 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -171,6 +171,40 @@ import { getIdFromGraphQLId } from '~/graphql_shared/utils'; const primaryKeyId = getIdFromGraphQLId(data.id); ``` +**It is required** to query global `id` for every GraphQL type that has an `id` in the schema: + +```javascript +query allReleases(...) { + project(...) { + id // Project has an ID in GraphQL schema so should fetch it + releases(...) { + nodes { + // Release has no ID property in GraphQL schema + name + tagName + tagPath + assets { + count + links { + nodes { + id // Link has an ID in GraphQL schema so should fetch it + name + } + } + } + } + pageInfo { + // PageInfo no ID property in GraphQL schema + startCursor + hasPreviousPage + hasNextPage + endCursor + } + } + } +} +``` + ## Immutability and cache updates From Apollo version 3.0.0 all the cache updates need to be immutable. It needs to be replaced entirely @@ -808,11 +842,11 @@ export default { #### Polling and Performance -While the Apollo client has support for simple polling, for performance reasons, our [Etag-based caching](../polling.md) is preferred to hitting the database each time. +While the Apollo client has support for simple polling, for performance reasons, our [ETag-based caching](../polling.md) is preferred to hitting the database each time. -Once the backend is set up, there are a few changes to make on the frontend. +After the ETag resource is set up to be cached from backend, there are a few changes to make on the frontend. -First, get your resource ETag path from the backend. In the example of the pipelines graph, this is called the `graphql_resource_etag`. This will be used to create new headers to add to the Apollo context: +First, get your ETag resource from the backend, which should be in the form of a URL path. In the example of the pipelines graph, this is called the `graphql_resource_etag`, which is used to create new headers to add to the Apollo context: ```javascript /* pipelines/components/graph/utils.js */ @@ -847,7 +881,51 @@ apollo: { }, ``` -Then, because ETags depend on the request being a `GET` instead of GraphQL's usual `POST`, but our default link library does not support `GET` we need to let our default Apollo client know to use a different library. +Here, the apollo query is watching for changes in `graphqlResourceEtag`. If your ETag resource dynamically changes, you should make sure the resource you are sending in the query headers is also updated. To do this, you can store and update the ETag resource dynamically in the local cache. + +You can see an example of this in the pipeline status of the pipeline editor. The pipeline editor watches for changes in the latest pipeline. When the user creates a new commit, we update the pipeline query to poll for changes in the new pipeline. + +```graphql +# pipeline_etag.graphql + +query getPipelineEtag { + pipelineEtag @client +} +``` + +```javascript +/* pipeline_editor/components/header/pipeline_status.vue */ + +import getPipelineEtag from '~/pipeline_editor/graphql/queries/client/pipeline_etag.graphql'; + +apollo: { + pipelineEtag: { + query: getPipelineEtag, + }, + pipeline: { + context() { + return getQueryHeaders(this.pipelineEtag); + }, + query: getPipelineQuery, + pollInterval: POLL_INTERVAL, + }, +} + +/* pipeline_editor/components/commit/commit_section.vue */ + +await this.$apollo.mutate({ + mutation: commitCIFile, + update(store, { data }) { + const pipelineEtag = data?.commitCreate?.commit?.commitPipelinePath; + + if (pipelineEtag) { + store.writeQuery({ query: getPipelineEtag, data: { pipelineEtag } }); + } + }, +}); +``` + +ETags depend on the request being a `GET` instead of GraphQL's usual `POST`. Our default link library does not support `GET` requests, so we must let our default Apollo client know to use a different library. Keep in mind, this means your app cannot batch queries. ```javascript /* componentMountIndex.js */ @@ -862,10 +940,35 @@ const apolloProvider = new VueApollo({ }); ``` -Keep in mind, this means your app will not batch queries. +Finally, we can add a visibility check so that the component pauses polling when the browser tab is not active. This should lessen the request load on the page. + +```javascript +/* component.vue */ + +import { toggleQueryPollingByVisibility } from '~/pipelines/components/graph/utils'; + +export default { + mounted() { + toggleQueryPollingByVisibility(this.$apollo.queries.pipeline, POLL_INTERVAL); + }, +}; +``` + +You can use [this MR](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59672/) as a reference on how to fully implement ETag caching on the frontend. Once subscriptions are mature, this process can be replaced by using them and we can remove the separate link library and return to batching queries. +##### How to test ETag caching + +You can test that your implementation works by checking requests on the network tab. If there are no changes in your ETag resource, all polled requests should: + +- Be `GET` requests instead of `POST` requests. +- Have an HTTP status of `304` instead of `200`. + +Make sure that caching is not disabled in your developer tools when testing. + +If you are using Chrome and keep seeing `200` HTTP status codes, it might be this bug: [Developer tools show 200 instead of 304](https://bugs.chromium.org/p/chromium/issues/detail?id=1269602). In this case, inspect the response headers' source to confirm that the request was actually cached and did return with a `304` status code. + #### Subscriptions We use [subscriptions](https://www.apollographql.com/docs/react/data/subscriptions/) to receive real-time updates from GraphQL API via websockets. Currently, the number of existing subscriptions is limited, you can check a list of available ones in [GraphqiQL explorer](https://gitlab.com/-/graphql-explorer) diff --git a/doc/user/packages/conan_repository/index.md b/doc/user/packages/conan_repository/index.md index 0f32f68d250..64cf2524bf0 100644 --- a/doc/user/packages/conan_repository/index.md +++ b/doc/user/packages/conan_repository/index.md @@ -103,6 +103,30 @@ A package with the recipe `Hello/0.1@mycompany/beta` is created. For more details about creating and managing Conan packages, see the [Conan documentation](https://docs.conan.io/en/latest/creating_packages.html). +#### Package without a username and a channel + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/345055) in GitLab 14.6 [with a flag](../../../administration/feature_flags.md) named `packages_conan_allow_empty_username_channel`. Disabled by default. +> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/345055) in GitLab 14.6. + +Even though they are [recommended](https://docs.conan.io/en/latest/reference/conanfile/attributes.html#user-channel) +to distinguish your package from a similarly named existing package, +the username and channel are not mandatory fields for a Conan package. + +You can create a package without a username and channel by removing them from +the `create` command: + +```shell +conan create . +``` + +The username _and_ the channel must be blank. If only one of these fields is +blank, the request is rejected. + +NOTE: +Empty usernames and channels can only be used if you use a [project remote](#add-a-remote-for-your-project). +If you use an [instance remote](#add-a-remote-for-your-instance), the username +and the channel must be set. + ## Add the Package Registry as a Conan remote To run `conan` commands, you must add the Package Registry as a Conan remote for diff --git a/lib/api/concerns/packages/conan_endpoints.rb b/lib/api/concerns/packages/conan_endpoints.rb index 3194cdebde8..fbb9e5cff53 100644 --- a/lib/api/concerns/packages/conan_endpoints.rb +++ b/lib/api/concerns/packages/conan_endpoints.rb @@ -105,10 +105,14 @@ module API params do requires :package_name, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package name' requires :package_version, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package version' - requires :package_username, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package username' - requires :package_channel, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package channel' + requires :package_username, type: String, packages_conan_user_channel: true, desc: 'Package username' + requires :package_channel, type: String, packages_conan_user_channel: true, desc: 'Package channel' end namespace 'conans/:package_name/:package_version/:package_username/:package_channel', requirements: PACKAGE_REQUIREMENTS do + after_validation do + check_username_channel if Feature.enabled?(:packages_conan_allow_empty_username_channel) + end + # Get the snapshot # # the snapshot is a hash of { filename: md5 hash } @@ -264,8 +268,8 @@ module API params do requires :package_name, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package name' requires :package_version, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package version' - requires :package_username, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package username' - requires :package_channel, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package channel' + requires :package_username, type: String, packages_conan_user_channel: true, desc: 'Package username' + requires :package_channel, type: String, packages_conan_user_channel: true, desc: 'Package channel' requires :recipe_revision, type: String, regexp: CONAN_REVISION_REGEX, desc: 'Conan Recipe Revision' end namespace 'files/:package_name/:package_version/:package_username/:package_channel/:recipe_revision', requirements: PACKAGE_REQUIREMENTS do @@ -273,6 +277,10 @@ module API authenticate_non_get! end + after_validation do + check_username_channel if Feature.enabled?(:packages_conan_allow_empty_username_channel) + end + params do requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES end diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index 4b6dac39348..031c29e7472 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -7,6 +7,21 @@ module API module ApiHelpers include Gitlab::Utils::StrongMemoize + def check_username_channel + username = declared(params)[:package_username] + channel = declared(params)[:package_channel] + + if username == ::Packages::Conan::Metadatum::NONE_VALUE && package_scope == :instance + # at the instance level, username must not be empty (naming convention) + # don't try to process the empty username and eagerly return not found. + not_found! + end + + ::Packages::Conan::Metadatum.validate_username_and_channel(username, channel) do |none_field| + bad_request!("#{none_field} can't be solely blank") + end + end + def present_download_urls(entity) authorize!(:read_package, project) diff --git a/lib/api/validations/validators/packages_conan_user_channel.rb b/lib/api/validations/validators/packages_conan_user_channel.rb new file mode 100644 index 00000000000..14afda9657c --- /dev/null +++ b/lib/api/validations/validators/packages_conan_user_channel.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module API + module Validations + module Validators + # TODO Delete this validator along with the packages_conan_allow_empty_username_channel feature flag + # Use a regexp validator in its place: regexp: Gitlab::Regex.conan_recipe_user_channel_regex + # https://gitlab.com/gitlab-org/gitlab/-/issues/346006 + class PackagesConanUserChannel < Grape::Validations::Base + def validate_param!(attr_name, params) + value = params[attr_name] + + if Feature.enabled?(:packages_conan_allow_empty_username_channel) + unless Gitlab::Regex.conan_recipe_user_channel_regex.match?(value) + raise Grape::Exceptions::Validation.new( + params: [@scope.full_name(attr_name)], + message: 'is invalid' + ) + end + else + unless Gitlab::Regex.conan_recipe_component_regex.match?(value) + raise Grape::Exceptions::Validation.new( + params: [@scope.full_name(attr_name)], + message: 'is invalid' + ) + end + end + end + end + end + end +end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index e247515bdde..b1fcf0d1956 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -16,8 +16,13 @@ module Gitlab @conan_revision_regex ||= %r{\A0\z}.freeze end + def conan_recipe_user_channel_regex + %r{\A(_|#{conan_name_regex})\z}.freeze + end + def conan_recipe_component_regex - @conan_recipe_component_regex ||= %r{\A[a-zA-Z0-9_][a-zA-Z0-9_\+\.-]{1,49}\z}.freeze + # https://docs.conan.io/en/latest/reference/conanfile/attributes.html#name + @conan_recipe_component_regex ||= %r{\A#{conan_name_regex}\z}.freeze end def composer_package_version_regex @@ -211,6 +216,12 @@ module Gitlab def generic_package_file_name_regex generic_package_name_regex end + + private + + def conan_name_regex + @conan_name_regex ||= %r{[a-zA-Z0-9_][a-zA-Z0-9_\+\.-]{1,49}}.freeze + end end extend self diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f5de4ae8bc2..9b5b254b522 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -30298,21 +30298,6 @@ msgstr "" msgid "SVG illustration" msgstr "" -msgid "SastEntryPoints|Add Security Testing" -msgstr "" - -msgid "SastEntryPoints|Catch your security vulnerabilities ahead of time!" -msgstr "" - -msgid "SastEntryPoints|GitLab can scan your code for security vulnerabilities. Static Application Security Testing (SAST) helps you worry less and build more." -msgstr "" - -msgid "SastEntryPoints|How do I set up SAST?" -msgstr "" - -msgid "SastEntryPoints|Learn more" -msgstr "" - msgid "Satisfied" msgstr "" @@ -40593,6 +40578,9 @@ msgstr "" msgid "can't be nil" msgstr "" +msgid "can't be solely blank" +msgstr "" + msgid "can't be the same as the source project" msgstr "" diff --git a/spec/dependencies/omniauth_saml_spec.rb b/spec/dependencies/omniauth_saml_spec.rb index fa179eb1516..8956fa44b7a 100644 --- a/spec/dependencies/omniauth_saml_spec.rb +++ b/spec/dependencies/omniauth_saml_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'processing of SAMLResponse in dependencies' do let(:mock_saml_response) { File.read('spec/fixtures/authentication/saml_response.xml') } let(:saml_strategy) { OmniAuth::Strategies::SAML.new({}) } let(:session_mock) { {} } - let(:settings) { OpenStruct.new({ soft: false, idp_cert_fingerprint: 'something' }) } + let(:settings) { double('settings', { soft: false, idp_cert_fingerprint: 'something' }) } let(:auth_hash) { Gitlab::Auth::Saml::AuthHash.new(saml_strategy) } subject { auth_hash.authn_context } diff --git a/spec/lib/api/validations/validators/packages_conan_user_channel_spec.rb b/spec/lib/api/validations/validators/packages_conan_user_channel_spec.rb new file mode 100644 index 00000000000..86cb4c54a57 --- /dev/null +++ b/spec/lib/api/validations/validators/packages_conan_user_channel_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Validations::Validators::PackagesConanUserChannel do + include ApiValidatorsHelpers + + describe '#validate_param!' do + subject do + described_class.new(['test'], {}, false, scope.new) + end + + shared_examples 'accepting valid values for conan user channels' do + let(:fifty_one_characters) { 'f_a' * 17} + + it { expect_no_validation_error('test' => 'foobar') } + it { expect_no_validation_error('test' => 'foo_bar') } + it { expect_no_validation_error('test' => 'foo+bar') } + it { expect_no_validation_error('test' => '_foo+bar-baz+1.0') } + it { expect_no_validation_error('test' => '1.0.0') } + it { expect_validation_error('test' => '-foo_bar') } + it { expect_validation_error('test' => '+foo_bar') } + it { expect_validation_error('test' => '.foo_bar') } + it { expect_validation_error('test' => 'foo@bar') } + it { expect_validation_error('test' => 'foo/bar') } + it { expect_validation_error('test' => '!!()()') } + it { expect_validation_error('test' => fifty_one_characters) } + end + + it_behaves_like 'accepting valid values for conan user channels' + it { expect_no_validation_error('test' => '_') } + + context 'with packages_conan_allow_empty_username_channel disabled' do + before do + stub_feature_flags(packages_conan_allow_empty_username_channel: false) + end + + it_behaves_like 'accepting valid values for conan user channels' + it { expect_validation_error('test' => '_') } + end + end +end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 9514654204b..97aa36bfb74 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -264,23 +264,37 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('1.2.3') } end - describe '.conan_recipe_component_regex' do - subject { described_class.conan_recipe_component_regex } + context 'conan recipe components' do + shared_examples 'accepting valid recipe components values' do + let(:fifty_one_characters) { 'f_a' * 17} - let(:fifty_one_characters) { 'f_a' * 17} + it { is_expected.to match('foobar') } + it { is_expected.to match('foo_bar') } + it { is_expected.to match('foo+bar') } + it { is_expected.to match('_foo+bar-baz+1.0') } + it { is_expected.to match('1.0.0') } + it { is_expected.not_to match('-foo_bar') } + it { is_expected.not_to match('+foo_bar') } + it { is_expected.not_to match('.foo_bar') } + it { is_expected.not_to match('foo@bar') } + it { is_expected.not_to match('foo/bar') } + it { is_expected.not_to match('!!()()') } + it { is_expected.not_to match(fifty_one_characters) } + end - it { is_expected.to match('foobar') } - it { is_expected.to match('foo_bar') } - it { is_expected.to match('foo+bar') } - it { is_expected.to match('_foo+bar-baz+1.0') } - it { is_expected.to match('1.0.0') } - it { is_expected.not_to match('-foo_bar') } - it { is_expected.not_to match('+foo_bar') } - it { is_expected.not_to match('.foo_bar') } - it { is_expected.not_to match('foo@bar') } - it { is_expected.not_to match('foo/bar') } - it { is_expected.not_to match('!!()()') } - it { is_expected.not_to match(fifty_one_characters) } + describe '.conan_recipe_component_regex' do + subject { described_class.conan_recipe_component_regex } + + it_behaves_like 'accepting valid recipe components values' + it { is_expected.not_to match('_') } + end + + describe '.conan_recipe_user_channel_regex' do + subject { described_class.conan_recipe_user_channel_regex } + + it_behaves_like 'accepting valid recipe components values' + it { is_expected.to match('_') } + end end describe '.package_name_regex' do diff --git a/spec/models/packages/conan/metadatum_spec.rb b/spec/models/packages/conan/metadatum_spec.rb index 112f395818b..1e0bcd7eede 100644 --- a/spec/models/packages/conan/metadatum_spec.rb +++ b/spec/models/packages/conan/metadatum_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Packages::Conan::Metadatum, type: :model do + using RSpec::Parameterized::TableSyntax + describe 'relationships' do it { is_expected.to belong_to(:package) } end @@ -45,6 +47,36 @@ RSpec.describe Packages::Conan::Metadatum, type: :model do it { is_expected.not_to allow_value("my@channel").for(:package_channel) } end + describe '#username_channel_none_values' do + let_it_be(:package) { create(:conan_package) } + + let(:metadatum) { package.conan_metadatum } + + subject { metadatum.valid? } + + where(:username, :channel, :feature_flag, :valid) do + 'username' | 'channel' | true | true + 'username' | '_' | true | false + '_' | 'channel' | true | false + '_' | '_' | true | true + + 'username' | 'channel' | false | true + 'username' | '_' | false | false + '_' | 'channel' | false | false + '_' | '_' | false | false + end + + with_them do + before do + metadatum.package_username = username + metadatum.package_channel = channel + stub_feature_flags(packages_conan_allow_empty_username_channel: feature_flag) + end + + it { is_expected.to eq(valid) } + end + end + describe '#conan_package_type' do it 'will not allow a package with a different package_type' do package = build('package') @@ -87,4 +119,27 @@ RSpec.describe Packages::Conan::Metadatum, type: :model do expect(described_class.full_path_from(package_username: username)).to eq('foo/bar/baz-buz') end end + + describe '.validate_username_and_channel' do + where(:username, :channel, :error_field) do + 'username' | 'channel' | nil + 'username' | '_' | :channel + '_' | 'channel' | :username + '_' | '_' | nil + end + + with_them do + if params[:error_field] + it 'yields the block when there is an error' do + described_class.validate_username_and_channel(username, channel) do |none_field| + expect(none_field).to eq(error_field) + end + end + else + it 'does not yield the block when there is no error' do + expect { |b| described_class.validate_username_and_channel(username, channel, &b) }.not_to yield_control + end + end + end + end end diff --git a/spec/requests/api/conan_project_packages_spec.rb b/spec/requests/api/conan_project_packages_spec.rb index da054ed2e96..c108f2efaaf 100644 --- a/spec/requests/api/conan_project_packages_spec.rb +++ b/spec/requests/api/conan_project_packages_spec.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe API::ConanProjectPackages, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/326194' do +RSpec.describe API::ConanProjectPackages do include_context 'conan api setup' let(:project_id) { project.id } + let(:snowplow_standard_context_params) { { user: user, project: project, namespace: project.namespace } } describe 'GET /api/v4/projects/:id/packages/conan/v1/ping' do let(:url) { "/projects/#{project.id}/packages/conan/v1/ping" } @@ -92,7 +93,7 @@ RSpec.describe API::ConanProjectPackages, quarantine: 'https://gitlab.com/gitlab end end - context 'file download endpoints' do + context 'file download endpoints', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/326194' do include_context 'conan file download endpoints' describe 'GET /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/ diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6a49b8f0832..c497f8245fe 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -481,6 +481,10 @@ Rugged::Settings['search_path_global'] = Rails.root.join('tmp/tests').to_s # Initialize FactoryDefault to use create_default helper TestProf::FactoryDefault.init +# Exclude the Geo proxy API request from getting on_next_request Warden handlers, +# necessary to prevent race conditions with feature tests not getting authenticated. +::Warden.asset_paths << %r{^/api/v4/geo/proxy$} + module TouchRackUploadedFile def initialize_from_file_path(path) super diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index 20606ae942d..e62314e624e 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -178,6 +178,60 @@ RSpec.shared_examples 'rejects invalid recipe' do end end +RSpec.shared_examples 'handling empty values for username and channel' do + using RSpec::Parameterized::TableSyntax + + let(:recipe_path) { "#{package.name}/#{package.version}/#{package_username}/#{channel}" } + + where(:username, :channel, :feature_flag, :status) do + 'username' | 'channel' | true | :ok + 'username' | '_' | true | :bad_request + '_' | 'channel' | true | :bad_request_or_not_found + '_' | '_' | true | :ok_or_not_found + + 'username' | 'channel' | false | :ok + 'username' | '_' | false | :bad_request + '_' | 'channel' | false | :bad_request + '_' | '_' | false | :bad_request + end + + with_them do + let(:package_username) do + if username == 'username' + package.conan_metadatum.package_username + else + username + end + end + + before do + stub_feature_flags(packages_conan_allow_empty_username_channel: feature_flag) + project.add_maintainer(user) # avoid any permission issue + end + + it 'returns the correct status code' do |example| + project_level = example.full_description.include?('api/v4/projects') + + expected_status = case status + when :ok_or_not_found + project_level ? :ok : :not_found + when :bad_request_or_not_found + project_level ? :bad_request : :not_found + else + status + end + + if expected_status == :ok + package.conan_metadatum.update!(package_username: package_username, package_channel: channel) + end + + subject + + expect(response).to have_gitlab_http_status(expected_status) + end + end +end + RSpec.shared_examples 'rejects invalid file_name' do |invalid_file_name| let(:file_name) { invalid_file_name } @@ -300,6 +354,7 @@ RSpec.shared_examples 'recipe snapshot endpoint' do it_behaves_like 'rejects invalid recipe' it_behaves_like 'rejects recipe for invalid project' it_behaves_like 'empty recipe for not found package' + it_behaves_like 'handling empty values for username and channel' context 'with existing package' do it 'returns a hash of files with their md5 hashes' do @@ -324,6 +379,7 @@ RSpec.shared_examples 'package snapshot endpoint' do it_behaves_like 'rejects invalid recipe' it_behaves_like 'rejects recipe for invalid project' it_behaves_like 'empty recipe for not found package' + it_behaves_like 'handling empty values for username and channel' context 'with existing package' do it 'returns a hash of md5 values for the files' do @@ -344,12 +400,14 @@ RSpec.shared_examples 'recipe download_urls endpoint' do it_behaves_like 'rejects invalid recipe' it_behaves_like 'rejects recipe for invalid project' it_behaves_like 'recipe download_urls' + it_behaves_like 'handling empty values for username and channel' end RSpec.shared_examples 'package download_urls endpoint' do it_behaves_like 'rejects invalid recipe' it_behaves_like 'rejects recipe for invalid project' it_behaves_like 'package download_urls' + it_behaves_like 'handling empty values for username and channel' end RSpec.shared_examples 'recipe upload_urls endpoint' do @@ -362,6 +420,7 @@ RSpec.shared_examples 'recipe upload_urls endpoint' do it_behaves_like 'rejects invalid recipe' it_behaves_like 'rejects invalid upload_url params' + it_behaves_like 'handling empty values for username and channel' it 'returns a set of upload urls for the files requested' do subject @@ -423,6 +482,7 @@ RSpec.shared_examples 'package upload_urls endpoint' do it_behaves_like 'rejects invalid recipe' it_behaves_like 'rejects invalid upload_url params' + it_behaves_like 'handling empty values for username and channel' it 'returns a set of upload urls for the files requested' do expected_response = { @@ -458,6 +518,7 @@ RSpec.shared_examples 'delete package endpoint' do let(:recipe_path) { package.conan_recipe_path } it_behaves_like 'rejects invalid recipe' + it_behaves_like 'handling empty values for username and channel' it 'returns unauthorized for users without valid permission' do subject @@ -568,12 +629,14 @@ RSpec.shared_examples 'recipe file download endpoint' do it_behaves_like 'a public project with packages' it_behaves_like 'an internal project with packages' it_behaves_like 'a private project with packages' + it_behaves_like 'handling empty values for username and channel' end RSpec.shared_examples 'package file download endpoint' do it_behaves_like 'a public project with packages' it_behaves_like 'an internal project with packages' it_behaves_like 'a private project with packages' + it_behaves_like 'handling empty values for username and channel' context 'tracking the conan_package.tgz download' do let(:package_file) { package.package_files.find_by(file_name: ::Packages::Conan::FileMetadatum::PACKAGE_BINARY) } @@ -598,6 +661,7 @@ RSpec.shared_examples 'workhorse authorize endpoint' do it_behaves_like 'rejects invalid recipe' it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' it_behaves_like 'workhorse authorization' + it_behaves_like 'handling empty values for username and channel' end RSpec.shared_examples 'workhorse recipe file upload endpoint' do @@ -619,6 +683,7 @@ RSpec.shared_examples 'workhorse recipe file upload endpoint' do it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' it_behaves_like 'uploads a package file' it_behaves_like 'creates build_info when there is a job' + it_behaves_like 'handling empty values for username and channel' end RSpec.shared_examples 'workhorse package file upload endpoint' do @@ -640,6 +705,7 @@ RSpec.shared_examples 'workhorse package file upload endpoint' do it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest' it_behaves_like 'uploads a package file' it_behaves_like 'creates build_info when there is a job' + it_behaves_like 'handling empty values for username and channel' context 'tracking the conan_package.tgz upload' do let(:file_name) { ::Packages::Conan::FileMetadatum::PACKAGE_BINARY } diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index e57f58d59dd..6835569dfa8 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -65,7 +65,7 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback Config: cfg, accessLogger: accessLogger, // Kind of a feature flag. See https://gitlab.com/groups/gitlab-org/-/epics/5914#note_564974130 - enableGeoProxyFeature: os.Getenv("GEO_SECONDARY_PROXY") == "1", + enableGeoProxyFeature: os.Getenv("GEO_SECONDARY_PROXY") != "0", geoProxyBackend: &url.URL{}, } if up.geoProxyPollSleep == nil { @@ -207,8 +207,8 @@ func (u *upstream) findGeoProxyRoute(cleanedPath string, r *http.Request) *route func (u *upstream) pollGeoProxyAPI() { for { - u.callGeoProxyAPI() u.geoProxyPollSleep(geoProxyApiPollingInterval) + u.callGeoProxyAPI() } } diff --git a/workhorse/internal/upstream/upstream_test.go b/workhorse/internal/upstream/upstream_test.go index 2031fa84ce2..c685f82c388 100644 --- a/workhorse/internal/upstream/upstream_test.go +++ b/workhorse/internal/upstream/upstream_test.go @@ -314,5 +314,9 @@ func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*h } } + // Since the first sleep happens before any API call, this ensures + // we call the API at least once. + waitForNextApiPoll() + return ws, ws.Close, waitForNextApiPoll }