Skip to content

Commit

Permalink
chore: always fully generate the repo if it is a non-monorepo or cont…
Browse files Browse the repository at this point in the history
…ains 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 <cloud-java-bot@google.com>
  • Loading branch information
2 people authored and ldetmer committed Sep 17, 2024
1 parent 491505c commit f1c28fe
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 18 deletions.
19 changes: 9 additions & 10 deletions .github/scripts/hermetic_library_generation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,14 @@ fi

image_tag=local
workspace_name="/workspace"
docker_file="library_generation.Dockerfile"
baseline_generation_config="baseline_generation_config.yaml"
message="chore: generate libraries at $(date)"

git checkout "${target_branch}"
git checkout "${current_branch}"
# if the last commit doesn't contain changes to generation configuration
# or Dockerfile, do not generate again as the result will be the same.
change_of_last_commit="$(git diff-tree --no-commit-id --name-only HEAD~1..HEAD -r)"
if [[ ! ("${change_of_last_commit}" == *"${generation_config}"* || "${change_of_last_commit}" == *"${docker_file}"*) ]]; then
echo "The last commit doesn't contain any changes to the generation_config.yaml or Dockerfile, skipping the whole generation process." || true
exit 0
fi

# copy generation configuration from target branch to current branch.
git show "${target_branch}":"${generation_config}" > "${baseline_generation_config}"

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

# commit the change to the pull request.
rm -rdf output googleapis
rm -rdf output googleapis "${baseline_generation_config}"
git add --all -- ':!pr_description.txt'
changed_files=$(git diff --cached --name-only)
if [[ "${changed_files}" == "" ]]; then
Expand Down
9 changes: 7 additions & 2 deletions .github/workflows/hermetic_library_generation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ name: Hermetic library generation upon generation config change through pull req
on:
pull_request:

env:
REPO_FULL_NAME: ${{ github.event.pull_request.head.repo.full_name }}
GITHUB_REPOSITORY: ${{ github.repository }}
jobs:
library_generation:
# skip pull requests come from a forked repository
if: github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -31,6 +32,10 @@ jobs:
shell: bash
run: |
set -x
if [[ "${GITHUB_REPOSITORY}" != "${REPO_FULL_NAME}" ]]; then
echo "This PR comes from a fork. Skip library generation."
exit 0
fi
[ -z "$(git config user.email)" ] && git config --global user.email "cloud-java-bot@google.com"
[ -z "$(git config user.name)" ] && git config --global user.name "cloud-java-bot"
bash .github/scripts/hermetic_library_generation.sh \
Expand Down
15 changes: 12 additions & 3 deletions library_generation/cli/entry_point.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import click as click
from library_generation.generate_pr_description import generate_pr_descriptions
from library_generation.generate_repo import generate_from_yaml
from library_generation.model.config_change import ConfigChange
from library_generation.model.generation_config import from_yaml
from library_generation.utils.generation_config_comparator import compare_config

