Skip to content

Commit

Permalink
Move messages only used to generate Go structs out of minder.proto (#…
Browse files Browse the repository at this point in the history
…3830)

* Move protobuf messages only used internal to an internal proto file

Adds a new proto file under the internal/ directory tree and modifies
the `buf.work.yaml` file so that the new internal/proto directory is a
module.

Resolves: #3829

* Flip Minder code to use the Go structs from the internal proto file

Uses the newly added Go structs that are generated out of the
internal.proto file.

* Remove the structs we moved to internal proto file from the v1 proto file

This commit just removes the now no longer used messages from the public
minder.v1 module.

* Only check proto files in the proto subdir for backwards compatibility

* Exclude internal/proto from code coverage
  • Loading branch information
jhrozek authored Jul 11, 2024
1 parent 4164955 commit df6cf1c
Show file tree
Hide file tree
Showing 29 changed files with 4,515 additions and 4,435 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/generation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ jobs:
github_token: ${{ secrets.GITHUB_TOKEN }}
- uses: bufbuild/buf-breaking-action@c57b3d842a5c3f3b454756ef65305a50a587c5ba # v1.1.4
with:
against: "https://github.com/stacklok/minder.git#branch=main"
input: 'proto'
against: "https://github.com/stacklok/minder.git#branch=main,subdir=proto"
sqlc-generation:
runs-on: ubuntu-latest
steps:
Expand Down
2 changes: 1 addition & 1 deletion .mk/test.mk
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# exclude auto-generated DB code as well as mocks
# in future, we may want to parse these from a file instead of hardcoding them
# in the Makefile
COVERAGE_EXCLUSIONS="internal/db\|/mock/\|internal/auth/keycloak/client"
COVERAGE_EXCLUSIONS="internal/db\|/mock/\|internal/auth/keycloak/client\|internal/proto"
COVERAGE_PACKAGES=./internal/...

.PHONY: clean
Expand Down
2 changes: 1 addition & 1 deletion buf.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ plugins:
# https://github.com/pseudomuto/protoc-gen-doc/issues/513
# buf.build/community/pseudomuto-doc:v1.5.1
out: docs/docs/ref
opt: "docs/proto_template.tmpl,proto.md"
opt: "docs/proto_template.tmpl,proto.md:internal.proto"
1 change: 1 addition & 0 deletions buf.work.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@
version: v1
directories:
- proto
- internal/proto
99 changes: 0 additions & 99 deletions docs/docs/ref/proto.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +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"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)
Expand Down Expand Up @@ -74,7 +75,7 @@ func evaluateHomoglyphs(
}

//nolint:govet
prContents, ok := res.Object.(*pb.PrContents)
prContents, ok := res.Object.(*pbinternal.PrContents)
if !ok {
return false, fmt.Errorf("invalid object type for homoglyphs evaluator")
}
Expand Down
3 changes: 2 additions & 1 deletion internal/engine/eval/trusty/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +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"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)
Expand Down Expand Up @@ -193,7 +194,7 @@ type templateScoreComponent struct {
}

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

// Reason captures the reason why a package was flagged
Reasons []RuleViolationReason
Expand Down
4 changes: 2 additions & 2 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"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
pbinternal "github.com/stacklok/minder/internal/proto"
)

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

func (c *config) getEcosystemConfig(ecosystem pb.DepEcosystem) *ecosystemConfig {
func (c *config) getEcosystemConfig(ecosystem pbinternal.DepEcosystem) *ecosystemConfig {
sEco := ecosystem.AsString()
if sEco == "" {
return nil
Expand Down
12 changes: 6 additions & 6 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"
pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
pbinternal "github.com/stacklok/minder/internal/proto"
provifv1 "github.com/stacklok/minder/pkg/providers/v1"
)

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

func getEcosystemConfig(
logger *zerolog.Logger, ruleConfig *config, dep *pb.PrDependencies_ContextualDependency,
logger *zerolog.Logger, ruleConfig *config, dep *pbinternal.PrDependencies_ContextualDependency,
) *ecosystemConfig {
ecoConfig := ruleConfig.getEcosystemConfig(dep.Dep.Ecosystem)
if ecoConfig == nil {
Expand All @@ -149,8 +149,8 @@ func getEcosystemConfig(
}

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

func getDependencyScore(
ctx context.Context, trustyClient *trusty.Trusty, dep *pb.PrDependencies_ContextualDependency,
ctx context.Context, trustyClient *trusty.Trusty, dep *pbinternal.PrDependencies_ContextualDependency,
) (*trustytypes.Reply, error) {
// Call the Trusty API
resp, err := trustyClient.Report(ctx, &trustytypes.Dependency{
Expand All @@ -242,7 +242,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 *pb.PrDependencies_ContextualDependency,
prSummary *summaryPrHandler, dep *pbinternal.PrDependencies_ContextualDependency,
) {
// Check all the policy violations
reasons := []RuleViolationReason{}
Expand Down
44 changes: 22 additions & 22 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"
v1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
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: &v1.Dependency{
Ecosystem: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Name: "requests",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
Summary: trustytypes.ScoreSummary{
Score: &sg,
},
Expand All @@ -76,14 +76,14 @@ func TestBuildEvalResult(t *testing.T) {
{"low-scored-package", &summaryPrHandler{
trackedAlternatives: []dependencyAlternatives{
{
Dependency: &v1.Dependency{
Ecosystem: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Name: "requests",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
Summary: trustytypes.ScoreSummary{
Score: &sg,
},
Expand All @@ -94,28 +94,28 @@ func TestBuildEvalResult(t *testing.T) {
{"malicious-and-low-score", &summaryPrHandler{
trackedAlternatives: []dependencyAlternatives{
{
Dependency: &v1.Dependency{
Ecosystem: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Name: "python-oauth",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
Summary: trustytypes.ScoreSummary{
Score: &sg,
},
},
},
{
Dependency: &v1.Dependency{
Ecosystem: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Dependency: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI,
Name: "requestts",
Version: "0.0.1",
},
trustyReply: &trustytypes.Reply{
PackageName: "requests",
PackageType: v1.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
PackageType: pbinternal.DepEcosystem_DEP_ECOSYSTEM_PYPI.AsString(),
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: &v1.PrDependencies{}}, mustErr: false},
{name: "normal", sut: &engif.Result{Object: &pbinternal.PrDependencies{}}, mustErr: false},
{name: "invalid-object", sut: &engif.Result{Object: context.Background()}, mustErr: true},
} {
tc := tc
Expand Down Expand Up @@ -243,9 +243,9 @@ func TestClassifyDependency(t *testing.T) {

ctx := context.Background()
logger := zerolog.Ctx(ctx).With().Logger()
dep := &v1.PrDependencies_ContextualDependency{
Dep: &v1.Dependency{
Ecosystem: v1.DepEcosystem_DEP_ECOSYSTEM_NPM,
dep := &pbinternal.PrDependencies_ContextualDependency{
Dep: &pbinternal.Dependency{
Ecosystem: pbinternal.DepEcosystem_DEP_ECOSYSTEM_NPM,
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: &v1.Dependency{},
Dependency: &pbinternal.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: &v1.Dependency{},
Dependency: &pbinternal.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: &v1.Dependency{},
Dependency: &pbinternal.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: &v1.Dependency{},
Dependency: &pbinternal.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: &v1.Dependency{},
Dependency: &pbinternal.Dependency{},
Reasons: []RuleViolationReason{},
trustyReply: &trustytypes.Reply{
Summary: trustytypes.ScoreSummary{
Expand Down
Loading

0 comments on commit df6cf1c

Please sign in to comment.