From 4482a803d149a0a0c1676f472dbffbe5626dbe21 Mon Sep 17 00:00:00 2001 From: ddog-nasirthomas Date: Mon, 17 Nov 2025 16:26:51 -0500 Subject: [PATCH 1/6] Add validation to ensure third-party dependencies are not in project.dependencies --- .../dev/tooling/dependencies.py | 39 +++++++-- .../tooling/commands/validate/test_dep.py | 82 +++++++++++++++++++ 2 files changed, 112 insertions(+), 9 deletions(-) create mode 100644 datadog_checks_dev/tests/tooling/commands/validate/test_dep.py diff --git a/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py b/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py index 8d5aa41decbcb..4d3c8039acf0c 100644 --- a/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py +++ b/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py @@ -97,20 +97,32 @@ def load_dependency_data_from_requirements(req_file, dependencies, errors, check def load_base_check(check_name, dependencies, errors): project_data = load_project_file_cached(check_name) check_dependencies = project_data['project'].get('dependencies', []) + dependency_errors = [] + found_base = False + for check_dependency in check_dependencies: try: req = Requirement(check_dependency) except InvalidRequirement as e: - errors.append(f'File `{check_name}/pyproject.toml` has an invalid dependency: `{check_dependency}`\n{e}') + errors.append(f'There is an invalid dependency: `{check_dependency}`\n{e}') continue name = normalize_project_name(req.name) if name == 'datadog-checks-base': dependencies[check_name] = get_normalized_dependency(req) - break - else: - errors.append(f'File `{check_name}/pyproject.toml` is missing the base check dependency `datadog-checks-base`') - + found_base = True + else: + dependency_errors.append(name) + + if not found_base: + errors.append('Missing the required base check dependency `datadog-checks-base` in project.dependencies') + + if dependency_errors: + dependency_errors_str = ', '.join(f'`{error}`' for error in dependency_errors) + errors.append( + f'Found third-party dependencies in project.dependencies: {dependency_errors_str}. ' + '\n - Third-party dependencies belong in project.optional-dependencies' + ) def load_base_check_legacy(req_file, dependencies, errors, check_name=None): for line in stream_file_lines(req_file): @@ -120,14 +132,14 @@ def load_base_check_legacy(req_file, dependencies, errors, check_name=None): dep = line.split(' = ')[1] req = Requirement(dep.strip("'\"")) except (IndexError, InvalidRequirement) as e: - errors.append(f'File `{req_file}` has an invalid base check dependency: `{line}`\n{e}') + errors.append(f'Has an invalid base check dependency: `{line}`\n{e}') return dependencies[check_name] = get_normalized_dependency(req) return # no `CHECKS_BASE_REQ` found in setup.py file .. - errors.append(f'File `{req_file}` missing base check dependency `CHECKS_BASE_REQ`') + errors.append(f'Missing base check dependency `CHECKS_BASE_REQ`') def read_check_dependencies(check=None): @@ -157,6 +169,7 @@ def read_check_base_dependencies(check=None): root = get_root() dependencies = {} errors = [] + error_msg = [] if isinstance(check, list): checks = sorted(check) @@ -171,11 +184,19 @@ def read_check_base_dependencies(check=None): if has_project_file(check_name): load_base_check(check_name, dependencies, errors) + if errors: + file_name = f"{check_name}/pyproject.toml" else: req_file = os.path.join(root, check_name, 'setup.py') load_base_check_legacy(req_file, dependencies, errors, check_name) - - return dependencies, errors + if errors: + file_name = req_file + + if errors: + errors_str = '\n'.join(f' - {error}' for error in errors) + error_msg = [f'\n{file_name} has the following errors:\n{errors_str}'] + + return dependencies, error_msg def update_check_dependencies_at(path, dependencies): diff --git a/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py b/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py new file mode 100644 index 0000000000000..bfbb5172f69ba --- /dev/null +++ b/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py @@ -0,0 +1,82 @@ +# (C) Datadog, Inc. 2025-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +import tomli +import tomli_w +import pytest + +from datadog_checks.dev.tooling.utils import set_root +from datadog_checks.dev.tooling.commands.validate.dep import dep +from click.testing import CliRunner + +def test_invalid_third_party_integration(fake_repo): + create_integration(fake_repo, 'foo', ['dep-a==1.0.0', 'dep-b==3.1.4']) + + runner = CliRunner() + result = runner.invoke(dep) + + assert result.exit_code == 1 + assert "Third-party" in result.output + assert "base check dependency" in result.output + +def test_multiple_invalid_third_party_integrations(fake_repo): + create_integration(fake_repo, 'foo11', ['dep-a==1.0.0', 'dep-b==3.1.4']) + create_integration(fake_repo, 'foo22', ['dep-a==1.0.0', 'dep-b==3.1.4']) + create_integration(fake_repo, 'foo33', ['dep-a==1.0.0', 'dep-b==3.1.4']) + create_integration(fake_repo, 'foo44', ['datadog-checks-base>=37.21.0','dep-a==1.0.0']) + + runner = CliRunner() + result = runner.invoke(dep) + + assert result.exit_code == 1 + assert "Third-party" in result.output + assert "base check dependency" in result.output + +def test_valid_integration(fake_repo): + create_integration(fake_repo, 'foo55', ['datadog-checks-base>=37.21.0']) + runner = CliRunner() + result = runner.invoke(dep) + + assert result.exit_code == 0 + assert "valid" in result.output + + +def test_one_valid_one_invalid_integration(fake_repo): + create_integration(fake_repo, 'foo66', ['dep-a==1.0.0', 'dep-b==3.1.4','datadog-checks-base>=37.21.0']) + create_integration(fake_repo, 'foo67', ['datadog-checks-base>=37.21.0']) + + runner = CliRunner() + result = runner.invoke(dep) + + assert result.exit_code == 1 + assert "Third-party" in result.output + +def create_integration(root, name, dependencies): + """Helper function to create a fake integration for testing.""" + integration_dir = root / name + integration_dir.mkdir(exist_ok=True) + with open(integration_dir / 'pyproject.toml', 'wb') as f: + tomli_w.dump({'project': {'dependencies': dependencies}}, f) + + # Fill stuff needed for it to be recognized as an agent check + (integration_dir / 'datadog_checks' / name).mkdir(parents=True) + (integration_dir / 'datadog_checks' / name / '__about__.py').touch() + (integration_dir / 'datadog_checks' / name / '__init__.py').write_text( + """ +import a +import b +""" + ) + +@pytest.fixture +def fake_repo(tmp_path, monkeypatch): + """Create a minimal fake repo without config_file dependency.""" + data_folder = tmp_path / 'datadog_checks_base' / 'datadog_checks' / 'base' / 'data' + data_folder.mkdir(parents=True) + + set_root(str(tmp_path)) + + yield tmp_path + + set_root('') + From 05fd10ddba2b03ce66d2841bffdb7503cef64f60 Mon Sep 17 00:00:00 2001 From: ddog-nasirthomas Date: Mon, 17 Nov 2025 16:28:10 -0500 Subject: [PATCH 2/6] Fix: Output all error files when multiple integrations have failures --- .../datadog_checks/dev/tooling/commands/validate/dep.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/dep.py b/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/dep.py index f66267dcbd36b..c44c36a4da4b8 100644 --- a/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/dep.py +++ b/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/dep.py @@ -220,9 +220,9 @@ def dep(check, require_base_check_version, min_base_check_version): check_base_dependencies, check_base_errors = read_check_base_dependencies(check_name) annotate_errors(base_req_source, check_base_errors) if check_base_errors: + failed = True for check_error in check_base_errors: echo_failure(check_error) - abort() for name, versions in sorted(check_dependencies.items()): if not verify_dependency('Checks', name, versions, req_source): @@ -279,6 +279,6 @@ def dep(check, require_base_check_version, min_base_check_version): annotate_error(agent_dependencies_file, message) continue - if failed: - abort() + if failed: + abort() echo_success("All dependencies are valid!") From 69e3527e2b32da436ebd4c2c5e194f5f6c4be919 Mon Sep 17 00:00:00 2001 From: ddog-nasirthomas Date: Mon, 17 Nov 2025 17:09:14 -0500 Subject: [PATCH 3/6] Add changelog --- datadog_checks_dev/changelog.d/21898.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 datadog_checks_dev/changelog.d/21898.fixed diff --git a/datadog_checks_dev/changelog.d/21898.fixed b/datadog_checks_dev/changelog.d/21898.fixed new file mode 100644 index 0000000000000..83fe87b120070 --- /dev/null +++ b/datadog_checks_dev/changelog.d/21898.fixed @@ -0,0 +1 @@ +Validate that dependencies are in the correct section in pyproject.toml From 9d196a5da1a08c5daaa883f1b6c73926637ae55f Mon Sep 17 00:00:00 2001 From: ddog-nasirthomas Date: Mon, 17 Nov 2025 17:13:07 -0500 Subject: [PATCH 4/6] Fix lint issues --- .../dev/tooling/dependencies.py | 7 ++--- .../tooling/commands/validate/test_dep.py | 26 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py b/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py index 4d3c8039acf0c..185711ff2292a 100644 --- a/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py +++ b/datadog_checks_dev/datadog_checks/dev/tooling/dependencies.py @@ -116,7 +116,7 @@ def load_base_check(check_name, dependencies, errors): if not found_base: errors.append('Missing the required base check dependency `datadog-checks-base` in project.dependencies') - + if dependency_errors: dependency_errors_str = ', '.join(f'`{error}`' for error in dependency_errors) errors.append( @@ -124,6 +124,7 @@ def load_base_check(check_name, dependencies, errors): '\n - Third-party dependencies belong in project.optional-dependencies' ) + def load_base_check_legacy(req_file, dependencies, errors, check_name=None): for line in stream_file_lines(req_file): line = line.strip() @@ -139,7 +140,7 @@ def load_base_check_legacy(req_file, dependencies, errors, check_name=None): return # no `CHECKS_BASE_REQ` found in setup.py file .. - errors.append(f'Missing base check dependency `CHECKS_BASE_REQ`') + errors.append('Missing base check dependency `CHECKS_BASE_REQ`') def read_check_dependencies(check=None): @@ -191,7 +192,7 @@ def read_check_base_dependencies(check=None): load_base_check_legacy(req_file, dependencies, errors, check_name) if errors: file_name = req_file - + if errors: errors_str = '\n'.join(f' - {error}' for error in errors) error_msg = [f'\n{file_name} has the following errors:\n{errors_str}'] diff --git a/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py b/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py index bfbb5172f69ba..7e111afe42222 100644 --- a/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py +++ b/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py @@ -1,14 +1,15 @@ # (C) Datadog, Inc. 2025-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) -import tomli -import tomli_w import pytest -from datadog_checks.dev.tooling.utils import set_root -from datadog_checks.dev.tooling.commands.validate.dep import dep +import tomli_w from click.testing import CliRunner +from datadog_checks.dev.tooling.commands.validate.dep import dep +from datadog_checks.dev.tooling.utils import set_root + + def test_invalid_third_party_integration(fake_repo): create_integration(fake_repo, 'foo', ['dep-a==1.0.0', 'dep-b==3.1.4']) @@ -19,11 +20,12 @@ def test_invalid_third_party_integration(fake_repo): assert "Third-party" in result.output assert "base check dependency" in result.output + def test_multiple_invalid_third_party_integrations(fake_repo): create_integration(fake_repo, 'foo11', ['dep-a==1.0.0', 'dep-b==3.1.4']) create_integration(fake_repo, 'foo22', ['dep-a==1.0.0', 'dep-b==3.1.4']) create_integration(fake_repo, 'foo33', ['dep-a==1.0.0', 'dep-b==3.1.4']) - create_integration(fake_repo, 'foo44', ['datadog-checks-base>=37.21.0','dep-a==1.0.0']) + create_integration(fake_repo, 'foo44', ['datadog-checks-base>=37.21.0', 'dep-a==1.0.0']) runner = CliRunner() result = runner.invoke(dep) @@ -32,6 +34,7 @@ def test_multiple_invalid_third_party_integrations(fake_repo): assert "Third-party" in result.output assert "base check dependency" in result.output + def test_valid_integration(fake_repo): create_integration(fake_repo, 'foo55', ['datadog-checks-base>=37.21.0']) runner = CliRunner() @@ -39,10 +42,10 @@ def test_valid_integration(fake_repo): assert result.exit_code == 0 assert "valid" in result.output - + def test_one_valid_one_invalid_integration(fake_repo): - create_integration(fake_repo, 'foo66', ['dep-a==1.0.0', 'dep-b==3.1.4','datadog-checks-base>=37.21.0']) + create_integration(fake_repo, 'foo66', ['dep-a==1.0.0', 'dep-b==3.1.4', 'datadog-checks-base>=37.21.0']) create_integration(fake_repo, 'foo67', ['datadog-checks-base>=37.21.0']) runner = CliRunner() @@ -51,6 +54,7 @@ def test_one_valid_one_invalid_integration(fake_repo): assert result.exit_code == 1 assert "Third-party" in result.output + def create_integration(root, name, dependencies): """Helper function to create a fake integration for testing.""" integration_dir = root / name @@ -68,15 +72,15 @@ def create_integration(root, name, dependencies): """ ) + @pytest.fixture def fake_repo(tmp_path, monkeypatch): """Create a minimal fake repo without config_file dependency.""" data_folder = tmp_path / 'datadog_checks_base' / 'datadog_checks' / 'base' / 'data' data_folder.mkdir(parents=True) - + set_root(str(tmp_path)) - + yield tmp_path - - set_root('') + set_root('') From 6fef18c2cf1825cdf03de0b32e3470fa8b331a20 Mon Sep 17 00:00:00 2001 From: ddog-nasirthomas Date: Mon, 17 Nov 2025 17:13:28 -0500 Subject: [PATCH 5/6] Fix lint issue --- datadog_checks_dev/tests/tooling/commands/validate/test_dep.py | 1 - 1 file changed, 1 deletion(-) diff --git a/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py b/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py index 7e111afe42222..bd25444dd84b2 100644 --- a/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py +++ b/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py @@ -2,7 +2,6 @@ # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) import pytest - import tomli_w from click.testing import CliRunner From 5214a5fd827d2bb2b8846f5983979a9ad0d763f7 Mon Sep 17 00:00:00 2001 From: ddog-nasirthomas Date: Wed, 19 Nov 2025 18:59:15 -0500 Subject: [PATCH 6/6] Moving tests to ddev folder --- .../tooling/commands/validate/test_dep.py | 85 -------------- ddev/tests/cli/validate/test_dep.py | 107 ++++++++++++++++++ 2 files changed, 107 insertions(+), 85 deletions(-) delete mode 100644 datadog_checks_dev/tests/tooling/commands/validate/test_dep.py create mode 100644 ddev/tests/cli/validate/test_dep.py diff --git a/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py b/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py deleted file mode 100644 index bd25444dd84b2..0000000000000 --- a/datadog_checks_dev/tests/tooling/commands/validate/test_dep.py +++ /dev/null @@ -1,85 +0,0 @@ -# (C) Datadog, Inc. 2025-present -# All rights reserved -# Licensed under a 3-clause BSD style license (see LICENSE) -import pytest -import tomli_w -from click.testing import CliRunner - -from datadog_checks.dev.tooling.commands.validate.dep import dep -from datadog_checks.dev.tooling.utils import set_root - - -def test_invalid_third_party_integration(fake_repo): - create_integration(fake_repo, 'foo', ['dep-a==1.0.0', 'dep-b==3.1.4']) - - runner = CliRunner() - result = runner.invoke(dep) - - assert result.exit_code == 1 - assert "Third-party" in result.output - assert "base check dependency" in result.output - - -def test_multiple_invalid_third_party_integrations(fake_repo): - create_integration(fake_repo, 'foo11', ['dep-a==1.0.0', 'dep-b==3.1.4']) - create_integration(fake_repo, 'foo22', ['dep-a==1.0.0', 'dep-b==3.1.4']) - create_integration(fake_repo, 'foo33', ['dep-a==1.0.0', 'dep-b==3.1.4']) - create_integration(fake_repo, 'foo44', ['datadog-checks-base>=37.21.0', 'dep-a==1.0.0']) - - runner = CliRunner() - result = runner.invoke(dep) - - assert result.exit_code == 1 - assert "Third-party" in result.output - assert "base check dependency" in result.output - - -def test_valid_integration(fake_repo): - create_integration(fake_repo, 'foo55', ['datadog-checks-base>=37.21.0']) - runner = CliRunner() - result = runner.invoke(dep) - - assert result.exit_code == 0 - assert "valid" in result.output - - -def test_one_valid_one_invalid_integration(fake_repo): - create_integration(fake_repo, 'foo66', ['dep-a==1.0.0', 'dep-b==3.1.4', 'datadog-checks-base>=37.21.0']) - create_integration(fake_repo, 'foo67', ['datadog-checks-base>=37.21.0']) - - runner = CliRunner() - result = runner.invoke(dep) - - assert result.exit_code == 1 - assert "Third-party" in result.output - - -def create_integration(root, name, dependencies): - """Helper function to create a fake integration for testing.""" - integration_dir = root / name - integration_dir.mkdir(exist_ok=True) - with open(integration_dir / 'pyproject.toml', 'wb') as f: - tomli_w.dump({'project': {'dependencies': dependencies}}, f) - - # Fill stuff needed for it to be recognized as an agent check - (integration_dir / 'datadog_checks' / name).mkdir(parents=True) - (integration_dir / 'datadog_checks' / name / '__about__.py').touch() - (integration_dir / 'datadog_checks' / name / '__init__.py').write_text( - """ -import a -import b -""" - ) - - -@pytest.fixture -def fake_repo(tmp_path, monkeypatch): - """Create a minimal fake repo without config_file dependency.""" - data_folder = tmp_path / 'datadog_checks_base' / 'datadog_checks' / 'base' / 'data' - data_folder.mkdir(parents=True) - - set_root(str(tmp_path)) - - yield tmp_path - - set_root('') diff --git a/ddev/tests/cli/validate/test_dep.py b/ddev/tests/cli/validate/test_dep.py new file mode 100644 index 0000000000000..a4a223b5baed8 --- /dev/null +++ b/ddev/tests/cli/validate/test_dep.py @@ -0,0 +1,107 @@ +# (C) Datadog, Inc. 2024-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +import re + +from tests.helpers.api import write_file + +error_regex = re.compile(r"(?s)^\s*[A-Za-z0-9_\/.-]+\.toml has the following errors:\n(?: - .+\n)+") +match_regex = re.compile(r"^\s*All dependencies are valid!\s*$") + + +def test_valid_integration(fake_repo, ddev): + write_file( + fake_repo.path / 'valid_check', + 'pyproject.toml', + """ + [project] + dependencies = [ + "datadog-checks-base>=37.21.0", + ] + """, + ) + result = ddev('validate', 'dep', 'valid_check') + assert result.exit_code == 0 + assert match_regex.match(result.output), f"Unexpected output: {result.output}" + + +def test_invalid_third_party_integration(fake_repo, ddev): + write_file( + fake_repo.path / 'bad_check', + 'pyproject.toml', + """ + [project] + dependencies = [ + "datadog-checks-base>=37.21.0", + "dep-d==1.5.0", + ] + """, + ) + result = ddev('validate', 'dep', 'bad_check') + assert result.exit_code == 1 + assert error_regex.search(result.output), f"Unexpected output: {result.output}" + + +def test_multiple_invalid_third_party_integrations(fake_repo, ddev): + write_file( + fake_repo.path / 'bad_check_2', + 'pyproject.toml', + """ + [project] + dependencies = [ + "dep-b==1.5.0", + "dep-e==1.5.0", + ] + """, + ) + + write_file( + fake_repo.path / 'bad_check_3', + 'pyproject.toml', + """ + [project] + dependencies = [ + "datadog-checks-base>=37.21.0", + "dep-f==1.5.0", + ] + """, + ) + + result = ddev('validate', 'dep', 'bad_check_2') + result_2 = ddev('validate', 'dep', 'bad_check_3') + assert result.exit_code == 1 + assert error_regex.search(result.output), f"Unexpected output: {result.output}" + assert result_2.exit_code == 1 + assert error_regex.search(result_2.output), f"Unexpected output: {result_2.output}" + + +def test_one_valid_one_invalid_integration(fake_repo, ddev): + write_file( + fake_repo.path / 'valid_check_2', + 'pyproject.toml', + """ + [project] + dependencies = [ + "datadog-checks-base>=37.21.0", + ] + """, + ) + + write_file( + fake_repo.path / 'bad_check_4', + 'pyproject.toml', + """ + [project] + dependencies = [ + "datadog-checks-base>=37.21.0", + "dep-f==1.5.0", + ] + """, + ) + + result = ddev('validate', 'dep', 'valid_check_2') + result_2 = ddev('validate', 'dep', 'bad_check_4') + assert result.exit_code == 0 + assert match_regex.match(result.output), f"Unexpected output: {result.output}" + assert result_2.exit_code == 1 + assert error_regex.search(result_2.output), f"Unexpected output: {result_2.output}"