diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index 650c5e5ce1d0767d98b1a3d2acbbc4c600eba734..1968fc1d6c36d56baf9b562bdea68433fcae09d6 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -304,7 +304,7 @@ func (df *dockerfile) ParseIgnore(dir string) ([]string, error) { } return ignores, errors.Wrap(err, "state dockerignore file failed") } - if err := util.CheckFileSize(fullPath, constant.MaxFileSize); err != nil { + if err := util.CheckFileInfoAndSize(fullPath, constant.MaxFileSize); err != nil { return ignores, err } diff --git a/builder/dockerfile/parser/parser_test.go b/builder/dockerfile/parser/parser_test.go index 870132f1771195f0c5d1394e5177c95ac0babc56..ba7fce5435d894a7d7bd0b0f82e39f2779b4708c 100644 --- a/builder/dockerfile/parser/parser_test.go +++ b/builder/dockerfile/parser/parser_test.go @@ -328,7 +328,7 @@ func TestParseIgnoreWithDir(t *testing.T) { df := dockerfile{} _, err := df.ParseIgnore(ctxDir.Path()) - assert.ErrorContains(t, err, "a directory") + assert.ErrorContains(t, err, "should be a regular file") } func TestParseWithHeadingArgs(t *testing.T) { diff --git a/builder/dockerfile/run.go b/builder/dockerfile/run.go index d33573f9f54cac3f0a7bfeea5db4e6d64a4070d8..5b066fb17dcd26dbeda17598075e1ea2ae6bd32e 100644 --- a/builder/dockerfile/run.go +++ b/builder/dockerfile/run.go @@ -171,7 +171,7 @@ func setupBindFiles(bundlePath string) (map[string]string, error) { } func generateHosts(bundlePath string) (string, error) { - if err := util.CheckFileSize(constant.HostsFilePath, constant.MaxFileSize); err != nil { + if err := util.CheckFileInfoAndSize(constant.HostsFilePath, constant.MaxFileSize); err != nil { return "", err } @@ -194,7 +194,7 @@ func generateHosts(bundlePath string) (string, error) { } func generateResolv(bundlePath string) (string, error) { - if err := util.CheckFileSize(constant.ResolvFilePath, constant.MaxFileSize); err != nil { + if err := util.CheckFileInfoAndSize(constant.ResolvFilePath, constant.MaxFileSize); err != nil { return "", err } diff --git a/cmd/cli/build.go b/cmd/cli/build.go index 4b4e6f5e14e29dfa0d8ad22b625c03d11e37ef61..c9efd4e4f03258a291d0e51c9aa754e99cc8474f 100644 --- a/cmd/cli/build.go +++ b/cmd/cli/build.go @@ -460,51 +460,34 @@ func readDockerfile() (string, string, error) { return string(buf), parts[1], nil } -func checkDockerfile(filePath string) error { - fileInfo, err := os.Stat(filePath) - if err != nil { - return err - } - - if !fileInfo.Mode().IsRegular() { - return errors.Errorf("file %s should be a regular file", filePath) - } - if fileInfo.Size() == 0 { - return errors.New("file is empty, is it a normal dockerfile?") - } - if fileInfo.Size() > constant.MaxFileSize { - return errors.Errorf("file is too big with size %v, is it a normal dockerfile?", fileInfo.Size()) - } - return nil -} - func resolveDockerfilePath() (string, error) { var resolvedPath = buildOpts.file var err error if buildOpts.file == "" { // filepath is empty, try to resolve with contextDir+Dockerfile resolvedPath = path.Join(buildOpts.contextDir, "Dockerfile") - err = checkDockerfile(resolvedPath) - if err != nil { + if err = util.CheckFileInfoAndSize(resolvedPath, constant.MaxFileSize); err != nil { logrus.Debugf("Stat dockerfile failed with path %s", resolvedPath) - return "", err + return "", errors.Wrap(err, "check dockerfile failed") } + return resolvedPath, nil } - err = checkDockerfile(resolvedPath) - if err != nil { - logrus.Debugf("Stat dockerfile failed with path %s", resolvedPath) - // not found with filepath, try to resolve with contextDir+filepath - resolvedPath = path.Join(buildOpts.contextDir, buildOpts.file) - err = checkDockerfile(resolvedPath) - if err != nil { - logrus.Debugf("Stat dockerfile failed again with path %s", resolvedPath) - return "", err - } + if err = util.CheckFileInfoAndSize(resolvedPath, constant.MaxFileSize); err == nil { + return resolvedPath, nil + } + logrus.Debugf("Stat dockerfile failed with path %s", resolvedPath) + + // not found with filepath, try to resolve with contextDir+filepath + resolvedPath = path.Join(buildOpts.contextDir, buildOpts.file) + if err = util.CheckFileInfoAndSize(resolvedPath, constant.MaxFileSize); err == nil { + return resolvedPath, nil + } + logrus.Debugf("Stat dockerfile failed again with path %s", resolvedPath) - return resolvedPath, nil + return "", errors.Wrap(err, "check dockerfile failed") } func getAbsPath(path string) (string, error) { diff --git a/cmd/cli/import.go b/cmd/cli/import.go index 96263db9420f2a5ea5afa6b2301a522767a4485d..40f933a3b74ea267f1f649057e8c941e242ad565 100644 --- a/cmd/cli/import.go +++ b/cmd/cli/import.go @@ -29,9 +29,8 @@ import ( ) const ( - maxTarballSize = 1024 * 1024 * 1024 // support tarball max size at most 1G - importExample = `isula-build ctr-img import busybox.tar busybox:isula` - importArgsLen = 1 + importExample = `isula-build ctr-img import busybox.tar busybox:isula` + importArgsLen = 1 ) type importOptions struct { @@ -57,7 +56,7 @@ func importCommand(c *cobra.Command, args []string) error { if len(args) < importArgsLen { return errors.New("requires at least one argument") } - if err := util.CheckFileSize(args[0], maxTarballSize); err != nil { + if err := util.CheckFileInfoAndSize(args[0], constant.MaxImportFileSize); err != nil { return err } importOpts.source = args[0] diff --git a/cmd/cli/load.go b/cmd/cli/load.go index 90d189af5d3f0728d7ddcd7d53ace36d1d8044e1..363c54e08fcdbe2f1bafc337fa2bcefbe2056660 100644 --- a/cmd/cli/load.go +++ b/cmd/cli/load.go @@ -128,7 +128,7 @@ func resolveLoadPath(path, pwd string) (string, error) { } path = util.MakeAbsolute(path, pwd) - if err := util.CheckLoadFile(path); err != nil { + if err := util.CheckFileInfoAndSize(path, constant.MaxLoadFileSize); err != nil { return "", err } diff --git a/cmd/cli/load_test.go b/cmd/cli/load_test.go index cb8217ce08313cb18edd7757186ceea0b5c8f729..a5b3c3067a8f2d2ffabe3af076e6afab1b760fdb 100644 --- a/cmd/cli/load_test.go +++ b/cmd/cli/load_test.go @@ -189,14 +189,19 @@ func TestCheckLoadOpts(t *testing.T) { assert.NilError(t, err) root := fs.NewDir(t, t.Name()) defer root.Remove() + + emptyTar := "empty.tar" emptyFile, err := os.Create(filepath.Join(root.Path(), "empty.tar")) assert.NilError(t, err) + fileWithContent, err := os.Create(filepath.Join(root.Path(), "test.tar")) assert.NilError(t, err) ioutil.WriteFile(fileWithContent.Name(), []byte("This is test file"), constant.DefaultRootFileMode) + baseFile, err := os.Create(filepath.Join(root.Path(), "base.tar")) assert.NilError(t, err) ioutil.WriteFile(baseFile.Name(), []byte("This is base file"), constant.DefaultRootFileMode) + libFile, err := os.Create(filepath.Join(root.Path(), "lib.tar")) ioutil.WriteFile(libFile.Name(), []byte("This is lib file"), constant.DefaultRootFileMode) @@ -228,7 +233,7 @@ func TestCheckLoadOpts(t *testing.T) { path: emptyFile.Name(), }, wantErr: true, - errMessage: "loading file is empty", + errMessage: "file " + emptyTar + " is empty", }, { name: "TC-separated load", @@ -290,7 +295,7 @@ func TestCheckLoadOpts(t *testing.T) { }, }, wantErr: true, - errMessage: "resolve base tarball path failed: loading file is empty", + errMessage: "resolve base tarball path failed: file " + emptyTar + " is empty", }, { name: "TC-separated load with empty lib tarball", @@ -303,7 +308,7 @@ func TestCheckLoadOpts(t *testing.T) { }, }, wantErr: true, - errMessage: "resolve lib tarball path failed: loading file is empty", + errMessage: "resolve lib tarball path failed: file " + emptyTar + " is empty", }, { name: "TC-separated load with same base and lib tarball", diff --git a/cmd/daemon/before.go b/cmd/daemon/before.go new file mode 100644 index 0000000000000000000000000000000000000000..ac13303241f3f05a2b4212848911ca1a0e9153ec --- /dev/null +++ b/cmd/daemon/before.go @@ -0,0 +1,250 @@ +// Copyright (c) Huawei Technologies Co., Ltd. 2020. All rights reserved. +// isula-build licensed under the Mulan PSL v2. +// You can use this software according to the terms and conditions of the Mulan PSL v2. +// You may obtain a copy of Mulan PSL v2 at: +// http://license.coscl.org.cn/MulanPSL2 +// THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND, EITHER EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR +// PURPOSE. +// See the Mulan PSL v2 for more details. +// Author: Xiang Li +// Create: 2022-01-30 +// Description: This file is used for isula-build daemon + +package main + +import ( + "io/ioutil" + "os" + "path/filepath" + + "github.com/BurntSushi/toml" + securejoin "github.com/cyphar/filepath-securejoin" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + + constant "isula.org/isula-build" + "isula.org/isula-build/cmd/daemon/config" + "isula.org/isula-build/image" + "isula.org/isula-build/store" + "isula.org/isula-build/util" +) + +// before parses input params for runDaemon() +func before(cmd *cobra.Command) error { + if !util.SetUmask() { + return errors.New("setting umask failed") + } + + logrus.SetOutput(os.Stdout) + logrus.SetFormatter(&logrus.TextFormatter{FullTimestamp: true}) + + if err := validateConfigFileAndMerge(cmd); err != nil { + return err + } + if err := setStoreAccordingToDaemonOpts(); err != nil { + return err + } + + if err := initLogging(); err != nil { + return err + } + + if err := setupWorkingDirectories(); err != nil { + return err + } + + image.SetSystemContext(daemonOpts.DataRoot) + + return nil +} + +func validateConfigFileAndMerge(cmd *cobra.Command) error { + confFiles := []struct { + path string + needed bool + mergeConfig func(cmd *cobra.Command) error + }{ + {path: constant.StorageConfigPath, needed: false, mergeConfig: mergeStorageConfig}, + {path: constant.RegistryConfigPath, needed: false, mergeConfig: nil}, + // policy.json file must exists + {path: constant.SignaturePolicyPath, needed: true, mergeConfig: nil}, + // main configuration comes last for the final merge operation + {path: constant.ConfigurationPath, needed: false, mergeConfig: loadMainConfiguration}, + } + + for _, file := range confFiles { + if exist, err := util.IsExist(file.path); !exist { + if !file.needed { + logrus.Warnf("Config file %q missing, the default configuration is used", file.path) + continue + } + + if err != nil { + return errors.Wrapf(err, "check config file %q failed", file.path) + } + return errors.Errorf("config file %q is not exist", file.path) + } + + if err := util.CheckFileInfoAndSize(file.path, constant.MaxFileSize); err != nil { + return err + } + if file.mergeConfig == nil { + continue + } + if err := file.mergeConfig(cmd); err != nil { + return err + } + } + + return nil +} + +func mergeStorageConfig(cmd *cobra.Command) error { + store.SetDefaultConfigFilePath(constant.StorageConfigPath) + option, err := store.GetDefaultStoreOptions(true) + if err != nil { + return err + } + + if !cmd.Flag("runroot").Changed && option.RunRoot != "" { + daemonOpts.RunRoot = option.RunRoot + } + if !cmd.Flag("dataroot").Changed && option.GraphRoot != "" { + daemonOpts.DataRoot = option.GraphRoot + } + if !cmd.Flag("storage-driver").Changed && option.GraphDriverName != "" { + daemonOpts.StorageDriver = option.GraphDriverName + } + if !cmd.Flag("storage-opt").Changed && len(option.GraphDriverOptions) > 0 { + daemonOpts.StorageOpts = option.GraphDriverOptions + } + + return nil +} + +func loadMainConfiguration(cmd *cobra.Command) error { + conf, err := loadConfig(constant.ConfigurationPath) + if err != nil { + logrus.Errorf("Load and parse main config file failed: %v", err) + os.Exit(constant.DefaultFailedCode) + } + + if err = mergeConfig(conf, cmd); err != nil { + return err + } + + return nil +} + +func loadConfig(path string) (config.TomlConfig, error) { + var conf config.TomlConfig + if err := util.CheckFileInfoAndSize(path, constant.MaxFileSize); err != nil { + return conf, err + } + + configData, err := ioutil.ReadFile(filepath.Clean(path)) + if err != nil { + return conf, err + } + _, err = toml.Decode(string(configData), &conf) + + return conf, err +} + +func mergeConfig(conf config.TomlConfig, cmd *cobra.Command) error { + if conf.Debug && !cmd.Flag("debug").Changed { + daemonOpts.Debug = true + } + if conf.Experimental && !cmd.Flag("experimental").Changed { + daemonOpts.Experimental = true + } + if conf.LogLevel != "" && !cmd.Flag("log-level").Changed { + daemonOpts.LogLevel = conf.LogLevel + } + if conf.Group != "" && !cmd.Flag("group").Changed { + daemonOpts.Group = conf.Group + } + if conf.Runtime != "" { + daemonOpts.RuntimePath = conf.Runtime + } + if conf.RunRoot != "" && !cmd.Flag("runroot").Changed { + daemonOpts.RunRoot = conf.RunRoot + } + if conf.DataRoot != "" && !cmd.Flag("dataroot").Changed { + daemonOpts.DataRoot = conf.DataRoot + } + + return nil +} + +func setStoreAccordingToDaemonOpts() error { + runRoot, err := securejoin.SecureJoin(daemonOpts.RunRoot, "storage") + if err != nil { + return err + } + dataRoot, err := securejoin.SecureJoin(daemonOpts.DataRoot, "storage") + if err != nil { + return err + } + + store.SetDefaultStoreOptions(store.DaemonStoreOptions{ + RunRoot: runRoot, + DataRoot: dataRoot, + Driver: daemonOpts.StorageDriver, + DriverOption: util.CopyStrings(daemonOpts.StorageOpts), + }) + + return nil +} + +func initLogging() error { + logrusLvl, err := logrus.ParseLevel(daemonOpts.LogLevel) + if err != nil { + return errors.Wrapf(err, "unable to parse log level") + } + logrus.SetLevel(logrusLvl) + if daemonOpts.Debug { + logrus.SetLevel(logrus.DebugLevel) + } + + return nil +} + +func setupWorkingDirectories() error { + if daemonOpts.RunRoot == daemonOpts.DataRoot { + return errors.Errorf("runroot(%q) and dataroot(%q) must be different paths", daemonOpts.RunRoot, daemonOpts.DataRoot) + } + + buildTmpDir, err := securejoin.SecureJoin(daemonOpts.DataRoot, constant.DataRootTmpDirPrefix) + if err != nil { + return err + } + dirs := []string{daemonOpts.DataRoot, daemonOpts.RunRoot, buildTmpDir} + for _, dir := range dirs { + if !filepath.IsAbs(dir) { + return errors.Errorf("%q not an absolute dir, the \"dataroot\" and \"runroot\" must be an absolute path", dir) + } + + if exist, err := util.IsExist(dir); err != nil { + return err + } else if !exist { + if err := os.MkdirAll(dir, constant.DefaultRootDirMode); err != nil { + return errors.Wrapf(err, "create directory for %q failed", dir) + } + continue + } + if !util.IsDirectory(dir) { + return errors.Errorf("%q is not a directory", dir) + } + } + + // change config root owner as group current defined + if err := util.ChangeGroup(constant.ConfigRoot, daemonOpts.Group); err != nil { + logrus.Errorf("Chown for %s failed: %v", constant.ConfigRoot, err) + return err + } + + return nil +} diff --git a/cmd/daemon/main_test.go b/cmd/daemon/before_test.go similarity index 64% rename from cmd/daemon/main_test.go rename to cmd/daemon/before_test.go index 3947f7aded2dadb4162142baba1b4cace87e23d5..b1b8859094c857f657fba23d15ffb924ffafdb36 100644 --- a/cmd/daemon/main_test.go +++ b/cmd/daemon/before_test.go @@ -112,6 +112,10 @@ func TestSetupWorkingDirectories(t *testing.T) { func TestRunAndDataRootSet(t *testing.T) { dataRoot := fs.NewDir(t, t.Name()) runRoot := fs.NewDir(t, t.Name()) + result := store.DaemonStoreOptions{ + DataRoot: dataRoot.Join("storage"), + RunRoot: runRoot.Join("storage"), + } conf := config.TomlConfig{ Debug: true, @@ -123,34 +127,41 @@ func TestRunAndDataRootSet(t *testing.T) { } cmd := newDaemonCommand() - result := store.DaemonStoreOptions{ - DataRoot: dataRoot.Join("storage"), - RunRoot: runRoot.Join("storage"), - } - - setStorage := func(content string) func() { - return func() { - if err := mergeConfig(conf, cmd); err != nil { - t.Fatalf("mrege config failed with error: %v", err) - } + setStorage := func(content string) { + // merge main config + if err := mergeConfig(conf, cmd); err != nil { + t.Fatalf("merge config failed with error: %v", err) + } - fileName := "storage.toml" - tmpDir := fs.NewDir(t, t.Name(), fs.WithFile(fileName, content)) - defer tmpDir.Remove() + // simulate storage.toml and merge + fileName := "storage.toml" + tmpDir := fs.NewDir(t, t.Name(), fs.WithFile(fileName, content)) + defer tmpDir.Remove() + filePath := tmpDir.Join(fileName) - filePath := tmpDir.Join(fileName) - store.SetDefaultConfigFilePath(filePath) - option, err := store.GetDefaultStoreOptions(true) - if err != nil { - t.Fatalf("get default store options failed with error: %v", err) - } + store.SetDefaultConfigFilePath(filePath) + option, err := store.GetDefaultStoreOptions(true) + if err != nil { + t.Fatalf("get default store options failed with error: %v", err) + } - var storeOpt store.DaemonStoreOptions - storeOpt.RunRoot = option.RunRoot - storeOpt.DataRoot = option.GraphRoot - store.SetDefaultStoreOptions(storeOpt) + if !cmd.Flag("runroot").Changed && option.RunRoot != "" { + daemonOpts.RunRoot = option.RunRoot + } + if !cmd.Flag("dataroot").Changed && option.GraphRoot != "" { + daemonOpts.DataRoot = option.GraphRoot + } + if !cmd.Flag("storage-driver").Changed && option.GraphDriverName != "" { + daemonOpts.StorageDriver = option.GraphDriverName + } + if !cmd.Flag("storage-opt").Changed && len(option.GraphDriverOptions) > 0 { + daemonOpts.StorageOpts = option.GraphDriverOptions } + // final set + if err := setStoreAccordingToDaemonOpts(); err != nil { + t.Fatalf("set store options failed with error: %v", err) + } } testcases := []struct { @@ -160,28 +171,28 @@ func TestRunAndDataRootSet(t *testing.T) { }{ { // first run so can not be affected by other testcase - name: "TC3 - all not set", - setF: setStorage("[storage]\ndriver = \"overlay\""), + name: "TC1 - all not set", + setF: func() { setStorage("[storage]\ndriver = \"overlay\"") }, expectation: store.DaemonStoreOptions{ DataRoot: "/var/lib/isula-build/storage", RunRoot: "/var/run/isula-build/storage", }, }, { - name: "TC1 - cmd set, configuration and storage not set", + name: "TC2 - cmd set, configuration and storage not set", setF: func() { cmd.PersistentFlags().Set("runroot", runRoot.Path()) cmd.PersistentFlags().Set("dataroot", dataRoot.Path()) - checkAndValidateConfig(cmd) + setStorage("[storage]\ndriver = \"overlay\"") }, expectation: result, }, { - name: "TC2 - cmd and storage not set, configuration set", + name: "TC3 - cmd and storage not set, configuration set", setF: func() { conf.DataRoot = dataRoot.Path() conf.RunRoot = runRoot.Path() - checkAndValidateConfig(cmd) + setStorage("[storage]\ndriver = \"overlay\"") }, expectation: result, }, @@ -190,8 +201,22 @@ func TestRunAndDataRootSet(t *testing.T) { setF: func() { config := fmt.Sprintf("[storage]\ndriver = \"%s\"\nrunroot = \"%s\"\ngraphroot = \"%s\"\n", "overlay", runRoot.Join("storage"), dataRoot.Join("storage")) - sT := setStorage(config) - sT() + setStorage(config) + }, + expectation: result, + }, + { + name: "TC5 - cmd not set, configuration and storage set, configuration first", + setF: func() { + conf.DataRoot = dataRoot.Path() + conf.RunRoot = runRoot.Path() + + dataRootStorage := fs.NewDir(t, t.Name()) + runRootStorage := fs.NewDir(t, t.Name()) + config := fmt.Sprintf("[storage]\ndriver = \"%s\"\nrunroot = \"%s\"\ngraphroot = \"%s\"\n", + "overlay", runRootStorage.Join("storage"), dataRootStorage.Join("storage")) + + setStorage(config) }, expectation: result, }, @@ -210,3 +235,29 @@ func TestRunAndDataRootSet(t *testing.T) { } } + +func TestValidateConfigFileAndMerge(t *testing.T) { + // cmd line args keep default. + cmd := newDaemonCommand() + err := validateConfigFileAndMerge(cmd) + assert.NilError(t, err) + + // cmd line runroot and dataroot args are set. + dataRoot := fs.NewDir(t, t.Name()) + runRoot := fs.NewDir(t, t.Name()) + cmd.PersistentFlags().Set("runroot", runRoot.Path()) + cmd.PersistentFlags().Set("dataroot", dataRoot.Path()) + err = validateConfigFileAndMerge(cmd) + assert.NilError(t, err) + + if err := setStoreAccordingToDaemonOpts(); err != nil { + t.Fatalf("set store options failed with error: %v", err) + } + storeOptions, err := store.GetDefaultStoreOptions(false) + if err != nil { + t.Fatalf("get default store options failed with error: %v", err) + } + + assert.Equal(t, storeOptions.GraphRoot, dataRoot.Join("storage")) + assert.Equal(t, storeOptions.RunRoot, runRoot.Join("storage")) +} diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index 3cecbf93b51baaac52902ac04bf061b7291db7c2..5b6327daeabd63b58183b20fcc76691464267c15 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -15,22 +15,15 @@ package main import ( "fmt" - "io/ioutil" "os" - "path/filepath" - "github.com/BurntSushi/toml" "github.com/containers/storage/pkg/reexec" - securejoin "github.com/cyphar/filepath-securejoin" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" constant "isula.org/isula-build" - "isula.org/isula-build/cmd/daemon/config" "isula.org/isula-build/daemon" _ "isula.org/isula-build/exporter/register" - "isula.org/isula-build/image" "isula.org/isula-build/pkg/version" "isula.org/isula-build/store" "isula.org/isula-build/util" @@ -132,286 +125,3 @@ func main() { os.Exit(constant.DefaultFailedCode) } } - -func initLogging() error { - logrusLvl, err := logrus.ParseLevel(daemonOpts.LogLevel) - if err != nil { - return errors.Wrapf(err, "unable to parse log level") - } - logrus.SetLevel(logrusLvl) - if daemonOpts.Debug { - logrus.SetLevel(logrus.DebugLevel) - } - - return nil -} - -// before parses input params for runDaemon() -func before(cmd *cobra.Command) error { - if !util.SetUmask() { - return errors.New("setting umask failed") - } - - logrus.SetOutput(os.Stdout) - logrus.SetFormatter(&logrus.TextFormatter{FullTimestamp: true}) - - runRoot, err := securejoin.SecureJoin(daemonOpts.RunRoot, "storage") - if err != nil { - return err - } - dataRoot, err := securejoin.SecureJoin(daemonOpts.DataRoot, "storage") - if err != nil { - return err - } - store.SetDefaultStoreOptions(store.DaemonStoreOptions{ - RunRoot: runRoot, - DataRoot: dataRoot, - Driver: daemonOpts.StorageDriver, - DriverOption: util.CopyStrings(daemonOpts.StorageOpts), - }) - - if err := checkAndValidateConfig(cmd); err != nil { - return err - } - - if err := initLogging(); err != nil { - return err - } - - if err := setupWorkingDirectories(); err != nil { - return err - } - - image.SetSystemContext(daemonOpts.DataRoot) - - return nil -} - -func loadConfig(path string) (config.TomlConfig, error) { - var conf config.TomlConfig - fi, err := os.Stat(path) - if err != nil { - return conf, err - } - - if !fi.Mode().IsRegular() { - return conf, errors.New("config file must be a regular file") - } - - if err = util.CheckFileSize(path, constant.MaxFileSize); err != nil { - return conf, err - } - configData, err := ioutil.ReadFile(filepath.Clean(path)) - if err != nil { - return conf, err - } - _, err = toml.Decode(string(configData), &conf) - - return conf, err -} - -func checkRootSetInConfig(path string) (setRunRoot, setGraphRoot bool, err error) { - fi, err := os.Stat(path) - if err != nil { - return false, false, err - } - - if !fi.Mode().IsRegular() { - err = errors.New("config file must be a regular file") - return false, false, err - } - - if err = util.CheckFileSize(path, constant.MaxFileSize); err != nil { - return false, false, err - } - - configData, err := ioutil.ReadFile(filepath.Clean(path)) - if err != nil { - return false, false, err - } - conf := struct { - Storage struct { - RunRoot string `toml:"runroot"` - DataRoot string `toml:"graphroot"` - } `toml:"storage"` - }{} - _, err = toml.Decode(string(configData), &conf) - return conf.Storage.RunRoot != "", conf.Storage.DataRoot != "", err -} - -func mergeStorageConfig(cmd *cobra.Command) error { - store.SetDefaultConfigFilePath(constant.StorageConfigPath) - option, err := store.GetDefaultStoreOptions(true) - if err == nil { - if option.GraphDriverName != "" && !cmd.Flag("storage-driver").Changed { - daemonOpts.StorageDriver = option.GraphDriverName - } - if len(option.GraphDriverOptions) > 0 && !cmd.Flag("storage-opt").Changed { - daemonOpts.StorageOpts = option.GraphDriverOptions - } - } - - var storeOpt store.DaemonStoreOptions - storeOpt.RunRoot = option.RunRoot - storeOpt.DataRoot = option.GraphRoot - - setRunRoot, setDataRoot, err := checkRootSetInConfig(constant.StorageConfigPath) - if err != nil { - return err - } - - if !setRunRoot { - storeOpt.RunRoot, err = securejoin.SecureJoin(daemonOpts.RunRoot, "storage") - if err != nil { - return err - } - } - if !setDataRoot { - storeOpt.DataRoot, err = securejoin.SecureJoin(daemonOpts.DataRoot, "storage") - if err != nil { - return err - } - } - if daemonOpts.StorageDriver != "" { - storeOpt.Driver = daemonOpts.StorageDriver - } - if len(daemonOpts.StorageOpts) > 0 { - storeOpt.DriverOption = util.CopyStrings(daemonOpts.StorageOpts) - } - store.SetDefaultStoreOptions(storeOpt) - - return nil -} - -func mergeConfig(conf config.TomlConfig, cmd *cobra.Command) error { - if conf.Debug && !cmd.Flag("debug").Changed { - daemonOpts.Debug = true - } - if conf.Experimental && !cmd.Flag("experimental").Changed { - daemonOpts.Experimental = true - } - if conf.LogLevel != "" && !cmd.Flag("log-level").Changed { - daemonOpts.LogLevel = conf.LogLevel - } - if conf.Group != "" && !cmd.Flag("group").Changed { - daemonOpts.Group = conf.Group - } - if conf.Runtime != "" { - daemonOpts.RuntimePath = conf.Runtime - } - if conf.RunRoot != "" && !cmd.Flag("runroot").Changed { - daemonOpts.RunRoot = conf.RunRoot - } - if conf.DataRoot != "" && !cmd.Flag("dataroot").Changed { - daemonOpts.DataRoot = conf.DataRoot - } - - runRoot, err := securejoin.SecureJoin(daemonOpts.RunRoot, "storage") - if err != nil { - return err - } - - dataRoot, err := securejoin.SecureJoin(daemonOpts.DataRoot, "storage") - if err != nil { - return err - } - store.SetDefaultStoreOptions(store.DaemonStoreOptions{ - DataRoot: dataRoot, - RunRoot: runRoot, - }) - - return nil -} - -func setupWorkingDirectories() error { - if daemonOpts.RunRoot == daemonOpts.DataRoot { - return errors.Errorf("runroot(%q) and dataroot(%q) must be different paths", daemonOpts.RunRoot, daemonOpts.DataRoot) - } - - buildTmpDir, err := securejoin.SecureJoin(daemonOpts.DataRoot, constant.DataRootTmpDirPrefix) - if err != nil { - return err - } - dirs := []string{daemonOpts.DataRoot, daemonOpts.RunRoot, buildTmpDir} - for _, dir := range dirs { - if !filepath.IsAbs(dir) { - return errors.Errorf("%q not an absolute dir, the \"dataroot\" and \"runroot\" must be an absolute path", dir) - } - - if exist, err := util.IsExist(dir); err != nil { - return err - } else if !exist { - if err := os.MkdirAll(dir, constant.DefaultRootDirMode); err != nil { - return errors.Wrapf(err, "create directory for %q failed", dir) - } - continue - } - if !util.IsDirectory(dir) { - return errors.Errorf("%q is not a directory", dir) - } - } - - // change config root owner as group current defined - if err := util.ChangeGroup(constant.ConfigRoot, daemonOpts.Group); err != nil { - logrus.Errorf("Chown for %s failed: %v", constant.ConfigRoot, err) - return err - } - - return nil -} - -func checkAndValidateConfig(cmd *cobra.Command) error { - // check if configuration.toml file exists, merge config if exists - if exist, err := util.IsExist(constant.ConfigurationPath); err != nil { - return err - } else if !exist { - logrus.Warnf("Main config file missing, the default configuration is used") - } else { - conf, err := loadConfig(constant.ConfigurationPath) - if err != nil { - logrus.Errorf("Load and parse main config file failed: %v", err) - os.Exit(constant.DefaultFailedCode) - } - - if err = mergeConfig(conf, cmd); err != nil { - return err - } - } - - // file policy.json must be exist - if exist, err := util.IsExist(constant.SignaturePolicyPath); err != nil { - return err - } else if !exist { - return errors.Errorf("policy config file %v is not exist", constant.SignaturePolicyPath) - } - - // check all config files - confFiles := []string{constant.RegistryConfigPath, constant.SignaturePolicyPath, constant.StorageConfigPath} - for _, file := range confFiles { - if exist, err := util.IsExist(file); err != nil { - return err - } else if exist { - fi, err := os.Stat(file) - if err != nil { - return errors.Wrapf(err, "stat file %q failed", file) - } - - if !fi.Mode().IsRegular() { - return errors.Errorf("file %s should be a regular file", fi.Name()) - } - - if err := util.CheckFileSize(file, constant.MaxFileSize); err != nil { - return err - } - } - } - - // if storage config file exists, merge storage config - if exist, err := util.IsExist(constant.StorageConfigPath); err != nil { - return err - } else if exist { - return mergeStorageConfig(cmd) - } - - return nil -} diff --git a/constant.go b/constant.go index 5af4fe2c7f7c428c53a0771e7d2bc1e1d0f65d54..47b7c2bdac1f0dc23e723542b05573a02054d3cb 100644 --- a/constant.go +++ b/constant.go @@ -67,8 +67,14 @@ const ( // CliLogBufferLen is log channel buffer size CliLogBufferLen = 8 - // MaxFileSize is the maximum file size allowed, set 1M + // MaxFileSize is the max size of normal config file at most 1M MaxFileSize = 1024 * 1024 + // JSONMaxFileSize is the max size of json file at most 10M + JSONMaxFileSize = 10 * 1024 * 1024 + // MaxImportFileSize is the max size of import image file at most 1G + MaxImportFileSize = 1024 * 1024 * 1024 + // MaxLoadFileSize is the max size of load image file at most 50G + MaxLoadFileSize = 50 * 1024 * 1024 * 1024 // DefaultHTTPTimeout includes the total time of dial, TLS handshake, request, resp headers and body DefaultHTTPTimeout = 3600 * time.Second // DefaultFailedCode is the exit code for most scenes diff --git a/daemon/load.go b/daemon/load.go index 1ee025b016109e6ad00e3617f23eba8f9021a1f3..2d0c15439f8eac992843185690a56d750a318277 100644 --- a/daemon/load.go +++ b/daemon/load.go @@ -62,7 +62,7 @@ func (b *Backend) getLoadOptions(req *pb.LoadRequest) (LoadOptions, error) { // normal image loading if !req.GetSep().GetEnabled() { - if err = util.CheckLoadFile(opt.path); err != nil { + if err = util.CheckFileInfoAndSize(opt.path, constant.MaxLoadFileSize); err != nil { return LoadOptions{}, err } return opt, nil diff --git a/hack/integration_coverage.sh b/hack/integration_coverage.sh index 7462c545b6734a8c171fcb34656183300f461360..e08e3afacc8120ec2826a773b7d42945122d00d3 100755 --- a/hack/integration_coverage.sh +++ b/hack/integration_coverage.sh @@ -24,7 +24,7 @@ go_test_mod_method="-mod=vendor" go_test_count_method="-count=1" go_test_cover_method="-covermode=set" main_pkg="${vendor_name}/${project_name}/${main_relative_path}" -main_test_file=${project_root}/${main_relative_path}/main_test.go +main_test_file=${project_root}/${main_relative_path}/before_test.go main_file=${project_root}/${main_relative_path}/main.go coverage_file=${project_root}/cover_integration_test_all.out coverage_html=${project_root}/cover_integration_test_all.html @@ -33,7 +33,7 @@ coverage_client_log=${project_root}/cover_integration_test_all_client.log main_test_binary_file=${project_root}/main.test function precheck() { - if pgrep isula-builder > /dev/null 2>&1; then + if pgrep isula-builder >/dev/null 2>&1; then echo "isula-builder is already running, please stop it first" exit 1 fi @@ -48,7 +48,7 @@ function modify_main_test() { sed -i "/${comment_pattern}/s/^#*/\/\/ /" "${main_file}" # add new line for main_test.go code_snippet="func TestMain(t *testing.T) { main() }" - echo "${code_snippet}" >> "${main_test_file}" + echo "${code_snippet}" >>"${main_test_file}" } function recover_main_test() { @@ -58,14 +58,14 @@ function recover_main_test() { function build_main_test_binary() { pkgs=$(go list "${go_test_mod_method}" "${project_root}"/... | grep -Ev "${exclude_pattern}" | tr "\r\n" ",") - go test -coverpkg="${pkgs}" "${main_pkg}" "${go_test_mod_method}" "${go_test_cover_method}" "${go_test_count_method}" -c -o="${main_test_binary_file}" > /dev/null 2>&1 + go test -coverpkg="${pkgs}" "${main_pkg}" "${go_test_mod_method}" "${go_test_cover_method}" "${go_test_count_method}" -c -o="${main_test_binary_file}" >/dev/null 2>&1 } function run_main_test_binary() { - ${main_test_binary_file} -test.coverprofile="${coverage_file}" > "${coverage_daemon_log}" 2>&1 & + ${main_test_binary_file} -test.coverprofile="${coverage_file}" >"${coverage_daemon_log}" 2>&1 & main_test_pid=$! for _ in $(seq 1 10); do - if isula-build info > /dev/null 2>&1; then + if isula-build info >/dev/null 2>&1; then break else sleep 1 @@ -77,8 +77,8 @@ function run_coverage_test() { # do cover tests while IFS= read -r testfile; do printf "%-60s" "test $(basename "${testfile}"): " - echo -e "\n$(basename "${testfile}"):" >> "${coverage_client_log}" - if ! bash "${testfile}" >> "${coverage_client_log}" 2>&1; then + echo -e "\n$(basename "${testfile}"):" >>"${coverage_client_log}" + if ! bash "${testfile}" >>"${coverage_client_log}" 2>&1; then echo "FAIL" return_code=1 else diff --git a/image/context.go b/image/context.go index ea826c686ae39589ea734e1785454e712e58ea03..6fd7725826b180a797ea7728670a81c10bee7f16 100644 --- a/image/context.go +++ b/image/context.go @@ -15,16 +15,12 @@ package image import ( "io" - "os" "sync" cp "github.com/containers/image/v5/copy" "github.com/containers/image/v5/types" - "github.com/pkg/errors" - "github.com/sirupsen/logrus" constant "isula.org/isula-build" - "isula.org/isula-build/util" ) var ( @@ -40,33 +36,8 @@ func init() { } } -func validateConfigFiles(configs []string) error { - var ( - cfgInfo os.FileInfo - err error - ) - for _, cfg := range configs { - if err = util.CheckFileSize(cfg, constant.MaxFileSize); err != nil { - return err - } - if cfgInfo, err = os.Stat(cfg); err != nil { - return err - } - if cfgInfo.Size() == 0 { - return errors.Errorf("config %q cannot be an empty file", cfg) - } - } - - return nil -} - // SetSystemContext set the values of globalSystemContext func SetSystemContext(dataRoot string) { - err := validateConfigFiles([]string{constant.SignaturePolicyPath, constant.RegistryConfigPath}) - if err != nil { - logrus.Fatal(err) - } - once.Do(func() { globalSystemContext.SignaturePolicyPath = constant.SignaturePolicyPath globalSystemContext.SystemRegistriesConfPath = constant.RegistryConfigPath diff --git a/image/context_test.go b/image/context_test.go index 131c3a2616bec3fe35ee213f2f5c6d9629b6b8fc..08d34e9405511ae70167dde042107bfe1e5dbd19 100644 --- a/image/context_test.go +++ b/image/context_test.go @@ -29,65 +29,6 @@ func doCmd(cmd string) { } } -func TestValidateConfigFiles(t *testing.T) { - type args struct { - configs []string - } - tests := []struct { - name string - args args - wantErr bool - prepareCmd string - cleanCmd string - }{ - { - name: "none file", - args: args{configs: []string{"/tmp/validate-config/policy.json"}}, - prepareCmd: "mkdir -p /tmp/validate-config/ && touch /tmp/validate-config/policy.json", - cleanCmd: "rm -rf /tmp/validate-config", - wantErr: true, - }, - { - name: "size zero", - args: args{configs: []string{"/tmp/validate-config/policy.json"}}, - prepareCmd: "mkdir -p /tmp/validate-config/ && touch /tmp/validate-config/policy.json", - cleanCmd: "rm -rf /tmp/validate-config", - wantErr: true, - }, - { - name: "big file", - args: args{configs: []string{"/tmp/validate-config/policy.json"}}, - prepareCmd: "mkdir -p /tmp/validate-config/ && dd if=/dev/zero of=/tmp/validate-config/policy.json bs=16k count=1024", - cleanCmd: "rm -rf /tmp/validate-config", - wantErr: true, - }, - { - name: "normal", - args: args{configs: []string{"/tmp/validate-config/policy.json"}}, - prepareCmd: "mkdir -p /tmp/validate-config/ && echo hello > /tmp/validate-config/policy.json", - cleanCmd: "rm -rf /tmp/validate-config", - wantErr: false, - }, - { - name: "normal", - args: args{configs: []string{"/tmp/validate-config/policy.json"}}, - prepareCmd: "mkdir -p /tmp/validate-config/ && echo hello > /tmp/validate-config/policy.json.bak &&" + - "ln -sf /tmp/validate-config/policy.json.bak /tmp/validate-config/policy.json", - cleanCmd: "rm -rf /tmp/validate-config", - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - doCmd(tt.prepareCmd) - if err := validateConfigFiles(tt.args.configs); (err != nil) != tt.wantErr { - t.Errorf("validateConfigFiles() error = %v, wantErr %v", err, tt.wantErr) - } - doCmd(tt.cleanCmd) - }) - } -} - func TestSetSystemContext(t *testing.T) { prepareFunc := func(path string) { if _, err := os.Stat(path); err != nil { diff --git a/runner/runner.go b/runner/runner.go index e813024c1fe59b1e02c45395d60fd7a3bfee78ea..dd439015ea3cba654a30c06d84b74364548cc36e 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -215,7 +215,7 @@ func (r *OCIRunner) runContainer() (unix.WaitStatus, error) { // nolint:gocyclo } func readPid(pidFilePath string) (int, error) { - if err := util.CheckFileSize(pidFilePath, constant.MaxFileSize); err != nil { + if err := util.CheckFileInfoAndSize(pidFilePath, constant.MaxFileSize); err != nil { return 0, err } pidValue, err := ioutil.ReadFile(filepath.Clean(pidFilePath)) diff --git a/tests/src/test_storage_root_priority_of_configuration.sh b/tests/src/test_storage_root_priority_of_configuration.sh new file mode 100644 index 0000000000000000000000000000000000000000..e585d0c40f3c4f2d2d396c133a96280687e5d21a --- /dev/null +++ b/tests/src/test_storage_root_priority_of_configuration.sh @@ -0,0 +1,92 @@ +#!/bin/bash + +# Copyright (c) Huawei Technologies Co., Ltd. 2020. All rights reserved. +# isula-build licensed under the Mulan PSL v2. +# You can use this software according to the terms and conditions of the Mulan PSL v2. +# You may obtain a copy of Mulan PSL v2 at: +# http://license.coscl.org.cn/MulanPSL2 +# THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND, EITHER EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR +# PURPOSE. +# See the Mulan PSL v2 for more details. +# Author: Weizheng Xing +# Create: 2022-01-25 +# Description: test priority of data and run root with different configuration locations( binary > configuration.toml > storage.toml) + +top_dir=$(git rev-parse --show-toplevel) +# shellcheck disable=SC1091 +source "$top_dir"/tests/lib/common.sh + +config_file="/etc/isula-build/configuration.toml" +storage_file="/etc/isula-build/storage.toml" +main_run_root="/tmp/run/main-isula-build" +main_data_root="/tmp/lib/main-isula-build" +storage_run_root="/tmp/run/storage-isula-build" +storage_data_root="/tmp/lib/storage-isula-build" + +# change to new data and run root +function pre_test() { + cp $config_file "$config_file".bak + cp $config_file "$config_file".bak + + cp $storage_file "$storage_file".bak + cp $storage_file "$storage_file".bak +} + +function clean() { + rm -f "$config_file" + rm -f "$storage_file" + + mv $config_file.bak "$config_file" + mv $storage_file.bak "$storage_file" +} + +function main_set_storage_not_set() { + sed -i "/run_root/d;/data_root/d" $config_file + sed -i "/runroot/d;/graphroot/d" $storage_file + echo "run_root=\"$main_run_root\"" >>$config_file + echo "data_root=\"$main_data_root\"" >>$config_file + + check_root "$main_run_root" "$main_data_root" +} + +function main_not_set_storage_set() { + sed -i "/run_root/d;/data_root/d" $config_file + sed -i "/runroot/d;/graphroot/d" $storage_file + eval "sed -i '/\[storage\]/a\runroot=\"$storage_run_root\"' $storage_file" + eval "sed -i '/\[storage\]/a\graphroot=\"$storage_data_root\"' $storage_file" + + check_root "$main_run_root" "$main_data_root" +} + +function main_set_storage_set() { + sed -i "/run_root/d;/data_root/d" $config_file + sed -i "/runroot/d;/graphroot/d" $storage_file + echo "run_root = \"$main_run_root}\"" >>$config_file + echo "data_root = \"$main_data_root\"" >>$config_file + eval "sed -i '/\[storage\]/a\runroot=\"$storage_run_root\"' $storage_file" + eval "sed -i '/\[storage\]/a\graphroot=\"$storage_data_root\"' $storage_file" + + check_root "$main_run_root" "$main_data_root" +} + +# run command and check its result +# $1 (run root) +# $1 (data root) +function check_root() { + local -r run_root="$1" + local -r data_root="$2" + + start_time=$(date '+%Y-%m-%d %H:%M:%S') + systemctl restart isula-build + + run_check_result "journalctl -u isula-build --since \"$start_time\" --no-pager | grep $run_root" 0 + run_check_result "journalctl -u isula-build --since \"$start_time\" --no-pager | grep $data_root" 0 +} + +pre_test +main_set_storage_not_set +main_not_set_storage_set +main_set_storage_set +clean +exit "$exit_flag" diff --git a/util/common.go b/util/common.go index ff85da9c468a4667142ba3a3ab3f205bad548a1c..42d81b8b5aaff0ea277efc2b65e46e85fca28235 100644 --- a/util/common.go +++ b/util/common.go @@ -29,10 +29,7 @@ import ( constant "isula.org/isula-build" ) -const ( - maxServerNameLength = 255 - maxLoadFileSize = 50 * 1024 * 1024 * 1024 -) +const maxServerNameLength = 255 // CopyMapStringString copies all KVs in a map[string]string to a new map func CopyMapStringString(m map[string]string) map[string]string { @@ -94,44 +91,21 @@ func SetUmask() bool { return unix.Umask(wanted) == wanted } -// CheckFileSize check whether the file size exceeds limit -func CheckFileSize(path string, sizeLimit int64) error { - filename := filepath.Base(path) +// CheckFileInfoAndSize check whether the file exists, is regular file, and if its size exceeds limit +func CheckFileInfoAndSize(path string, sizeLimit int64) error { f, err := os.Stat(filepath.Clean(path)) - // file not exist, file size check ok - if os.IsNotExist(err) { - return nil - } if err != nil { - return errors.Errorf("stat file %v err: %v", filename, err) - } - if f.IsDir() { - return errors.Errorf("file %s is a directory", filename) + return err } - if f.Size() > sizeLimit { - return errors.Errorf("file %v size is: %v, exceeds limit %v", filename, f.Size(), sizeLimit) + if !f.Mode().IsRegular() { + return errors.Errorf("file %s should be a regular file", f.Name()) } - return nil -} - -// CheckLoadFile checks the file which will be loaded -func CheckLoadFile(path string) error { - fi, err := os.Stat(path) - if err != nil { - return errors.Wrapf(err, "stat %q failed", path) - } - - if !fi.Mode().IsRegular() { - return errors.Errorf("loading file %s should be a regular file", fi.Name()) + if f.Size() == 0 { + return errors.Errorf("file %s is empty", f.Name()) } - - if fi.Size() == 0 { - return errors.New("loading file is empty") - } - - if fi.Size() > maxLoadFileSize { - return errors.Errorf("file %s size is: %v, exceeds limit %v", fi.Name(), fi.Size(), maxLoadFileSize) + if f.Size() > sizeLimit { + return errors.Errorf("file %s size is: %d, exceeds limit %d", f.Name(), f.Size(), sizeLimit) } return nil diff --git a/util/common_test.go b/util/common_test.go index 9831971a0e415fa4041e0df96dc3d8b6dbd870d2..7d2444a60032125d133954352aed7db6666ac7c0 100644 --- a/util/common_test.go +++ b/util/common_test.go @@ -14,17 +14,14 @@ package util import ( - "io/ioutil" - "os" "path/filepath" "testing" "gotest.tools/v3/assert" "gotest.tools/v3/fs" - constant "isula.org/isula-build" ) -func TestCheckFileSize(t *testing.T) { +func TestCheckFileInfoAndSize(t *testing.T) { content := ` ARG testArg ARG testArg2 @@ -48,6 +45,12 @@ func TestCheckFileSize(t *testing.T) { ctxDir: fs.NewDir(t, t.Name(), fs.WithFile("Dockerfile", content)), sizeLimit: 200, }, + { + name: "empty file", + ctxDir: fs.NewDir(t, t.Name(), fs.WithFile("Dockerfile", "")), + isErr: true, + errStr: "is empty", + }, { name: "exceeds limit file", ctxDir: fs.NewDir(t, t.Name(), fs.WithFile("Dockerfile", content)), @@ -60,12 +63,7 @@ func TestCheckFileSize(t *testing.T) { ctxDir: fs.NewDir(t, t.Name(), fs.WithFile("Dockerfile", content)), isDir: true, isErr: true, - errStr: "is a directory", - }, - { - name: "not exist file", - ctxDir: fs.NewDir(t, t.Name()), - sizeLimit: 5, + errStr: "should be a regular file", }, { name: "exceeds limit directory", @@ -83,7 +81,7 @@ func TestCheckFileSize(t *testing.T) { if !c.isDir { path = filepath.Join(path, "Dockerfile") } - err := CheckFileSize(path, c.sizeLimit) + err := CheckFileInfoAndSize(path, c.sizeLimit) assert.Equal(t, err != nil, c.isErr) if c.isErr { assert.ErrorContains(t, err, c.errStr) @@ -250,63 +248,3 @@ func TestAnyFlagSet(t *testing.T) { }) } } - -func TestCheckLoadFile(t *testing.T) { - loadFile := fs.NewFile(t, t.Name()) - defer loadFile.Remove() - err := ioutil.WriteFile(loadFile.Path(), []byte("hello"), constant.DefaultRootFileMode) - assert.NilError(t, err) - - emptyFile := fs.NewFile(t, t.Name()) - defer emptyFile.Remove() - - root := fs.NewDir(t, t.Name()) - defer root.Remove() - - bigFile := filepath.Join(root.Path(), "bigFile") - f, err := os.Create(bigFile) - assert.NilError(t, err) - defer os.Remove(f.Name()) - err = f.Truncate(maxLoadFileSize + 1) - assert.NilError(t, err) - - type args struct { - path string - } - tests := []struct { - name string - args args - wantErr bool - }{ - { - name: "TC-normal load file", - args: args{path: loadFile.Path()}, - }, - { - name: "TC-load file not exist", - wantErr: true, - }, - { - name: "TC-empty load file", - args: args{path: emptyFile.Path()}, - wantErr: true, - }, - { - name: "TC-invalid load file", - args: args{path: "/dev/cdrom"}, - wantErr: true, - }, - { - name: "TC-load file too big", - args: args{path: bigFile}, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := CheckLoadFile(tt.args.path); (err != nil) != tt.wantErr { - t.Errorf("CheckLoadFile() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} diff --git a/util/file.go b/util/file.go index e0353898553e279836ac66f89117f7381d7430e7..07123ce04bf5e58232ed2721bd82cc5d3107e467 100644 --- a/util/file.go +++ b/util/file.go @@ -23,10 +23,8 @@ import ( "github.com/containers/storage/pkg/archive" "github.com/pkg/errors" -) -const ( - fileMaxSize = 10 * 1024 * 1024 // 10MB + constant "isula.org/isula-build" ) var ( @@ -34,33 +32,18 @@ var ( accessTime = time.Date(2017, time.January, 0, 0, 0, 0, 0, time.UTC) ) -// ReadSmallFile read small file less than 10MB -func ReadSmallFile(path string) ([]byte, error) { - st, err := os.Lstat(path) +// LoadJSONFile load json files and store it into v +func LoadJSONFile(file string, v interface{}) error { + err := CheckFileInfoAndSize(file, constant.JSONMaxFileSize) if err != nil { - return nil, err - } - - if !st.Mode().IsRegular() { - return nil, errors.Errorf("loading file %s should be a regular file", st.Name()) - } - - if st.Size() == 0 { - return nil, errors.New("loading file is empty") - } - - if st.Size() > fileMaxSize { - return nil, errors.Errorf("file %q too big", path) + return err } - return ioutil.ReadFile(path) // nolint: gosec -} -// LoadJSONFile load json files and store it into v -func LoadJSONFile(file string, v interface{}) error { - f, err := ReadSmallFile(file) + f, err := ioutil.ReadFile(file) // nolint: gosec if err != nil { return err } + return json.Unmarshal(f, v) } diff --git a/util/file_test.go b/util/file_test.go index 09aed41df93baf3ecb1fb2f68208173061fa145d..b23b474a786550f353b327feddce87fa7190cfe4 100644 --- a/util/file_test.go +++ b/util/file_test.go @@ -18,7 +18,6 @@ import ( "io/ioutil" "os" "path/filepath" - "reflect" "testing" "time" @@ -28,73 +27,6 @@ import ( constant "isula.org/isula-build" ) -func TestReadSmallFile(t *testing.T) { - smallFile := fs.NewFile(t, t.Name()) - defer smallFile.Remove() - err := ioutil.WriteFile(smallFile.Path(), []byte("small file"), constant.DefaultRootFileMode) - assert.NilError(t, err) - - root := fs.NewDir(t, t.Name()) - defer root.Remove() - - bigFile := filepath.Join(root.Path(), "bigFile") - f, err := os.Create(bigFile) - assert.NilError(t, err) - defer os.Remove(f.Name()) - err = f.Truncate(fileMaxSize + 1) - assert.NilError(t, err) - - emptyFile := fs.NewFile(t, t.Name()) - defer emptyFile.Remove() - - type args struct { - path string - } - tests := []struct { - name string - args args - want []byte - wantErr bool - }{ - { - name: "TC-normal read", - args: args{path: smallFile.Path()}, - want: []byte("small file"), - }, - { - name: "TC-not exist path", - wantErr: true, - }, - { - name: "TC-file too big", - args: args{path: bigFile}, - wantErr: true, - }, - { - name: "TC-empty file", - args: args{path: emptyFile.Path()}, - wantErr: true, - }, - { - name: "TC-invalid file", - args: args{path: "/dev/cdrom"}, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := ReadSmallFile(tt.args.path) - if (err != nil) != tt.wantErr { - t.Errorf("ReadSmallFile() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("ReadSmallFile() = %v, want %v", got, tt.want) - } - }) - } -} - func TestLoadJSONFile(t *testing.T) { type rename struct { Name string `json:"name"` diff --git a/util/user.go b/util/user.go index ce398ff711b7001b19f1b30784da51007568ee15..5bf12ef4c73a40190e41d3fb9154b00809bfc3fe 100644 --- a/util/user.go +++ b/util/user.go @@ -76,7 +76,7 @@ func GetChownOptions(chown, mountpoint string) (idtools.IDPair, error) { // searchUserGroup searches user in etc/passwd and group in etc/group // function caller should make sure the path is clean func searchUserGroup(name, path string, userFlag bool) (int, error) { - if err := CheckFileSize(path, constant.MaxFileSize); err != nil { + if err := CheckFileInfoAndSize(path, constant.MaxFileSize); err != nil { return 0, err } f, err := os.Open(path) // nolint:gosec