Skip to content

Commit

Permalink
Replace several internal protobufs with Go structs (#3878)
Browse files Browse the repository at this point in the history 
A number of internal-only types are defined as protobufs. These are used
solely inside the engine code. Remove them and replace them with plain
Go structs.
  • Loading branch information
@dmjb
dmjb authored Jul 12, 2024
1 parent 638124b commit 40cbccd
Show file tree
Hide file tree
Showing 22 changed files with 365 additions and 960 deletions.
10 changes: 5 additions & 5 deletions internal/engine/eval/homoglyphs/application/homoglyphs_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/stacklok/minder/internal/engine/eval/homoglyphs/communication"
"github.com/stacklok/minder/internal/engine/eval/homoglyphs/domain"
engif "github.com/stacklok/minder/internal/engine/interfaces"
pbinternal "github.com/stacklok/minder/internal/proto"
"github.com/stacklok/minder/internal/engine/models"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)
Expand Down Expand Up @@ -75,13 +75,13 @@ func evaluateHomoglyphs(
}

//nolint:govet
prContents, ok := res.Object.(*pbinternal.PrContents)
prContents, ok := res.Object.(*models.PRContents)
if !ok {
return false, fmt.Errorf("invalid object type for homoglyphs evaluator")
}

if prContents.Pr == nil || prContents.Files == nil {
return false, fmt.Errorf("invalid prContents fields: %v, %v", prContents.Pr, prContents.Files)
if prContents.PR == nil || prContents.Files == nil {
return false, fmt.Errorf("invalid prContents fields: %v, %v", prContents.PR, prContents.Files)
}

if len(prContents.Files) == 0 {
Expand All @@ -90,7 +90,7 @@ func evaluateHomoglyphs(

// Note: This is a mandatory step to reassign certain fields in the handler.
// This is a workaround to avoid recreating the object.
reviewHandler.Hydrate(ctx, prContents.Pr)
reviewHandler.Hydrate(ctx, prContents.PR)

for _, file := range prContents.Files {
for _, line := range file.PatchLines {
Expand Down
12 changes: 6 additions & 6 deletions internal/engine/eval/trusty/actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (

"github.com/stacklok/minder/internal/constants"
"github.com/stacklok/minder/internal/engine/eval/pr_actions"
pbinternal "github.com/stacklok/minder/internal/proto"
"github.com/stacklok/minder/internal/engine/models"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)
Expand Down Expand Up @@ -194,7 +194,7 @@ type templateScoreComponent struct {
}

type dependencyAlternatives struct {
Dependency *pbinternal.Dependency
Dependency *models.Dependency

// Reason captures the reason why a package was flagged
Reasons []RuleViolationReason
Expand Down Expand Up @@ -289,11 +289,11 @@ func (sph *summaryPrHandler) generateSummary() (string, error) {
score = *alternative.trustyReply.Summary.Score
}
packageData := templatePackageData{
Ecosystem: alternative.Dependency.Ecosystem.AsString(),
Ecosystem: string(alternative.Dependency.Ecosystem),
PackageName: alternative.Dependency.Name,
TrustyURL: fmt.Sprintf(
"%s%s/%s", constants.TrustyHttpURL,
strings.ToLower(alternative.Dependency.Ecosystem.AsString()),
strings.ToLower(string(alternative.Dependency.Ecosystem)),
url.PathEscape(alternative.trustyReply.PackageName),
),
Score: score,
Expand Down Expand Up @@ -326,11 +326,11 @@ func (sph *summaryPrHandler) generateSummary() (string, error) {

altPackageData := templateAlternative{
templatePackageData: templatePackageData{
Ecosystem: alternative.Dependency.Ecosystem.AsString(),
Ecosystem: string(alternative.Dependency.Ecosystem),
PackageName: altData.PackageName,
TrustyURL: fmt.Sprintf(
"%s%s/%s", constants.TrustyHttpURL,
strings.ToLower(alternative.Dependency.Ecosystem.AsString()),
strings.ToLower(string(alternative.Dependency.Ecosystem)),
url.PathEscape(altData.PackageName),
),
Score: altData.Score,
Expand Down
6 changes: 3 additions & 3 deletions internal/engine/eval/trusty/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/go-viper/mapstructure/v2"

"github.com/stacklok/minder/internal/engine/eval/pr_actions"
pbinternal "github.com/stacklok/minder/internal/proto"
"github.com/stacklok/minder/internal/engine/models"
)

var (
Expand Down Expand Up @@ -110,8 +110,8 @@ func parseConfig(ruleCfg map[string]any) (*config, error) {
return &conf, nil
}

func (c *config) getEcosystemConfig(ecosystem pbinternal.DepEcosystem) *ecosystemConfig {
sEco := ecosystem.AsString()
func (c *config) getEcosystemConfig(ecosystem models.DependencyEcosystem) *ecosystemConfig {
sEco := string(ecosystem)
if sEco == "" {
return nil
}
Expand Down
47 changes: 32 additions & 15 deletions internal/engine/eval/trusty/trusty.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
evalerrors "github.com/stacklok/minder/internal/engine/errors"
"github.com/stacklok/minder/internal/engine/eval/pr_actions"
engif "github.com/stacklok/minder/internal/engine/interfaces"
pbinternal "github.com/stacklok/minder/internal/proto"
"github.com/stacklok/minder/internal/engine/models"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)

Expand Down Expand Up @@ -87,24 +87,24 @@ func (e *Evaluator) Eval(ctx context.Context, pol map[string]any, res *engif.Res
}

logger := zerolog.Ctx(ctx).With().
Int64("pull-number", prDependencies.Pr.Number).
Str("repo-owner", prDependencies.Pr.RepoOwner).
Str("repo-name", prDependencies.Pr.RepoName).Logger()
Int64("pull-number", prDependencies.PR.Number).
Str("repo-owner", prDependencies.PR.RepoOwner).
Str("repo-name", prDependencies.PR.RepoName).Logger()

// Parse the profile data to get the policy configuration
ruleConfig, err := parseRuleConfig(pol)
if err != nil {
return fmt.Errorf("parsing policy configuration: %w", err)
}

prSummaryHandler, err := newSummaryPrHandler(prDependencies.Pr, e.cli, e.endpoint)
prSummaryHandler, err := newSummaryPrHandler(prDependencies.PR, e.cli, e.endpoint)
if err != nil {
return fmt.Errorf("failed to create summary handler: %w", err)
}

// Classify all dependencies, tracking all that are malicious or scored low
for _, dep := range prDependencies.Deps {
depscore, err := getDependencyScore(ctx, e.client, dep)
depscore, err := getDependencyScore(ctx, e.client, &dep)
if err != nil {
logger.Error().Msgf("error fetching trusty data: %s", err)
return fmt.Errorf("getting dependency score: %w", err)
Expand Down Expand Up @@ -135,22 +135,22 @@ func (e *Evaluator) Eval(ctx context.Context, pol map[string]any, res *engif.Res
}

func getEcosystemConfig(
logger *zerolog.Logger, ruleConfig *config, dep *pbinternal.PrDependencies_ContextualDependency,
logger *zerolog.Logger, ruleConfig *config, dep models.ContextualDependency,
) *ecosystemConfig {
ecoConfig := ruleConfig.getEcosystemConfig(dep.Dep.Ecosystem)
if ecoConfig == nil {
logger.Info().
Str("dependency", dep.Dep.Name).
Str("ecosystem", dep.Dep.Ecosystem.AsString()).
Str("ecosystem", string(dep.Dep.Ecosystem)).
Msgf("no config for ecosystem, skipping")
return nil
}
return ecoConfig
}

// readPullRequestDependencies returns the dependencies found in theingestion results
func readPullRequestDependencies(res *engif.Result) (*pbinternal.PrDependencies, error) {
prdeps, ok := res.Object.(*pbinternal.PrDependencies)
// readPullRequestDependencies returns the dependencies found in the ingestion results
func readPullRequestDependencies(res *engif.Result) (*models.PRDependencies, error) {
prdeps, ok := res.Object.(*models.PRDependencies)
if !ok {
return nil, fmt.Errorf("object type incompatible with the Trusty evaluator")
}
Expand Down Expand Up @@ -224,13 +224,17 @@ func buildEvalResult(prSummary *summaryPrHandler) error {
}

func getDependencyScore(
ctx context.Context, trustyClient *trusty.Trusty, dep *pbinternal.PrDependencies_ContextualDependency,
ctx context.Context, trustyClient *trusty.Trusty, dep *models.ContextualDependency,
) (*trustytypes.Reply, error) {
trustyEcosystem, err := toTrustyEcosystem(dep.Dep.Ecosystem)
if err != nil {
return nil, err
}
// Call the Trusty API
resp, err := trustyClient.Report(ctx, &trustytypes.Dependency{
Name: dep.Dep.Name,
Version: dep.Dep.Version,
Ecosystem: trustytypes.Ecosystem(dep.Dep.Ecosystem),
Ecosystem: trustyEcosystem,
})
if err != nil {
return nil, fmt.Errorf("failed to send request: %w", err)
Expand All @@ -242,7 +246,7 @@ func getDependencyScore(
// low scores and adds them to the summary if needed
func classifyDependency(
_ context.Context, logger *zerolog.Logger, resp *trustytypes.Reply, ruleConfig *config,
prSummary *summaryPrHandler, dep *pbinternal.PrDependencies_ContextualDependency,
prSummary *summaryPrHandler, dep models.ContextualDependency,
) {
// Check all the policy violations
reasons := []RuleViolationReason{}
Expand Down Expand Up @@ -311,7 +315,7 @@ func classifyDependency(
Msgf("the dependency has lower score than threshold or is malicious, tracking")

prSummary.trackAlternatives(dependencyAlternatives{
Dependency: dep.Dep,
Dependency: &dep.Dep,
Reasons: reasons,
BlockPR: shouldBlockPR,
trustyReply: resp,
Expand Down Expand Up @@ -344,3 +348,16 @@ func readPackageDescription(resp *trustytypes.Reply) map[string]any {
}
return descr
}

func toTrustyEcosystem(ecosystem models.DependencyEcosystem) (trustytypes.Ecosystem, error) {
switch ecosystem {
case models.NPMDependency:
return trustytypes.ECOSYSTEM_NPM, nil
case models.PyPIDependency:
return trustytypes.ECOSYSTEM_PYPI, nil
case models.GoDependency:
return trustytypes.ECOSYSTEM_GO, nil
default:
return 0, fmt.Errorf("unexpected ecosystem %s", ecosystem)
}
}
48 changes: 24 additions & 24 deletions internal/engine/eval/trusty/trusty_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import (

"github.com/stacklok/minder/internal/engine/eval/pr_actions"
engif "github.com/stacklok/minder/internal/engine/interfaces"
pbinternal "github.com/stacklok/minder/internal/proto"
mock_github "github.com/stacklok/minder/internal/providers/github/mock"
"github.com/stacklok/minder/internal/engine/models"
mockgithub "github.com/stacklok/minder/internal/providers/github/mock"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)

Expand All @@ -46,14 +46,14 @@ func TestBuildEvalResult(t *testing.T) {
{"malicious-package", &summaryPrHandler{
trackedAlternatives: []dependencyAlternatives{
{
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &models.Dependency{
Ecosystem: models.PyPIDependency,
Name: "requests",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: string(models.PyPIDependency),
Summary: trustytypes.ScoreSummary{
Score: &sg,
},
Expand All @@ -76,14 +76,14 @@ func TestBuildEvalResult(t *testing.T) {
{"low-scored-package", &summaryPrHandler{
trackedAlternatives: []dependencyAlternatives{
{
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &models.Dependency{
Ecosystem: models.PyPIDependency,
Name: "requests",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: string(models.PyPIDependency),
Summary: trustytypes.ScoreSummary{
Score: &sg,
},
Expand All @@ -94,28 +94,28 @@ func TestBuildEvalResult(t *testing.T) {
{"malicious-and-low-score", &summaryPrHandler{
trackedAlternatives: []dependencyAlternatives{
{
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &models.Dependency{
Ecosystem: models.PyPIDependency,
Name: "python-oauth",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: string(models.PyPIDependency),
Summary: trustytypes.ScoreSummary{
Score: &sg,
},
},
},
{
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &models.Dependency{
Ecosystem: models.PyPIDependency,
Name: "requestts",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: string(models.PyPIDependency),
Summary: trustytypes.ScoreSummary{
Score: &sg,
},
Expand Down Expand Up @@ -195,7 +195,7 @@ func TestReadPullRequestDependencies(t *testing.T) {
sut *engif.Result
mustErr bool
}{
{name: "normal", sut: &engif.Result{Object: &pbinternal.PrDependencies{}}, mustErr: false},
{name: "normal", sut: &engif.Result{Object: &models.PRDependencies{}}, mustErr: false},
{name: "invalid-object", sut: &engif.Result{Object: context.Background()}, mustErr: true},
} {
tc := tc
Expand All @@ -213,7 +213,7 @@ func TestReadPullRequestDependencies(t *testing.T) {
}

func TestNewTrustyEvaluator(t *testing.T) {
ghProvider := mock_github.NewMockGitHub(nil)
ghProvider := mockgithub.NewMockGitHub(nil)
t.Parallel()
for _, tc := range []struct {
name string
Expand Down Expand Up @@ -243,9 +243,9 @@ func TestClassifyDependency(t *testing.T) {

ctx := context.Background()
logger := zerolog.Ctx(ctx).With().Logger()
dep := &pbinternal.PrDependencies_ContextualDependency{
Dep: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_NPM,
dep := models.ContextualDependency{
Dep: models.Dependency{
Ecosystem: models.NPMDependency,
Name: "test",
Version: "v0.0.1",
},
Expand Down Expand Up @@ -398,7 +398,7 @@ func TestBuildScoreMatrix(t *testing.T) {
{
name: "no-description",
sut: dependencyAlternatives{
Dependency: &pbinternal.Dependency{},
Dependency: &models.Dependency{},
Reasons: []RuleViolationReason{},
trustyReply: &trustytypes.Reply{
Summary: trustytypes.ScoreSummary{},
Expand All @@ -408,7 +408,7 @@ func TestBuildScoreMatrix(t *testing.T) {
{
name: "normal-response",
sut: dependencyAlternatives{
Dependency: &pbinternal.Dependency{},
Dependency: &models.Dependency{},
Reasons: []RuleViolationReason{},
trustyReply: &trustytypes.Reply{
Summary: trustytypes.ScoreSummary{
Expand All @@ -431,7 +431,7 @@ func TestBuildScoreMatrix(t *testing.T) {
{
name: "normal-response",
sut: dependencyAlternatives{
Dependency: &pbinternal.Dependency{},
Dependency: &models.Dependency{},
Reasons: []RuleViolationReason{},
trustyReply: &trustytypes.Reply{
Summary: trustytypes.ScoreSummary{
Expand All @@ -454,7 +454,7 @@ func TestBuildScoreMatrix(t *testing.T) {
{
name: "typosquatting-low",
sut: dependencyAlternatives{
Dependency: &pbinternal.Dependency{},
Dependency: &models.Dependency{},
Reasons: []RuleViolationReason{},
trustyReply: &trustytypes.Reply{
Summary: trustytypes.ScoreSummary{
Expand All @@ -469,7 +469,7 @@ func TestBuildScoreMatrix(t *testing.T) {
{
name: "typosquatting-high",
sut: dependencyAlternatives{
Dependency: &pbinternal.Dependency{},
Dependency: &models.Dependency{},
Reasons: []RuleViolationReason{},
trustyReply: &trustytypes.Reply{
Summary: trustytypes.ScoreSummary{
Expand Down
4 changes: 2 additions & 2 deletions internal/engine/eval/vulncheck/actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ import (
"github.com/google/go-github/v61/github"

"github.com/stacklok/minder/internal/engine/eval/pr_actions"
pbinternal "github.com/stacklok/minder/internal/proto"
"github.com/stacklok/minder/internal/engine/models"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)

type prStatusHandler interface {
trackVulnerableDep(
ctx context.Context,
dep *pbinternal.PrDependencies_ContextualDependency,
dep models.ContextualDependency,
vulnResp *VulnerabilityResponse,
patch patchLocatorFormatter,
) error
Expand Down
Loading

0 comments on commit 40cbccd

Please sign in to comment.