Skip to content

Commit 9196723

Browse files
committed
Add automation to enforce releaseChannel annotation on new fields
Adds a lint check that ensures newly added proto fields include a +cue-gen:<APIName>:releaseChannel:extended annotation. Fixes #3147 Signed-off-by: Yukun Cao <caoyukun0430@yeah.net>
1 parent ea30db2 commit 9196723

3 files changed

Lines changed: 286 additions & 0 deletions

File tree

Makefile.core.mk

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ local-lint-protos:
9595
@buf lint
9696
@./scripts/check-operator-proto.sh
9797
@./scripts/check-imports.sh
98+
@./scripts/check-release-channel.sh $(UPDATE_BRANCH)
9899

99100
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
100101
@$(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/"

scripts/check-release-channel.sh

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
#!/usr/bin/env bash
2+
# Copyright Istio Authors
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
# check-release-channel.sh
16+
#
17+
# Ensures that newly added proto fields include a
18+
# +cue-gen:<APIName>:releaseChannel:extended annotation in their comment block.
19+
#
20+
# Per GUIDELINES.md, new fields added to stable v1 APIs must go through the
21+
# "extended" release channel before being promoted to stable. This is indicated
22+
# by a comment annotation above the field definition. Existing stable fields
23+
# intentionally lack this annotation, so we use a diff-based approach: only
24+
# fields added relative to the base branch are checked.
25+
#
26+
# How it works:
27+
# 1. Find proto files changed vs the base branch (excluding common-protos/).
28+
# 2. Parse the git diff to extract added lines with their line numbers.
29+
# 3. Identify lines that are proto field definitions (type name = N;),
30+
# skipping enum values, reserved statements, and options.
31+
# 4. For each new field, walk upward through the contiguous comment block
32+
# immediately above it, looking for the releaseChannel annotation.
33+
# 5. Report any fields missing the annotation and exit non-zero.
34+
#
35+
# Usage:
36+
# ./scripts/check-release-channel.sh <base-branch>
37+
# ./scripts/check-release-channel.sh master
38+
#
39+
# See https://github.com/istio/api/issues/3147
40+
41+
set -euo pipefail
42+
43+
branch="${1:-master}"
44+
errors=0
45+
46+
# Step 1: Find changed/added proto files, excluding third-party common-protos.
47+
changed_protos=$(git diff "${branch}" --name-only --diff-filter=AM -- '*.proto' | grep -v common-protos || true)
48+
if [[ -z "${changed_protos}" ]]; then
49+
exit 0
50+
fi
51+
52+
# check_comment_block: starting from the line above a field definition,
53+
# walk upward through comment lines (// ...) and blank lines. If we find
54+
# a +cue-gen:*:releaseChannel: annotation, return 0 (success). If we hit
55+
# a non-comment, non-blank line (previous field, message boundary, etc.),
56+
# stop and return 1 (not found). This prevents matching annotations that
57+
# belong to a different field.
58+
check_comment_block() {
59+
local file="$1" lineno="$2"
60+
local i=$((lineno - 1))
61+
while [[ ${i} -ge 1 ]]; do
62+
local line
63+
line=$(sed -n "${i}p" "${file}")
64+
# Comment line — check for the annotation
65+
if echo "${line}" | grep -qP '^\s*//'; then
66+
if echo "${line}" | grep -q '+cue-gen:.*:releaseChannel:'; then
67+
return 0
68+
fi
69+
i=$((i - 1))
70+
continue
71+
fi
72+
# Blank line — skip (comments may have a gap before the field)
73+
if echo "${line}" | grep -qP '^\s*$'; then
74+
i=$((i - 1))
75+
continue
76+
fi
77+
# Any other line (field, message, etc.) — stop searching
78+
break
79+
done
80+
return 1
81+
}
82+
83+
for file in ${changed_protos}; do
84+
# Step 2: Parse "git diff -U0" to get added lines with their new-file line numbers.
85+
# -U0 means no context lines, so we only see actual additions.
86+
# The awk script reads @@ hunk headers to get the starting line number,
87+
# then counts up for each "+" line (skipping the "+++ b/file" header).
88+
added_lines=$(git diff "${branch}" -U0 -- "${file}" | awk '
89+
/^@@/ { match($0, /\+([0-9]+)/, a); nr = a[1]; next }
90+
/^\+/ && !/^\+\+\+/ { print nr ":" substr($0, 2); nr++ }
91+
')
92+
[[ -z "${added_lines}" ]] && continue
93+
94+
while IFS= read -r entry; do
95+
[[ -z "${entry}" ]] && continue
96+
lineno="${entry%%:*}"
97+
content="${entry#*:}"
98+
99+
# Step 3: Match proto field definitions.
100+
# Pattern: [repeated|optional] type name = N; or map<K,V> name = N;
101+
# This matches:
102+
# string foo = 1;
103+
# repeated string foo = 2;
104+
# google.protobuf.Duration timeout = 3;
105+
# map<string, string> labels = 4;
106+
# But NOT enum values (single word before =): SOME_VALUE = 0;
107+
echo "${content}" | grep -qP '^\s*((repeated|optional)\s+)?((\w[\w.]*\s+\w+)|(map<.*>\s+\w+))\s*=\s*\d+\s*[;\[]' || continue
108+
# Skip reserved/option/comment-only lines that might match the pattern
109+
echo "${content}" | grep -qP '^\s*(reserved|option|//)\s' && continue
110+
111+
# Step 4: Walk upward through the comment block above this field.
112+
if ! check_comment_block "${file}" "${lineno}"; then
113+
# Step 5: Report the error.
114+
echo "ERROR: ${file}:${lineno}: new field missing releaseChannel annotation"
115+
echo " ${content}"
116+
errors=$((errors + 1))
117+
fi
118+
done <<< "${added_lines}"
119+
done
120+
121+
if [[ ${errors} -gt 0 ]]; then
122+
echo ""
123+
echo "Found ${errors} new field(s) without releaseChannel annotation."
124+
echo "Add: // +cue-gen:<APIName>:releaseChannel:extended"
125+
echo "See GUIDELINES.md for details."
126+
exit 1
127+
fi
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
#!/usr/bin/env bash
2+
# Copyright Istio Authors
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
# Tests for check-release-channel.sh
16+
17+
set -euo pipefail
18+
19+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
20+
CHECK_SCRIPT="${SCRIPT_DIR}/check-release-channel.sh"
21+
TMPDIR=$(mktemp -d)
22+
trap 'rm -rf "${TMPDIR}"' EXIT
23+
24+
pass=0
25+
fail=0
26+
27+
run_test() {
28+
local name="$1" want_exit="$2"
29+
shift 2
30+
if output=$("$@" 2>&1); then
31+
got_exit=0
32+
else
33+
got_exit=$?
34+
fi
35+
if [[ ${got_exit} -eq ${want_exit} ]]; then
36+
echo "PASS: ${name}"
37+
pass=$((pass + 1))
38+
else
39+
echo "FAIL: ${name} (want exit=${want_exit}, got exit=${got_exit})"
40+
echo "${output}"
41+
fail=$((fail + 1))
42+
fi
43+
}
44+
45+
# Set up a temp git repo with a base proto on 'base' branch,
46+
# then switch to a 'work' branch for changes.
47+
setup_repo() {
48+
rm -rf "${TMPDIR}/repo"
49+
mkdir -p "${TMPDIR}/repo/security/v1beta1"
50+
cd "${TMPDIR}/repo"
51+
git init -q
52+
git checkout -q -b base
53+
cat > security/v1beta1/test.proto << 'EOF'
54+
syntax = "proto3";
55+
package istio.security.v1beta1;
56+
57+
message Source {
58+
string principal = 1;
59+
repeated string namespaces = 2;
60+
}
61+
EOF
62+
cp "${CHECK_SCRIPT}" ./check.sh
63+
chmod +x ./check.sh
64+
git add -A && git commit -q -m "base"
65+
git checkout -q -b work
66+
}
67+
68+
# Test 1: no proto changes -> pass
69+
setup_repo
70+
run_test "no changes" 0 ./check.sh base
71+
72+
# Test 2: new field without annotation -> fail
73+
setup_repo
74+
cat >> security/v1beta1/test.proto << 'EOF'
75+
76+
message Rule {
77+
// some comment
78+
string bad_field = 1;
79+
}
80+
EOF
81+
git add -A && git commit -q -m "add bad field"
82+
run_test "field without annotation fails" 1 ./check.sh base
83+
84+
# Test 3: new field with annotation -> pass
85+
setup_repo
86+
cat >> security/v1beta1/test.proto << 'EOF'
87+
88+
message Rule {
89+
// +cue-gen:Rule:releaseChannel:extended
90+
string good_field = 1;
91+
}
92+
EOF
93+
git add -A && git commit -q -m "add good field"
94+
run_test "field with annotation passes" 0 ./check.sh base
95+
96+
# Test 4: repeated field without annotation -> fail
97+
setup_repo
98+
cat >> security/v1beta1/test.proto << 'EOF'
99+
100+
message Rule {
101+
repeated string bad_repeated = 1;
102+
}
103+
EOF
104+
git add -A && git commit -q -m "add bad repeated"
105+
run_test "repeated field without annotation fails" 1 ./check.sh base
106+
107+
# Test 5: map field without annotation -> fail
108+
setup_repo
109+
cat >> security/v1beta1/test.proto << 'EOF'
110+
111+
message Rule {
112+
map<string, string> bad_map = 1;
113+
}
114+
EOF
115+
git add -A && git commit -q -m "add bad map"
116+
run_test "map field without annotation fails" 1 ./check.sh base
117+
118+
# Test 6: enum value (not a field) without annotation -> pass
119+
setup_repo
120+
cat >> security/v1beta1/test.proto << 'EOF'
121+
122+
enum Mode {
123+
DEFAULT = 0;
124+
STRICT = 1;
125+
}
126+
EOF
127+
git add -A && git commit -q -m "add enum"
128+
run_test "enum values ignored" 0 ./check.sh base
129+
130+
# Test 7: mixed good and bad fields -> fail
131+
setup_repo
132+
cat >> security/v1beta1/test.proto << 'EOF'
133+
134+
message Rule {
135+
// +cue-gen:Rule:releaseChannel:extended
136+
string good = 1;
137+
138+
string bad = 2;
139+
}
140+
EOF
141+
git add -A && git commit -q -m "add mixed"
142+
run_test "mixed fields catches bad only" 1 ./check.sh base
143+
144+
# Test 8: common-protos changes are ignored
145+
setup_repo
146+
mkdir -p common-protos/google
147+
cat > common-protos/google/test.proto << 'EOF'
148+
syntax = "proto3";
149+
message Foo {
150+
string no_annotation = 1;
151+
}
152+
EOF
153+
git add -A && git commit -q -m "add common-proto"
154+
run_test "common-protos ignored" 0 ./check.sh base
155+
156+
echo ""
157+
echo "Results: ${pass} passed, ${fail} failed"
158+
[[ ${fail} -eq 0 ]]

0 commit comments

Comments
 (0)