From a35f8ae914418a48c433618a592b849eb3e61c6a Mon Sep 17 00:00:00 2001 From: Onur Filiz Date: Sat, 4 Feb 2017 12:01:47 -0800 Subject: [PATCH] invoke: Enable plugin file names with extensions A CNI network configuration file contains the plugin's executable file name. Some platforms like Windows require a file name extension for executables. This causes unnecessary burden on admins as they now have to maintain two versions of each type of netconfig file, which differ only by the ".exe" extension. A much simpler design is for invoke package to also look for well-known extensions on platforms that require it. Existing tests are improved and new tests are added to cover the new behavior. Fixes #360 --- invoke/find.go | 16 ++++++---------- invoke/find_test.go | 33 +++++++++++++++++++++++++++++---- invoke/os_unix.go | 20 ++++++++++++++++++++ invoke/os_windows.go | 18 ++++++++++++++++++ 4 files changed, 73 insertions(+), 14 deletions(-) create mode 100644 invoke/os_unix.go create mode 100644 invoke/os_windows.go diff --git a/invoke/find.go b/invoke/find.go index 3b037907..e815404c 100644 --- a/invoke/find.go +++ b/invoke/find.go @@ -30,18 +30,14 @@ func FindInPath(plugin string, paths []string) (string, error) { return "", fmt.Errorf("no paths provided") } - var fullpath string for _, path := range paths { - full := filepath.Join(path, plugin) - if fi, err := os.Stat(full); err == nil && fi.Mode().IsRegular() { - fullpath = full - break + for _, fe := range ExecutableFileExtensions { + fullpath := filepath.Join(path, plugin) + fe + if fi, err := os.Stat(fullpath); err == nil && fi.Mode().IsRegular() { + return fullpath, nil + } } } - if fullpath == "" { - return "", fmt.Errorf("failed to find plugin %q in path %s", plugin, paths) - } - - return fullpath, nil + return "", fmt.Errorf("failed to find plugin %q in path %s", plugin, paths) } diff --git a/invoke/find_test.go b/invoke/find_test.go index be4cc2dd..58543131 100644 --- a/invoke/find_test.go +++ b/invoke/find_test.go @@ -17,7 +17,9 @@ package invoke_test import ( "fmt" "io/ioutil" + "os" "path/filepath" + "strings" "github.com/containernetworking/cni/pkg/invoke" . "github.com/onsi/ginkgo" @@ -26,10 +28,12 @@ import ( var _ = Describe("FindInPath", func() { var ( - multiplePaths []string - pluginName string - pluginDir string - anotherTempDir string + multiplePaths []string + pluginName string + plugin2NameWithExt string + plugin2NameWithoutExt string + pluginDir string + anotherTempDir string ) BeforeEach(func() { @@ -37,11 +41,24 @@ var _ = Describe("FindInPath", func() { Expect(err).NotTo(HaveOccurred()) plugin, err := ioutil.TempFile(tempDir, "a-cni-plugin") + Expect(err).NotTo(HaveOccurred()) + + plugin2Name := "a-plugin-with-extension" + invoke.ExecutableFileExtensions[0] + plugin2, err := os.Create(filepath.Join(tempDir, plugin2Name)) + Expect(err).NotTo(HaveOccurred()) anotherTempDir, err = ioutil.TempDir("", "nothing-here") + Expect(err).NotTo(HaveOccurred()) multiplePaths = []string{anotherTempDir, tempDir} pluginDir, pluginName = filepath.Split(plugin.Name()) + _, plugin2NameWithExt = filepath.Split(plugin2.Name()) + plugin2NameWithoutExt = strings.Split(plugin2NameWithExt, ".")[0] + }) + + AfterEach(func() { + os.RemoveAll(pluginDir) + os.RemoveAll(anotherTempDir) }) Context("when multiple paths are provided", func() { @@ -52,6 +69,14 @@ var _ = Describe("FindInPath", func() { }) }) + Context("when a plugin name without its file name extension is provided", func() { + It("returns the path to the plugin, including its extension", func() { + pluginPath, err := invoke.FindInPath(plugin2NameWithoutExt, multiplePaths) + Expect(err).NotTo(HaveOccurred()) + Expect(pluginPath).To(Equal(filepath.Join(pluginDir, plugin2NameWithExt))) + }) + }) + Context("when an error occurs", func() { Context("when no paths are provided", func() { It("returns an error noting no paths were provided", func() { diff --git a/invoke/os_unix.go b/invoke/os_unix.go new file mode 100644 index 00000000..bab5737a --- /dev/null +++ b/invoke/os_unix.go @@ -0,0 +1,20 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build darwin dragonfly freebsd linux netbsd opensbd solaris + +package invoke + +// Valid file extensions for plugin executables. +var ExecutableFileExtensions = []string{""} diff --git a/invoke/os_windows.go b/invoke/os_windows.go new file mode 100644 index 00000000..7665125b --- /dev/null +++ b/invoke/os_windows.go @@ -0,0 +1,18 @@ +// Copyright 2016 CNI authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package invoke + +// Valid file extensions for plugin executables. +var ExecutableFileExtensions = []string{".exe", ""}