Skip to content

Commit

Permalink
Cleanup: Remove v2beta resources and Legacy query logic (#172)
Browse files Browse the repository at this point in the history 
* Remove deprecated resources
* Remove v1 translated handler
* Remove proxy logic
* Remove v2beta resources from api spec
* Update references to v2/platform/monitoring
* Update test references to v2/nearest
* Update references to /v2/nearest
  • Loading branch information
@stephen-soltesz
stephen-soltesz committed Dec 1, 2023
1 parent 43008cc commit 80f3d8b
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 686 deletions.
10 changes: 5 additions & 5 deletions cmd/monitoring-token/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

The locate service issues access tokens based on the client use-case.

Clients using the `/v2/query` path receive access tokens for a specific
service. Clients using the `/v2/monitoring` path receive access tokens
Clients using the `/v2/nearest` path receive access tokens for a specific
service. Clients using the `/v2/platform/monitoring` path receive access tokens
specifically for monitoring.

These special purpose, monitoring access tokens allow the target server to
Expand All @@ -19,10 +19,10 @@ The locate service uses a private signing key that issues access tokens. A
privileged, end to end monitoring client will also have the ability to create
access tokens to request monitoring access tokens from the locate service.

Basic sequence diagram for a /v2/monitoring request:
Basic sequence diagram for a /v2/platform/monitoring request:

```txt
Get access token: monitoring-token <------> locate/v2/monitoring
Get access token: monitoring-token <------> locate/v2/platform/monitoring
Use access token: e2e-client -------> service
```

Expand All @@ -31,7 +31,7 @@ get an access token from the locate service, pass a service URL to a command
through an environment variable. For example:

```sh
export LOCATE_URL=https://locate-dot-mlab-sandbox.appspot.com/v2/monitoring/
export LOCATE_URL=https://locate-dot-mlab-sandbox.appspot.com/v2/platform/monitoring/
export MONITORING_SIGNER_KEY=/path/to/key.json

monitoring-token \
Expand Down
4 changes: 2 additions & 2 deletions cmd/monitoring-token/monitoring-token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
)

var (
locate = flagx.MustNewURL("http://localhost:8080/v2/monitoring/")
locate = flagx.MustNewURL("http://localhost:8080/v2/platform/monitoring/")
privKey flagx.FileBytes
machine string
service string
Expand Down Expand Up @@ -56,7 +56,7 @@ func main() {
flag.Parse()
rtx.Must(flagx.ArgsFromEnvWithLog(flag.CommandLine, false), "Failed to read args from env")

// This process signs access tokens for /v2/monitoring requests to the
// This process signs access tokens for /v2/platform/monitoring requests to the
// locate service. NOTE: the locate service MUST be configured with the
// corresponding public key to verify these access tokens.
priv, err := token.NewSigner(privKey)
Expand Down
4 changes: 2 additions & 2 deletions cmd/monitoring-token/monitoring-token_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ func Test_main(t *testing.T) {
}
privKey = []byte(insecurePrivateKey)
mux := http.NewServeMux()
mux.HandleFunc("/v2/monitoring/", handler)
mux.HandleFunc("/v2/platform/monitoring/", handler)
srv := httptest.NewServer(mux)
defer srv.Close()
var err error
locate.URL, err = url.Parse(srv.URL + "/v2/monitoring/")
locate.URL, err = url.Parse(srv.URL + "/v2/platform/monitoring/")
rtx.Must(err, "failed to parse url: %q", srv.URL)

main()
Expand Down
61 changes: 3 additions & 58 deletions handler/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,12 @@ type Signer interface {
type Client struct {
Signer
project string
Locator
LocatorV2
ClientLocator
PrometheusClient
targetTmpl *template.Template
}

// Locator defines how the TranslatedQuery handler requests machines nearest to
// the client.
type Locator interface {
Nearest(ctx context.Context, service, lat, lon string) ([]v2.Target, error)
}

// LocatorV2 defines how the Nearest handler requests machines nearest to the
// client.
type LocatorV2 interface {
Expand Down Expand Up @@ -87,11 +80,10 @@ func init() {
}

// NewClient creates a new client.
func NewClient(project string, private Signer, locator Locator, locatorV2 LocatorV2, client ClientLocator, prom PrometheusClient) *Client {
func NewClient(project string, private Signer, locatorV2 LocatorV2, client ClientLocator, prom PrometheusClient) *Client {
return &Client{
Signer: private,
project: project,
Locator: locator,
LocatorV2: locatorV2,
ClientLocator: client,
PrometheusClient: prom,
Expand All @@ -100,11 +92,10 @@ func NewClient(project string, private Signer, locator Locator, locatorV2 Locato
}

// NewClientDirect creates a new client with a target template using only the target machine.
func NewClientDirect(project string, private Signer, locator Locator, locatorV2 LocatorV2, client ClientLocator, prom PrometheusClient) *Client {
func NewClientDirect(project string, private Signer, locatorV2 LocatorV2, client ClientLocator, prom PrometheusClient) *Client {
return &Client{
Signer: private,
project: project,
Locator: locator,
LocatorV2: locatorV2,
ClientLocator: client,
PrometheusClient: prom,
Expand Down Expand Up @@ -141,52 +132,6 @@ func extraParams(hostname string, index int, p paramOpts) url.Values {
return v
}

// TranslatedQuery uses the legacy mlab-ns service for liveness as a
// transitional step in loading state directly.
func (c *Client) TranslatedQuery(rw http.ResponseWriter, req *http.Request) {
req.ParseForm() // Parse any raw query parameters into req.Form url.Values.
result := v2.NearestResult{}
experiment, service := getExperimentAndService(req.URL.Path)
setHeaders(rw)

// Check whether the service is valid before all other steps to fail fast.
ports, ok := static.Configs[service]
if !ok {
result.Error = v2.NewError("config", "Unknown service: "+service, http.StatusBadRequest)
writeResult(rw, result.Error.Status, &result)
return
}

// Look up client location.
loc, err := c.checkClientLocation(rw, req)
if err != nil {
status := http.StatusServiceUnavailable
result.Error = v2.NewError("nearest", "Failed to lookup nearest machines", status)
writeResult(rw, result.Error.Status, &result)
return
}

// Find the nearest targets using provided lat,lon.
targets, err := c.Locator.Nearest(req.Context(), service, loc.Latitude, loc.Longitude)
if err != nil {
status := http.StatusInternalServerError
result.Error = v2.NewError("nearest", "Failed to lookup nearest machines", status)
writeResult(rw, result.Error.Status, &result)
return
}

// Update targets with empty URLs.
for i := range targets {
targets[i].URLs = map[string]string{}
}

pOpts := paramOpts{raw: req.Form, version: "v2_proxy"}
// Populate targets URLs and write out response.
c.populateURLs(targets, ports, experiment, pOpts)
result.Results = targets
writeResult(rw, http.StatusOK, &result)
}

// Nearest uses an implementation of the LocatorV2 interface to look up
// nearest servers.
func (c *Client) Nearest(rw http.ResponseWriter, req *http.Request) {
Expand Down Expand Up @@ -351,7 +296,7 @@ func writeResult(rw http.ResponseWriter, status int, result interface{}) {
}

// getExperimentAndService takes an http request path and extracts the last two
// fields. For correct requests (e.g. "/v2/query/ndt/ndt5"), this will be the
// fields. For correct requests (e.g. "/v2/nearest/ndt/ndt5"), this will be the
// experiment name (e.g. "ndt") and the datatype (e.g. "ndt5").
func getExperimentAndService(p string) (string, string) {
datatype := path.Base(p)
Expand Down
164 changes: 5 additions & 159 deletions handler/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,159 +83,6 @@ func (l *fakeAppEngineLocator) Locate(req *http.Request) (*clientgeo.Location, e
return l.loc, l.err
}

func TestClient_TranslatedQuery(t *testing.T) {
tests := []struct {
name string
path string
signer Signer
locator *fakeLocator
project string
latlon string
header http.Header
wantLatLon string
wantKey string
wantStatus int
}{
{
name: "error-bad-service",
path: "no-experiment-has-this/datatype-name",
wantStatus: http.StatusBadRequest,
},
{
name: "error-nearest-failure",
path: "ndt/ndt5",
header: http.Header{
"X-AppEngine-CityLatLong": []string{"40.3,-70.4"},
},
wantLatLon: "40.3,-70.4", // Client receives lat/lon provided by AppEngine.
locator: &fakeLocator{
err: errors.New("Fake signer error"),
},
wantStatus: http.StatusInternalServerError,
},
{
name: "error-nearest-failure-no-content",
path: "ndt/ndt5",
locator: &fakeLocator{
err: proxy.ErrNoContent,
},
wantStatus: http.StatusServiceUnavailable,
},
{
name: "error-corrupt-latlon",
path: "ndt/ndt5",
signer: &fakeSigner{},
locator: &fakeLocator{
targets: []v2.Target{{Machine: "mlab1-lga0t.measurement-lab.org"}},
},
header: http.Header{
"X-AppEngine-CityLatLong": []string{"corrupt-value"},
},
wantLatLon: "",
wantKey: "ws://:3001/ndt_protocol",
wantStatus: http.StatusServiceUnavailable,
},
{
name: "success-nearest-server",
path: "ndt/ndt5",
signer: &fakeSigner{},
locator: &fakeLocator{
targets: []v2.Target{{Machine: "mlab1-lga0t.measurement-lab.org"}},
},
header: http.Header{
"X-AppEngine-CityLatLong": []string{"40.3,-70.4"},
},
wantLatLon: "40.3,-70.4", // Client receives lat/lon provided by AppEngine.
wantKey: "ws://:3001/ndt_protocol",
wantStatus: http.StatusOK,
},
{
name: "success-nearest-server-using-region",
path: "ndt/ndt5",
signer: &fakeSigner{},
locator: &fakeLocator{
targets: []v2.Target{{Machine: "mlab1-lga0t.measurement-lab.org"}},
},
header: http.Header{
"X-AppEngine-Country": []string{"US"},
"X-AppEngine-Region": []string{"ny"},
},
wantLatLon: "43.19880000,-75.3242000", // Region center.
wantKey: "ws://:3001/ndt_protocol",
wantStatus: http.StatusOK,
},
{
name: "success-nearest-server-using-country",
path: "ndt/ndt5",
signer: &fakeSigner{},
locator: &fakeLocator{
targets: []v2.Target{{Machine: "mlab1-lga0t.measurement-lab.org"}},
},
header: http.Header{
"X-AppEngine-Region": []string{"fake-region"},
"X-AppEngine-Country": []string{"US"},
"X-AppEngine-CityLatLong": []string{"0.000000,0.000000"},
},
wantLatLon: "37.09024,-95.712891", // Country center.
wantKey: "ws://:3001/ndt_protocol",
wantStatus: http.StatusOK,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cl := clientgeo.NewAppEngineLocator()
c := NewClient(tt.project, tt.signer, tt.locator, &fakeLocatorV2{}, cl, prom.NewAPI(nil))

mux := http.NewServeMux()
mux.HandleFunc("/v2/nearest/", c.TranslatedQuery)
srv := httptest.NewServer(mux)
defer srv.Close()

req, err := http.NewRequest(http.MethodGet, srv.URL+"/v2/nearest/"+tt.path+"?client_name=foo", nil)
rtx.Must(err, "Failed to create request")
req.Header = tt.header

result := &v2.NearestResult{}
resp, err := proxy.UnmarshalResponse(req, result)
if err != nil {
t.Fatalf("Failed to get response from: %s %s", srv.URL, tt.path)
}
if resp.Header.Get("Access-Control-Allow-Origin") != "*" {
t.Errorf("TranslatedQuery() wrong Access-Control-Allow-Origin header; got %s, want '*'",
resp.Header.Get("Access-Control-Allow-Origin"))
}
if resp.Header.Get("Content-Type") != "application/json" {
t.Errorf("TranslatedQuery() wrong Content-Type header; got %s, want 'application/json'",
resp.Header.Get("Content-Type"))
}
if resp.Header.Get("X-Locate-ClientLatLon") != tt.wantLatLon {
t.Errorf("TranslatedQuery() wrong X-Locate-ClientLatLon header; got %s, want '%s'",
resp.Header.Get("X-Locate-ClientLatLon"), tt.wantLatLon)
}
if result.Error != nil && result.Error.Status != tt.wantStatus {
t.Errorf("TranslatedQuery() wrong status; got %d, want %d", result.Error.Status, tt.wantStatus)
}
if result.Error != nil {
return
}
if result.Results == nil && tt.wantStatus == http.StatusOK {
t.Errorf("TranslatedQuery() wrong status; got %d, want %d", result.Error.Status, tt.wantStatus)
}
if len(tt.locator.targets) != len(result.Results) {
t.Errorf("TranslateQuery() wrong result count; got %d, want %d",
len(result.Results), len(tt.locator.targets))
}
if len(result.Results[0].URLs) != len(static.Configs[tt.path]) {
t.Errorf("TranslateQuery() result wrong URL count; got %d, want %d",
len(result.Results[0].URLs), len(static.Configs[tt.path]))
}
if _, ok := result.Results[0].URLs[tt.wantKey]; !ok {
t.Errorf("TranslateQuery() result missing URLs key; want %q", tt.wantKey)
}
})
}
}

func TestClient_Nearest(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -365,14 +212,14 @@ func TestClient_Nearest(t *testing.T) {
if tt.cl == nil {
tt.cl = clientgeo.NewAppEngineLocator()
}
c := NewClient(tt.project, tt.signer, &fakeLocator{}, tt.locator, tt.cl, prom.NewAPI(nil))
c := NewClient(tt.project, tt.signer, tt.locator, tt.cl, prom.NewAPI(nil))

mux := http.NewServeMux()
mux.HandleFunc("/v2beta2/nearest/", c.Nearest)
mux.HandleFunc("/v2/nearest/", c.Nearest)
srv := httptest.NewServer(mux)
defer srv.Close()

req, err := http.NewRequest(http.MethodGet, srv.URL+"/v2beta2/nearest/"+tt.path+"?client_name=foo", nil)
req, err := http.NewRequest(http.MethodGet, srv.URL+"/v2/nearest/"+tt.path+"?client_name=foo", nil)
rtx.Must(err, "Failed to create request")
req.Header = tt.header

Expand Down Expand Up @@ -419,7 +266,7 @@ func TestClient_Nearest(t *testing.T) {

func TestNewClientDirect(t *testing.T) {
t.Run("success", func(t *testing.T) {
c := NewClientDirect("fake-project", nil, nil, nil, nil, nil)
c := NewClientDirect("fake-project", nil, nil, nil, nil)
if c == nil {
t.Error("got nil client!")
}
Expand All @@ -444,8 +291,7 @@ func TestClient_Ready(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := NewClient("foo", &fakeSigner{}, &fakeLocator{},
&fakeLocatorV2{StatusTracker: &heartbeattest.FakeStatusTracker{Err: tt.fakeErr}}, nil, nil)
c := NewClient("foo", &fakeSigner{}, &fakeLocatorV2{StatusTracker: &heartbeattest.FakeStatusTracker{Err: tt.fakeErr}}, nil, nil)

mux := http.NewServeMux()
mux.HandleFunc("/ready/", c.Ready)
Expand Down
2 changes: 1 addition & 1 deletion handler/heartbeat_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestClient_handleHeartbeats(t *testing.T) {

func fakeClient(t heartbeat.StatusTracker) *Client {
locatorv2 := fakeLocatorV2{StatusTracker: t}
return NewClient("mlab-sandbox", &fakeSigner{}, &fakeLocator{}, &locatorv2,
return NewClient("mlab-sandbox", &fakeSigner{}, &locatorv2,
clientgeo.NewAppEngineLocator(), prom.NewAPI(nil))
}

Expand Down
2 changes: 1 addition & 1 deletion handler/monitoring.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/m-lab/locate/static"
)

// Monitoring implements /v2/monitoring requests.
// Monitoring issues access tokens for end to end monitoring requests.
func (c *Client) Monitoring(rw http.ResponseWriter, req *http.Request) {
result := v2.MonitoringResult{}

Expand Down
Loading

0 comments on commit 80f3d8b

Please sign in to comment.