Problem/Motivation

I have several dozen hostnames that can be used across all my environments. My live site alone has at least half-a-dozen, from three different base domains. Some of these are proxied and some aren't. My log is littered with unnecessary "Request came through without being routed through CloudFlare" warnings, and if any of those warnings are actually legit, I can't tell, because the error message doesn't give me enough information about what triggered it.

Proposed resolution

  1. At a minimum, error messages should include the URL being accessed and the clients IP address, like you'd get with a 403 or 404 error.
  2. Allow users to specify the specific hostname(s) they want tracked. Perhaps a textarea that can be used either as a whitelist or a blacklist, which I've seen implemented in other contexts throughout Drupal's UI.
  3. Provide an option to suppress warnings for all authenticated users, so we don't have to rely on editors using an alternate hostname.

User interface changes

The attached image shows a possible UI.

CommentFileSizeAuthor
#6 2023-10-25_06-18.png29.3 KBscott_euser
Screen Shot 2023-05-05 at 11.23.19 AM.png107.73 KBjstoller

Issue fork cloudflare-3358559

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jstoller created an issue. See original summary.

scott_euser made their first commit to this issue’s fork.

scott_euser’s picture

scott_euser commented
Title: Improve error logging and implement a whitelist/blacklist » Add option to suppress bypass error logging

Updating title, actually going through the code there is nothing else to suppress other than bypass error logging. Will similarly make the field names and labels reflect that.

Similarly I do not think its a huge ask for a site builder to specify the domains to restrict suppressing the error message for. Leaving blank for all means a config override can be added like the below to settings.local.php for local development for example.

$config['cloudflare.settings']['suppress_bypass_errors'] = TRUE;

Similarly could be wrapped in a more dynamic statement to have it bypassed on non-live hosts that automatically spin up environments like using PANTHEON_ENVIRONMENT for instance.

scott_euser opened merge request !15

scott_euser’s picture

scott_euser commented
Status: Active » Needs review
scott_euser’s picture

scott_euser commented
StatusFileSize
new 2023-10-25_06-18.png29.3 KB

Here are the form inputs shown in the attached MR. The hostnames is conditional (form #states) on the suppress checkbox being checked.

Screenshot of the form inputs to suppress bypass error logging

andreastkdf made their first commit to this issue’s fork.

andreastkdf’s picture

andreastkdf commented

The last commit is a re-roll for 2.0.x - merged 2.0.x and fixed conflicts. Pls ignore the other previous ones showing here, they are wrong, I've done a force reset, so the MR on git is ok and only showing my merge/fix conflicts commit.

pasive’s picture

pasive commented

As a side note to avoid other warnings the use of this MR requires a Database update and correct export of cloudflare config.
Pay a special attention to behaviour when use config split module.

I don't know how to push to this branch to resolve pipeline issues with phpcs and etc.

scott_euser’s picture

scott_euser commented

Thanks for your comment

The pipeline issues are part of the main branch and unrelated to this: https://git.drupalcode.org/project/cloudflare/-/pipelines - this does not introduce new issues, so that should be resolved elsewhere.

The fact that update hooks update config is expected, you can read more on that here: https://www.drupal.org/node/2535454

If the issue itself is resolved for you (ie, no more flooded logs) after using the merge request, perhaps you could mark it as 'Reviewed & tested by the community' to help progress the issue.

Thanks!

pasive’s picture

pasive commented
Status: Needs review » Reviewed & tested by the community

Done - reviewed!

Thanks for advice.

rosk0’s picture

rosk0
he/him
Primary language Russian
Location Wellington
commented
Version: 8.x-1.x-dev » 2.0.x-dev
Component: User interface » Code
Status: Reviewed & tested by the community » Needs work

Needs work for:

  1. bring up to version 2
  2. hostname schema must be sequence not a string

A more details review would follow up once above is done and the merge request pipeline is green.

scott_euser’s picture

scott_euser commented
Status: Needs work » Needs review

Unable to retest because of #3362051: Remove Cloudflare SDK dependency due to lack of maintenance but resolved conflicts. Its stored as a string so left as a string not sequence. Or do you want to split the textarea prior to save and recombine when setting default value?

igor.barato made their first commit to this issue’s fork.