From 2271a70584e86913ecaee654c26d2705ec238d8d Mon Sep 17 00:00:00 2001 From: Nicolas Schweitzer Date: Thu, 30 Jan 2025 18:54:36 +0100 Subject: [PATCH 1/3] feat(protobuf): Test the protoc version --- pkg/proto/datadog/README.md | 2 +- tasks/libs/common/check_tools_version.py | 12 +++++ tasks/protobuf.py | 35 +++++++++++++-- .../libs/common/check_tools_tests.py | 45 +++++++++++++++++++ 4 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 tasks/unit_tests/libs/common/check_tools_tests.py diff --git a/pkg/proto/datadog/README.md b/pkg/proto/datadog/README.md index 3fc631553e9a0..ec4c58608fb74 100644 --- a/pkg/proto/datadog/README.md +++ b/pkg/proto/datadog/README.md @@ -1,4 +1,4 @@ ## gRPC: Protobuf and Gateway code generation 1. Ensure that you have the all the tools installed in your `$PATH` by running `inv -e install-tools`. -2. To generate the code for the `.proto` files run `inv -e generate-protobuf`. +2. To generate the code for the `.proto` files run `inv -e protobuf.generate`. diff --git a/tasks/libs/common/check_tools_version.py b/tasks/libs/common/check_tools_version.py index 5b2b33db959ab..63ab655eef436 100644 --- a/tasks/libs/common/check_tools_version.py +++ b/tasks/libs/common/check_tools_version.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import shutil import sys from invoke import Context, Exit @@ -102,3 +103,14 @@ def check_tools_version(ctx: Context, tools_list: list[str], debug: bool = False if should_exit: raise Exit(code=1) return True + + +def check_tools_installed(tools: list) -> bool: + """ + Check if the tools are installed in the system. + """ + for tool in tools: + if not shutil.which(tool): + print(f"{tool} is not installed. Please install it before running this task.") + return False + return True diff --git a/tasks/protobuf.py b/tasks/protobuf.py index 07bdb3035ec53..93a693a031115 100644 --- a/tasks/protobuf.py +++ b/tasks/protobuf.py @@ -1,8 +1,12 @@ import glob import os +import re from pathlib import Path -from invoke import Exit, task +from invoke import Exit, UnexpectedExit, task + +from tasks.install_tasks import TOOL_LIST_PROTO +from tasks.libs.common.check_tools_version import check_tools_installed PROTO_PKGS = { 'model/v1': (False, False), @@ -39,6 +43,7 @@ def generate(ctx, do_mockgen=True): We must build the packages one at a time due to protoc-gen-go limitations """ # Key: path, Value: grpc_gateway, inject_tags + check_dependencies(ctx) base = os.path.dirname(os.path.abspath(__file__)) repo_root = os.path.abspath(os.path.join(base, "..")) proto_root = os.path.join(repo_root, "pkg", "proto") @@ -145,9 +150,31 @@ def generate(ctx, do_mockgen=True): ctx.run(f"git apply {switches} --unsafe-paths --directory='{pbgo_dir}/{pkg}' {patch_file}") # Check the generated files were properly committed - updates = ctx.run("git status -suno").stdout.strip() - if updates: + git_status = ctx.run("git status -suno", hide=True).stdout + # git_status = ctx.run("git status -suno | grep -E 'pkg/proto/pbgo.*.pb(.gw)?.go'").stdout.strip() + proto_file = re.compile(r"pkg/proto/pbgo/.*\.pb(\.gw)?\.go$") + if any(proto_file.search(line) for line in git_status.split("\n")): raise Exit( - "Generated files were not properly committed. Please run `inv protobuf.generate` and commit the changes.", + f"Generated files were not properly committed.\n{git_status}\nPlease run `inv protobuf.generate` and commit the changes.", code=1, ) + + +def check_dependencies(ctx): + """ + Check if all the required dependencies are installed + """ + tools = [tool.split("/")[-1] for tool in TOOL_LIST_PROTO] + if not check_tools_installed(tools): + raise Exit("Please install the required tools with `inv install-tools` before running this task.", code=1) + try: + current_version = ctx.run("protoc --version", hide=True).stdout.strip().removeprefix("libprotoc ") + with open(".protoc-version") as f: + expected_version = f.read().strip() + if current_version != expected_version: + raise Exit( + f"Expected protoc version {expected_version}, found {current_version}. Please run `inv install-protoc` before running this task.", + code=1, + ) from None + except UnexpectedExit as e: + raise Exit("protoc is not installed. Please install it before running this task.", code=1) from e diff --git a/tasks/unit_tests/libs/common/check_tools_tests.py b/tasks/unit_tests/libs/common/check_tools_tests.py new file mode 100644 index 0000000000000..a4e0137ed0633 --- /dev/null +++ b/tasks/unit_tests/libs/common/check_tools_tests.py @@ -0,0 +1,45 @@ +import unittest +from unittest.mock import MagicMock, patch + +from invoke import Context, Exit, MockContext, Result, UnexpectedExit + +from tasks.libs.common.check_tools_version import check_tools_installed +from tasks.protobuf import check_dependencies + + +class TestCheckToolsInstalled(unittest.TestCase): + def test_single_tool_installed(self): + self.assertTrue(check_tools_installed(["git"])) + + def test_several_tools_installed(self): + self.assertTrue(check_tools_installed(["git", "ls"])) + + def test_tool_not_installed(self): + self.assertFalse(check_tools_installed(["not_installed_tool", "ls"])) + + +class TestCheckDependencies(unittest.TestCase): + @patch('tasks.protobuf.check_tools_installed', new=MagicMock(return_value=False)) + def test_tools_not_installed(self): + c = Context() + with self.assertRaises(Exit) as e: + check_dependencies(c) + print(e) + self.assertEqual( + e.exception.message, "Please install the required tools with `inv install-tools` before running this task." + ) + + @patch('tasks.protobuf.check_tools_installed', new=MagicMock(return_value=True)) + def test_bad_protoc(self): + c = MockContext(run={'protoc --version': Result("libprotoc 1.98.2")}) + with self.assertRaises(Exit) as e: + check_dependencies(c) + self.assertTrue(e.exception.message.startswith("Expected protoc version 29.3, found")) + + @patch('tasks.protobuf.check_tools_installed', new=MagicMock(return_value=True)) + def test_protoc_not_installed(self): + c = MagicMock() + c.run.side_effect = UnexpectedExit("protoc --version") + with self.assertRaises(Exit) as e: + check_dependencies(c) + self.assertEqual(e.exception.message, "protoc is not installed. Please install it before running this task.") From 6250ee68a35b839c422ab834884e5c4ec97ecdc6 Mon Sep 17 00:00:00 2001 From: Nicolas Schweitzer Date: Mon, 3 Feb 2025 13:45:38 +0100 Subject: [PATCH 2/3] add automatic update of the protoc version --- renovate.json | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/renovate.json b/renovate.json index bd95cb1f50961..4a1d607cb4461 100644 --- a/renovate.json +++ b/renovate.json @@ -12,6 +12,16 @@ "depNameTemplate": "buildimages", "versioningTemplate": "loose", "datasourceTemplate": "custom.buildimages" + }, + { + "customType": "regex", + "fileMatch": [".protoc-version"], + "matchStrings": [ + "(?[0-9]+.[0-9]+)" + ], + "depNameTemplate": "protoc", + "versioningTemplate": "loose", + "datasourceTemplate": "custom.protoc" } ], "customDatasources": { @@ -20,6 +30,12 @@ "transformTemplates": [ "{\"releases\": $map(results, function($v) { {\"version\": $v.name, \"releaseTimestamp\": $v.last_updated } }) }" ] + }, + "protoc": { + "defaultRegistryUrlTemplate": "https://api.github.com/repos/protocolbuffers/protobuf/releases", + "transformTemplates": [ + "{\"releases\": $map($.[tag_name,published_at], function($v) { {\"version\": $v[0], \"releaseTimestamp\": $v[1] } }) }" + ] } } } From c40480bf97b1cdd018132bdf9c00ac44cf8962e4 Mon Sep 17 00:00:00 2001 From: Nicolas Schweitzer Date: Tue, 4 Feb 2025 16:15:41 +0100 Subject: [PATCH 3/3] codereview: rename, remove useless comments, improve prints --- tasks/libs/common/check_tools_version.py | 8 ++++---- tasks/protobuf.py | 7 +++---- tasks/unit_tests/libs/common/check_tools_tests.py | 11 +++++------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/tasks/libs/common/check_tools_version.py b/tasks/libs/common/check_tools_version.py index 63ab655eef436..fbab35f25ccda 100644 --- a/tasks/libs/common/check_tools_version.py +++ b/tasks/libs/common/check_tools_version.py @@ -109,8 +109,8 @@ def check_tools_installed(tools: list) -> bool: """ Check if the tools are installed in the system. """ - for tool in tools: - if not shutil.which(tool): - print(f"{tool} is not installed. Please install it before running this task.") - return False + not_installed = [tool for tool in tools if not shutil.which(tool)] + if not_installed: + print(f"{color_message('ERROR', Color.RED)}: The following tools are not installed: {', '.join(not_installed)}") + return False return True diff --git a/tasks/protobuf.py b/tasks/protobuf.py index 93a693a031115..bf1ca6d321728 100644 --- a/tasks/protobuf.py +++ b/tasks/protobuf.py @@ -43,7 +43,7 @@ def generate(ctx, do_mockgen=True): We must build the packages one at a time due to protoc-gen-go limitations """ # Key: path, Value: grpc_gateway, inject_tags - check_dependencies(ctx) + check_tools(ctx) base = os.path.dirname(os.path.abspath(__file__)) repo_root = os.path.abspath(os.path.join(base, "..")) proto_root = os.path.join(repo_root, "pkg", "proto") @@ -151,7 +151,6 @@ def generate(ctx, do_mockgen=True): # Check the generated files were properly committed git_status = ctx.run("git status -suno", hide=True).stdout - # git_status = ctx.run("git status -suno | grep -E 'pkg/proto/pbgo.*.pb(.gw)?.go'").stdout.strip() proto_file = re.compile(r"pkg/proto/pbgo/.*\.pb(\.gw)?\.go$") if any(proto_file.search(line) for line in git_status.split("\n")): raise Exit( @@ -160,7 +159,7 @@ def generate(ctx, do_mockgen=True): ) -def check_dependencies(ctx): +def check_tools(ctx): """ Check if all the required dependencies are installed """ @@ -175,6 +174,6 @@ def check_dependencies(ctx): raise Exit( f"Expected protoc version {expected_version}, found {current_version}. Please run `inv install-protoc` before running this task.", code=1, - ) from None + ) except UnexpectedExit as e: raise Exit("protoc is not installed. Please install it before running this task.", code=1) from e diff --git a/tasks/unit_tests/libs/common/check_tools_tests.py b/tasks/unit_tests/libs/common/check_tools_tests.py index a4e0137ed0633..f390751c2f75c 100644 --- a/tasks/unit_tests/libs/common/check_tools_tests.py +++ b/tasks/unit_tests/libs/common/check_tools_tests.py @@ -4,7 +4,7 @@ from invoke import Context, Exit, MockContext, Result, UnexpectedExit from tasks.libs.common.check_tools_version import check_tools_installed -from tasks.protobuf import check_dependencies +from tasks.protobuf import check_tools class TestCheckToolsInstalled(unittest.TestCase): @@ -18,13 +18,12 @@ def test_tool_not_installed(self): self.assertFalse(check_tools_installed(["not_installed_tool", "ls"])) -class TestCheckDependencies(unittest.TestCase): +class TestCheckTools(unittest.TestCase): @patch('tasks.protobuf.check_tools_installed', new=MagicMock(return_value=False)) def test_tools_not_installed(self): c = Context() with self.assertRaises(Exit) as e: - check_dependencies(c) - print(e) + check_tools(c) self.assertEqual( e.exception.message, "Please install the required tools with `inv install-tools` before running this task." ) @@ -33,7 +32,7 @@ def test_tools_not_installed(self): def test_bad_protoc(self): c = MockContext(run={'protoc --version': Result("libprotoc 1.98.2")}) with self.assertRaises(Exit) as e: - check_dependencies(c) + check_tools(c) self.assertTrue(e.exception.message.startswith("Expected protoc version 29.3, found")) @patch('tasks.protobuf.check_tools_installed', new=MagicMock(return_value=True)) @@ -41,5 +40,5 @@ def test_protoc_not_installed(self): c = MagicMock() c.run.side_effect = UnexpectedExit("protoc --version") with self.assertRaises(Exit) as e: - check_dependencies(c) + check_tools(c) self.assertEqual(e.exception.message, "protoc is not installed. Please install it before running this task.")