13647: Add MungeLegacyConfigArgs.
authorTom Clegg <tclegg@veritasgenetics.com>
Thu, 11 Jul 2019 21:00:45 +0000 (17:00 -0400)
committerTom Clegg <tclegg@veritasgenetics.com>
Thu, 11 Jul 2019 21:00:45 +0000 (17:00 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

lib/config/load.go
lib/config/load_test.go

index 897d99d51453aa3cfa1a32626ca2797fab99be26..6e0b32677a87708afcd6d85199ba2c6fa1a8f426 100644 (file)
@@ -55,6 +55,65 @@ func (ldr *Loader) SetupFlags(flagset *flag.FlagSet) {
        flagset.StringVar(&ldr.KeepstorePath, "legacy-keepstore-config", defaultKeepstoreConfigPath, "Legacy keepstore configuration `file`")
 }
 
+// MungeLegacyConfigArgs checks args for a -config flag whose argument
+// is a regular file (or a symlink to one), but doesn't have a
+// top-level "Clusters" key and therefore isn't a valid cluster
+// configuration file. If it finds such a flag, it replaces -config
+// with legacyConfigArg (e.g., "-legacy-keepstore-config").
+//
+// This is used by programs that still need to accept "-config" as a
+// way to specify a per-component config file until their config has
+// been migrated.
+//
+// If any errors are encountered while reading or parsing a config
+// file, the given args are not munged. We presume the same errors
+// will be encountered again and reported later on when trying to load
+// cluster configuration from the same file, regardless of which
+// struct we end up using.
+func (ldr *Loader) MungeLegacyConfigArgs(lgr logrus.FieldLogger, args []string, legacyConfigArg string) []string {
+       munged := append([]string(nil), args...)
+       for i := 0; i < len(args); i++ {
+               if !strings.HasPrefix(args[i], "-") || strings.SplitN(strings.TrimPrefix(args[i], "-"), "=", 2)[0] != "config" {
+                       continue
+               }
+               var operand string
+               if strings.Contains(args[i], "=") {
+                       operand = strings.SplitN(args[i], "=", 2)[1]
+               } else if i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") {
+                       i++
+                       operand = args[i]
+               } else {
+                       continue
+               }
+               if fi, err := os.Stat(operand); err != nil || !fi.Mode().IsRegular() {
+                       continue
+               }
+               f, err := os.Open(operand)
+               if err != nil {
+                       continue
+               }
+               defer f.Close()
+               buf, err := ioutil.ReadAll(f)
+               if err != nil {
+                       continue
+               }
+               var cfg arvados.Config
+               err = yaml.Unmarshal(buf, &cfg)
+               if err != nil {
+                       continue
+               }
+               if len(cfg.Clusters) == 0 {
+                       lgr.Warnf("%s is not a cluster config file -- interpreting %s as %s (please migrate your config!)", operand, "-config", legacyConfigArg)
+                       if operand == args[i] {
+                               munged[i-1] = legacyConfigArg
+                       } else {
+                               munged[i] = legacyConfigArg + "=" + operand
+                       }
+               }
+       }
+       return munged
+}
+
 func (ldr *Loader) loadBytes(path string) ([]byte, error) {
        if path == "-" {
                return ioutil.ReadAll(ldr.Stdin)
index 0ef149dd8a117b2c9dbf0d61b721660e3417a55e..0d905a2178bda33c34b592caca30527f31ab9eb2 100644 (file)
@@ -6,7 +6,9 @@ package config
 
 import (
        "bytes"
+       "fmt"
        "io"
+       "io/ioutil"
        "os"
        "os/exec"
        "strings"
@@ -45,6 +47,93 @@ func (s *LoadSuite) TestNoConfigs(c *check.C) {
        c.Check(cc.API.MaxItemsPerResponse, check.Equals, 1000)
 }
 
+func (s *LoadSuite) TestMungeLegacyConfigArgs(c *check.C) {
+       f, err := ioutil.TempFile("", "")
+       c.Check(err, check.IsNil)
+       defer os.Remove(f.Name())
+       io.WriteString(f, "Debug: true\n")
+       oldfile := f.Name()
+
+       f, err = ioutil.TempFile("", "")
+       c.Check(err, check.IsNil)
+       defer os.Remove(f.Name())
+       io.WriteString(f, "Clusters: {aaaaa: {}}\n")
+       newfile := f.Name()
+
+       for _, trial := range []struct {
+               argsIn  []string
+               argsOut []string
+       }{
+               {
+                       []string{"-config", oldfile},
+                       []string{"-old-config", oldfile},
+               },
+               {
+                       []string{"-config=" + oldfile},
+                       []string{"-old-config=" + oldfile},
+               },
+               {
+                       []string{"-config", newfile},
+                       []string{"-config", newfile},
+               },
+               {
+                       []string{"-config=" + newfile},
+                       []string{"-config=" + newfile},
+               },
+               {
+                       []string{"-foo", oldfile},
+                       []string{"-foo", oldfile},
+               },
+               {
+                       []string{"-foo=" + oldfile},
+                       []string{"-foo=" + oldfile},
+               },
+               {
+                       []string{"-foo", "-config=" + oldfile},
+                       []string{"-foo", "-old-config=" + oldfile},
+               },
+               {
+                       []string{"-foo", "bar", "-config=" + oldfile},
+                       []string{"-foo", "bar", "-old-config=" + oldfile},
+               },
+               {
+                       []string{"-foo=bar", "baz", "-config=" + oldfile},
+                       []string{"-foo=bar", "baz", "-old-config=" + oldfile},
+               },
+               {
+                       []string{"-config=/dev/null"},
+                       []string{"-config=/dev/null"},
+               },
+               {
+                       []string{"-config=-"},
+                       []string{"-config=-"},
+               },
+               {
+                       []string{"-config="},
+                       []string{"-config="},
+               },
+               {
+                       []string{"-foo=bar", "baz", "-config"},
+                       []string{"-foo=bar", "baz", "-config"},
+               },
+               {
+                       []string{},
+                       nil,
+               },
+       } {
+               var logbuf bytes.Buffer
+               logger := logrus.New()
+               logger.Out = &logbuf
+
+               var ldr Loader
+               args := ldr.MungeLegacyConfigArgs(logger, trial.argsIn, "-old-config")
+               c.Check(args, check.DeepEquals, trial.argsOut)
+               if fmt.Sprintf("%v", trial.argsIn) != fmt.Sprintf("%v", trial.argsOut) {
+                       c.Check(logbuf.String(), check.Matches, `.*`+oldfile+` is not a cluster config file -- interpreting -config as -old-config.*\n`)
+               }
+       }
+}
+
 func (s *LoadSuite) TestSampleKeys(c *check.C) {
        for _, yaml := range []string{
                `{"Clusters":{"z1111":{}}}`,