From 91154fa55c7fb6c4b5df4524d585b9fa875e3459 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 31 Jul 2018 18:36:52 +0200 Subject: dashboard/app: refactor config checking Split a very long function into several smaller functions. Update #538 --- dashboard/app/config.go | 148 +++++++++++++++++++++++++----------------------- 1 file changed, 76 insertions(+), 72 deletions(-) diff --git a/dashboard/app/config.go b/dashboard/app/config.go index c4d702d08..8c60e78b9 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -140,6 +140,13 @@ func (cfg *Config) ReportingByName(name string) *Reporting { // or from a separate file that provides actual production config. var config *GlobalConfig +func init() { + // Prevents gometalinter from considering everything as dead code. + if false { + installConfig(nil) + } +} + func installConfig(cfg *GlobalConfig) { if config != nil { panic("another config is already installed") @@ -156,74 +163,7 @@ func installConfig(cfg *GlobalConfig) { checkClients(clientNames, cfg.Clients) checkConfigAccessLevel(&cfg.AccessLevel, AccessPublic, "global") for ns, cfg := range cfg.Namespaces { - if ns == "" { - panic("empty namespace name") - } - if namespaces[ns] { - panic(fmt.Sprintf("duplicate namespace %q", ns)) - } - namespaces[ns] = true - if cfg.DisplayTitle == "" { - cfg.DisplayTitle = ns - } - checkClients(clientNames, cfg.Clients) - for name, mgr := range cfg.Managers { - if mgr.Decommissioned && mgr.DelegatedTo == "" { - panic(fmt.Sprintf("decommissioned manager %v/%v does not have delegate", - ns, name)) - } - if !mgr.Decommissioned && mgr.DelegatedTo != "" { - panic(fmt.Sprintf("non-decommissioned manager %v/%v has delegate", - ns, name)) - } - if mgr.RestrictedTestingRepo != "" && mgr.RestrictedTestingReason == "" { - panic(fmt.Sprintf("restricted manager %v/%v does not have restriction reason", - ns, name)) - } - if mgr.RestrictedTestingRepo == "" && mgr.RestrictedTestingReason != "" { - panic(fmt.Sprintf("unrestricted manager %v/%v has restriction reason", - ns, name)) - } - } - if !clientKeyRe.MatchString(cfg.Key) { - panic(fmt.Sprintf("bad namespace %q key: %q", ns, cfg.Key)) - } - if len(cfg.Reporting) == 0 { - panic(fmt.Sprintf("no reporting in namespace %q", ns)) - } - checkConfigAccessLevel(&cfg.AccessLevel, cfg.AccessLevel, fmt.Sprintf("namespace %q", ns)) - parentAccessLevel := cfg.AccessLevel - reportingNames := make(map[string]bool) - // Go backwards because access levels get stricter backwards. - for ri := len(cfg.Reporting) - 1; ri >= 0; ri-- { - reporting := &cfg.Reporting[ri] - if reporting.Name == "" { - panic(fmt.Sprintf("empty reporting name in namespace %q", ns)) - } - if reportingNames[reporting.Name] { - panic(fmt.Sprintf("duplicate reporting name %q", reporting.Name)) - } - if reporting.DisplayTitle == "" { - reporting.DisplayTitle = reporting.Name - } - checkConfigAccessLevel(&reporting.AccessLevel, parentAccessLevel, - fmt.Sprintf("reporting %q/%q", ns, reporting.Name)) - parentAccessLevel = reporting.AccessLevel - if reporting.Filter == nil { - reporting.Filter = func(bug *Bug) FilterResult { return FilterReport } - } - reportingNames[reporting.Name] = true - if reporting.Config.Type() == "" { - panic(fmt.Sprintf("empty reporting type for %q", reporting.Name)) - } - if err := reporting.Config.Validate(); err != nil { - panic(err) - } - if _, err := json.Marshal(reporting.Config); err != nil { - panic(fmt.Sprintf("failed to json marshal %q config: %v", - reporting.Name, err)) - } - } + checkNamespace(ns, cfg, namespaces, clientNames) } for repo, info := range cfg.KernelRepos { if info.Alias == "" { @@ -239,10 +179,74 @@ func installConfig(cfg *GlobalConfig) { initAPIHandlers() } -func init() { - // Prevents gometalinter from considering everything as dead code. - if false { - installConfig(nil) +func checkNamespace(ns string, cfg *Config, namespaces, clientNames map[string]bool) { + if ns == "" { + panic("empty namespace name") + } + if namespaces[ns] { + panic(fmt.Sprintf("duplicate namespace %q", ns)) + } + namespaces[ns] = true + if cfg.DisplayTitle == "" { + cfg.DisplayTitle = ns + } + checkClients(clientNames, cfg.Clients) + for name, mgr := range cfg.Managers { + checkManager(ns, name, mgr) + } + if !clientKeyRe.MatchString(cfg.Key) { + panic(fmt.Sprintf("bad namespace %q key: %q", ns, cfg.Key)) + } + if len(cfg.Reporting) == 0 { + panic(fmt.Sprintf("no reporting in namespace %q", ns)) + } + checkConfigAccessLevel(&cfg.AccessLevel, cfg.AccessLevel, fmt.Sprintf("namespace %q", ns)) + parentAccessLevel := cfg.AccessLevel + reportingNames := make(map[string]bool) + // Go backwards because access levels get stricter backwards. + for ri := len(cfg.Reporting) - 1; ri >= 0; ri-- { + reporting := &cfg.Reporting[ri] + if reporting.Name == "" { + panic(fmt.Sprintf("empty reporting name in namespace %q", ns)) + } + if reportingNames[reporting.Name] { + panic(fmt.Sprintf("duplicate reporting name %q", reporting.Name)) + } + if reporting.DisplayTitle == "" { + reporting.DisplayTitle = reporting.Name + } + checkConfigAccessLevel(&reporting.AccessLevel, parentAccessLevel, + fmt.Sprintf("reporting %q/%q", ns, reporting.Name)) + parentAccessLevel = reporting.AccessLevel + if reporting.Filter == nil { + reporting.Filter = func(bug *Bug) FilterResult { return FilterReport } + } + reportingNames[reporting.Name] = true + if reporting.Config.Type() == "" { + panic(fmt.Sprintf("empty reporting type for %q", reporting.Name)) + } + if err := reporting.Config.Validate(); err != nil { + panic(err) + } + if _, err := json.Marshal(reporting.Config); err != nil { + panic(fmt.Sprintf("failed to json marshal %q config: %v", + reporting.Name, err)) + } + } +} + +func checkManager(ns, name string, mgr ConfigManager) { + if mgr.Decommissioned && mgr.DelegatedTo == "" { + panic(fmt.Sprintf("decommissioned manager %v/%v does not have delegate", ns, name)) + } + if !mgr.Decommissioned && mgr.DelegatedTo != "" { + panic(fmt.Sprintf("non-decommissioned manager %v/%v has delegate", ns, name)) + } + if mgr.RestrictedTestingRepo != "" && mgr.RestrictedTestingReason == "" { + panic(fmt.Sprintf("restricted manager %v/%v does not have restriction reason", ns, name)) + } + if mgr.RestrictedTestingRepo == "" && mgr.RestrictedTestingReason != "" { + panic(fmt.Sprintf("unrestricted manager %v/%v has restriction reason", ns, name)) } } -- cgit mrf-deployment