diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 72aa0d55e..61cbdd175 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,4 +10,4 @@ jobs: env: SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }} with: - args: snyk ignore --id=SNYK-GOLANG-GITHUBCOMDISINTEGRATIONIMAGING-5880692 --expiry=2024-03-12 --policy-path=.snyk + args: --package-manager=govendor snyk ignore --id=SNYK-GOLANG-GITHUBCOMDISINTEGRATIONIMAGING-5880692 --expiry=2024-03-12 --policy-path=.snyk diff --git a/.travis.yml b/.travis.yml index f0360aee8..e5587cf85 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,20 +12,22 @@ go_import_path: github.com/tidepool-org/platform env: global: - MONGODB=6.0.11 - - MONGOSH=1.10.6 + - MONGOSH=2.0.2 - DIST=jammy before_install: - - sudo apt-get remove -y mongodb-org mongodb-org-mongos mongodb-org-server mongodb-org-shell mongodb-org-tools - - wget -nv https://repo.mongodb.org/apt/ubuntu/dists/${DIST}/mongodb-org/6.0/multiverse/binary-amd64/mongodb-org-server_${MONGODB}_amd64.deb -O /tmp/mongodb.deb - - wget -nv https://downloads.mongodb.com/compass/mongodb-mongosh_${MONGOSH}_amd64.deb -O /tmp/mongosh.deb - - sudo apt install /tmp/mongodb.deb /tmp/mongosh.deb + - sudo apt update + - sudo apt install -y docker-buildx mongodb-org=${MONGODB} mongodb-org-database=${MONGODB} mongodb-org-server=${MONGODB} mongodb-mongosh=${MONGOSH} mongodb-org-mongos=${MONGODB} mongodb-org-tools=${MONGODB} - mkdir /tmp/data - /usr/bin/mongod --replSet rs0 --dbpath /tmp/data --bind_ip 127.0.0.1 --logpath ${PWD}/mongod.log &> /dev/null & - until nc -z localhost 27017; do echo Waiting for MongoDB; sleep 1; done - /usr/bin/mongosh --host 127.0.0.1 --port 27017 --eval 'rs.initiate(); while (rs.status().startupStatus || (rs.status().hasOwnProperty("myState") && rs.status().myState != 1)) { printjson( rs.status() ); sleep(1000); }; printjson( rs.status() );' addons: + apt: + sources: + - sourceline: 'deb https://repo.mongodb.org/apt/ubuntu jammy/mongodb-org/6.0 multiverse' + key_url: 'https://pgp.mongodb.com/server-6.0.asc' artifacts: s3_region: us-west-2 paths: diff --git a/alerts/client.go b/alerts/client.go new file mode 100644 index 000000000..c02d5049f --- /dev/null +++ b/alerts/client.go @@ -0,0 +1,136 @@ +package alerts + +import ( + "context" + "fmt" + "net/http" + + "github.com/kelseyhightower/envconfig" + + "github.com/tidepool-org/platform/auth" + "github.com/tidepool-org/platform/client" + platformlog "github.com/tidepool-org/platform/log" + "github.com/tidepool-org/platform/log/null" + "github.com/tidepool-org/platform/platform" + "github.com/tidepool-org/platform/request" +) + +// Client for managing alerts configs. +type Client struct { + client PlatformClient + logger platformlog.Logger + token TokenProvider +} + +// NewClient builds a client for interacting with alerts API endpoints. +// +// If no logger is provided, a null logger is used. +func NewClient(client PlatformClient, token TokenProvider, logger platformlog.Logger) *Client { + if logger == nil { + logger = null.NewLogger() + } + return &Client{ + client: client, + logger: logger, + token: token, + } +} + +// platform.Client is one implementation +type PlatformClient interface { + ConstructURL(paths ...string) string + RequestData(ctx context.Context, method string, url string, mutators []request.RequestMutator, + requestBody interface{}, responseBody interface{}, inspectors ...request.ResponseInspector) error +} + +// client.External is one implementation +type TokenProvider interface { + // ServerSessionToken provides a server-to-server API authentication token. + ServerSessionToken() (string, error) +} + +// request performs common operations before passing a request off to the +// underlying platform.Client. +func (c *Client) request(ctx context.Context, method, url string, body any) error { + // Platform's client.Client expects a logger to exist in the request's + // context. If it doesn't exist, request processing will panic. + loggingCtx := platformlog.NewContextWithLogger(ctx, c.logger) + // Make sure the auth token is injected into the request's headers. + return c.requestWithAuth(loggingCtx, method, url, body) +} + +// requestWithAuth injects an auth token before calling platform.Client.RequestData. +// +// At time of writing, this is the only way to inject credentials into +// platform.Client. It might be nice to be able to use a mutator, but the auth +// is specifically handled by the platform.Client via the context field, and +// if left blank, platform.Client errors. +func (c *Client) requestWithAuth(ctx context.Context, method, url string, body any) error { + authCtx, err := c.ctxWithAuth(ctx) + if err != nil { + return err + } + return c.client.RequestData(authCtx, method, url, nil, body, nil) +} + +// Upsert updates cfg if it exists or creates it if it doesn't. +func (c *Client) Upsert(ctx context.Context, cfg *Config) error { + url := c.client.ConstructURL("v1", "alerts", cfg.UserID, cfg.FollowedUserID) + return c.request(ctx, http.MethodPost, url, cfg) +} + +// Delete the alerts config. +func (c *Client) Delete(ctx context.Context, cfg *Config) error { + url := c.client.ConstructURL("v1", "alerts", cfg.UserID, cfg.FollowedUserID) + return c.request(ctx, http.MethodDelete, url, nil) +} + +// ctxWithAuth injects a server session token into the context. +func (c *Client) ctxWithAuth(ctx context.Context) (context.Context, error) { + token, err := c.token.ServerSessionToken() + if err != nil { + return nil, fmt.Errorf("retrieving token: %w", err) + } + return auth.NewContextWithServerSessionToken(ctx, token), nil +} + +// ConfigLoader abstracts the method by which config values are loaded. +type ConfigLoader interface { + Load(*ClientConfig) error +} + +// envconfigLoader adapts envconfig to implement ConfigLoader. +type envconfigLoader struct { + platform.ConfigLoader +} + +// NewEnvconfigLoader loads values via envconfig. +// +// If loader is nil, it defaults to envconfig for platform values. +func NewEnvconfigLoader(loader platform.ConfigLoader) *envconfigLoader { + if loader == nil { + loader = platform.NewEnvconfigLoader(nil) + } + return &envconfigLoader{ + ConfigLoader: loader, + } +} + +// Load implements ConfigLoader. +func (l *envconfigLoader) Load(cfg *ClientConfig) error { + if err := l.ConfigLoader.Load(cfg.Config); err != nil { + return err + } + if err := envconfig.Process(client.EnvconfigEmptyPrefix, cfg); err != nil { + return err + } + // Override client.Client.Address to point to the data service. + cfg.Address = cfg.DataServiceAddress + return nil +} + +type ClientConfig struct { + *platform.Config + // DataServiceAddress is used to override client.Client.Address. + DataServiceAddress string `envconfig:"TIDEPOOL_DATA_SERVICE_ADDRESS" required:"true"` +} diff --git a/alerts/client_test.go b/alerts/client_test.go new file mode 100644 index 000000000..c5a771256 --- /dev/null +++ b/alerts/client_test.go @@ -0,0 +1,118 @@ +package alerts + +import ( + "context" + "net/http" + "net/http/httptest" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/tidepool-org/platform/auth" + "github.com/tidepool-org/platform/client" + "github.com/tidepool-org/platform/log" + "github.com/tidepool-org/platform/log/null" + "github.com/tidepool-org/platform/platform" +) + +const testToken = "auth-me" + +var _ = Describe("Client", func() { + var test404Server, test200Server *httptest.Server + var testAuthServer func(*string) *httptest.Server + + BeforeEach(func() { + t := GinkgoT() + // There's no need to create these before each test, but I can't get + // Ginkgo to let me start these just once. + test404Server = testServer(t, func(w http.ResponseWriter, r *http.Request) { + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + }) + test200Server = testServer(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + testAuthServer = func(token *string) *httptest.Server { + return testServer(t, func(w http.ResponseWriter, r *http.Request) { + *token = r.Header.Get(auth.TidepoolSessionTokenHeaderKey) + w.WriteHeader(http.StatusOK) + }) + } + }) + + Context("Delete", func() { + It("returns an error on non-200 responses", func() { + client, ctx := newAlertsClientTest(test404Server) + err := client.Delete(ctx, &Config{}) + Expect(err).Should(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("resource not found"))) + }) + + It("returns nil on success", func() { + client, ctx := newAlertsClientTest(test200Server) + err := client.Delete(ctx, &Config{}) + Expect(err).ShouldNot(HaveOccurred()) + }) + + It("injects an auth token", func() { + token := "" + client, ctx := newAlertsClientTest(testAuthServer(&token)) + _ = client.Delete(ctx, &Config{}) + Expect(token).To(Equal(testToken)) + }) + }) + + Context("Upsert", func() { + It("returns an error on non-200 responses", func() { + client, ctx := newAlertsClientTest(test404Server) + err := client.Upsert(ctx, &Config{}) + Expect(err).Should(HaveOccurred()) + Expect(err).To(MatchError(ContainSubstring("resource not found"))) + }) + + It("returns nil on success", func() { + client, ctx := newAlertsClientTest(test200Server) + err := client.Upsert(ctx, &Config{}) + Expect(err).ShouldNot(HaveOccurred()) + }) + + It("injects an auth token", func() { + token := "" + client, ctx := newAlertsClientTest(testAuthServer(&token)) + _ = client.Upsert(ctx, &Config{}) + Expect(token).To(Equal(testToken)) + }) + }) +}) + +func buildTestClient(s *httptest.Server) *Client { + pCfg := &platform.Config{ + Config: &client.Config{ + Address: s.URL, + }, + } + token := mockTokenProvider(testToken) + pc, err := platform.NewClient(pCfg, platform.AuthorizeAsService) + Expect(err).ToNot(HaveOccurred()) + client := NewClient(pc, token, null.NewLogger()) + return client +} + +func newAlertsClientTest(server *httptest.Server) (*Client, context.Context) { + return buildTestClient(server), contextWithNullLogger() +} + +func contextWithNullLogger() context.Context { + return log.NewContextWithLogger(context.Background(), null.NewLogger()) +} + +type mockTokenProvider string + +func (p mockTokenProvider) ServerSessionToken() (string, error) { + return string(p), nil +} + +func testServer(t GinkgoTInterface, handler http.HandlerFunc) *httptest.Server { + s := httptest.NewServer(http.HandlerFunc(handler)) + t.Cleanup(s.Close) + return s +} diff --git a/alerts/repo.go b/alerts/repo.go deleted file mode 100644 index 3dc80686f..000000000 --- a/alerts/repo.go +++ /dev/null @@ -1 +0,0 @@ -package alerts diff --git a/auth/client/client.go b/auth/client/client.go index bd8adde3d..fd92f6efb 100644 --- a/auth/client/client.go +++ b/auth/client/client.go @@ -5,7 +5,6 @@ import ( "net/http" "github.com/tidepool-org/platform/auth" - "github.com/tidepool-org/platform/config" "github.com/tidepool-org/platform/errors" "github.com/tidepool-org/platform/log" "github.com/tidepool-org/platform/page" @@ -26,11 +25,8 @@ func NewConfig() *Config { } } -func (c *Config) Load(configReporter config.Reporter) error { - if err := c.Config.Load(configReporter); err != nil { - return err - } - return c.ExternalConfig.Load(configReporter.WithScopes("external")) +func (c *Config) Load(loader ConfigLoader) error { + return loader.Load(c) } func (c *Config) Validate() error { @@ -311,3 +307,34 @@ func (c *Client) DeleteRestrictedToken(ctx context.Context, id string) error { url := c.client.ConstructURL("v1", "restricted_tokens", id) return c.client.RequestData(ctx, http.MethodDelete, url, nil, nil, nil) } + +type ConfigLoader interface { + Load(*Config) error +} + +// configHybridLoader combines an ExternalConfigLoader with a platform.ConfigLoader. +// +// Whereas we usually have different implementations, in this case, it's just +// two other loaders together, so no need for multiple other implementations +// here. +type configHybridLoader struct { + ExternalConfigLoader + platform.ConfigLoader +} + +func NewConfigLoader(ext ExternalConfigLoader, plt platform.ConfigLoader) *configHybridLoader { + return &configHybridLoader{ + ExternalConfigLoader: ext, + ConfigLoader: plt, + } +} + +func (l *configHybridLoader) Load(cfg *Config) error { + if err := l.ExternalConfigLoader.Load(cfg.ExternalConfig); err != nil { + return err + } + if err := l.ConfigLoader.Load(cfg.Config); err != nil { + return err + } + return nil +} diff --git a/auth/client/external.go b/auth/client/external.go index 035d53dca..8eb9f45ff 100644 --- a/auth/client/external.go +++ b/auth/client/external.go @@ -9,7 +9,10 @@ import ( "go.uber.org/fx" + "github.com/kelseyhightower/envconfig" + "github.com/tidepool-org/platform/auth" + "github.com/tidepool-org/platform/client" "github.com/tidepool-org/platform/config" "github.com/tidepool-org/platform/errors" "github.com/tidepool-org/platform/log" @@ -26,10 +29,10 @@ const ( ServerSessionTokenTimeoutOnFailureLast = 60 * time.Second ) -var ExternalClientModule = fx.Provide(func(name ServiceName, reporter config.Reporter, logger log.Logger, lifecycle fx.Lifecycle) (auth.ExternalAccessor, error) { +var ExternalClientModule = fx.Provide(func(name ServiceName, loader ExternalConfigLoader, logger log.Logger, lifecycle fx.Lifecycle) (auth.ExternalAccessor, error) { cfg := NewExternalConfig() cfg.Config.UserAgent = string(name) - if err := cfg.Load(reporter.WithScopes("auth", "client", "external")); err != nil { + if err := cfg.Load(loader); err != nil { return nil, err } external, err := NewExternal(cfg, platform.AuthorizeAsService, string(name), logger) @@ -54,37 +57,30 @@ func ProvideServiceName(name string) fx.Option { return fx.Supply(ServiceName(name)) } +func ProvideExternalLoader(reporter config.Reporter) ExternalConfigLoader { + scoped := reporter.WithScopes("auth", "client", "external") + return NewExternalConfigReporterLoader(scoped) +} + type ServiceName string type ExternalConfig struct { *platform.Config - ServerSessionTokenSecret string - ServerSessionTokenTimeout time.Duration + ServerSessionTokenSecret string `envconfig:"TIDEPOOL_AUTH_CLIENT_EXTERNAL_SERVER_SESSION_TOKEN_SECRET"` + ServerSessionTokenTimeout time.Duration `envconfig:"TIDEPOOL_AUTH_CLIENT_EXTERNAL_SERVER_SESSION_TOKEN_TIMEOUT" default:"1h"` } func NewExternalConfig() *ExternalConfig { return &ExternalConfig{ Config: platform.NewConfig(), - ServerSessionTokenTimeout: 3600 * time.Second, + ServerSessionTokenTimeout: ServerSessionTokenTimeout, } } -func (e *ExternalConfig) Load(configReporter config.Reporter) error { - if err := e.Config.Load(configReporter); err != nil { - return err - } - - e.ServerSessionTokenSecret = configReporter.GetWithDefault("server_session_token_secret", "") - if serverSessionTokenTimeoutString, err := configReporter.Get("server_session_token_timeout"); err == nil { - var serverSessionTokenTimeoutInteger int64 - serverSessionTokenTimeoutInteger, err = strconv.ParseInt(serverSessionTokenTimeoutString, 10, 0) - if err != nil { - return errors.New("server session token timeout is invalid") - } - e.ServerSessionTokenTimeout = time.Duration(serverSessionTokenTimeoutInteger) * time.Second - } +const ServerSessionTokenTimeout = time.Hour - return nil +func (e *ExternalConfig) Load(loader ExternalConfigLoader) error { + return loader.Load(e) } func (e *ExternalConfig) Validate() error { @@ -349,3 +345,77 @@ func (e *External) serverSessionToken() string { return e.serverSessionTokenSafe } + +// ExternalConfigLoader abstracts the method by which config values are loaded. +type ExternalConfigLoader interface { + // Load sets config values for the properties of ExternalConfig. + Load(*ExternalConfig) error +} + +// externalConfigReporterLoader adapts a config.Reporter to implement ConfigLoader. +type externalConfigReporterLoader struct { + Reporter config.Reporter + platform.ConfigLoader +} + +func NewExternalConfigReporterLoader(reporter config.Reporter) *externalConfigReporterLoader { + return &externalConfigReporterLoader{ + Reporter: reporter, + ConfigLoader: platform.NewConfigReporterLoader(reporter), + } +} + +// Load implements ConfigLoader. +func (l *externalConfigReporterLoader) Load(cfg *ExternalConfig) error { + if err := l.ConfigLoader.Load(cfg.Config); err != nil { + return err + } + cfg.ServerSessionTokenSecret = l.Reporter.GetWithDefault("server_session_token_secret", "") + if serverSessionTokenTimeoutString, err := l.Reporter.Get("server_session_token_timeout"); err == nil { + var serverSessionTokenTimeoutInteger int64 + serverSessionTokenTimeoutInteger, err = strconv.ParseInt(serverSessionTokenTimeoutString, 10, 0) + if err != nil { + return errors.New("server session token timeout is invalid") + } + cfg.ServerSessionTokenTimeout = time.Duration(serverSessionTokenTimeoutInteger) * time.Second + } + + return nil +} + +// externalEnvconfigLoader adapts envconfig to implement ConfigLoader. +type externalEnvconfigLoader struct { + platform.ConfigLoader +} + +// NewExternalEnvconfigLoader loads values via envconfig. +// +// If loader is nil, it defaults to envconfig for platform values. +func NewExternalEnvconfigLoader(loader platform.ConfigLoader) *externalEnvconfigLoader { + if loader == nil { + loader = platform.NewEnvconfigLoader(nil) + } + return &externalEnvconfigLoader{ + ConfigLoader: loader, + } +} + +// Load implements ConfigLoader. +func (l *externalEnvconfigLoader) Load(cfg *ExternalConfig) error { + eeCfg := &struct { + Address string `envconfig:"TIDEPOOL_AUTH_CLIENT_EXTERNAL_ADDRESS" required:"true"` + *ExternalConfig + }{ExternalConfig: cfg} + if err := envconfig.Process(client.EnvconfigEmptyPrefix, eeCfg); err != nil { + return err + } + // Override the client.Config.Address. It's not possible to change the + // envconfig tag on the config.Client at runtime. In addition, we don't + // want to use the envconfig.Prefix so that the code is more easily + // searched. The results is that we have to override this value. + cfg.Config.Config.Address = eeCfg.Address + if err := l.ConfigLoader.Load(cfg.Config); err != nil { + return err + } + return nil +} diff --git a/auth/service/service/service.go b/auth/service/service/service.go index a95afa393..abc501197 100644 --- a/auth/service/service/service.go +++ b/auth/service/service/service.go @@ -245,7 +245,9 @@ func (s *Service) initializeDataSourceClient() error { cfg := platform.NewConfig() cfg.UserAgent = s.UserAgent() - if err := cfg.Load(s.ConfigReporter().WithScopes("data_source", "client")); err != nil { + reporter := s.ConfigReporter().WithScopes("data_source", "client") + loader := platform.NewConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load data source client config") } @@ -306,7 +308,9 @@ func (s *Service) initializeTaskClient() error { cfg := platform.NewConfig() cfg.UserAgent = s.UserAgent() - if err := cfg.Load(s.ConfigReporter().WithScopes("task", "client")); err != nil { + reporter := s.ConfigReporter().WithScopes("task", "client") + loader := platform.NewConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load task client config") } @@ -359,7 +363,9 @@ func (s *Service) initializeAuthClient() error { cfg := client.NewExternalConfig() cfg.UserAgent = s.UserAgent() - if err := cfg.Load(s.ConfigReporter().WithScopes("auth", "client", "external")); err != nil { + reporter := s.ConfigReporter().WithScopes("auth", "client", "external") + loader := client.NewExternalConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load auth client config") } diff --git a/client/client.go b/client/client.go index eb4682250..7468948ba 100644 --- a/client/client.go +++ b/client/client.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "net/http" "net/url" "reflect" @@ -118,7 +117,9 @@ func (c *Client) createRequest(ctx context.Context, method string, url string, m return nil, errors.New("url is missing") } - mutators = append(mutators, request.NewHeaderMutator("User-Agent", c.userAgent)) + if c.userAgent != "" { + mutators = append(mutators, request.NewHeaderMutator("User-Agent", c.userAgent)) + } var body io.Reader if requestBody != nil { @@ -173,7 +174,7 @@ func (c *Client) handleResponse(ctx context.Context, res *http.Response, req *ht serializable := &errors.Serializable{} - if bites, err := ioutil.ReadAll(io.LimitReader(res.Body, 1<<20)); err != nil { + if bites, err := io.ReadAll(io.LimitReader(res.Body, 1<<20)); err != nil { return nil, errors.Wrap(err, "unable to read response body") } else if len(bites) == 0 { logger.Error("Response body is empty, using defacto error for status code") @@ -228,6 +229,6 @@ func responseBodyFromBytes(bites []byte) interface{} { } func drainAndClose(reader io.ReadCloser) { - io.Copy(ioutil.Discard, reader) + io.Copy(io.Discard, reader) reader.Close() } diff --git a/client/client_test.go b/client/client_test.go index 169f5f2d0..c7c778831 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -33,13 +33,10 @@ type ResponseBody struct { } var _ = Describe("Client", func() { - var userAgent string var config *client.Config BeforeEach(func() { - userAgent = testHttp.NewUserAgent() config = client.NewConfig() - config.UserAgent = userAgent }) Context("New", func() { @@ -225,6 +222,45 @@ var _ = Describe("Client", func() { } }) + Context("with a user agent", func() { + var userAgent = testHttp.NewUserAgent() + JustBeforeEach(func() { + config.UserAgent = userAgent + clnt, err = client.New(config) + Expect(err).ToNot(HaveOccurred()) + }) + + It("sets the User-Agent header in requests", func() { + server.AppendHandlers(CombineHandlers( + VerifyHeaderKV("User-Agent", userAgent), + RespondWith(http.StatusNoContent, nil))) + + _, err = clnt.RequestStreamWithHTTPClient(ctx, method, url, mutators, nil, inspectors, httpClient) + + Expect(err).ToNot(HaveOccurred()) + Expect(server.ReceivedRequests()).To(HaveLen(1)) + }) + }) + + Context("without a user agent", func() { + JustBeforeEach(func() { + config.UserAgent = "" + clnt, err = client.New(config) + Expect(err).ToNot(HaveOccurred()) + }) + + It("doesn't set one (and the Go default is used)", func() { + server.AppendHandlers(CombineHandlers( + VerifyHeaderKV("User-Agent", "Go-http-client/1.1"), + RespondWith(http.StatusNoContent, nil))) + + _, err = clnt.RequestStreamWithHTTPClient(ctx, method, url, mutators, nil, inspectors, httpClient) + + Expect(err).ToNot(HaveOccurred()) + Expect(server.ReceivedRequests()).To(HaveLen(1)) + }) + }) + It("returns error if http client is missing", func() { reader, err = clnt.RequestStreamWithHTTPClient(ctx, method, url, mutators, requestBody, inspectors, nil) Expect(err).To(MatchError("http client is missing")) @@ -286,7 +322,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(nil), RespondWith(http.StatusOK, []byte(responseString), responseHeaders), @@ -314,7 +349,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -339,7 +373,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -361,7 +394,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -383,7 +415,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -405,7 +436,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -430,7 +460,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -452,7 +481,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -474,7 +502,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -496,7 +523,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -521,7 +547,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -543,7 +568,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -565,7 +589,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -587,7 +610,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(nil), RespondWith(http.StatusOK, []byte(responseString)), @@ -609,7 +631,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody([]byte(requestString)), RespondWith(http.StatusOK, []byte(responseString)), @@ -631,7 +652,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -704,7 +724,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(nil), RespondWith(http.StatusOK, test.MarshalResponseBody(&ResponseBody{Response: responseString}), responseHeaders), @@ -732,7 +751,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(nil), RespondWith(http.StatusOK, test.MarshalResponseBody(&ResponseBody{Response: responseString}), responseHeaders), @@ -753,7 +771,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -777,7 +794,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -798,7 +814,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -819,7 +834,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -840,7 +854,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -864,7 +877,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -885,7 +897,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -906,7 +917,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -927,7 +937,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -945,13 +954,11 @@ var _ = Describe("Client", func() { Context("with an unexpected response 500 with deserializable error body", func() { var responseErr error - BeforeEach(func() { responseErr = errorsTest.RandomError() server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -972,7 +979,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -993,7 +999,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -1015,7 +1020,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -1037,7 +1041,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(nil), RespondWith(http.StatusOK, test.MarshalResponseBody(&ResponseBody{Response: responseString}), responseHeaders), @@ -1058,7 +1061,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody([]byte(requestString)), RespondWith(http.StatusOK, test.MarshalResponseBody(&ResponseBody{Response: responseString}), responseHeaders), @@ -1079,7 +1081,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), @@ -1101,7 +1102,6 @@ var _ = Describe("Client", func() { server.AppendHandlers( CombineHandlers( VerifyRequest(method, path, fmt.Sprintf("%s=%s", parameterMutator.Key, parameterMutator.Value)), - VerifyHeaderKV("User-Agent", userAgent), VerifyHeaderKV("Content-Type", "application/json; charset=utf-8"), VerifyHeaderKV(headerMutator.Key, headerMutator.Value), VerifyBody(test.MarshalRequestBody(requestBody)), diff --git a/client/config.go b/client/config.go index d2cb93143..b74720633 100644 --- a/client/config.go +++ b/client/config.go @@ -2,31 +2,33 @@ package client import ( "net/url" - "time" + + "github.com/kelseyhightower/envconfig" "github.com/tidepool-org/platform/config" "github.com/tidepool-org/platform/errors" ) type Config struct { - Address string - UserAgent string - Timeout *time.Duration + Address string // this should be overridden for loaders using envconfig + // UserAgent is an optional way for a client to identify itself. + // + // This is usually set to the name of the service that's using the + // client. If left empty, the default Go http.Client value should be used. + // + // This value can be helpful when debugging. But remember that these + // values can be spoofed, so when in doubt, verify the client's source IP. + // + // More info: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent + UserAgent string `envconfig:"TIDEPOOL_USER_AGENT"` } func NewConfig() *Config { return &Config{} } -func (c *Config) Load(configReporter config.Reporter) error { - if configReporter == nil { - return errors.New("config reporter is missing") - } - - c.Address = configReporter.GetWithDefault("address", c.Address) - c.UserAgent = configReporter.GetWithDefault("user_agent", c.UserAgent) - - return nil +func (c *Config) Load(loader ConfigLoader) error { + return loader.Load(c) } func (c *Config) Validate() error { @@ -35,9 +37,48 @@ func (c *Config) Validate() error { } else if _, err := url.Parse(c.Address); err != nil { return errors.New("address is invalid") } - if c.UserAgent == "" { - return errors.New("user agent is missing") + + return nil +} + +// ConfigLoader abstracts the method by which config values are loaded. +type ConfigLoader interface { + // Load sets config values for the properties of Config. + Load(*Config) error +} + +// configReporterLoader adapts a config.Reporter to implement ConfigLoader. +type configReporterLoader struct { + Reporter config.Reporter +} + +func NewConfigReporterLoader(reporter config.Reporter) *configReporterLoader { + return &configReporterLoader{ + Reporter: reporter, } +} +// Load implements ConfigLoader. +func (l *configReporterLoader) Load(cfg *Config) error { + cfg.Address = l.Reporter.GetWithDefault("address", cfg.Address) + cfg.UserAgent = l.Reporter.GetWithDefault("user_agent", cfg.UserAgent) return nil } + +// EnvconfigEmptyPrefix should be the empty string. +// +// By forcing the use of the environment variable name in each tag, we aim to +// make the code more easily searchable. +const EnvconfigEmptyPrefix = "" + +// envconfigLoader adapts envconfig to implement ConfigLoader. +type envconfigLoader struct{} + +func NewEnvconfigLoader() *envconfigLoader { + return &envconfigLoader{} +} + +// Load implements ConfigLoader. +func (l *envconfigLoader) Load(cfg *Config) error { + return envconfig.Process(EnvconfigEmptyPrefix, cfg) +} diff --git a/client/config_test.go b/client/config_test.go index a9245e472..4c22fbcf2 100644 --- a/client/config_test.go +++ b/client/config_test.go @@ -38,22 +38,20 @@ var _ = Describe("Config", func() { Context("Load", func() { var configReporter *configTest.Reporter + var loader client.ConfigLoader BeforeEach(func() { configReporter = configTest.NewReporter() configReporter.Config["address"] = address configReporter.Config["user_agent"] = userAgent - }) - - It("returns an error if config reporter is missing", func() { - Expect(cfg.Load(nil)).To(MatchError("config reporter is missing")) + loader = client.NewConfigReporterLoader(configReporter) }) It("uses existing address if not set", func() { existingAddress := testHttp.NewAddress() cfg.Address = existingAddress delete(configReporter.Config, "address") - Expect(cfg.Load(configReporter)).To(Succeed()) + Expect(cfg.Load(loader)).To(Succeed()) Expect(cfg.Address).To(Equal(existingAddress)) Expect(cfg.UserAgent).To(Equal(userAgent)) }) @@ -62,13 +60,13 @@ var _ = Describe("Config", func() { existingUserAgent := testHttp.NewUserAgent() cfg.UserAgent = existingUserAgent delete(configReporter.Config, "user_agent") - Expect(cfg.Load(configReporter)).To(Succeed()) + Expect(cfg.Load(loader)).To(Succeed()) Expect(cfg.Address).To(Equal(address)) Expect(cfg.UserAgent).To(Equal(existingUserAgent)) }) It("returns successfully and uses values from config reporter", func() { - Expect(cfg.Load(configReporter)).To(Succeed()) + Expect(cfg.Load(loader)).To(Succeed()) Expect(cfg.Address).To(Equal(address)) Expect(cfg.UserAgent).To(Equal(userAgent)) }) @@ -91,11 +89,6 @@ var _ = Describe("Config", func() { Expect(cfg.Validate()).To(MatchError("address is invalid")) }) - It("returns an error if the user agent is missing", func() { - cfg.UserAgent = "" - Expect(cfg.Validate()).To(MatchError("user agent is missing")) - }) - It("returns success", func() { Expect(cfg.Validate()).To(Succeed()) Expect(cfg.Address).To(Equal(address)) diff --git a/data/client/client.go b/data/client/client.go index 4449ecde9..b824202b8 100644 --- a/data/client/client.go +++ b/data/client/client.go @@ -11,14 +11,15 @@ import ( "github.com/tidepool-org/platform/errors" "github.com/tidepool-org/platform/page" "github.com/tidepool-org/platform/platform" - "github.com/tidepool-org/platform/pointer" "github.com/tidepool-org/platform/request" "github.com/tidepool-org/platform/service" structureValidator "github.com/tidepool-org/platform/structure/validator" ) const ( - ExtendedTimeout = 300 * time.Second + // ExtendedTimeout is used by requests that we expect to take longer than + // usual. + ExtendedTimeout = 5 * time.Minute ) // TODO: Move interface to data package once upload dependency broken @@ -51,15 +52,8 @@ func New(cfg *platform.Config, authorizeAs platform.AuthorizeAs) (*ClientImpl, e return nil, err } - cfg.Timeout = pointer.FromDuration(ExtendedTimeout) - extendedTimeoutClient, err := platform.NewClient(cfg, authorizeAs) - if err != nil { - return nil, err - } - return &ClientImpl{ - client: clnt, - extendedTimeoutClient: extendedTimeoutClient, + client: clnt, }, nil } @@ -217,9 +211,11 @@ func (c *ClientImpl) UpdateBGMSummary(ctx context.Context, userId string) (*type func (c *ClientImpl) BackfillSummaries(ctx context.Context, typ string) (int, error) { var count int - url := c.extendedTimeoutClient.ConstructURL("v1", "summaries", "backfill", typ) + url := c.client.ConstructURL("v1", "summaries", "backfill", typ) - if err := c.extendedTimeoutClient.RequestData(ctx, http.MethodPost, url, nil, nil, &count); err != nil { + ctxWithTimeout, cancel := context.WithTimeout(ctx, ExtendedTimeout) + defer cancel() + if err := c.client.RequestData(ctxWithTimeout, http.MethodPost, url, nil, nil, &count); err != nil { return count, errors.Wrap(err, "backfill request returned an error") } diff --git a/data/client/client_test.go b/data/client/client_test.go index 6a9e3ec48..24f09550e 100644 --- a/data/client/client_test.go +++ b/data/client/client_test.go @@ -43,13 +43,6 @@ var _ = Describe("Client", func() { Expect(clnt).To(BeNil()) }) - It("returns an error if config user agent is missing", func() { - config.UserAgent = "" - clnt, err := dataClient.New(config, platform.AuthorizeAsService) - Expect(err).To(MatchError("config is invalid; user agent is missing")) - Expect(clnt).To(BeNil()) - }) - It("returns success", func() { clnt, err := dataClient.New(config, platform.AuthorizeAsService) Expect(err).ToNot(HaveOccurred()) diff --git a/data/service/service/standard.go b/data/service/service/standard.go index 9b73be431..9111aaabc 100644 --- a/data/service/service/standard.go +++ b/data/service/service/standard.go @@ -154,7 +154,9 @@ func (s *Standard) initializeMetricClient() error { cfg := platform.NewConfig() cfg.UserAgent = s.UserAgent() - if err := cfg.Load(s.ConfigReporter().WithScopes("metric", "client")); err != nil { + reporter := s.ConfigReporter().WithScopes("metric", "client") + loader := platform.NewConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load metric client config") } @@ -174,7 +176,9 @@ func (s *Standard) initializePermissionClient() error { cfg := platform.NewConfig() cfg.UserAgent = s.UserAgent() - if err := cfg.Load(s.ConfigReporter().WithScopes("permission", "client")); err != nil { + reporter := s.ConfigReporter().WithScopes("permission", "client") + loader := platform.NewConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load permission client config") } diff --git a/metric/client/client_test.go b/metric/client/client_test.go index 1e49ac893..ccdcef0dd 100644 --- a/metric/client/client_test.go +++ b/metric/client/client_test.go @@ -66,13 +66,6 @@ var _ = Describe("Client", func() { Expect(clnt).To(BeNil()) }) - It("returns an error if config user agent is missing", func() { - config.UserAgent = "" - clnt, err := metricClient.New(config, platform.AuthorizeAsUser, name, versionReporter) - Expect(err).To(MatchError("config is invalid; user agent is missing")) - Expect(clnt).To(BeNil()) - }) - It("returns success", func() { clnt, err := metricClient.New(config, platform.AuthorizeAsUser, name, versionReporter) Expect(err).ToNot(HaveOccurred()) diff --git a/platform/client.go b/platform/client.go index c921da055..aadac65d4 100644 --- a/platform/client.go +++ b/platform/client.go @@ -4,7 +4,6 @@ import ( "context" "io" "net/http" - "time" "github.com/tidepool-org/platform/auth" "github.com/tidepool-org/platform/client" @@ -17,7 +16,6 @@ type AuthorizeAs int const ( AuthorizeAsService AuthorizeAs = iota AuthorizeAsUser - DefaultTimeout = 60 * time.Second ) type Client struct { @@ -48,22 +46,12 @@ func NewClient(cfg *Config, authorizeAs AuthorizeAs) (*Client, error) { // return errors.New("service secret is missing") // } // } - var timeout time.Duration - if cfg.Timeout != nil { - timeout = *cfg.Timeout - } else { - timeout = DefaultTimeout - } - - httpClient := &http.Client{ - Timeout: timeout, - } return &Client{ Client: clnt, authorizeAs: authorizeAs, serviceSecret: cfg.ServiceSecret, - httpClient: httpClient, + httpClient: &http.Client{}, }, nil } @@ -83,6 +71,10 @@ func (c *Client) Mutators(ctx context.Context) ([]request.RequestMutator, error) } else if serverSessionToken := auth.ServerSessionTokenFromContext(ctx); serverSessionToken != "" { authorizationMutator = NewSessionTokenHeaderMutator(serverSessionToken) } else { + // TODO: Should this really error? It might be nice to allow other + // clients the option of handling authentication on their own if + // they'd like, rather than enforcing that this method must be + // used. return nil, errors.New("service secret is missing") } } else { diff --git a/platform/config.go b/platform/config.go index f52347495..f9de53581 100644 --- a/platform/config.go +++ b/platform/config.go @@ -1,13 +1,16 @@ package platform import ( + "github.com/kelseyhightower/envconfig" + "github.com/tidepool-org/platform/client" "github.com/tidepool-org/platform/config" ) +// Config extends client.Config with additional properties. type Config struct { *client.Config - ServiceSecret string + ServiceSecret string `envconfig:"TIDEPOOL_SERVICE_SECRET"` // this should be overridden for loaders using envconfig } func NewConfig() *Config { @@ -16,12 +19,58 @@ func NewConfig() *Config { } } -func (c *Config) Load(configReporter config.Reporter) error { - if err := c.Config.Load(configReporter); err != nil { +func (c *Config) Load(loader ConfigLoader) error { + return loader.Load(c) +} + +// ConfigLoader abstracts the method by which config values are loaded. +type ConfigLoader interface { + Load(*Config) error +} + +// configReporterLoader adapts config.Reporter to implement ConfigLoader. +type configReporterLoader struct { + Reporter config.Reporter + client.ConfigLoader +} + +func NewConfigReporterLoader(reporter config.Reporter) *configReporterLoader { + return &configReporterLoader{ + ConfigLoader: client.NewConfigReporterLoader(reporter), + Reporter: reporter, + } +} + +// LoadPlatform implements ConfigLoader. +func (l *configReporterLoader) Load(cfg *Config) error { + if err := l.ConfigLoader.Load(cfg.Config); err != nil { return err } + cfg.ServiceSecret = l.Reporter.GetWithDefault("service_secret", cfg.ServiceSecret) + return nil +} - c.ServiceSecret = configReporter.GetWithDefault("service_secret", c.ServiceSecret) +// envconfigLoader adapts envconfig to implement ConfigLoader. +type envconfigLoader struct { + client.ConfigLoader +} - return nil +// NewEnvconfigLoader loads values via envconfig. +// +// If loader is nil, it defaults to envconfig for client values. +func NewEnvconfigLoader(loader client.ConfigLoader) *envconfigLoader { + if loader == nil { + loader = client.NewEnvconfigLoader() + } + return &envconfigLoader{ + ConfigLoader: loader, + } +} + +// Load implements ConfigLoader. +func (l *envconfigLoader) Load(cfg *Config) error { + if err := l.ConfigLoader.Load(cfg.Config); err != nil { + return err + } + return envconfig.Process(client.EnvconfigEmptyPrefix, cfg) } diff --git a/platform/config_test.go b/platform/config_test.go index 812692779..edba20a42 100644 --- a/platform/config_test.go +++ b/platform/config_test.go @@ -45,23 +45,21 @@ var _ = Describe("Config", func() { Context("Load", func() { var configReporter *configTest.Reporter + var loader platform.ConfigLoader BeforeEach(func() { configReporter = configTest.NewReporter() configReporter.Config["address"] = address configReporter.Config["user_agent"] = userAgent configReporter.Config["service_secret"] = serviceSecret - }) - - It("returns an error if config reporter is missing", func() { - Expect(cfg.Load(nil)).To(MatchError("config reporter is missing")) + loader = platform.NewConfigReporterLoader(configReporter) }) It("uses existing address if not set", func() { existingAddress := testHttp.NewAddress() cfg.Address = existingAddress delete(configReporter.Config, "address") - Expect(cfg.Load(configReporter)).To(Succeed()) + Expect(cfg.Load(loader)).To(Succeed()) Expect(cfg.Address).To(Equal(existingAddress)) Expect(cfg.UserAgent).To(Equal(userAgent)) Expect(cfg.ServiceSecret).To(Equal(serviceSecret)) @@ -71,7 +69,7 @@ var _ = Describe("Config", func() { existingUserAgent := testHttp.NewUserAgent() cfg.UserAgent = existingUserAgent delete(configReporter.Config, "user_agent") - Expect(cfg.Load(configReporter)).To(Succeed()) + Expect(cfg.Load(loader)).To(Succeed()) Expect(cfg.Config).ToNot(BeNil()) Expect(cfg.Address).To(Equal(address)) Expect(cfg.UserAgent).To(Equal(existingUserAgent)) @@ -82,14 +80,14 @@ var _ = Describe("Config", func() { existingServiceSecret := test.RandomStringFromRangeAndCharset(1, 64, test.CharsetText) cfg.ServiceSecret = existingServiceSecret delete(configReporter.Config, "service_secret") - Expect(cfg.Load(configReporter)).To(Succeed()) + Expect(cfg.Load(loader)).To(Succeed()) Expect(cfg.Address).To(Equal(address)) Expect(cfg.UserAgent).To(Equal(userAgent)) Expect(cfg.ServiceSecret).To(Equal(existingServiceSecret)) }) It("returns successfully and uses values from config reporter", func() { - Expect(cfg.Load(configReporter)).To(Succeed()) + Expect(cfg.Load(loader)).To(Succeed()) Expect(cfg.Config).ToNot(BeNil()) Expect(cfg.Address).To(Equal(address)) Expect(cfg.UserAgent).To(Equal(userAgent)) @@ -115,11 +113,6 @@ var _ = Describe("Config", func() { Expect(cfg.Validate()).To(MatchError("address is invalid")) }) - It("returns an error if the user agent is missing", func() { - cfg.UserAgent = "" - Expect(cfg.Validate()).To(MatchError("user agent is missing")) - }) - It("returns success", func() { Expect(cfg.Validate()).To(Succeed()) Expect(cfg.Config).ToNot(BeNil()) diff --git a/prescription/application/application.go b/prescription/application/application.go index 558f7fb0a..f7cfd7cbc 100644 --- a/prescription/application/application.go +++ b/prescription/application/application.go @@ -19,6 +19,7 @@ var Prescription = fx.Options( client.ProvideServiceName("prescription"), client.ExternalClientModule, fx.Provide( + client.ProvideExternalLoader, prescriptionMongo.NewStore, prescriptionMongo.NewStatusReporter, service.NewDeviceSettingsValidator, diff --git a/service/service/DEPRECATED_service.go b/service/service/DEPRECATED_service.go index c48a50f73..5240c8f40 100644 --- a/service/service/DEPRECATED_service.go +++ b/service/service/DEPRECATED_service.go @@ -67,7 +67,11 @@ func (d *DEPRECATEDService) initializeAuthClient() error { cfg := authClient.NewConfig() cfg.UserAgent = d.UserAgent() cfg.ExternalConfig.UserAgent = d.UserAgent() - if err := cfg.Load(d.ConfigReporter().WithScopes("auth", "client")); err != nil { + reporter := d.ConfigReporter().WithScopes("auth", "client") + ext := authClient.NewExternalConfigReporterLoader(reporter.WithScopes("external")) + plt := platform.NewConfigReporterLoader(reporter) + loader := authClient.NewConfigLoader(ext, plt) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load auth client config") } diff --git a/service/service/authenticated.go b/service/service/authenticated.go index fc003a89b..c5621d6df 100644 --- a/service/service/authenticated.go +++ b/service/service/authenticated.go @@ -37,7 +37,11 @@ func (a *Authenticated) initializeAuthClient() error { cfg := authClient.NewConfig() cfg.UserAgent = a.UserAgent() cfg.ExternalConfig.UserAgent = a.UserAgent() - if err := cfg.Load(a.ConfigReporter().WithScopes("auth", "client")); err != nil { + reporter := a.ConfigReporter().WithScopes("auth", "client") + ext := authClient.NewExternalConfigReporterLoader(reporter.WithScopes("external")) + plt := platform.NewConfigReporterLoader(reporter) + loader := authClient.NewConfigLoader(ext, plt) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load auth client config") } diff --git a/task/service/service/service.go b/task/service/service/service.go index abae9e3b7..874254535 100644 --- a/task/service/service/service.go +++ b/task/service/service/service.go @@ -168,7 +168,9 @@ func (s *Service) initializeDataClient() error { cfg := platform.NewConfig() cfg.UserAgent = s.UserAgent() - if err := cfg.Load(s.ConfigReporter().WithScopes("data", "client")); err != nil { + reporter := s.ConfigReporter().WithScopes("data", "client") + loader := platform.NewConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load data client config") } @@ -195,7 +197,9 @@ func (s *Service) initializeDataSourceClient() error { cfg := platform.NewConfig() cfg.UserAgent = s.UserAgent() - if err := cfg.Load(s.ConfigReporter().WithScopes("data_source", "client")); err != nil { + reporter := s.ConfigReporter().WithScopes("data_source", "client") + loader := platform.NewConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load data source client config") } @@ -228,7 +232,9 @@ func (s *Service) initializeDexcomClient() error { cfg := client.NewConfig() cfg.UserAgent = s.UserAgent() - if err = cfg.Load(s.ConfigReporter().WithScopes("dexcom", "client")); err != nil { + reporter := s.ConfigReporter().WithScopes("dexcom", "client") + loader := client.NewConfigReporterLoader(reporter) + if err = cfg.Load(loader); err != nil { return errors.Wrap(err, "unable to load dexcom client config") } diff --git a/user/client/client.go b/user/client/client.go index a516dc0ed..8fab73a28 100644 --- a/user/client/client.go +++ b/user/client/client.go @@ -34,7 +34,9 @@ func NewDefaultClient(p Params) (user.Client, error) { cfg := platform.NewConfig() cfg.UserAgent = p.UserAgent - if err := cfg.Load(p.ConfigReporter.WithScopes("user", "client")); err != nil { + reporter := p.ConfigReporter.WithScopes("user", "client") + loader := platform.NewConfigReporterLoader(reporter) + if err := cfg.Load(loader); err != nil { return nil, errors.Wrap(err, "unable to get user client config") }