feat: add disabled validator and API config placeholders for streams

[buger]

User description

• Add EnableAll config option to disable streams validator, e.g. enable all components
• Add support for using API configuration in stream configs, $api_config.key

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

---

PR Type

Enhancement, Tests, Configuration changes

---

Description

• Introduced EnableAll validator to bypass stream validation.

• Added support for $api_config placeholders in stream configurations.

• Enhanced test coverage for new validator and API config placeholders.

• Updated dependencies and configuration structures for streaming.

---

Changes walkthrough 📝

	Relevant files
Enhancement	4 files validator.go Added `EnableAll` validator to bypass validation.                +14/-0    validator.go Integrated `EnableAll` validator into stream validation logic. +16/-0    middleware.go Integrated `EnableAll` validator into middleware validation. +14/-1    mw_url_rewrite.go Added support for `$api_config` variable replacement.        +9/-0
Tests	2 files validator_test.go Added tests for `EnableAll` validator and API config placeholders. +63/-2    mw_streaming_test.go Enhanced streaming tests with new configurations and scenarios. +178/-32
Configuration changes	1 files config.go Added `EnableAll` flag to streaming configuration.              +2/-0
Dependencies	2 files stream.go Updated Bento components import for streaming.                      +3/-3      go.mod Updated dependencies for streaming and validation enhancements. +234/-10
Additional files	1 files go.sum +2227/-16

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[buger]

A JIRA Issue ID is missing from your branch name, PR title and PR description! 🦄

Your branch: feature/streaming-enable-all

Your PR title: feat: add disabled validator and API config placeholders for streams

Your PR description: - Add EnableAll config option to disable streams validator, e.g. enable all components - Add support for using API configuration in stream configs, $api_config.key

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

If this is your first time contributing to this repository - welcome!

---

Please refer to jira-lint to get started.

Without the JIRA Issue ID in your branch name you would lose out on automatic updates to JIRA via SCM; some GitHub status checks might fail.

Valid sample branch names:

‣ feature/shiny-new-feature--mojo-10'
‣ 'chore/changelogUpdate_mojo-123'
‣ 'bugfix/fix-some-strange-bug_GAL-2345'

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The new $api_config placeholder replacement logic in the ReplaceTykVariables function should be carefully reviewed to ensure it handles edge cases, such as missing or malformed $api_config variables, and does not introduce unintended behavior. if strings.Contains(in, apiConfigLabel) { if apiDef := ctx.GetDefinition(r); apiDef != nil && !apiDef.ConfigDataDisabled { vars := apiConfigMatch.FindAllString(in, -1) in = gw.replaceVariables(in, vars, apiDef.ConfigData, apiConfigLabel, escape) } } Validation Bypass The introduction of the EnabledAll validator bypasses all validation checks. This could lead to potential misconfigurations or security issues if not properly controlled or documented. // If using enabled-all validator, skip validation if bentoValidatorKind == bento.EnabledAll { return nil } Validation Error Handling The error handling for stream validation in the EnabledForSpec function should be reviewed to ensure that validation errors are logged and handled appropriately, especially when using the EnabledAll validator. if err := streams.ValidateOASObjectWithConfig(oasBytes, "", streamingConfig.EnableAll); err != nil { s.Logger().WithError(err).Error("Failed to validate streams configuration") return false

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add nil check for ConfigData Validate that apiDef.ConfigData is not nil before passing it to replaceVariables to avoid potential nil pointer dereferences. gateway/mw_url_rewrite.go [225] -in = gw.replaceVariables(in, vars, apiDef.ConfigData, apiConfigLabel, escape) +if apiDef.ConfigData != nil { + in = gw.replaceVariables(in, vars, apiDef.ConfigData, apiConfigLabel, escape) +} Suggestion importance[1-10]: 9 __ Why: Adding a nil check for apiDef.ConfigData before passing it to replaceVariables is a critical improvement to avoid potential nil pointer dereferences, ensuring the code's stability and reliability.	High
	Add nil check for streamingConfig Ensure that streamingConfig is checked for nil before accessing its EnableAll field to prevent runtime panics. ee/middleware/streams/middleware.go [83-85] +if streamingConfig == nil { + s.Logger().Error("Streaming configuration is nil") + return false +} if err := streams.ValidateOASObjectWithConfig(oasBytes, "", streamingConfig.EnableAll); err != nil { s.Logger().WithError(err).Error("Failed to validate streams configuration") return false } Suggestion importance[1-10]: 8 __ Why: Adding a nil check for streamingConfig is crucial to prevent runtime panics when accessing its EnableAll field. This is a significant improvement to ensure robustness and avoid potential crashes.	Medium
	Add nil check for document Add a check to ensure that the document parameter in the Validate method of EnableAllConfigValidator is not nil to avoid potential nil pointer dereferences in future extensions. apidef/streams/bento/validator.go [121-123] func (v *EnableAllConfigValidator) Validate(document []byte) error { + if document == nil { + return fmt.Errorf("document cannot be nil") + } return nil } Suggestion importance[1-10]: 3 __ Why: Adding a nil check for document in the Validate method is a precautionary measure to prevent potential issues in future extensions. However, since the current implementation always returns nil and does not use document, the impact is minimal.	Low

[github-actions]

API Changes

