Skip to content

Commit

Permalink
feat: allow go1.21 builds (#1424)
Browse files Browse the repository at this point in the history
We can ~safely compile our net/http and crypto/tls forks with a later
version of Go, because they use public packages in the standard library,
which should be covered by the Go 1.x stability guarantee. Whether that
is 100% safe, I don't know, but I suppose we should be fine unless
there's really something subtle. (We will continue building our packages
with the correct version of Go, but I also understand how distribution
maintainers have other needs.)

The real price to pay for compiling with go1.21 is disabling Psiphon.
There isn't much we can do here. It's not possible to have a build
configuration with fixed flags that disable GQUIC for Psiphon, so I have
two choices (a) either ooniprobe does not build for go1.21 because of
Psiphon and I need to tell people to apply a flag; or (b) when you
compile with go1.21, everything is WAI but for Psiphon. I chose the
latter in this pull request.

Note that I originally tried to make oohttp and oocrypto optional.
However, it turns out making oohttp optional and using net/http instead
cripples ooniprobe entirely. This is why I concluded that maybe
compiling go1.20 code using the go1.21 compiler and stdlib is not that
bad, considering that our stdlib forks do not (obviously) depend on
internals.

Part of ooni/probe#2556; closes
ooni/probe#2548.
  • Loading branch information
bassosimone authored Dec 13, 2023
1 parent 5a1b93b commit f8c6189
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 11 deletions.
24 changes: 24 additions & 0 deletions .github/workflows/go1.21.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Runs the whole test suite using go1.21
name: alltests-go1.21
on:
pull_request:
push:
branches:
- "release/**"
- "fullbuild"
- "alltestsbuild"

jobs:
test:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v3

- uses: magnetikonline/action-golang-cache@v4
with:
go-version: ~1.21
cache-key-suffix: "-alltests-go1.21"

# We cannot run buildtool tests using an unexpected version of Go because the
# tests check whether we're using the expected version of Go 😂😂😂😂.
- run: go test -race -tags shaping $(go list ./...|grep -v 'internal/cmd/buildtool')
7 changes: 4 additions & 3 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ Debian/Ubuntu. Once `ooniprobe` is installed, refer to the

## Developer instructions

This repository requires _exactly_ the Go version mentioned by the
[GOVERSION](GOVERSION) file (i.e., go1.20.12). Using a different version of
Go _may_ work as intended but is not recommended: we depend
This repository _should really_ use the Go version mentioned by the
[GOVERSION](GOVERSION) file (i.e., go1.20.12). Using a later version of
Go _should_ work as intended. Using an older version of Go is
_definitely not recommended_ and _may not even compile_. Here's why: we rely
on packages forked from the standard library; so, it is
more robust to use the same version of Go from which
we forked those packages from.
Expand Down
4 changes: 4 additions & 0 deletions internal/feature/psiphonfeat/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Package psiphonfeat implements the psiphon feature. When it is possible
// to enable this feature, we are able to start psiphon tunnels. Otherwise, it's
// not possible to start psiphon tunnels.
package psiphonfeat
31 changes: 31 additions & 0 deletions internal/feature/psiphonfeat/psiphon_enabled.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//go:build !go1.21 && !ooni_feature_disable_psiphon

package psiphonfeat

import (
"context"

"github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
)

// Enabled indicates whether this feature is enabled.
const Enabled = true

// Start attempts to start the Psiphon tunnel and returns either a Tunnel or an error.
func Start(ctx context.Context, config []byte, workdir string) (Tunnel, error) {
tun, err := clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{
DataRootDirectory: &workdir}, nil, nil)
if err != nil {
return nil, err
}
return &tunnel{tun}, nil
}

type tunnel struct {
*clientlib.PsiphonTunnel
}

// GetSOCKSProxyPort implements Tunnel.
func (t *tunnel) GetSOCKSProxyPort() int {
return t.SOCKSProxyPort
}
13 changes: 13 additions & 0 deletions internal/feature/psiphonfeat/psiphon_otherwise.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//go:build go1.21 || ooni_feature_disable_psiphon

package psiphonfeat

import "context"

// Enabled indicates whether this feature is enabled.
const Enabled = false

// Start attempts to start the Psiphon tunnel and returns either a Tunnel or an error.
func Start(ctx context.Context, config []byte, workdir string) (Tunnel, error) {
return nil, ErrFeatureNotEnabled
}
15 changes: 15 additions & 0 deletions internal/feature/psiphonfeat/tunnel.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package psiphonfeat

import "errors"

// Tunnel is the interface implementing the Psiphon tunnel.
type Tunnel interface {
// Stop stops a running Psiphon tunnel.
Stop()

// GetSOCKSProxyPort returns the SOCKS5 port used by the tunnel.
GetSOCKSProxyPort() int
}

// ErrFeatureNotEnabled indicates that the Psiphon feature is not enabled in this build.
var ErrFeatureNotEnabled = errors.New("psiphonfeat: not enabled")
11 changes: 5 additions & 6 deletions internal/tunnel/psiphon.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ import (
"path/filepath"
"time"

"github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
"github.com/ooni/probe-cli/v3/internal/feature/psiphonfeat"
)

// mockableStartPsiphon allows us to test for psiphon startup failures.
var mockableStartPsiphon = func(
ctx context.Context, config []byte, workdir string) (*clientlib.PsiphonTunnel, error) {
return clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{
DataRootDirectory: &workdir}, nil, nil)
ctx context.Context, config []byte, workdir string) (psiphonfeat.Tunnel, error) {
return psiphonfeat.Start(ctx, config, workdir)
}

// psiphonTunnel is a psiphon tunnel
Expand All @@ -24,7 +23,7 @@ type psiphonTunnel struct {
bootstrapTime time.Duration

// tunnel is the underlying psiphon tunnel
tunnel *clientlib.PsiphonTunnel
tunnel psiphonfeat.Tunnel
}

// psiphonMakeWorkingDir creates the working directory
Expand Down Expand Up @@ -81,7 +80,7 @@ func (t *psiphonTunnel) SOCKS5ProxyURL() *url.URL {
return &url.URL{
Scheme: "socks5",
Host: net.JoinHostPort(
"127.0.0.1", fmt.Sprintf("%d", t.tunnel.SOCKSProxyPort)),
"127.0.0.1", fmt.Sprintf("%d", t.tunnel.GetSOCKSProxyPort())),
}
}

Expand Down
4 changes: 4 additions & 0 deletions internal/tunnel/psiphon_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ import (

"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/feature/psiphonfeat"
"github.com/ooni/probe-cli/v3/internal/tunnel"
)

func TestPsiphonStartStop(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
if !psiphonfeat.Enabled {
t.Skip("psiphon feature not enabled")
}
tunnelDir, err := ioutil.TempDir("testdata", "psiphon")
if err != nil {
t.Fatal(err)
Expand Down
4 changes: 2 additions & 2 deletions internal/tunnel/psiphon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"os"
"testing"

"github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
"github.com/ooni/probe-cli/v3/internal/feature/psiphonfeat"
)

func TestPsiphonWithCancelledContext(t *testing.T) {
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestPsiphonStartFailure(t *testing.T) {
mockableStartPsiphon = oldStartPsiphon
}()
mockableStartPsiphon = func(ctx context.Context, config []byte,
workdir string) (*clientlib.PsiphonTunnel, error) {
workdir string) (psiphonfeat.Tunnel, error) {
return nil, expected
}
tunnel, _, err := psiphonStart(context.Background(), &Config{
Expand Down

0 comments on commit f8c6189

Please sign in to comment.