Skip to content

Commit

Permalink
[BACK-2737] adds an alerts client (#679)
Browse files Browse the repository at this point in the history
* remove Timeout from config.Config

Removing the timeout simplifies the Config and related Clients which no longer
need to be concerned with it. It's used by a single client request. That
request now sets a timeout on the context, preventing a bunch of unnecessary
complication everywhere else.

* deprecated functions are deprecated

* add ConfigLoader interface

This interface provides a way to migrate away from the env.reporter. The
intention is to use envconfig in future work.

I tried implementing the existing config.Reporter interface with envconfig,
but wasn't able to find a solution that felt good. I wrote a solution using
reflection, but it felt overcomplicated (as anything using reflection does),
and I tried a brute force approach, but it was heavily coupled to the config
structs, and was brittle as a result.

This solution uses an inversion of responsibility pattern. Unfortunately it
gets a bit repetitive with all the config/client/platform terminology that
already exists, but each individual piece at least is simple and
straightforward. Hopefully in time, we can further simplify or even just
better name some of the pieces to let the code read more clearly.

BACK-2500

* adds an alerts client

BACK-2500

* make client config's UserAgent property optional

The value is useful for debugging, but it's not being set shouldn't stop a
test or any other process from running.

Also fixed a few calls to Go stdlib deprecated functions.

* update mongo/travis config

Stolen from Alex's good work in #681

* force snyk to use govendor as its package manager

Its autodetection isn't working correctly.
  • Loading branch information
ewollesen authored Nov 15, 2023
1 parent b8166fb commit f8e6c2c
Show file tree
Hide file tree
Showing 24 changed files with 601 additions and 171 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 7 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
136 changes: 136 additions & 0 deletions alerts/client.go
Original file line number Diff line number Diff line change
@@ -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"`
}
118 changes: 118 additions & 0 deletions alerts/client_test.go
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 0 additions & 1 deletion alerts/repo.go

This file was deleted.

39 changes: 33 additions & 6 deletions auth/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit f8e6c2c

Please sign in to comment.