--- prev.txt	2025-05-21 07:23:51.114654503 +0000
+++ current.txt	2025-05-21 07:23:46.752673464 +0000
@@ -5154,6 +5154,11 @@
     ValidateOASObjectWithBentoConfigValidator validates a Tyk Streams document
     against a particular OAS version and takes an optional ConfigValidator
 
+func ValidateOASObjectWithConfig(documentBody []byte, oasVersion string, disableValidator bool) error
+    ValidateOASObjectWithConfig validates a Tyk Streams document against a
+    particular OAS version, using the provided configuration to determine if
+    validation should be disabled.
+
 func ValidateOASTemplate(documentBody []byte, oasVersion string) error
     ValidateOASTemplate checks a Tyk Streams OAS API template for necessary
     fields, acknowledging that some standard Tyk OAS API fields are optional in
@@ -5170,6 +5175,7 @@
 const (
 	DefaultBentoConfigSchemaName string        = "bento-config-schema.json"
 	DefaultValidator             ValidatorKind = "default-validator"
+	EnabledAll                   ValidatorKind = "enabled-all"
 )
 
 TYPES
@@ -5186,6 +5192,16 @@
 
 func (v *DefaultConfigValidator) Validate(document []byte) error
 
+type EnableAllConfigValidator struct{}
+    EnableAllConfigValidator is a validator that skips all validation
+
+func NewEnableAllConfigValidator() *EnableAllConfigValidator
+    NewEnableAllConfigValidator creates a new validator that skips all
+    validation
+
+func (v *EnableAllConfigValidator) Validate(_ []byte) error
+    Validate always returns nil, effectively enabling all configurations
+
 type ValidatorKind string
 
 # Package: ./apidef/streams/bento/schema
@@ -6785,6 +6801,8 @@
 	// are filtered out. This field allows administrators to explicitly permit specific unsafe components when needed.
 	// Use with caution as enabling unsafe components may introduce security vulnerabilities.
 	AllowUnsafe []string `json:"allow_unsafe"`
+	// EnableAll enables all Bento plugins (except unsafe ones) by disabling the streams validator
+	EnableAll bool `json:"enable_all"`
 }
     StreamingConfig holds the configuration for Tyk Streaming functionalities
 

[buger]

/probe review

[buger]

/probe review

[buger]

/probe review

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[github-actions]

Error during chat processing: Error: Failed to get response from AI model during iteration 1. Failed after 3 attempts. Last error: Bad Gateway

---

Tip: Start comment with /probe <request>.
Built by probe

[buger]

/probe review

[tykbot]

@buger Sorry, but I do not understand!

[github-actions]

Error during chat processing: Error: Failed to get response from AI model during iteration 23. prompt is too long: 202797 tokens > 200000 maximum

---

Tip: Start comment with /probe <request>.
Built by probe

[buraksezer]

/probe review this PR

[github-actions]

Code Review: Add Disabled Validator and API Config Placeholders for Streams

This PR introduces two significant enhancements to Tyk's streaming functionality:

1. EnableAll Validator for Streams

The PR adds a new validator type that bypasses validation checks, effectively enabling all components in stream configurations except unsafe ones. This is implemented through:

• A new EnabledAll validator kind in the bento package
• An EnableAllConfigValidator struct that always returns nil for validation
• An EnableAll flag in the StreamingConfig struct
• Integration with the streaming middleware to use this flag

Strengths:

• Clean implementation following existing patterns
• Good test coverage for the new validator
• Proper integration with existing validation flow

Concerns:

• Security Implications: While the PR bypasses validation for most components, it's important to ensure unsafe components are still blocked. The PR description mentions this is handled, but it should be explicitly verified.
• Documentation: It's not clear if documentation will be updated to explain this feature and its potential risks.

2. API Config Placeholders in Stream Configurations

The PR adds support for using API configuration values in stream configurations via $api_config.key syntax, implemented through:

• A new apiConfigLabel constant and apiConfigMatch regex
• Updated ReplaceTykVariables function to handle these placeholders
• Tests verifying the functionality works with both valid and missing fields

Strengths:

• Consistent implementation with other variable replacement patterns in the codebase
• Good test coverage including edge cases (missing fields)
• Useful feature that enhances configuration flexibility

Concerns:

• Nil Check Missing: There's no nil check for apiDef.ConfigData before passing it to replaceVariables, which could lead to a nil pointer dereference if ConfigData is nil.
• Silent Failures: When a referenced config key doesn't exist, it's replaced with an empty string and only logged at debug level. This silent failure might be confusing to debug in production.

Tests

The PR includes comprehensive tests for both features:

• Tests for the EnableAll validator, verifying that invalid configurations pass validation when enabled
• Tests for API config placeholders, including cases where the referenced key exists and where it doesn't
• Integration tests showing both features working together

Overall Assessment

The PR is well-implemented and follows existing patterns in the codebase. The features provide valuable flexibility for stream configurations. However, there are a few improvements that should be considered:

1. Add a nil check for apiDef.ConfigData before using it in replaceVariables
2. Consider adding more visible logging when a referenced API config key doesn't exist
3. Ensure documentation is updated to reflect these new features and their potential implications

The test coverage is good, with tests for both normal operation and edge cases. The code is clean and follows the project's conventions.

Recommendation: Approve with the suggested improvements, particularly the nil check for ConfigData.

---

Tip: Mention me again using /probe <request>.
Powered by Probe AI

[sonarqubecloud]

Quality Gate failed

Failed conditions
38.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud