From c9774c2085da7446f4f867093ff15651ad3342ce Mon Sep 17 00:00:00 2001 From: Sarah Wang Date: Wed, 9 Aug 2023 15:49:23 -0400 Subject: [PATCH] migrate `ddev validate http` to ddev --- ddev/src/ddev/cli/validate/__init__.py | 2 +- ddev/src/ddev/cli/validate/http.py | 163 ++++++++++++++++++ ddev/tests/cli/validate/test_http.py | 106 ++++++++++++ .../kube_apiserver_metrics.py | 2 +- marklogic/datadog_checks/marklogic/api.py | 2 +- marklogic/datadog_checks/marklogic/check.py | 2 +- 6 files changed, 273 insertions(+), 4 deletions(-) create mode 100644 ddev/src/ddev/cli/validate/http.py create mode 100644 ddev/tests/cli/validate/test_http.py diff --git a/ddev/src/ddev/cli/validate/__init__.py b/ddev/src/ddev/cli/validate/__init__.py index c968920c9a8a2..f904bdd3ea07a 100644 --- a/ddev/src/ddev/cli/validate/__init__.py +++ b/ddev/src/ddev/cli/validate/__init__.py @@ -10,7 +10,6 @@ from datadog_checks.dev.tooling.commands.validate.dashboards import dashboards from datadog_checks.dev.tooling.commands.validate.dep import dep from datadog_checks.dev.tooling.commands.validate.eula import eula -from datadog_checks.dev.tooling.commands.validate.http import http from datadog_checks.dev.tooling.commands.validate.imports import imports from datadog_checks.dev.tooling.commands.validate.integration_style import integration_style from datadog_checks.dev.tooling.commands.validate.jmx_metrics import jmx_metrics @@ -24,6 +23,7 @@ from datadog_checks.dev.tooling.commands.validate.typos import typos from ddev.cli.validate.ci import ci +from ddev.cli.validate.http import http from ddev.cli.validate.licenses import licenses from ddev.cli.validate.manifest import manifest from ddev.cli.validate.metadata import metadata diff --git a/ddev/src/ddev/cli/validate/http.py b/ddev/src/ddev/cli/validate/http.py new file mode 100644 index 0000000000000..a131b505d0ef9 --- /dev/null +++ b/ddev/src/ddev/cli/validate/http.py @@ -0,0 +1,163 @@ +# (C) Datadog, Inc. 2023-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +from __future__ import annotations +import os +import re + +import click +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from ddev.cli.application import Application +EXCLUDED_INTEGRATIONS = {'kubelet', 'openstack'} + +REQUEST_LIBRARY_FUNC_RE = r"requests.[get|post|head|put|patch|delete]*\(" +HTTP_WRAPPER_INIT_CONFIG_RE = r"init_config\/[http|openmetrics_legacy|openmetrics]*" +HTTP_WRAPPER_INSTANCE_RE = r"instances\/[http|openmetrics_legacy|openmetrics]*" + +def get_default_config_spec(check_name, app): + return os.path.join(app.repo.path, check_name, 'assets', 'configuration', 'spec.yaml') + +def validate_config_http(file, check): + """Determines if integration with http wrapper class + uses the http template in its spec.yaml file. + + file -- filepath of file to validate + check -- name of the check that file belongs to + """ + error_message = [] + if not os.path.exists(file): + return + + has_failed = False + with open(file, 'r', encoding='utf-8') as f: + read_file = f.read() + has_init_config_http = re.search(HTTP_WRAPPER_INIT_CONFIG_RE, read_file) + has_instance_http = re.search(HTTP_WRAPPER_INSTANCE_RE, read_file) + + if has_init_config_http and has_instance_http: + return + + if not has_instance_http: + message = ( + f"Detected {check} is missing `instances/http` or `instances/openmetrics_legacy` template in spec.yaml" + ) + error_message.append(message) + + has_failed = True + + if not has_init_config_http: + message = ( + f"Detected {check} is missing `init_config/http` or `init_config/openmetrics_legacy` template in spec.yaml" + ) + + error_message.append(message) + has_failed = True + + return has_failed, error_message + +def validate_use_http_wrapper_file(file, check): + """Return true if the file uses the http wrapper class. + Also outputs every instance of deprecated request library function use + + file -- filepath of file to validate + check -- name of the check + """ + file_uses_http_wrapper = False + has_failed = False + error_message = '' + with open(file, 'r', encoding='utf-8') as f: + read_file = f.read() + found_match_arg = re.search(r"auth=|header=", read_file) + found_http = re.search(r"self.http|OpenMetricsBaseCheck", read_file) + skip_validation = re.search(r"SKIP_HTTP_VALIDATION", read_file) + http_func = re.search(REQUEST_LIBRARY_FUNC_RE, read_file) + if http_func and not skip_validation: + error_message += ( + f'Check `{check}` uses `{http_func.group(0)}` in `{os.path.basename(file)}`, ' + f'please use the HTTP wrapper instead\n' + f"If this a genuine usage of the parameters, " + f"please inline comment `# SKIP_HTTP_VALIDATION`\n" + ) + return False, True, None, error_message + if found_http and not skip_validation: + return found_http, has_failed, found_match_arg, error_message + + return file_uses_http_wrapper, has_failed, None, error_message + +def validate_use_http_wrapper(check, app): + """Return true if the check uses the http wrapper class in any of its files. + If any of the check's files uses the request library, abort. + + check -- name of the check + """ + has_failed = False + check_uses_http_wrapper = False + warning_message = '' + error_message = '' + for file in app.repo.integrations.get(check).package_files(): + file_str = str(file) + if file_str.endswith('.py'): + file_uses_http_wrapper, file_uses_request_lib, has_arg_warning, error = validate_use_http_wrapper_file(file_str, check) + has_failed = has_failed or file_uses_request_lib + error_message += error + check_uses_http_wrapper = check_uses_http_wrapper or file_uses_http_wrapper + if check_uses_http_wrapper and has_arg_warning: + # Check for headers= or auth= + warning_message += ( + f" The HTTP wrapper contains parameter `{has_arg_warning.group().replace('=', '')}`, " + f"this configuration is handled by the wrapper automatically.\n" + f" If this a genuine usage of the parameters, " + f"please inline comment `# SKIP_HTTP_VALIDATION`\n" + ) + pass + + if has_failed: + return check_uses_http_wrapper, warning_message, error_message + app.abort() + return check_uses_http_wrapper, warning_message, error_message + + +@click.command(short_help='Validate usage of http wrapper') +@click.argument('check', nargs=-1) +@click.pass_obj +def http(app: Application, check: tuple[str, ...]): + """Validate all integrations for usage of http wrapper. + + If `check` is specified, only the check will be validated, + an 'all' `check` value will validate all checks. + """ + validation_tracker = app.create_validation_tracker('HTTP wrapper validation') + has_failed = False + + check_iterable = app.repo.integrations.iter(check) + app.display_info(f"Validating {sum(1 for _ in check_iterable)} integrations for usage of http wrapper...") + + for curr_check in app.repo.integrations.iter(check): + check_uses_http_wrapper = False + + # Validate use of http wrapper (self.http.[...]) in check's .py files + if curr_check.name not in EXCLUDED_INTEGRATIONS: + check_uses_http_wrapper, warning_message, error_message = validate_use_http_wrapper(curr_check.name, app) + + if warning_message: + validation_tracker.warning((curr_check.display_name,), message=warning_message) + if error_message: + has_failed = True + validation_tracker.error((curr_check.display_name,), message=error_message) + # Validate use of http template in check's spec.yaml (if exists) + if check_uses_http_wrapper: + validate_config_result = validate_config_http(get_default_config_spec(curr_check.name, app), curr_check.name) + if validate_config_result: + config_http_failure, config_http_msg = validate_config_result + has_failed = config_http_failure or has_failed + validation_tracker.error((curr_check.display_name,), message='\n'.join(config_http_msg)) + + if has_failed: + validation_tracker.display() + app.abort() + else: + validation_tracker.success() + validation_tracker.display() + app.display_success('Completed http validation!') diff --git a/ddev/tests/cli/validate/test_http.py b/ddev/tests/cli/validate/test_http.py new file mode 100644 index 0000000000000..6e67ba99ccadf --- /dev/null +++ b/ddev/tests/cli/validate/test_http.py @@ -0,0 +1,106 @@ +# (C) Datadog, Inc. 2023-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +def test_warn_headers_auth(ddev, repository, helpers): + check = 'apache' + file_path = repository.path / check / 'datadog_checks' / check / 'apache.py' + with file_path.open(encoding='utf-8') as file: + file_contents = file.readlines() + + file_contents[16] = " auth='test'" + + with file_path.open(mode='w', encoding='utf-8') as file: + file.writelines(file_contents) + + result = ddev('validate', 'http', check) + assert result.exit_code == 0, result.output + warning = 'The HTTP wrapper contains parameter `auth`, this configuration is' + assert warning in helpers.remove_trailing_spaces(result.output) + +def test_uses_requests(ddev, repository, helpers): + check = 'apache' + file_path = repository.path / check / 'datadog_checks' / check / 'apache.py' + with file_path.open(encoding='utf-8') as file: + file_contents = file.readlines() + + file_contents[16] = " test=requests.get()" + + with file_path.open(mode='w', encoding='utf-8') as file: + file.writelines(file_contents) + + result = ddev('validate', 'http', check) + assert result.exit_code == 1, result.output + error = 'Check `apache` uses `requests.get(` in `apache.py`,' + assert error in helpers.remove_trailing_spaces(result.output) + +def test_spec_missing_info_config(ddev, repository, helpers): + import yaml + check = 'apache' + + spec_yaml = repository.path / check / 'assets' / 'configuration' / 'spec.yaml' + with spec_yaml.open(encoding='utf-8') as file: + spec_info = yaml.safe_load(file) + + spec_info['files'][0]['options'][0]['options'] = [] + + output = yaml.safe_dump(spec_info, default_flow_style=False, sort_keys=False) + with spec_yaml.open(mode='w', encoding='utf-8') as file: + file.write(output) + + result = ddev('validate', 'http', check) + + assert result.exit_code == 1, result.output + error = 'Detected apache is missing `init_config/http` or' + assert error in helpers.remove_trailing_spaces(result.output) + +def test_spec_missing_instance(ddev, repository, helpers): + import yaml + check = 'apache' + + spec_yaml = repository.path / check / 'assets' / 'configuration' / 'spec.yaml' + with spec_yaml.open(encoding='utf-8') as file: + spec_info = yaml.safe_load(file) + + spec_info['files'][0]['options'][1]['options'] = spec_info['files'][0]['options'][1]['options'][0] + + output = yaml.safe_dump(spec_info, default_flow_style=False, sort_keys=False) + with spec_yaml.open(mode='w', encoding='utf-8') as file: + file.write(output) + + result = ddev('validate', 'http', check) + + assert result.exit_code == 1, result.output + error = 'Detected apache is missing `instances/http` or' + assert error in helpers.remove_trailing_spaces(result.output) + + +def test_validate_http_success(ddev, repository, helpers): + result = ddev('validate', 'http', 'apache', 'arangodb', 'zk') + assert result.exit_code == 0, result.output + assert helpers.remove_trailing_spaces(result.output) == helpers.dedent( + """ + Validating 3 integrations for usage of http wrapper... + HTTP wrapper validation + + Passed: 1 + Completed http validation! + """ + ) + +# def test_exactly_one_flag(ddev, repository, helpers): +# codecov_yaml = repository.path / '.codecov.yml' + +# with codecov_yaml.open(encoding='utf-8') as file: +# codecov_yaml_info = yaml.safe_load(file) + +# codecov_yaml_info['coverage']['status']['project']['ActiveMQ_XML']['flags'].append('test') + +# output = yaml.safe_dump(codecov_yaml_info, default_flow_style=False, sort_keys=False) +# with codecov_yaml.open(mode='w', encoding='utf-8') as file: +# file.write(output) + +# result = ddev("validate", "ci") + +# assert result.exit_code == 1, result.output +# error = "Project `ActiveMQ_XML` must have exactly one flag" +# assert error in helpers.remove_trailing_spaces(result.output) \ No newline at end of file diff --git a/kube_apiserver_metrics/datadog_checks/kube_apiserver_metrics/kube_apiserver_metrics.py b/kube_apiserver_metrics/datadog_checks/kube_apiserver_metrics/kube_apiserver_metrics.py index b5e1fcfa7bb67..c489b5e9e5e4a 100644 --- a/kube_apiserver_metrics/datadog_checks/kube_apiserver_metrics/kube_apiserver_metrics.py +++ b/kube_apiserver_metrics/datadog_checks/kube_apiserver_metrics/kube_apiserver_metrics.py @@ -193,7 +193,7 @@ def apiserver_audit_event_total(self, metric, scraper_config): def rest_client_requests_total(self, metric, scraper_config): self.submit_metric('.rest_client_requests_total', metric, scraper_config) - def http_requests_total(self, metric, scraper_config): + def http_requests_total(self, metric, scraper_config): # SKIP_HTTP_VALIDATION self.submit_metric('.http_requests_total', metric, scraper_config) def apiserver_request_count(self, metric, scraper_config): diff --git a/marklogic/datadog_checks/marklogic/api.py b/marklogic/datadog_checks/marklogic/api.py index f17d492e04dd2..1f07b447933be 100644 --- a/marklogic/datadog_checks/marklogic/api.py +++ b/marklogic/datadog_checks/marklogic/api.py @@ -43,7 +43,7 @@ def get_status_data(self, resource=None): return self.http_get(route, params) - def get_requests_data(self, resource=None, name=None, group=None): + def get_requests_data(self, resource=None, name=None, group=None): # SKIP_HTTP_VALIDATION # type: (str, str, str) -> Dict[str, Any] """ https://docs.marklogic.com/REST/GET/manage/v2/requests diff --git a/marklogic/datadog_checks/marklogic/check.py b/marklogic/datadog_checks/marklogic/check.py index 9d8770070b9a0..453ed4a8d5c79 100644 --- a/marklogic/datadog_checks/marklogic/check.py +++ b/marklogic/datadog_checks/marklogic/check.py @@ -178,7 +178,7 @@ def _collect_resource_storage_metrics(self, resource_type, name, group, tags): def _collect_resource_request_metrics(self, resource_type, name, group, tags): # type: (str, str, str, List[str]) -> None """Collect request metrics of a specific resource""" - data = self.api.get_requests_data(resource=resource_type, name=name, group=group) + data = self.api.get_requests_data(resource=resource_type, name=name, group=group) # SKIP_HTTP_VALIDATION metrics = parse_per_resource_request_metrics(data, tags) self.submit_metrics(metrics)