Summary: | [ews] Ensure that uat instance doesn't send emails | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||||
Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aakash_jain, darin, jbedard, krollin, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Other | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=222035 https://bugs.webkit.org/show_bug.cgi?id=235552 |
||||||||||
Attachments: |
|
Description
Aakash Jain
2020-10-27 05:18:09 PDT
Created attachment 412414 [details]
Patch
Comment on attachment 412414 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412414&action=review > Tools/BuildSlaveSupport/ews-build/send_email.py:52 > + if 'uat' in HOSTNAME: This seems like an imprecise check. Is there no chance that a host might have "uat" as a substring of its name? It’s in words like "graduate". Heh. The word that came to my mind was "squat". The domain in question was "https://ews-build.webkit-uat.org". Would it cause problems to look for "ews-build.webkit-uat.org"? Or "webkit-uat.org"? Or "webkit-uat"? or "-uat.org"? (In reply to Keith Rollin from comment #3) > The domain in question was "https://ews-build.webkit-uat.org". Would it > cause problems to look for "ews-build.webkit-uat.org"? Or "webkit-uat.org"? > Or "webkit-uat"? or "-uat.org"? or "-uat." or /\buat\b/ (In reply to Keith Rollin from comment #3) > Would it cause problems to look for "ews-build.webkit-uat.org"? Or "webkit-uat.org"? Or "webkit-uat"? or "-uat.org"? Not really. This particular check is specifically for this host. In fact any other host matching this check isn't much of a concern, since almost all other hosts will fail the previous check of is_test_mode_enabled. is_test_mode_enabled is disabled only when BUILDBOT_PRODUCTION env variable is set, which is not set by default. We need to set this variable on uat instance since we need it to mimic production environment (e.g.: reading passwords.json file so that we can easily connect bots to it). Also, no other instance (like local testing instance) should send emails. Instead of disallowing this particular host, I have reverted the check in updated patch to allow only one host: production EWS instance. Created attachment 412525 [details]
Patch
Created attachment 412529 [details]
Patch
Committed r269106: <https://trac.webkit.org/changeset/269106> All reviewed patches have been landed. Closing bug and clearing flags on attachment 412529 [details]. |