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

plugin/file should not reload with wrong file #6697

Open
Lin-1997 opened this issue May 22, 2024 · 0 comments · May be fixed by #6699
Open

plugin/file should not reload with wrong file #6697

Lin-1997 opened this issue May 22, 2024 · 0 comments · May be fixed by #6699

Comments

@Lin-1997
Copy link

I use plugin/auto as the main plugin to provide dns service. Zone files are dynamically generated by other programs, and plugin/auto will automatically reload these files regularly. These zone files may have errors, for example an A record is set to 127.0.0.300 like the following:

@ 600 IN SOA ns1.my.com. dns.my.com. 1716348976 900 180 1209600 180
@ 600 IN NS ns1.my.com.
right 600 IN A 127.0.0.1
wrong 600 IN A 127.0.0.300
right2 600 IN A 127.0.0.2

The current implementation will ignore the error line and the lines after it, saying "wrong" and "right2", and make the lines before the error line valid, saying "SOA", "NS" and "right". This causes some data to be lost and errors to be hidden.

coredns/plugin/file/file.go

Lines 133 to 163 in 621ffde

func Parse(f io.Reader, origin, fileName string, serial int64) (*Zone, error) {
zp := dns.NewZoneParser(f, dns.Fqdn(origin), fileName)
zp.SetIncludeAllowed(true)
z := NewZone(origin, fileName)
seenSOA := false
for rr, ok := zp.Next(); ok; rr, ok = zp.Next() {
if err := zp.Err(); err != nil {
return nil, err
}
if !seenSOA {
if s, ok := rr.(*dns.SOA); ok {
seenSOA = true
// -1 is valid serial is we failed to load the file on startup.
if serial >= 0 && s.Serial == uint32(serial) { // same serial
return nil, &serialErr{err: "no change in SOA serial", origin: origin, zone: fileName, serial: serial}
}
}
}
if err := z.Insert(rr); err != nil {
return nil, err
}
}
if !seenSOA {
return nil, fmt.Errorf("file %q has no SOA record for origin %s", fileName, origin)
}
return z, nil

I think in addition to judging the "seenSOA", we should also judge whether zp.Next() exits normally or exits with an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant