feature: Tightly couple all JSON schemas and code/plugins #2030
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughThe changes involve a significant update to the JSON schema for the Roadrunner configuration file, version 3. The update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Roadrunner
participant SQS
participant BoltDB
User->>Roadrunner: Configure PipelineProperties
Roadrunner->>SQS: Validate configuration (key, secret, region, etc.)
SQS-->>Roadrunner: Configuration valid
Roadrunner->>BoltDB: Set permissions
BoltDB-->>Roadrunner: Permissions set
Roadrunner->>User: Configuration successful
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- schemas/config/3.0.schema.json (6 hunks)
🧰 Additional context used
🪛 Gitleaks
schemas/config/3.0.schema.json
1371-1371: Identified a pattern that may indicate AWS credentials, risking unauthorized cloud resource access and data breaches on AWS platforms.
(aws-access-token)
🔇 Additional comments (4)
schemas/config/3.0.schema.json (4)
2026-2050: Well-structured common properties definitionThe addition of the
PipelinePropertiesdefinition is a good improvement. It centralizes common configuration options for different queues, promoting code reuse and consistency across the schema. The properties are well-described and provide sensible default values.
1366-1408: Improved SQS configuration clarityThe updates to the SQS section enhance the clarity of the configuration options. The added titles, improved descriptions, and examples make it easier for users to understand and correctly configure the SQS integration.
🧰 Tools
🪛 Gitleaks
1371-1371: Identified a pattern that may indicate AWS credentials, risking unauthorized cloud resource access and data breaches on AWS platforms.
(aws-access-token)
Line range hint
1417-1845: Enhanced jobs configuration with improved clarity and flexibilityThe updates to the "jobs" section significantly improve the configuration schema:
- The descriptions for
num_pollers,timeout,pipeline_size, andconsumeare now more precise and informative.- The restructuring of the "pipelines" configuration provides greater flexibility, allowing for different driver types and their specific configurations.
- The use of
$refto include commonPipelinePropertiesreduces redundancy and promotes consistency.These changes will make it easier for users to configure job pipelines correctly and take advantage of different driver options.
2063-2064: Improved BoltDB permissions configurationThe update to the "permission" property in the BoltDB configuration is a good improvement. By using an integer with a pattern
^[0-7]{3}$, it ensures that only valid octal file permission values (e.g., 644, 755) can be used. This change helps prevent configuration errors and improves security by enforcing proper file permissions.
|
Note that the formatting issues are because the project ships with |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
schemas/config/3.0.schema.json (1)
Line range hint
2064-2102: Improved pipeline property definitions and BoltDB configuration
PipelineProperties:
The addition of thePipelinePropertiesdefinition is an excellent improvement. It standardizes common properties (priority,prefetch,consume_all, andqueue) across different pipeline configurations, promoting consistency and reducing redundancy in the schema.BoltDB update:
Thepermissionproperty in the BoltDB definition has been updated to use an integer value with a default of 777. This change aligns with typical Unix permission representation.Consider adding a comment or expanding the description for the
permissionproperty in the BoltDB definition to clarify that the value should be in octal format. This would help prevent potential misunderstandings. For example:"permission": { "description": "Access permission for the DB file (in octal format, e.g., 644 for rw-r--r--).", "type": "integer", "default": 777, "pattern": "^[0-7]{3}$" }This clarification would ensure users understand how to correctly set file permissions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- schemas/config/3.0.schema.json (21 hunks)
🧰 Additional context used
🪛 Gitleaks
schemas/config/3.0.schema.json
1371-1371: Identified a pattern that may indicate AWS credentials, risking unauthorized cloud resource access and data breaches on AWS platforms.
(aws-access-token)
🔇 Additional comments (4)
schemas/config/3.0.schema.json (4)
417-417: LGTM: Improved description accuracyThe description for the BoltDB driver has been updated from "boltdb driver" to "BoltDB driver". This change improves the accuracy of the description by using the correct capitalization for BoltDB.
1316-1329: Excellent addition of TLS configuration optionsThe TLS configuration for AMQP has been significantly improved with the addition of detailed options:
key: Path to the key filecert: Path to the cert fileroot_ca: Path to the CA certificate, with a clear explanation of its useclient_auth_type: Options for client authentication typesThese additions provide much more granular control over the TLS setup, enhancing security configuration capabilities.
Line range hint
1417-1594: Comprehensive improvements to jobs and pipelines configurationThe jobs and pipelines sections have been significantly enhanced:
Job configurations:
- Improved descriptions for
num_pollers,timeout, andpipeline_size, providing clearer guidance on their usage and effects.- Updated description for
consume, clarifying its relationship with thepipelinessection.Pipeline configurations:
- New
driverproperty to specify the plugin to be used.- Expanded
configoptions for various drivers (BoltDB, AMQP, Kafka, Memory, Beanstalk, SQS, NATS).- Introduction of common properties like
priority,prefetch,consume_all, andqueueacross different drivers.These changes offer more flexibility and control in configuring jobs and pipelines, while also improving the clarity of the documentation. The expanded options for different drivers will allow users to fine-tune their configurations based on their specific needs.
Line range hint
1-2610: Overall excellent improvements to the configuration schemaThe changes made to this configuration schema file are comprehensive and beneficial. Key improvements include:
- Enhanced security configurations, especially in the AMQP TLS settings.
- More detailed and accurate descriptions across various sections (SQS, jobs, pipelines).
- Standardization of common properties through the new
PipelinePropertiesdefinition.- Expanded configuration options for different drivers in the pipelines section.
- Updated BoltDB permissions to align with Unix standards.
These changes will significantly improve the usability of the configuration, provide more flexibility for users, and enhance the overall documentation quality. The schema now offers clearer guidance and more granular control over various aspects of the system configuration.
Great work on these improvements! They will undoubtedly make the system more robust and user-friendly.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2030 +/- ##
==========================================
- Coverage 37.29% 35.95% -1.35%
==========================================
Files 16 17 +1
Lines 866 1018 +152
==========================================
+ Hits 323 366 +43
- Misses 517 621 +104
- Partials 26 31 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
schemas/config/3.0.schema.json (1)
Overly Permissive Defaults and Sensitive Credentials Identified
The verification process has confirmed the following security concerns in
schemas/config/3.0.schema.json:
Overly Permissive Default Permissions:
- Lines 1814, 2335, 2842: Default permissions are set to
777for BoltDB and BoltDB_J configurations. This level of permission is too permissive and poses a security risk.Exposure of Example AWS Credentials:
- Lines 1365, 1374: Realistic-looking AWS credentials (
ASIAIOSFODNN7EXAMPLEandwJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY) are present. Even though they are examples, they can potentially be misused or mistaken for real credentials.Action Items:
Restrict Default Permissions:
- Change the default permissions from
777to a more secure setting, such as600, to prevent unauthorized access.Remove or Modify Example AWS Credentials:
- Replace the existing example credentials with clearly fictional or obfuscated values to eliminate any risk of misuse.
Addressing these issues will enhance the security posture of the RoadRunner configuration schema and prevent potential vulnerabilities.
🔗 Analysis chain
Line range hint
1-2844: Overall improvements with some security considerationsThis update to the RoadRunner configuration schema brings significant enhancements:
- Improved documentation and descriptions across multiple sections.
- New configuration options for various components (OpenTelemetry, SQS, Kafka, etc.).
- Introduction of the
PipelinePropertiesdefinition for better structure and consistency.- Expanded job pipeline configurations for multiple queue systems.
These changes will greatly improve the usability and flexibility of RoadRunner configurations.
However, there are two security concerns that should be addressed:
- Default permissions (777) for BoltDB and BoltDB_J are too permissive. Consider using more restrictive defaults (e.g., 600).
- Example AWS credentials in the SQS section should be replaced with clearly fictional values to prevent any potential misuse.
To ensure these security concerns are properly addressed, please run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for overly permissive default permissions and realistic-looking AWS credentials echo "Checking for overly permissive default permissions:" grep -n '"default": *777' schemas/config/3.0.schema.json echo "Checking for realistic-looking AWS credentials:" grep -n 'ASIAIOSFODNN7EXAMPLE' schemas/config/3.0.schema.json grep -n 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY' schemas/config/3.0.schema.json echo "If any lines are returned above, please review and update them as suggested in the previous comments."Length of output: 912
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .rr.yaml (1 hunks)
- schemas/config/3.0.schema.json (22 hunks)
🧰 Additional context used
🪛 Gitleaks
schemas/config/3.0.schema.json
1365-1365: Identified a pattern that may indicate AWS credentials, risking unauthorized cloud resource access and data breaches on AWS platforms.
(aws-access-token)
🔇 Additional comments (17)
.rr.yaml (11)
Line range hint
1-13: Configuration version looks good.The RoadRunner configuration version is set to '3', which is appropriate for recent versions of RoadRunner. This ensures compatibility with the latest features and syntax.
Line range hint
14-21: RPC configuration is appropriate.The RPC server is configured to listen on "tcp://127.0.0.1:6001", which is a secure default as it's only accessible locally. This configuration is suitable for most setups.
Line range hint
22-101: Server configuration enhanced withon_initsection.The server configuration has been expanded to include an
on_initsection, which allows for executing commands before the main server command. This addition provides more flexibility in worker initialization. The configuration also includes detailed settings for environment variables, user/group permissions, and worker management, which are all appropriate for a production environment.
Line range hint
102-186: Logging configuration significantly enhanced.The logs section has been greatly improved:
- File logger options have been added, allowing for log rotation and compression.
- Channel-specific logging configurations enable fine-grained control over log levels and outputs for different components.
- The addition of various encoding and output options provides flexibility in log formatting and storage.
These enhancements will greatly improve debugging and monitoring capabilities in production environments.
Line range hint
187-324: New temporal configuration added for workflow management.A comprehensive temporal configuration has been introduced, indicating integration with a temporal workflow engine. Key points:
- Settings for address, cache size, and namespace are provided.
- Metrics configuration allows for monitoring using Prometheus or StatsD.
- TLS configuration options enhance security for temporal communications.
- Activities pool settings allow for fine-tuning of worker behavior.
This addition significantly enhances RoadRunner's capabilities for managing complex, distributed workflows. Ensure that the temporal server is properly set up and accessible at the specified address.
Line range hint
325-458: Comprehensive KV plugin configuration with multiple storage options.The KV plugin settings provide a wide range of storage options:
- BoltDB for local file-based storage
- Memcached for distributed memory caching
- Redis for advanced key-value storage with clustering support
- In-memory storage for high-speed, non-persistent data
This configuration offers great flexibility to choose the most appropriate storage solution based on the application's needs. Each storage type is well-configured with relevant options for performance tuning and connection management.
Line range hint
459-545: Service plugin well-configured for managing additional processes.The service plugin configuration defines two services (
some_service_1andsome_service_2) with detailed settings:
- Custom commands and environment variables for each service.
- Process management options like
process_num,timeout_stop_sec, andexec_timeout.- Restart behavior control with
remain_after_exitandrestart_sec.This configuration provides excellent control over additional processes that need to run alongside RoadRunner, allowing for complex setups and improved reliability.
Line range hint
546-595: OpenTelemetry (otel) configuration enhances observability.The addition of OpenTelemetry configuration significantly improves the application's observability:
- Detailed resource information including service name, version, and namespace.
- Support for both HTTP and gRPC clients for sending telemetry data.
- Flexible exporter options, supporting OTLP, Zipkin, and custom endpoints.
- Configuration for compression and security (insecure option) of telemetry data.
This setup will greatly aid in monitoring, debugging, and performance analysis of the application in production environments.
Line range hint
596-1005: Extensive HTTP plugin configuration with new cache middleware.The HTTP plugin configuration is comprehensive and well-structured:
- Detailed server settings including address, error codes, and trusted subnets.
- Middleware configuration including headers, gzip, and the new cache middleware.
- File upload settings for security and control.
- SSL/TLS configuration for secure communications.
- Worker pool settings for performance tuning.
The addition of the cache middleware (using Souin) is a significant enhancement:
- Configurable caching strategies per URL pattern.
- Support for distributed caching using etcd or Olric.
- Fine-grained control over cache headers and TTL.
This configuration provides a robust and performant setup for HTTP services. The cache middleware, in particular, can significantly improve response times and reduce server load for appropriate endpoints.
Line range hint
1006-2270: Comprehensive configuration for additional features and integrations.The configuration file includes extensive settings for various advanced features:
Job queue systems:
- Support for NATS, AMQP, Beanstalk, SQS, and Kafka, providing flexibility in choosing a message queue.
- Detailed configuration options for each queue system, allowing for fine-tuned performance and reliability.
gRPC server:
- Configurable listen address, protocol buffers, and TLS settings.
- Advanced options for connection management and timeouts.
TCP plugin:
- New feature allowing for raw TCP servers with customizable settings per server.
Fileserver:
- Configuration for serving static files with options for caching, compression, and range requests.
Centrifugo server plugin:
- Integration with Centrifugo for real-time messaging capabilities.
These additions significantly expand RoadRunner's capabilities, allowing it to serve as a comprehensive application server for various use cases. The configurations are well-structured and provide detailed control over each feature.
Ensure that only the necessary features are enabled and properly secured in your production environment.
Line range hint
1-2270: Comprehensive and well-structured RoadRunner configuration.This
.rr.yamlfile represents a significant evolution in RoadRunner's configuration capabilities:
- It covers a wide range of features from basic server settings to advanced integrations like Temporal and Centrifugo.
- New features such as the cache middleware, TCP plugin, and enhanced logging options have been added.
- The configuration provides fine-grained control over various aspects of the application server, allowing for highly customized setups.
- Support for multiple job queue systems, gRPC, and fileserving capabilities make RoadRunner a versatile application server.
While the configuration is comprehensive, it's important to note that not all features may be necessary for every application. It's recommended to carefully review and enable only the required features for your specific use case, ensuring optimal performance and security.
Remember to keep sensitive information like API keys and passwords secure, potentially using environment variables instead of hardcoding them in the configuration file.
Overall, this configuration file sets up RoadRunner as a powerful and flexible application server capable of handling a wide variety of PHP application requirements.
schemas/config/3.0.schema.json (6)
417-417: Improved consistency in BoltDB driver descriptionThe capitalization of "BoltDB" in the driver description has been corrected, improving consistency with the rest of the document.
484-491: Enhanced OpenTelemetry configuration optionsThe
otelsection has been improved with additional options:
- New exporter types added:
zipkin,stdout,stderr,otlp,jaeger, andjaeger_agent.- Client types now include
httpandgrpc.These additions provide more flexibility in configuring OpenTelemetry, allowing users to choose from a wider range of exporters and clients.
Also applies to: 507-510
2298-2321: New PipelineProperties definition improves schema structureA new
PipelinePropertiesdefinition has been added, which centralizes common configuration options for different queues. This addition:
- Promotes consistency across queue configurations.
- Reduces redundancy in the schema.
- Makes it easier to maintain and update common properties.
The definition includes properties such as
priority,prefetch,consume_all, andqueue, with clear descriptions and default values.
Line range hint
1411-1434: Comprehensive update to jobs configurationThe
jobssection has undergone significant improvements:
Enhanced descriptions for properties like
num_pollers,timeout,pipeline_size, andconsume, providing clearer guidance on their usage and effects.Extensive expansion of the
pipelinesconfiguration, now including detailed settings for multiple queue systems:
- AMQP
- SQS
- Beanstalk
- BoltDB
- In-memory
- NATS
- Kafka
These changes offer much greater flexibility and customization options for job processing, allowing users to fine-tune their configurations for various queue systems.
The expanded configuration options and improved descriptions will significantly enhance the usability and power of the jobs system.
Also applies to: 1442-1927
1308-1323: Enhanced AMQP TLS configuration optionsNew TLS configuration options have been added to the AMQP section, including:
key: Path to the key filecert: Path to the cert fileroot_ca: Path to the CA certificateclient_auth_type: Client authentication typeThese additions provide more granular control over TLS settings for AMQP connections, allowing users to implement stronger security measures and comply with specific security requirements.
Line range hint
2774-2832: Comprehensive Kafka SASL configuration options addedThe Kafka configuration has been enhanced with detailed SASL (Simple Authentication and Security Layer) options:
- Support for multiple authentication mechanisms:
aws_msk_iam,plain,SCRAM-SHA-256, andSCRAM-SHA-512.- Options for username, password, and nonce for standard SASL authentication.
- AWS MSK IAM specific options:
access_key,secret_key,session_token, anduser_agent.- Additional options like
is_tokenandzidfor advanced authentication scenarios.These additions significantly improve the flexibility and security options for Kafka connections, especially in cloud environments like AWS MSK. Users now have more control over authentication methods and can implement more secure and compliant Kafka setups.
|
I went through this after trying to make the tests pass, and I found quit a lot of errors that I have now fixed.
The validation now passes in strict mode without any warnings. I changed the logic for the jobs pipeline driver validation, making it impossible to provide unsupported/unsued configuration parameters for any queue. Before, this was possible as the enum for type was not tightly coupled to the possible configuration parameters (i.e. you could provide SQS parameters for a Kafka queue). Edit: A lot of work is still required to bring the quality of the JSON schema up-to-par across the board, but merging this in is a good start. Again, I wan't to make it clear that this depends on merging roadrunner-server/docs#27 and roadrunner-server/sqs#570. I think this is also too much text for the CodeRabbit AI. It's full-on hallucinating about some of the contents of the PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- schemas/config/3.0.schema.json (23 hunks)
🧰 Additional context used
🪛 Gitleaks
schemas/config/3.0.schema.json
1363-1363: Identified a pattern that may indicate AWS credentials, risking unauthorized cloud resource access and data breaches on AWS platforms.
(aws-access-token)
🔇 Additional comments (7)
schemas/config/3.0.schema.json (7)
417-417: Improved description for BoltDB driverThe description for the BoltDB driver has been updated from "boltdb driver" to "BoltDB driver". This change improves consistency in capitalization and clarity.
484-491: Enhanced OpenTelemetry configuration optionsThe
otelsection has been expanded with new options:
- Additional exporters: "stdout", "stderr", "otlp", "jaeger", and "jaeger_agent" have been added to the
exporterproperty.- New client types: "http" and "grpc" have been added to the
clientproperty.These additions provide more flexibility in configuring OpenTelemetry, allowing for a wider range of export and client options.
Also applies to: 506-509
1306-1321: Improved AMQP configuration and security optionsThe AMQP section has been enhanced with the following changes:
The
tlsproperty has been restructured and expanded, now including options forkey,cert,root_ca, andclient_auth_type. This provides more granular control over TLS settings.New properties have been added:
delete_queue_on_stop: Allows for automatic queue deletion when stopping the pipeline.durable: Determines whether the queue is durable.These additions provide more flexibility in configuring AMQP connections and improve security options with the expanded TLS settings.
Also applies to: 1470-1473, 1493-1497
Line range hint
1409-1925: Comprehensive improvements to jobs configurationThe
jobssection has undergone significant enhancements:
Updated descriptions for properties like
num_pollers,timeout,pipeline_size, andconsume, providing clearer guidance on their usage and effects.The
pipelinesproperty has been extensively restructured and expanded:
- Detailed configuration options for various drivers (AMQP, SQS, Beanstalk, BoltDB, Memory, NATS, Kafka) have been added.
- Each driver now has its own set of specific configuration options, improving flexibility and clarity.
- Common configuration options have been extracted into a
PipelinePropertiesdefinition, promoting reusability and consistency.New options have been added for several drivers, such as
delete_queue_on_stopfor AMQP andskip_queue_declarationfor SQS.Improved structure and organization make it easier to understand and configure different job drivers.
These changes significantly improve the flexibility and clarity of the jobs configuration, allowing for more precise and driver-specific setups.
Line range hint
2772-2830: Enhanced Kafka SASL configuration optionsThe Kafka configuration has been significantly improved with the addition of comprehensive SASL (Simple Authentication and Security Layer) options:
- New SASL mechanisms have been added, including "aws_msk_iam", "plain", "SCRAM-SHA-256", and "SCRAM-SHA-512".
- Additional properties for authentication have been included:
usernameandpasswordfor basic authenticationnonceandzidfor more advanced SASL mechanisms- AWS-specific options like
access_key,secret_key, andsession_tokenuser_agentfor custom identificationThese additions provide much more flexibility in configuring Kafka authentication, supporting a wide range of security setups including AWS MSK IAM authentication.
The detailed comments for each option also improve the usability of the configuration.
2333-2335:⚠️ Potential issueSecurity concern: Default file permissions too permissive
The
permissionproperty for BoltDB now has a default value of 777, which grants full read, write, and execute permissions to all users. This is generally considered too permissive and could pose a security risk.Consider changing the default value to a more restrictive permission, such as 600 (read and write permissions only for the owner). This would be a safer default while still allowing users to set more permissive values if needed.
"permission": { "description": "Access permission for the DB file.", "type": "integer", - "default": 777, + "default": 600, "minimum": 0, "maximum": 777 }Likely invalid or redundant comment.
2840-2842:⚠️ Potential issueSecurity concern: Default BoltDB_J permissions too permissive
The
permissionsproperty for BoltDB_J has a default value of 777, which grants full read, write, and execute permissions to all users. This is generally considered too permissive and could pose a security risk.Consider changing the default value to a more restrictive permission, such as 600 (read and write permissions only for the owner). This would be a safer default while still allowing users to set more permissive values if needed.
"permissions": { "type": "integer", - "default": 777, + "default": 600, "minimum": 0, "maximum": 777 }Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
schemas/config/3.0.schema.json (1)
Line range hint
2785-2843: Enhanced Kafka authentication optionsThe
KAFKA_Jdefinition has been expanded with a detailedsaslproperty, providing support for various authentication mechanisms:
- Added support for
aws_msk_iam,plain,SCRAM-SHA-256, andSCRAM-SHA-512authentication mechanisms.- Included options for username, password, and additional parameters like nonce, token usage, and authorization ID.
- Added support for AWS-specific authentication with access key, secret key, and session token.
These additions greatly enhance the flexibility of Kafka authentication, allowing users to configure secure connections to Kafka clusters using various authentication methods, including AWS MSK IAM authentication.
To further improve security, consider adding a note in the description recommending the use of environment variables or secure secret management solutions for sensitive information like passwords and access keys, rather than storing them directly in the configuration file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- schemas/config/3.0.schema.json (27 hunks)
🧰 Additional context used
🪛 Gitleaks
schemas/config/3.0.schema.json
1371-1371: Identified a pattern that may indicate AWS credentials, risking unauthorized cloud resource access and data breaches on AWS platforms.
(aws-access-token)
🔇 Additional comments (7)
schemas/config/3.0.schema.json (7)
Line range hint
185-227: Improved logging configuration with stricter schemaThe changes to the
logssection enhance the configuration options and improve schema validation:
- The new
line_endingproperty allows customization of log line endings, providing better control over log formatting.- Setting
additionalPropertiestofalsefor both thelogssection and itschannelssubsection makes the schema stricter. This prevents undefined properties from being added, reducing the risk of configuration errors.These changes improve both flexibility and reliability of the logging configuration.
Line range hint
1314-1339: Enhanced AMQP security with TLS configurationThe addition of TLS configuration to the
amqpsection significantly improves security for AMQP connections:
- New properties for
key,cert, androot_caallow for proper SSL/TLS setup.- The
client_auth_typeproperty provides fine-grained control over client authentication, with options ranging from no client certificates to requiring and verifying client certificates.These changes enable secure, encrypted communications for AMQP, which is crucial for protecting sensitive data in transit.
Line range hint
1417-2120: Significant improvements to jobs configurationThe
jobssection has undergone substantial enhancements:
Descriptions for properties like
num_pollers,timeout,pipeline_size, andconsumehave been updated to provide clearer explanations of their purpose and behavior.A new
PipelinePropertiesdefinition has been introduced, containing common configuration options such aspriority,prefetch,consume_all, andqueue. This helps reduce redundancy in the schema and promotes consistency across different pipeline configurations.The
pipelinesproperty has been extensively updated to support various job queue drivers, including AMQP, SQS, Beanstalk, BoltDB, in-memory, NATS, and Kafka. Each driver now has its own detailed configuration options, providing great flexibility in setting up different types of job queues.New properties have been added to various drivers, such as
delete_queue_on_stopfor AMQP andskip_queue_declarationfor SQS, allowing for more fine-grained control over queue behavior.The SQS driver configuration now includes detailed options for queue attributes and tags, aligning closely with AWS SQS features.
These changes significantly improve the flexibility and clarity of the jobs configuration, allowing users to set up and fine-tune various types of job queues to suit their specific needs.
Line range hint
1934-2120: Enhanced Kafka configuration optionsThe Kafka configuration within the
jobs.pipelinessection has been significantly expanded, providing much more detailed and fine-grained control:
- Added
auto_create_topics_enableto allow automatic topic creation.- Introduced
producer_optionswith detailed settings likedisable_idempotent,required_acks,max_message_bytes,compression_codec, and various timeouts.- Added
group_optionsfor configuring consumer groups, includinggroup_idandblock_rebalance_on_poll.- Expanded
consumer_optionswith properties liketopics,consume_regexp,max_fetch_message_size,min_fetch_message_size,consume_offset, andconsume_partitions.These additions provide users with powerful tools to fine-tune their Kafka producers and consumers, aligning well with Kafka's extensive feature set. The new options allow for precise control over message delivery, consumption patterns, and performance optimizations.
This level of detail in the configuration will be particularly useful for users with complex Kafka setups or specific performance requirements.
2304-2327: New PipelineProperties definition improves configuration consistencyThe addition of the
PipelinePropertiesdefinition is a positive change that promotes consistency across different pipeline configurations. It includes common options such aspriority,prefetch,consume_all, andqueue, which can be reused in various pipeline driver configurations.This abstraction reduces redundancy in the schema and makes it easier to maintain consistent behavior across different types of pipelines.
2621-2629: Improved Duration examplesThe examples for the
Durationtype have been expanded to include more varied and complex durations. This is a helpful change that illustrates the flexibility of the duration format, including fractional values and combinations of different time units.These examples will assist users in correctly formatting duration values in their configurations.
2680-2684: New LogLineEnding definitionThe addition of the
LogLineEndingdefinition provides a way to customize the line ending used in logs. This can be particularly useful when logs need to be processed by systems with specific line ending requirements.The default value of "\n" is appropriate for most Unix-like systems, but users can change it if needed for compatibility with other systems or log processing tools.
nickdnk
changed the title
rustatian
changed the title
JSON errors in JSON schema
| @@ -1763,7 +1677,7 @@ jobs: | |||
| # keep this less than a group's session timeout. | |||
| # | |||
| # Optional, default 40s. Possible values: 10s, 10m. | |||
| transaction_timeout: 100 | |||
| transaction_timeout: 100s | |||
There was a problem hiding this comment.
Was wrong according to docs. Should be a Duration. Please double check.
schemas/config/3.0.schema.json
Outdated
| "minProperties": 1, | ||
| "patternProperties": { | ||
| "[a-zA-Z0-9_-]*": { | ||
| "description": "The name of the key-value storage, as used in your application.", |
There was a problem hiding this comment.
Created if-else here so options validate correctly depending on the selected driver.
| "type": "object", | ||
| "patternProperties": { | ||
| "^[a-zA-Z0-9._-]+$": { | ||
| "allOf": [ |
| "type": "object", | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "service_name": { |
There was a problem hiding this comment.
This section was missing.
| ":8080" | ||
| ] | ||
| }, | ||
| "internal_error_code": { |
There was a problem hiding this comment.
internal_error_code was missing.
| "fe80::/10" | ||
| ] | ||
| }, | ||
| "cache": { |
There was a problem hiding this comment.
Entire cache key removed as it's third party.
| "$ref": "#/definitions/WorkersPool" | ||
| }, | ||
| "ssl": { | ||
| "title": "SSL/TLS (HTTPS) Configuration", |
There was a problem hiding this comment.
Added validation logic that requires either acme or key and cert, but not both at the same time.
| ] | ||
| }, | ||
| "redirect": { | ||
| "description": "Whether to automatically redirect from HTTP to HTTPS.", |
There was a problem hiding this comment.
Turned all these into reusable definitions because they're used in multiple places.
schemas/config/3.0.schema.json
Outdated
| } | ||
| }, | ||
| "kafka": { | ||
| "description": "Kafka jobs driver", |
There was a problem hiding this comment.
Moved KAFKA_J and NATS_J inline as they were only being used in one place.
schemas/config/3.0.schema.json
Outdated
| "$ref": "#/definitions/WorkersPool" | ||
| }, | ||
| "pipelines": { | ||
| "description": "List of broker pipelines associated with the configured drivers. This option is not required since you can declare pipelines at runtime. The selected pipeline `driver` must be configured in the root of your configuration file.", |
There was a problem hiding this comment.
If-else logic here to only allow valid combinations of driver and config.
schemas/config/3.0.schema.json
Outdated
| "type": "string", | ||
| "default": "roadrunner+uuid" | ||
| }, | ||
| "exchange_auto_delete": { |
schemas/config/3.0.schema.json
Outdated
| "default": null | ||
| }, | ||
| "attributes": { | ||
| "title": "AWS queue attributes. Attributes are only applied to the queue if RR creates it. Existing queues will not be modified. Must be any of the attributes listed here: https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_SetQueueAttributes.html.", |
There was a problem hiding this comment.
Added all possibly queue Attributes from the SQS docs to avoid typos.
| }, | ||
| "tcp": { | ||
| "type": "object", | ||
| "description": "Plugin to handle RAW TCP packets.", |
There was a problem hiding this comment.
I've removed all instances of "available since" and similar statements, as documentation should always just reflect latest version, and the entire file should be version controlled if you want to document releases independently.
schemas/config/3.0.schema.json
Outdated
| } | ||
| } | ||
| }, | ||
| "DatabaseProperties": { |
There was a problem hiding this comment.
Some of the Bolt properties were being reused, but not the entire object, so I created a definition just for the reused props.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
schemas/readme.md (1)
14-14: Approved: Addition of--spec=draft2019optionThe addition of the
--spec=draft2019option to theajv validatecommand is a good improvement. It ensures that the validation process uses the Draft 2019 JSON Schema specification, which includes enhancements over earlier drafts.Consider adding a brief comment in the README to explain the purpose of the
--spec=draft2019option. This would help users understand why this specific draft is being used. For example:-s ./schemas/config/3.0.schema.json \ - --spec=draft2019 \ + --spec=draft2019 \ # Use Draft 2019 JSON Schema specification for improved validation -d ./.rr*.y*ml".rr.yaml (1)
Line range hint
666-666: Unusual change in internal_error_codeThe
internal_error_codehas been changed from the standard 500 to 505. This is unusual as HTTP 505 typically means "HTTP Version Not Supported", which might not be appropriate for internal RoadRunner errors.Consider keeping the standard 500 error code for internal errors unless there's a specific reason for this change. If there is a reason, please document it in a comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .github/workflows/tests.yml (1 hunks)
- .rr.yaml (15 hunks)
- schemas/readme.md (1 hunks)
🧰 Additional context used
🔇 Additional comments (11)
schemas/readme.md (2)
10-10: Approved: Docker image update to Node.js 20Updating the Docker image to
node:20-alpineis a positive change. It ensures that the validation process uses a more recent and supported version of Node.js, which can provide performance improvements and better compatibility with modern npm packages.
Line range hint
1-17: Overall, these changes improve the validation processThe updates to the Docker image and the
ajv validatecommand in this README file represent positive improvements to the validation process. They ensure that the project uses up-to-date tools and specifications, which can lead to better performance, compatibility, and validation accuracy.🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: You might be missing the article “an” here.
Context: ...directory inside.** - You can validate existing config file using the following command...(AI_EN_LECTOR_MISSING_DETERMINER_AN)
.github/workflows/tests.yml (1)
27-27: LGTM: Improved JSON Schema validation specificityThe addition of
--spec=draft2019to theajv validatecommand is a positive change. It explicitly specifies the use of the Draft 2019 JSON Schema specification for validation, which aligns with the updates mentioned in the PR objectives and theschemas/readme.mdfile. This enhancement ensures that the validation process uses the correct JSON Schema draft, potentially preventing inconsistencies or unexpected behavior in the future..rr.yaml (8)
7-7: Schema reference added: Excellent!The addition of the schema reference
$schema: "./schemas/config/3.0.schema.json"is a great improvement. This will enhance IDE support and help with configuration validation.
44-45: Environment variables format improvedThe format for defining environment variables has been simplified from a list to a map format. This change enhances readability and makes the configuration more intuitive.
Line range hint
131-149: Enhanced logging configuration: Great improvement!The addition of specific logging channels for different plugins (http, server, rpc) allows for more granular control over log levels and outputs. This will significantly improve observability and debugging capabilities.
1680-1680: New transaction_timeout setting addedThe addition of
transaction_timeout: 100sprovides better control over Temporal workflow transactions. This is a good improvement for managing long-running processes.Please verify that the 100-second timeout aligns with your application's requirements and doesn't conflict with other timeout settings, such as group session timeouts.
258-268: TLS configurations added: Good for security, but be cautious with placeholdersThe addition of TLS configurations across multiple sections (temporal, amqp, kafka, grpc, centrifugo) is excellent for improving security. However, be cautious about using placeholder paths like "/ssl/key.pem", "/ssl/cert.crt", and "/ssl/ca.crt" in the configuration file.
Ensure that these placeholder paths are replaced with actual paths in production environments. Consider using environment variables for sensitive paths to enhance security further.
Also applies to: 1068-1078, 1151-1161, 1814-1824, 2186-2191
Line range hint
852-862: ACME support added: Great for automated SSL, but watch out for placeholderThe addition of ACME (Let's Encrypt) support is excellent for automating SSL certificate management. This will simplify the process of obtaining and renewing SSL certificates.
Please ensure that the placeholder email "your-email-here@email" is replaced with a valid email address before using this configuration in any environment. Using an invalid email could cause issues with certificate issuance and renewal.
898-916: SSL certificate paths commented outThe paths for SSL cert, key, and root_ca have been commented out. This is consistent with the addition of ACME support for automated certificate management.
Ensure that either the ACME configuration is properly set up and working, or uncomment and properly configure these paths if manual SSL configuration is needed. Do not leave SSL unconfigured in a production environment.
Line range hint
1-2191: Overall assessment: Significant improvements with some areas needing attentionThis update to the .rr.yaml file introduces numerous enhancements to the RoadRunner configuration:
- Added schema reference for improved validation
- Enhanced logging capabilities with plugin-specific channels
- Introduced ACME support for automated SSL management
- Expanded TLS configurations across multiple sections
However, there are a few areas that require careful attention:
- The change in
internal_error_codefrom 500 to 505 needs justification- The empty
forbidarray in static middleware could pose a security risk- Placeholder values (especially for emails and SSL paths) need to be replaced with actual values
Please address these concerns and perform a thorough security review before deploying this configuration to any environment.
Conduct a comprehensive security audit of the configuration, paying special attention to TLS settings, access controls, and exposure of sensitive information.
Update ajv cli
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
.rr.yaml (2)
Line range hint
186-234: Temporal metrics configuration added: Good for observabilityThe addition of the
metricssubsection undertemporalis excellent for improving observability. The Prometheus configuration is well-defined and will be useful for monitoring Temporal workflows.However, I noticed that the Statsd configuration is commented out.
Consider either removing the commented Statsd configuration or adding a comment explaining why it's there and when it might be used. This will improve clarity for other developers who might work on this configuration in the future.
258-268: TLS configurations updated: Security improvementThe update of TLS configurations across multiple sections (temporal, amqp, kafka, grpc, centrifuge) with actual file paths instead of empty strings is a good security practice. This ensures that the necessary certificates and keys are properly specified.
Consider using environment variables for these paths instead of hardcoding them. This would make the configuration more flexible across different environments. For example:
key: "${SSL_KEY_PATH:/ssl/key.pem}" cert: "${SSL_CERT_PATH:/ssl/cert.crt}" root_ca: "${SSL_ROOT_CA_PATH:/ssl/ca.crt}"This way, you can easily override these paths using environment variables when needed, while still having default values.
Also applies to: 1068-1078, 1151-1161, 1814-1824, 2186-2191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .github/workflows/tests.yml (1 hunks)
- .rr.yaml (15 hunks)
- schemas/readme.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/tests.yml
- schemas/readme.md
🧰 Additional context used
🔇 Additional comments (8)
.rr.yaml (8)
7-7: Schema reference added: Excellent!The addition of the schema reference
$schema: "./schemas/config/3.0.schema.json"is a great practice. It allows for better validation and autocompletion of the configuration file.
44-45: Environment variables format improvedThe change from a list format to a map format for environment variables in both the
on_initand mainserversections is a good improvement. This new format is more intuitive, easier to read, and allows for simpler value overriding.Also applies to: 71-72
Line range hint
137-156: Enhanced logging configuration: Great addition!The new
channelssubsection underlogsis a valuable addition. It allows for fine-grained control over logging for different plugins (http, server, rpc). This granularity is extremely useful for debugging and monitoring, especially in production environments where you might want different log levels for different components.
400-402: Redis configuration improved for clusteringThe update to use an array format for Redis addresses (
addr: [ "localhost:11211" ]) is a good improvement. This change allows for specifying multiple Redis nodes, which is beneficial for setting up Redis clusters or failover configurations.
Line range hint
666-666: Verify change in internal error codeThe
internal_error_codehas been changed from the standard 500 to 505. This is an unusual change that might affect how errors are handled and reported in your application.Please confirm that this change is intentional and that your application is prepared to handle this non-standard error code. If it's not intentional, consider reverting to the standard 500 error code.
1680-1680: Kafka transaction timeout clarifiedThe change of
transaction_timeoutfrom100to100sin the Kafka configuration is a good clarification. It explicitly specifies that the timeout is in seconds, reducing any potential ambiguity.
898-916: Verify HTTPS configurationThe SSL configuration for HTTPS has been commented out. This could potentially lead to insecure HTTP connections if not handled properly elsewhere in your setup.
Please confirm that HTTPS is still correctly configured for your application, possibly through other means such as a reverse proxy or load balancer. If HTTPS is required and not configured elsewhere, consider uncommenting and properly configuring this section.
Line range hint
1-2191: Overall configuration update: Mostly positive with a few points to verifyThe
.rr.yamlconfiguration file has undergone significant updates across multiple sections. Most changes appear to be improvements, including:
- Addition of a schema reference
- Improved environment variable formatting
- Enhanced logging configuration
- Addition of metrics for Temporal
- More explicit TLS configurations
- Improved Redis configuration for clustering
- Clarification of timeouts
However, there are a few points that require verification or further consideration:
- The change in internal error code from 500 to 505
- The empty 'forbid' array in static middleware
- The commented-out SSL configuration for HTTPS
Please review these points to ensure they align with your intended application behavior and security requirements.
Adjust description of pipeline_size
|
This now includes changes from roadrunner-server/sqs#571 |
nickdnk
changed the title
JSON errors in JSON schema
nickdnk
changed the title
Remove unused Cache key definition
Reason for This PR
Edit: This PR now overhauls the JSON schema structure and references schemas in the other repos. This way we can keep code and changes to the schemas tightly coupled.
For instance, the https://github.com/roadrunner-server/sqs plugin now has its own schema that defines its properties. See https://github.com/roadrunner-server/sqs/blob/master/schema.json. This makes it **a lot ** easier to avoid documentation duplication, and we'll also be able to merge changes/additions to the schema with the same PR that adds or changes configurations.
All the other projects have had their files merged, but I need this in the main repo in order to continue, as there is a bit of a "chicken-and-egg" issue going on with references.
Tests won't pass for this PR, but I would request that it still gets merged, as once it's merged, I can write the tests properly with
ajvand@apidevtools/json-schema-ref-parser.I will then be able to continue doing the same to the rest of the plugins. Currently only
jobsand all the pipeline/kv drivers (sqs,amqp,boltdbetc.) have been moved to this format, but I would like to do it for every part of the configuration file that resides in a different repository than this one.License Acceptance
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.][Reviewer TODO: Verify that these criteria are met. Request changes if not]git commit -s).CHANGELOG.md.Deleted RabbitAI summary as it's no longer correct