-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat: adguard home provider #3249
Conversation
|
Welcome @markussiebert! |
} | ||
|
||
func genAdguardCustomRuleRecord(e *endpoint.Endpoint) string { | ||
return fmt.Sprintf("%s$dnstype=%s,dnsrewrite=NOERROR;%s;%s", e.DNSName, e.RecordType, e.RecordType, e.Targets.String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest wrapping the DNSName in |%s| because otherwise a record for example.com will also match test.example.com or test.example.com.example.com
Default matching mode in AGH appears to be "contains substring"
As per https://github.com/AdguardTeam/AdGuardHome/wiki/Hosts-Blocklists#adblock-style
wrapping the record name in || to match the beginning and end should cause it to perform an exact match.
i.e. "|%s|$dnstype=%s,dnsrewrite=NOERROR;%s;%s"
|
||
func parseAdguardCustomRuleRecord(s string) *endpoint.Endpoint { | ||
|
||
matched := regex.FindStringSubmatch(s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex for this will likely need to be altered for the above comment too.
// NewAdguardHomeProvider initializes a new Vultr BNS based provider | ||
func NewAdguardHomeProvider(domainFilter endpoint.DomainFilter, dryRun bool) (*AdguardHomeProvider, error) { | ||
adguardHomeURL, adguardHomeUrlOk := os.LookupEnv("ADGUARD_HOME_URL") | ||
if !adguardHomeUrlOk { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to document that the AGH URL passed here needs to be suffixed with /control/ for a regular AGH deployment. Since that's where the APIs are under.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got confused trying to troubleshoot that with a very unhelpful error about a nil reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right - it took me some time to figure Out the right path...
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: markussiebert The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@mloiseleur: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Description
This PR adds the adguard home provider. With this provider you can manage records in adguard home.
"AdGuard Home is a network-wide software for blocking ads and tracking. After you set it up, it'll cover ALL your home devices, and you don't need any client-side software for that. It operates as a DNS server that re-routes tracking domains to a “black hole”, thus preventing your devices from connecting to those servers. It's based on software we use for our public AdGuard DNS servers, and both share a lot of code."
from adguard home
Fixes #3243
Checklist