diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000000..47118873be --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,15 @@ +# see https://golangci-lint.run/usage/configuration/ for documentation +linters: + enable: + - forbidigo +linters-settings: + forbidigo: + # Forbid the following identifiers (list of regexp) + # + # Add a comment to the offending line with '//nolint:forbidigo' to suppress the rule if + # you're working inside the main() function. + forbid: + # These APIs exit the process. These are OK to use in the main() function, + # and not ANYWHERE else. + - 'os\.Exit(# Do not use except for inside main(). Suppress this message with //nolint:forbidigo at the end of the line if it is correct.)?' + - 'log\.Fatal(f|ln)?(# Do not use except for inside main(). Suppress this message with //nolint:forbidigo at the end of the line if it is correct.)?' \ No newline at end of file diff --git a/cmd/appcore-async/main.go b/cmd/appcore-async/main.go index 8aac81f5fa..afad14d985 100644 --- a/cmd/appcore-async/main.go +++ b/cmd/appcore-async/main.go @@ -32,19 +32,19 @@ func main() { defaultConfig := fmt.Sprintf("radius-%s.yaml", hostoptions.Environment()) flag.StringVar(&configFile, "config-file", defaultConfig, "The service configuration file.") if configFile == "" { - log.Fatal("config-file is empty.") + log.Fatal("config-file is empty.") //nolint:forbidigo // this is OK inside the main function. } flag.Parse() options, err := hostoptions.NewHostOptionsFromEnvironment(configFile) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } logger, flush, err := ucplog.NewLogger(logging.AppCoreLoggerName, &options.Config.Logging) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } defer flush() @@ -95,7 +95,7 @@ func main() { // gracefully, so just crash if that happens. err = <-stopped if err == nil { - os.Exit(0) + os.Exit(0) //nolint:forbidigo // this is OK inside the main function. } else { panic(err) } diff --git a/cmd/appcore-rp/main.go b/cmd/appcore-rp/main.go index b495e03f09..1812f52458 100644 --- a/cmd/appcore-rp/main.go +++ b/cmd/appcore-rp/main.go @@ -34,18 +34,18 @@ import ( const serviceName = "applications.core" -func newLinkHosts(configFile string, enableAsyncWorker bool) ([]hosting.Service, *hostoptions.HostOptions) { +func newLinkHosts(configFile string, enableAsyncWorker bool) ([]hosting.Service, *hostoptions.HostOptions, error) { hostings := []hosting.Service{} options, err := hostoptions.NewHostOptionsFromEnvironment(configFile) if err != nil { - log.Fatal(err) + return nil, nil, err } hostings = append(hostings, link_frontend.NewService(options)) if enableAsyncWorker { hostings = append(hostings, link_backend.NewService(options)) } - return hostings, &options + return hostings, &options, nil } func main() { @@ -64,14 +64,14 @@ func main() { flag.StringVar(&linkConfigFile, "link-config", defaultLinkConfig, "The service configuration file for Applications.Link.") if configFile == "" { - log.Fatal("config-file is empty.") + log.Fatal("config-file is empty.") //nolint:forbidigo // this is OK inside the main function. } flag.Parse() options, err := hostoptions.NewHostOptionsFromEnvironment(configFile) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } hostingSvc := []hosting.Service{frontend.NewService(options)} @@ -88,7 +88,7 @@ func main() { logger, flush, err := ucplog.NewLogger(logging.AppCoreLoggerName, &options.Config.Logging) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } defer flush() @@ -102,7 +102,11 @@ func main() { if runLink && linkConfigFile != "" { logger.Info("Run Applications.Link.") var linkSvcs []hosting.Service - linkSvcs, linkOpts = newLinkHosts(linkConfigFile, enableAsyncWorker) + var err error + linkSvcs, linkOpts, err = newLinkHosts(linkConfigFile, enableAsyncWorker) + if err != nil { + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. + } hostingSvc = append(hostingSvc, linkSvcs...) } @@ -136,11 +140,11 @@ func main() { tracerOpts.ServiceName = serviceName shutdown, err := trace.InitTracer(tracerOpts) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } defer func() { if err := shutdown(ctx); err != nil { - log.Fatal("failed to shutdown TracerProvider: %w", err) + log.Printf("failed to shutdown TracerProvider: %v\n", err) } }() @@ -165,7 +169,7 @@ func main() { // gracefully, so just crash if that happens. err = <-stopped if err == nil { - os.Exit(0) + os.Exit(0) //nolint:forbidigo // this is OK inside the main function. } else { panic(err) } diff --git a/cmd/applink-rp/main.go b/cmd/applink-rp/main.go index 0d0867ee11..535f2310a8 100644 --- a/cmd/applink-rp/main.go +++ b/cmd/applink-rp/main.go @@ -40,14 +40,14 @@ func main() { flag.BoolVar(&enableAsyncWorker, "enable-asyncworker", true, "Flag to run async request process worker (for private preview and dev/test purpose).") if configFile == "" { - log.Fatal("config-file is empty.") + log.Fatal("config-file is empty.") //nolint:forbidigo // this is OK inside the main function. } flag.Parse() options, err := hostoptions.NewHostOptionsFromEnvironment(configFile) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } hostingSvc := []hosting.Service{frontend.NewService(options)} @@ -64,7 +64,7 @@ func main() { logger, flush, err := ucplog.NewLogger(logging.AppLinkLoggerName, &options.Config.Logging) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } defer flush() @@ -98,11 +98,11 @@ func main() { tracerOpts.ServiceName = serviceName shutdown, err := trace.InitTracer(tracerOpts) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } defer func() { if err := shutdown(ctx); err != nil { - log.Fatal("failed to shutdown TracerProvider: %w", err) + log.Printf("failed to shutdown TracerProvider: %v\n", err) } }() @@ -127,7 +127,7 @@ func main() { // gracefully, so just crash if that happens. err = <-stopped if err == nil { - os.Exit(0) + os.Exit(0) //nolint:forbidigo // this is OK inside the main function. } else { panic(err) } diff --git a/cmd/docgen/main.go b/cmd/docgen/main.go index 2257fe2805..c2633cd211 100644 --- a/cmd/docgen/main.go +++ b/cmd/docgen/main.go @@ -20,7 +20,7 @@ import ( func main() { if len(os.Args) != 2 { - log.Fatal("usage: go run cmd/docgen/main.go ") + log.Fatal("usage: go run cmd/docgen/main.go ") //nolint:forbidigo // this is OK inside the main function. } output := os.Args[1] @@ -28,15 +28,15 @@ func main() { if os.IsNotExist(err) { err = os.Mkdir(output, 0755) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } } else if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } err = doc.GenMarkdownTreeCustom(cmd.RootCmd, output, frontmatter, link) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } } diff --git a/cmd/rad/cmd/root.go b/cmd/rad/cmd/root.go index afa63e58cd..10871abb91 100644 --- a/cmd/rad/cmd/root.go +++ b/cmd/rad/cmd/root.go @@ -10,7 +10,6 @@ import ( "encoding/json" "errors" "fmt" - "log" "os" "strings" @@ -109,15 +108,19 @@ func prettyPrintJSON(o any) (string, error) { // Execute adds all child commands to the root command and sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. // It also initializes the tracerprovider for cli. -func Execute() { +// +// Execute returns true +func Execute() error { ctx := context.WithValue(context.Background(), ConfigHolderKey, ConfigHolder) shutdown, err := trace.InitTracer(trace.Options{ ServiceName: serviceName, }) if err != nil { - log.Fatal(err) + fmt.Println("Error:", err) + return err } + defer func() { _ = shutdown(ctx) }() @@ -130,12 +133,14 @@ func Execute() { if errors.Is(&cli.FriendlyError{}, err) { fmt.Println(err.Error()) fmt.Println("\nTraceId: ", span.SpanContext().TraceID().String()) - os.Exit(1) + return err } else if err != nil { fmt.Println("Error:", prettyPrintRPError(err)) fmt.Println("\nTraceId: ", span.SpanContext().TraceID().String()) - os.Exit(1) + return err } + + return nil } func init() { @@ -262,7 +267,7 @@ func initConfig() { v, err := cli.LoadConfig(ConfigHolder.ConfigFilePath) if err != nil { fmt.Printf("Error: failed to load config: %v\n", err) - os.Exit(1) + os.Exit(1) //nolint:forbidigo // this is OK inside the CLI startup. } ConfigHolder.Config = v @@ -270,13 +275,13 @@ func initConfig() { wd, err := os.Getwd() if err != nil { fmt.Printf("Error: failed to find current working directory: %v\n", err) - os.Exit(1) + os.Exit(1) //nolint:forbidigo // this is OK inside the CLI startup. } dc, err := config.LoadDirectoryConfig(wd) if err != nil { fmt.Printf("Error: failed to load config: %v\n", err) - os.Exit(1) + os.Exit(1) //nolint:forbidigo // this is OK inside the CLI startup. } ConfigHolder.DirectoryConfig = dc diff --git a/cmd/rad/main.go b/cmd/rad/main.go index 3b6aafa893..37cd1a921f 100644 --- a/cmd/rad/main.go +++ b/cmd/rad/main.go @@ -6,9 +6,14 @@ package main import ( + "os" + "github.com/project-radius/radius/cmd/rad/cmd" ) func main() { - cmd.Execute() + err := cmd.Execute() + if err != nil { + os.Exit(1) //nolint:forbidigo // this is OK inside the main function. + } } diff --git a/cmd/ucpd/cmd/root.go b/cmd/ucpd/cmd/root.go index efa6d87cec..ba6945508d 100644 --- a/cmd/ucpd/cmd/root.go +++ b/cmd/ucpd/cmd/root.go @@ -34,7 +34,7 @@ var rootCmd = &cobra.Command{ logger, flush, err := ucplog.NewLogger(ucplog.LoggerName, &options.LoggingOptions) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } defer flush() @@ -58,11 +58,11 @@ var rootCmd = &cobra.Command{ options.TracerProviderOptions.ServiceName = server.ServiceName shutdown, err := trace.InitTracer(options.TracerProviderOptions) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:forbidigo // this is OK inside the main function. } defer func() { if err := shutdown(ctx); err != nil { - log.Fatal("failed to shutdown TracerProvider: %w", err) + log.Printf("failed to shutdown TracerProvider: %v\n", err) } }() @@ -87,7 +87,7 @@ var rootCmd = &cobra.Command{ // gracefully, so just crash if that happens. err = <-stopped if err == nil { - os.Exit(0) + os.Exit(0) //nolint:forbidigo // this is OK inside the main function. } else { panic(err) } diff --git a/pkg/cli/config.go b/pkg/cli/config.go index 88846d83ff..e2516fd7ed 100644 --- a/pkg/cli/config.go +++ b/pkg/cli/config.go @@ -137,15 +137,17 @@ func GetWorkspace(v *viper.Viper, name string) (*workspaces.Workspace, error) { return section.GetWorkspace(name) } -func getConfig(configFilePath string) *viper.Viper { +func getConfig(configFilePath string) (*viper.Viper, error) { config := viper.New() if configFilePath == "" { // Set config file using the HOME directory. + + // This is extremely unlikely to fail on us. This would only happen + // if the user has no HOME (or USERPROFILE on Windows) directory. home, err := homedir.Dir() if err != nil { - fmt.Println(err) - os.Exit(1) + return nil, fmt.Errorf("failed to find the user's home directory: %w", err) } rad := path.Join(home, ".rad") @@ -154,7 +156,7 @@ func getConfig(configFilePath string) *viper.Viper { } else { config.SetConfigFile(configFilePath) } - return config + return config, nil } // Create a config if its not present @@ -173,10 +175,18 @@ func createConfigFile(configFilePath string) error { } func LoadConfigNoLock(configFilePath string) (*viper.Viper, error) { - config := getConfig(configFilePath) - configFile := GetConfigFilePath(config) + config, err := getConfig(configFilePath) + if err != nil { + return nil, err + } + + configFile, err := GetConfigFilePath(config) + if err != nil { + return nil, err + } + // On Ubuntu OS, getConfig() function doesnt create a config file if its not present. - err := createConfigFile(configFile) + err = createConfigFile(configFile) if err != nil { return nil, err } @@ -195,11 +205,18 @@ func LoadConfigNoLock(configFilePath string) (*viper.Viper, error) { } func LoadConfig(configFilePath string) (*viper.Viper, error) { - config := getConfig(configFilePath) - configFile := GetConfigFilePath(config) + config, err := getConfig(configFilePath) + if err != nil { + return nil, err + } + + configFile, err := GetConfigFilePath(config) + if err != nil { + return nil, err + } // On Ubuntu OS, getConfig() function doesnt create a config file if its not present. - err := createConfigFile(configFile) + err = createConfigFile(configFile) if err != nil { return nil, err } @@ -235,19 +252,21 @@ func LoadConfig(configFilePath string) (*viper.Viper, error) { return config, nil } -func GetConfigFilePath(v *viper.Viper) string { +func GetConfigFilePath(v *viper.Viper) (string, error) { configFilePath := v.ConfigFileUsed() + + // Set config file using the HOME directory. if configFilePath == "" { - // Set config file using the HOME directory. + // This is extremely unlikely to fail on us. This would only happen + // if the user has no HOME (or USERPROFILE on Windows) directory. home, err := homedir.Dir() if err != nil { - fmt.Println(err) - os.Exit(1) + return "", fmt.Errorf("failed to find the user's home directory: %w", err) } configFilePath = path.Join(home, ".rad", "config.yaml") } - return configFilePath + return configFilePath, nil } func UpdateAzProvider(section *WorkspaceSection, provider workspaces.AzureProvider, contextName string) { @@ -312,11 +331,15 @@ func EditWorkspaces(ctx context.Context, config *viper.Viper, editor func(sectio func SaveConfigOnLock(ctx context.Context, config *viper.Viper, updateConfig func(*viper.Viper) error) error { // Acquire exclusive lock on the config.yaml.lock file. // Retry it every second for 5 times if other goroutine is holding the lock i.e other cmd is writing to the config file. - configFilePath := GetConfigFilePath(config) + configFilePath, err := GetConfigFilePath(config) + if err != nil { + return err + } + fileLock := flock.New(configFilePath + ".lock") lockCtx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - _, err := fileLock.TryLockContext(lockCtx, 1*time.Second) + _, err = fileLock.TryLockContext(lockCtx, 1*time.Second) if err != nil { return fmt.Errorf("failed to acquire lock on '%s': %w", configFilePath, err) } @@ -339,9 +362,12 @@ func SaveConfigOnLock(ctx context.Context, config *viper.Viper, updateConfig fun } func SaveConfig(v *viper.Viper) error { - configFilePath := GetConfigFilePath(v) + configFilePath, err := GetConfigFilePath(v) + if err != nil { + return err + } - err := v.WriteConfigAs(configFilePath) + err = v.WriteConfigAs(configFilePath) if err != nil { return fmt.Errorf("failed to write config to '%s': %w", configFilePath, err) } diff --git a/test/magpiego/bindings/keyvault.go b/test/magpiego/bindings/keyvault.go index 84cf030804..919cfd11a4 100644 --- a/test/magpiego/bindings/keyvault.go +++ b/test/magpiego/bindings/keyvault.go @@ -40,7 +40,7 @@ func KeyVaultBinding(envParams map[string]string) BindingStatus { // Get the secret _, err = client.GetSecret(context.TODO(), "TestSecret", resp.ID.Version(), nil) if err != nil { - log.Fatalf("failed to get the secret: %v", err) + log.Printf("failed to get the secret: %v\n", err) return BindingStatus{false, "failed to get the secret"} } diff --git a/test/magpiego/main.go b/test/magpiego/main.go index fb316af56d..4220e993ae 100644 --- a/test/magpiego/main.go +++ b/test/magpiego/main.go @@ -23,6 +23,6 @@ func main() { } if err != nil { log.Println("Terminating Magpie. Encountered error - ", err.Error()) - os.Exit(1) + os.Exit(1) //nolint:forbidigo // this is OK inside the main function. } }