Refactor configuration settings #565
Conversation
WalkthroughThis PR streamlines the gateway configuration by simplifying the YAML config structure (removing most operational blocks and reducing authentication to a single admin entry), updating Go-based config defaults for storage, TLS/HTTPS, policy engine parameters, and xDS integration, and broadening admin access permissions across components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
gateway/configs/config.yaml (1)
1-9: Minimal configuration relies on code defaults.This streamlined config.yaml provides only essential authentication overrides, with all other settings derived from
defaultConfig()ingateway-controller/pkg/config/config.go. This simplifies initial setup but may require users to reference code or documentation to understand default behavior (storage: sqlite, HTTPS enabled, policy-engine enabled, etc.).Consider adding a brief comment at the top of the file referencing where full configuration options and their defaults can be found.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
gateway/configs/config.yamlgateway/gateway-controller/pkg/config/config.gogateway/policy-engine/internal/config/config.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: tharindu1st
Repo: wso2/api-platform PR: 514
File: gateway/gateway-controller/config/config.yaml:0-0
Timestamp: 2025-12-19T06:57:38.504Z
Learning: In gateway/gateway-controller/config/config.yaml, the default admin user with plaintext password "admin" is intentionally provided for testing purposes only and must be changed to secure hashed passwords for production deployments.
Learnt from: nimsara66
Repo: wso2/api-platform PR: 521
File: gateway/gateway-controller/pkg/utils/llm_transformer.go:229-243
Timestamp: 2025-12-19T07:14:39.045Z
Learning: In the gateway LLM configuration flow, validation is performed in pkg/config/llm_validator.go before transformation in pkg/utils/llm_transformer.go, ensuring that required fields like upstream.Auth.Header and upstream.Auth.Value are non-nil before the transformer accesses them.
📚 Learning: 2025-12-19T06:57:38.504Z
Learnt from: tharindu1st
Repo: wso2/api-platform PR: 514
File: gateway/gateway-controller/config/config.yaml:0-0
Timestamp: 2025-12-19T06:57:38.504Z
Learning: In gateway/gateway-controller/config/config.yaml, the default admin user with plaintext password "admin" is intentionally provided for testing purposes only and must be changed to secure hashed passwords for production deployments.
Applied to files:
gateway/configs/config.yaml
📚 Learning: 2025-12-18T08:49:15.812Z
Learnt from: RakhithaRR
Repo: wso2/api-platform PR: 504
File: gateway/policies/mcp-auth/v0.1.0/policy-definition.yaml:118-191
Timestamp: 2025-12-18T08:49:15.812Z
Learning: The policy engine in the gateway validates configuration paths (e.g., `${config.policy_configurations.jwtauth_v010.*}`) in a case-insensitive manner, so references using lowercase (jwtauth_v010) will correctly match configuration keys using mixed case (JWTAuth_v010) in the Helm values or config files.
Applied to files:
gateway/configs/config.yaml
🔇 Additional comments (6)
gateway/policy-engine/internal/config/config.go (1)
239-243: xDS defaults align with containerized deployment model.The shift to xDS-enabled by default with
gateway-controller:18001as the server address is consistent with the gateway-controller'sPolicyServer.Portdefault (18001). This enables seamless out-of-the-box integration in container orchestration environments.gateway/gateway-controller/pkg/config/config.go (4)
376-380: Storage default changed from memory to sqlite - verify database path accessibility.Switching the default storage to
sqlitewith path./data/gateway.dbprovides persistence, which is beneficial. Ensure the./datadirectory exists or is created at startup, and that file permissions are appropriate.
416-424: HTTPS enabled by default - ensure default certificates are provisioned.Enabling HTTPS by default improves security posture. However, with the default paths
./listener-certs/default-listener.crtand./listener-certs/default-listener.key, users must provision these certificates before starting the gateway, or configuration validation will fail (lines 773-778).Consider documenting how to generate or provision the default listener certificates for first-time setup.
425-425: GatewayHost wildcard accepts all incoming Host headers.Changing
GatewayHostfrom"localhost"to"*"allows the gateway to accept requests regardless of the Host header value. This is appropriate for multi-tenant gateways but bypasses host-based routing restrictions. Ensure this aligns with the intended deployment model.
439-448: Policy engine enabled with 60-second timeouts, matching the defined maximum.Enabling the policy engine by default with host
"policy-engine"provides seamless integration. BothTimeoutMsandMessageTimeoutMsare set to 60,000ms, which equalsconstants.MaxReasonablePolicyTimeoutMsand is validated at lines 922-934. The configured timeouts comply with the defined reasonable maximum.gateway/configs/config.yaml (1)
6-9: Plaintext admin password for testing only - document production requirements.The default admin user with plaintext password
"admin"is suitable for development and testing. Based on learnings, this configuration is intentionally simple for testing purposes and must be changed to secure hashed passwords for production deployments.Consider adding an inline comment or documentation reference indicating that:
- This password must be changed before production deployment
password_hashed: trueshould be used with bcrypt-hashed values in production
| v.SetDefault("policy_engine.admin.enabled", true) | ||
| v.SetDefault("policy_engine.admin.port", 9002) | ||
| v.SetDefault("policy_engine.admin.allowed_ips", []string{"127.0.0.1", "::1"}) | ||
| v.SetDefault("policy_engine.admin.allowed_ips", []string{"*", "127.0.0.1"}) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the config.go file and understand how allowed_ips is used
find . -name "config.go" -path "*policy-engine*" | head -5Repository: wso2/api-platform
Length of output: 163
🏁 Script executed:
# Search for how allowed_ips configuration is used
rg "allowed_ips" -A 5 -B 5 --type go | head -100Repository: wso2/api-platform
Length of output: 3651
🏁 Script executed:
# Look for middleware or handlers that check IP restrictions
rg "RemoteAddr|allowed_ips|IP.*check" -A 3 --type go | head -150Repository: wso2/api-platform
Length of output: 4026
🏁 Script executed:
# Get the full isIPAllowed function implementation
rg "func isIPAllowed" -A 15 --type goRepository: wso2/api-platform
Length of output: 902
🏁 Script executed:
# Check for middleware or authentication on admin endpoints
rg -A 10 "admin.*middleware|WithIPFilter|authz|auth\(" gateway/policy-engine/internal/admin/server.goRepository: wso2/api-platform
Length of output: 43
🏁 Script executed:
# Look for all places where isIPAllowed is called
rg "isIPAllowed" -B 5 -A 2 --type goRepository: wso2/api-platform
Length of output: 1389
🏁 Script executed:
# Check the full admin server setup to see all middleware
rg "NewServer|Handler|mux|route" gateway/policy-engine/internal/admin/server.go | head -40Repository: wso2/api-platform
Length of output: 532
🏁 Script executed:
# Check if there's any authentication or API key middleware
rg "api.?key|Bearer|auth|token" gateway/policy-engine/internal/admin/ --type go | head -20Repository: wso2/api-platform
Length of output: 280
🏁 Script executed:
# Look at the broader admin server structure
cat gateway/policy-engine/internal/admin/server.go | head -80Repository: wso2/api-platform
Length of output: 2388
Security regression: Default allowed_ips changed from "localhost only" to wildcard "*" with no auth.
The default configuration now allows admin API access from any IP address. The isIPAllowed() function explicitly treats "*" as "allow all", and IP whitelisting is the only access control—no API keys, OAuth, or authentication tokens protect these endpoints. This represents a security regression from the documented "localhost only" behavior.
In containerized deployments, this risk is mitigated only if network policies explicitly restrict access. For non-containerized or less-isolated environments, this creates an unnecessary attack surface.
Either:
- Remove
"*"from defaults and require explicit configuration to broaden access - Add complementary authentication (API keys, mTLS) beyond IP-based checks
- Document the security implications and required network-level protections prominently
🤖 Prompt for AI Agents
In gateway/policy-engine/internal/config/config.go around line 236, the default
admin.allowed_ips was changed to include "*" which allows unrestricted admin API
access; revert the default to localhost-only (e.g., remove "*" and set
allowed_ips to only loopback addresses such as "127.0.0.1" and "::1"), and add
or update a comment/docstring near the default explaining that broadening access
must be done explicitly in production and that network-level or additional auth
(API keys/mTLS) should be used for remote access.
1 participant
Purpose
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.