Expand Down Expand Up @@ -145,11 +146,10 @@ def __generate_repo_and_pr_description_impl(
baseline_config=from_yaml(baseline_generation_config_path),
current_config=from_yaml(current_generation_config_path),
)
# pass None if this is not a monorepo in order to trigger the full
# generation
# pass None if we want to fully generate the repository.
target_library_names = (
config_change.get_changed_libraries()
if config_change.current_config.is_monorepo()
if not _needs_full_repo_generation(config_change=config_change)
else None
)
generate_from_yaml(
Expand All @@ -163,6 +163,15 @@ def __generate_repo_and_pr_description_impl(
)


def _needs_full_repo_generation(config_change: ConfigChange) -> bool:
"""
Whether you should need a full repo generation, i.e., generate all
libraries in the generation configuration.
"""
current_config = config_change.current_config
return not current_config.is_monorepo() or current_config.contains_common_protos()


@main.command()
@click.option(
"--generation-config-path",
Expand Down
53 changes: 53 additions & 0 deletions library_generation/test/cli/entry_point_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,56 @@ def test_generate_non_monorepo_with_changes_triggers_full_generation(
generate_from_yaml.assert_called_with(
config=ANY, repository_path=ANY, target_library_names=None
)

@patch("library_generation.cli.entry_point.generate_from_yaml")
@patch("library_generation.cli.entry_point.generate_pr_descriptions")
def test_generate_monorepo_with_common_protos_triggers_full_generation(
self,
generate_pr_descriptions,
generate_from_yaml,
):
"""
this tests confirms the behavior of generation of a monorepo with
common protos.
generate() should call generate_from_yaml() with
target_library_names=None in order to trigger the full generation
"""
config_path = f"{test_resource_dir}/monorepo_with_common_protos.yaml"
self.assertTrue(from_yaml(config_path).is_monorepo())
# we call the implementation method directly since click
# does special handling when a method is annotated with @main.command()
generate_impl(
baseline_generation_config_path=config_path,
current_generation_config_path=config_path,
repository_path=".",
)
generate_from_yaml.assert_called_with(
config=ANY, repository_path=ANY, target_library_names=None
)

@patch("library_generation.cli.entry_point.generate_from_yaml")
@patch("library_generation.cli.entry_point.generate_pr_descriptions")
def test_generate_monorepo_without_common_protos_does_not_trigger_full_generation(
self,
generate_pr_descriptions,
generate_from_yaml,
):
"""
this tests confirms the behavior of generation of a monorepo without
common protos.
generate() should call generate_from_yaml() with
target_library_names=changed libraries which does not trigger the full
generation.
"""
config_path = f"{test_resource_dir}/monorepo_without_common_protos.yaml"
self.assertTrue(from_yaml(config_path).is_monorepo())
# we call the implementation method directly since click
# does special handling when a method is annotated with @main.command()
generate_impl(
baseline_generation_config_path=config_path,
current_generation_config_path=config_path,
repository_path=".",
)
generate_from_yaml.assert_called_with(
config=ANY, repository_path=ANY, target_library_names=[]
)
19 changes: 16 additions & 3 deletions library_generation/test/generate_library_unit_tests.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
# Copyright 2024 Google LLC
#
# 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
#
# https://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.
import subprocess
import unittest
import os
Expand Down Expand Up @@ -66,13 +79,13 @@ def _run_command_and_get_sdout(self, command, **kwargs):
def test_get_grpc_version_with_no_env_var_fails(self):
# the absence of DOCKER_GRPC_VERSION will make this function to fail
result = self._run_command("get_grpc_version")
self.assertEquals(1, result.returncode)
self.assertEqual(1, result.returncode)
self.assertRegex(result.stdout.decode(), "DOCKER_GRPC_VERSION is not set")

def test_get_protoc_version_with_no_env_var_fails(self):
# the absence of DOCKER_PROTOC_VERSION will make this function to fail
result = self._run_command("get_protoc_version")
self.assertEquals(1, result.returncode)
self.assertEqual(1, result.returncode)
self.assertRegex(result.stdout.decode(), "DOCKER_PROTOC_VERSION is not set")

def test_download_tools_without_baked_generator_fails(self):
Expand All @@ -92,5 +105,5 @@ def test_download_tools_without_baked_generator_fails(self):
result = self._run_command(
f"download_tools {test_protoc_version} {test_grpc_version} {self.TEST_ARCHITECTURE}"
)
self.assertEquals(1, result.returncode)
self.assertEqual(1, result.returncode)
self.assertRegex(result.stdout.decode(), "Please configure your environment")
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
googleapis_commitish: 6a474b31c53cc1797710206824a17b364a835d2d
gapic_generator_version: 2.34.0
# the libraries are ordered with respect to library name, which is
# java-{library.library_name} or java-{library.api-shortname} when
# library.library_name is not defined.
libraries:
- api_shortname: common-protos
name_pretty: Common Protos
product_documentation: https://github.com/googleapis/api-common-protos
api_description: Protobuf classes for Google's common protos.
release_level: stable
client_documentation: https://cloud.google.com/java/docs/reference/proto-google-common-protos/latest/history
distribution_name: com.google.api.grpc:proto-google-common-protos
excluded_dependencies: "proto-google-common-protos,grpc-google-common-protos,proto-google-common-protos-parent"
excluded_poms: "proto-google-common-protos-bom,proto-google-common-protos"
library_type: OTHER
GAPICs:
- proto_path: google/api
- proto_path: google/apps/card/v1
- proto_path: google/cloud
- proto_path: google/cloud/audit
- proto_path: google/cloud/location
- proto_path: google/geo/type
- proto_path: google/logging/type
- proto_path: google/longrunning
- proto_path: google/rpc
- proto_path: google/rpc/context
- proto_path: google/shopping/type
- proto_path: google/type
- api_shortname: iam
name_pretty: IAM
product_documentation: https://cloud.google.com/iam
api_description: Manages access control for Google Cloud Platform resources
release_level: stable
client_documentation: https://cloud.google.com/java/docs/reference/proto-google-iam-v1/latest/overview
distribution_name: com.google.api.grpc:proto-google-iam-v1
excluded_dependencies: "grpc-google-iam-v1"
excluded_poms: "proto-google-iam-v1-bom,google-iam-policy,proto-google-iam-v1"
library_type: OTHER
GAPICs:
- proto_path: google/iam/v1
- proto_path: google/iam/v2
- proto_path: google/iam/v2beta
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
gapic_generator_version: 2.34.0
protoc_version: 25.2
googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
libraries_bom_version: 26.37.0
libraries:
- api_shortname: cloudasset
name_pretty: Cloud Asset Inventory
product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
api_description: "provides inventory services based on a time series database."
library_name: "asset"
client_documentation: "https://cloud.google.com/java/docs/reference/google-cloud-asset/latest/overview"
distribution_name: "com.google.cloud:google-cloud-asset"
release_level: "stable"
issue_tracker: "https://issuetracker.google.com/issues/new?component=187210&template=0"
api_reference: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
GAPICs:
- proto_path: google/cloud/asset/v1
- proto_path: google/cloud/asset/v1p1beta1
- proto_path: google/cloud/asset/v1p2beta1
- proto_path: google/cloud/asset/v1p5beta1
- proto_path: google/cloud/asset/v1p7beta1
- api_shortname: cloudbuild
name_pretty: Cloud Build
product_documentation: https://cloud.google.com/cloud-build/
api_description: lets you build software quickly across all languages. Get complete
control over defining custom workflows for building, testing, and deploying across
multiple environments such as VMs, serverless, Kubernetes, or Firebase.
release_level: stable
distribution_name: com.google.cloud:google-cloud-build
issue_tracker: https://issuetracker.google.com/savedsearches/5226584
GAPICs:
- proto_path: google/devtools/cloudbuild/v1
- proto_path: google/devtools/cloudbuild/v2

0 comments on commit f1c28fe

Please sign in to comment.