Rewrite force replication's verify target for pre-1.22.2 servers - #259
Open
JayChung0258 wants to merge 1 commit into
Open
Rewrite force replication's verify target for pre-1.22.2 servers#259JayChung0258 wants to merge 1 commit into
JayChung0258 wants to merge 1 commit into
Conversation
Temporal servers older than v1.22.2 ignore TargetClusterName in the VerifyReplicationTasks activity and dial TargetClusterEndpoint verbatim. The caller fills that with the remote cluster's own address, which is generally not routable from the local cluster, so verification never completes and the migration stalls while replication itself succeeds. Add an opt-in translator that rewrites that one field to this proxy's replicationEndpoint, which is the same correction already applied to FrontendAddress in AddOrUpdateRemoteCluster. That one is a typed proto field; this one rides inside a workflow-args payload. TargetClusterName is left alone, so servers from v1.22.2 on are unaffected whether or not the flag stays set. Scoped narrowly: inbound only, StartWorkflowExecution only, and only when the workflow type is force-replication. Enabled by a new forceReplicationEndpointOverride key, which also requires replicationEndpoint to be set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds an opt-in translator that rewrites
ForceReplicationParams.TargetClusterEndpointin theStartWorkflowExecutionrequest that starts the force replication workflow, pointing it at this proxy'sreplicationEndpoint.Why
Temporal servers older than v1.22.2 ignore
TargetClusterNamein theVerifyReplicationTasksactivity and dialTargetClusterEndpointverbatim. The caller fills that with the remote cluster's own address, which is generally not routable from the local cluster. Replication succeeds, verification never completes, and the migration sits in progress while the activity retries for a week.The routable address is the one this proxy already writes into the local cluster's registry when it rewrites
FrontendAddressduringAddOrUpdateRemoteCluster. Same correction, different carrier: that one is a typed proto field, this one rides inside a workflow-args payload.TargetClusterNameis left untouched, so servers from v1.22.2 on prefer the name and ignore this field whether or not the flag stays set.Config
Off by default. The translator is only constructed when the flag is true and
replicationEndpointis non-empty. Belongs on the self-hosted side, not on a cloud-side proxy.Scope
Inbound only,
StartWorkflowExecutiononly, and only when the workflow type isforce-replication. It type-asserts before anything else, which keeps it inert on the stream path whereMatchMethodis not consulted. Only the one JSON key is replaced; every other param and the payload metadata are preserved.Testing
Unit tests cover the rewrite, other workflow types, other RPCs, near-miss method names, nil input, empty payloads, a nil payload element, wrong encoding, malformed JSON, a missing key, idempotency, and that other params survive.
go build ./...,go test -race -tags test_dep ./interceptor/... ./config/...andmake lintare all clean.Known gap: no e2e case.
StartWorkflowExecutionis stubbed toPermissionDeniedin the echo fixture, so one would require implementing it there first. Follow-up.