Skip to content

Commit c7a5639

Browse files
mwnicksmwnicks
committed
Merge branch 'release/1.17.0'
2 parents 9bde42b + 3bcfec4 commit c7a5639

File tree

8 files changed

+111
-109
lines changed

8 files changed

+111
-109
lines changed

‎dp-api-router.nomad

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ job "dp-api-router" {
5151
service {
5252
name = "dp-api-router"
5353
port = "http"
54-
tags = ["web"]
54+
tags = ["web", "system_job"]
5555
check {
5656
type = "http"
5757
path = "/health"
@@ -115,7 +115,7 @@ job "dp-api-router" {
115115
service {
116116
name = "dp-api-router"
117117
port = "http"
118-
tags = ["publishing"]
118+
tags = ["publishing", "system_job"]
119119
check {
120120
type = "http"
121121
path = "/health"

‎interceptor/interceptor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ const (
4141
)
4242

4343
var (
44-
re = regexp.MustCompile(`^(.+:\/\/)(.+)(\/v\d)$`)
44+
re = regexp.MustCompile(`^(.+://)(.+)(/v\d)$`)
4545
)
4646

4747
// RoundTrip intercepts the response body and post processes to add the correct enviornment

‎interceptor/interceptor_test.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ func TestUnitInterceptor(t *testing.T) {
3838

3939
b, _ := ioutil.ReadAll(resp.Body)
4040

41+
err = resp.Body.Close()
42+
So(err, ShouldBeNil)
4143
So(len(b), ShouldEqual, 0)
4244
})
4345

@@ -52,6 +54,8 @@ func TestUnitInterceptor(t *testing.T) {
5254

5355
b, _ := ioutil.ReadAll(resp.Body)
5456

57+
err = resp.Body.Close()
58+
So(err, ShouldBeNil)
5559
So(string(b), ShouldEqual, `{"links":{"self":{"href":"https://api.beta.ons.gov.uk/v1/datasets/12345"}}}`+"\n")
5660
})
5761

@@ -66,8 +70,9 @@ func TestUnitInterceptor(t *testing.T) {
6670

6771
b, _ := ioutil.ReadAll(resp.Body)
6872

73+
err = resp.Body.Close()
74+
So(err, ShouldBeNil)
6975
So(len(b), ShouldEqual, 76)
70-
7176
So(string(b), ShouldEqual, `{"links":{"self":{"href":"https://api.beta.ons.gov.uk/v1/datasets/12345"}}}`+"\n")
7277
})
7378

@@ -82,8 +87,9 @@ func TestUnitInterceptor(t *testing.T) {
8287

8388
b, _ := ioutil.ReadAll(resp.Body)
8489

90+
err = resp.Body.Close()
91+
So(err, ShouldBeNil)
8592
So(len(b), ShouldEqual, 102)
86-
8793
So(string(b), ShouldEqual, `{"@context":"context.json","links":{"self":{"href":"https://api.beta.ons.gov.uk/v1/datasets/12345"}}}`+"\n")
8894
})
8995

@@ -98,8 +104,9 @@ func TestUnitInterceptor(t *testing.T) {
98104

99105
b, _ := ioutil.ReadAll(resp.Body)
100106

107+
err = resp.Body.Close()
108+
So(err, ShouldBeNil)
101109
So(len(b), ShouldEqual, 77)
102-
103110
So(string(b), ShouldEqual, `{"downloads":{"csv":{"href":"https://download.beta.ons.gov.uk/myfile.csv"}}}`+"\n")
104111
})
105112

@@ -113,8 +120,9 @@ func TestUnitInterceptor(t *testing.T) {
113120
So(err, ShouldBeNil)
114121

115122
b, _ := ioutil.ReadAll(resp.Body)
123+
err = resp.Body.Close()
124+
So(err, ShouldBeNil)
116125
So(len(b), ShouldEqual, 77)
117-
118126
So(string(b), ShouldEqual, `{"dimensions":[{"href":"https://api.beta.ons.gov.uk/v1/codelists/1234567"}]}`+"\n")
119127
})
120128

@@ -128,8 +136,9 @@ func TestUnitInterceptor(t *testing.T) {
128136
So(err, ShouldBeNil)
129137

130138
b, _ := ioutil.ReadAll(resp.Body)
139+
err = resp.Body.Close()
140+
So(err, ShouldBeNil)
131141
So(len(b), ShouldEqual, 88)
132-
133142
So(string(b), ShouldEqual, `{"items":[{"links":{"self":{"href":"https://api.beta.ons.gov.uk/v1/datasets/12345"}}}]}`+"\n")
134143
})
135144

