Feature/jwt support#1
Merged
ilarimikkonen merged 418 commits intoProfirator:masterfrom Dec 30, 2019
Merged
Conversation
The timing of the keepalive tests are a bit finicky, so try to make them a bit looser to fix intermittent CI failures, since we don't necessarily care about the strictness of this behavior.
If the "core" task was modified but the admin-ui and web-app's caches were cleared, then the core task could fail to load. Persist the core's dependencies, so the core task can run successfully in the CI environment, even if the other normal build files are cleared.
Just to better tidy things up as we shift most of the scripts into ./tasks.
Build process and caching improvements
Under rare circumstances, some of the tests could fail because the timestamps could get rounded incorrectly when converting to milliseconds. This usually cropped up when publishing ConfigVersion records to the database, since the millisecond "version" timestamp Ruby had within the test suite, didn't actually match the real time inserted into the database, so the test suite would get stuck waiting for a ConfigVersion record that would never actually be published. As an example, the following Time.now could get generated: 2018-06-16T10:09:04.437 Which would get inserted into MongoDB as the following (correct) timestamp: 1529158144437 However, calling `.to_f` on that same time object could incorrectly result in the milliseconds being off slightly, due to random float precision issues: 1529158144.438 So by relying on strftime and extracting the milliseconds field from the time object, we can fix the floating point issues when we need to convert Ruby times to milliseconds since epoch.
Add retry logic for a couple of other errors that could rarely crop up during mongodb replicaset changes. This helps fix rare, but intermittent test suite failures with the mongodb replicaset tests.
For the tests that change trafficserver config that require restarts (DNS tests and keepalive tests), these were randomly failing in the CI environment. While I wasn't able to precisely reproduce this locally, I believe the underlying cause was the "sleep 1" after sending the kill signal to trafficserver. In some cases, this wasn't long enough, which I believe explains the CI failures, since essentially the tests were taking place without trafficserver having actually restarted (so the expected configuration was not actually in effect). This attempts to fix it by more properly waiting for the trafficserver process to restart, by ensuring the PID actually changes before proceeding. This also better abstracts some of the similar perpctl commands in the test suite, so other restarts and signals are better handled.
Hopefully this will finally settle this test for good, since rarely the memory growth is reported upwards of 12000kb. So change some things around, and check the overall memory of the process at the start to try and better test for this when running in the full suite.
This follows up on the changes made in 27e73cd. While those issues are probably still good to have and eliminate other race-conditions, it turns out the real issue was related to sending the reload signal to trafficserver at the same time we were writing new templates. Trafficserver by default tries to manage and write the config files itself on config changes, so this could lead to races between the reload signal writing a new config file, and API Umbrella installing a new configuration file based on the template. To fix this, we disable Trafficserver's ability to modify the configuration files itself, which should always ensure our installed templates are the definitive version of the config. I was finally able to reproduce this issue locally (sporadically) by running preceding the DNS tests with some other tests (not sure why), and this seems to finally fix things. Command used to reproduce locally for reference: SEED=42546 bundle exec minitest test/proxy/test_mongodb_replica_set.rb test/proxy/dns/test_custom_server.rb
The badge didn't actually take into account the multiple files (eg both package.json and Gemfile), only the first Gemfile. It also didn't acknowledge the manually ignored issues, and was still reporting them as vulnerabilities (which they aren't).
In Trafficserver 8, the behavior for stale DNS usage has changed slightly. If a valid NXDOMAIN result is returned, then the stale record is expired and will no longer be used. Instead, stale records are only used if the lookup actually fails. This new behavior seems fine (just different), So we need to change our test suite to account for these differences.
This is likely only an issue in our test suite, but if sending rapid reload commands to the api-umbrella process, the TrafficServer config could go missing for a short period of time, which could lead to requests not being routed. This fixes it by only changing the config files if they actually differ (to prevent unnecessary reloads by TrafficServer), and then also ensuring that all config files are fully written to disk before being moved into place (so processes don't ever pick up on partially written config files).
Regardless of order in which processes start versus config files are re-written.
…ile.lock to reduce vulnerabilities The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-RUBY-ACTIONVIEW-173783 - https://snyk.io/vuln/SNYK-RUBY-ACTIONVIEW-173784
[Snyk] Fix for 2 vulnerable dependencies
Use the version of chromedriver that's already installed on the CI image, and should match the version of chrome. I think things are maybe breaking due to mismatched versions of chrome and chromedriver.
|
Hi @ilarimikkonen, is this repo supposed to be the new official one for our fork? In addition, have a look at the original PR as @aarranz fixed the issue with new Maxmind terms and conditions. |
|
hi @fdelavega let's use this. AFAIK, no one is maintaining the old one.. I'll take a look at the Maxmind thing. Thanks! |
fdelavega
pushed a commit
to fdelavega/api-umbrella
that referenced
this pull request
Jul 13, 2022
…nvironments Fix template processing on environments where root is not a superuser
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopt apinf#15