-
Notifications
You must be signed in to change notification settings - Fork 308
Add MySQL storage support to operator #6936
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change adds MySQL as a first-class storage type alongside PostgreSQL, enabling users to deploy Apicurio Registry with MySQL databases through the operator. Key changes: - Added MYSQL to StorageType enum - Renamed PostgresSql to SqlStorage to support multiple SQL databases - Implemented JDBC URL validation to reject type/URL mismatches - Uncommented MySQL documentation and updated examples to use type: mysql - Added integration test for MySQL datasource configuration Fixes #6919
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds MySQL as a first-class storage type to the Apicurio Registry operator, expanding the supported database backends beyond PostgreSQL and KafkaSQL. The implementation refactors the PostgreSQL-specific code into a more generic SQL storage handler that supports both PostgreSQL and MySQL databases, with proper validation to prevent misconfigurations.
Key Changes:
- Added
MYSQLenum value toStorageTypealongside PostgreSQL and KafkaSQL - Refactored
PostgresSqlclass toSqlStorageto handle both PostgreSQL and MySQL configurations - Implemented JDBC URL validation that ensures the URL scheme matches the declared storage type
- Updated automatic CR migration logic to recognize both PostgreSQL and MySQL as SQL storage types
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/StorageType.java |
Added MYSQL enum value to support MySQL as a storage type |
operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/StorageSpec.java |
Updated JavaDoc and schema descriptions to document MySQL support |
operator/install/install.yaml |
Updated CRD schema to include mysql enum value and updated version to 3.1.5-SNAPSHOT |
operator/controller/src/test/resources/k8s/examples/mysql/example-mysql.apicurioregistry3.yaml |
Added example MySQL configuration with JDBC URL and credentials |
operator/controller/src/test/resources/k8s/examples/mysql/example-mysql-database.yaml |
Added MySQL StatefulSet, Service, and Secret for testing purposes |
operator/controller/src/test/java/io/apicurio/registry/operator/it/MysqlDataSourceITTest.java |
Added integration test verifying MySQL datasource configuration and deployment |
operator/controller/src/main/java/io/apicurio/registry/operator/updater/SqlCRUpdater.java |
Updated CR migration logic to handle both PostgreSQL and MySQL as SQL storage types |
operator/controller/src/main/java/io/apicurio/registry/operator/resource/app/AppDeploymentResource.java |
Updated deployment resource to use SqlStorage for both PostgreSQL and MySQL |
operator/controller/src/main/java/io/apicurio/registry/operator/feat/SqlStorage.java |
New class replacing PostgresSql with support for both database types and JDBC URL validation |
operator/controller/src/main/java/io/apicurio/registry/operator/feat/PostgresSql.java |
Deleted and replaced by SqlStorage |
docs/modules/ROOT/pages/getting-started/assembly-operator-config-reference.adoc |
Uncommented MySQL example configuration in documentation |
docs/modules/ROOT/pages/getting-started/assembly-deploying-registry-operator.adoc |
Uncommented and updated MySQL deployment guide section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static String getSqlKind(StorageType storageType) { | ||
| if (storageType == null) { | ||
| throw new OperatorException("Storage type must be specified for SQL storage"); | ||
| } | ||
| return switch (storageType) { | ||
| case POSTGRESQL -> "postgresql"; | ||
| case MYSQL -> "mysql"; | ||
| default -> throw new OperatorException( | ||
| "Unsupported SQL storage type: " + storageType.getValue()); | ||
| }; | ||
| } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getSqlKind method lacks test coverage. While the integration test verifies MySQL configuration works end-to-end, there are no unit tests for the getSqlKind method that verify:
StorageType.POSTGRESQLreturns "postgresql"StorageType.MYSQLreturns "mysql"- Null storage type throws OperatorException with appropriate message
- Other storage types (e.g., KAFKASQL) throw OperatorException
Consider adding unit tests for this method to ensure the correct SQL kind is returned for each storage type.
| private static boolean isSqlStorageType(StorageType storageType) { | ||
| return StorageType.POSTGRESQL.equals(storageType) || StorageType.MYSQL.equals(storageType); | ||
| } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isSqlStorageType helper method in SqlCRUpdater lacks test coverage. This method is critical for backward compatibility during automatic CR migrations. Consider adding unit tests to verify:
- Returns true for
StorageType.POSTGRESQL - Returns true for
StorageType.MYSQL - Returns false for
StorageType.KAFKASQL - Returns false for null
This ensures the automatic migration logic correctly identifies SQL storage types.
| private static void validateJdbcUrl(StorageType storageType, String jdbcUrl) { | ||
| if (storageType == null || jdbcUrl == null) { | ||
| return; | ||
| } | ||
|
|
||
| String expectedPrefix = switch (storageType) { | ||
| case POSTGRESQL -> "jdbc:postgresql://"; | ||
| case MYSQL -> "jdbc:mysql://"; | ||
| default -> null; | ||
| }; | ||
|
|
||
| if (expectedPrefix != null && !jdbcUrl.startsWith(expectedPrefix)) { | ||
| throw new OperatorException(String.format( | ||
| "JDBC URL mismatch: storage type is '%s' but JDBC URL is '%s'. " | ||
| + "Expected URL to start with '%s'", | ||
| storageType.getValue(), jdbcUrl, expectedPrefix)); | ||
| } | ||
| } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JDBC URL validation logic lacks test coverage. The PR description mentions testing that validation rejects mismatched storage types and URLs, but there are no unit tests for the validateJdbcUrl method. Consider adding unit tests to verify:
type: mysqlwithjdbc:postgresql://URL throws OperatorExceptiontype: postgresqlwithjdbc:mysql://URL throws OperatorException- Valid combinations pass validation
- Null URL or storage type is handled correctly
This validation is critical for preventing misconfigurations in production deployments.
operator/controller/src/main/java/io/apicurio/registry/operator/updater/SqlCRUpdater.java
Outdated
Show resolved
Hide resolved
operator/controller/src/main/java/io/apicurio/registry/operator/updater/SqlCRUpdater.java
Outdated
Show resolved
Hide resolved
operator/controller/src/main/java/io/apicurio/registry/operator/updater/SqlCRUpdater.java
Show resolved
Hide resolved
operator/controller/src/test/java/io/apicurio/registry/operator/it/MysqlDataSourceITTest.java
Show resolved
Hide resolved
- Move isSqlStorageType() to StorageType enum for better reusability - Remove PostgreSQL default in CR updater, use existing storage type instead - Update warning message to mention both SQL storage types - Rename SqlDataSourceITTest to PostgresqlDataSourceITTest for clarity Changes based on review feedback in PR #6936: 1. SqlCRUpdater now uses storageType.orElse(POSTGRESQL) instead of hardcoding POSTGRESQL 2. Storage type validation now uses StorageType.isSqlStorageType() instance method 3. Test naming is clearer with separate PostgreSQL and MySQL test classes
- Copy storageType instead of defaulting to PostgreSQL in SqlCRUpdater - Move isSqlStorageType logic to StorageType.isSql() method - Rename SqlDataSourceITTest to PostgresqlDataSourceITTest for consistency
Summary
This PR adds MySQL storage support to the Apicurio Registry operator, enabling users to deploy Registry instances with MySQL databases.
mysqlas a first-class storage type alongside PostgreSQL and KafkaSQLPostgresSqlclass toSqlStorageto support multiple SQL database typesRelated Issue
Fixes #6919
Test Plan
MysqlDataSourceITTestintegration testtype: mysqlstoragetype: mysqlwithjdbc:postgresql://URLtype: postgresqlwithjdbc:mysql://URL