diff options
34 files changed, 663 insertions, 373 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d0a0c7b2414..b72ad359532 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -5,9 +5,7 @@ stages: - fixtures - test - post-test - - review-prepare - review - - dast - qa - post-qa - pages diff --git a/.gitlab/ci/dast.gitlab-ci.yml b/.gitlab/ci/review-apps/dast.gitlab-ci.yml index 512c850b7da..512c850b7da 100644 --- a/.gitlab/ci/dast.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/dast.gitlab-ci.yml diff --git a/.gitlab/ci/review-apps/main.gitlab-ci.yml b/.gitlab/ci/review-apps/main.gitlab-ci.yml new file mode 100644 index 00000000000..6fe9e39cb82 --- /dev/null +++ b/.gitlab/ci/review-apps/main.gitlab-ci.yml @@ -0,0 +1,106 @@ +stages: + - prepare + - deploy + - qa + - post-qa + - dast + +include: + - local: .gitlab/ci/global.gitlab-ci.yml + - local: .gitlab/ci/rules.gitlab-ci.yml + - local: .gitlab/ci/review-apps/qa.gitlab-ci.yml + - local: .gitlab/ci/review-apps/dast.gitlab-ci.yml + +.base-before_script: &base-before_script + - source ./scripts/utils.sh + - source ./scripts/review_apps/review-apps.sh + - install_api_client_dependencies_with_apk + +review-build-cng: + extends: + - .default-retry + - .review:rules:review-build-cng + image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine3.13 + stage: prepare + variables: + CNG_PROJECT_ACCESS_TOKEN: "${CNG_MIRROR_PROJECT_ACCESS_TOKEN}" # "Multi-pipeline (from 'gitlab-org/gitlab' 'review-build-cng' job)" at https://gitlab.com/gitlab-org/build/CNG-mirror/-/settings/access_tokens + CNG_PROJECT_PATH: "gitlab-org/build/CNG-mirror" + before_script: + - source ./scripts/utils.sh + - install_gitlab_gem + script: + - ./scripts/trigger-build cng + +.review-workflow-base: + extends: + - .default-retry + image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-helm3.5-kubectl1.17 + variables: + HOST_SUFFIX: "${CI_ENVIRONMENT_SLUG}" + DOMAIN: "-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN}" + GITLAB_HELM_CHART_REF: "v5.2.1" + environment: + name: review/${CI_COMMIT_REF_SLUG}${FREQUENCY} + url: https://gitlab-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN} + on_stop: review-stop + auto_stop_in: 48 hours + +review-deploy: + extends: + - .review-workflow-base + - .review:rules:review-deploy + stage: deploy + needs: ["review-build-cng"] + resource_group: "review/${CI_COMMIT_REF_NAME}" + before_script: + - export GITLAB_SHELL_VERSION=$(<GITLAB_SHELL_VERSION) + - export GITALY_VERSION=$(<GITALY_SERVER_VERSION) + - export GITLAB_WORKHORSE_VERSION=$(<GITLAB_WORKHORSE_VERSION) + - echo "${CI_ENVIRONMENT_URL}" > environment_url.txt + - *base-before_script + script: + - check_kube_domain + - download_chart + - date + - deploy || (display_deployment_debug && exit 1) + - verify_deploy || exit 1 + - disable_sign_ups || (delete_release && exit 1) + after_script: + # Run seed-dast-test-data.sh only when DAST_RUN is set to true. This is to pupulate review app with data for DAST scan. + # Set DAST_RUN to true when jobs are manually scheduled. + - if [ "$DAST_RUN" == "true" ]; then source scripts/review_apps/seed-dast-test-data.sh; TRACE=1 trigger_proj_user_creation; fi + artifacts: + paths: + - environment_url.txt + - curl_output.txt + expire_in: 7 days + when: always + +.review-stop-base: + extends: .review-workflow-base + environment: + action: stop + dependencies: [] + variables: + # We're cloning the repo instead of downloading the script for now + # because some repos are private and CI_JOB_TOKEN cannot access files. + # See https://gitlab.com/gitlab-org/gitlab/issues/191273 + GIT_DEPTH: 1 + before_script: + - *base-before_script + +review-delete-deployment: + extends: + - .review-stop-base + - .review:rules:review-delete-deployment + stage: prepare + script: + - delete_release + +review-stop: + extends: + - .review-stop-base + - .review:rules:review-stop + stage: post-qa + script: + - delete_k8s_release_namespace diff --git a/.gitlab/ci/review-apps/qa.gitlab-ci.yml b/.gitlab/ci/review-apps/qa.gitlab-ci.yml new file mode 100644 index 00000000000..04eddb80ad5 --- /dev/null +++ b/.gitlab/ci/review-apps/qa.gitlab-ci.yml @@ -0,0 +1,126 @@ +.review-qa-base: + extends: + - .use-docker-in-docker + image: + name: ${QA_IMAGE} + entrypoint: [""] + stage: qa + needs: ["review-deploy"] + variables: + QA_DEBUG: "true" + QA_CAN_TEST_GIT_PROTOCOL_V2: "false" + QA_GENERATE_ALLURE_REPORT: "true" + GITLAB_USERNAME: "root" + GITLAB_PASSWORD: "${REVIEW_APPS_ROOT_PASSWORD}" + GITLAB_ADMIN_USERNAME: "root" + GITLAB_ADMIN_PASSWORD: "${REVIEW_APPS_ROOT_PASSWORD}" + GITHUB_ACCESS_TOKEN: "${REVIEW_APPS_QA_GITHUB_ACCESS_TOKEN}" + EE_LICENSE: "${REVIEW_APPS_EE_LICENSE}" + SIGNUP_DISABLED: "true" + before_script: + # Use $CI_MERGE_REQUEST_SOURCE_BRANCH_SHA so that GitLab image built in omnibus-gitlab-mirror and QA image are in sync. + - if [ -n "$CI_MERGE_REQUEST_SOURCE_BRANCH_SHA" ]; then + git checkout -f ${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}; + fi + - export CI_ENVIRONMENT_URL="$(cat environment_url.txt)" + - echo "${CI_ENVIRONMENT_URL}" + - cd qa + artifacts: + paths: + - qa/tmp + expire_in: 7 days + when: always + +.allure-report-base: + image: + name: ${GITLAB_DEPENDENCY_PROXY}andrcuns/allure-report-publisher:0.3.4 + entrypoint: [""] + stage: post-qa + variables: + GIT_STRATEGY: none + STORAGE_CREDENTIALS: $QA_ALLURE_REPORT_GCS_CREDENTIALS + GITLAB_AUTH_TOKEN: $GITLAB_QA_MR_ALLURE_REPORT_TOKEN + allow_failure: true + script: + - | + allure-report-publisher upload gcs \ + --results-glob="qa/tmp/allure-results/*" \ + --bucket="gitlab-qa-allure-reports" \ + --prefix="$ALLURE_REPORT_PATH_PREFIX/$CI_COMMIT_REF_SLUG" \ + --update-pr="comment" \ + --copy-latest \ + --ignore-missing-results \ + --color + +review-qa-smoke: + extends: + - .review-qa-base + - .review:rules:review-qa-smoke + retry: 1 # This is confusing but this means "2 runs at max". + variables: + QA_RUN_TYPE: review-qa-smoke + script: + - bin/test Test::Instance::Smoke "${CI_ENVIRONMENT_URL}" + +review-qa-all: + extends: + - .review-qa-base + - .review:rules:review-qa-all + variables: + QA_RUN_TYPE: review-qa-all + parallel: 5 + script: + - export KNAPSACK_REPORT_PATH=knapsack/master_report.json + - export KNAPSACK_TEST_FILE_PATTERN=qa/specs/features/**/*_spec.rb + - | + bin/test Test::Instance::All "${CI_ENVIRONMENT_URL}" \ + -- \ + --color --format documentation \ + --format RspecJunitFormatter --out tmp/rspec.xml + artifacts: + reports: + junit: qa/tmp/rspec.xml + +review-performance: + extends: + - .default-retry + - .review:rules:review-performance + image: + name: sitespeedio/sitespeed.io + entrypoint: [""] + stage: qa + needs: ["review-deploy"] + before_script: + - export CI_ENVIRONMENT_URL="$(cat environment_url.txt)" + - echo "${CI_ENVIRONMENT_URL}" + - mkdir -p gitlab-exporter + - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js + - mkdir -p sitespeed-results + script: + - /start.sh --plugins.add ./gitlab-exporter --outputFolder sitespeed-results "${CI_ENVIRONMENT_URL}" + after_script: + - mv sitespeed-results/data/performance.json performance.json + artifacts: + paths: + - sitespeed-results/ + reports: + performance: performance.json + expire_in: 31d + +allure-report-qa-smoke: + extends: + - .allure-report-base + - .review:rules:review-qa-smoke-report + needs: ["review-qa-smoke"] + variables: + ALLURE_REPORT_PATH_PREFIX: gitlab-review-smoke + ALLURE_JOB_NAME: review-qa-smoke + +allure-report-qa-all: + extends: + - .allure-report-base + - .review:rules:review-qa-all-report + needs: ["review-qa-all"] + variables: + ALLURE_REPORT_PATH_PREFIX: gitlab-review-all + ALLURE_JOB_NAME: review-qa-all diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 2fa8c2519f7..c89fcff6451 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -16,229 +16,19 @@ review-cleanup: - ruby -rrubygems scripts/review_apps/automated_cleanup.rb - gcp_cleanup -.base-before_script: &base-before_script - - source ./scripts/utils.sh - - source ./scripts/review_apps/review-apps.sh - - install_api_client_dependencies_with_apk - -review-build-cng: +start-review-app-pipeline: extends: - - .default-retry - - .review:rules:review-build-cng - image: ${GITLAB_DEPENDENCY_PROXY}ruby:2.7-alpine3.13 - stage: review-prepare + - .review:rules:review-app-pipeline + stage: review needs: - - job: compile-production-assets + - job: build-assets-image artifacts: false - variables: - CNG_PROJECT_ACCESS_TOKEN: "${CNG_MIRROR_PROJECT_ACCESS_TOKEN}" # "Multi-pipeline (from 'gitlab-org/gitlab' 'review-build-cng' job)" at https://gitlab.com/gitlab-org/build/CNG-mirror/-/settings/access_tokens - CNG_PROJECT_PATH: "gitlab-org/build/CNG-mirror" - before_script: - - source ./scripts/utils.sh - - install_gitlab_gem - script: - - ./scripts/trigger-build cng - -.review-workflow-base: - extends: - - .default-retry - image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-helm3.5-kubectl1.17 - variables: - HOST_SUFFIX: "${CI_ENVIRONMENT_SLUG}" - DOMAIN: "-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN}" - GITLAB_HELM_CHART_REF: "v5.2.1" - environment: - name: review/${CI_COMMIT_REF_SLUG}${FREQUENCY} - url: https://gitlab-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN} - on_stop: review-stop - auto_stop_in: 48 hours - -review-deploy: - extends: - - .review-workflow-base - - .review:rules:review-deploy - stage: review - needs: ["review-build-cng"] - resource_group: "review/${CI_COMMIT_REF_NAME}" - before_script: - - export GITLAB_SHELL_VERSION=$(<GITLAB_SHELL_VERSION) - - export GITALY_VERSION=$(<GITALY_SERVER_VERSION) - - export GITLAB_WORKHORSE_VERSION=$(<GITLAB_WORKHORSE_VERSION) - - echo "${CI_ENVIRONMENT_URL}" > environment_url.txt - - *base-before_script - script: - - check_kube_domain - - download_chart - - date - - deploy || (display_deployment_debug && exit 1) - - verify_deploy || exit 1 - - disable_sign_ups || (delete_release && exit 1) - after_script: - # Run seed-dast-test-data.sh only when DAST_RUN is set to true. This is to pupulate review app with data for DAST scan. - # Set DAST_RUN to true when jobs are manually scheduled. - - if [ "$DAST_RUN" == "true" ]; then source scripts/review_apps/seed-dast-test-data.sh; TRACE=1 trigger_proj_user_creation; fi - artifacts: - paths: - - environment_url.txt - - curl_output.txt - expire_in: 7 days - when: always - -.review-stop-base: - extends: .review-workflow-base - environment: - action: stop - dependencies: [] - variables: - # We're cloning the repo instead of downloading the script for now - # because some repos are private and CI_JOB_TOKEN cannot access files. - # See https://gitlab.com/gitlab-org/gitlab/issues/191273 - GIT_DEPTH: 1 - before_script: - - *base-before_script - -review-delete-deployment: - extends: - - .review-stop-base - - .review:rules:review-delete-deployment - stage: prepare - script: - - delete_release - -review-stop: - extends: - - .review-stop-base - - .review:rules:review-stop - stage: post-qa - script: - - delete_k8s_release_namespace - -.review-qa-base: - extends: - - .use-docker-in-docker - image: - name: ${QA_IMAGE} - entrypoint: [""] - stage: qa - needs: ["build-qa-image", "review-deploy"] - variables: - QA_DEBUG: "true" - QA_CAN_TEST_GIT_PROTOCOL_V2: "false" - QA_GENERATE_ALLURE_REPORT: "true" - GITLAB_USERNAME: "root" - GITLAB_PASSWORD: "${REVIEW_APPS_ROOT_PASSWORD}" - GITLAB_ADMIN_USERNAME: "root" - GITLAB_ADMIN_PASSWORD: "${REVIEW_APPS_ROOT_PASSWORD}" - GITHUB_ACCESS_TOKEN: "${REVIEW_APPS_QA_GITHUB_ACCESS_TOKEN}" - EE_LICENSE: "${REVIEW_APPS_EE_LICENSE}" - SIGNUP_DISABLED: "true" - before_script: - # Use $CI_MERGE_REQUEST_SOURCE_BRANCH_SHA so that GitLab image built in omnibus-gitlab-mirror and QA image are in sync. - - if [ -n "$CI_MERGE_REQUEST_SOURCE_BRANCH_SHA" ]; then - git checkout -f ${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}; - fi - - export CI_ENVIRONMENT_URL="$(cat environment_url.txt)" - - echo "${CI_ENVIRONMENT_URL}" - - cd qa - artifacts: - paths: - - qa/tmp - expire_in: 7 days - when: always - -.allure-report-base: - image: - name: ${GITLAB_DEPENDENCY_PROXY}andrcuns/allure-report-publisher:0.3.4 - entrypoint: [""] - stage: post-qa - variables: - GIT_STRATEGY: none - STORAGE_CREDENTIALS: $QA_ALLURE_REPORT_GCS_CREDENTIALS - GITLAB_AUTH_TOKEN: $GITLAB_QA_MR_ALLURE_REPORT_TOKEN - allow_failure: true - script: - - | - allure-report-publisher upload gcs \ - --results-glob="qa/tmp/allure-results/*" \ - --bucket="gitlab-qa-allure-reports" \ - --prefix="$ALLURE_REPORT_PATH_PREFIX/$CI_COMMIT_REF_SLUG" \ - --update-pr="comment" \ - --copy-latest \ - --ignore-missing-results \ - --color - -review-qa-smoke: - extends: - - .review-qa-base - - .review:rules:review-qa-smoke - retry: 1 # This is confusing but this means "2 runs at max". - variables: - QA_RUN_TYPE: review-qa-smoke - script: - - bin/test Test::Instance::Smoke "${CI_ENVIRONMENT_URL}" - -review-qa-all: - extends: - - .review-qa-base - - .review:rules:review-qa-all - parallel: 5 - variables: - QA_RUN_TYPE: review-qa-all - script: - - export KNAPSACK_REPORT_PATH=knapsack/master_report.json - - export KNAPSACK_TEST_FILE_PATTERN=qa/specs/features/**/*_spec.rb - - | - bin/test Test::Instance::All "${CI_ENVIRONMENT_URL}" \ - -- \ - --color --format documentation \ - --format RspecJunitFormatter --out tmp/rspec.xml - artifacts: - reports: - junit: qa/tmp/rspec.xml - -review-performance: - extends: - - .default-retry - - .review:rules:review-performance - image: - name: sitespeedio/sitespeed.io - entrypoint: [""] - stage: qa - needs: ["review-deploy"] - before_script: - - export CI_ENVIRONMENT_URL="$(cat environment_url.txt)" - - echo "${CI_ENVIRONMENT_URL}" - - mkdir -p gitlab-exporter - - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js - - mkdir -p sitespeed-results - script: - - /start.sh --plugins.add ./gitlab-exporter --outputFolder sitespeed-results "${CI_ENVIRONMENT_URL}" - after_script: - - mv sitespeed-results/data/performance.json performance.json - artifacts: - paths: - - sitespeed-results/ - reports: - performance: performance.json - expire_in: 31d - -allure-report-qa-smoke: - extends: - - .allure-report-base - - .review:rules:review-qa-smoke-report - needs: ["review-qa-smoke"] - variables: - ALLURE_REPORT_PATH_PREFIX: gitlab-review-smoke - ALLURE_JOB_NAME: review-qa-smoke - -allure-report-qa-all: - extends: - - .allure-report-base - - .review:rules:review-qa-all-report - needs: ["review-qa-all"] - variables: - ALLURE_REPORT_PATH_PREFIX: gitlab-review-all - ALLURE_JOB_NAME: review-qa-all + - job: build-qa-image + artifacts: false + trigger: + include: + - local: .gitlab/ci/review-apps/main.gitlab-ci.yml + strategy: depend danger-review: extends: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 5e04db6701c..8d27e14f86d 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -10,6 +10,9 @@ .if-not-foss: &if-not-foss if: '$CI_PROJECT_NAME != "gitlab-foss" && $CI_PROJECT_NAME != "gitlab-ce" && $CI_PROJECT_NAME != "gitlabhq"' +.if-jh: &if-jh + if: '$CI_PROJECT_PATH == "gitlab-jh/gitlab"' + .if-default-refs: &if-default-refs if: '$CI_COMMIT_REF_NAME == $CI_DEFAULT_BRANCH || $CI_COMMIT_REF_NAME =~ /^[\d-]+-stable(-ee)?$/ || $CI_COMMIT_REF_NAME =~ /^\d+-\d+-auto-deploy-\d+$/ || $CI_COMMIT_REF_NAME =~ /^security\// || $CI_MERGE_REQUEST_IID || $CI_COMMIT_TAG || $FORCE_GITLAB_CI' @@ -116,6 +119,7 @@ - ".gitlab/ci/frontend.gitlab-ci.yml" - ".gitlab/ci/build-images.gitlab-ci.yml" - ".gitlab/ci/review.gitlab-ci.yml" + - ".gitlab/ci/review-apps/**/*" - "scripts/review_apps/base-config.yaml" - "scripts/review_apps/review-apps.sh" - "scripts/trigger-build" @@ -503,6 +507,8 @@ rules: - <<: *if-not-ee when: never + - <<: *if-jh + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - changes: *code-backstage-qa-patterns @@ -517,6 +523,8 @@ rules: - <<: *if-not-ee when: never + - <<: *if-jh + when: never - <<: *if-security-merge-request changes: *code-backstage-patterns - <<: *if-merge-request-labels-as-if-foss @@ -530,6 +538,8 @@ rules: - <<: *if-not-ee when: never + - <<: *if-jh + when: never - <<: *if-security-merge-request changes: *code-backstage-patterns - <<: *if-merge-request-labels-as-if-jh @@ -579,6 +589,8 @@ rules: - <<: *if-not-ee when: never + - <<: *if-jh + when: never # We already have `static-analysis as-if-foss` which already runs `lint:eslint:all` if the `pipeline:run-as-if-foss` label is set. - <<: *if-merge-request-labels-as-if-foss when: never @@ -648,6 +660,8 @@ rules: - <<: *if-not-ee when: never + - <<: *if-jh + when: never - <<: *if-security-merge-request changes: *code-qa-patterns - <<: *if-merge-request-labels-as-if-foss @@ -1378,7 +1392,7 @@ ################ # Review rules # ################ -.review:rules:review-build-cng: +.review:rules:review-app-pipeline: rules: - <<: *if-not-ee when: never @@ -1395,6 +1409,22 @@ allow_failure: true - <<: *if-dot-com-gitlab-org-schedule +.review:rules:review-build-cng: + rules: + - <<: *if-not-ee + when: never + - <<: *if-dot-com-gitlab-org-merge-request + changes: *ci-review-patterns + - <<: *if-dot-com-gitlab-org-merge-request + changes: *frontend-patterns + - <<: *if-dot-com-gitlab-org-merge-request + changes: *code-patterns + allow_failure: true + - <<: *if-dot-com-gitlab-org-merge-request + changes: *qa-patterns + allow_failure: true + - <<: *if-dot-com-gitlab-org-schedule + .review:rules:review-deploy: rules: - <<: *if-not-ee @@ -1597,6 +1627,8 @@ rules: - <<: *if-not-ee when: never + - <<: *if-jh + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - changes: *code-backstage-qa-patterns diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue index 4324c64ab3b..ba567023946 100644 --- a/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue +++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue @@ -1,5 +1,4 @@ <script> -import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import CommitSection from './components/commit/commit_section.vue'; import PipelineEditorDrawer from './components/drawer/pipeline_editor_drawer.vue'; import PipelineEditorFileNav from './components/file_nav/pipeline_editor_file_nav.vue'; @@ -15,7 +14,6 @@ export default { PipelineEditorHeader, PipelineEditorTabs, }, - mixins: [glFeatureFlagMixin()], props: { ciConfigData: { type: Object, @@ -44,9 +42,6 @@ export default { showCommitForm() { return TABS_WITH_COMMIT_FORM.includes(this.currentTab); }, - showPipelineDrawer() { - return this.glFeatures.pipelineEditorDrawer; - }, }, methods: { setCurrentTab(tabName) { @@ -77,6 +72,6 @@ export default { :commit-sha="commitSha" v-on="$listeners" /> - <pipeline-editor-drawer v-if="showPipelineDrawer" /> + <pipeline-editor-drawer /> </div> </template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue index fb883af330e..023367a794e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue @@ -30,6 +30,7 @@ export default { <template> <div> <gl-dropdown + v-if="tertiaryButtons" :text="dropdownLabel" icon="ellipsis_v" no-caret @@ -38,6 +39,7 @@ export default { lazy text-sr-only size="small" + toggle-class="gl-p-2!" class="gl-display-block gl-md-display-none!" > <gl-dropdown-item diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue index ce5861dc238..298f7c7ad8c 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue @@ -138,18 +138,21 @@ export default { <template> <section class="media-section" data-testid="widget-extension"> <div class="media gl-p-5"> - <status-icon :name="widgetLabel" :is-loading="isLoadingSummary" :icon-name="statusIconName" /> - <div - class="media-body gl-display-flex gl-align-self-center gl-align-items-center gl-flex-direction-row!" - > + <status-icon + :name="$options.label || $options.name" + :is-loading="isLoadingSummary" + :icon-name="statusIconName" + /> + <div class="media-body gl-display-flex gl-flex-direction-row!"> <div class="gl-flex-grow-1"> <template v-if="isLoadingSummary">{{ widgetLoadingText }}</template> <div v-else v-safe-html="summary(collapsedData)"></div> </div> - <actions :widget="widgetLabel" :tertiary-buttons="tertiaryActionsButtons" /> - <div - class="gl-float-right gl-align-self-center gl-border-l-1 gl-border-l-solid gl-border-gray-100 gl-ml-3 gl-pl-3" - > + <actions + :widget="$options.label || $options.name" + :tertiary-buttons="tertiaryActionsButtons" + /> + <div class="gl-border-l-1 gl-border-l-solid gl-border-gray-100 gl-ml-3 gl-pl-3 gl-h-6"> <gl-button v-if="isCollapsible" v-gl-tooltip diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/status_icon.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/status_icon.vue index 12e3c483f98..01d8de132e7 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/status_icon.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/status_icon.vue @@ -47,7 +47,7 @@ export default { { 'mr-widget-extension-icon': !isLoading && size === 16 }, { 'gl-p-2': isLoading || size === 16 }, ]" - class="align-self-center gl-rounded-full gl-mr-3 gl-relative" + class="gl-rounded-full gl-mr-3 gl-relative gl-p-2" > <gl-loading-icon v-if="isLoading" size="md" inline class="gl-display-block" /> <gl-icon diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js b/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js index c1d3d6b2aa0..349e9d29355 100644 --- a/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/issues.js @@ -19,7 +19,7 @@ export default { // Small summary text to be displayed in the collapsed state // Receives the collapsed data as an argument summary(count) { - return 'Summary text'; + return 'Summary text<br/>Second line'; }, // Status icon to be used next to the summary text // Receives the collapsed data as an argument diff --git a/app/controllers/groups/dependency_proxy/application_controller.rb b/app/controllers/groups/dependency_proxy/application_controller.rb index fd9db41f748..18a6ff93e15 100644 --- a/app/controllers/groups/dependency_proxy/application_controller.rb +++ b/app/controllers/groups/dependency_proxy/application_controller.rb @@ -21,8 +21,14 @@ module Groups authenticate_with_http_token do |token, _| @authentication_result = EMPTY_AUTH_RESULT - found_user = user_from_token(token) - sign_in(found_user) if found_user.is_a?(User) + user_or_deploy_token = ::DependencyProxy::AuthTokenService.user_or_deploy_token_from_jwt(token) + + if user_or_deploy_token.is_a?(User) + @authentication_result = Gitlab::Auth::Result.new(user_or_deploy_token, nil, :user, []) + sign_in(user_or_deploy_token) + elsif user_or_deploy_token.is_a?(DeployToken) + @authentication_result = Gitlab::Auth::Result.new(user_or_deploy_token, nil, :deploy_token, []) + end end request_bearer_token! unless authenticated_user @@ -39,28 +45,6 @@ module Groups response.headers['WWW-Authenticate'] = ::DependencyProxy::Registry.authenticate_header render plain: '', status: :unauthorized end - - def user_from_token(token) - token_payload = ::DependencyProxy::AuthTokenService.decoded_token_payload(token) - - if token_payload['user_id'] - token_user = User.find(token_payload['user_id']) - return unless token_user - - @authentication_result = Gitlab::Auth::Result.new(token_user, nil, :user, []) - return token_user - elsif token_payload['deploy_token'] - deploy_token = DeployToken.active.find_by_token(token_payload['deploy_token']) - return unless deploy_token - - @authentication_result = Gitlab::Auth::Result.new(deploy_token, nil, :deploy_token, []) - return deploy_token - end - - nil - rescue JWT::DecodeError, JWT::ExpiredSignature, JWT::ImmatureSignature - nil - end end end end diff --git a/app/controllers/projects/ci/pipeline_editor_controller.rb b/app/controllers/projects/ci/pipeline_editor_controller.rb index e925bbea5e9..22cd247644d 100644 --- a/app/controllers/projects/ci/pipeline_editor_controller.rb +++ b/app/controllers/projects/ci/pipeline_editor_controller.rb @@ -3,7 +3,6 @@ class Projects::Ci::PipelineEditorController < Projects::ApplicationController before_action :check_can_collaborate! before_action do - push_frontend_feature_flag(:pipeline_editor_drawer, @project, default_enabled: :yaml) push_frontend_feature_flag(:pipeline_editor_mini_graph, @project, default_enabled: :yaml) push_frontend_feature_flag(:schema_linting, @project, default_enabled: :yaml) end diff --git a/app/models/operations/feature_flag.rb b/app/models/operations/feature_flag.rb index 28e8cf52d83..7db396bcad5 100644 --- a/app/models/operations/feature_flag.rb +++ b/app/models/operations/feature_flag.rb @@ -98,13 +98,6 @@ module Operations Ability.issues_readable_by_user(issues, current_user) end - def execute_hooks(current_user) - run_after_commit do - feature_flag_data = Gitlab::DataBuilder::FeatureFlag.build(self, current_user) - project.execute_hooks(feature_flag_data, :feature_flag_hooks) - end - end - def hook_attrs { id: id, diff --git a/app/services/dependency_proxy/auth_token_service.rb b/app/services/dependency_proxy/auth_token_service.rb index 16279ed12b0..c6c9eb534bb 100644 --- a/app/services/dependency_proxy/auth_token_service.rb +++ b/app/services/dependency_proxy/auth_token_service.rb @@ -12,10 +12,16 @@ module DependencyProxy JSONWebToken::HMACToken.decode(token, ::Auth::DependencyProxyAuthenticationService.secret).first end - class << self - def decoded_token_payload(token) - self.new(token).execute + def self.user_or_deploy_token_from_jwt(raw_jwt) + token_payload = self.new(raw_jwt).execute + + if token_payload['user_id'] + User.find(token_payload['user_id']) + elsif token_payload['deploy_token'] + DeployToken.active.find_by_token(token_payload['deploy_token']) end + rescue JWT::DecodeError, JWT::ExpiredSignature, JWT::ImmatureSignature + nil end end end diff --git a/app/services/feature_flags/base_service.rb b/app/services/feature_flags/base_service.rb index 9ae9ab4de63..ca0b6b89199 100644 --- a/app/services/feature_flags/base_service.rb +++ b/app/services/feature_flags/base_service.rb @@ -7,6 +7,8 @@ module FeatureFlags AUDITABLE_ATTRIBUTES = %w(name description active).freeze def success(**args) + audit_event = args.fetch(:audit_event) { audit_event(args[:feature_flag]) } + save_audit_event(audit_event) sync_to_jira(args[:feature_flag]) super end @@ -66,5 +68,11 @@ module FeatureFlags feature_flag_by_name.scopes.find_by_environment_scope(params[:environment_scope]) end end + + private + + def audit_message(feature_flag) + raise NotImplementedError, "This method should be overriden by subclasses" + end end end diff --git a/app/services/feature_flags/create_service.rb b/app/services/feature_flags/create_service.rb index 65f8f8e33f6..ebbe71f39c7 100644 --- a/app/services/feature_flags/create_service.rb +++ b/app/services/feature_flags/create_service.rb @@ -10,8 +10,6 @@ module FeatureFlags feature_flag = project.operations_feature_flags.new(params) if feature_flag.save - save_audit_event(audit_event(feature_flag)) - success(feature_flag: feature_flag) else error(feature_flag.errors.full_messages, 400) diff --git a/app/services/feature_flags/destroy_service.rb b/app/services/feature_flags/destroy_service.rb index 986fe004db6..817a80940c0 100644 --- a/app/services/feature_flags/destroy_service.rb +++ b/app/services/feature_flags/destroy_service.rb @@ -13,8 +13,6 @@ module FeatureFlags ApplicationRecord.transaction do if feature_flag.destroy - save_audit_event(audit_event(feature_flag)) - success(feature_flag: feature_flag) else error(feature_flag.errors.full_messages) diff --git a/app/services/feature_flags/hook_service.rb b/app/services/feature_flags/hook_service.rb new file mode 100644 index 00000000000..6f77a70bd09 --- /dev/null +++ b/app/services/feature_flags/hook_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module FeatureFlags + class HookService + HOOK_NAME = :feature_flag_hooks + + def initialize(feature_flag, current_user) + @feature_flag = feature_flag + @current_user = current_user + end + + def execute + project.execute_hooks(hook_data, HOOK_NAME) + end + + private + + attr_reader :feature_flag, :current_user + + def project + @project ||= feature_flag.project + end + + def hook_data + Gitlab::DataBuilder::FeatureFlag.build(feature_flag, current_user) + end + end +end diff --git a/app/services/feature_flags/update_service.rb b/app/services/feature_flags/update_service.rb index ccfd1b57d44..bcfd2c15189 100644 --- a/app/services/feature_flags/update_service.rb +++ b/app/services/feature_flags/update_service.rb @@ -7,6 +7,11 @@ module FeatureFlags 'parameters' => 'parameters' }.freeze + def success(**args) + execute_hooks_after_commit(args[:feature_flag]) + super + end + def execute(feature_flag) return error('Access Denied', 403) unless can_update?(feature_flag) return error('Not Found', 404) unless valid_user_list_ids?(feature_flag, user_list_ids(params)) @@ -20,16 +25,11 @@ module FeatureFlags end end + # We generate the audit event before the feature flag is saved as #changed_strategies_messages depends on the strategies' states before save audit_event = audit_event(feature_flag) - if feature_flag.active_changed? - feature_flag.execute_hooks(current_user) - end - if feature_flag.save - save_audit_event(audit_event) - - success(feature_flag: feature_flag) + success(feature_flag: feature_flag, audit_event: audit_event) else error(feature_flag.errors.full_messages, :bad_request) end @@ -38,6 +38,16 @@ module FeatureFlags private + def execute_hooks_after_commit(feature_flag) + return unless feature_flag.active_previously_changed? + + # The `current_user` method (defined in `BaseService`) is not available within the `run_after_commit` block + user = current_user + feature_flag.run_after_commit do + HookService.new(feature_flag, user).execute + end + end + def audit_message(feature_flag) changes = changed_attributes_messages(feature_flag) changes += changed_strategies_messages(feature_flag) diff --git a/app/views/groups/settings/_two_factor_auth.html.haml b/app/views/groups/settings/_two_factor_auth.html.haml index f7fed8c01d0..f86bcb24e63 100644 --- a/app/views/groups/settings/_two_factor_auth.html.haml +++ b/app/views/groups/settings/_two_factor_auth.html.haml @@ -11,8 +11,8 @@ _('Require all users in this group to set up two-factor authentication'), checkbox_options: { data: { qa_selector: 'require_2fa_checkbox' } } .form-group - = f.label :two_factor_grace_period, _('Time before enforced'), class: 'label-bold' - = f.text_field :two_factor_grace_period, class: 'form-control form-control-sm w-auto' + = f.label :two_factor_grace_period, _('Time before enforced') + = f.text_field :two_factor_grace_period, class: 'form-control form-control-sm w-auto gl-form-input gl-mb-3' .form-text.text-muted= _('Time (in hours) that users are allowed to skip forced configuration of two-factor authentication.') - unless group.has_parent? .form-group diff --git a/config/feature_flags/development/pipeline_editor_drawer.yml b/config/feature_flags/development/pipeline_editor_drawer.yml deleted file mode 100644 index df73c4be01e..00000000000 --- a/config/feature_flags/development/pipeline_editor_drawer.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: pipeline_editor_drawer -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60856 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/329806 -milestone: '13.12' -type: development -group: group::pipeline authoring -default_enabled: true diff --git a/doc/development/testing_guide/img/review-app-parent-pipeline.png b/doc/development/testing_guide/img/review-app-parent-pipeline.png Binary files differnew file mode 100644 index 00000000000..5686d5f6ebe --- /dev/null +++ b/doc/development/testing_guide/img/review-app-parent-pipeline.png diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 4c5ece57f69..928fcd6b648 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -6,12 +6,11 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Review Apps -Review Apps are automatically deployed by [the -pipeline](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/6665). +Review Apps are deployed using the `start-review-app-pipeline` job. This job triggers a child pipeline containing a series of jobs to perform the various tasks needed to deploy a Review App. -## When are Review Apps automatically deployed? + -A Review App is automatically deployed for: +For any of the following scenarios, the `start-review-app-pipeline` job would be automatically started: - for merge requests with CI config changes - for merge requests with frontend changes diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb index 08214bbd449..1a9259a4f0e 100644 --- a/lib/gitlab/auth/request_authenticator.rb +++ b/lib/gitlab/auth/request_authenticator.rb @@ -30,7 +30,8 @@ module Gitlab end def find_sessionless_user(request_format) - find_user_from_web_access_token(request_format, scopes: [:api, :read_api]) || + find_user_from_dependency_proxy_token || + find_user_from_web_access_token(request_format, scopes: [:api, :read_api]) || find_user_from_feed_token(request_format) || find_user_from_static_object_token(request_format) || find_user_from_basic_auth_job || @@ -82,6 +83,28 @@ module Gitlab basic_auth_personal_access_token: api_request? || git_request? } end + + def find_user_from_dependency_proxy_token + return unless dependency_proxy_request? + + token, _ = ActionController::HttpAuthentication::Token.token_and_options(current_request) + + return unless token + + user_or_deploy_token = ::DependencyProxy::AuthTokenService.user_or_deploy_token_from_jwt(token) + + # Do not return deploy tokens + # See https://gitlab.com/gitlab-org/gitlab/-/issues/342481 + return unless user_or_deploy_token.is_a?(::User) + + user_or_deploy_token + rescue ActiveRecord::RecordNotFound + nil # invalid id used return no user + end + + def dependency_proxy_request? + Gitlab::PathRegex.dependency_proxy_route_regex.match?(current_request.path) + end end end end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index c648f4d1fd0..06a26c4830f 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -262,6 +262,10 @@ module Gitlab @container_image_blob_sha_regex ||= %r{[\w+.-]+:?\w+}.freeze end + def dependency_proxy_route_regex + @dependency_proxy_route_regex ||= %r{\A/v2/#{full_namespace_route_regex}/dependency_proxy/containers/#{container_image_regex}/(manifests|blobs)/#{container_image_blob_sha_regex}\z} + end + private def personal_snippet_path_regex diff --git a/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js b/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js index 7aba336b8e8..335049892ec 100644 --- a/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js +++ b/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js @@ -25,7 +25,6 @@ describe('Pipeline editor home wrapper', () => { }, provide: { glFeatures: { - pipelineEditorDrawer: true, ...glFeatures, }, }, @@ -94,12 +93,4 @@ describe('Pipeline editor home wrapper', () => { expect(findCommitSection().exists()).toBe(true); }); }); - - describe('Pipeline drawer', () => { - it('hides the drawer when the feature flag is off', () => { - createComponent({ glFeatures: { pipelineEditorDrawer: false } }); - - expect(findPipelineEditorDrawer().exists()).toBe(false); - }); - }); }); diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index 2543eb3a5e9..6f3d6187076 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Auth::RequestAuthenticator do + include DependencyProxyHelpers + let(:env) do { 'rack.input' => '', @@ -15,8 +17,8 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do subject { described_class.new(request) } describe '#user' do - let!(:sessionless_user) { build(:user) } - let!(:session_user) { build(:user) } + let_it_be(:sessionless_user) { build(:user) } + let_it_be(:session_user) { build(:user) } it 'returns sessionless user first' do allow_any_instance_of(described_class).to receive(:find_sessionless_user).and_return(sessionless_user) @@ -41,15 +43,25 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do end describe '#find_sessionless_user' do - let!(:access_token_user) { build(:user) } - let!(:feed_token_user) { build(:user) } - let!(:static_object_token_user) { build(:user) } - let!(:job_token_user) { build(:user) } - let!(:lfs_token_user) { build(:user) } - let!(:basic_auth_access_token_user) { build(:user) } - let!(:basic_auth_password_user) { build(:user) } - - it 'returns access_token user first' do + let_it_be(:dependency_proxy_user) { build(:user) } + let_it_be(:access_token_user) { build(:user) } + let_it_be(:feed_token_user) { build(:user) } + let_it_be(:static_object_token_user) { build(:user) } + let_it_be(:job_token_user) { build(:user) } + let_it_be(:lfs_token_user) { build(:user) } + let_it_be(:basic_auth_access_token_user) { build(:user) } + let_it_be(:basic_auth_password_user) { build(:user) } + + it 'returns dependency_proxy user first' do + allow_any_instance_of(described_class).to receive(:find_user_from_dependency_proxy_token) + .and_return(dependency_proxy_user) + + allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token).and_return(access_token_user) + + expect(subject.find_sessionless_user(:api)).to eq dependency_proxy_user + end + + it 'returns access_token user if no dependency_proxy user found' do allow_any_instance_of(described_class).to receive(:find_user_from_web_access_token) .with(anything, scopes: [:api, :read_api]) .and_return(access_token_user) @@ -154,6 +166,75 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do expect(subject.find_sessionless_user(:api)).to be_blank end + + context 'dependency proxy' do + let_it_be(:dependency_proxy_user) { create(:user) } + + let(:token) { build_jwt(dependency_proxy_user).encoded } + let(:authenticator) { described_class.new(request) } + + subject { authenticator.find_sessionless_user(:api) } + + before do + env['SCRIPT_NAME'] = accessed_path + env['HTTP_AUTHORIZATION'] = "Bearer #{token}" + end + + shared_examples 'identifying dependency proxy urls properly with' do |user_type| + context 'with pulling a manifest' do + let(:accessed_path) { '/v2/group1/dependency_proxy/containers/alpine/manifests/latest' } + + it { is_expected.to eq(dependency_proxy_user) } if user_type == :user + it { is_expected.to eq(nil) } if user_type == :no_user + end + + context 'with pulling a blob' do + let(:accessed_path) { '/v2/group1/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e' } + + it { is_expected.to eq(dependency_proxy_user) } if user_type == :user + it { is_expected.to eq(nil) } if user_type == :no_user + end + + context 'with any other path' do + let(:accessed_path) { '/foo/bar' } + + it { is_expected.to eq(nil) } + end + end + + context 'with a user' do + it_behaves_like 'identifying dependency proxy urls properly with', :user + + context 'with an invalid id' do + let(:token) { build_jwt { |jwt| jwt['user_id'] = 'this_is_not_a_user' } } + + it_behaves_like 'identifying dependency proxy urls properly with', :no_user + end + end + + context 'with a deploy token' do + let_it_be(:dependency_proxy_user) { create(:deploy_token) } + + it_behaves_like 'identifying dependency proxy urls properly with', :no_user + end + + context 'with no jwt token' do + let(:token) { nil } + + it_behaves_like 'identifying dependency proxy urls properly with', :no_user + end + + context 'with an expired jwt token' do + let(:token) { build_jwt(dependency_proxy_user).encoded } + let(:accessed_path) { '/v2/group1/dependency_proxy/containers/alpine/manifests/latest' } + + it 'returns nil' do + travel_to(Time.zone.now + Auth::DependencyProxyAuthenticationService.token_expire_at + 1.minute) do + expect(subject).to eq(nil) + end + end + end + end end describe '#find_personal_access_token_from_http_basic_auth' do @@ -201,8 +282,8 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do end describe '#find_user_from_job_token' do - let!(:user) { build(:user) } - let!(:job) { build(:ci_build, user: user, status: :running) } + let_it_be(:user) { build(:user) } + let_it_be(:job) { build(:ci_build, user: user, status: :running) } before do env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = 'token' @@ -239,7 +320,7 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do end describe '#runner' do - let!(:runner) { build(:ci_runner) } + let_it_be(:runner) { build(:ci_runner) } it 'returns the runner using #find_runner_from_token' do expect_any_instance_of(described_class) diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index aa13660deb4..2f38ed58727 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -561,4 +561,25 @@ RSpec.describe Gitlab::PathRegex do expect(subject.match('sha256:asdf1234%2f')[0]).to eq('sha256:asdf1234') end end + + describe '.dependency_proxy_route_regex' do + subject { described_class.dependency_proxy_route_regex } + + it { is_expected.to match('/v2/group1/dependency_proxy/containers/alpine/manifests/latest') } + it { is_expected.to match('/v2/group1/dependency_proxy/containers/alpine/blobs/sha256:14119a10abf4669e8cdbdff324a9f9605d99697215a0d21c360fe8dfa8471bab') } + + it { is_expected.not_to match('') } + it { is_expected.not_to match('/v3/group1/dependency_proxy/containers/alpine/manifests/latest') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/container/alpine/manifests/latest') } + it { is_expected.not_to match('/v2/group1/dependency_prox/containers/alpine/manifests/latest') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/containers/alpine/manifest/latest') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/containers/alpine/manifest/la%2Ftest') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/containers/alpine/manifest/latest/../one') } + it { is_expected.not_to match('/v3/group1/dependency_proxy/containers/alpine/blobs/sha256:14119a10abf4669e8cdbdff324a9f9605d99697215a0d21c360fe8dfa8471bab') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/container/alpine/blobs/sha256:14119a10abf4669e8cdbdff324a9f9605d99697215a0d21c360fe8dfa8471bab') } + it { is_expected.not_to match('/v2/group1/dependency_prox/containers/alpine/blobs/sha256:14119a10abf4669e8cdbdff324a9f9605d99697215a0d21c360fe8dfa8471bab') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/containers/alpine/blob/sha256:14119a10abf4669e8cdbdff324a9f9605d99697215a0d21c360fe8dfa8471bab') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/containers/alpine/blob/sha256:F14119a10abf4669e8cdbdff324a9f9605d99697215a0d21c360fe8dfa8471bab/../latest') } + it { is_expected.not_to match('/v2/group1/dependency_proxy/containers/alpine/blob/sha256:F14119a10abf4669e8cdbdff324a9f9605d99697215a0d21c360fe8dfa8471bab/latest') } + end end diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index aabe5b70ade..e709470b312 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -164,26 +164,4 @@ RSpec.describe Operations::FeatureFlag do expect(subject.hook_attrs).to eq(hook_attrs) end end - - describe "#execute_hooks" do - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project) } - let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) } - - it 'does not execute the hook when feature_flag event is disabled' do - create(:project_hook, project: project, feature_flag_events: false) - expect(WebHookWorker).not_to receive(:perform_async) - - feature_flag.execute_hooks(user) - feature_flag.touch - end - - it 'executes hook when feature_flag event is enabled' do - hook = create(:project_hook, project: project, feature_flag_events: true) - expect(WebHookWorker).to receive(:perform_async).with(hook.id, an_instance_of(Hash), 'feature_flag_hooks') - - feature_flag.execute_hooks(user) - feature_flag.touch - end - end end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 6491c9ab65a..35ce942ed7e 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -483,6 +483,67 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac end end + describe 'dependency proxy' do + include DependencyProxyHelpers + + let_it_be_with_reload(:group) { create(:group) } + let_it_be_with_reload(:other_group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } + + let(:throttle_setting_prefix) { 'throttle_authenticated_web' } + let(:jwt_token) { build_jwt(user) } + let(:other_jwt_token) { build_jwt(other_user) } + let(:request_args) { [path, headers: jwt_token_authorization_headers(jwt_token)] } + let(:other_user_request_args) { [other_path, headers: jwt_token_authorization_headers(other_jwt_token)] } + + before do + group.add_owner(user) + group.create_dependency_proxy_setting!(enabled: true) + other_group.add_owner(other_user) + other_group.create_dependency_proxy_setting!(enabled: true) + + allow(Gitlab.config.dependency_proxy) + .to receive(:enabled).and_return(true) + token_response = { status: :success, token: 'abcd1234' } + allow_next_instance_of(DependencyProxy::RequestTokenService) do |instance| + allow(instance).to receive(:execute).and_return(token_response) + end + end + + context 'getting a manifest' do + let_it_be(:manifest) { create(:dependency_proxy_manifest) } + + let(:path) { "/v2/#{group.path}/dependency_proxy/containers/alpine/manifests/latest" } + let(:other_path) { "/v2/#{other_group.path}/dependency_proxy/containers/alpine/manifests/latest" } + let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } } + + before do + allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance| + allow(instance).to receive(:execute).and_return(pull_response) + end + end + + it_behaves_like 'rate-limited token-authenticated requests' + end + + context 'getting a blob' do + let_it_be(:blob) { create(:dependency_proxy_blob) } + + let(:path) { "/v2/#{group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } + let(:other_path) { "/v2/#{other_group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" } + let(:blob_response) { { status: :success, blob: blob, from_cache: false } } + + before do + allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance| + allow(instance).to receive(:execute).and_return(blob_response) + end + end + + it_behaves_like 'rate-limited token-authenticated requests' + end + end + describe 'authenticated git lfs requests', :api do let_it_be(:project) { create(:project, :internal) } let_it_be(:user) { create(:user) } diff --git a/spec/services/dependency_proxy/auth_token_service_spec.rb b/spec/services/dependency_proxy/auth_token_service_spec.rb index 6214d75dfa0..c686f57c5cb 100644 --- a/spec/services/dependency_proxy/auth_token_service_spec.rb +++ b/spec/services/dependency_proxy/auth_token_service_spec.rb @@ -4,47 +4,72 @@ require 'spec_helper' RSpec.describe DependencyProxy::AuthTokenService do include DependencyProxyHelpers - describe '.decoded_token_payload' do - let_it_be(:user) { create(:user) } - let_it_be(:token) { build_jwt(user) } + let_it_be(:user) { create(:user) } + let_it_be(:deploy_token) { create(:deploy_token) } - subject { described_class.decoded_token_payload(token.encoded) } + describe '.user_or_deploy_token_from_jwt' do + subject { described_class.user_or_deploy_token_from_jwt(token.encoded) } - it 'returns the user' do - result = subject + shared_examples 'handling token errors' do + context 'with a decoding error' do + before do + allow(JWT).to receive(:decode).and_raise(JWT::DecodeError) + end - expect(result['user_id']).to eq(user.id) - expect(result['deploy_token']).to be_nil - end + it { is_expected.to eq(nil) } + end - context 'with a deploy token' do - let_it_be(:deploy_token) { create(:deploy_token) } - let_it_be(:token) { build_jwt(deploy_token) } + context 'with an immature signature error' do + before do + allow(JWT).to receive(:decode).and_raise(JWT::ImmatureSignature) + end - it 'returns the deploy token' do - result = subject + it { is_expected.to eq(nil) } + end - expect(result['deploy_token']).to eq(deploy_token.token) - expect(result['user_id']).to be_nil + context 'with an expired signature error' do + it 'returns nil' do + travel_to(Time.zone.now + Auth::DependencyProxyAuthenticationService.token_expire_at + 1.minute) do + expect(subject).to eq(nil) + end + end end end - it 'raises an error if the token is expired' do - travel_to(Time.zone.now + Auth::DependencyProxyAuthenticationService.token_expire_at + 1.minute) do - expect { subject }.to raise_error(JWT::ExpiredSignature) + context 'with a user' do + let_it_be(:token) { build_jwt(user) } + + it { is_expected.to eq(user) } + + context 'with an invalid user id' do + let_it_be(:token) { build_jwt { |jwt| jwt['user_id'] = 'this_is_not_a_user_id' } } + + it 'raises an not found error' do + expect { subject }.to raise_error(ActiveRecord::RecordNotFound) + end end + + it_behaves_like 'handling token errors' end - it 'raises an error if decoding fails' do - allow(JWT).to receive(:decode).and_raise(JWT::DecodeError) + context 'with a deploy token' do + let_it_be(:token) { build_jwt(deploy_token) } + + it { is_expected.to eq(deploy_token) } + + context 'with an invalid token' do + let_it_be(:token) { build_jwt { |jwt| jwt['deploy_token'] = 'this_is_not_a_token' } } + + it { is_expected.to eq(nil) } + end - expect { subject }.to raise_error(JWT::DecodeError) + it_behaves_like 'handling token errors' end - it 'raises an error if signature is immature' do - allow(JWT).to receive(:decode).and_raise(JWT::ImmatureSignature) + context 'with an empty token payload' do + let_it_be(:token) { build_jwt(nil) } - expect { subject }.to raise_error(JWT::ImmatureSignature) + it { is_expected.to eq(nil) } end end end diff --git a/spec/services/feature_flags/hook_service_spec.rb b/spec/services/feature_flags/hook_service_spec.rb new file mode 100644 index 00000000000..02cdbbd86ac --- /dev/null +++ b/spec/services/feature_flags/hook_service_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FeatureFlags::HookService do + describe '#execute_hooks' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, :repository, namespace: namespace) } + let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) } + let_it_be(:user) { namespace.owner } + + let!(:hook) { create(:project_hook, project: project) } + let(:hook_data) { double } + + subject(:service) { described_class.new(feature_flag, user) } + + describe 'HOOK_NAME' do + specify { expect(described_class::HOOK_NAME).to eq(:feature_flag_hooks) } + end + + before do + allow(Gitlab::DataBuilder::FeatureFlag).to receive(:build).with(feature_flag, user).once.and_return(hook_data) + end + + it 'calls feature_flag.project.execute_hooks' do + expect(feature_flag.project).to receive(:execute_hooks).with(hook_data, described_class::HOOK_NAME) + + service.execute + end + end +end diff --git a/spec/support/helpers/dependency_proxy_helpers.rb b/spec/support/helpers/dependency_proxy_helpers.rb index 9413cb93199..75dc09ec159 100644 --- a/spec/support/helpers/dependency_proxy_helpers.rb +++ b/spec/support/helpers/dependency_proxy_helpers.rb @@ -34,12 +34,20 @@ module DependencyProxyHelpers def build_jwt(user = nil, expire_time: nil) JSONWebToken::HMACToken.new(::Auth::DependencyProxyAuthenticationService.secret).tap do |jwt| - jwt['user_id'] = user.id if user.is_a?(User) - jwt['deploy_token'] = user.token if user.is_a?(DeployToken) - jwt.expire_time = expire_time || jwt.issued_at + 1.minute + if block_given? + yield(jwt) + else + jwt['user_id'] = user.id if user.is_a?(User) + jwt['deploy_token'] = user.token if user.is_a?(DeployToken) + jwt.expire_time = expire_time || jwt.issued_at + 1.minute + end end end + def jwt_token_authorization_headers(jwt) + { 'AUTHORIZATION' => "Bearer #{jwt.encoded}" } + end + private def registry |
