refactor(ci): Consolidate compliance checks into pre-commit hook
Move custom file compliance checks (logger pattern, future annotations, cli imports, mTLS endpoints) from GHA inline bash scripts to a unified python script (compliance_checks.py) and expose it as a local pre-commit hook. Remove the compliance-check job from CI workflow. Co-authored-by: Shangjie Chen <deanchen@google.com> PiperOrigin-RevId: 940081615
This commit is contained in:
Shangjie Chen
committed by
Copybara-Service
Copybara-Service
parent
4c862b9660
commit
8a7656b857
@@ -149,128 +149,4 @@ jobs:
|
||||
--ignore=tests/unittests/artifacts/test_artifact_service.py \
|
||||
--ignore=tests/unittests/tools/google_api_tool/test_googleapi_to_openapi_converter.py
|
||||
|
||||
# 4. Custom file content compliance checks (PR only)
|
||||
compliance-check:
|
||||
name: File Content Compliance
|
||||
runs-on: ubuntu-latest
|
||||
if: github.event_name == 'pull_request'
|
||||
steps:
|
||||
- name: Checkout Code
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
# Fetch full history (depth: 0) instead of shallow clone (depth: 2) to ensure
|
||||
# git diff origin/${base_ref}...HEAD can reliably find the merge base,
|
||||
# preventing fatal git errors on deep PRs or when the target branch has progressed.
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Check for logger pattern in all changed Python files
|
||||
run: |
|
||||
git fetch origin ${GITHUB_BASE_REF}
|
||||
CHANGED_FILES=$(git diff --diff-filter=ACMR --name-only origin/${GITHUB_BASE_REF}...HEAD | grep -E '\.py$' || true)
|
||||
if [ -n "$CHANGED_FILES" ]; then
|
||||
echo "Changed Python files to check:"
|
||||
echo "$CHANGED_FILES"
|
||||
echo ""
|
||||
|
||||
# Check for 'logger = logging.getLogger(__name__)' in changed .py files.
|
||||
set +e
|
||||
FILES_WITH_FORBIDDEN_LOGGER=$(grep -lE 'logger = logging\.getLogger\(__name__\)' $CHANGED_FILES)
|
||||
GREP_EXIT_CODE=$?
|
||||
set -e
|
||||
|
||||
if [ $GREP_EXIT_CODE -eq 0 ]; then
|
||||
echo "❌ Found forbidden use of 'logger = logging.getLogger(__name__)'. Please use 'logger = logging.getLogger('google_adk.' + __name__)' instead."
|
||||
echo "The following files contain the forbidden pattern:"
|
||||
echo "$FILES_WITH_FORBIDDEN_LOGGER"
|
||||
exit 1
|
||||
elif [ $GREP_EXIT_CODE -eq 1 ]; then
|
||||
echo "✅ No instances of 'logger = logging.getLogger(__name__)' found in changed Python files."
|
||||
fi
|
||||
else
|
||||
echo "✅ No relevant Python files found."
|
||||
fi
|
||||
|
||||
- name: Check for import pattern in certain changed Python files
|
||||
run: |
|
||||
git fetch origin ${GITHUB_BASE_REF}
|
||||
CHANGED_FILES=$(git diff --diff-filter=ACMR --name-only origin/${GITHUB_BASE_REF}...HEAD | grep -E '\.py$' | grep -v -E '__init__.py$|version.py$|tests/.*|contributing/samples/' || true)
|
||||
if [ -n "$CHANGED_FILES" ]; then
|
||||
echo "Changed Python files to check:"
|
||||
echo "$CHANGED_FILES"
|
||||
echo ""
|
||||
|
||||
# Use grep -L to find files that DO NOT contain the pattern.
|
||||
FILES_MISSING_IMPORT=$(grep -L 'from __future__ import annotations' $CHANGED_FILES || true)
|
||||
|
||||
if [ -z "$FILES_MISSING_IMPORT" ]; then
|
||||
echo "✅ All modified Python files include 'from __future__ import annotations'."
|
||||
exit 0
|
||||
else
|
||||
echo "❌ The following files are missing 'from __future__ import annotations':"
|
||||
echo "$FILES_MISSING_IMPORT"
|
||||
echo "This import is required to allow forward references in type annotations without quotes."
|
||||
exit 1
|
||||
fi
|
||||
else
|
||||
echo "✅ No relevant Python files found."
|
||||
fi
|
||||
|
||||
- name: Check for import from cli package in certain changed Python files
|
||||
run: |
|
||||
git fetch origin ${GITHUB_BASE_REF}
|
||||
CHANGED_FILES=$(git diff --diff-filter=ACMR --name-only origin/${GITHUB_BASE_REF}...HEAD | grep -E '\.py$' | grep -v -E 'cli/.*|src/google/adk/tools/apihub_tool/apihub_toolset.py|tests/.*|contributing/samples/' || true)
|
||||
if [ -n "$CHANGED_FILES" ]; then
|
||||
echo "Changed Python files to check:"
|
||||
echo "$CHANGED_FILES"
|
||||
echo ""
|
||||
|
||||
set +e
|
||||
FILES_WITH_FORBIDDEN_IMPORT=$(grep -lE '^from.*\bcli\b.*import.*$' $CHANGED_FILES)
|
||||
GREP_EXIT_CODE=$?
|
||||
set -e
|
||||
|
||||
if [[ $GREP_EXIT_CODE -eq 0 ]]; then
|
||||
echo "❌ Do not import from the cli package outside of the cli package. If you need to reuse the code elsewhere, please move the code outside of the cli package."
|
||||
echo "The following files contain the forbidden pattern:"
|
||||
echo "$FILES_WITH_FORBIDDEN_IMPORT"
|
||||
exit 1
|
||||
else
|
||||
echo "✅ No instances of importing from the cli package found in relevant changed Python files."
|
||||
fi
|
||||
else
|
||||
echo "✅ No relevant Python files found."
|
||||
fi
|
||||
|
||||
- name: Check for hardcoded googleapis.com endpoints
|
||||
run: |
|
||||
git fetch origin ${GITHUB_BASE_REF}
|
||||
CHANGED_FILES=$(git diff --diff-filter=ACMR --name-only origin/${GITHUB_BASE_REF}...HEAD | grep -E '\.py$' || true)
|
||||
if [ -n "$CHANGED_FILES" ]; then
|
||||
echo "Checking for hardcoded endpoints in: $CHANGED_FILES"
|
||||
|
||||
# 1. Identify files containing any googleapis.com URL.
|
||||
set +e
|
||||
FILES_WITH_ENDPOINTS=$(grep -lE 'https?://[a-zA-Z0-9.-]+\.googleapis\.com' $CHANGED_FILES)
|
||||
|
||||
# 2. From those, identify files that are MISSING the required mTLS version.
|
||||
if [ -n "$FILES_WITH_ENDPOINTS" ]; then
|
||||
FILES_MISSING_MTLS=$(grep -L '.mtls.googleapis.com' $FILES_WITH_ENDPOINTS)
|
||||
fi
|
||||
set -e
|
||||
|
||||
if [ -n "$FILES_MISSING_MTLS" ]; then
|
||||
echo "❌ Found hardcoded googleapis.com endpoints without mTLS support."
|
||||
echo "The following files must define both standard and mTLS (.mtls.googleapis.com) endpoints"
|
||||
echo "to support dynamic endpoint selection as required by security policy:"
|
||||
echo "$FILES_MISSING_MTLS"
|
||||
echo ""
|
||||
echo "To fix this, please follow these steps:"
|
||||
echo "1. Initialize an AuthorizedSession with your credentials."
|
||||
echo "2. Use 'mtls.has_default_client_cert_source() from google-auth' to check for available client certificates."
|
||||
echo "3. If certificates are present, use 'session.configure_mtls_channel()'."
|
||||
echo "4. Dynamically select the '.mtls.' variant of the endpoint when mTLS is active."
|
||||
exit 1
|
||||
else
|
||||
echo "✅ All hardcoded endpoints have corresponding mTLS definitions or no endpoints found."
|
||||
fi
|
||||
fi
|
||||
|
||||
@@ -56,6 +56,11 @@ repos:
|
||||
language: script
|
||||
files: ^src/google/adk/.*\.py$
|
||||
pass_filenames: false
|
||||
- id: compliance-checks
|
||||
name: ADK Compliance Checks
|
||||
entry: scripts/compliance_checks.py
|
||||
language: script
|
||||
files: \.py$
|
||||
- repo: https://github.com/executablebooks/mdformat
|
||||
rev: 0.7.22
|
||||
hooks:
|
||||
|
||||
@@ -33,7 +33,7 @@ from mcp.server.fastmcp import FastMCP
|
||||
import uvicorn
|
||||
|
||||
logging.basicConfig(level=logging.INFO)
|
||||
logger = logging.getLogger(__name__)
|
||||
logger = logging.getLogger('google_adk.' + __name__)
|
||||
|
||||
# Expected OAuth token for testing
|
||||
VALID_TOKEN = 'test_access_token_12345'
|
||||
|
||||
Executable
+178
@@ -0,0 +1,178 @@
|
||||
#!/usr/bin/env python3
|
||||
# Copyright 2026 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
|
||||
#
|
||||
# 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.
|
||||
|
||||
"""Runs compliance checks on ADK source files.
|
||||
|
||||
This script is used as a pre-commit hook and in CI to enforce coding standards.
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
# Legacy files that are temporarily excluded from the mTLS check.
|
||||
# Do not add new files to this list. All new code must support mTLS.
|
||||
_EXCLUDED_FROM_MTLS = {
|
||||
'contributing/samples/environment_and_skills/e2b_environment/agent.py',
|
||||
'contributing/samples/integrations/bigquery_mcp/agent.py',
|
||||
'contributing/samples/integrations/bigtable/agent.py',
|
||||
'contributing/samples/integrations/data_agent/agent.py',
|
||||
'contributing/samples/integrations/gcp_auth/agent.py',
|
||||
'contributing/samples/integrations/gcs/agent.py',
|
||||
'contributing/samples/integrations/gcs_admin/agent.py',
|
||||
'contributing/samples/integrations/integration_connector_euc_agent/agent.py',
|
||||
'contributing/samples/integrations/oauth_calendar_agent/agent.py',
|
||||
'contributing/samples/integrations/spanner/agent.py',
|
||||
'contributing/samples/integrations/spanner_admin/agent.py',
|
||||
'contributing/samples/integrations/spanner_rag_agent/agent.py',
|
||||
'contributing/samples/mcp/mcp_service_account_agent/agent.py',
|
||||
'contributing/samples/models/interactions_api/main.py',
|
||||
'contributing/samples/multimodal/static_non_text_content/agent.py',
|
||||
'src/google/adk/auth/auth_credential.py',
|
||||
'src/google/adk/integrations/api_registry/api_registry.py',
|
||||
'src/google/adk/integrations/bigquery/bigquery_credentials.py',
|
||||
'src/google/adk/integrations/bigquery/data_insights_tool.py',
|
||||
'src/google/adk/integrations/bigquery/metadata_tool.py',
|
||||
'src/google/adk/integrations/gcs/gcs_credentials.py',
|
||||
'src/google/adk/plugins/bigquery_agent_analytics_plugin.py',
|
||||
'src/google/adk/tools/_google_credentials.py',
|
||||
'src/google/adk/tools/apihub_tool/clients/apihub_client.py',
|
||||
'src/google/adk/tools/application_integration_tool/application_integration_toolset.py',
|
||||
'src/google/adk/tools/application_integration_tool/clients/connections_client.py',
|
||||
'src/google/adk/tools/application_integration_tool/clients/integration_client.py',
|
||||
'src/google/adk/tools/bigtable/bigtable_credentials.py',
|
||||
'src/google/adk/tools/data_agent/credentials.py',
|
||||
'src/google/adk/tools/data_agent/data_agent_tool.py',
|
||||
'src/google/adk/tools/google_api_tool/google_api_toolset.py',
|
||||
'src/google/adk/tools/google_api_tool/googleapi_to_openapi_converter.py',
|
||||
'src/google/adk/tools/mcp_tool/mcp_session_manager.py',
|
||||
'src/google/adk/tools/openapi_tool/auth/auth_helpers.py',
|
||||
'src/google/adk/tools/openapi_tool/auth/credential_exchangers/service_account_exchanger.py',
|
||||
'src/google/adk/tools/pubsub/pubsub_credentials.py',
|
||||
'src/google/adk/tools/spanner/spanner_credentials.py',
|
||||
'tests/unittests/auth/test_credential_manager.py',
|
||||
'tests/unittests/cli/utils/test_gcp_utils.py',
|
||||
'tests/unittests/flows/llm_flows/test_functions_request_euc.py',
|
||||
'tests/unittests/integrations/api_registry/test_api_registry.py',
|
||||
'tests/unittests/integrations/bigquery/test_bigquery_credentials.py',
|
||||
'tests/unittests/tools/apihub_tool/clients/test_apihub_client.py',
|
||||
'tests/unittests/tools/application_integration_tool/clients/test_connections_client.py',
|
||||
'tests/unittests/tools/application_integration_tool/clients/test_integration_client.py',
|
||||
'tests/unittests/tools/application_integration_tool/test_application_integration_toolset.py',
|
||||
'tests/unittests/tools/data_agent/test_data_agent_tool.py',
|
||||
'tests/unittests/tools/google_api_tool/test_docs_batchupdate.py',
|
||||
'tests/unittests/tools/google_api_tool/test_google_api_toolset.py',
|
||||
'tests/unittests/tools/google_api_tool/test_googleapi_to_openapi_converter.py',
|
||||
'tests/unittests/tools/openapi_tool/auth/credential_exchangers/test_service_account_exchanger.py',
|
||||
'tests/unittests/tools/openapi_tool/openapi_spec_parser/test_openapi_toolset.py',
|
||||
'tests/unittests/tools/openapi_tool/openapi_spec_parser/test_rest_api_tool.py',
|
||||
'tests/unittests/tools/spanner/test_spanner_credentials.py',
|
||||
'tests/unittests/tools/test_base_google_credentials_manager.py',
|
||||
'tests/unittests/tools/test_google_tool.py',
|
||||
'tests/unittests/workflow/utils/test_workflow_hitl_utils.py',
|
||||
}
|
||||
|
||||
|
||||
def check_logger(content: str) -> bool:
|
||||
# Forbidden: 'logger = logging.getLogger(__name__)'
|
||||
pattern = re.compile(r'logger\s*=\s*logging\.getLogger\(__name__\)')
|
||||
return not pattern.search(content)
|
||||
|
||||
|
||||
def check_future_annotations(content: str, filename: str) -> bool:
|
||||
# Exclude: __init__.py, version.py, tests/, contributing/samples/
|
||||
if (
|
||||
filename.endswith('__init__.py')
|
||||
or filename.endswith('version.py')
|
||||
or 'tests/' in filename
|
||||
or 'contributing/samples/' in filename
|
||||
):
|
||||
return True
|
||||
return 'from __future__ import annotations' in content
|
||||
|
||||
|
||||
def check_cli_import(content: str, filename: str) -> bool:
|
||||
# Exclude: cli/, apihub_toolset.py, tests/, contributing/samples/
|
||||
if (
|
||||
'cli/' in filename
|
||||
or filename.endswith('apihub_toolset.py')
|
||||
or 'tests/' in filename
|
||||
or 'contributing/samples/' in filename
|
||||
):
|
||||
return True
|
||||
# Pattern: ^from.*\bcli\b.*import.*$ (multiline)
|
||||
pattern = re.compile(r'^from.*\bcli\b.*import.*$', re.MULTILINE)
|
||||
return not pattern.search(content)
|
||||
|
||||
|
||||
def check_mtls(content: str, filename: str) -> bool:
|
||||
if filename in _EXCLUDED_FROM_MTLS:
|
||||
return True
|
||||
# Pattern for googleapis: https?://[a-zA-Z0-9.-]+\.googleapis\.com
|
||||
endpoint_pattern = re.compile(r'https?://[a-zA-Z0-9.-]+\.googleapis\.com')
|
||||
if endpoint_pattern.search(content):
|
||||
return '.mtls.googleapis.com' in content
|
||||
return True
|
||||
|
||||
|
||||
def main() -> None:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument('files', nargs='*', help='Files to check')
|
||||
args = parser.parse_args()
|
||||
|
||||
failed = False
|
||||
for f in args.files:
|
||||
# Skip directories if they are passed accidentally
|
||||
if not os.path.isfile(f):
|
||||
continue
|
||||
try:
|
||||
with open(f, 'r', encoding='utf-8') as file:
|
||||
content = file.read()
|
||||
except Exception as e: # pylint: disable=broad-except
|
||||
print(f"Error reading {f}: {e}")
|
||||
continue
|
||||
|
||||
# Run checks
|
||||
if not check_logger(content):
|
||||
print(
|
||||
f"❌ {f}: Found forbidden use of 'logger = logging.getLogger(__name__)'. "
|
||||
"Please use 'logger = logging.getLogger(\"google_adk.\" + __name__)' instead."
|
||||
)
|
||||
failed = True
|
||||
|
||||
if not check_future_annotations(content, f):
|
||||
print(f"❌ {f}: Missing 'from __future__ import annotations'.")
|
||||
failed = True
|
||||
|
||||
if not check_cli_import(content, f):
|
||||
print(
|
||||
f"❌ {f}: Do not import from the cli package outside of the cli package."
|
||||
)
|
||||
failed = True
|
||||
|
||||
if not check_mtls(content, f):
|
||||
print(
|
||||
f"❌ {f}: Found hardcoded googleapis.com endpoints without mTLS support."
|
||||
)
|
||||
failed = True
|
||||
|
||||
if failed:
|
||||
sys.exit(1)
|
||||
sys.exit(0)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
main()
|
||||
Reference in New Issue
Block a user