Przeglądaj źródła

Revert the tailscale/portmapper vendor patch

- The vendor patch is too awkward for psiphon-tunnel-core module users
- GOPATH builds and `go test` runs skip the portmapper dependency check
- Go module builds run the check and a new GitHub Actions test exercises this
Rod Hynes 9 miesięcy temu
rodzic
commit
1af8241cca

+ 8 - 1
.github/workflows/tests.yml

@@ -16,7 +16,7 @@ jobs:
       matrix:
         os: [ "ubuntu" ]
         go: [ "1.24.3" ]
-        test-type: [ "detector", "coverage", "memory", "custom-build-tags", "code-vetting" ]
+        test-type: [ "detector", "coverage", "memory", "custom-build-tags", "code-vetting", "dependency-check" ]
 
     runs-on: ${{ matrix.os }}-latest
 
@@ -170,6 +170,13 @@ jobs:
           cd ${{ github.workspace }}/go/src/github.com/Psiphon-Labs/psiphon-tunnel-core
           go vet -tags "PSIPHON_ENABLE_INPROXY PSIPHON_ENABLE_REFRACTION_NETWORKING" ./psiphon/... ./ClientLibrary/... ./ConsoleClient/... ./MobileLibrary/psi ./Server/...
 
+      - name: Build and run ConsoleClient to invoke the panic-on-fail, init-time portmapper dependency check (see psiphon/common/inproxy/portmapper.go)
+        if: ${{ matrix.test-type == 'dependency-check' }}
+        run: |
+          cd ${{ github.workspace }}/go/src/github.com/Psiphon-Labs/psiphon-tunnel-core/ConsoleClient
+          go build -a -v -tags "PSIPHON_ENABLE_INPROXY"
+          ./ConsoleClient --version
+
       # License check ignore cases:
       #
       # - github.com/Psiphon-Labs,github.com/Psiphon-Inc: Psiphon code with

+ 16 - 21
psiphon/common/inproxy/portmapper.go

@@ -144,31 +144,29 @@ func newPortMapper(
 	return p, nil
 }
 
-var portmapperDependencyVersionCheck bool
-
 func init() {
 	expectedDependencyVersion := "v1.58.2"
 
-	// portmapper.Version is a temporary vendor patch, in the dependency, to
-	// accomodate GOPATH builds which cannot use debug.ReadBuildInfo, and go
-	// tests, before Go 1.24, which don't get dependency info in the returned
-	// BuildInfo.
-	//
-	// TODO: replace temporary patch with full fork of portmapper, and remove
-	// this temporary case, if not the entire dependency version check
-	// (see "TODO: fork" in cloneProbe).
-	portmapperDependencyVersionCheck = portmapper.Version == expectedDependencyVersion
-
 	buildInfo, ok := debug.ReadBuildInfo()
-	if !ok {
+
+	// In GOPATH builds, BuildInfo is not available; in `go test` runs,
+	// BuildInfo dependency information is not available. In these case, we
+	// skip the check and assume that contemporaneous go module build runs
+	// will catch a check failure.
+	if !ok ||
+		buildInfo.Main.Path == "" ||
+		buildInfo.Main.Path == "command-line-arguments" ||
+		strings.HasSuffix(buildInfo.Path, ".test") {
 		return
 	}
+
 	for _, dep := range buildInfo.Deps {
 		if dep.Path == "tailscale.com" && dep.Version == expectedDependencyVersion {
-			portmapperDependencyVersionCheck = true
 			return
 		}
 	}
+
+	panic("portmapper dependency version check failed")
 }
 
 // cloneProbe copies the port mapping service data gather by Client.Probe from
@@ -195,17 +193,14 @@ func (p *portMapper) cloneProbe(probe *PortMappingProbe) error {
 	// probe, so the probe is idle when cloned
 	// (see Proxy.networkDiscoveryMutex).
 	//
-	// An explicit dependency version pin check is made since potential logic
-	// changes in future versions of the dependency may break the above
+	// An explicit dependency version pin check is made above, since potential
+	// logic changes in future versions of the dependency may break the above
 	// assumptions while the reflect operation might still succeed.
 	//
 	// TODO: fork the dependency to add internal support for shared probe
 	// state, trim additional tailscale dependencies, use Psiphon's custom
-	// dialer, and remove globals (see clientmetric.Metrics below).
-
-	if !portmapperDependencyVersionCheck {
-		return errors.TraceNew("dependency version check failed")
-	}
+	// dialer, remove globals (see clientmetric.Metrics below), and remove
+	// the dependency version check.
 
 	src := reflect.ValueOf(probe.client).Elem()
 	dst := reflect.ValueOf(p.client).Elem()

+ 0 - 8
vendor/tailscale.com/net/portmapper/portmapper.go

@@ -33,14 +33,6 @@ import (
 	"tailscale.com/util/clientmetric"
 )
 
-// [Psiphon]
-// Version is used to check this dependency version in GOPATH builds where
-// debug.ReadBuildInfo is not available, and go tests, before Go 1.24, which
-// don't get dependency info in the returned BuildInfo. This is currently a
-// temporary vendor patch which needs to be restored after any "go mod
-// vendor".
-const Version = "v1.58.2"
-
 // DebugKnobs contains debug configuration that can be provided when creating a
 // Client. The zero value is valid for use.
 type DebugKnobs struct {