Fix config loading of cli (#3764)

cli/common/flags.go

@@ -27,18 +27,18 @@ var GlobalFlags = append([]cli.Flag{
 		Aliases: []string{"c"},
 		Usage:   "path to config file",
 	},
-	&cli.StringFlag{
-		EnvVars: []string{"WOODPECKER_TOKEN"},
-		Name:    "token",
-		Aliases: []string{"t"},
-		Usage:   "server auth token",
-	},
 	&cli.StringFlag{
 		EnvVars: []string{"WOODPECKER_SERVER"},
 		Name:    "server",
 		Aliases: []string{"s"},
 		Usage:   "server address",
 	},
+	&cli.StringFlag{
+		EnvVars: []string{"WOODPECKER_TOKEN"},
+		Name:    "token",
+		Aliases: []string{"t"},
+		Usage:   "server auth token",
+	},
 	&cli.BoolFlag{
 		EnvVars: []string{"WOODPECKER_DISABLE_UPDATE_CHECK"},
 		Name:    "disable-update-check",

cli/internal/config/config.go

@@ -18,6 +18,18 @@ type Config struct {
 	LogLevel  string `json:"log_level"`
 }
 
+func (c *Config) MergeIfNotSet(c2 *Config) {
+	if c.ServerURL == "" {
+		c.ServerURL = c2.ServerURL
+	}
+	if c.Token == "" {
+		c.Token = c2.Token
+	}
+	if c.LogLevel == "" {
+		c.LogLevel = c2.LogLevel
+	}
+}
+
 var skipSetupForCommands = []string{"setup", "help", "h", "version", "update", "lint", "exec", ""}
 
 func Load(c *cli.Context) error {
@@ -30,40 +42,28 @@ func Load(c *cli.Context) error {
 		return err
 	}
 
-	if config == nil {
-		config = &Config{
-			LogLevel:  "info",
-			ServerURL: c.String("server-url"),
-			Token:     c.String("token"),
-		}
-	}
-
-	if !c.IsSet("server") {
-		err = c.Set("server", config.ServerURL)
-		if err != nil {
-			return err
-		}
-	}
-
-	if !c.IsSet("token") {
-		err = c.Set("token", config.Token)
-		if err != nil {
-			return err
-		}
-	}
-
-	if !c.IsSet("log-level") {
-		err = c.Set("log-level", config.LogLevel)
-		if err != nil {
-			return err
-		}
-	}
-
 	if config.ServerURL == "" || config.Token == "" {
 		log.Info().Msg("The woodpecker-cli is not yet set up. Please run `woodpecker-cli setup` or provide the required environment variables / flags.")
 		return errors.New("woodpecker-cli is not configured")
 	}
 
+	err = c.Set("server", config.ServerURL)
+	if err != nil {
+		return err
+	}
+
+	err = c.Set("token", config.Token)
+	if err != nil {
+		return err
+	}
+
+	err = c.Set("log-level", config.LogLevel)
+	if err != nil {
+		return err
+	}
+
+	log.Debug().Any("config", config).Msg("Loaded config")
+
 	return nil
 }
 
@@ -81,35 +81,55 @@ func getConfigPath(configPath string) (string, error) {
 }
 
 func Get(ctx *cli.Context, _configPath string) (*Config, error) {
+	c := &Config{
+		LogLevel:  ctx.String("log-level"),
+		Token:     ctx.String("token"),
+		ServerURL: ctx.String("server"),
+	}
+
 	configPath, err := getConfigPath(_configPath)
 	if err != nil {
 		return nil, err
 	}
 
+	log.Debug().Str("configPath", configPath).Msg("Checking for config file")
+
 	content, err := os.ReadFile(configPath)
-	if err != nil && !os.IsNotExist(err) {
+	switch {
+	case err != nil && !os.IsNotExist(err):
 		log.Debug().Err(err).Msg("Failed to read the config file")
 		return nil, err
-	} else if err != nil && os.IsNotExist(err) {
-		log.Debug().Msg("The config file does not exist")
-		return nil, nil
-	}
 
-	c := &Config{}
-	err = json.Unmarshal(content, c)
+	case err != nil && os.IsNotExist(err):
+		log.Debug().Msg("The config file does not exist")
+
+	default:
+		configFromFile := &Config{}
+		err = json.Unmarshal(content, configFromFile)
 		if err != nil {
 			return nil, err
 		}
+		c.MergeIfNotSet(configFromFile)
+		log.Debug().Msg("Loaded config from file")
+	}
+
+	// if server or token are explicitly set, use them
+	if ctx.IsSet("server") || ctx.IsSet("token") {
+		return c, nil
+	}
 
 	// load token from keyring
 	service := ctx.App.Name
 	secret, err := keyring.Get(service, c.ServerURL)
-	if err != nil && !errors.Is(err, keyring.ErrNotFound) {
-		return nil, err
+	if errors.Is(err, keyring.ErrUnsupportedPlatform) {
+		log.Warn().Msg("Keyring is not supported on this platform")
+		return c, nil
+	}
+	if errors.Is(err, keyring.ErrNotFound) {
+		log.Warn().Msg("Token not found in keyring")
+		return c, nil
 	}
-	if err == nil {
 	c.Token = secret
-	}
 
 	return c, nil
 }

cli/internal/config/config_test.go

@@ -0,0 +1,27 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConfigMerge(t *testing.T) {
+	config := &Config{
+		ServerURL: "http://localhost:8080",
+		Token:     "1234567890",
+		LogLevel:  "debug",
+	}
+
+	configFromFile := &Config{
+		ServerURL: "https://ci.woodpecker-ci.org",
+		Token:     "",
+		LogLevel:  "info",
+	}
+
+	config.MergeIfNotSet(configFromFile)
+
+	assert.Equal(t, config.ServerURL, "http://localhost:8080")
+	assert.Equal(t, config.Token, "1234567890")
+	assert.Equal(t, config.LogLevel, "debug")
+}

cli/setup/setup.go

@@ -16,10 +16,10 @@ var Command = &cli.Command{
 	Name:      "setup",
 	Usage:     "setup the woodpecker-cli for the first time",
 	Args:      true,
-	ArgsUsage: "[server-url]",
+	ArgsUsage: "[server]",
 	Flags: []cli.Flag{
 		&cli.StringFlag{
-			Name:  "server-url",
+			Name:  "server",
 			Usage: "The URL of the woodpecker server",
 		},
 		&cli.StringFlag{
@@ -46,7 +46,7 @@ func setup(c *cli.Context) error {
 		}
 	}
 
-	serverURL := c.String("server-url")
+	serverURL := c.String("server")
 	if serverURL == "" {
 		serverURL = c.Args().First()
 	}

docker/Dockerfile.cli.alpine.multiarch

@@ -12,6 +12,7 @@ FROM docker.io/alpine:3.20
 ENV CA_CERTIFICATES_VERSION="20240226-r0"
 RUN apk add -U --no-cache ca-certificates=${CA_CERTIFICATES_VERSION}
 ENV GODEBUG=netdns=go
+ENV WOODPECKER_DISABLE_UPDATE_CHECK=true
 
 COPY --from=build /src/dist/woodpecker-cli /bin/
 

docker/Dockerfile.cli.multiarch

@@ -9,6 +9,7 @@ RUN --mount=type=cache,target=/root/.cache/go-build \
 
 FROM scratch
 ENV GODEBUG=netdns=go
+ENV WOODPECKER_DISABLE_UPDATE_CHECK=true
 
 # copy certs from build image
 COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt