Skip to content
Snippets Groups Projects

Add low level api cookie passing

Merged Zeff Morgan requested to merge qa/zm-api-cookie-update into master

What does this MR do and why?

Adds low level cookie passing functionality in API calls that previously ignored the "gitlab_canary" cookie.

How to set up and validate locally

  1. Execute an end-to-end test that features an API call with the QA_COOKIES ENV variable set.
    CHROME_HEADLESS=false QA_COOKIES=gitlab_canary=true bundle exec bin/qa Test::Instance::All http://localhost:3000 qa/specs/features/browser_ui/3_create/merge_request/create_merge_request_spec.rb
  2. Open Chrome devtools and inspect the Cookie values in the header of the API call(s)
  3. Alternatively, you could install a proxy such as mitmproxy to inspect traffic.
     brew install mitmproxy

These methods can be difficult to track and implement due to the speed of the tests. You may need to insert pauses within the test to more easily capture the network calls. Wireshark is another option that would capture even calls missed through devtools. The other alternative is to run the test locally using pry-byebug and inserting breakpoints/stepping through the test and inspecting the values.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Closes #343654 (closed)

Edited by Zeff Morgan

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Zeff Morgan changed milestone to %14.10

    changed milestone to %14.10

  • 1 Message
    :book: CHANGELOG missing:

    If you want to create a changelog entry for GitLab FOSS, add the Changelog trailer to the commit message you want to add to the changelog.

    If you want to create a changelog entry for GitLab EE, also add the EE: true trailer to your commit message.

    If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    QA Edgars Brālītis (@ebralitis) (UTC+3, 7 hours ahead of @zeffmorgan) Dan Davison (@ddavison) (UTC-4, same timezone as @zeffmorgan)

    To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    Generated by :no_entry_sign: Danger

  • mentioned in issue #343654 (closed)

  • mentioned in issue #341427 (closed)

  • Allure report

    allure-report-publisher generated test report!

    review-qa-smoke: :pencil: test report
    review-qa-reliable: :pencil: test report
    review-qa-all: :pencil: test report
    package-and-qa: :pencil: test report

    review-qa-blocking: :x: test report for 9ac3034a

    expand test summary
    +---------------------------------------------------------------------------+
    |                              suites summary                               |
    +----------------------+--------+--------+---------+-------+-------+--------+
    |                      | passed | failed | skipped | flaky | total | result |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Plan                 | 47     | 1      | 1       | 31    | 49    | ❌     |
    | Secure               | 6      | 0      | 0       | 2     | 6     | ❗     |
    | Verify               | 12     | 0      | 1       | 9     | 13    | ❗     |
    | Create               | 21     | 2      | 2       | 15    | 25    | ❌     |
    | Manage               | 36     | 1      | 2       | 17    | 39    | ❌     |
    | Protect              | 2      | 0      | 0       | 2     | 2     | ❗     |
    | Configure            | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Package              | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Version sanity check | 0      | 0      | 1       | 0     | 1     | ➖     |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Total                | 124    | 4      | 9       | 76    | 137   | ❌     |
    +----------------------+--------+--------+---------+-------+-------+--------+

    review-qa-all: :x: test report for 9ac3034a

    expand test summary
    +---------------------------------------------------------------------------+
    |                              suites summary                               |
    +----------------------+--------+--------+---------+-------+-------+--------+
    |                      | passed | failed | skipped | flaky | total | result |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Secure               | 15     | 0      | 2       | 15    | 17    | ❗     |
    | Manage               | 52     | 6      | 4       | 44    | 62    | ❌     |
    | Verify               | 28     | 0      | 2       | 24    | 30    | ❗     |
    | Create               | 119    | 6      | 4       | 98    | 129   | ❌     |
    | Release              | 6      | 0      | 0       | 6     | 6     | ❗     |
    | Configure            | 0      | 0      | 2       | 0     | 2     | ➖     |
    | Fulfillment          | 1      | 0      | 12      | 1     | 13    | ❗     |
    | Plan                 | 4      | 0      | 1       | 4     | 5     | ❗     |
    | Package              | 0      | 0      | 2       | 0     | 2     | ➖     |
    | Product Intelligence | 1      | 0      | 1       | 1     | 2     | ❗     |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Total                | 226    | 12     | 30      | 193   | 268   | ❌     |
    +----------------------+--------+--------+---------+-------+-------+--------+

    package-and-qa: :x: test report for 9ac3034a

    expand test summary
    +---------------------------------------------------------------------------+
    |                              suites summary                               |
    +----------------------+--------+--------+---------+-------+-------+--------+
    |                      | passed | failed | skipped | flaky | total | result |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Create               | 918    | 4      | 37      | 44    | 959   | ❌     |
    | Product Intelligence | 13     | 0      | 0       | 0     | 13    | ✅     |
    | Verify               | 239    | 1      | 18      | 0     | 258   | ❌     |
    | Manage               | 616    | 6      | 24      | 5     | 646   | ❌     |
    | Plan                 | 323    | 3      | 6       | 0     | 332   | ❌     |
    | Configure            | 1      | 0      | 18      | 0     | 19    | ✅     |
    | Package              | 149    | 0      | 33      | 6     | 182   | ❗     |
    | Version sanity check | 0      | 0      | 6       | 6     | 6     | ➖     |
    | Secure               | 119    | 1      | 12      | 13    | 132   | ❌     |
    | Fulfillment          | 13     | 0      | 66      | 0     | 79    | ✅     |
    | Enablement:Search    | 9      | 0      | 1       | 0     | 10    | ✅     |
    | Release              | 36     | 0      | 0       | 0     | 36    | ✅     |
    | Protect              | 12     | 0      | 0       | 0     | 12    | ✅     |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Total                | 2448   | 15     | 221     | 74    | 2684  | ❌     |
    +----------------------+--------+--------+---------+-------+-------+--------+
  • Zeff Morgan marked this merge request as draft

    marked this merge request as draft

  • Zeff Morgan added 990 commits

    added 990 commits

    Compare with previous version

  • Zeff Morgan added 1 commit

    added 1 commit

    Compare with previous version

  • Zeff Morgan changed the description

    changed the description

  • Zeff Morgan marked this merge request as ready

    marked this merge request as ready

  • Zeff Morgan requested review from @sgregory2

    requested review from @sgregory2

  • Author Contributor

    @sgregory2 would you be willing to give this a quick review just to see if there's anything egregious about my approach that you would recommend changing? This is the MR I was running into issues with when I mentioned doing a little pairing. That might still be required.

  • 🤖 GitLab Bot 🤖 changed milestone to %15.0

    changed milestone to %15.0

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading