Skip to content

Commit e1ca868

Browse files
committed
apm-2006: log response headers properly and supress the x-powered-by header
1 parent fa48380 commit e1ca868

7 files changed

Lines changed: 38 additions & 17 deletions

File tree

azure/azure-build-pipeline.yml

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,24 @@ extends:
3131
cache_steps:
3232
- task: s3-cache-action@1
3333
inputs:
34-
key: 'node modules | ${{ variables.service_name }}/docker/async-slowapp/package-lock.json'
35-
location: '${{ variables.service_name }}/docker/async-slowapp/node_modules'
34+
key: 'node modules | ./docker/async-slowapp/package-lock.json'
35+
location: './docker/async-slowapp/node_modules'
3636
debug: true
37+
workingDirectory: '${{ variables.service_name }}'
3738
displayName: cache async-slowapp node modules
3839
- task: s3-cache-action@1
3940
inputs:
40-
key: 'node modules | ${{ variables.service_name }}/docker/sync-wrap/package-lock.json'
41-
location: '${{ variables.service_name }}/docker/sync-wrap/node_modules'
41+
key: 'node modules | ./docker/sync-wrap/package-lock.json'
42+
location: './docker/sync-wrap/node_modules'
4243
debug: true
44+
workingDirectory: '${{ variables.service_name }}'
4345
displayName: cache sync-wrap node modules
4446
- task: s3-cache-action@1
4547
inputs:
46-
key: 'poetry | ${{ variables.service_name }}/poetry.lock'
47-
location: '${{ variables.service_name }}/.venv'
48+
key: 'poetry | ./poetry.lock'
49+
location: './.venv'
4850
debug: true
51+
workingDirectory: '${{ variables.service_name }}'
4952
displayName: cache python dependencies
5053
test_steps:
5154
- bash: "make test-report"
@@ -54,6 +57,6 @@ extends:
5457
- task: PublishTestResults@2
5558
inputs:
5659
testResultsFormat: "JUnit"
57-
testResultsFiles: "${{ variables.service_name }}/reports/tests/*.xml"
60+
testResultsFiles: "./reports/tests/*.xml"
5861
failTaskOnFailedTests: true
5962
displayName: publish test report

azure/templates/e2e.yml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,18 @@ steps:
55

66
- task: s3-cache-action@1
77
inputs:
8-
key: 'poetry | $(SERVICE_DIR)/poetry.lock'
9-
location: '$(SERVICE_DIR)/.venv'
8+
key: 'poetry | ./poetry.lock'
9+
location: '.venv'
1010
debug: true
11+
workingDirectory: $(SERVICE_DIR)
12+
name: poetryCache
1113
displayName: cache python dependencies
1214

1315
- bash: |
1416
make install-python
1517
workingDirectory: $(SERVICE_DIR)
16-
displayName: Setup e2e tests
18+
condition: ne(variables['poetryCache.cacheRestored'], 'true')
19+
displayName: poetry install
1720
1821
- bash: |
1922
export RELEASE_RELEASEID=$(Build.BuildId)
@@ -24,10 +27,10 @@ steps:
2427
make -C e2e ${{ parameters.test_type }}
2528
2629
workingDirectory: $(SERVICE_DIR)
27-
displayName: 'Run ${{ parameters.test_type }} Tests'
30+
displayName: run ${{ parameters.test_type }} tests
2831
2932
- task: PublishTestResults@2
30-
displayName: 'Publish ${{ parameters.test_type }} Test Results'
33+
displayName: publish ${{ parameters.test_type }} test results
3134
inputs:
3235
testResultsFiles: |
3336
$(SERVICE_DIR)/reports/${{ parameters.test_type }}.xml

docker/sync-wrap/src/app.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ function setup(options) {
3636
let https = app.locals.upstream.protocol === "https:";
3737

3838
app.locals.conn = https ? require("https") : require("http");
39-
39+
app.disable('x-powered-by');
4040
let default_options = {
4141
host: app.locals.upstream.hostname,
4242
port: app.locals.upstream.port
@@ -117,7 +117,7 @@ function after_request(req, res, next) {
117117
if (log.getLevel()<2) {
118118
// debug
119119
log_entry.req.headers = (req.rawHeaders || []).asMultiValue();
120-
log_entry.res.headers = (res.rawHeaders || []).asMultiValue();
120+
log_entry.res.headers = res.getHeaders();
121121
}
122122
log.info(JSON.stringify(log_entry));
123123

docker/sync-wrap/src/app.slowapp.spec.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ describe("express with slowap", function () {
8181
it("responds to /_status", (done) => {
8282
request(server)
8383
.get("/_status")
84+
.expect(res => {
85+
assert.isUndefined(res.header['x-powered-by']);
86+
})
8487
.expect(200, {
8588
status: "pass",
8689
ping: "pong",
@@ -127,6 +130,19 @@ describe("express with slowap", function () {
127130
.expect(504, done);
128131
}).timeout(10000);
129132

133+
it("when patching it times out if x-sync-wait shorter than initial response", (done) => {
134+
request(server)
135+
.patch("/slow?delay=5")
136+
.send({test: 'data'})
137+
.set("x-sync-wait", "0.25")
138+
.set("Accept", "application/json")
139+
.expect(res => {
140+
assert.isUndefined(res.header['x-powered-by']);
141+
assert.equal(res.header['content-length'], "0");
142+
})
143+
.expect(504, done);
144+
}).timeout(10000);
145+
130146
it("slowapp default x-sync-wait final status 500", (done) => {
131147
request(server)
132148
.get("/slow?final_status=500&complete_in=0.5")

docker/sync-wrap/src/handlers.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ async function proxy(req, res, next) {
519519
await send_response(response.statusCode, {headers: headers, response: response});
520520
})
521521
.catch(async (fin) => {
522-
await send_response(fin.error === "timeout" ? 504 : 502, {headers: undefined, error: fin.error || fin});
522+
await send_response(fin.error === "timeout" ? 504 : 502, { error: fin.error || fin });
523523
});
524524

525525
}

proxies/live/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ build:
1010
cp -r ./apiproxy ../../build/proxies/$(dirname)
1111

1212
deploy: build
13-
cd ../../build/$(dirname) && apigeetool deployproxy -V -o nhsd-nonprod -e internal-dev -n sync-wrap-local -t $$(get_token)
13+
cd ../../build/proxies/$(dirname) && apigeetool deployproxy -V -o nhsd-nonprod -e internal-dev -n sync-wrap-local -t $$(get_token)
1414

1515
undeploy:
1616
apigeetool undeploy -V -o nhsd-nonprod -e internal-dev -n sync-wrap-local -t $$(get_token)

pyproject.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ flake8 = "^3.7.9"
3939
black = "^19.10b0"
4040
pip-licenses = "^2.0.1"
4141
jinja2 = "^2.11.1"
42-
4342
api-test-utils = {url = "https://github.com/NHSDigital/apim-test-utils/releases/download/v1.1.2-alpha/api_test_utils-1.1.2a0-py3-none-any.whl"}
4443

4544
[tool.poetry.scripts]

0 commit comments

Comments
 (0)