Skip to content

Commit 7f0ccc6

Browse files
JoeWang1127cloud-java-bot
JoeWang1127
and
cloud-java-bot
authored
chore: always fully generate the repo if it is a non-monorepo or contains common protos (#3162)
In this PR: - Always perform a full generation in a non-monorepo or the repo contains common protos. - Secure generation workflow (use environment variable to avoid script injection), inspired by googleapis/java-bigtable#2317 This PR also brings changes in common protos and iam due to protoc updates (25.3 -> 25.4). --------- Co-authored-by: cloud-java-bot <[email protected]>
1 parent 12003a0 commit 7f0ccc6

7 files changed

+173
-18
lines changed

‎.github/scripts/hermetic_library_generation.sh

+9-10
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,14 @@ fi
6464

6565
image_tag=local
6666
workspace_name="/workspace"
67-
docker_file="library_generation.Dockerfile"
67+
baseline_generation_config="baseline_generation_config.yaml"
6868
message="chore: generate libraries at $(date)"
6969

70+
git checkout "${target_branch}"
7071
git checkout "${current_branch}"
71-
# if the last commit doesn't contain changes to generation configuration
72-
# or Dockerfile, do not generate again as the result will be the same.
73-
change_of_last_commit="$(git diff-tree --no-commit-id --name-only HEAD~1..HEAD -r)"
74-
if [[ ! ("${change_of_last_commit}" == *"${generation_config}"* || "${change_of_last_commit}" == *"${docker_file}"*) ]]; then
75-
echo "The last commit doesn't contain any changes to the generation_config.yaml or Dockerfile, skipping the whole generation process." || true
76-
exit 0
77-
fi
72+
73+
# copy generation configuration from target branch to current branch.
74+
git show "${target_branch}":"${generation_config}" > "${baseline_generation_config}"
7875

7976
generator_version=$(mvn help:evaluate -Dexpression=project.version -q -DforceStdout -pl gapic-generator-java)
8077
echo "Local generator version: ${generator_version}"
@@ -94,10 +91,12 @@ docker run \
9491
-v "$(pwd):${workspace_name}" \
9592
-v "$HOME"/.m2:/home/.m2 \
9693
-e GENERATOR_VERSION="${generator_version}" \
97-
gcr.io/cloud-devrel-public-resources/java-library-generation:"${image_tag}"
94+
gcr.io/cloud-devrel-public-resources/java-library-generation:"${image_tag}" \
95+
--baseline-generation-config-path="${workspace_name}/${baseline_generation_config}" \
96+
--current-generation-config-path="${workspace_name}/${generation_config}"
9897

9998
# commit the change to the pull request.
100-
rm -rdf output googleapis
99+
rm -rdf output googleapis "${baseline_generation_config}"
101100
git add --all -- ':!pr_description.txt'
102101
changed_files=$(git diff --cached --name-only)
103102
if [[ "${changed_files}" == "" ]]; then

‎.github/workflows/hermetic_library_generation.yaml

+7-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ name: Hermetic library generation upon generation config change through pull req
1717
on:
1818
pull_request:
1919

20+
env:
21+
REPO_FULL_NAME: ${{ github.event.pull_request.head.repo.full_name }}
22+
GITHUB_REPOSITORY: ${{ github.repository }}
2023
jobs:
2124
library_generation:
22-
# skip pull requests come from a forked repository
23-
if: github.event.pull_request.head.repo.full_name == github.repository
2425
runs-on: ubuntu-latest
2526
steps:
2627
- uses: actions/checkout@v4
@@ -31,6 +32,10 @@ jobs:
3132
shell: bash
3233
run: |
3334
set -x
35+
if [[ "${GITHUB_REPOSITORY}" != "${REPO_FULL_NAME}" ]]; then
36+
echo "This PR comes from a fork. Skip library generation."
37+
exit 0
38+
fi
3439
[ -z "$(git config user.email)" ] && git config --global user.email "[email protected]"
3540
[ -z "$(git config user.name)" ] && git config --global user.name "cloud-java-bot"
3641
bash .github/scripts/hermetic_library_generation.sh \

‎library_generation/cli/entry_point.py

+12-3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import click as click
1818
from library_generation.generate_pr_description import generate_pr_descriptions
1919
from library_generation.generate_repo import generate_from_yaml
20+
from library_generation.model.config_change import ConfigChange
2021
from library_generation.model.generation_config import from_yaml
2122
from library_generation.utils.generation_config_comparator import compare_config
2223

@@ -145,11 +146,10 @@ def __generate_repo_and_pr_description_impl(
145146
baseline_config=from_yaml(baseline_generation_config_path),
146147
current_config=from_yaml(current_generation_config_path),
147148
)
148-
# pass None if this is not a monorepo in order to trigger the full
149-
# generation
149+
# pass None if we want to fully generate the repository.
150150
target_library_names = (
151151
config_change.get_changed_libraries()
152-
if config_change.current_config.is_monorepo()
152+
if not _needs_full_repo_generation(config_change=config_change)
153153
else None
154154
)
155155
generate_from_yaml(
@@ -163,6 +163,15 @@ def __generate_repo_and_pr_description_impl(
163163
)
164164

165165

166+
def _needs_full_repo_generation(config_change: ConfigChange) -> bool:
167+
"""
168+
Whether you should need a full repo generation, i.e., generate all
169+
libraries in the generation configuration.
170+
"""
171+
current_config = config_change.current_config
172+
return not current_config.is_monorepo() or current_config.contains_common_protos()
173+
174+
166175
@main.command()
167176
@click.option(
168177
"--generation-config-path",

‎library_generation/test/cli/entry_point_unit_tests.py

+53
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,56 @@ def test_generate_non_monorepo_with_changes_triggers_full_generation(
138138
generate_from_yaml.assert_called_with(
139139
config=ANY, repository_path=ANY, target_library_names=None
140140
)
141+
142+
@patch("library_generation.cli.entry_point.generate_from_yaml")
143+
@patch("library_generation.cli.entry_point.generate_pr_descriptions")
144+
def test_generate_monorepo_with_common_protos_triggers_full_generation(
145+
self,
146+
generate_pr_descriptions,
147+
generate_from_yaml,
148+
):
149+
"""
150+
this tests confirms the behavior of generation of a monorepo with
151+
common protos.
152+
generate() should call generate_from_yaml() with
153+
target_library_names=None in order to trigger the full generation
154+
"""
155+
config_path = f"{test_resource_dir}/monorepo_with_common_protos.yaml"
156+
self.assertTrue(from_yaml(config_path).is_monorepo())
157+
# we call the implementation method directly since click
158+
# does special handling when a method is annotated with @main.command()
159+
generate_impl(
160+
baseline_generation_config_path=config_path,
161+
current_generation_config_path=config_path,
162+
repository_path=".",
163+
)
164+
generate_from_yaml.assert_called_with(
165+
config=ANY, repository_path=ANY, target_library_names=None
166+
)
167+
168+
@patch("library_generation.cli.entry_point.generate_from_yaml")
169+
@patch("library_generation.cli.entry_point.generate_pr_descriptions")
170+
def test_generate_monorepo_without_common_protos_does_not_trigger_full_generation(
171+
self,
172+
generate_pr_descriptions,
173+
generate_from_yaml,
174+
):
175+
"""
176+
this tests confirms the behavior of generation of a monorepo without
177+
common protos.
178+
generate() should call generate_from_yaml() with
179+
target_library_names=changed libraries which does not trigger the full
180+
generation.
181+
"""
182+
config_path = f"{test_resource_dir}/monorepo_without_common_protos.yaml"
183+
self.assertTrue(from_yaml(config_path).is_monorepo())
184+
# we call the implementation method directly since click
185+
# does special handling when a method is annotated with @main.command()
186+
generate_impl(
187+
baseline_generation_config_path=config_path,
188+
current_generation_config_path=config_path,
189+
repository_path=".",
190+
)
191+
generate_from_yaml.assert_called_with(
192+
config=ANY, repository_path=ANY, target_library_names=[]
193+
)

‎library_generation/test/generate_library_unit_tests.py

+16-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
# Copyright 2024 Google LLC
2+
#
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+
# https://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.
114
import subprocess
215
import unittest
316
import os
@@ -66,13 +79,13 @@ def _run_command_and_get_sdout(self, command, **kwargs):
6679
def test_get_grpc_version_with_no_env_var_fails(self):
6780
# the absence of DOCKER_GRPC_VERSION will make this function to fail
6881
result = self._run_command("get_grpc_version")
69-
self.assertEquals(1, result.returncode)
82+
self.assertEqual(1, result.returncode)
7083
self.assertRegex(result.stdout.decode(), "DOCKER_GRPC_VERSION is not set")
7184

7285
def test_get_protoc_version_with_no_env_var_fails(self):
7386
# the absence of DOCKER_PROTOC_VERSION will make this function to fail
7487
result = self._run_command("get_protoc_version")
75-
self.assertEquals(1, result.returncode)
88+
self.assertEqual(1, result.returncode)
7689
self.assertRegex(result.stdout.decode(), "DOCKER_PROTOC_VERSION is not set")
7790

7891
def test_download_tools_without_baked_generator_fails(self):
@@ -92,5 +105,5 @@ def test_download_tools_without_baked_generator_fails(self):
92105
result = self._run_command(
93106
f"download_tools {test_protoc_version} {test_grpc_version} {self.TEST_ARCHITECTURE}"
94107
)
95-
self.assertEquals(1, result.returncode)
108+
self.assertEqual(1, result.returncode)
96109
self.assertRegex(result.stdout.decode(), "Please configure your environment")

‎library_generation/test/resources/test-config/monorepo_with_common_protos.yaml

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
googleapis_commitish: 6a474b31c53cc1797710206824a17b364a835d2d
2+
gapic_generator_version: 2.34.0
3+
# the libraries are ordered with respect to library name, which is
4+
# java-{library.library_name} or java-{library.api-shortname} when
5+
# library.library_name is not defined.
6+
libraries:
7+
- api_shortname: common-protos
8+
name_pretty: Common Protos
9+
product_documentation: https://github.com/googleapis/api-common-protos
10+
api_description: Protobuf classes for Google's common protos.
11+
release_level: stable
12+
client_documentation: https://cloud.google.com/java/docs/reference/proto-google-common-protos/latest/history
13+
distribution_name: com.google.api.grpc:proto-google-common-protos
14+
excluded_dependencies: "proto-google-common-protos,grpc-google-common-protos,proto-google-common-protos-parent"
15+
excluded_poms: "proto-google-common-protos-bom,proto-google-common-protos"
16+
library_type: OTHER
17+
GAPICs:
18+
- proto_path: google/api
19+
- proto_path: google/apps/card/v1
20+
- proto_path: google/cloud
21+
- proto_path: google/cloud/audit
22+
- proto_path: google/cloud/location
23+
- proto_path: google/geo/type
24+
- proto_path: google/logging/type
25+
- proto_path: google/longrunning
26+
- proto_path: google/rpc
27+
- proto_path: google/rpc/context
28+
- proto_path: google/shopping/type
29+
- proto_path: google/type
30+
- api_shortname: iam
31+
name_pretty: IAM
32+
product_documentation: https://cloud.google.com/iam
33+
api_description: Manages access control for Google Cloud Platform resources
34+
release_level: stable
35+
client_documentation: https://cloud.google.com/java/docs/reference/proto-google-iam-v1/latest/overview
36+
distribution_name: com.google.api.grpc:proto-google-iam-v1
37+
excluded_dependencies: "grpc-google-iam-v1"
38+
excluded_poms: "proto-google-iam-v1-bom,google-iam-policy,proto-google-iam-v1"
39+
library_type: OTHER
40+
GAPICs:
41+
- proto_path: google/iam/v1
42+
- proto_path: google/iam/v2
43+
- proto_path: google/iam/v2beta

‎library_generation/test/resources/test-config/monorepo_without_common_protos.yaml

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
gapic_generator_version: 2.34.0
2+
protoc_version: 25.2
3+
googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
4+
libraries_bom_version: 26.37.0
5+
libraries:
6+
- api_shortname: cloudasset
7+
name_pretty: Cloud Asset Inventory
8+
product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
9+
api_description: "provides inventory services based on a time series database."
10+
library_name: "asset"
11+
client_documentation: "https://cloud.google.com/java/docs/reference/google-cloud-asset/latest/overview"
12+
distribution_name: "com.google.cloud:google-cloud-asset"
13+
release_level: "stable"
14+
issue_tracker: "https://issuetracker.google.com/issues/new?component=187210&template=0"
15+
api_reference: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
16+
GAPICs:
17+
- proto_path: google/cloud/asset/v1
18+
- proto_path: google/cloud/asset/v1p1beta1
19+
- proto_path: google/cloud/asset/v1p2beta1
20+
- proto_path: google/cloud/asset/v1p5beta1
21+
- proto_path: google/cloud/asset/v1p7beta1
22+
- api_shortname: cloudbuild
23+
name_pretty: Cloud Build
24+
product_documentation: https://cloud.google.com/cloud-build/
25+
api_description: lets you build software quickly across all languages. Get complete
26+
control over defining custom workflows for building, testing, and deploying across
27+
multiple environments such as VMs, serverless, Kubernetes, or Firebase.
28+
release_level: stable
29+
distribution_name: com.google.cloud:google-cloud-build
30+
issue_tracker: https://issuetracker.google.com/savedsearches/5226584
31+
GAPICs:
32+
- proto_path: google/devtools/cloudbuild/v1
33+
- proto_path: google/devtools/cloudbuild/v2

0 commit comments

Comments
 (0)