diff --git a/weed/command/volume.go b/weed/command/volume.go index 91b95cd33..73dc0ec80 100644 --- a/weed/command/volume.go +++ b/weed/command/volume.go @@ -347,6 +347,7 @@ func (v VolumeServerOptions) startVolumeServer(volumeFolders, maxVolumeCounts, v // volume server. One identity truth across VS, block service, control // bridge, and runtime ownership. blockService.SetServerID(volumeServerId) + blockService.SetAdvertisedIP(*v.ip) // CP13-2: routable IP, not opaque ID volumeServer.SetBlockService(blockService) } diff --git a/weed/server/qa_block_soak_test.go b/weed/server/qa_block_soak_test.go index b1df0e659..dcec41653 100644 --- a/weed/server/qa_block_soak_test.go +++ b/weed/server/qa_block_soak_test.go @@ -84,6 +84,7 @@ func newSoakSetup(t *testing.T) *soakSetup { blockDir: filepath.Join(dir, "vs1_9333"), listenAddr: "127.0.0.1:3260", localServerID: "vs1:9333", + advertisedIP: "127.0.0.1", v2Bridge: v2bridge.NewControlBridge(), v2Orchestrator: engine.NewRecoveryOrchestrator(), replStates: make(map[string]*volReplState), diff --git a/weed/server/volume_server_block.go b/weed/server/volume_server_block.go index 870083248..3aa8a5820 100644 --- a/weed/server/volume_server_block.go +++ b/weed/server/volume_server_block.go @@ -59,9 +59,14 @@ type BlockService struct { lastAssignMu sync.RWMutex lastAssign map[string]lastAppliedAssignment // localServerID: stable identity for this volume server. - // INTERIM: uses listenAddr (transport-shaped). Should be replaced - // with a registry-assigned stable server ID in a later hardening pass. + // May be an opaque string (from -id flag) or ip:port (default fallback). + // NOT guaranteed to be a routable address — do not use for transport endpoints. localServerID string + + // advertisedIP: routable IP for this volume server (from -ip flag or auto-detected). + // Used by CP13-2 to canonicalize wildcard-bind replica listener addresses to + // routable ip:port. This is always a real IP, never an opaque identity string. + advertisedIP string } // V2Orchestrator returns the V2 engine orchestrator for inspection/testing. @@ -70,12 +75,18 @@ func (bs *BlockService) V2Orchestrator() *engine.RecoveryOrchestrator { } // SetServerID sets the stable server identity for V2 control semantics. -// Should be called with the gRPC address that the master knows this VS by, -// replacing the interim listenAddr-based identity. +// This may be an opaque string (from -id flag) — not guaranteed routable. func (bs *BlockService) SetServerID(id string) { bs.localServerID = id } +// SetAdvertisedIP sets the routable IP for replica endpoint canonicalization. +// This must be a real IP address (from -ip flag or auto-detected), never an +// opaque server identity. Called at startup from volume.go. +func (bs *BlockService) SetAdvertisedIP(ip string) { + bs.advertisedIP = ip +} + // WireStateChangeNotify sets up shipper state change callbacks on all // registered volumes so that degradation/recovery triggers an immediate // heartbeat via the provided channel. Non-blocking send (buffered chan 1). @@ -525,16 +536,19 @@ func (bs *BlockService) setupPrimaryReplicationMulti(path string, addrs []blockv // setupReplicaReceiver starts the replica WAL receiver. func (bs *BlockService) setupReplicaReceiver(path, dataAddr, ctrlAddr string) { - // CP13-2: Pass the canonical host from localServerID so wildcard-bind - // listeners resolve to the authoritative routable IP, not outbound-IP fallback. - advertisedHost := bs.localServerID - if idx := strings.LastIndex(advertisedHost, ":"); idx >= 0 { - advertisedHost = advertisedHost[:idx] - } + // CP13-2: Pass the routable advertisedIP (from -ip flag, NOT from -id/serverID) + // so wildcard-bind listeners resolve to a real IP, not an opaque identity string. var canonDataAddr, canonCtrlAddr string + advIP := bs.advertisedIP if err := bs.blockStore.WithVolume(path, func(vol *blockvol.BlockVol) error { - if err := vol.StartReplicaReceiver(dataAddr, ctrlAddr, advertisedHost); err != nil { - return err + if advIP != "" { + if err := vol.StartReplicaReceiver(dataAddr, ctrlAddr, advIP); err != nil { + return err + } + } else { + if err := vol.StartReplicaReceiver(dataAddr, ctrlAddr); err != nil { + return err + } } // Read back canonical addresses from the receiver. if vol.ReplicaReceiverAddr() != nil { diff --git a/weed/storage/blockvol/sync_all_bug_test.go b/weed/storage/blockvol/sync_all_bug_test.go index 184da06b7..372b7bdc1 100644 --- a/weed/storage/blockvol/sync_all_bug_test.go +++ b/weed/storage/blockvol/sync_all_bug_test.go @@ -104,6 +104,29 @@ func TestBug3_ReplicaAddr_MustBeIPPort_WildcardBind(t *testing.T) { t.Fatalf("CtrlAddr %q does not use advertisedHost 10.0.0.99", info.CtrlAddr) } }) + + // Case 4: opaque non-IP advertisedHost must NOT produce a usable routable address. + // CP13-2: this proves that passing an opaque server identity (e.g., from -id flag) + // as the advertised host would produce a non-routable address. The production path + // uses advertisedIP (from -ip flag), not localServerID, to prevent this. + t.Run("opaque_identity_not_routable", func(t *testing.T) { + vol3 := createTestVol(t) + defer vol3.Close() + // Pass an opaque identity string — NOT an IP address. + if err := vol3.StartReplicaReceiver(":0", ":0", "my-custom-server-id"); err != nil { + t.Fatal(err) + } + info := vol3.ReplicaReceiverAddr() + if info == nil { + t.Fatal("ReplicaReceiverAddr() returned nil") + } + // The address will be "my-custom-server-id:port" — NOT parseable as a routable IP. + ip := net.ParseIP(strings.Split(info.DataAddr, ":")[0]) + if ip != nil { + t.Fatalf("opaque identity should NOT produce a parseable IP, got %s", info.DataAddr) + } + t.Logf("correctly non-routable: %s (production uses advertisedIP, not serverID)", info.DataAddr) + }) } // --- Bug 2: LSN gap after shipper degradation ---