Always all System Callback URL #8308
Conversation
There was a problem hiding this comment.
I don't think you need to worry about whether to allow insecure connections for temporal://system since those internally-routed requests will always use our internal local frontend client which should have already been configured with the correct secure/insecure setting.
components/callbacks/config.go
Outdated
| URLs are checked against each in order when starting a workflow with attached callbacks and only need to match one to pass validation. URL: "temporal://system" is always valid. | ||
| Default is no address rules. Any invalid entries are ignored. Each entry is a map with possible values: |
There was a problem hiding this comment.
Could you just merge the two statements saying that tempral://system is always valid and the default is no address rules just to clarify a bit? Also mention that this config is required for external endpoint targets.
service/frontend/workflow_handler.go
Outdated
| if cfg.MatchHost(u.Host) { | ||
| if !cfg.CheckSchemeSecureLevel(u.Scheme) { |
There was a problem hiding this comment.
I would change this signature to cfg.Allow(url) error.
This will allow you to match temporal://system properly because you need to verify both the scheme and the host together and the regexp based validation doesn't work here.
There was a problem hiding this comment.
awesome, I was thinking about going this direction.
What changed?
This PR allows the
temporal://systemcallback URL at all times. If this URL is passed in as a Nexus Callback URL, it indicates the Callback will be routed within the Temporal System.Why?
This is one step in allows Self-Hosted Temporal Servers to support nexus with zero dynamic configuration required.
See this issue:
How did you test it?