Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mods to dns_list_base plugin #2422

Merged
merged 4 commits into from
Jul 17, 2018

Conversation

celesteking
Copy link
Contributor

@celesteking celesteking commented May 16, 2018

  • introduce global plugin.lookback_is_rejected flag that allows dnsbls that answer on silence testpoint to function.

Some dnsbl (hostkarma.junkemailfilter.com) return so-called blocked status on queries to 127.0.0.1. This change allows such zones to continue functioning.

In your dnsbl plugin:

    plugin.inherits('dns_list_base');

    plugin.load_config();
    plugin.lookback_is_rejected = plugin.cfg.main.lookback_is_rejected;

This will need a proper per-zone solution some day.

Checklist:

  • docs updated
  • tests updated
  • Changes updated

Copy link
Member

@msimerson msimerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_dumb lacks specificity. DNSBLs make plenty of choices that others consider dumb. Please choose a better name for your variable, like lookback_is_rejected or some such.

@celesteking
Copy link
Contributor Author

All this patch submission drama is way harder than I thought.
I have also added tests, which will increase test time by ~6 sec

@celesteking
Copy link
Contributor Author

Why isn't codecov covering this file?

@msimerson
Copy link
Member

Why isn't codecov covering this file?

Two reasons:

msimerson
msimerson previously approved these changes May 30, 2018
Copy link
Member

@msimerson msimerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@haraka haraka deleted a comment from codecov bot May 30, 2018
@msimerson
Copy link
Member

msimerson commented May 30, 2018

Intermittent test failure:

✔ lookback_is_rejected - zones with quirks pass through when lookback_is_rejected=true
[INFO] [-] [dns_list_base] re-enabling zone hostkarma.junkemailfilter.com
[INFO] [-] [dns_list_base] re-enabling zone bl.spamcop.net
✖ lookback_is_rejected - zones with quirks are disabled when lookback_is_rejected=false
Assertion Message: Enabled all zones back? This should not have happened
AssertionError: [ 'bl.spamcop.net', 'hostkarma.junkemailfilter.com' ] deepEqual [ 'bl.spamcop.net' ]
    at Object.deepEqual (/home/travis/build/haraka/Haraka/node_modules/nodeunit/lib/types.js:83:39)
    at zone_disable_test_func.call (/home/travis/build/haraka/Haraka/tests/plugins/dns_list_base.js:237:18)
    at fin_check (/home/travis/build/haraka/Haraka/tests/plugins/dns_list_base.js:206:9)
    at Timeout.setTimeout (/home/travis/build/haraka/Haraka/tests/plugins/dns_list_base.js:214:67)
    at ontimeout (timers.js:498:11)
    at tryOnTimeout (timers.js:323:5)
    at Timer.listOnTimeout (timers.js:290:5)

@celesteking
Copy link
Contributor Author

This shouldn't have happened. Will get to it once I have time. Obviously, you'll need a working DNS resolver that isn't banned by spamhaus & hostkarma.

@msimerson
Copy link
Member

Update: I've been seeing some random test failures (mostly related to npm) in branches other than this one. The errors I've seen in this PR though are specific to DNSBLs:

dns_list_base
✔ disable_zone - empty request
✔ disable_zone - testbl1, no zones
✔ disable_zone - testbl1, zones miss
[WARN] [-] [dns_list_base] disabling zone 'testbl1': test result
✔ disable_zone - testbl1, zones hit
✔ lookup - Spamcop, test IP
✔ lookup - Spamcop, unlisted IP
✔ multi - Spamcop
✔ multi - CBL
✔ multi - Spamcop + CBL
✔ multi - Spamcop + CBL + negative result
✔ multi - IPv6 addresses supported
✔ first - positive result
✔ first - negative result
✔ first - each_cb
[INFO] [-] [dns_list_base] re-enabling zone bl.spamcop.net
[INFO] [-] [dns_list_base] re-enabling zone hostkarma.junkemailfilter.com
✔ lookback_is_rejected - zones with quirks pass through when lookback_is_rejected=true
[INFO] [-] [dns_list_base] re-enabling zone hostkarma.junkemailfilter.com
[INFO] [-] [dns_list_base] re-enabling zone bl.spamcop.net
Reloading file: /home/travis/build/haraka/Haraka/config/testPlugin.timeout
✖ lookback_is_rejected - zones with quirks are disabled when lookback_is_rejected=false
Assertion Message: Enabled all zones back? This should not have happened
AssertionError: [ 'bl.spamcop.net', 'hostkarma.junkemailfilter.com' ] deepEqual [ 'bl.spamcop.net' ]
    at Object.deepEqual (/home/travis/build/haraka/Haraka/node_modules/nodeunit/lib/types.js:83:39)
    at zone_disable_test_func.call (/home/travis/build/haraka/Haraka/tests/plugins/dns_list_base.js:237:18)
    at fin_check (/home/travis/build/haraka/Haraka/tests/plugins/dns_list_base.js:206:9)
    at Timeout.setTimeout (/home/travis/build/haraka/Haraka/tests/plugins/dns_list_base.js:214:67)
    at ontimeout (timers.js:427:11)
    at tryOnTimeout (timers.js:289:5)
    at listOnTimeout (timers.js:252:5)
    at Timer.processTimers (timers.js:212:10)

…d flag

	* plugin.lookback_is_rejected flag allows "dumb" dnsbls to function (those that answer on silence testpoint)
	* a fix to testpoint check routine: don't assume only 1 record was returned, check them all.
@celesteking
Copy link
Contributor Author

celesteking commented Jun 29, 2018

Have updated the code and squashed the commits.

msimerson
msimerson previously approved these changes Jun 29, 2018
Copy link
Member

@msimerson msimerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better, and tests are passing.

Copy link
Member

@msimerson msimerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@msimerson msimerson merged commit 5557002 into haraka:master Jul 17, 2018
@celesteking celesteking deleted the feature/yu-up-dnsbase branch April 28, 2020 20:39
@haraka haraka deleted a comment from codecov bot Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants