From bcc4746cc8fd92bee5cbfb756fcc818c9d88322c Mon Sep 17 00:00:00 2001 From: Gabriel Cao <13051583793@yeah.net> Date: Mon, 30 Mar 2026 18:10:32 +0200 Subject: [PATCH] Add automation to enforce releaseChannel annotation on new fields Adds a lint check that ensures newly added proto fields include a +cue-gen::releaseChannel:extended annotation. Fixes #3147 Signed-off-by: Gabriel Cao <13051583793@yeah.net> --- Makefile.core.mk | 5 +- scripts/check-release-channel.sh | 127 +++++++++++++++++++++ scripts/check-release-channel_test.sh | 158 ++++++++++++++++++++++++++ 3 files changed, 289 insertions(+), 1 deletion(-) create mode 100755 scripts/check-release-channel.sh create mode 100755 scripts/check-release-channel_test.sh diff --git a/Makefile.core.mk b/Makefile.core.mk index 5d6b0c9e0f..8e64a57740 100644 --- a/Makefile.core.mk +++ b/Makefile.core.mk @@ -99,7 +99,10 @@ local-lint-protos: lint: lint-dockerfiles lint-scripts lint-yaml lint-helm lint-copyright-banner lint-go lint-python lint-markdown lint-sass lint-typescript lint-licenses local-lint-protos @$(htmlproofer) . --url-swap "istio.io:preliminary.istio.io" --assume-extension --check-html --check-external-hash --check-opengraph --timeframe 2d --storage-dir $(repo_dir)/.htmlproofer --url-ignore "/localhost/" -test: breaking +check-release-channel: + @./scripts/check-release-channel.sh $(UPDATE_BRANCH) + +test: breaking check-release-channel (pushd tests && go test -v ./...) fmt: format-python diff --git a/scripts/check-release-channel.sh b/scripts/check-release-channel.sh new file mode 100755 index 0000000000..2e9fb0ce6e --- /dev/null +++ b/scripts/check-release-channel.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash +# Copyright Istio Authors +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# check-release-channel.sh +# +# Ensures that newly added proto fields include a +# +cue-gen::releaseChannel:extended annotation in their comment block. +# +# Per GUIDELINES.md, new fields added to stable v1 APIs must go through the +# "extended" release channel before being promoted to stable. This is indicated +# by a comment annotation above the field definition. Existing stable fields +# intentionally lack this annotation, so we use a diff-based approach: only +# fields added relative to the base branch are checked. +# +# How it works: +# 1. Find proto files changed vs the base branch (excluding common-protos/). +# 2. Parse the git diff to extract added lines with their line numbers. +# 3. Identify lines that are proto field definitions (type name = N;), +# skipping enum values, reserved statements, and options. +# 4. For each new field, walk upward through the contiguous comment block +# immediately above it, looking for the releaseChannel annotation. +# 5. Report any fields missing the annotation and exit non-zero. +# +# Usage: +# ./scripts/check-release-channel.sh +# ./scripts/check-release-channel.sh master +# +# See https://github.com/istio/api/issues/3147 + +set -euo pipefail + +branch="${1:-master}" +errors=0 + +# Step 1: Find changed/added proto files, excluding third-party common-protos. +changed_protos=$(git diff "${branch}" --name-only --diff-filter=AM -- '*.proto' | grep -v common-protos || true) +if [[ -z "${changed_protos}" ]]; then + exit 0 +fi + +# check_comment_block: starting from the line above a field definition, +# walk upward through comment lines (// ...) and blank lines. If we find +# a +cue-gen:*:releaseChannel: annotation, return 0 (success). If we hit +# a non-comment, non-blank line (previous field, message boundary, etc.), +# stop and return 1 (not found). This prevents matching annotations that +# belong to a different field. +check_comment_block() { + local file="$1" lineno="$2" + local i=$((lineno - 1)) + while [[ ${i} -ge 1 ]]; do + local line + line=$(sed -n "${i}p" "${file}") + # Comment line — check for the annotation + if echo "${line}" | grep -qP '^\s*//'; then + if echo "${line}" | grep -q '+cue-gen:.*:releaseChannel:'; then + return 0 + fi + i=$((i - 1)) + continue + fi + # Blank line — skip (comments may have a gap before the field) + if echo "${line}" | grep -qP '^\s*$'; then + i=$((i - 1)) + continue + fi + # Any other line (field, message, etc.) — stop searching + break + done + return 1 +} + +for file in ${changed_protos}; do + # Step 2: Parse "git diff -U0" to get added lines with their new-file line numbers. + # -U0 means no context lines, so we only see actual additions. + # The awk script reads @@ hunk headers to get the starting line number, + # then counts up for each "+" line (skipping the "+++ b/file" header). + added_lines=$(git diff "${branch}" -U0 -- "${file}" | awk ' + /^@@/ { match($0, /\+([0-9]+)/, a); nr = a[1]; next } + /^\+/ && !/^\+\+\+/ { print nr ":" substr($0, 2); nr++ } + ') + [[ -z "${added_lines}" ]] && continue + + while IFS= read -r entry; do + [[ -z "${entry}" ]] && continue + lineno="${entry%%:*}" + content="${entry#*:}" + + # Step 3: Match proto field definitions. + # Pattern: [repeated|optional] type name = N; or map name = N; + # This matches: + # string foo = 1; + # repeated string foo = 2; + # google.protobuf.Duration timeout = 3; + # map labels = 4; + # But NOT enum values (single word before =): SOME_VALUE = 0; + echo "${content}" | grep -qP '^\s*((repeated|optional)\s+)?((\w[\w.]*\s+\w+)|(map<.*>\s+\w+))\s*=\s*\d+\s*[;\[]' || continue + # Skip reserved/option/comment-only lines that might match the pattern + echo "${content}" | grep -qP '^\s*(reserved|option|//)\s' && continue + + # Step 4: Walk upward through the comment block above this field. + if ! check_comment_block "${file}" "${lineno}"; then + # Step 5: Report the error. + echo "ERROR: ${file}:${lineno}: new field missing releaseChannel annotation" + echo " ${content}" + errors=$((errors + 1)) + fi + done <<< "${added_lines}" +done + +if [[ ${errors} -gt 0 ]]; then + echo "" + echo "Found ${errors} new field(s) without releaseChannel annotation." + echo "Add: // +cue-gen::releaseChannel:extended" + echo "See GUIDELINES.md for details." + exit 1 +fi diff --git a/scripts/check-release-channel_test.sh b/scripts/check-release-channel_test.sh new file mode 100755 index 0000000000..8550fa72e3 --- /dev/null +++ b/scripts/check-release-channel_test.sh @@ -0,0 +1,158 @@ +#!/usr/bin/env bash +# Copyright Istio Authors +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Tests for check-release-channel.sh + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +CHECK_SCRIPT="${SCRIPT_DIR}/check-release-channel.sh" +TMPDIR=$(mktemp -d) +trap 'rm -rf "${TMPDIR}"' EXIT + +pass=0 +fail=0 + +run_test() { + local name="$1" want_exit="$2" + shift 2 + if output=$("$@" 2>&1); then + got_exit=0 + else + got_exit=$? + fi + if [[ ${got_exit} -eq ${want_exit} ]]; then + echo "PASS: ${name}" + pass=$((pass + 1)) + else + echo "FAIL: ${name} (want exit=${want_exit}, got exit=${got_exit})" + echo "${output}" + fail=$((fail + 1)) + fi +} + +# Set up a temp git repo with a base proto on 'base' branch, +# then switch to a 'work' branch for changes. +setup_repo() { + rm -rf "${TMPDIR}/repo" + mkdir -p "${TMPDIR}/repo/security/v1beta1" + cd "${TMPDIR}/repo" + git init -q + git checkout -q -b base + cat > security/v1beta1/test.proto << 'EOF' +syntax = "proto3"; +package istio.security.v1beta1; + +message Source { + string principal = 1; + repeated string namespaces = 2; +} +EOF + cp "${CHECK_SCRIPT}" ./check.sh + chmod +x ./check.sh + git add -A && git commit -q -m "base" + git checkout -q -b work +} + +# Test 1: no proto changes -> pass +setup_repo +run_test "no changes" 0 ./check.sh base + +# Test 2: new field without annotation -> fail +setup_repo +cat >> security/v1beta1/test.proto << 'EOF' + +message Rule { + // some comment + string bad_field = 1; +} +EOF +git add -A && git commit -q -m "add bad field" +run_test "field without annotation fails" 1 ./check.sh base + +# Test 3: new field with annotation -> pass +setup_repo +cat >> security/v1beta1/test.proto << 'EOF' + +message Rule { + // +cue-gen:Rule:releaseChannel:extended + string good_field = 1; +} +EOF +git add -A && git commit -q -m "add good field" +run_test "field with annotation passes" 0 ./check.sh base + +# Test 4: repeated field without annotation -> fail +setup_repo +cat >> security/v1beta1/test.proto << 'EOF' + +message Rule { + repeated string bad_repeated = 1; +} +EOF +git add -A && git commit -q -m "add bad repeated" +run_test "repeated field without annotation fails" 1 ./check.sh base + +# Test 5: map field without annotation -> fail +setup_repo +cat >> security/v1beta1/test.proto << 'EOF' + +message Rule { + map bad_map = 1; +} +EOF +git add -A && git commit -q -m "add bad map" +run_test "map field without annotation fails" 1 ./check.sh base + +# Test 6: enum value (not a field) without annotation -> pass +setup_repo +cat >> security/v1beta1/test.proto << 'EOF' + +enum Mode { + DEFAULT = 0; + STRICT = 1; +} +EOF +git add -A && git commit -q -m "add enum" +run_test "enum values ignored" 0 ./check.sh base + +# Test 7: mixed good and bad fields -> fail +setup_repo +cat >> security/v1beta1/test.proto << 'EOF' + +message Rule { + // +cue-gen:Rule:releaseChannel:extended + string good = 1; + + string bad = 2; +} +EOF +git add -A && git commit -q -m "add mixed" +run_test "mixed fields catches bad only" 1 ./check.sh base + +# Test 8: common-protos changes are ignored +setup_repo +mkdir -p common-protos/google +cat > common-protos/google/test.proto << 'EOF' +syntax = "proto3"; +message Foo { + string no_annotation = 1; +} +EOF +git add -A && git commit -q -m "add common-proto" +run_test "common-protos ignored" 0 ./check.sh base + +echo "" +echo "Results: ${pass} passed, ${fail} failed" +[[ ${fail} -eq 0 ]]