Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace several internal protobufs with Go structs #3878

Merged
merged 2 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
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
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