X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/4fd53a5f75ac8359cd2611f35ec582b5899f2cb8..ae92d144610446849eb568247a44f02ae985c281:/lib/config/load_test.go diff --git a/lib/config/load_test.go b/lib/config/load_test.go index 340eb0a0a7..2d87b906c9 100644 --- a/lib/config/load_test.go +++ b/lib/config/load_test.go @@ -15,8 +15,8 @@ import ( "strings" "testing" - "git.curoverse.com/arvados.git/sdk/go/arvados" - "git.curoverse.com/arvados.git/sdk/go/ctxlog" + "git.arvados.org/arvados.git/sdk/go/arvados" + "git.arvados.org/arvados.git/sdk/go/ctxlog" "github.com/ghodss/yaml" "github.com/sirupsen/logrus" check "gopkg.in/check.v1" @@ -29,6 +29,8 @@ func Test(t *testing.T) { var _ = check.Suite(&LoadSuite{}) +var emptyConfigYAML = `Clusters: {"z1111": {}}` + // Return a new Loader that reads cluster config from configdata // (instead of the usual default /etc/arvados/config.yml), and logs to // logdst or (if that's nil) c.Log. @@ -46,6 +48,12 @@ func testLoader(c *check.C, configdata string, logdst io.Writer) *Loader { type LoadSuite struct{} +func (s *LoadSuite) SetUpSuite(c *check.C) { + os.Unsetenv("ARVADOS_API_HOST") + os.Unsetenv("ARVADOS_API_HOST_INSECURE") + os.Unsetenv("ARVADOS_API_TOKEN") +} + func (s *LoadSuite) TestEmpty(c *check.C) { cfg, err := testLoader(c, "", nil).Load() c.Check(cfg, check.IsNil) @@ -53,7 +61,7 @@ func (s *LoadSuite) TestEmpty(c *check.C) { } func (s *LoadSuite) TestNoConfigs(c *check.C) { - cfg, err := testLoader(c, `Clusters: {"z1111": {}}`, nil).Load() + cfg, err := testLoader(c, emptyConfigYAML, nil).Load() c.Assert(err, check.IsNil) c.Assert(cfg.Clusters, check.HasLen, 1) cc, err := cfg.GetCluster("z1111") @@ -63,6 +71,18 @@ func (s *LoadSuite) TestNoConfigs(c *check.C) { c.Check(cc.API.MaxItemsPerResponse, check.Equals, 1000) } +func (s *LoadSuite) TestNullKeyDoesNotOverrideDefault(c *check.C) { + ldr := testLoader(c, `{"Clusters":{"z1111":{"API":}}}`, nil) + ldr.SkipDeprecated = true + cfg, err := ldr.Load() + c.Assert(err, check.IsNil) + c1, err := cfg.GetCluster("z1111") + c.Assert(err, check.IsNil) + c.Check(c1.ClusterID, check.Equals, "z1111") + c.Check(c1.API.MaxRequestAmplification, check.Equals, 4) + c.Check(c1.API.MaxItemsPerResponse, check.Equals, 1000) +} + func (s *LoadSuite) TestMungeLegacyConfigArgs(c *check.C) { f, err := ioutil.TempFile("", "") c.Check(err, check.IsNil) @@ -73,7 +93,7 @@ func (s *LoadSuite) TestMungeLegacyConfigArgs(c *check.C) { f, err = ioutil.TempFile("", "") c.Check(err, check.IsNil) defer os.Remove(f.Name()) - io.WriteString(f, "Clusters: {aaaaa: {}}\n") + io.WriteString(f, emptyConfigYAML) newfile := f.Name() for _, trial := range []struct { @@ -158,6 +178,7 @@ func (s *LoadSuite) TestSampleKeys(c *check.C) { cfg, err := testLoader(c, yaml, nil).Load() c.Assert(err, check.IsNil) cc, err := cfg.GetCluster("z1111") + c.Assert(err, check.IsNil) _, hasSample := cc.InstanceTypes["SAMPLE"] c.Check(hasSample, check.Equals, false) if strings.Contains(yaml, "Foo") { @@ -168,7 +189,9 @@ func (s *LoadSuite) TestSampleKeys(c *check.C) { } func (s *LoadSuite) TestMultipleClusters(c *check.C) { - cfg, err := testLoader(c, `{"Clusters":{"z1111":{},"z2222":{}}}`, nil).Load() + ldr := testLoader(c, `{"Clusters":{"z1111":{},"z2222":{}}}`, nil) + ldr.SkipDeprecated = true + cfg, err := ldr.Load() c.Assert(err, check.IsNil) c1, err := cfg.GetCluster("z1111") c.Assert(err, check.IsNil) @@ -183,21 +206,41 @@ func (s *LoadSuite) TestDeprecatedOrUnknownWarning(c *check.C) { _, err := testLoader(c, ` Clusters: zzzzz: - postgresql: {} - BadKey: {} - Containers: {} + ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + Collections: + BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + PostgreSQL: {} + BadKey1: {} + Containers: + RunTimeEngine: abc RemoteClusters: z2222: Host: z2222.arvadosapi.com Proxy: true - BadKey: badValue + BadKey2: badValue + Services: + KeepStore: + InternalURLs: + "http://host.example:12345": {} + Keepstore: + InternalURLs: + "http://host.example:12345": + RendezVous: x + ServiceS: + Keepstore: + InternalURLs: + "http://host.example:12345": {} + Volumes: + zzzzz-nyw5e-aaaaaaaaaaaaaaa: {} `, &logbuf).Load() c.Assert(err, check.IsNil) + c.Log(logbuf.String()) logs := strings.Split(strings.TrimSuffix(logbuf.String(), "\n"), "\n") for _, log := range logs { - c.Check(log, check.Matches, `.*deprecated or unknown config entry:.*BadKey.*`) + c.Check(log, check.Matches, `.*deprecated or unknown config entry:.*(RunTimeEngine.*RuntimeEngine|BadKey1|BadKey2|KeepStore|ServiceS|RendezVous).*`) } - c.Check(logs, check.HasLen, 2) + c.Check(logs, check.HasLen, 6) } func (s *LoadSuite) checkSAMPLEKeys(c *check.C, path string, x interface{}) { @@ -252,15 +295,23 @@ func (s *LoadSuite) TestNoUnrecognizedKeysInDefaultConfig(c *check.C) { err = yaml.Unmarshal(buf, &loaded) c.Assert(err, check.IsNil) + c.Check(logbuf.String(), check.Matches, `(?ms).*SystemRootToken: secret token is not set.*`) + c.Check(logbuf.String(), check.Matches, `(?ms).*ManagementToken: secret token is not set.*`) + c.Check(logbuf.String(), check.Matches, `(?ms).*Collections.BlobSigningKey: secret token is not set.*`) + logbuf.Reset() loader.logExtraKeys(loaded, supplied, "") c.Check(logbuf.String(), check.Equals, "") } func (s *LoadSuite) TestNoWarningsForDumpedConfig(c *check.C) { var logbuf bytes.Buffer - logger := logrus.New() - logger.Out = &logbuf - cfg, err := testLoader(c, `{"Clusters":{"zzzzz":{}}}`, &logbuf).Load() + cfg, err := testLoader(c, ` +Clusters: + zzzzz: + ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + Collections: + BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`, &logbuf).Load() c.Assert(err, check.IsNil) yaml, err := yaml.Marshal(cfg) c.Assert(err, check.IsNil) @@ -270,18 +321,78 @@ func (s *LoadSuite) TestNoWarningsForDumpedConfig(c *check.C) { c.Check(logbuf.String(), check.Equals, "") } +func (s *LoadSuite) TestUnacceptableTokens(c *check.C) { + for _, trial := range []struct { + short bool + configPath string + example string + }{ + {false, "SystemRootToken", "SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_b_c"}, + {false, "ManagementToken", "ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa b c"}, + {false, "ManagementToken", "ManagementToken: \"$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabc\""}, + {false, "Collections.BlobSigningKey", "Collections: {BlobSigningKey: \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa⛵\"}"}, + {true, "SystemRootToken", "SystemRootToken: a_b_c"}, + {true, "ManagementToken", "ManagementToken: a b c"}, + {true, "ManagementToken", "ManagementToken: \"$abc\""}, + {true, "Collections.BlobSigningKey", "Collections: {BlobSigningKey: \"⛵\"}"}, + } { + c.Logf("trying bogus config: %s", trial.example) + _, err := testLoader(c, "Clusters:\n zzzzz:\n "+trial.example, nil).Load() + c.Check(err, check.ErrorMatches, `Clusters.zzzzz.`+trial.configPath+`: unacceptable characters in token.*`) + } +} + func (s *LoadSuite) TestPostgreSQLKeyConflict(c *check.C) { _, err := testLoader(c, ` Clusters: zzzzz: - postgresql: - connection: + PostgreSQL: + Connection: DBName: dbname Host: host `, nil).Load() c.Check(err, check.ErrorMatches, `Clusters.zzzzz.PostgreSQL.Connection: multiple entries for "(dbname|host)".*`) } +func (s *LoadSuite) TestBadClusterIDs(c *check.C) { + for _, data := range []string{` +Clusters: + 123456: + ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + Collections: + BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +`, ` +Clusters: + 12345: + ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + Collections: + BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + RemoteClusters: + Zzzzz: + Host: Zzzzz.arvadosapi.com + Proxy: true +`, ` +Clusters: + abcde: + ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + Collections: + BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + Login: + LoginCluster: zz-zz +`, + } { + c.Log(data) + v, err := testLoader(c, data, nil).Load() + if v != nil { + c.Logf("%#v", v.Clusters) + } + c.Check(err, check.ErrorMatches, `.*cluster ID should be 5 alphanumeric characters.*`) + } +} + func (s *LoadSuite) TestBadType(c *check.C) { for _, data := range []string{` Clusters: @@ -319,7 +430,7 @@ Clusters: } func (s *LoadSuite) TestMovedKeys(c *check.C) { - s.checkEquivalent(c, `# config has old keys only + checkEquivalent(c, `# config has old keys only Clusters: zzzzz: RequestLimits: @@ -332,7 +443,7 @@ Clusters: MaxRequestAmplification: 3 MaxItemsPerResponse: 999 `) - s.checkEquivalent(c, `# config has both old and new keys; old values win + checkEquivalent(c, `# config has both old and new keys; old values win Clusters: zzzzz: RequestLimits: @@ -350,26 +461,300 @@ Clusters: `) } -func (s *LoadSuite) checkEquivalent(c *check.C, goty, expectedy string) { - got, err := testLoader(c, goty, nil).Load() +func checkEquivalent(c *check.C, goty, expectedy string) string { + var logbuf bytes.Buffer + gotldr := testLoader(c, goty, &logbuf) + expectedldr := testLoader(c, expectedy, nil) + checkEquivalentLoaders(c, gotldr, expectedldr) + return logbuf.String() +} + +func checkEqualYAML(c *check.C, got, expected interface{}) { + expectedyaml, err := yaml.Marshal(expected) c.Assert(err, check.IsNil) - expected, err := testLoader(c, expectedy, nil).Load() + gotyaml, err := yaml.Marshal(got) c.Assert(err, check.IsNil) - if !c.Check(got, check.DeepEquals, expected) { + if !bytes.Equal(gotyaml, expectedyaml) { cmd := exec.Command("diff", "-u", "--label", "expected", "--label", "got", "/dev/fd/3", "/dev/fd/4") - for _, obj := range []interface{}{expected, got} { - y, _ := yaml.Marshal(obj) + for _, y := range [][]byte{expectedyaml, gotyaml} { pr, pw, err := os.Pipe() c.Assert(err, check.IsNil) defer pr.Close() - go func() { - io.Copy(pw, bytes.NewBuffer(y)) + go func(data []byte) { + pw.Write(data) pw.Close() - }() + }(y) cmd.ExtraFiles = append(cmd.ExtraFiles, pr) } diff, err := cmd.CombinedOutput() + // diff should report differences and exit non-zero. + c.Check(err, check.NotNil) c.Log(string(diff)) - c.Check(err, check.IsNil) + c.Error("got != expected; see diff (-expected +got) above") } } + +func checkEquivalentLoaders(c *check.C, gotldr, expectedldr *Loader) { + got, err := gotldr.Load() + c.Assert(err, check.IsNil) + expected, err := expectedldr.Load() + c.Assert(err, check.IsNil) + checkEqualYAML(c, got, expected) +} + +func checkListKeys(path string, x interface{}) (err error) { + v := reflect.Indirect(reflect.ValueOf(x)) + switch v.Kind() { + case reflect.Map: + iter := v.MapRange() + for iter.Next() { + k := iter.Key() + if k.Kind() == reflect.String { + if err = checkListKeys(path+"."+k.String(), iter.Value().Interface()); err != nil { + return + } + } + } + return + + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + val := v.Field(i) + structField := v.Type().Field(i) + fieldname := structField.Name + endsWithList := strings.HasSuffix(fieldname, "List") + isAnArray := structField.Type.Kind() == reflect.Slice + if endsWithList != isAnArray { + if endsWithList { + err = fmt.Errorf("%s.%s ends with 'List' but field is not an array (type %v)", path, fieldname, val.Kind()) + return + } + if isAnArray && structField.Type.Elem().Kind() != reflect.Uint8 { + err = fmt.Errorf("%s.%s is an array but field name does not end in 'List' (slice of %v)", path, fieldname, structField.Type.Elem().Kind()) + return + } + } + if val.CanInterface() { + checkListKeys(path+"."+fieldname, val.Interface()) + } + } + } + return +} + +func (s *LoadSuite) TestListKeys(c *check.C) { + v1 := struct { + EndInList []string + }{[]string{"a", "b"}} + var m1 = make(map[string]interface{}) + m1["c"] = &v1 + if err := checkListKeys("", m1); err != nil { + c.Error(err) + } + + v2 := struct { + DoesNot []string + }{[]string{"a", "b"}} + var m2 = make(map[string]interface{}) + m2["c"] = &v2 + if err := checkListKeys("", m2); err == nil { + c.Errorf("Should have produced an error") + } + + v3 := struct { + EndInList string + }{"a"} + var m3 = make(map[string]interface{}) + m3["c"] = &v3 + if err := checkListKeys("", m3); err == nil { + c.Errorf("Should have produced an error") + } + + loader := testLoader(c, string(DefaultYAML), nil) + cfg, err := loader.Load() + c.Assert(err, check.IsNil) + if err := checkListKeys("", cfg); err != nil { + c.Error(err) + } +} + +func (s *LoadSuite) TestImplicitStorageClasses(c *check.C) { + // If StorageClasses and Volumes.*.StorageClasses are all + // empty, there is a default storage class named "default". + ldr := testLoader(c, `{"Clusters":{"z1111":{}}}`, nil) + cfg, err := ldr.Load() + c.Assert(err, check.IsNil) + cc, err := cfg.GetCluster("z1111") + c.Assert(err, check.IsNil) + c.Check(cc.StorageClasses, check.HasLen, 1) + c.Check(cc.StorageClasses["default"].Default, check.Equals, true) + c.Check(cc.StorageClasses["default"].Priority, check.Equals, 0) + + // The implicit "default" storage class is used by all + // volumes. + ldr = testLoader(c, ` +Clusters: + z1111: + Volumes: + z: {}`, nil) + cfg, err = ldr.Load() + c.Assert(err, check.IsNil) + cc, err = cfg.GetCluster("z1111") + c.Assert(err, check.IsNil) + c.Check(cc.StorageClasses, check.HasLen, 1) + c.Check(cc.StorageClasses["default"].Default, check.Equals, true) + c.Check(cc.StorageClasses["default"].Priority, check.Equals, 0) + c.Check(cc.Volumes["z"].StorageClasses["default"], check.Equals, true) + + // The "default" storage class isn't implicit if any classes + // are configured explicitly. + ldr = testLoader(c, ` +Clusters: + z1111: + StorageClasses: + local: + Default: true + Priority: 111 + Volumes: + z: + StorageClasses: + local: true`, nil) + cfg, err = ldr.Load() + c.Assert(err, check.IsNil) + cc, err = cfg.GetCluster("z1111") + c.Assert(err, check.IsNil) + c.Check(cc.StorageClasses, check.HasLen, 1) + c.Check(cc.StorageClasses["local"].Default, check.Equals, true) + c.Check(cc.StorageClasses["local"].Priority, check.Equals, 111) + + // It is an error for a volume to refer to a storage class + // that isn't listed in StorageClasses. + ldr = testLoader(c, ` +Clusters: + z1111: + StorageClasses: + local: + Default: true + Priority: 111 + Volumes: + z: + StorageClasses: + nx: true`, nil) + _, err = ldr.Load() + c.Assert(err, check.ErrorMatches, `z: volume refers to storage class "nx" that is not defined.*`) + + // It is an error for a volume to refer to a storage class + // that isn't listed in StorageClasses ... even if it's + // "default", which would exist implicitly if it weren't + // referenced explicitly by a volume. + ldr = testLoader(c, ` +Clusters: + z1111: + Volumes: + z: + StorageClasses: + default: true`, nil) + _, err = ldr.Load() + c.Assert(err, check.ErrorMatches, `z: volume refers to storage class "default" that is not defined.*`) + + // If the "default" storage class is configured explicitly, it + // is not used implicitly by any volumes, even if it's the + // only storage class. + var logbuf bytes.Buffer + ldr = testLoader(c, ` +Clusters: + z1111: + StorageClasses: + default: + Default: true + Priority: 111 + Volumes: + z: {}`, &logbuf) + _, err = ldr.Load() + c.Assert(err, check.ErrorMatches, `z: volume has no StorageClasses listed`) + + // If StorageClasses are configured explicitly, there must be + // at least one with Default: true. (Calling one "default" is + // not sufficient.) + ldr = testLoader(c, ` +Clusters: + z1111: + StorageClasses: + default: + Priority: 111 + Volumes: + z: + StorageClasses: + default: true`, nil) + _, err = ldr.Load() + c.Assert(err, check.ErrorMatches, `there is no default storage class.*`) +} + +func (s *LoadSuite) TestPreemptiblePriceFactor(c *check.C) { + yaml := ` +Clusters: + z1111: + InstanceTypes: + Type1: + RAM: 12345M + VCPUs: 8 + Price: 1.23 + z2222: + Containers: + PreemptiblePriceFactor: 0.5 + InstanceTypes: + Type1: + RAM: 12345M + VCPUs: 8 + Price: 1.23 + z3333: + Containers: + PreemptiblePriceFactor: 0.5 + InstanceTypes: + Type1: + RAM: 12345M + VCPUs: 8 + Price: 1.23 + Type1.preemptible: # higher price than the auto-added variant would use -- should generate warning + ProviderType: Type1 + RAM: 12345M + VCPUs: 8 + Price: 1.23 + Preemptible: true + Type2: + RAM: 23456M + VCPUs: 16 + Price: 2.46 + Type2.preemptible: # identical to the auto-added variant -- so no warning + ProviderType: Type2 + RAM: 23456M + VCPUs: 16 + Price: 1.23 + Preemptible: true +` + var logbuf bytes.Buffer + cfg, err := testLoader(c, yaml, &logbuf).Load() + c.Assert(err, check.IsNil) + cc, err := cfg.GetCluster("z1111") + c.Assert(err, check.IsNil) + c.Check(cc.InstanceTypes["Type1"].Price, check.Equals, 1.23) + c.Check(cc.InstanceTypes, check.HasLen, 1) + + cc, err = cfg.GetCluster("z2222") + c.Assert(err, check.IsNil) + c.Check(cc.InstanceTypes["Type1"].Preemptible, check.Equals, false) + c.Check(cc.InstanceTypes["Type1"].Price, check.Equals, 1.23) + c.Check(cc.InstanceTypes["Type1.preemptible"].Preemptible, check.Equals, true) + c.Check(cc.InstanceTypes["Type1.preemptible"].Price, check.Equals, 1.23/2) + c.Check(cc.InstanceTypes["Type1.preemptible"].ProviderType, check.Equals, "Type1") + c.Check(cc.InstanceTypes, check.HasLen, 2) + + cc, err = cfg.GetCluster("z3333") + c.Assert(err, check.IsNil) + // Don't overwrite the explicitly configured preemptible variant + c.Check(cc.InstanceTypes["Type1.preemptible"].Price, check.Equals, 1.23) + c.Check(cc.InstanceTypes, check.HasLen, 4) + c.Check(logbuf.String(), check.Matches, `(?ms).*Clusters\.z3333\.InstanceTypes\[Type1\.preemptible\]: already exists, so not automatically adding a preemptible variant of Type1.*`) + c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*Type2\.preemptible.*`) + c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*(z1111|z2222)[^\n]*InstanceTypes.*`) +}