syz-manager: add concept of partial configs

syz-ci uses partial (incomplete) manager config in several places.
Currently it is implemented in some ugly way.
Provide better support and unexport DefaultValues and SplitTarget.

Update #501

4 files changed, 68 insertions, 59 deletions

diff --git a/syz-ci/jobs.go b/syz-ci/jobs.go
index d3df278d9..e4518f897 100644
--- a/syz-ci/jobs.go
+++ b/syz-ci/jobs.go
@@ -11,7 +11,6 @@ import (
 	"time"
 
 	"github.com/google/syzkaller/dashboard/dashapi"
-	"github.com/google/syzkaller/pkg/config"
 	"github.com/google/syzkaller/pkg/csource"
 	"github.com/google/syzkaller/pkg/git"
 	"github.com/google/syzkaller/pkg/kernel"
@@ -279,15 +278,10 @@ func (jp *JobProcessor) buildImage(job *Job) error {
 	mgrcfg.Syzkaller = syzkallerDir
 	mgrcfg.Image = image
 	mgrcfg.SSHKey = key
-
-	// Reload config to fill derived fields (ugly hack).
-	cfgdata, err := config.SaveData(mgrcfg)
-	if err != nil {
-		return fmt.Errorf("failed to save manager config: %v", err)
-	}
-	if job.mgrcfg, err = mgrconfig.LoadData(cfgdata); err != nil {
-		return fmt.Errorf("failed to reload manager config: %v", err)
+	if err := mgrconfig.Complete(mgrcfg); err != nil {
+		return fmt.Errorf("bad manager config: %v", err)
 	}
+	job.mgrcfg = mgrcfg
 	return nil
 }
 
diff --git a/syz-ci/manager.go b/syz-ci/manager.go
index db20b37bb..fda534940 100644
--- a/syz-ci/manager.go
+++ b/syz-ci/manager.go
@@ -88,11 +88,7 @@ func createManager(cfg *Config, mgrcfg *ManagerConfig, stop chan struct{}) *Mana
 	}
 
 	// Prepare manager config skeleton (other fields are filled in writeConfig).
-	managercfg := mgrconfig.DefaultValues()
-	if err := config.LoadData(mgrcfg.ManagerConfig, managercfg); err != nil {
-		log.Fatalf("failed to load manager %v config: %v", mgrcfg.Name, err)
-	}
-	managercfg.TargetOS, managercfg.TargetVMArch, managercfg.TargetArch, err = mgrconfig.SplitTarget(managercfg.Target)
+	managercfg, err := mgrconfig.LoadPartialData(mgrcfg.ManagerConfig)
 	if err != nil {
 		log.Fatalf("failed to load manager %v config: %v", mgrcfg.Name, err)
 	}
@@ -423,13 +419,8 @@ func (mgr *Manager) createTestConfig(imageDir string, info *BuildInfo) (*mgrconf
 	mgrcfg.SSHKey = filepath.Join(imageDir, "key")
 	mgrcfg.KernelSrc = mgr.kernelDir
 	mgrcfg.Syzkaller = filepath.FromSlash("syzkaller/current")
-	cfgdata, err := config.SaveData(mgrcfg)
-	if err != nil {
-		return nil, fmt.Errorf("failed to save manager config: %v", err)
-	}
-	mgrcfg, err = mgrconfig.LoadData(cfgdata)
-	if err != nil {
-		return nil, fmt.Errorf("failed to reload manager config: %v", err)
+	if err := mgrconfig.Complete(mgrcfg); err != nil {
+		return nil, fmt.Errorf("bad manager config: %v", err)
 	}
 	return mgrcfg, nil
 }
diff --git a/syz-ci/syzupdater.go b/syz-ci/syzupdater.go
index 134c044fa..4df6337ff 100644
--- a/syz-ci/syzupdater.go
+++ b/syz-ci/syzupdater.go
@@ -12,7 +12,6 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/google/syzkaller/pkg/config"
 	"github.com/google/syzkaller/pkg/git"
 	"github.com/google/syzkaller/pkg/log"
 	"github.com/google/syzkaller/pkg/osutil"
@@ -73,14 +72,11 @@ func NewSyzUpdater(cfg *Config) *SyzUpdater {
 	}
 	targets := make(map[string]bool)
 	for _, mgr := range cfg.Managers {
-		mgrcfg := new(mgrconfig.Config)
-		if err := config.LoadData(mgr.ManagerConfig, mgrcfg); err != nil {
-			log.Fatalf("failed to load manager %v config: %v", mgr.Name, err)
-		}
-		os, vmarch, arch, err := mgrconfig.SplitTarget(mgrcfg.Target)
+		mgrcfg, err := mgrconfig.LoadPartialData(mgr.ManagerConfig)
 		if err != nil {
 			log.Fatalf("failed to load manager %v config: %v", mgr.Name, err)
 		}
+		os, vmarch, arch := mgrcfg.TargetOS, mgrcfg.TargetVMArch, mgrcfg.TargetArch
 		targets[os+"/"+vmarch+"/"+arch] = true
 		files[fmt.Sprintf("bin/%v_%v/syz-fuzzer", os, vmarch)] = true
 		files[fmt.Sprintf("bin/%v_%v/syz-execprog", os, vmarch)] = true
diff --git a/syz-manager/mgrconfig/mgrconfig.go b/syz-manager/mgrconfig/mgrconfig.go
index 2a1b7bf8a..bc8c5eafa 100644
--- a/syz-manager/mgrconfig/mgrconfig.go
+++ b/syz-manager/mgrconfig/mgrconfig.go
@@ -97,14 +97,44 @@ type Config struct {
 }
 
 func LoadData(data []byte) (*Config, error) {
-	return load(data, "")
+	cfg, err := LoadPartialData(data)
+	if err != nil {
+		return nil, err
+	}
+	if err := Complete(cfg); err != nil {
+		return nil, err
+	}
+	return cfg, nil
 }
 
 func LoadFile(filename string) (*Config, error) {
-	return load(nil, filename)
+	cfg, err := LoadPartialFile(filename)
+	if err != nil {
+		return nil, err
+	}
+	if err := Complete(cfg); err != nil {
+		return nil, err
+	}
+	return cfg, nil
+}
+
+func LoadPartialData(data []byte) (*Config, error) {
+	cfg := defaultValues()
+	if err := config.LoadData(data, cfg); err != nil {
+		return nil, err
+	}
+	return loadPartial(cfg)
+}
+
+func LoadPartialFile(filename string) (*Config, error) {
+	cfg := defaultValues()
+	if err := config.LoadFile(filename, cfg); err != nil {
+		return nil, err
+	}
+	return loadPartial(cfg)
 }
 
-func DefaultValues() *Config {
+func defaultValues() *Config {
 	return &Config{
 		SSHUser:   "root",
 		Cover:     true,
@@ -115,24 +145,12 @@ func DefaultValues() *Config {
 	}
 }
 
-func load(data []byte, filename string) (*Config, error) {
-	cfg := DefaultValues()
-	if data != nil {
-		if err := config.LoadData(data, cfg); err != nil {
-			return nil, err
-		}
-	} else {
-		if err := config.LoadFile(filename, cfg); err != nil {
-			return nil, err
-		}
-	}
-
+func loadPartial(cfg *Config) (*Config, error) {
 	var err error
-	cfg.TargetOS, cfg.TargetVMArch, cfg.TargetArch, err = SplitTarget(cfg.Target)
+	cfg.TargetOS, cfg.TargetVMArch, cfg.TargetArch, err = splitTarget(cfg.Target)
 	if err != nil {
 		return nil, err
 	}
-
 	targetBin := func(name, arch string) string {
 		exe := ""
 		if cfg.TargetOS == "windows" {
@@ -143,39 +161,49 @@ func load(data []byte, filename string) (*Config, error) {
 	cfg.SyzFuzzerBin = targetBin("syz-fuzzer", cfg.TargetVMArch)
 	cfg.SyzExecprogBin = targetBin("syz-execprog", cfg.TargetVMArch)
 	cfg.SyzExecutorBin = targetBin("syz-executor", cfg.TargetArch)
+	return cfg, nil
+}
+
+func Complete(cfg *Config) error {
+	if cfg.TargetOS == "" || cfg.TargetVMArch == "" || cfg.TargetArch == "" {
+		return fmt.Errorf("target parameters are not filled in")
+	}
+	if cfg.SSHUser == "" {
+		return fmt.Errorf("bad config syzkaller param: ssh user is empty")
+	}
 	if !osutil.IsExist(cfg.SyzFuzzerBin) {
-		return nil, fmt.Errorf("bad config syzkaller param: can't find %v", cfg.SyzFuzzerBin)
+		return fmt.Errorf("bad config syzkaller param: can't find %v", cfg.SyzFuzzerBin)
 	}
 	if !osutil.IsExist(cfg.SyzExecprogBin) {
-		return nil, fmt.Errorf("bad config syzkaller param: can't find %v", cfg.SyzExecprogBin)
+		return fmt.Errorf("bad config syzkaller param: can't find %v", cfg.SyzExecprogBin)
 	}
 	if !osutil.IsExist(cfg.SyzExecutorBin) {
-		return nil, fmt.Errorf("bad config syzkaller param: can't find %v", cfg.SyzExecutorBin)
+		return fmt.Errorf("bad config syzkaller param: can't find %v", cfg.SyzExecutorBin)
 	}
 	if cfg.HTTP == "" {
-		return nil, fmt.Errorf("config param http is empty")
+		return fmt.Errorf("config param http is empty")
 	}
 	if cfg.Workdir == "" {
-		return nil, fmt.Errorf("config param workdir is empty")
+		return fmt.Errorf("config param workdir is empty")
 	}
 	if cfg.Type == "" {
-		return nil, fmt.Errorf("config param type is empty")
+		return fmt.Errorf("config param type is empty")
 	}
 	if cfg.Procs < 1 || cfg.Procs > 32 {
-		return nil, fmt.Errorf("bad config param procs: '%v', want [1, 32]", cfg.Procs)
+		return fmt.Errorf("bad config param procs: '%v', want [1, 32]", cfg.Procs)
 	}
 	switch cfg.Sandbox {
 	case "none", "setuid", "namespace":
 	default:
-		return nil, fmt.Errorf("config param sandbox must contain one of none/setuid/namespace")
+		return fmt.Errorf("config param sandbox must contain one of none/setuid/namespace")
 	}
 	if cfg.SSHKey != "" {
 		info, err := os.Stat(cfg.SSHKey)
 		if err != nil {
-			return nil, err
+			return err
 		}
 		if info.Mode()&0077 != 0 {
-			return nil, fmt.Errorf("sshkey %v is unprotected, ssh will reject it, do chmod 0600 on it", cfg.SSHKey)
+			return fmt.Errorf("sshkey %v is unprotected, ssh will reject it, do chmod 0600", cfg.SSHKey)
 		}
 	}
 
@@ -187,22 +215,22 @@ func load(data []byte, filename string) (*Config, error) {
 	}
 
 	if err := parseSuppressions(cfg); err != nil {
-		return nil, err
+		return err
 	}
 
 	if cfg.HubClient != "" && (cfg.Name == "" || cfg.HubAddr == "" || cfg.HubKey == "") {
-		return nil, fmt.Errorf("hub_client is set, but name/hub_addr/hub_key is empty")
+		return fmt.Errorf("hub_client is set, but name/hub_addr/hub_key is empty")
 	}
 	if cfg.DashboardClient != "" && (cfg.Name == "" ||
 		cfg.DashboardAddr == "" ||
 		cfg.DashboardKey == "") {
-		return nil, fmt.Errorf("dashboard_client is set, but name/dashboard_addr/dashboard_key is empty")
+		return fmt.Errorf("dashboard_client is set, but name/dashboard_addr/dashboard_key is empty")
 	}
 
-	return cfg, nil
+	return nil
 }
 
-func SplitTarget(target string) (string, string, string, error) {
+func splitTarget(target string) (string, string, string, error) {
 	if target == "" {
 		return "", "", "", fmt.Errorf("target is empty")
 	}