From 2679db37937a7a16be631ee8cff8cea8fc3863bd Mon Sep 17 00:00:00 2001 From: Bianca Danforth Date: Sat, 20 Jul 2024 13:54:37 -0400 Subject: [PATCH] bug-1906139: systemtests download API improvements Because: * We will rely on systemtests as a primary way to validate the Tecken GCP migration. * We were not testing HEAD requests or verifying response headers in the systemtests for the download API. This commit: * Tests the HTTP response for HEAD requests * Checks response headers for both the Tecken redirect response and the storage backend response. --- systemtests/bin/download_sym_files.py | 148 ++++++++++++++++++++++---- 1 file changed, 127 insertions(+), 21 deletions(-) diff --git a/systemtests/bin/download_sym_files.py b/systemtests/bin/download_sym_files.py index bafe3768d..cecf42bdf 100755 --- a/systemtests/bin/download_sym_files.py +++ b/systemtests/bin/download_sym_files.py @@ -14,6 +14,20 @@ import markus from markus.backends import BackendBase import requests +from unittest.mock import ANY + +# These are exclusively security headers added by nginx +TECKEN_RESPONSE_HEADERS = { + "X-Content-Type-Options": "nosniff", + "X-Frame-Options": "DENY", + "Content-Security-Policy": "font-src 'self'; object-src 'none'; script-src 'self'; img-src 'self'; default-src 'self'; style-src 'self'; connect-src 'self'; frame-ancestors 'none'", + "Strict-Transport-Security": "max-age=31536000", +} + +STORAGE_BACKEND_RESPONSE_HEADERS = { + "Content-Encoding": "gzip", + "Content-Length": ANY, +} class StdoutMetrics(BackendBase): @@ -25,18 +39,92 @@ def emit(self, record): METRICS = markus.get_metrics() +# Check the response headers for both the Tecken redirect response and the storage +# backend response. +def check_headers(response): + for item in response.history: + if item.status_code in (301, 302): + click.echo(f">>> GET redirect: {item.url}") + for expected_key, expected_value in TECKEN_RESPONSE_HEADERS.items(): + if expected_key not in item.headers: + click.echo( + click.style( + f">>> FAIL: GET expected redirect response header {expected_key} is missing.", + fg="red", + ) + ) + continue + + if item.headers[expected_key] == expected_value: + click.echo( + click.style( + f">>> SUCCESS: GET expected redirect response header {expected_key}: {expected_value}.", + fg="green", + ) + ) + else: + click.echo( + click.style( + f""" +>>> FAIL: GET expected response header {expected_key} does not have expected value: +actual: {item.headers[expected_key]} +expected: {expected_value} +""", + fg="red", + ) + ) + + for ( + expected_key, + expected_value, + ) in STORAGE_BACKEND_RESPONSE_HEADERS.items(): + if expected_key not in response.headers: + click.echo( + click.style( + f">>> FAIL: GET expected response header {expected_key} is missing.", + fg="red", + ) + ) + continue + + if response.headers[expected_key] == expected_value: + click.echo( + click.style( + f">>> SUCCESS: GET expected response header {expected_key}: {expected_value}.", + fg="green", + ) + ) + else: + click.echo( + click.style( + f""" +>>> FAIL: GET expected response header {expected_key} does not have expected value: +actual: {response.headers[expected_key]} +expected: {expected_value} +""", + fg="red", + ) + ) + + @click.command() @click.option( "--base-url", default="https://symbols.mozilla.org/", help="Base url to use for downloading SYM files.", ) +@click.option( + "--test-headers", + type=bool, + default=True, + help="Whether to check response headers from the Tecken redirect response and the storage backend response. Should be False for local and True otherwise.", +) @click.argument( "csv_file", nargs=1, type=click.Path(), ) -def download_sym_files(base_url, csv_file): +def download_sym_files(base_url, test_headers, csv_file): """Tests downloading SYM files. Takes a CSV file and a base url, composes urls for SYM files to download, @@ -68,28 +156,46 @@ def download_sym_files(base_url, csv_file): url = url + "?try" click.echo(click.style(f"Working on {url} ...", fg="yellow")) - # Download the file - headers = {"User-Agent": "tecken-systemtests"} - resp = requests.get(url, headers=headers, timeout=60) + headers = { + "User-Agent": "tecken-systemtests", + # We know storage backends will honor "Accept-Encoding": "gzip", + # so we test the unusual case only to ensure the response is + # still gzipped. + "Accept-Encoding": "identity", + } + + for method in ["HEAD", "GET"]: + resp = requests.request( + method, + url, + headers=headers, + timeout=60, + ) - for item in resp.history: - if item.status_code in (301, 302): - click.echo(f">>> redirect: {item.url}") - click.echo(f">>> final: {resp.url}") - click.echo(f">>> status code: {resp.status_code}") - if resp.status_code == 200: - click.echo(f">>> file size {len(resp.content):,} bytes") - - # Compare status code with expected status code - if resp.status_code != int(expected_status_code): - click.echo( - click.style( - f"FAIL: Status code: {resp.status_code} != {expected_status_code}", - fg="red", + if resp.status_code != int(expected_status_code): + click.echo( + click.style( + f"FAIL: {method} status code: {resp.status_code} != {expected_status_code}", + fg="red", + ) ) - ) - else: - click.echo(click.style("Success!", fg="green")) + else: + click.echo(click.style(f"SUCCESS: {method} request!", fg="green")) + + # If we're testing a 404, we should not expect the same response + # headers from the storage backend as for a 200 (e.g. Content-Encoding + # isn't included in a 404 response). + if method == "GET" and expected_status_code != "404" and test_headers: + check_headers(resp) + + click.echo(f">>> {method} final: {resp.url}") + + # Tecken's download API currently returns a 200 for HEAD requests + # when a file exists. + click.echo(f">>> {method} status code: {resp.status_code}") + + if resp.status_code == 200: + click.echo(f">>> {method} file size {len(resp.content):,} bytes") if __name__ == "__main__":