@@ -143,8 +152,9 @@ func TestUnitInterceptor(t *testing.T) {
143152
So(err, ShouldBeNil)
144153

145154
b, _ := ioutil.ReadAll(resp.Body)
155+
err = resp.Body.Close()
156+
So(err, ShouldBeNil)
146157
So(len(b), ShouldEqual, 83)
147-
148158
So(string(b), ShouldEqual, `{"links":{"instances":[{"href":"https://api.beta.ons.gov.uk/v1/datasets/12345"}]}}`+"\n")
149159
})
150160

@@ -158,8 +168,9 @@ func TestUnitInterceptor(t *testing.T) {
158168
So(err, ShouldBeNil)
159169

160170
b, _ := ioutil.ReadAll(resp.Body)
171+
err = resp.Body.Close()
172+
So(err, ShouldBeNil)
161173
So(len(b), ShouldEqual, 91)
162-
163174
So(string(b), ShouldEqual, `{"dimensions":{"time":{"option":{"href":"https://api.beta.ons.gov.uk/v1/datasets/time"}}}}`+"\n")
164175
})
165176

@@ -173,8 +184,9 @@ func TestUnitInterceptor(t *testing.T) {
173184
So(err, ShouldBeNil)
174185

175186
b, _ := ioutil.ReadAll(resp.Body)
187+
err = resp.Body.Close()
188+
So(err, ShouldBeNil)
176189
So(len(b), ShouldEqual, 116)
177-
178190
So(string(b), ShouldEqual, `{"dimensions":{"time":{"option":{"href":"https://api.beta.ons.gov.uk/v1/datasets/time?hello=world&mobile=phone"}}}}`+"\n")
179191
})
180192

@@ -189,8 +201,9 @@ func TestUnitInterceptor(t *testing.T) {
189201

190202
b, _ := ioutil.ReadAll(resp.Body)
191203

204+
err = resp.Body.Close()
205+
So(err, ShouldBeNil)
192206
So(len(b), ShouldEqual, 154)
193-
194207
So(string(b), ShouldEqual, `[{"links":{"self":{"href":"https://api.beta.ons.gov.uk/v1/datasets/12345"}}},{"links":{"self":{"href":"https://api.beta.ons.gov.uk/v1/datasets/12345"}}}]`+"\n")
195208
})
196209
}

