diff --git a/CHANGELOG.md b/CHANGELOG.md index 0413fb05e21..f49e0e22243 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to ### Added -- Linux interfaces' configuration file "source" directive support ([#3257]). +- `source` directives support in `/etc/network/interfaces` on Linux ([#3257]). - RFC 9000 support in DNS-over-QUIC. - Completely disabling statistics by setting the statistics interval to zero ([#2141]). diff --git a/HACKING.md b/HACKING.md index 4c2ee095fb2..101a77579b3 100644 --- a/HACKING.md +++ b/HACKING.md @@ -98,8 +98,9 @@ attributes to make it work in Markdown renderers that strip "id". --> * Constructors should validate their arguments and return meaningful errors. As a corollary, avoid lazy initialization. - * Define `MarshalFoo` methods on non-pointer receivers, as pointer receivers - [can have surprising results][staticcheck-911]. + * Prefer to define methods on pointer receievers, unless the type is small or + a non-pointer receiever is required, for example `MarshalFoo` methods (see + [staticcheck-911]). * Don't mix horizontal and vertical placement of arguments in function and method calls. That is, either this: diff --git a/internal/aghnet/net_linux.go b/internal/aghnet/net_linux.go index 5f410f87704..bf0c65d0004 100644 --- a/internal/aghnet/net_linux.go +++ b/internal/aghnet/net_linux.go @@ -20,16 +20,25 @@ import ( "golang.org/x/sys/unix" ) +// recurrentChecker is used to check all the files which may include references +// for other ones. type recurrentChecker struct { - checker func(r io.Reader, ifaceName string) (patterns []string, has bool, err error) + // checker is the function to check if r's stream contains the desired + // attribute. It must return all the patterns for files which should + // also be checked and each of them should be valid for filepath.Glob + // function. + checker func(r io.Reader, desired string) (patterns []string, has bool, err error) + // initPath is the path of the first member in the sequence of checked + // files. initPath string } -// maxConfigFileSize is the maximum assumed length of the interfaces -// configuration file. -const maxConfigFileSize = 1024 * 1024 +// maxCheckedFileSize is the maximum length of the file that recurrentChecker +// may check. +const maxCheckedFileSize = 1024 * 1024 -func (rc recurrentChecker) checkFile(sourcePath, ifaceName string) ( +// checkFile tries to open and to check single file located on the sourcePath. +func (rc *recurrentChecker) checkFile(sourcePath, desired string) ( subsources []string, has bool, err error, @@ -40,17 +49,15 @@ func (rc recurrentChecker) checkFile(sourcePath, ifaceName string) ( return nil, false, err } - err = nil - defer func() { err = errors.WithDeferred(err, f.Close()) }() - var fileReader io.Reader - fileReader, err = aghio.LimitReader(f, maxConfigFileSize) + var r io.Reader + r, err = aghio.LimitReader(f, maxCheckedFileSize) if err != nil { return nil, false, err } - subsources, has, err = rc.checker(fileReader, ifaceName) + subsources, has, err = rc.checker(r, desired) if err != nil { return nil, false, err } @@ -62,7 +69,8 @@ func (rc recurrentChecker) checkFile(sourcePath, ifaceName string) ( return subsources, has, nil } -func (rc recurrentChecker) handlePatterns(sourcesSet *aghstrings.Set, patterns []string) ( +// handlePatterns parses the patterns and takes care of duplicates. +func (rc *recurrentChecker) handlePatterns(sourcesSet *aghstrings.Set, patterns []string) ( subsources []string, err error, ) { @@ -87,7 +95,8 @@ func (rc recurrentChecker) handlePatterns(sourcesSet *aghstrings.Set, patterns [ return subsources, nil } -func (rc recurrentChecker) check(ifaceName string) (has bool, err error) { +// check walks through all the files searching for the desired attribute. +func (rc *recurrentChecker) check(desired string) (has bool, err error) { var i int sources := []string{rc.initPath} @@ -103,7 +112,7 @@ func (rc recurrentChecker) check(ifaceName string) (has bool, err error) { // The slice of sources is separate from the set of sources to keep the // order in which the files are walked. for sourcesSet := aghstrings.NewSet(rc.initPath); i < len(sources); i++ { - patterns, has, err = rc.checkFile(sources[i], ifaceName) + patterns, has, err = rc.checkFile(sources[i], desired) if err != nil { if errors.Is(err, os.ErrNotExist) { continue @@ -133,7 +142,7 @@ func ifaceHasStaticIP(ifaceName string) (has bool, err error) { // /etc/network/interfaces doesn't, it will return true. Perhaps this // is not the most desirable behavior. - for _, rc := range []recurrentChecker{{ + for _, rc := range []*recurrentChecker{{ checker: dhcpcdStaticConfig, initPath: "/etc/dhcpcd.conf", }, { diff --git a/internal/aghnet/net_linux_test.go b/internal/aghnet/net_linux_test.go index 9897abc2705..9dd9a8665e2 100644 --- a/internal/aghnet/net_linux_test.go +++ b/internal/aghnet/net_linux_test.go @@ -13,7 +13,7 @@ import ( ) func TestRecurrentChecker(t *testing.T) { - c := recurrentChecker{ + c := &recurrentChecker{ checker: ifacesStaticConfig, initPath: "./testdata/include-subsources", } @@ -101,7 +101,7 @@ func TestIfacesStaticConfig(t *testing.T) { want: true, wantPatterns: []string{}, }, { - name: "return_subsources", + name: "return_patterns", data: []byte(`source hello` + nl + `source world` + nl + `#iface enp0s3 inet static` + nl, @@ -109,7 +109,9 @@ func TestIfacesStaticConfig(t *testing.T) { want: false, wantPatterns: []string{"hello", "world"}, }, { - name: "ignore_subsources", + // This one tests if the first found valid interface prevents + // checking files under the `source` directive. + name: "ignore_patterns", data: []byte(`source hello` + nl + `source world` + nl + `iface enp0s3 inet static` + nl, diff --git a/internal/aghnet/testdata/include-subsources b/internal/aghnet/testdata/include-subsources index efa415c5e10..1663e423c85 100644 --- a/internal/aghnet/testdata/include-subsources +++ b/internal/aghnet/testdata/include-subsources @@ -1,8 +1,5 @@ -# Including the subsource. -source ./testdata/ifaces - -# Including subsource wildcard. -source ./testdata/* +# The "testdata" part is added here because the test is actually run from the +# parent directory. Real interface files usually contain only absolute paths. -# ./testdata is the path to the current directory relative to the *_test.go. -# Real interfaces configuration files are usually contain only absolute paths. \ No newline at end of file +source ./testdata/ifaces +source ./testdata/* \ No newline at end of file