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

$INCLUDE is not relative to file's location #537

Closed
miekg opened this issue Oct 17, 2017 · 7 comments
Closed

$INCLUDE is not relative to file's location #537

miekg opened this issue Oct 17, 2017 · 7 comments
Labels

Comments

@miekg
Copy link
Owner

miekg commented Oct 17, 2017

It seems like the working/excution directory from coredns is used and not something set in the root mw.

Corefile:

  .:5053 {
    root /zones/
    auto {
      directory . (.*)
      transfer *
    }
    errors stdout
    log stdout
  }

Main zone (example.com):

@                   3600 IN SOA     ns.example.com. domains.example.com. (
                                      2017091317   ; serial
                                      15m          ; refresh
                                      5m           ; retry
                                      1w           ; expire
                                      12h    )     ; minimum

@                  86400 IN NS     ns.example.com

test IN A 127.0.0.1

; works
$INCLUDE /zones/test.example.com test

; doesn't work
$INCLUDE test.example.com test
@miekg
Copy link
Owner Author

miekg commented Oct 17, 2017

from: coredns/coredns#1068

@miekg
Copy link
Owner Author

miekg commented Nov 8, 2017

btw: not trivial to fix because Go lacks a good where is this file located API; you need to work from the process' CWD up to the files opened, keeping track of relative and absolute file names used.

@stp-ip
Copy link

stp-ip commented Nov 8, 2017

Yeah don't have a simple way besides keeping track and using osext.ExecutableFolder(), but that doesn't follow symlinks afaik so not even that predictable. I still think that $INCLUDE in it's current form is a UI bug.
Maybe leave it open as reference for people to find and rework once coredns k8s stuff settles.

@gibson042
Copy link
Collaborator

Can't we just update parseZone to accept and utilize the file parameter that is already provided by ParseZone? Do you have an example of where something like this would fail?

-			r1, e1 := os.Open(l.token)
+			includePath := l.token
+			if !filepath.IsAbs(includePath) {
+				includePath = filepath.Join(filepath.Dir(f), target)
+			}
+			r1, e1 := os.Open(includePath)
 			if e1 != nil {
-				t <- &Token{Error: &ParseError{f, "failed to open `" + l.token + "'", l}}
+				msg := fmt.Sprintf("failed to open `%s'", l.token)
+				if !filepath.IsAbs(l.token) {
+					msg += fmt.Sprintf(" as `%s'", includePath)
+				}
+				t <- &Token{Error: &ParseError{f, msg, l}}
 				return
 			}
 			if include+1 > 7 {
 				t <- &Token{Error: &ParseError{f, "too deeply nested $INCLUDE", l}}
 				return
 			}
-			parseZone(r1, neworigin, defttl, l.token, t, include+1)
+			parseZone(r1, neworigin, defttl, includePath, t, include+1)

@miekg miekg added the bug label Nov 24, 2017
@miekg
Copy link
Owner Author

miekg commented Nov 24, 2017

@gibson042 just now seeing your update. I think I tries something like this, but failed.

Let me try your patch.

@miekg
Copy link
Owner Author

miekg commented Nov 24, 2017

TL;DR: patch works, I was clearly overthinking it:

example.org -> $INCLUDE include1.org -> $INCLUDE include2.org
% g/src/github.com/coredns/coredns/coredns -dns.port=1053 -conf Corefile.example

% dig @localhost -p 1053 axfr example.org | grep include[1-9]
include1.example.org.	3600	IN	A	127.0.0.53
include2.example.org.	3600	IN	A	127.0.0.54

example.org -> $INCLUDE include1.org -> $INCLUDE include2.org
% cd tmp
% ../g/src/github.com/coredns/coredns/coredns -dns.port=1053 -conf ../Corefile.example

% dig @localhost -p 1053 axfr example.org | grep include[1-9]
include1.example.org.	3600	IN	A	127.0.0.53
include2.example.org.	3600	IN	A	127.0.0.54

/home/miek/example.org -> $INCLUDE include1.org -> $INCLUDE include2.org

works

example.org -> $INCLUDE /tmp/include1.org -> $INCLUDE include2.org

works.

/tmp/example.org -> $INCLUDE a/include1.org -> $INCLUDE include2.org

works.

@gibson042 want to submit that as a PR?

@miekg
Copy link
Owner Author

miekg commented Nov 28, 2017

I'll like to get this fixed in coredns - meaning fixing it here - I'll submit a PR later today.

miekg added a commit that referenced this issue Dec 6, 2017
When using a relative file in an $INCLUDE the file is referenced from
the cwd from the calling processes; this changes it to be down from the
view point where the file exists.

Code from #537 (comment)

Fixes #537
@miekg miekg closed this as completed in #598 Dec 6, 2017
miekg added a commit that referenced this issue Dec 6, 2017
When using a relative file in an $INCLUDE the file is referenced from
the cwd from the calling processes; this changes it to be down from the
view point where the file exists.

Code from #537 (comment)

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

No branches or pull requests

3 participants