From c9774c2085da7446f4f867093ff15651ad3342ce Mon Sep 17 00:00:00 2001 From: Sarah Wang Date: Wed, 9 Aug 2023 15:49:23 -0400 Subject: [PATCH 01/10] 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) From e86292dce09fd714c63a3589ca07563098e4cb75 Mon Sep 17 00:00:00 2001 From: Sarah Wang Date: Wed, 9 Aug 2023 16:54:22 -0400 Subject: [PATCH 02/10] style fixes --- ddev/src/ddev/cli/validate/http.py | 17 +++++++++++++---- ddev/tests/cli/validate/test_http.py | 22 ++++++++++++++-------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/ddev/src/ddev/cli/validate/http.py b/ddev/src/ddev/cli/validate/http.py index a131b505d0ef9..e7bc5445dcca4 100644 --- a/ddev/src/ddev/cli/validate/http.py +++ b/ddev/src/ddev/cli/validate/http.py @@ -2,11 +2,12 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) from __future__ import annotations + import os import re +from typing import TYPE_CHECKING import click -from typing import TYPE_CHECKING if TYPE_CHECKING: from ddev.cli.application import Application @@ -16,9 +17,11 @@ 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. @@ -57,6 +60,7 @@ def validate_config_http(file, check): 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 @@ -86,6 +90,7 @@ def validate_use_http_wrapper_file(file, check): 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. @@ -99,7 +104,9 @@ def validate_use_http_wrapper(check, app): 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) + 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 @@ -125,7 +132,7 @@ def validate_use_http_wrapper(check, app): 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, + 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') @@ -148,7 +155,9 @@ def http(app: Application, check: tuple[str, ...]): 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) + 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 diff --git a/ddev/tests/cli/validate/test_http.py b/ddev/tests/cli/validate/test_http.py index 6e67ba99ccadf..37fc07120ad37 100644 --- a/ddev/tests/cli/validate/test_http.py +++ b/ddev/tests/cli/validate/test_http.py @@ -17,6 +17,7 @@ def test_warn_headers_auth(ddev, repository, helpers): 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' @@ -33,14 +34,16 @@ def test_uses_requests(ddev, repository, helpers): 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) @@ -48,19 +51,21 @@ def test_spec_missing_info_config(ddev, repository, helpers): 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) @@ -68,7 +73,7 @@ def test_spec_missing_instance(ddev, repository, helpers): 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) @@ -86,7 +91,8 @@ def test_validate_http_success(ddev, repository, helpers): Completed http validation! """ ) - + + # def test_exactly_one_flag(ddev, repository, helpers): # codecov_yaml = repository.path / '.codecov.yml' @@ -103,4 +109,4 @@ def test_validate_http_success(ddev, repository, helpers): # 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 +# assert error in helpers.remove_trailing_spaces(result.output) From ec45dfb3fb3d7a98deaef05fb2938ccdd3d72837 Mon Sep 17 00:00:00 2001 From: Sarah Wang Date: Wed, 9 Aug 2023 17:39:17 -0400 Subject: [PATCH 03/10] style fixes cont --- .../kube_apiserver_metrics/kube_apiserver_metrics.py | 2 +- marklogic/datadog_checks/marklogic/api.py | 2 +- marklogic/datadog_checks/marklogic/check.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 c489b5e9e5e4a..a19577c9d2e50 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): # SKIP_HTTP_VALIDATION + 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 1f07b447933be..a9fb2827d8ac7 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): # SKIP_HTTP_VALIDATION + 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 453ed4a8d5c79..d3ce3971c7e23 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) # SKIP_HTTP_VALIDATION + 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) From 2d68ec2b2f6fdbcc88d100d43220adda8ea5c857 Mon Sep 17 00:00:00 2001 From: Sarah Wang Date: Wed, 9 Aug 2023 21:51:24 -0400 Subject: [PATCH 04/10] remove extra commented test --- ddev/tests/cli/validate/test_http.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/ddev/tests/cli/validate/test_http.py b/ddev/tests/cli/validate/test_http.py index 37fc07120ad37..ea8c8bf867721 100644 --- a/ddev/tests/cli/validate/test_http.py +++ b/ddev/tests/cli/validate/test_http.py @@ -92,21 +92,3 @@ def test_validate_http_success(ddev, repository, helpers): """ ) - -# 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) From e06b2602aada93b497747ab29bbd8999f1d217aa Mon Sep 17 00:00:00 2001 From: Sarah Wang Date: Wed, 9 Aug 2023 22:12:16 -0400 Subject: [PATCH 05/10] style fix --- ddev/tests/cli/validate/test_http.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ddev/tests/cli/validate/test_http.py b/ddev/tests/cli/validate/test_http.py index ea8c8bf867721..95884cf4789b2 100644 --- a/ddev/tests/cli/validate/test_http.py +++ b/ddev/tests/cli/validate/test_http.py @@ -91,4 +91,3 @@ def test_validate_http_success(ddev, repository, helpers): Completed http validation! """ ) - From 1052f82da6429a18a34f33fbda5423a14357127b Mon Sep 17 00:00:00 2001 From: Sarah Wang Date: Thu, 10 Aug 2023 13:28:51 -0400 Subject: [PATCH 06/10] update validation tracker count --- ddev/src/ddev/cli/validate/http.py | 8 +++++--- ddev/tests/cli/validate/test_http.py | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ddev/src/ddev/cli/validate/http.py b/ddev/src/ddev/cli/validate/http.py index e7bc5445dcca4..b164c9b94b1b2 100644 --- a/ddev/src/ddev/cli/validate/http.py +++ b/ddev/src/ddev/cli/validate/http.py @@ -122,7 +122,6 @@ def validate_use_http_wrapper(check, app): if has_failed: return check_uses_http_wrapper, warning_message, error_message - app.abort() return check_uses_http_wrapper, warning_message, error_message @@ -162,11 +161,14 @@ def http(app: Application, check: tuple[str, ...]): 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)) + else: + validation_tracker.success() + else: + if not error_message: + validation_tracker.success() 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 index 95884cf4789b2..7fce6579c0f7e 100644 --- a/ddev/tests/cli/validate/test_http.py +++ b/ddev/tests/cli/validate/test_http.py @@ -87,7 +87,6 @@ def test_validate_http_success(ddev, repository, helpers): Validating 3 integrations for usage of http wrapper... HTTP wrapper validation - Passed: 1 - Completed http validation! + Passed: 3 """ ) From 6e276abaf39975a0b20eaeff2cfce08314e9bcc0 Mon Sep 17 00:00:00 2001 From: Ofek Lev Date: Fri, 11 Aug 2023 11:38:46 -0400 Subject: [PATCH 07/10] cleanup --- ddev/pyproject.toml | 2 +- ddev/src/ddev/cli/validate/http.py | 51 ++++++-------- ddev/tests/cli/status/test_status.py | 1 + ddev/tests/cli/validate/test_http.py | 67 ++++++++++++++++--- ddev/tests/cli/validate/test_licenses.py | 1 + ddev/tests/cli/validate/test_manifest.py | 1 + ddev/tests/config/test_model.py | 1 + ddev/tests/conftest.py | 1 + ddev/tests/repo/test_core.py | 1 + ddev/tests/utils/test_platform.py | 1 + ddev/tests/validation/test_tracker.py | 3 +- kubelet/datadog_checks/kubelet/cadvisor.py | 2 +- .../datadog_checks/openstack/openstack.py | 2 +- 13 files changed, 90 insertions(+), 44 deletions(-) diff --git a/ddev/pyproject.toml b/ddev/pyproject.toml index a2d9e8b863be2..eddc5eb007d39 100644 --- a/ddev/pyproject.toml +++ b/ddev/pyproject.toml @@ -103,7 +103,7 @@ unfixable = [ ] [tool.ruff.isort] -known-first-party = ["{template_config['package_name']}"] +known-first-party = ["ddev"] [tool.ruff.flake8-tidy-imports] ban-relative-imports = "all" diff --git a/ddev/src/ddev/cli/validate/http.py b/ddev/src/ddev/cli/validate/http.py index b164c9b94b1b2..8e5e6f7376584 100644 --- a/ddev/src/ddev/cli/validate/http.py +++ b/ddev/src/ddev/cli/validate/http.py @@ -11,7 +11,6 @@ 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]*" @@ -82,7 +81,7 @@ def validate_use_http_wrapper_file(file, check): 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" + f"please inline comment `# SKIP_HTTP_VALIDATION`" ) return False, True, None, error_message if found_http and not skip_validation: @@ -113,10 +112,10 @@ def validate_use_http_wrapper(check, app): 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"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" + f"If this a genuine usage of the parameters, " + f"please inline comment `# SKIP_HTTP_VALIDATION`" ) pass @@ -125,50 +124,42 @@ def validate_use_http_wrapper(check, app): return check_uses_http_wrapper, warning_message, error_message -@click.command(short_help='Validate usage of http wrapper') -@click.argument('check', nargs=-1) +@click.command(short_help='Validate HTTP usage') +@click.argument('integrations', nargs=-1) @click.pass_obj -def http(app: Application, check: tuple[str, ...]): - """Validate all integrations for usage of http wrapper. +def http(app: Application, integrations: tuple[str, ...]): + """Validate all integrations for usage of HTTP wrapper. - If `check` is specified, only the check will be validated, + If `integrations` is specified, only those 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 + excluded = set(app.repo.config.get('/overrides/validate/http/exclude', [])) + for integration in app.repo.integrations.iter(integrations): + if integration.name in excluded or not integration.is_integration: + continue - # 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) + check_uses_http_wrapper, warning_message, error_message = validate_use_http_wrapper(integration.name, app) if warning_message: - validation_tracker.warning((curr_check.display_name,), message=warning_message) + validation_tracker.warning((integration.display_name,), message=warning_message) if error_message: - has_failed = True - validation_tracker.error((curr_check.display_name,), message=error_message) + validation_tracker.error((integration.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 + get_default_config_spec(integration.name, app), integration.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)) + _, config_http_msg = validate_config_result + validation_tracker.error((integration.display_name,), message='\n'.join(config_http_msg)) else: validation_tracker.success() else: if not error_message: validation_tracker.success() - if has_failed: - validation_tracker.display() + validation_tracker.display() + if validation_tracker.errors: app.abort() - else: - validation_tracker.display() diff --git a/ddev/tests/cli/status/test_status.py b/ddev/tests/cli/status/test_status.py index c9e157db5d0c6..50faae80960b0 100644 --- a/ddev/tests/cli/status/test_status.py +++ b/ddev/tests/cli/status/test_status.py @@ -2,6 +2,7 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import pytest + from ddev.config.constants import AppEnvVars from ddev.utils.structures import EnvVars diff --git a/ddev/tests/cli/validate/test_http.py b/ddev/tests/cli/validate/test_http.py index 7fce6579c0f7e..d518db74a13ca 100644 --- a/ddev/tests/cli/validate/test_http.py +++ b/ddev/tests/cli/validate/test_http.py @@ -1,6 +1,17 @@ # (C) Datadog, Inc. 2023-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) +import pytest + +from ddev.utils.structures import EnvVars + + +@pytest.fixture(scope='module', autouse=True) +def terminal_width(): + with EnvVars({'COLUMNS': '200'}): + yield + + def test_warn_headers_auth(ddev, repository, helpers): check = 'apache' file_path = repository.path / check / 'datadog_checks' / check / 'apache.py' @@ -13,9 +24,20 @@ def test_warn_headers_auth(ddev, repository, helpers): 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) + assert helpers.remove_trailing_spaces(result.output) == helpers.dedent( + """ + HTTP wrapper validation + └── Apache + + The HTTP wrapper contains parameter `auth`, this configuration is handled by the wrapper automatically. + If this a genuine usage of the parameters, please inline comment `# SKIP_HTTP_VALIDATION` + + Passed: 1 + Warnings: 1 + """ + ) def test_uses_requests(ddev, repository, helpers): @@ -30,12 +52,22 @@ def test_uses_requests(ddev, repository, helpers): 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) + assert helpers.remove_trailing_spaces(result.output) == helpers.dedent( + """ + HTTP wrapper validation + └── Apache + Check `apache` uses `requests.get(` in `apache.py`, please use the HTTP wrapper instead + If this a genuine usage of the parameters, please inline comment `# SKIP_HTTP_VALIDATION` -def test_spec_missing_info_config(ddev, repository, helpers): + Errors: 1 + """ + ) + + +def test_spec_missing_init_config(ddev, repository, helpers): import yaml check = 'apache' @@ -53,8 +85,16 @@ def test_spec_missing_info_config(ddev, repository, helpers): 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) + assert helpers.remove_trailing_spaces(result.output) == helpers.dedent( + """ + HTTP wrapper validation + └── Apache + + Detected apache is missing `init_config/http` or `init_config/openmetrics_legacy` template in spec.yaml + + Errors: 1 + """ + ) def test_spec_missing_instance(ddev, repository, helpers): @@ -75,8 +115,16 @@ def test_spec_missing_instance(ddev, repository, helpers): 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) + assert helpers.remove_trailing_spaces(result.output) == helpers.dedent( + """ + HTTP wrapper validation + └── Apache + + Detected apache is missing `instances/http` or `instances/openmetrics_legacy` template in spec.yaml + + Errors: 1 + """ + ) def test_validate_http_success(ddev, repository, helpers): @@ -84,7 +132,6 @@ def test_validate_http_success(ddev, repository, helpers): 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: 3 diff --git a/ddev/tests/cli/validate/test_licenses.py b/ddev/tests/cli/validate/test_licenses.py index 963d8025c64e5..5a0ea053d3251 100644 --- a/ddev/tests/cli/validate/test_licenses.py +++ b/ddev/tests/cli/validate/test_licenses.py @@ -6,6 +6,7 @@ import os import pytest + from ddev.utils.toml import dump_toml_data, load_toml_file diff --git a/ddev/tests/cli/validate/test_manifest.py b/ddev/tests/cli/validate/test_manifest.py index 21986ffb21313..b0a107a97a45d 100644 --- a/ddev/tests/cli/validate/test_manifest.py +++ b/ddev/tests/cli/validate/test_manifest.py @@ -4,6 +4,7 @@ import json import pytest + from ddev.utils.structures import EnvVars diff --git a/ddev/tests/config/test_model.py b/ddev/tests/config/test_model.py index d423fc20a6caf..44e31e3803284 100644 --- a/ddev/tests/config/test_model.py +++ b/ddev/tests/config/test_model.py @@ -4,6 +4,7 @@ import os import pytest + from ddev.config.model import ConfigurationError, RootConfig, get_github_token, get_github_user diff --git a/ddev/tests/conftest.py b/ddev/tests/conftest.py index 5d38d32ac0c8b..0e2bd66b514a7 100644 --- a/ddev/tests/conftest.py +++ b/ddev/tests/conftest.py @@ -12,6 +12,7 @@ import vcr from click.testing import CliRunner as __CliRunner from datadog_checks.dev.tooling.utils import set_root + from ddev.cli.terminal import Terminal from ddev.config.constants import AppEnvVars, ConfigEnvVars from ddev.config.file import ConfigFile diff --git a/ddev/tests/repo/test_core.py b/ddev/tests/repo/test_core.py index 722892675b3ff..fa546756af9e4 100644 --- a/ddev/tests/repo/test_core.py +++ b/ddev/tests/repo/test_core.py @@ -4,6 +4,7 @@ import os import pytest + from ddev.integration.core import Integration from ddev.repo.config import RepositoryConfig from ddev.repo.constants import NOT_SHIPPABLE diff --git a/ddev/tests/utils/test_platform.py b/ddev/tests/utils/test_platform.py index 62c74ecf2fd5f..ae37767355f2c 100644 --- a/ddev/tests/utils/test_platform.py +++ b/ddev/tests/utils/test_platform.py @@ -2,6 +2,7 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import pytest + from ddev.utils.platform import Platform diff --git a/ddev/tests/validation/test_tracker.py b/ddev/tests/validation/test_tracker.py index 2eae9338c6214..39b662769236e 100644 --- a/ddev/tests/validation/test_tracker.py +++ b/ddev/tests/validation/test_tracker.py @@ -2,11 +2,12 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import pytest -from ddev.validation.tracker import ValidationTracker from rich.console import Console from rich.style import Style from rich.tree import Tree +from ddev.validation.tracker import ValidationTracker + def get_tracker(): return ValidationTracker( diff --git a/kubelet/datadog_checks/kubelet/cadvisor.py b/kubelet/datadog_checks/kubelet/cadvisor.py index 8e7fa1f787b22..9e7572016053a 100644 --- a/kubelet/datadog_checks/kubelet/cadvisor.py +++ b/kubelet/datadog_checks/kubelet/cadvisor.py @@ -51,7 +51,7 @@ def detect_cadvisor(kubelet_url, cadvisor_port): url = "http://{}:{}{}".format(kubelet_hostname, cadvisor_port, LEGACY_CADVISOR_METRICS_PATH) # Test the endpoint is present - r = requests.head(url, timeout=1) + r = requests.head(url, timeout=1) # SKIP_HTTP_VALIDATION r.raise_for_status() return url diff --git a/openstack/datadog_checks/openstack/openstack.py b/openstack/datadog_checks/openstack/openstack.py index 2d2602d75b842..b7d9952e55114 100644 --- a/openstack/datadog_checks/openstack/openstack.py +++ b/openstack/datadog_checks/openstack/openstack.py @@ -144,7 +144,7 @@ def request_auth_token(cls, auth_scope, identity, keystone_server_url, ssl_verif auth_url = urljoin(keystone_server_url, "{0}/auth/tokens".format(DEFAULT_KEYSTONE_API_VERSION)) headers = {'Content-Type': 'application/json'} - resp = requests.post( + resp = requests.post( # SKIP_HTTP_VALIDATION auth_url, headers=headers, data=json.dumps(payload), From 77bb6a5f61c75cf7555475f8d4e5808763123715 Mon Sep 17 00:00:00 2001 From: Ofek Lev Date: Fri, 11 Aug 2023 11:44:32 -0400 Subject: [PATCH 08/10] Update test_changelog.py --- ddev/tests/cli/release/agent/test_changelog.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ddev/tests/cli/release/agent/test_changelog.py b/ddev/tests/cli/release/agent/test_changelog.py index 79118d131ec15..c0206845fd13f 100644 --- a/ddev/tests/cli/release/agent/test_changelog.py +++ b/ddev/tests/cli/release/agent/test_changelog.py @@ -4,6 +4,7 @@ import re import pytest + from ddev.repo.core import Repository From f3a11dd5d31a0e9a35b0c04a2530d45ea3b645f4 Mon Sep 17 00:00:00 2001 From: Ofek Lev Date: Fri, 11 Aug 2023 11:50:16 -0400 Subject: [PATCH 09/10] remove --- datadog_checks_dev/CHANGELOG.md | 4 + .../dev/tooling/commands/validate/__init__.py | 2 - .../dev/tooling/commands/validate/http.py | 145 ------------------ ddev/CHANGELOG.md | 4 + 4 files changed, 8 insertions(+), 147 deletions(-) delete mode 100644 datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/http.py diff --git a/datadog_checks_dev/CHANGELOG.md b/datadog_checks_dev/CHANGELOG.md index dc55aed849566..07ab9476a489e 100644 --- a/datadog_checks_dev/CHANGELOG.md +++ b/datadog_checks_dev/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +***Removed***: + +* Migrate `validate http` to ddev ([#15526](https://github.com/DataDog/integrations-core/pull/15526)) + ***Fixed***: * Stop using the TOX_ENV_NAME variable ([#15528](https://github.com/DataDog/integrations-core/pull/15528)) diff --git a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/__init__.py b/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/__init__.py index a2e13b989155f..118ddb53cf034 100644 --- a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/__init__.py +++ b/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/__init__.py @@ -13,7 +13,6 @@ from .dashboards import dashboards from .dep import dep from .eula import eula -from .http import http from .imports import imports from .integration_style import integration_style from .jmx_metrics import jmx_metrics @@ -38,7 +37,6 @@ dashboards, dep, eula, - http, imports, integration_style, jmx_metrics, diff --git a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/http.py b/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/http.py deleted file mode 100644 index 5662f4335fcde..0000000000000 --- a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/http.py +++ /dev/null @@ -1,145 +0,0 @@ -# (C) Datadog, Inc. 2020-present -# All rights reserved -# Licensed under a 3-clause BSD style license (see LICENSE) -import os -import re - -import click - -from ...testing import process_checks_option -from ...utils import complete_valid_checks, get_check_files, get_default_config_spec -from ..console import CONTEXT_SETTINGS, abort, annotate_error, echo_failure, echo_info, echo_success, echo_warning - -# Integrations that are not fully updated to http wrapper class but is owned partially by a different organization - -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 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 - """ - - 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" - ) - echo_failure(message) - annotate_error(file, 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" - ) - echo_failure(message) - annotate_error(file, message) - has_failed = True - - return has_failed - - -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 - 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) - if found_http and not skip_validation: - return found_http, has_failed, found_match_arg - - http_func = re.search(REQUEST_LIBRARY_FUNC_RE, read_file) - if http_func: - echo_failure( - f'Check `{check}` uses `{http_func}` in `{os.path.basename(file)}`, ' - f'please use the HTTP wrapper instead' - ) - annotate_error( - file, - "Detected use of `{}`, please use the HTTP wrapper instead".format(http_func), - ) - return False, True, None - - return file_uses_http_wrapper, has_failed, None - - -def validate_use_http_wrapper(check): - """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 - for file in get_check_files(check, include_tests=False): - if file.endswith('.py'): - file_uses_http_wrapper, file_uses_request_lib, has_arg_warning = validate_use_http_wrapper_file(file, check) - has_failed = has_failed or file_uses_request_lib - 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= - echo_warning( - f"{check}: \n" - 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`" - ) - pass - - if has_failed: - abort() - return check_uses_http_wrapper - - -@click.command(context_settings=CONTEXT_SETTINGS, short_help='Validate usage of http wrapper') -@click.argument('check', shell_complete=complete_valid_checks, required=False) -def http(check): - """Validate all integrations for usage of http wrapper.""" - - has_failed = False - - checks = process_checks_option(check, source='integrations') - echo_info(f"Validating {len(checks)} integrations for usage of http wrapper...") - - for check in checks: - check_uses_http_wrapper = False - - # Validate use of http wrapper (self.http.[...]) in check's .py files - if check not in EXCLUDED_INTEGRATIONS: - check_uses_http_wrapper = validate_use_http_wrapper(check) - - # Validate use of http template in check's spec.yaml (if exists) - if check_uses_http_wrapper: - has_failed = validate_config_http(get_default_config_spec(check), check) or has_failed - - if has_failed: - abort() - - echo_success('Completed http validation!') diff --git a/ddev/CHANGELOG.md b/ddev/CHANGELOG.md index 3de75f33475c1..bb1fd57b7159f 100644 --- a/ddev/CHANGELOG.md +++ b/ddev/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +***Added***: + +* Migrate `validate http` to ddev ([#15526](https://github.com/DataDog/integrations-core/pull/15526)) + ***Fixed***: * Output changelog to stdout instead of stderr on `ddev release agent changelog` ([#15548](https://github.com/DataDog/integrations-core/pull/15548)) From d9a6a50d6843c3fec00ac181d87073317385831e Mon Sep 17 00:00:00 2001 From: Ofek Lev Date: Fri, 11 Aug 2023 12:03:24 -0400 Subject: [PATCH 10/10] . --- .../commands/validate/all_validations.py | 2 -- ddev/src/ddev/cli/validate/http.py | 36 ++++++++----------- ddev/src/ddev/integration/core.py | 5 +++ 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/all_validations.py b/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/all_validations.py index 8574b2bac1474..0b0a1855de59c 100644 --- a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/all_validations.py +++ b/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/all_validations.py @@ -12,7 +12,6 @@ from .dashboards import dashboards from .dep import dep from .eula import eula -from .http import http from .imports import imports from .jmx_metrics import jmx_metrics from .manifest import manifest @@ -34,7 +33,6 @@ (dep, ('core',)), (eula, ('marketplace',)), (jmx_metrics, (None,)), - (http, ('core',)), (imports, (None,)), (manifest, (None,)), (metadata, (None,)), diff --git a/ddev/src/ddev/cli/validate/http.py b/ddev/src/ddev/cli/validate/http.py index 8e5e6f7376584..a6daec6f552c5 100644 --- a/ddev/src/ddev/cli/validate/http.py +++ b/ddev/src/ddev/cli/validate/http.py @@ -12,13 +12,9 @@ if TYPE_CHECKING: from ddev.cli.application import Application -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') +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 validate_config_http(file, check): @@ -43,7 +39,7 @@ def validate_config_http(file, check): if not has_instance_http: message = ( - f"Detected {check} is missing `instances/http` or `instances/openmetrics_legacy` template in spec.yaml" + f'Detected {check} is missing `instances/http` or `instances/openmetrics_legacy` template in spec.yaml' ) error_message.append(message) @@ -51,7 +47,7 @@ def validate_config_http(file, check): if not has_init_config_http: message = ( - f"Detected {check} is missing `init_config/http` or `init_config/openmetrics_legacy` template in spec.yaml" + f'Detected {check} is missing `init_config/http` or `init_config/openmetrics_legacy` template in spec.yaml' ) error_message.append(message) @@ -72,16 +68,16 @@ def validate_use_http_wrapper_file(file, check): 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) + 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`" + f'If this a genuine usage of the parameters, ' + f'please inline comment `# SKIP_HTTP_VALIDATION`' ) return False, True, None, error_message if found_http and not skip_validation: @@ -112,10 +108,10 @@ def validate_use_http_wrapper(check, app): 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`" + 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`' ) pass @@ -148,9 +144,7 @@ def http(app: Application, integrations: tuple[str, ...]): validation_tracker.error((integration.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(integration.name, app), integration.name - ) + validate_config_result = validate_config_http(str(integration.config_spec), integration.name) if validate_config_result: _, config_http_msg = validate_config_result validation_tracker.error((integration.display_name,), message='\n'.join(config_http_msg)) diff --git a/ddev/src/ddev/integration/core.py b/ddev/src/ddev/integration/core.py index 766efa7ec17b0..8eca1d899c437 100644 --- a/ddev/src/ddev/integration/core.py +++ b/ddev/src/ddev/integration/core.py @@ -104,6 +104,11 @@ def metrics_file(self) -> Path: relative_path = self.manifest.get('/assets/integration/metrics/metadata_path', 'metadata.csv') return self.path / relative_path + @cached_property + def config_spec(self) -> Path: + relative_path = self.manifest.get('/assets/integration/configuration/spec', 'assets/configuration/spec.yaml') + return self.path / relative_path + @cached_property def is_valid(self) -> bool: return self.is_integration or self.is_package