Repository: ossf/package-analysis
Pull request #610 (2023-02-02 07:04:27)

move Ecosystem and RunPhase types to external API package
These types need to be accessible externally, for the detection project.

In terms of naming, I had a discussion with @calebbrown who suggested putting the public types in an `api` package. This makes it clear both internally and externally to users of the types/constants that they form part of the public interface for the project. 

We may be able to move more types into the `api` package in the future.

Signed-off-by: Max Fisher <maxfisher@google.com>


Patch:
diff --git a/cmd/analyze/main.go b/cmd/analyze/main.go
index d13c99b..7395df9 100644
--- a/cmd/analyze/main.go
+++ b/cmd/analyze/main.go
@@ -15,6 +15,7 @@ import (
 	"github.com/ossf/package-analysis/internal/staticanalysis"
 	"github.com/ossf/package-analysis/internal/utils"
 	"github.com/ossf/package-analysis/internal/worker"
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 var (
@@ -158,7 +159,7 @@ func main() {
 		return
 	}
 
-	manager := pkgecosystem.Manager(pkgecosystem.Ecosystem(*ecosystem))
+	manager := pkgecosystem.Manager(api.Ecosystem(*ecosystem))
 	if manager == nil {
 		log.Panic("Unsupported pkg manager",
 			log.Label("ecosystem", *ecosystem))
diff --git a/cmd/worker/main.go b/cmd/worker/main.go
index 930f395..ac93396 100644
--- a/cmd/worker/main.go
+++ b/cmd/worker/main.go
@@ -25,6 +25,7 @@ import (
 	"github.com/ossf/package-analysis/internal/resultstore"
 	"github.com/ossf/package-analysis/internal/sandbox"
 	"github.com/ossf/package-analysis/internal/worker"
+	"github.com/ossf/package-analysis/pkg/api"
 	"github.com/ossf/package-analysis/pkg/result"
 )
 
@@ -115,7 +116,7 @@ func handleMessage(ctx context.Context, msg *pubsub.Message, packagesBucket *blo
 		return nil
 	}
 
-	manager := pkgecosystem.Manager(pkgecosystem.Ecosystem(ecosystem))
+	manager := pkgecosystem.Manager(api.Ecosystem(ecosystem))
 	if manager == nil {
 		log.Warn("Unsupported pkg manager",
 			log.Label("ecosystem", ecosystem),
diff --git a/internal/pkgecosystem/crates.io.go b/internal/pkgecosystem/crates.io.go
index 5a1996f..327306a 100644
--- a/internal/pkgecosystem/crates.io.go
+++ b/internal/pkgecosystem/crates.io.go
@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 type cratesJSON struct {
@@ -30,11 +32,11 @@ func getCratesLatest(pkg string) (string, error) {
 }
 
 var cratesPkgManager = PkgManager{
-	ecosystem:     CratesIO,
+	ecosystem:     api.EcosystemCratesIO,
 	image:         "gcr.io/ossf-malware-analysis/crates.io",
 	command:       "/usr/local/bin/analyze.py",
 	latestVersion: getCratesLatest,
-	runPhases: []RunPhase{
-		Install,
+	runPhases: []api.RunPhase{
+		api.RunPhaseInstall,
 	},
 }
diff --git a/internal/pkgecosystem/ecosystem.go b/internal/pkgecosystem/ecosystem.go
index 2cde4d9..7a8b4c3 100644
--- a/internal/pkgecosystem/ecosystem.go
+++ b/internal/pkgecosystem/ecosystem.go
@@ -3,40 +3,24 @@ package pkgecosystem
 import (
 	"fmt"
 	"strings"
-)
-
-// Ecosystem represents an open source package ecosystem from which packages can be downloaded
-type Ecosystem string
-
-// RunPhase represents a way to 'run' a package during its usage lifecycle.
-// This is relevant to dynamic analysis.
-type RunPhase string
-
-const (
-	Import  RunPhase = "import"
-	Install RunPhase = "install"
 
-	CratesIO  Ecosystem = "crates.io"
-	NPM       Ecosystem = "npm"
-	Packagist Ecosystem = "packagist"
-	PyPI      Ecosystem = "pypi"
-	RubyGems  Ecosystem = "rubygems"
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 // PkgManager
 // Represents how packages from a common ecosystem are accessed
 type PkgManager struct {
-	ecosystem      Ecosystem
+	ecosystem      api.Ecosystem
 	image          string
 	command        string
 	latestVersion  func(string) (string, error)
 	archiveUrl     func(string, string) (string, error)
 	extractArchive func(string, string) error
-	runPhases      []RunPhase
+	runPhases      []api.RunPhase
 }
 
 var (
-	supportedPkgManagers = map[Ecosystem]*PkgManager{
+	supportedPkgManagers = map[api.Ecosystem]*PkgManager{
 		npmPkgManager.ecosystem:       &npmPkgManager,
 		pypiPkgManager.ecosystem:      &pypiPkgManager,
 		rubygemsPkgManager.ecosystem:  &rubygemsPkgManager,
@@ -45,7 +29,7 @@ var (
 	}
 )
 
-func Manager(ecosystem Ecosystem) *PkgManager {
+func Manager(ecosystem api.Ecosystem) *PkgManager {
 	return supportedPkgManagers[ecosystem]
 }
 
@@ -58,7 +42,7 @@ func (p *PkgManager) DynamicAnalysisImage() string {
 	return p.image
 }
 
-func (p *PkgManager) RunPhases() []RunPhase {
+func (p *PkgManager) RunPhases() []api.RunPhase {
 	return p.runPhases
 }
 
diff --git a/internal/pkgecosystem/npm.go b/internal/pkgecosystem/npm.go
index aa86e40..c266bae 100644
--- a/internal/pkgecosystem/npm.go
+++ b/internal/pkgecosystem/npm.go
@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/ossf/package-analysis/internal/utils"
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 // npmPackageJSON represents relevant JSON data from the NPM registry response
@@ -70,14 +71,14 @@ func getNPMArchiveURL(pkgName, version string) (string, error) {
 }
 
 var npmPkgManager = PkgManager{
-	ecosystem:      NPM,
+	ecosystem:      api.EcosystemNPM,
 	image:          "gcr.io/ossf-malware-analysis/node",
 	command:        "/usr/local/bin/analyze.js",
 	latestVersion:  getNPMLatest,
 	archiveUrl:     getNPMArchiveURL,
 	extractArchive: utils.ExtractTarGzFile,
-	runPhases: []RunPhase{
-		Install,
-		Import,
+	runPhases: []api.RunPhase{
+		api.RunPhaseInstall,
+		api.RunPhaseImport,
 	},
 }
diff --git a/internal/pkgecosystem/package.go b/internal/pkgecosystem/package.go
index bd231c3..0eca337 100644
--- a/internal/pkgecosystem/package.go
+++ b/internal/pkgecosystem/package.go
@@ -1,5 +1,7 @@
 package pkgecosystem
 
+import "github.com/ossf/package-analysis/pkg/api"
+
 type Pkg struct {
 	name    string
 	version string
@@ -15,7 +17,7 @@ func (p *Pkg) Version() string {
 	return p.version
 }
 
-func (p *Pkg) Ecosystem() Ecosystem {
+func (p *Pkg) Ecosystem() api.Ecosystem {
 	return p.manager.ecosystem
 }
 
@@ -36,7 +38,7 @@ func (p *Pkg) LocalPath() string {
 }
 
 // Command returns the analysis command for the package.
-func (p *Pkg) Command(phase RunPhase) []string {
+func (p *Pkg) Command(phase api.RunPhase) []string {
 	args := make([]string, 0)
 	args = append(args, p.manager.command)
 
diff --git a/internal/pkgecosystem/packagist.go b/internal/pkgecosystem/packagist.go
index b26d944..e82fc73 100644
--- a/internal/pkgecosystem/packagist.go
+++ b/internal/pkgecosystem/packagist.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"net/http"
 	"time"
+
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 type packagistJSON struct {
@@ -47,12 +49,12 @@ func getPackagistLatest(pkg string) (string, error) {
 }
 
 var packagistPkgManager = PkgManager{
-	ecosystem:     Packagist,
+	ecosystem:     api.EcosystemPackagist,
 	image:         "gcr.io/ossf-malware-analysis/packagist",
 	command:       "/usr/local/bin/analyze.php",
 	latestVersion: getPackagistLatest,
-	runPhases: []RunPhase{
-		Install,
-		Import,
+	runPhases: []api.RunPhase{
+		api.RunPhaseInstall,
+		api.RunPhaseImport,
 	},
 }
diff --git a/internal/pkgecosystem/pypi.go b/internal/pkgecosystem/pypi.go
index d5c5e32..7fde2b8 100644
--- a/internal/pkgecosystem/pypi.go
+++ b/internal/pkgecosystem/pypi.go
@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/ossf/package-analysis/internal/utils"
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 // pypiPackageInfoJSON represents relevant JSON data from the PyPI web API response
@@ -78,14 +79,14 @@ func getPyPIArchiveURL(pkgName, version string) (string, error) {
 }
 
 var pypiPkgManager = PkgManager{
-	ecosystem:      PyPI,
+	ecosystem:      api.EcosystemPyPI,
 	image:          "gcr.io/ossf-malware-analysis/python",
 	command:        "/usr/local/bin/analyze.py",
 	latestVersion:  getPyPILatest,
 	archiveUrl:     getPyPIArchiveURL,
 	extractArchive: utils.ExtractTarGzFile,
-	runPhases: []RunPhase{
-		Install,
-		Import,
+	runPhases: []api.RunPhase{
+		api.RunPhaseInstall,
+		api.RunPhaseImport,
 	},
 }
diff --git a/internal/pkgecosystem/rubygems.go b/internal/pkgecosystem/rubygems.go
index bb0bcb2..18716c9 100644
--- a/internal/pkgecosystem/rubygems.go
+++ b/internal/pkgecosystem/rubygems.go
@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 type rubygemsJSON struct {
@@ -28,12 +30,12 @@ func getRubyGemsLatest(pkg string) (string, error) {
 }
 
 var rubygemsPkgManager = PkgManager{
-	ecosystem:     RubyGems,
+	ecosystem:     api.EcosystemRubyGems,
 	image:         "gcr.io/ossf-malware-analysis/ruby",
 	command:       "/usr/local/bin/analyze.rb",
 	latestVersion: getRubyGemsLatest,
-	runPhases: []RunPhase{
-		Install,
-		Import,
+	runPhases: []api.RunPhase{
+		api.RunPhaseInstall,
+		api.RunPhaseImport,
 	},
 }
diff --git a/internal/worker/logging.go b/internal/worker/logging.go
index 8a89fe2..4a13340 100644
--- a/internal/worker/logging.go
+++ b/internal/worker/logging.go
@@ -4,6 +4,7 @@ import (
 	"github.com/ossf/package-analysis/internal/analysis"
 	"github.com/ossf/package-analysis/internal/log"
 	"github.com/ossf/package-analysis/internal/pkgecosystem"
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 /*
@@ -22,7 +23,7 @@ const gotRequestLogMsg = "Got request"
 // LogDynamicAnalysisError indicates some error happened while attempting to run
 // the package code, which was not caused by the package itself. This means it was
 // not possible to analyse the package properly, and the results are invalid.
-func LogDynamicAnalysisError(pkg *pkgecosystem.Pkg, errorPhase pkgecosystem.RunPhase, err error) {
+func LogDynamicAnalysisError(pkg *pkgecosystem.Pkg, errorPhase api.RunPhase, err error) {
 	log.Error(runErrorLogMsg,
 		log.Label("ecosystem", pkg.EcosystemName()),
 		log.Label("name", pkg.Name()),
@@ -34,7 +35,7 @@ func LogDynamicAnalysisError(pkg *pkgecosystem.Pkg, errorPhase pkgecosystem.RunP
 // LogDynamicAnalysisResult indicates that the package code was run successfully,
 // and what happened when it was run. This may include errors in the analysis
 // of the package, but not errors in the running itself.
-func LogDynamicAnalysisResult(pkg *pkgecosystem.Pkg, finalPhase pkgecosystem.RunPhase, finalStatus analysis.Status) {
+func LogDynamicAnalysisResult(pkg *pkgecosystem.Pkg, finalPhase api.RunPhase, finalStatus analysis.Status) {
 	ecosystem := pkg.EcosystemName()
 	name := pkg.Name()
 	version := pkg.Version()
diff --git a/internal/worker/rundynamic.go b/internal/worker/rundynamic.go
index 2d70033..7c60ad6 100644
--- a/internal/worker/rundynamic.go
+++ b/internal/worker/rundynamic.go
@@ -8,6 +8,7 @@ import (
 	"github.com/ossf/package-analysis/internal/log"
 	"github.com/ossf/package-analysis/internal/pkgecosystem"
 	"github.com/ossf/package-analysis/internal/sandbox"
+	"github.com/ossf/package-analysis/pkg/api"
 	"github.com/ossf/package-analysis/pkg/result"
 )
 
@@ -34,7 +35,7 @@ error: Any error that occurred in the runtime/sandbox infrastructure.
 This does not include errors caused by the package under analysis.
 */
 
-func RunDynamicAnalysis(pkg *pkgecosystem.Pkg, sbOpts []sandbox.Option) (result.DynamicAnalysisResults, pkgecosystem.RunPhase, analysis.Status, error) {
+func RunDynamicAnalysis(pkg *pkgecosystem.Pkg, sbOpts []sandbox.Option) (result.DynamicAnalysisResults, api.RunPhase, analysis.Status, error) {
 	results := result.DynamicAnalysisResults{
 		StraceSummary: make(result.DynamicAnalysisStraceSummary),
 		FileWrites:    make(result.DynamicAnalysisFileWrites),
@@ -48,7 +49,7 @@ func RunDynamicAnalysis(pkg *pkgecosystem.Pkg, sbOpts []sandbox.Option) (result.
 		}
 	}()
 
-	var lastRunPhase pkgecosystem.RunPhase
+	var lastRunPhase api.RunPhase
 	var lastStatus analysis.Status
 	var lastError error
 	for _, phase := range pkg.Manager().RunPhases() {
diff --git a/pkg/api/api.go b/pkg/api/api.go
new file mode 100644
index 0000000..9d65edc
--- /dev/null
+++ b/pkg/api/api.go
@@ -0,0 +1,21 @@
+package api
+
+// Defines types and some constants used in the external API for Package Analysis
+
+// Ecosystem represents an open source package ecosystem from which packages can be downloaded
+type Ecosystem string
+
+// RunPhase represents a way to 'run' a package during its usage lifecycle.
+// This is relevant to dynamic analysis.
+type RunPhase string
+
+const (
+	RunPhaseImport  RunPhase = "import"
+	RunPhaseInstall RunPhase = "install"
+
+	EcosystemCratesIO  Ecosystem = "crates.io"
+	EcosystemNPM       Ecosystem = "npm"
+	EcosystemPackagist Ecosystem = "packagist"
+	EcosystemPyPI      Ecosystem = "pypi"
+	EcosystemRubyGems  Ecosystem = "rubygems"
+)
diff --git a/pkg/result/result.go b/pkg/result/result.go
index 10f716e..133c3be 100644
--- a/pkg/result/result.go
+++ b/pkg/result/result.go
@@ -4,12 +4,12 @@ import (
 	"encoding/json"
 
 	"github.com/ossf/package-analysis/internal/analysis"
-	"github.com/ossf/package-analysis/internal/pkgecosystem"
 	"github.com/ossf/package-analysis/internal/strace"
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
-type DynamicAnalysisStraceSummary map[pkgecosystem.RunPhase]*StraceSummary
-type DynamicAnalysisFileWrites map[pkgecosystem.RunPhase]*FileWrites
+type DynamicAnalysisStraceSummary map[api.RunPhase]*StraceSummary
+type DynamicAnalysisFileWrites map[api.RunPhase]*FileWrites
 
 type DynamicAnalysisResults struct {
 	StraceSummary DynamicAnalysisStraceSummary
diff --git a/sandboxes/staticanalysis/staticanalyze.go b/sandboxes/staticanalysis/staticanalyze.go
index 6b29587..dfce03c 100644
--- a/sandboxes/staticanalysis/staticanalyze.go
+++ b/sandboxes/staticanalysis/staticanalyze.go
@@ -19,6 +19,7 @@ import (
 	"github.com/ossf/package-analysis/internal/staticanalysis/parsing/js"
 	"github.com/ossf/package-analysis/internal/utils"
 	"github.com/ossf/package-analysis/internal/worker"
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 var (
@@ -183,7 +184,7 @@ func run() (err error) {
 		return fmt.Errorf("ecosystem and package are required arguments")
 	}
 
-	manager := pkgecosystem.Manager(pkgecosystem.Ecosystem(*ecosystem))
+	manager := pkgecosystem.Manager(api.Ecosystem(*ecosystem))
 	if manager == nil {
 		return fmt.Errorf("unsupported pkg manager for ecosystem %s", *ecosystem)
 	}


Test patch:
diff --git a/internal/pkgecosystem/download_test.go b/internal/pkgecosystem/download_test.go
index 7b72ba7..8023190 100644
--- a/internal/pkgecosystem/download_test.go
+++ b/internal/pkgecosystem/download_test.go
@@ -3,6 +3,8 @@ package pkgecosystem
 import (
 	"os"
 	"testing"
+
+	"github.com/ossf/package-analysis/pkg/api"
 )
 
 type downloadTestSpec struct {
@@ -57,7 +59,7 @@ func TestDownloadNpmArchive(t *testing.T) {
 		{"pkgname=(invalid)", "fr(2t5j923)", "latest", true},
 	}
 
-	testDownload(t, tests, Manager(NPM))
+	testDownload(t, tests, Manager(api.EcosystemNPM))
 }
 
 func TestDownloadPyPIArchive(t *testing.T) {
@@ -67,7 +69,7 @@ func TestDownloadPyPIArchive(t *testing.T) {
 		{"pkgname=(invalid)", "fr(2t5j923)", "123", true},
 	}
 
-	testDownload(t, tests, Manager(PyPI))
+	testDownload(t, tests, Manager(api.EcosystemPyPI))
 }
 
 func TestDownloadToDirectory(t *testing.T) {