Add low level api cookie passing
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
- 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
- Open Chrome devtools and inspect the
Cookie
values in the header of the API call(s) - 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.
-
I have evaluated the MR acceptance checklist for this MR.
Closes #343654 (closed)
Merge request reports
Activity
changed milestone to %14.10
added Eng-ConsumerQuality Eng-ProducerQuality Engineering Allocation Quality + 1 deleted label
assigned to @zeffmorgan
added typefeature label
1 Message 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
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
Dangermentioned in issue #343654 (closed)
mentioned in issue #341427 (closed)
Allure report
allure-report-publisher
generated test report!review-qa-smoke:
test report
review-qa-reliable: test report
review-qa-all: test report
package-and-qa: test reportreview-qa-blocking:
test report for 9ac3034aexpand 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:
test report for 9ac3034aexpand 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:
test report for 9ac3034aexpand 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 | ❌ | +----------------------+--------+--------+---------+-------+-------+--------+
added 990 commits
-
8969a225...b0a4f182 - 989 commits from branch
master
- def6cd98 - add low level api cookie
-
8969a225...b0a4f182 - 989 commits from branch
requested review from @sgregory2
@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.
- Resolved by Sean Gregory
- Resolved by Sean Gregory
- Resolved by Sean Gregory
changed milestone to %15.0
added missed:14.10 label