-
[OPIK-2414] [CI/CD] Add PR check for ON CLUSTER statement in ClickHouse migrations (#3322)
frostbyte_neo 发布于
2025-09-19 07:15:12 +00:00 - [OPIK-2414] Add PR check for ON CLUSTER statement in ClickHouse migrations
- Create validation script to check ClickHouse migrations for proper ON CLUSTER clause usage
- Add GitHub workflow to run validation on PRs and main branch changes
- Ensure all DDL operations (CREATE/DROP/ALTER/RENAME) include ON CLUSTER '{cluster}'
- Prevent cluster inconsistencies in distributed ClickHouse deployments
- Reference: https://clickhouse.com/docs/sql-reference/distributed-ddl
- Revision 2: Only validate changed migration files in PR
- Update validation script to accept specific files as arguments
- Modify GitHub workflow to only check added/modified migration files
- Skip validation of existing files (cannot be changed & were fine before replication)
- Improve workflow efficiency by targeting only relevant files
- Add proper diff filtering for PRs vs main branch pushes
- Revision 3: Fix false positive in ClickHouse migration validation
- Replace xargs with bash parameter expansion to preserve quotes during whitespace trimming
- xargs was incorrectly stripping quotes from '{cluster}', causing valid files to fail validation
- Script now correctly validates files with ON CLUSTER '{cluster}' (with quotes)
- Maintains detection of files missing ON CLUSTER clause entirely
- Tested with both valid and invalid migration files
-
Revision 4: Add validation for rollback DDL statements
-
Revision 5: Clean up check_ddl_statement logic
- Remove confusing rollback skip logic from check_ddl_statement function
- Rollback statements are now properly handled in main parsing logic
- Function is cleaner and more predictable
- Maintains all validation functionality
- Revision 6: Enforce correct ON CLUSTER syntax with quotes
- Only accept 'ON CLUSTER '{cluster}'' (with quotes) as correct syntax
- Remove acceptance of 'ON CLUSTER {cluster}' (without quotes)
- This was from earlier confusion when xargs was stripping quotes
- Stricter validation ensures consistent syntax across all migrations
- Updated error messages to show only the correct syntax
- Revision 7: Centralize DDL regex and make it comprehensive
- Add centralized DDL_COMMANDS_REGEX and DDL_DETECTION_REGEX at top of script
- Remove restrictive TABLE|INDEX limitation from DDL detection
- Now detects DDL for all ClickHouse objects: DATABASE, VIEW, MATERIALIZED VIEW, etc.
- Apply comprehensive detection to both regular DDL and rollback statements
- Maintain single source of truth for DDL patterns
- Tested with CREATE DATABASE, CREATE VIEW, CREATE MATERIALIZED VIEW, etc.
- Revision 8: Enforce exact ON CLUSTER '{cluster}' pattern
- Update regex to exactly match 'ON CLUSTER '{cluster}'' (not any value in quotes)
- Ensures consistency across all migration files in this project
- Rejects ON CLUSTER with different values like '{different}' or 'prod'
- Maintains support for both single and double quotes around {cluster}
- Tested with various patterns to ensure exact matching
- Revision 9: Centralize ON CLUSTER regex and cleanup unused variables
- Move ON_CLUSTER_REGEX to top-level configuration variables
- Remove unused CLUSTER_OBJECTS_REGEX variable (no longer needed)
- Use centralized ON_CLUSTER_REGEX in check_ddl_statement function
- Improves maintainability with single source of truth for regex patterns
- All validation logic now uses centralized patterns
- Revision 10: Centralize rollback comment detection regex
- Add ROLLBACK_COMMENT_REGEX for detecting rollback comments
- Add ROLLBACK_EXTRACT_REGEX for extracting DDL from rollback comments
- Move all rollback-related regex patterns to top-level configuration
- Use centralized patterns in rollback detection logic
- Improves maintainability with single source of truth for all regex patterns
- Revision 11: Align GitHub workflow with backend formatting patterns
- Add run-name for better workflow visibility
- Set timeout-minutes: 5 to match backend formatting check
- Use fetch-depth: 1 for consistency
- Update checkout action to v4.1.1
- Switch to GitHub API for PR file detection (works with fetch-depth=1)
- Improve file processing logic for API response handling
- Maintain git diff approach for push events
- Revision 12: Restrict workflow trigger to SQL files only
- Update paths to trigger only on .sql files in migrations directory
- Prevents unnecessary workflow runs for non-SQL files
- More efficient CI/CD by targeting only relevant file changes
- Aligns with the actual validation scope of the script
- Potential fix for code scanning alert no. 230: Workflow does not contain permissions
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
下载附件