Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Avoid using viper.IsSet, which is broken
Browse files Browse the repository at this point in the history
In some places we need to know whether a flag has been explicitly
set. When it was just flags, `pflag.Changed` served this purpose. But
`viper.IsSet`, which is supposed to do the same thing over flags,
config files, and environment entries, is broken:
spf13/viper#276.

Therefore: I've made my own isSet predicate, which checks the flags,
config and environment individually. Since we can't actually check the
environment separately, it has to construct a viper value that _only_
looks at the env, and check that.
  • Loading branch information
squaremo committed Jan 29, 2020
1 parent 7dce46e commit d02e173
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 4 deletions.
5 changes: 5 additions & 0 deletions chart/flux/flux-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
git-url: "git@github.com:weaveworks/flux-get-started"
ssh-keygen-dir: "/var/fluxd/keygen"
git-branch: "master"
listen-metrics: ":3031"
memcached-service: ""
20 changes: 16 additions & 4 deletions cmd/fluxd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,18 @@ func main() {

// --- From this point, we can just consult the config struct for values ---

// viper.IsSet, which we could otherwise use for determining
// whether a flag has been supplied, is broken:
// https://github.com/spf13/viper/pull/331. So we have to proceed
// by other means.
envOnly := viper.New()
envOnly.SetEnvPrefix("FLUXD")
envOnly.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
envOnly.AutomaticEnv()
isSet := func(flag string) bool {
return fs.Changed(flag) || viper.InConfig(flag) || envOnly.IsSet(flag)
}

// Explicitly initialize klog to enable stderr logging,
// and parse our own flags.
klogFlags := flag.NewFlagSet("klog", flag.ExitOnError)
Expand Down Expand Up @@ -346,7 +358,7 @@ func main() {
}
var changedGitRelatedFlags []string
for _, gitRelatedFlag := range gitRelatedFlags {
if viper.IsSet(gitRelatedFlag) {
if isSet(gitRelatedFlag) {
changedGitRelatedFlags = append(changedGitRelatedFlags, gitRelatedFlag)
}
}
Expand All @@ -358,18 +370,18 @@ func main() {
// Maintain backwards compatibility with the --registry-poll-interval
// _flag_, but only if the --automation-interval is not set to a custom
// (non default) value anywhere.
if fs.Changed("registry-poll-interval") && !viper.IsSet("automation-interval") {
if fs.Changed("registry-poll-interval") && !isSet("automation-interval") {
config.AutomationInterval = config.RegistryPollInterval
}

// Sort out values for the git tag and notes ref. There are
// running deployments that assume the defaults as given, so don't
// mess with those unless explicitly told.
if viper.IsSet("git-label") {
if isSet("git-label") {
config.GitSyncTag = config.GitLabel
config.GitNotesRef = config.GitLabel
for _, f := range []string{"git-sync-tag", "git-notes-ref"} {
if viper.IsSet(f) {
if isSet(f) {
logger.Log("overridden", f, "value", config.GitLabel)
}
}
Expand Down

0 comments on commit d02e173

Please sign in to comment.