Copilot commented on code in PR #3419:
URL: https://github.com/apache/dubbo-go/pull/3419#discussion_r3417590905


##########
cluster/router/script/router.go:
##########
@@ -66,6 +66,46 @@ func parseRoute(routeContent string) (*global.RouterConfig, 
error) {
        return routerConfig, nil
 }
 
+// SetStaticConfig applies a RouterConfig directly, bypassing YAML parsing.
+// Static and dynamic rules are not merged: later Process updates replace the
+// current state built here.
+func (s *ScriptRouter) SetStaticConfig(cfg *global.RouterConfig) {
+       if cfg == nil || cfg.Scope != constant.RouterScopeApplication || 
cfg.ScriptType == "" || cfg.Script == "" {
+               return
+       }
+
+       s.mu.Lock()
+       defer s.mu.Unlock()
+
+       if s.enabled && s.scriptType != "" {
+               in, err := ins.GetInstances(s.scriptType)
+               if err != nil {
+                       logger.Errorf("[Router][Script] GetInstances failed to 
destroy static config, error=%v", err)
+               } else {
+                       in.Destroy(s.rawScript)
+               }
+       }
+
+       s.scriptType = cfg.ScriptType
+       s.rawScript = cfg.Script
+       s.enabled = cfg.Enabled == nil || *cfg.Enabled
+       if !s.enabled {
+               logger.Infof("[Router][Script] static config is disabled, 
script=%s", cfg.Script)
+               return
+       }

Review Comment:
   Logging the full script body when static config is disabled can flood logs 
and may expose rule contents. Prefer logging identifying metadata (key/type) 
instead of the entire script.



##########
cluster/router/script/router.go:
##########
@@ -66,6 +66,46 @@ func parseRoute(routeContent string) (*global.RouterConfig, 
error) {
        return routerConfig, nil
 }
 
+// SetStaticConfig applies a RouterConfig directly, bypassing YAML parsing.
+// Static and dynamic rules are not merged: later Process updates replace the
+// current state built here.
+func (s *ScriptRouter) SetStaticConfig(cfg *global.RouterConfig) {
+       if cfg == nil || cfg.Scope != constant.RouterScopeApplication || 
cfg.ScriptType == "" || cfg.Script == "" {
+               return
+       }

Review Comment:
   SetStaticConfig currently applies a script rule even when cfg.Key is empty. 
Dynamic Process() rejects configs without Key (applicationName field), and 
RouterConfig marks Key as required; accepting empty Key here makes it easier 
for malformed static configs to be silently applied to all routes.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to