From dff7a5abb329e0d5e49f083c9f9c1498127ddca8 Mon Sep 17 00:00:00 2001 From: tanyamadaan Date: Fri, 28 Mar 2025 12:20:08 +0530 Subject: [PATCH] Changes as per integration testing --- pkg/plugin/definition/router.go | 6 +- .../implementation/router/cmd/plugin.go | 2 +- pkg/plugin/implementation/router/router.go | 121 ++++++++++++------ .../implementation/router/router_test.go | 64 ++++++++- .../router/testData/bap_receiver.yaml | 2 +- .../router/testData/bpp_receiver.yaml | 4 +- 6 files changed, 146 insertions(+), 53 deletions(-) diff --git a/pkg/plugin/definition/router.go b/pkg/plugin/definition/router.go index 21b1be6..05e2e30 100644 --- a/pkg/plugin/definition/router.go +++ b/pkg/plugin/definition/router.go @@ -7,9 +7,9 @@ import ( // Route defines the structure for the Route returned. type Route struct { - TargetType string // "url" or "msgq" or "bap" or "bpp" - PublisherID string // For message queues - URL string // For API calls + TargetType string // "url" or "msgq" or "bap" or "bpp" + PublisherID string // For message queues + URL *url.URL // For API calls } // RouterProvider initializes the a new Router instance with the given config. diff --git a/pkg/plugin/implementation/router/cmd/plugin.go b/pkg/plugin/implementation/router/cmd/plugin.go index 23816ee..556f129 100644 --- a/pkg/plugin/implementation/router/cmd/plugin.go +++ b/pkg/plugin/implementation/router/cmd/plugin.go @@ -28,4 +28,4 @@ func (rp RouterProvider) New(ctx context.Context, config map[string]string) (def } // Provider is the exported symbol that the plugin manager will look for. -var Provider definition.RouterProvider = RouterProvider{} +var Provider = RouterProvider{} diff --git a/pkg/plugin/implementation/router/router.go b/pkg/plugin/implementation/router/router.go index ae98baf..9d16767 100644 --- a/pkg/plugin/implementation/router/router.go +++ b/pkg/plugin/implementation/router/router.go @@ -33,7 +33,7 @@ type Router struct { type routingRule struct { Domain string `yaml:"domain"` Version string `yaml:"version"` - TargetType string `yaml:"targetType"` // "url", "msgq", "bpp", or "bap" + TargetType string `yaml:"targetType"` // "url", "publisher", "bpp", or "bap" Target target `yaml:"target,omitempty"` Endpoints []string `yaml:"endpoints"` } @@ -46,10 +46,10 @@ type target struct { // TargetType defines possible target destinations. const ( - targetTypeURL = "url" // Route to a specific URL - targetTypeMSGQ = "msgq" // Route to a message queue - targetTypeBPP = "bpp" // Route to a BPP endpoint - targetTypeBAP = "bap" // Route to a BAP endpoint + targetTypeURL = "url" // Route to a specific URL + targetTypePublisher = "publisher" // Route to a publisher + targetTypeBPP = "bpp" // Route to a BPP endpoint + targetTypeBAP = "bap" // Route to a BAP endpoint ) // New initializes a new Router instance with the provided configuration. @@ -71,6 +71,30 @@ func New(ctx context.Context, config *Config) (*Router, func() error, error) { return router, nil, nil } +// parseTargetURL parses a URL string into a url.URL object with strict validation +func parseTargetURL(urlStr string) (*url.URL, error) { + if urlStr == "" { + return nil, nil + } + + parsed, err := url.Parse(urlStr) + if err != nil { + return nil, fmt.Errorf("invalid URL '%s': %w", urlStr, err) + } + + // Enforce scheme requirement + if parsed.Scheme == "" { + return nil, fmt.Errorf("URL '%s' must include a scheme (http/https)", urlStr) + } + + // Optionally validate scheme is http or https + if parsed.Scheme != "https" { + return nil, fmt.Errorf("URL '%s' must use https scheme", urlStr) + } + + return parsed, nil +} + // LoadRules reads and parses routing rules from the YAML configuration file. func (r *Router) loadRules(configPath string) error { if configPath == "" { @@ -105,25 +129,33 @@ func (r *Router) loadRules(configPath string) error { for _, endpoint := range rule.Endpoints { var route *definition.Route switch rule.TargetType { - case targetTypeMSGQ: + case targetTypePublisher: route = &definition.Route{ TargetType: rule.TargetType, PublisherID: rule.Target.PublisherID, } case targetTypeURL: + parsedURL, err := parseTargetURL(rule.Target.URL) + if err != nil { + return fmt.Errorf("invalid URL in rule: %w", err) + } route = &definition.Route{ TargetType: rule.TargetType, - URL: rule.Target.URL, + URL: parsedURL, } case targetTypeBPP, targetTypeBAP: + var parsedURL *url.URL + if rule.Target.URL != "" { + parsedURL, err = parseTargetURL(rule.Target.URL) + if err != nil { + return fmt.Errorf("invalid URL in rule: %w", err) + } + } route = &definition.Route{ TargetType: rule.TargetType, - URL: rule.Target.URL, // Fallback URL if URI not provided in request + URL: parsedURL, } } - - fmt.Print(r.rules) - r.rules[rule.Domain][rule.Version][endpoint] = route } } @@ -145,15 +177,19 @@ func validateRules(rules []routingRule) error { if rule.Target.URL == "" { return fmt.Errorf("invalid rule: url is required for targetType 'url'") } - if _, err := url.ParseRequestURI(rule.Target.URL); err != nil { - return fmt.Errorf("invalid URL in rule: %w", err) + if _, err := parseTargetURL(rule.Target.URL); err != nil { + return fmt.Errorf("invalid URL - %s: %w", rule.Target.URL, err) } - case targetTypeMSGQ: + case targetTypePublisher: if rule.Target.PublisherID == "" { - return fmt.Errorf("invalid rule: publisherID is required for targetType 'msgq'") + return fmt.Errorf("invalid rule: publisherID is required for targetType 'publisher'") } case targetTypeBPP, targetTypeBAP: - // No target validation needed for bpp/bap, as they use URIs from the request body + if rule.Target.URL != "" { + if _, err := parseTargetURL(rule.Target.URL); err != nil { + return fmt.Errorf("invalid URL - %s defined in routing config for target type %s: %w", rule.Target.URL, rule.TargetType, err) + } + } continue default: return fmt.Errorf("invalid rule: unknown targetType '%s'", rule.TargetType) @@ -197,36 +233,43 @@ func (r *Router) Route(ctx context.Context, url *url.URL, body []byte) (*definit return nil, fmt.Errorf("endpoint '%s' is not supported for domain %s and version %s in routing config", endpoint, requestBody.Context.Domain, requestBody.Context.Version) } - // Handle BPP/BAP routing with request URIs switch route.TargetType { case targetTypeBPP: - uri := strings.TrimSpace(requestBody.Context.BPPURI) - target := strings.TrimSpace(route.URL) - if len(uri) != 0 { - target = uri - } - if len(target) == 0 { - return nil, fmt.Errorf("could not determine destination for endpoint '%s': neither request contained a BPP URI nor was a default URL configured in routing rules", endpoint) - } - route = &definition.Route{ - TargetType: route.TargetType, - URL: target, - } + return handleProtocolMapping(route, requestBody.Context.BPPURI, endpoint) case targetTypeBAP: - uri := strings.TrimSpace(requestBody.Context.BAPURI) - target := strings.TrimSpace(route.URL) - if len(uri) != 0 { - target = uri + return handleProtocolMapping(route, requestBody.Context.BAPURI, endpoint) + } + return route, nil +} + +// handleProtocolMapping handles both BPP and BAP routing with proper URL construction +func handleProtocolMapping(route *definition.Route, requestURI, endpoint string) (*definition.Route, error) { + uri := strings.TrimSpace(requestURI) + var targetURL *url.URL + if len(uri) != 0 { + parsedURL, err := parseTargetURL(uri) + if err != nil { + return nil, fmt.Errorf("invalid %s URI - %s in request body for %s: %w", strings.ToUpper(route.TargetType), uri, endpoint, err) } - if len(target) == 0 { - return nil, fmt.Errorf("could not determine destination for endpoint '%s': neither request contained a BAP URI nor was a default URL configured in routing rules", endpoint) + targetURL = parsedURL + } + + // If no request URI, fall back to configured URL with endpoint appended + if targetURL == nil { + if route.URL == nil { + return nil, fmt.Errorf("could not determine destination for endpoint '%s': neither request contained a %s URI nor was a default URL configured in routing rules", endpoint, strings.ToUpper(route.TargetType)) } - route = &definition.Route{ - TargetType: route.TargetType, - URL: target, + + targetURL = &url.URL{ + Scheme: route.URL.Scheme, + Host: route.URL.Host, + Path: path.Join(route.URL.Path, endpoint), } } - return route, nil + return &definition.Route{ + TargetType: targetTypeURL, + URL: targetURL, + }, nil } diff --git a/pkg/plugin/implementation/router/router_test.go b/pkg/plugin/implementation/router/router_test.go index f67b577..7937ca5 100644 --- a/pkg/plugin/implementation/router/router_test.go +++ b/pkg/plugin/implementation/router/router_test.go @@ -143,12 +143,12 @@ func TestValidateRulesSuccess(t *testing.T) { }, }, { - name: "Valid rules with msgq routing", + name: "Valid rules with publisher routing", rules: []routingRule{ { Domain: "retail", Version: "1.0.0", - TargetType: "msgq", + TargetType: "publisher", Target: target{ PublisherID: "example_topic", }, @@ -284,19 +284,64 @@ func TestValidateRulesFailure(t *testing.T) { wantErr: "invalid rule: url is required for targetType 'url'", }, { - name: "Missing topic_id for targetType: msgq", + name: "Invalid URL format for targetType: url", rules: []routingRule{ { Domain: "retail", Version: "1.0.0", - TargetType: "msgq", + TargetType: "url", + Target: target{ + URL: "htp://invalid-url.com", // Invalid scheme + }, + Endpoints: []string{"search"}, + }, + }, + wantErr: "invalid URL - htp://invalid-url.com: URL 'htp://invalid-url.com' must use https scheme", + }, + { + name: "Missing topic_id for targetType: publisher", + rules: []routingRule{ + { + Domain: "retail", + Version: "1.0.0", + TargetType: "publisher", Target: target{ // PublisherID is missing }, Endpoints: []string{"search", "select"}, }, }, - wantErr: "invalid rule: publisherID is required for targetType 'msgq'", + wantErr: "invalid rule: publisherID is required for targetType 'publisher'", + }, + { + name: "Invalid URL for BPP targetType", + rules: []routingRule{ + { + Domain: "retail", + Version: "1.0.0", + TargetType: "bpp", + Target: target{ + URL: "htp://invalid-url.com", // Invalid URL + }, + Endpoints: []string{"search"}, + }, + }, + wantErr: "invalid URL - htp://invalid-url.com defined in routing config for target type bpp", + }, + { + name: "Invalid URL for BAP targetType", + rules: []routingRule{ + { + Domain: "retail", + Version: "1.0.0", + TargetType: "bap", + Target: target{ + URL: "http://[invalid].com", // Invalid host + }, + Endpoints: []string{"search"}, + }, + }, + wantErr: "invalid URL - http://[invalid].com defined in routing config for target type bap", }, } @@ -340,7 +385,7 @@ func TestRouteSuccess(t *testing.T) { body: `{"context": {"domain": "ONDC:TRV10", "version": "2.0.0"}}`, }, { - name: "Valid domain, version, and endpoint (msgq routing)", + name: "Valid domain, version, and endpoint (publisher routing)", configFile: "bpp_receiver.yaml", url: "https://example.com/v1/ondc/search", body: `{"context": {"domain": "ONDC:TRV10", "version": "2.0.0"}}`, @@ -415,6 +460,13 @@ func TestRouteFailure(t *testing.T) { body: `{"context": {"domain": "ONDC:TRV10", "version": "2.0.0"}}`, wantErr: "could not determine destination for endpoint 'select': neither request contained a BPP URI nor was a default URL configured in routing rules", }, + { + name: "Invalid bpp_uri format in request", + configFile: "bap_caller.yaml", + url: "https://example.com/v1/ondc/select", + body: `{"context": {"domain": "ONDC:TRV10", "version": "2.0.0", "bpp_uri": "htp://invalid-url"}}`, // Invalid scheme (htp instead of http) + wantErr: "invalid BPP URI - htp://invalid-url in request body for select: URL 'htp://invalid-url' must use https scheme", + }, } for _, tt := range tests { diff --git a/pkg/plugin/implementation/router/testData/bap_receiver.yaml b/pkg/plugin/implementation/router/testData/bap_receiver.yaml index 353ecc7..af98401 100644 --- a/pkg/plugin/implementation/router/testData/bap_receiver.yaml +++ b/pkg/plugin/implementation/router/testData/bap_receiver.yaml @@ -13,7 +13,7 @@ routingRules: - on_cancel - domain: "ONDC:TRV10" version: "2.0.0" - targetType: "msgq" + targetType: "publisher" target: publisherId: "trv_topic_id1" endpoints: diff --git a/pkg/plugin/implementation/router/testData/bpp_receiver.yaml b/pkg/plugin/implementation/router/testData/bpp_receiver.yaml index 9633923..a8f668e 100644 --- a/pkg/plugin/implementation/router/testData/bpp_receiver.yaml +++ b/pkg/plugin/implementation/router/testData/bpp_receiver.yaml @@ -10,15 +10,13 @@ routingRules: - confirm - status - cancel - - domain: "ONDC:TRV10" version: "2.0.0" - targetType: "msgq" + targetType: "publisher" target: publisherId: "trv_topic_id1" endpoints: - search - - domain: "ONDC:TRV11" version: "2.0.0" targetType: "url"