scripts: pin the opentelemetry-proto revision used by codegen

README.md

@@ -40,9 +40,9 @@ pip install . ./tests/snowflake-telemetry-test-utils
 
 To regenerate the code under `src/snowflake/_internal/opentelemetry/proto/`, execute the script `./scripts/proto_codegen.sh`. The script expects the `src/snowflake/_internal/opentelemetry/proto/` directory to exist, and will delete all .py files in it before regerating the code.
 
-The commit/branch/tag of [opentelemetry-proto](https://github.com/open-telemetry/opentelemetry-proto) that the code is generated from is pinned to PROTO_REPO_BRANCH_OR_COMMIT, which can be configured in the script. It is currently pinned to the same tag as [opentelemetry-python](https://github.com/open-telemetry/opentelemetry-python/blob/main/scripts/proto_codegen.sh#L15).
+The [opentelemetry-proto](https://github.com/open-telemetry/opentelemetry-proto) source used for code generation is pinned to the immutable commit in `PROTO_REPO_COMMIT`. The current commit corresponds to the `v1.7.0` release used by [opentelemetry-python](https://github.com/open-telemetry/opentelemetry-python/blob/main/scripts/proto_codegen.sh#L15). When updating the proto version, update the full commit hash rather than using a movable branch or tag.
 
 
 ### Release
 
-Release to pypi is done via [Upload Python Package](https://github.com/snowflakedb/snowflake-telemetry-python/actions/workflows/python-publish.yml) workflow and it is triggered whenever a new [release](https://github.com/snowflakedb/snowflake-telemetry-python/releases) is created. Before creating a release, the PR to update versions ([example](https://github.com/snowflakedb/snowflake-telemetry-python/pull/54)) should be merged to main branch first.
+Release to pypi is done via [Upload Python Package](https://github.com/snowflakedb/snowflake-telemetry-python/actions/workflows/python-publish.yml) workflow and it is triggered whenever a new [release](https://github.com/snowflakedb/snowflake-telemetry-python/releases) is created. Before creating a release, the PR to update versions ([example](https://github.com/snowflakedb/snowflake-telemetry-python/pull/54)) should be merged to main branch first.

scripts/codegen_lib.sh

@@ -0,0 +1,63 @@
+#!/bin/bash
+#
+# Shared helpers for the proto code-generation scripts.
+#
+# This lives in its own file, and intentionally has NO top-level side effects,
+# so the protoc invocation can be exercised directly by unit tests (see
+# tests/test_proto_codegen_revision_pin.py) without having to run the full
+# clone / virtualenv / pip pipeline that proto_codegen.sh performs.
+
+# checkout_proto_commit <proto_repo_dir> <expected_commit>
+#
+# Fetch the configured remotes, check out the pinned upstream commit, and
+# fail unless HEAD resolves to that exact commit. Only full commit hashes are
+# accepted, so the pinned revision cannot change over time.
+checkout_proto_commit() {
+    local proto_repo_dir="$1"
+    local expected_commit="$2"
+
+    if [[ ! "$expected_commit" =~ ^[0-9a-f]{40}$ ]]; then
+        echo "OpenTelemetry proto revision must be a full commit hash" >&2
+        return 1
+    fi
+
+    git -C "$proto_repo_dir" fetch --all
+    git -C "$proto_repo_dir" checkout --detach "$expected_commit"
+
+    local actual_commit
+    actual_commit="$(git -C "$proto_repo_dir" rev-parse HEAD)"
+    if [[ "$actual_commit" != "$expected_commit" ]]; then
+        echo "Expected OpenTelemetry proto commit $expected_commit, got $actual_commit" >&2
+        return 1
+    fi
+}
+
+# generate_marshaler_code <proto_repo_dir> <repo_root>
+#
+# Discover every *.proto file under <proto_repo_dir> and run the custom
+# marshaler codegen plugin over them.
+#
+# The discovered paths are collected NUL-delimited into a bash array and then
+# expanded *quoted* ("${all_protos[@]}"), so each filename is passed to protoc
+# as exactly one argv element, regardless of whitespace or special characters
+# in the name.
+generate_marshaler_code() {
+    local proto_repo_dir="$1"
+    local repo_root="$2"
+
+    # Allow tests to substitute the protoc invocation. This is an internal
+    # value, not derived from any discovered filename.
+    local protoc_cmd="${PROTOC:-python -m grpc_tools.protoc}"
+
+    local all_protos=()
+    local proto_file
+    while IFS= read -r -d '' proto_file; do
+        all_protos+=("$proto_file")
+    done < <(find "$proto_repo_dir/" -iname "*.proto" -print0)
+
+    OPENTELEMETRY_PROTO_DIR="$proto_repo_dir" $protoc_cmd \
+        -I "$proto_repo_dir" \
+        --plugin=protoc-gen-custom-plugin="$repo_root/scripts/plugin.py" \
+        --custom-plugin_out=. \
+        "${all_protos[@]}"
+}

scripts/proto_codegen.sh

@@ -3,16 +3,16 @@
 # Regenerate python code from OTLP protos in
 # https://github.com/open-telemetry/opentelemetry-proto
 #
-# To use, update PROTO_REPO_BRANCH_OR_COMMIT variable below to a commit hash or
-# tag in opentelemtry-proto repo that you want to build off of. Then, just run
+# To use, update PROTO_REPO_COMMIT below to the full commit hash in the
+# opentelemetry-proto repository that you want to build from. Then, just run
 # this script to update the proto files. Commit the changes as well as any
 # fixes needed in the OTLP exporter.
 #
 # Optional envars:
 #   PROTO_REPO_DIR - the path to an existing checkout of the opentelemetry-proto repo
 
-# Pinned commit/branch/tag for the current version used in opentelemetry-proto python package.
-PROTO_REPO_BRANCH_OR_COMMIT="v1.7.0"
+# Immutable commit for opentelemetry-proto v1.7.0.
+PROTO_REPO_COMMIT="8654ab7a5a43ca25fe8046e59dcd6935c3f76de0"
 
 set -e
 
@@ -21,6 +21,10 @@ PROTO_REPO_DIR=${PROTO_REPO_DIR:-"/tmp/opentelemetry-proto"}
 repo_root="$(git rev-parse --show-toplevel)"
 venv_dir="/tmp/proto_codegen_venv"
 
+# Shared codegen helpers (defines generate_marshaler_code). Sourcing has no
+# side effects; it only defines functions.
+source "$repo_root/scripts/codegen_lib.sh"
+
 # run on exit even if crash
 cleanup() {
     echo "Deleting $venv_dir"
@@ -43,14 +47,8 @@ if [ ! -d "$PROTO_REPO_DIR" ]; then
     git clone https://github.com/open-telemetry/opentelemetry-proto.git $PROTO_REPO_DIR
 fi
 
-# Pull in changes and switch to requested branch
-(
-    cd $PROTO_REPO_DIR
-    git fetch --all
-    git checkout $PROTO_REPO_BRANCH_OR_COMMIT
-    # pull if PROTO_REPO_BRANCH_OR_COMMIT is not a detached head
-    git symbolic-ref -q HEAD && git pull --ff-only || true
-)
+# Fetch and check out the pinned upstream revision.
+checkout_proto_commit "$PROTO_REPO_DIR" "$PROTO_REPO_COMMIT"
 
 cd $repo_root/src/snowflake/telemetry/_internal
 
@@ -59,9 +57,7 @@ mkdir -p opentelemetry/proto
 find opentelemetry/proto/ -regex ".*_marshaler\.py" -exec rm {} +
 
 # generate proto code for all protos
-all_protos=$(find $PROTO_REPO_DIR/ -iname "*.proto")
-OPENTELEMETRY_PROTO_DIR=$PROTO_REPO_DIR python -m grpc_tools.protoc \
-    -I $PROTO_REPO_DIR \
-    --plugin=protoc-gen-custom-plugin=$repo_root/scripts/plugin.py \
-    --custom-plugin_out=. \
-    $all_protos
+# Proto paths are discovered and passed to protoc by generate_marshaler_code,
+# which collects them into a quoted bash array so each filename is passed as a
+# single argument. See scripts/codegen_lib.sh.
+generate_marshaler_code "$PROTO_REPO_DIR" "$repo_root"

tests/test_proto_codegen_revision_pin.py

@@ -0,0 +1,172 @@
+import os
+import subprocess
+import tempfile
+import unittest
+
+REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir))
+CODEGEN_LIB = os.path.join(REPO_ROOT, "scripts", "codegen_lib.sh")
+CODEGEN_SCRIPT = os.path.join(REPO_ROOT, "scripts", "proto_codegen.sh")
+EXPECTED_V1_7_0_COMMIT = "8654ab7a5a43ca25fe8046e59dcd6935c3f76de0"
+
+
+class TestProtoCodegenRevisionPin(unittest.TestCase):
+    def _git(self, repo, *args, check=True):
+        return subprocess.run(
+            ["git", "-C", repo, *args],
+            check=check,
+            capture_output=True,
+            text=True,
+        )
+
+    def _checkout_proto_commit(self, repo, commit):
+        return subprocess.run(
+            [
+                "bash",
+                "-c",
+                'source "$1" && checkout_proto_commit "$2" "$3"',
+                "checkout-proto-commit",
+                CODEGEN_LIB,
+                repo,
+                commit,
+            ],
+            capture_output=True,
+            text=True,
+        )
+
+    def test_codegen_script_pins_v1_7_0_to_immutable_commit(self):
+        with open(CODEGEN_SCRIPT) as script_file:
+            script = script_file.read()
+
+        self.assertIn(
+            'PROTO_REPO_COMMIT="%s"' % EXPECTED_V1_7_0_COMMIT,
+            script,
+        )
+        self.assertIn(
+            'checkout_proto_commit "$PROTO_REPO_DIR" "$PROTO_REPO_COMMIT"',
+            script,
+        )
+
+    def test_checkout_uses_pinned_commit_after_tag_moves(self):
+        with tempfile.TemporaryDirectory() as repo:
+            self._git(repo, "init")
+            self._git(repo, "config", "user.email", "test@example.com")
+            self._git(repo, "config", "user.name", "Test User")
+
+            tracked_file = os.path.join(repo, "revision.txt")
+            with open(tracked_file, "w") as revision_file:
+                revision_file.write("trusted\n")
+            self._git(repo, "add", "revision.txt")
+            self._git(repo, "commit", "-m", "trusted revision")
+            trusted_commit = self._git(repo, "rev-parse", "HEAD").stdout.strip()
+            self._git(repo, "tag", "v1.7.0")
+
+            with open(tracked_file, "w") as revision_file:
+                revision_file.write("moved tag\n")
+            self._git(repo, "commit", "-am", "move tag target")
+            moved_commit = self._git(repo, "rev-parse", "HEAD").stdout.strip()
+            self._git(repo, "tag", "-f", "v1.7.0")
+            self.assertNotEqual(trusted_commit, moved_commit)
+
+            result = self._checkout_proto_commit(repo, trusted_commit)
+            self.assertEqual(result.returncode, 0, result.stderr)
+            self.assertEqual(
+                self._git(repo, "rev-parse", "HEAD").stdout.strip(),
+                trusted_commit,
+            )
+            self.assertNotEqual(
+                self._git(repo, "rev-parse", "v1.7.0").stdout.strip(),
+                trusted_commit,
+            )
+            self.assertNotEqual(
+                self._git(repo, "symbolic-ref", "-q", "HEAD", check=False).returncode,
+                0,
+            )
+
+    def test_checkout_fails_closed_for_unknown_commit(self):
+        with tempfile.TemporaryDirectory() as repo:
+            self._git(repo, "init")
+            result = self._checkout_proto_commit(repo, "0" * 40)
+            self.assertNotEqual(result.returncode, 0)
+
+    def test_checkout_rejects_movable_tag(self):
+        with tempfile.TemporaryDirectory() as repo:
+            self._git(repo, "init")
+            result = self._checkout_proto_commit(repo, "v1.7.0")
+            self.assertNotEqual(result.returncode, 0)
+            self.assertIn("full commit hash", result.stderr)
+
+
+class TestGenerateMarshalerCode(unittest.TestCase):
+    def _generate_argv(self, proto_dir, repo_root):
+        # Run generate_marshaler_code with a stubbed protoc and return the
+        # argv it received (one element per line).
+        with tempfile.TemporaryDirectory() as work:
+            argv_file = os.path.join(work, "argv")
+            stub = os.path.join(work, "protoc-stub")
+            with open(stub, "w") as stub_file:
+                stub_file.write('#!/bin/bash\nprintf \'%s\\n\' "$@" > "$ARGV_CAPTURE"\n')
+            os.chmod(stub, 0o755)
+            env = dict(os.environ)
+            env["PROTOC"] = stub
+            env["ARGV_CAPTURE"] = argv_file
+            result = subprocess.run(
+                [
+                    "bash",
+                    "-c",
+                    'source "$1" && generate_marshaler_code "$2" "$3"',
+                    "generate-marshaler-code",
+                    CODEGEN_LIB,
+                    proto_dir,
+                    repo_root,
+                ],
+                capture_output=True,
+                text=True,
+                env=env,
+            )
+            self.assertEqual(result.returncode, 0, result.stderr)
+            with open(argv_file) as captured:
+                return captured.read().splitlines()
+
+    def test_filenames_with_spaces_pass_as_single_argument(self):
+        with tempfile.TemporaryDirectory() as proto_dir, tempfile.TemporaryDirectory() as repo_root:
+            spaced_dir = os.path.join(proto_dir, "dir with spaces")
+            os.makedirs(spaced_dir)
+            proto_file = os.path.join(spaced_dir, "file.proto")
+            open(proto_file, "w").close()
+
+            argv = self._generate_argv(proto_dir, repo_root)
+
+            self.assertEqual(argv[0], "-I")
+            self.assertEqual(argv[1], proto_dir)
+            self.assertEqual(
+                argv[2],
+                "--plugin=protoc-gen-custom-plugin=%s"
+                % os.path.join(repo_root, "scripts", "plugin.py"),
+            )
+            self.assertEqual(argv[3], "--custom-plugin_out=.")
+            self.assertEqual(argv[4:], [proto_file])
+
+    def test_no_extra_protoc_options_from_filenames(self):
+        with tempfile.TemporaryDirectory() as proto_dir, tempfile.TemporaryDirectory() as repo_root:
+            option_like_dir = os.path.join(
+                proto_dir, "x --plugin=protoc-gen-custom-plugin=other"
+            )
+            os.makedirs(option_like_dir)
+            proto_file = os.path.join(option_like_dir, "y.proto")
+            open(proto_file, "w").close()
+
+            argv = self._generate_argv(proto_dir, repo_root)
+
+            plugin_options = [arg for arg in argv if arg.startswith("--plugin=")]
+            self.assertEqual(
+                plugin_options,
+                [
+                    "--plugin=protoc-gen-custom-plugin=%s"
+                    % os.path.join(repo_root, "scripts", "plugin.py")
+                ],
+            )
+            self.assertIn(proto_file, argv)
+
+
+if __name__ == "__main__":
+    unittest.main()