From ef6855936b3638d3db198a05a4e7c1e4d85dce12 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Thu, 6 Jun 2024 07:41:43 +0200 Subject: [PATCH 1/3] Fix config loading of cli --- cli/common/flags.go | 12 ++-- cli/internal/config/config.go | 85 +++++++++++++++----------- cli/internal/config/config_test.go | 27 ++++++++ cli/setup/setup.go | 6 +- docker/Dockerfile.cli.alpine.multiarch | 1 + docker/Dockerfile.cli.multiarch | 1 + 6 files changed, 89 insertions(+), 43 deletions(-) create mode 100644 cli/internal/config/config_test.go diff --git a/cli/common/flags.go b/cli/common/flags.go index c4f022dbe0..6a74312495 100644 --- a/cli/common/flags.go +++ b/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", diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index 50d32bec0d..2772a04320 100644 --- a/cli/internal/config/config.go +++ b/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,39 +42,27 @@ 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 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") } - if !c.IsSet("server") { - err = c.Set("server", config.ServerURL) - if err != nil { - return err - } + 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 - } + 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 - } + 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") - } + log.Debug().Any("config", config).Msg("Loaded config") return nil } @@ -81,35 +81,52 @@ 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) { 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 + } else { + configFromFile := &Config{} + log.Debug().Msg("Reading the config file") + err = json.Unmarshal(content, configFromFile) + if err != nil { + return nil, err + } + c.MergeIfNotSet(configFromFile) } - c := &Config{} - err = json.Unmarshal(content, c) - if err != nil { - return nil, err + // 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 err == nil { - c.Token = secret + if errors.Is(err, keyring.ErrNotFound) { + log.Warn().Msg("Token not found in keyring") + return c, nil } + c.Token = secret return c, nil } diff --git a/cli/internal/config/config_test.go b/cli/internal/config/config_test.go new file mode 100644 index 0000000000..110805dcc0 --- /dev/null +++ b/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") +} diff --git a/cli/setup/setup.go b/cli/setup/setup.go index bdacd01278..1d3126efe5 100644 --- a/cli/setup/setup.go +++ b/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() } diff --git a/docker/Dockerfile.cli.alpine.multiarch b/docker/Dockerfile.cli.alpine.multiarch index 09537396dc..e8348da878 100644 --- a/docker/Dockerfile.cli.alpine.multiarch +++ b/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/ diff --git a/docker/Dockerfile.cli.multiarch b/docker/Dockerfile.cli.multiarch index e33258b865..2afeb82e22 100644 --- a/docker/Dockerfile.cli.multiarch +++ b/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 From 52462c08bea30a0762f831815189faefffe2cf51 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Thu, 6 Jun 2024 11:38:35 +0200 Subject: [PATCH 2/3] trigger ci From e7d03842f5769d59af43fa35f14faaf08ed53071 Mon Sep 17 00:00:00 2001 From: Anbraten <6918444+anbraten@users.noreply.github.com> Date: Thu, 6 Jun 2024 12:09:15 +0200 Subject: [PATCH 3/3] fix lint --- cli/internal/config/config.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index 2772a04320..29741212e6 100644 --- a/cli/internal/config/config.go +++ b/cli/internal/config/config.go @@ -95,19 +95,22 @@ func Get(ctx *cli.Context, _configPath string) (*Config, error) { 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) { + + case err != nil && os.IsNotExist(err): log.Debug().Msg("The config file does not exist") - } else { + + default: configFromFile := &Config{} - log.Debug().Msg("Reading the config file") 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