13647: Check ARVADOS_* env vars when loading config.

Previously they were only checked when starting up a service, so they
weren't noticed by config-dump etc., and couldn't be used by the
keepstore config migration when loading the keep_services endpoint.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index 019d5edd0186f43aa3edfb3c92692c7131f25a85..fb1cba38b4857d4b253933158f3f8a64a002cb48 100644 (file)
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -24,6 +24,12 @@ var (
 
 type CommandSuite struct{}
 
 
 type CommandSuite struct{}
 

+func (s *CommandSuite) SetUpSuite(c *check.C) {
+       os.Unsetenv("ARVADOS_API_HOST")
+       os.Unsetenv("ARVADOS_API_HOST_INSECURE")
+       os.Unsetenv("ARVADOS_API_TOKEN")
+}
+

 func (s *CommandSuite) TestBadArg(c *check.C) {
        var stderr bytes.Buffer
        code := DumpCommand.RunCommand("arvados config-dump", []string{"-badarg"}, bytes.NewBuffer(nil), bytes.NewBuffer(nil), &stderr)
 func (s *CommandSuite) TestBadArg(c *check.C) {
        var stderr bytes.Buffer
        code := DumpCommand.RunCommand("arvados config-dump", []string{"-badarg"}, bytes.NewBuffer(nil), bytes.NewBuffer(nil), &stderr)

diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index 0a030fb040db9d36de290e29d67f84db76b8522c..d0e61dbca06454d3488b1f50473b55f67b2a66b1 100644 (file)
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -7,6 +7,7 @@ package config
 import (
        "fmt"
        "io/ioutil"
 import (
        "fmt"
        "io/ioutil"

+       "net/url"

        "os"
        "strings"
 
        "os"
        "strings"
 


@@ -474,3 +475,30 @@ func (ldr *Loader) loadOldGitHttpdConfig(cfg *arvados.Config) error {
        cfg.Clusters[cluster.ClusterID] = *cluster
        return nil
 }
        cfg.Clusters[cluster.ClusterID] = *cluster
        return nil
 }

+
+func (ldr *Loader) loadOldEnvironmentVariables(cfg *arvados.Config) error {
+       if os.Getenv("ARVADOS_API_TOKEN") == "" && os.Getenv("ARVADOS_API_HOST") == "" {
+               return nil
+       }
+       cluster, err := cfg.GetCluster("")
+       if err != nil {
+               return err
+       }
+       if tok := os.Getenv("ARVADOS_API_TOKEN"); tok != "" && cluster.SystemRootToken == "" {
+               ldr.Logger.Warn("SystemRootToken missing from cluster config, falling back to ARVADOS_API_TOKEN environment variable")
+               cluster.SystemRootToken = tok
+       }
+       if apihost := os.Getenv("ARVADOS_API_HOST"); apihost != "" && cluster.Services.Controller.ExternalURL.Host == "" {
+               ldr.Logger.Warn("Services.Controller.ExternalURL missing from cluster config, falling back to ARVADOS_API_HOST(_INSECURE) environment variables")
+               u, err := url.Parse("https://" + apihost)
+               if err != nil {
+                       return fmt.Errorf("cannot parse ARVADOS_API_HOST: %s", err)
+               }
+               cluster.Services.Controller.ExternalURL = arvados.URL(*u)
+               if i := os.Getenv("ARVADOS_API_HOST_INSECURE"); i != "" && i != "0" {
+                       cluster.TLS.Insecure = true
+               }
+       }
+       cfg.Clusters[cluster.ClusterID] = *cluster
+       return nil
+}

diff --git a/lib/config/deprecated_keepstore.go b/lib/config/deprecated_keepstore.go
index a7d41e985376f4e55119fba360c71c4953c118ec..29019296d64e46e1a894595668453d28045f6c92 100644 (file)
--- a/lib/config/deprecated_keepstore.go
+++ b/lib/config/deprecated_keepstore.go
@@ -584,7 +584,7 @@ func keepServiceIsMe(ks arvados.KeepService, hostname string, listen string) boo
 // been warned about in loadOldKeepstoreConfig() -- i.e., unmigrated
 // keepstore hosts other than the present host, and obsolete content
 // in the keep_services table.
 // been warned about in loadOldKeepstoreConfig() -- i.e., unmigrated
 // keepstore hosts other than the present host, and obsolete content
 // in the keep_services table.

-func (ldr *Loader) checkPendingKeepstoreMigrations(cluster arvados.Cluster) error {
+func (ldr *Loader) checkPendingKeepstoreMigrations(cluster *arvados.Cluster) error {

        if cluster.Services.Controller.ExternalURL.String() == "" {
                ldr.Logger.Debug("Services.Controller.ExternalURL not configured -- skipping check for pending keepstore config migrations")
                return nil
        if cluster.Services.Controller.ExternalURL.String() == "" {
                ldr.Logger.Debug("Services.Controller.ExternalURL not configured -- skipping check for pending keepstore config migrations")
                return nil


@@ -593,7 +593,7 @@ func (ldr *Loader) checkPendingKeepstoreMigrations(cluster arvados.Cluster) erro
                ldr.Logger.Debug("(Loader).SkipAPICalls == true -- skipping check for pending keepstore config migrations")
                return nil
        }
                ldr.Logger.Debug("(Loader).SkipAPICalls == true -- skipping check for pending keepstore config migrations")
                return nil
        }

-       client, err := arvados.NewClientFromConfig(&cluster)
+       client, err := arvados.NewClientFromConfig(cluster)

        if err != nil {
                return err
        }
        if err != nil {
                return err
        }

diff --git a/lib/config/deprecated_keepstore_test.go b/lib/config/deprecated_keepstore_test.go
index d1028e8bd0bfe35efccd632df719a2fdaedebe73..62f3f1e3fe5b2e59865d1b756f5edfaca5d32d49 100644 (file)
--- a/lib/config/deprecated_keepstore_test.go
+++ b/lib/config/deprecated_keepstore_test.go
@@ -30,6 +30,10 @@ type KeepstoreMigrationSuite struct {
 var _ = check.Suite(&KeepstoreMigrationSuite{})
 
 func (s *KeepstoreMigrationSuite) SetUpSuite(c *check.C) {
 var _ = check.Suite(&KeepstoreMigrationSuite{})
 
 func (s *KeepstoreMigrationSuite) SetUpSuite(c *check.C) {

+       os.Setenv("ARVADOS_API_HOST", os.Getenv("ARVADOS_TEST_API_HOST"))
+       os.Setenv("ARVADOS_API_HOST_INSECURE", "1")
+       os.Setenv("ARVADOS_API_TOKEN", arvadostest.AdminToken)
+

        // We don't need the keepstore servers, but we do need
        // keep_services listings that point to localhost, rather than
        // the apiserver fixtures that point to fictional hosts
        // We don't need the keepstore servers, but we do need
        // keep_services listings that point to localhost, rather than
        // the apiserver fixtures that point to fictional hosts

diff --git a/lib/config/load.go b/lib/config/load.go
index d9ee97b516de49b1f3af63f8f02f2a717d5349b3..61d80a3c58f535733ad65b9053b55283c9d08510 100644 (file)
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -245,6 +245,7 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
                // * no primary config was loaded, and this is the
                // legacy config file for the current component
                for _, err := range []error{
                // * no primary config was loaded, and this is the
                // legacy config file for the current component
                for _, err := range []error{

+                       ldr.loadOldEnvironmentVariables(&cfg),

                        ldr.loadOldKeepstoreConfig(&cfg),
                        ldr.loadOldKeepWebConfig(&cfg),
                        ldr.loadOldCrunchDispatchSlurmConfig(&cfg),
                        ldr.loadOldKeepstoreConfig(&cfg),
                        ldr.loadOldKeepWebConfig(&cfg),
                        ldr.loadOldCrunchDispatchSlurmConfig(&cfg),

diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 17e0af7ba0bcf9526458641b9f89e8f92aae5a4e..2e521ab0932218cd2f2e1e7f7f976dbbdf3c26d4 100644 (file)
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -46,6 +46,12 @@ func testLoader(c *check.C, configdata string, logdst io.Writer) *Loader {
 
 type LoadSuite struct{}
 
 
 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)
 func (s *LoadSuite) TestEmpty(c *check.C) {
        cfg, err := testLoader(c, "", nil).Load()
        c.Check(cfg, check.IsNil)

diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index d5619b87b307c53e781ea77b78ba943b20f78735..ddff5f47a444739998086e6c7d2a748d13f40a8d 100644 (file)
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -12,7 +12,6 @@ import (
        "io"
        "net"
        "net/http"
        "io"
        "net"
        "net/http"

-       "net/url"

        "os"
        "strings"
 
        "os"
        "strings"
 


@@ -110,23 +109,6 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
        }
        ctx = context.WithValue(ctx, contextKeyURL{}, listenURL)
 
        }
        ctx = context.WithValue(ctx, contextKeyURL{}, listenURL)
 

-       if cluster.SystemRootToken == "" {
-               logger.Warn("SystemRootToken missing from cluster config, falling back to ARVADOS_API_TOKEN environment variable")
-               cluster.SystemRootToken = os.Getenv("ARVADOS_API_TOKEN")
-       }
-       if cluster.Services.Controller.ExternalURL.Host == "" {
-               logger.Warn("Services.Controller.ExternalURL missing from cluster config, falling back to ARVADOS_API_HOST(_INSECURE) environment variables")
-               u, err := url.Parse("https://" + os.Getenv("ARVADOS_API_HOST"))
-               if err != nil {
-                       err = fmt.Errorf("ARVADOS_API_HOST: %s", err)
-                       return 1
-               }
-               cluster.Services.Controller.ExternalURL = arvados.URL(*u)
-               if i := os.Getenv("ARVADOS_API_HOST_INSECURE"); i != "" && i != "0" {
-                       cluster.TLS.Insecure = true
-               }
-       }
-

        reg := prometheus.NewRegistry()
        handler := c.newHandler(ctx, cluster, cluster.SystemRootToken, reg)
        if err = handler.CheckHealth(); err != nil {
        reg := prometheus.NewRegistry()
        handler := c.newHandler(ctx, cluster, cluster.SystemRootToken, reg)
        if err = handler.CheckHealth(); err != nil {