‎middleware/audit.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func AuditHandler(auditProducer *event.AvroProducer,
106106

107107
// Retrieve Identity from Zebedee, which is stored in context.
108108
// if it fails, try to audit with the statusCode before returning
109-
ctx, statusCode, err := retrieveIdentity(w, r, idClient, zebedeeURL)
109+
ctx, statusCode, err := retrieveIdentity(w, r, idClient)
110110
if err != nil {
111111
// error already handled in retrieveIdentity. Try to audit it.
112112
auditEvent.StatusCode = int32(statusCode)
@@ -156,7 +156,11 @@ func AuditHandler(auditProducer *event.AvroProducer,
156156

157157
// Copy the intercepted body and header to the original response writer
158158
w.WriteHeader(rec.statusCode)
159-
io.Copy(w, rec.body)
159+
_, err = io.Copy(w, rec.body)
160+
if err != nil {
161+
handleError(r.Context(), w, r, http.StatusInternalServerError, "failed to copy intercepted response to original response writer", err, log.Data{"event": auditEvent})
162+
return
163+
}
160164

161165
// Finally send the outbound audit message
162166
auditProducer.Send(eventBytes)
@@ -207,10 +211,9 @@ func GenerateAuditEvent(req *http.Request) *event.Audit {
207211
return auditEvent
208212
}
209213

210-
// retrieveIdentity requests the user and caller identity from Zebedee, using the provided client and url.
211-
func retrieveIdentity(w http.ResponseWriter, req *http.Request, idClient *clientsidentity.Client, zebedeeURL string) (ctx context.Context, status int, err error) {
214+
// retrieveIdentity requests the user and caller identity from Zebedee, using the provided client.
215+
func retrieveIdentity(w http.ResponseWriter, req *http.Request, idClient *clientsidentity.Client) (ctx context.Context, status int, err error) {
212216
ctx = req.Context()
213-
log.Event(ctx, "executing identity check for auditing purposes", log.INFO)
214217

215218
florenceToken, err := getFlorenceToken(ctx, req)
216219
if err != nil {
@@ -224,7 +227,7 @@ func retrieveIdentity(w http.ResponseWriter, req *http.Request, idClient *client
224227
return ctx, http.StatusInternalServerError, err
225228
}
226229

227-
// CheckRequest performs the call to Zebedee GET /identity and stores the values in context
230+
// CheckRequest performs the call to Zebedee GET /identity and stores the values in context
228231
ctx, statusCode, authFailure, err := idClient.CheckRequest(req, florenceToken, serviceAuthToken)
229232
logData := log.Data{"auth_status_code": statusCode}
230233
if err != nil {
@@ -291,7 +294,7 @@ func getServiceAuthToken(ctx context.Context, req *http.Request) (string, error)
291294
return authToken, err
292295
}
293296

294-
// Now is a time.Now wrapper
297+
// Now is a time.Now wrapper specifically for testing purposes, and should not me unlambda'd - despite what golangci-lint says
295298
var Now = func() time.Time {
296299
return time.Now()
297300
}

‎middleware/audit_test.go

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121
"github.com/ONSdigital/dp-api-router/middleware"
2222
"github.com/ONSdigital/dp-api-router/schema"
2323

24-
"github.com/ONSdigital/dp-kafka/v2"
24+
kafka "github.com/ONSdigital/dp-kafka/v2"
2525
"github.com/ONSdigital/dp-kafka/v2/kafkatest"
2626
dphttp "github.com/ONSdigital/dp-net/http"
2727
dprequest "github.com/ONSdigital/dp-net/request"
@@ -43,7 +43,6 @@ var (
4343
testTimeMillisOutbound int64 = 1587884752456
4444
testBody = []byte{1, 2, 3, 4}
4545
errMarshal = errors.New("avro marshal error")
46-
errDo = errors.New("identity client Do failed")
4746
)
4847

4948
// valid identity response for testing
@@ -52,10 +51,11 @@ var testIdentityResponse = &dprequest.IdentityResponse{
5251
}
5352

5453
// utility function to generate handlers for testing, which return the provided status code and body
55-
func testHandler(statusCode int, body []byte) http.Handler {
54+
func testHandler(statusCode int, body []byte, c C) http.Handler {
5655
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
5756
w.WriteHeader(statusCode)
58-
w.Write(body)
57+
_, err := w.Write(body)
58+
c.So(err, ShouldBeNil)
5959
})
6060
}
6161

@@ -89,7 +89,6 @@ func createHTTPClientMock(retCode int, retBody interface{}) *dphttp.ClienterMock
8989
return []string{}
9090
},
9191
SetPathsWithNoRetriesFunc: func([]string) {
92-
return
9392
},
9493
DoFunc: func(ctx context.Context, req *http.Request) (*http.Response, error) {
9594
body, _ := json.Marshal(retBody)
@@ -215,7 +214,7 @@ func TestAuditHandlerHeaders(t *testing.T) {
215214

216215
Convey("And a valid audit handler with successful downstream", func(c C) {
217216
p, a := createValidAuditHandler()
218-
auditHandler := a(testHandler(http.StatusOK, testBody))
217+
auditHandler := a(testHandler(http.StatusOK, testBody, c))
219218

220219
// execute request and wait for 2 audit events
221220
auditEvents := serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 2)
@@ -248,7 +247,7 @@ func TestAuditHandlerHeaders(t *testing.T) {
248247

249248
Convey("And a failing audit handler with successful downstream", func(c C) {
250249
p, a := createFailingAuditHandler()
251-
auditHandler := a(testHandler(http.StatusOK, testBody))
250+
auditHandler := a(testHandler(http.StatusOK, testBody, c))
252251

253252
// execute request and don't expect audit events
254253
serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 0)
@@ -270,7 +269,7 @@ func TestAuditHandlerHeaders(t *testing.T) {
270269

271270
Convey("And a valid audit handler with successful downstream", func(c C) {
272271
p, a := createValidAuditHandler()
273-
auditHandler := a(testHandler(http.StatusOK, testBody))
272+
auditHandler := a(testHandler(http.StatusOK, testBody, c))
274273

275274
// execute request and wait for 2 audit events
276275
auditEvents := serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 2)
@@ -303,7 +302,7 @@ func TestAuditHandlerHeaders(t *testing.T) {
303302

304303
Convey("And a failing audit handler with successful downstream", func(c C) {
305304
p, a := createFailingAuditHandler()
306-
auditHandler := a(testHandler(http.StatusOK, testBody))
305+
auditHandler := a(testHandler(http.StatusOK, testBody, c))
307306

308307
// execute request and don't expect audit events
309308
serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 0)
@@ -328,7 +327,7 @@ func TestAuditHandlerHeaders(t *testing.T) {
328327

329328
Convey("And a valid audit handler with successful downstream", func(c C) {
330329
p, a := createValidAuditHandler()
331-
auditHandler := a(testHandler(http.StatusOK, testBody))
330+
auditHandler := a(testHandler(http.StatusOK, testBody, c))
332331

333332
// execute request and wait for 2 audit events
334333
auditEvents := serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 2)
@@ -387,7 +386,7 @@ func TestAuditHandler(t *testing.T) {
387386

388387
Convey("And a valid audit handler with unsuccessful (Forbidden) downstream", func(c C) {
389388
p, a := createValidAuditHandler()
390-
auditHandler := a(testHandler(http.StatusForbidden, testBody))
389+
auditHandler := a(testHandler(http.StatusForbidden, testBody, c))
391390

392391
// execute request and wait for 2 audit events
393392
auditEvents := serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 2)
@@ -420,7 +419,7 @@ func TestAuditHandler(t *testing.T) {
420419

421420
Convey("And a failing audit handler with unsuccessful (Forbidden) downstream", func(c C) {
422421
p, a := createFailingAuditHandler()
423-
auditHandler := a(testHandler(http.StatusForbidden, testBody))
422+
auditHandler := a(testHandler(http.StatusForbidden, testBody, c))
424423

425424
// execute request and don't expect audit events
426425
serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 0)
@@ -455,7 +454,7 @@ func TestAuditHandler(t *testing.T) {
455454
p := kafkatest.NewMessageProducer(true)
456455
a := event.NewAvroProducer(p.Channels().Output, failingMarshaller)
457456
enableZebedeeAudit := true
458-
auditHandler := middleware.AuditHandler(a, cliMock, testZebedeeURL, testVersionPrefix, enableZebedeeAudit, nil)(testHandler(http.StatusForbidden, testBody))
457+
auditHandler := middleware.AuditHandler(a, cliMock, testZebedeeURL, testVersionPrefix, enableZebedeeAudit, nil)(testHandler(http.StatusForbidden, testBody, c))
459458

460459
// execute request and expect only 1 audit event
461460
auditEvents := serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 1)
@@ -484,7 +483,7 @@ func TestAuditIgnoreSkip(t *testing.T) {
484483
Convey("And a valid audit handler without downstream", func(c C) {
485484

486485
p, a := createValidAuditHandler()
487-
auditHandler := a(testHandler(http.StatusForbidden, testBody))
486+
auditHandler := a(testHandler(http.StatusForbidden, testBody, c))
488487

489488
// execute request and don't wait for audit events
490489
serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 0)
@@ -506,7 +505,7 @@ func TestAuditIgnoreSkip(t *testing.T) {
506505
Convey("And a valid audit handler without downstream", func(c C) {
507506

508507
p, a := createValidAuditHandler()
509-
auditHandler := a(testHandler(http.StatusForbidden, testBody))
508+
auditHandler := a(testHandler(http.StatusForbidden, testBody, c))
510509

511510
// execute request and wait for 2 audit events
512511
auditEvents := serveAndCaptureAudit(c, w, req, auditHandler, p.Channels().Output, 2)
@@ -542,7 +541,7 @@ func TestSkipZebedeeAudit(t *testing.T) {
542541
},
543542
}
544543
auditMiddleware := middleware.AuditHandler(auditProducer, cliMock, testZebedeeURL, testVersionPrefix, enableZebedeeAudit, routerMock)
545-
auditHandler := auditMiddleware(testHandler(http.StatusOK, testBody))
544+
auditHandler := auditMiddleware(testHandler(http.StatusOK, testBody, c))
546545

547546
Convey("When the handler receives a Zebedee request", func(c C) {
548547
req, err := http.NewRequest(http.MethodGet, "/data", nil)
@@ -581,7 +580,7 @@ func TestSkipZebedeeAudit(t *testing.T) {
581580
},
582581
}
583582
auditMiddleware := middleware.AuditHandler(auditProducer, cliMock, testZebedeeURL, testVersionPrefix, enableZebedeeAudit, routerMock)
584-
auditHandler := auditMiddleware(testHandler(http.StatusOK, testBody))
583+
auditHandler := auditMiddleware(testHandler(http.StatusOK, testBody, c))
585584

586585
Convey("When the handler receives a request for a known route (not zebedee)", func(c C) {
587586
req, err := http.NewRequest(http.MethodGet, "/v1/datasets?q1=v1&q2=v2", nil)

0 commit comments

Comments
 (0)