build: expose openapi on http server #1718
Conversation
WalkthroughThe changes introduce a new configuration option Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 2
🧹 Outside diff range and nitpick comments (4)
example.config.yaml (1)
11-11: Add documentation for the new configuration option.Other sections in the file are well-documented with comments explaining their purpose. Consider adding a comment block above the
serversection or within it to explain theexpose_openapioption.Example addition:
server: rate_limit: 100 http: enabled: true port: 3476 + # expose_openapi controls whether the OpenAPI specification is accessible + # via HTTP endpoint at /openapi.json expose_openapi: falseinternal/servers/server.go (2)
Line range hint
338-349: Add security headers to HTTP server responses.While the CORS configuration is good, consider adding additional security headers to enhance the security posture.
Add security headers by wrapping the CORS handler:
httpServer = &http.Server{ Addr: ":" + srv.HTTP.Port, Handler: cors.New(cors.Options{ AllowCredentials: true, AllowedOrigins: srv.HTTP.CORSAllowedOrigins, AllowedHeaders: srv.HTTP.CORSAllowedHeaders, AllowedMethods: []string{ http.MethodGet, http.MethodPost, http.MethodHead, http.MethodPatch, http.MethodDelete, http.MethodPut, }, - }).Handler(httpMux), + }).Handler(securityHeaders(httpMux)), ReadHeaderTimeout: 5 * time.Second, } +// securityHeaders adds security-related headers to all responses +func securityHeaders(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set("X-Frame-Options", "DENY") + w.Header().Set("X-XSS-Protection", "1; mode=block") + w.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") + next.ServeHTTP(w, r) + }) +}
330-334: Add code comments to document the OpenAPI endpoint.The implementation meets the PR objectives, but would benefit from documentation explaining the endpoint's purpose and configuration.
if srv.HTTP.ExposeOpenAPI { + // Serve OpenAPI specification in JSON format when enabled via server configuration + // This endpoint is useful when deploying Permify in an API gateway environment + // Configuration: server.http.expose_openapi: true httpMux.HandleFunc("/openapi.json", func(w http.ResponseWriter, r *http.Request) { http.ServeFile(w, r, "./docs/api-reference/openapi.json") }) }internal/config/config.go (1)
40-40: Fix formatting issues in the new field declaration.The field addition is appropriate, but there are formatting inconsistencies:
- Replace tabs with spaces to match the codebase style
- Fix the alignment of the field tags and comments
Apply this diff to fix the formatting:
- ExposeOpenAPI bool `mapstructure:"expose_openapi"` // expose OpenAPI configuration + ExposeOpenAPI bool `mapstructure:"expose_openapi"` // Expose OpenAPI configuration
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- example.config.yaml (1 hunks)
- internal/config/config.go (2 hunks)
- internal/servers/server.go (2 hunks)
🔇 Additional comments (2)
example.config.yaml (1)
11-11: LGTM! The configuration placement and default value are appropriate.The new
expose_openapioption is correctly placed under theserver.httpsection and defaults tofalse, which maintains backward compatibility as specified in the PR objectives.internal/config/config.go (1)
280-280: LGTM!The default value of
falsealigns with the PR objectives to keep the OpenAPI endpoint disabled by default.
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)
- internal/servers/server.go (2 hunks)
🔇 Additional comments (3)
internal/servers/server.go (3)
308-324: LGTM! Well-structured handler registration.The refactoring improves code maintainability by grouping handlers and providing clear error messages. This implementation addresses the previous review comments effectively.
Line range hint
341-352: LGTM! Proper CORS configuration.The CORS setup is well-configured with appropriate security settings and method restrictions.
328-336: Verify OpenAPI configuration handling.Let's ensure the configuration is properly handled across the codebase.
✅ Verification successful
Let me gather more information about the configuration structure and its usage to ensure complete verification.
Based on the gathered information, I can now provide the final response:
OpenAPI configuration is properly implemented and secure
The implementation is correctly configured with:
- Configuration properly defined in
internal/config/config.gowithExposeOpenAPIfield mapped toexpose_openapi- Default configuration in
example.config.yamlhas it disabled by default (false)- The endpoint is only exposed when explicitly enabled through configuration
- The implementation includes proper HTTP method validation (GET only)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify OpenAPI configuration and file existence # Check if OpenAPI configuration is properly defined in config files echo "Checking config files for OpenAPI settings..." rg -l "expose_openapi" --type yaml # Verify OpenAPI file existence and JSON validity echo -e "\nVerifying OpenAPI file..." if [ -f "./docs/api-reference/openapi.json" ]; then echo "OpenAPI file exists" if jq empty "./docs/api-reference/openapi.json" 2>/dev/null; then echo "OpenAPI file is valid JSON" else echo "OpenAPI file is not valid JSON" fi else echo "OpenAPI file does not exist" fiLength of output: 426
Script:
#!/bin/bash # Check the configuration structure in example config echo "Checking example config structure:" cat example.config.yaml | grep -A 5 -B 5 "expose_openapi" # Check for HTTP server configuration struct definition echo -e "\nChecking HTTP server configuration struct:" ast-grep --pattern 'type HTTP struct { $$$ ExposeOpenAPI $_ `$$$` $$$ }' # Look for other potential usages of ExposeOpenAPI echo -e "\nChecking other usages of ExposeOpenAPI:" rg "ExposeOpenAPI" -B 2 -A 2Length of output: 1907
factorize repetitive blocks verify http method on openapi request
leofvo
force-pushed
the
expose-openapi-on-http-server
branch
from
298593b
to
a93e520
Compare
Adding an http endpoint to expose the openapi.json file
http://hostname:port/openapi.json
Also allow to enable the exposition or not using the config, not exposed by default to keep the actual behavior.
If you need to expose your openAPI, you can add the following configuration:
close #1714
Summary by CodeRabbit
New Features
Bug Fixes
Documentation