From 28d809469ccb3155b0bd77d456f34faddb72b678 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 19 Dec 2024 13:16:26 +0100 Subject: [PATCH] chore: test with go1.24-rc.1 (#475) --- .github/workflows/validate.yml | 2 +- _integration-tests/go.mod | 2 +- _integration-tests/tests/os/lfi.go | 4 ++ internal/cmd/server.go | 21 ++++++-- internal/jobserver/pkgs/resolve.go | 15 ++++-- internal/toolexec/aspect/linkdeps/linkdeps.go | 2 +- internal/toolexec/aspect/oncompile.go | 50 +++++++++++-------- internal/toolexec/aspect/onlink.go | 1 + internal/toolexec/aspect/resolve.go | 8 +++ 9 files changed, 72 insertions(+), 33 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index e3f331b2..7c930688 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -267,7 +267,7 @@ jobs: - macos - ubuntu - windows - go-version: [oldstable, stable] + go-version: [oldstable, stable, '~1.24.0-rc.1'] build-mode: [DRIVER] include: # Alternate build modes (only on ubuntu, latest go; to save CI time) diff --git a/_integration-tests/go.mod b/_integration-tests/go.mod index 73ab04a0..05450f81 100644 --- a/_integration-tests/go.mod +++ b/_integration-tests/go.mod @@ -7,6 +7,7 @@ replace github.com/DataDog/orchestrion => ../ require ( cloud.google.com/go/pubsub v1.45.1 github.com/99designs/gqlgen v0.17.57 + github.com/DataDog/go-libddwaf/v3 v3.5.1 github.com/DataDog/orchestrion v1.0.1 github.com/IBM/sarama v1.43.3 github.com/Shopify/sarama v1.38.1 @@ -87,7 +88,6 @@ require ( github.com/DataDog/datadog-agent/pkg/util/log v0.59.1 // indirect github.com/DataDog/datadog-agent/pkg/util/scrubber v0.59.1 // indirect github.com/DataDog/datadog-go/v5 v5.5.0 // indirect - github.com/DataDog/go-libddwaf/v3 v3.5.1 // indirect github.com/DataDog/go-runtime-metrics-internal v0.0.3 // indirect github.com/DataDog/go-sqllexer v0.0.17 // indirect github.com/DataDog/go-tuf v1.1.0-0.5.2 // indirect diff --git a/_integration-tests/tests/os/lfi.go b/_integration-tests/tests/os/lfi.go index 10ba7222..db6aa101 100644 --- a/_integration-tests/tests/os/lfi.go +++ b/_integration-tests/tests/os/lfi.go @@ -18,6 +18,7 @@ import ( "datadoghq.dev/orchestrion/_integration-tests/utils" "datadoghq.dev/orchestrion/_integration-tests/validator/trace" + waf "github.com/DataDog/go-libddwaf/v3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/DataDog/dd-trace-go.v1/appsec/events" @@ -33,6 +34,9 @@ func (tc *TestCase) Setup(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("appsec does not support Windows") } + if ok, err := waf.Health(); !ok { + t.Skip("WAF is not available:", err) + } t.Setenv("DD_APPSEC_RULES", "../testdata/rasp-only-rules.json") t.Setenv("DD_APPSEC_ENABLED", "true") diff --git a/internal/cmd/server.go b/internal/cmd/server.go index 78461d82..d78beb81 100644 --- a/internal/cmd/server.go +++ b/internal/cmd/server.go @@ -13,10 +13,12 @@ import ( "time" "github.com/DataDog/orchestrion/internal/filelock" + "github.com/DataDog/orchestrion/internal/goflags" "github.com/DataDog/orchestrion/internal/jobserver" "github.com/DataDog/orchestrion/internal/jobserver/client" "github.com/DataDog/orchestrion/internal/log" "github.com/fsnotify/fsnotify" + "gopkg.in/yaml.v3" "github.com/urfave/cli/v2" ) @@ -32,18 +34,31 @@ var Server = &cli.Command{ }, &cli.IntFlag{ Name: "port", - Usage: "Choose a port to listen on", + Usage: "Choose a port to listen on.", Value: -1, DefaultText: "random", }, &cli.DurationFlag{ Name: "inactivity-timeout", - Usage: "Automatically shut down after a period without any connected client", + Usage: "Automatically shut down after a period without any connected client.", Value: time.Minute, }, &cli.BoolFlag{ Name: "nats-logging", - Usage: "Enable NATS server logging", + Usage: "Enable NATS server logging.", + }, + &cli.StringFlag{ + Name: "build-flags", + Usage: "Specify the 'go build' flags to use when resolving packages. This is specified as a YAML array and must start with a valid go subcommand (e.g, 'build').", + DefaultText: "Looked up the process hierarchy", + Action: func(_ *cli.Context, val string) error { + var args []string + if err := yaml.Unmarshal([]byte(val), &args); err != nil { + return cli.Exit(fmt.Errorf("invalid -build-flags value: %w", err), 2) + } + goflags.SetFlags(".", args) + return nil + }, }, }, Hidden: true, diff --git a/internal/jobserver/pkgs/resolve.go b/internal/jobserver/pkgs/resolve.go index 8745b502..5ba510d8 100644 --- a/internal/jobserver/pkgs/resolve.go +++ b/internal/jobserver/pkgs/resolve.go @@ -117,7 +117,7 @@ func (s *service) resolve(req *ResolveRequest) (ResolveResponse, error) { } resp, err := s.resolved.Load(reqHash, func() (ResolveResponse, error) { - log.Tracef("[JOBSERVER] pkgs.Resolve(%s in %s)\n", req.Pattern, req.Dir) + log.Debugf("[JOBSERVER] pkgs.Resolve(%s in %s)\n", req.Pattern, req.Dir) env := req.Env if req.toolexecImportpath != "" { @@ -152,14 +152,16 @@ func (s *service) resolve(req *ResolveRequest) (ResolveResponse, error) { Mode: // We need the export file (the whole point of the resolution) packages.NeedExportFile | - // We want to also resolve transitive dependencies, so we need Deps & Imports - packages.NeedDeps | packages.NeedImports | + // We want to also resolve transitive dependencies, so we need Deps & Imports. We also + // need CompiledGoFiles in order to see imports possibly added by the toolchain (cgo, + // cover, etc...) + packages.NeedCompiledGoFiles | packages.NeedDeps | packages.NeedImports | // Finally, we need the resolved package import path packages.NeedName, Dir: req.Dir, Env: env, BuildFlags: buildFlags, - Logf: func(format string, args ...any) { log.Infof("[JOBSERVER] packages.Load -- "+format+"\n", args...) }, + Logf: func(format string, args ...any) { log.Tracef("[JOBSERVER] packages.Load -- "+format+"\n", args...) }, }, req.Pattern, ) @@ -167,6 +169,9 @@ func (s *service) resolve(req *ResolveRequest) (ResolveResponse, error) { log.Errorf("[JOBSERVER] pkgs.Resolve(%s) failed: %v\n", req.Pattern, err) return nil, err } + if len(pkgs) == 0 { + return nil, fmt.Errorf("no packages returned for pattern: %q", req.Pattern) + } resp := make(ResolveResponse) var errs error @@ -179,7 +184,7 @@ func (s *service) resolve(req *ResolveRequest) (ResolveResponse, error) { return nil, errs } - log.Tracef("[JOBSERVER] pkgs.Resolve(%s) result: %#v\n", req.Pattern, resp) + log.Debugf("[JOBSERVER] pkgs.Resolve(%s) result: %#v\n", req.Pattern, resp) return resp, nil }) if err != nil { diff --git a/internal/toolexec/aspect/linkdeps/linkdeps.go b/internal/toolexec/aspect/linkdeps/linkdeps.go index 6670fd5b..5cb2f9e2 100644 --- a/internal/toolexec/aspect/linkdeps/linkdeps.go +++ b/internal/toolexec/aspect/linkdeps/linkdeps.go @@ -47,7 +47,7 @@ func FromImportConfig(importcfg *importcfg.ImportConfig) (LinkDeps, error) { return LinkDeps{}, fmt.Errorf("reading %s from %s=%s: %w", Filename, importPath, archivePath, err) } - for _, dep := range ld.Dependencies() { + for dep := range ld.deps { if _, satisfied := importcfg.PackageFile[dep]; satisfied { // This transitive link-time dependency is already satisfied at // compile-time, so we don't need to carry it over. diff --git a/internal/toolexec/aspect/oncompile.go b/internal/toolexec/aspect/oncompile.go index 82ea286d..7911eebb 100644 --- a/internal/toolexec/aspect/oncompile.go +++ b/internal/toolexec/aspect/oncompile.go @@ -183,33 +183,39 @@ func (w Weaver) OnCompile(cmd *proxy.CompileCommand) (err error) { log.Debugf("Recording synthetic dependency: %q => %v\n", depImportPath, kind) linkDeps.Add(depImportPath) - if kind == typed.ImportStatement { - // Imported packages need to be provided in the compilation's importcfg file - deps, err := resolvePackageFiles(depImportPath, cmd.WorkDir) + if kind != typed.ImportStatement { + // We cannot attempt to resolve link-time dependencies (relocation targets), as these are + // typically used to avoid creating dependency cycles. Corrollary to this, the `link.deps` + // file will not contain transitive closures for these packages, so we need to resolve these + // at link-time. + continue + } + + // Imported packages need to be provided in the compilation's importcfg file + deps, err := resolvePackageFiles(depImportPath, cmd.WorkDir) + if err != nil { + return fmt.Errorf("resolving woven dependency on %s: %w", depImportPath, err) + } + for dep, archive := range deps { + deps, err := linkdeps.FromArchive(archive) if err != nil { - return fmt.Errorf("resolving woven dependency on %s: %w", depImportPath, err) + return fmt.Errorf("reading %s from %s[%s]: %w", linkdeps.Filename, dep, archive, err) } - for dep, archive := range deps { - deps, err := linkdeps.FromArchive(archive) - if err != nil { - return fmt.Errorf("reading %s from %s[%s]: %w", linkdeps.Filename, dep, archive, err) - } - log.Debugf("Processing %s dependencies from %s...\n", linkdeps.Filename, dep) - for _, tDep := range deps.Dependencies() { - if _, found := imports.PackageFile[tDep]; !found { - log.Debugf("Copying transitive %s dependency on %q inherited from %q via %q\n", linkdeps.Filename, tDep, depImportPath, dep) - linkDeps.Add(tDep) - } + log.Debugf("Processing %s dependencies from %s...\n", linkdeps.Filename, dep) + for _, tDep := range deps.Dependencies() { + if _, found := imports.PackageFile[tDep]; !found { + log.Debugf("Copying transitive %s dependency on %q inherited from %q via %q\n", linkdeps.Filename, tDep, depImportPath, dep) + linkDeps.Add(tDep) } + } - if _, ok := imports.PackageFile[dep]; ok { - // Already part of natural dependencies, nothing to do... - continue - } - log.Debugf("Recording transitive dependency of %q: %q => %q\n", depImportPath, dep, archive) - imports.PackageFile[dep] = archive - regUpdated = true + if _, ok := imports.PackageFile[dep]; ok { + // Already part of natural dependencies, nothing to do... + continue } + log.Debugf("Recording transitive dependency of %q: %q => %q\n", depImportPath, dep, archive) + imports.PackageFile[dep] = archive + regUpdated = true } } diff --git a/internal/toolexec/aspect/onlink.go b/internal/toolexec/aspect/onlink.go index 412fbb97..8816a46b 100644 --- a/internal/toolexec/aspect/onlink.go +++ b/internal/toolexec/aspect/onlink.go @@ -38,6 +38,7 @@ func (Weaver) OnLink(cmd *proxy.LinkCommand) error { continue } + log.Tracef("Resolving %s dependency on %q...\n", linkdeps.Filename, depPath) deps, err := resolvePackageFiles(depPath, cmd.WorkDir) if err != nil { return fmt.Errorf("resolving %q: %w", depPath, err) diff --git a/internal/toolexec/aspect/resolve.go b/internal/toolexec/aspect/resolve.go index 93717756..2834e9ef 100644 --- a/internal/toolexec/aspect/resolve.go +++ b/internal/toolexec/aspect/resolve.go @@ -45,10 +45,18 @@ func resolvePackageFiles(importPath string, workDir string) (map[string]string, } // Check for missing archives... + var found bool for ip, arch := range archives { if arch == "" { return nil, fmt.Errorf("failed to resolve archive for %q", ip) } + if ip == importPath { + found = true + } + } + + if !found { + return nil, fmt.Errorf("resolution did not include requested package %q", importPath) } return archives, nil