Remove ProviderBuilder from engine

Relates to #2845

Refactor engine to use ProviderManager. Various bits of refactoring
along the way, including renaming some struct fields and variables for
clarity.

cmd/dev/app/rule_type/rttst.go

@@ -17,7 +17,6 @@ package rule_type
 import (
 	"bytes"
 	"context"
-	"database/sql"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -38,8 +37,10 @@ import (
 	"github.com/stacklok/minder/internal/engine/eval/rego"
 	engif "github.com/stacklok/minder/internal/engine/interfaces"
 	"github.com/stacklok/minder/internal/logger"
-	"github.com/stacklok/minder/internal/providers"
 	"github.com/stacklok/minder/internal/providers/credentials"
+	"github.com/stacklok/minder/internal/providers/github/clients"
+	"github.com/stacklok/minder/internal/providers/ratecache"
+	"github.com/stacklok/minder/internal/providers/telemetry"
 	"github.com/stacklok/minder/internal/util/jsonyaml"
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
@@ -97,24 +98,24 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
 		cmd.Println("If the rule you're testing is rego-based, you will not be able to use `print` statements for debugging.")
 	}
 
-	rt, err := readRuleTypeFromFile(rtpath.Value.String())
+	ruletype, err := readRuleTypeFromFile(rtpath.Value.String())
 	if err != nil {
 		return fmt.Errorf("error reading rule type from file: %w", err)
 	}
 
 	provider := "test"
 	rootProject := "00000000-0000-0000-0000-000000000002"
-	rt.Context = &minderv1.Context{
+	ruletype.Context = &minderv1.Context{
 		Provider: &provider,
 		Project:  &rootProject,
 	}
 
-	ent, err := readEntityFromFile(epath.Value.String(), minderv1.EntityFromString(rt.Def.InEntity))
+	ent, err := readEntityFromFile(epath.Value.String(), minderv1.EntityFromString(ruletype.Def.InEntity))
 	if err != nil {
 		return fmt.Errorf("error reading entity from file: %w", err)
 	}
 
-	p, err := engine.ReadProfileFromFile(ppath.Value.String())
+	profile, err := engine.ReadProfileFromFile(ppath.Value.String())
 	if err != nil {
 		return fmt.Errorf("error reading fragment from file: %w", err)
 	}
@@ -148,38 +149,30 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
 
 	// Disable actions
 	off := "off"
-	p.Alert = &off
+	profile.Alert = &off
 
-	rules, err := engine.GetRulesFromProfileOfType(p, rt)
+	rules, err := engine.GetRulesFromProfileOfType(profile, ruletype)
 	if err != nil {
 		return fmt.Errorf("error getting relevant fragment: %w", err)
 	}
 
-	// TODO: Read this from a providers file instead so we can make it pluggable
-	eng, err := engine.NewRuleTypeEngine(context.Background(), p, rt, providers.NewProviderBuilder(
-		&db.Provider{
-			Name:    "test",
-			Version: "v1",
-			Implements: []db.ProviderType{
-				"rest",
-				"repo-lister",
-				"git",
-				"github",
-			},
-			Definition: json.RawMessage(`{
-				"github-app": {}
-			}`),
-		},
-		sql.NullString{},
-		false,
+	// TODO: Whenever we add more Provider classes, we will need to rethink this
+	client, err := clients.NewGitHubAppProvider(
+		&minderv1.GitHubAppProviderConfig{},
+		&serverconfig.GitHubAppConfig{AppName: "test"},
+		&ratecache.NoopRestClientCache{},
 		credentials.NewGitHubTokenCredential(token),
-		&serverconfig.ProviderConfig{
-			GitHubApp: &serverconfig.GitHubAppConfig{
-				AppName: "test",
-			},
-		},
-		nil, // this is unused here
-	))
+		nil,
+		clients.NewGitHubClientFactory(telemetry.NewNoopMetrics()),
+		false,
+	)
+	if err != nil {
+		return fmt.Errorf("error instantiating github provider: %w", err)
+	}
+
+	// TODO: use cobra context here
+	eng, err := engine.NewRuleTypeEngine(context.Background(), profile, ruletype, client)
+
 	inf := &entities.EntityInfoWrapper{
 		Entity:      ent,
 		ExecutionID: &uuid.Nil,
@@ -189,7 +182,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
 	}
 
 	if len(rules) == 0 {
-		return fmt.Errorf("no rules found with type %s", rt.Name)
+		return fmt.Errorf("no rules found with type %s", ruletype.Name)
 	}
 
 	return runEvaluationForRules(cmd, eng, inf, remediateStatus, remMetadata, rules)

cmd/dev/app/testserver/testserver.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"os/signal"
 
+	"github.com/ThreeDotsLabs/watermill/message"
 	"github.com/google/go-github/v61/github"
 	"github.com/rs/zerolog"
 	"github.com/spf13/cobra"
@@ -35,7 +36,6 @@ import (
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/controlplane/metrics"
 	"github.com/stacklok/minder/internal/db/embedded"
-	"github.com/stacklok/minder/internal/engine"
 	"github.com/stacklok/minder/internal/logger"
 	"github.com/stacklok/minder/internal/providers/ratecache"
 	provtelemetry "github.com/stacklok/minder/internal/providers/telemetry"
@@ -101,6 +101,6 @@ func runTestServer(cmd *cobra.Command, _ []string) error {
 		&auth.IdentityClient{},
 		metrics.NewNoopMetrics(),
 		provtelemetry.NewNoopMetrics(),
-		[]engine.ExecutorOption{},
+		[]message.HandlerMiddleware{},
 	)
 }

cmd/server/app/serve.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"os/signal"
 
+	"github.com/ThreeDotsLabs/watermill/message"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
@@ -33,7 +34,6 @@ import (
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	cpmetrics "github.com/stacklok/minder/internal/controlplane/metrics"
 	"github.com/stacklok/minder/internal/db"
-	"github.com/stacklok/minder/internal/engine"
 	"github.com/stacklok/minder/internal/logger"
 	"github.com/stacklok/minder/internal/providers/ratecache"
 	provtelemetry "github.com/stacklok/minder/internal/providers/telemetry"
@@ -118,12 +118,7 @@ var serveCmd = &cobra.Command{
 		restClientCache := ratecache.NewRestClientCache(ctx)
 		defer restClientCache.Close()
 
-		tsmdw := logger.NewTelemetryStoreWMMiddleware(l)
-		executorOpts := []engine.ExecutorOption{
-			engine.WithProviderMetrics(providerMetrics),
-			engine.WithMiddleware(tsmdw.TelemetryStoreMiddleware),
-		}
-
+		telemetryMiddleware := logger.NewTelemetryStoreWMMiddleware(l)
 		return service.AllInOneServerService(
 			ctx,
 			cfg,
@@ -134,7 +129,7 @@ var serveCmd = &cobra.Command{
 			idClient,
 			cpmetrics.NewMetrics(),
 			providerMetrics,
-			executorOpts,
+			[]message.HandlerMiddleware{telemetryMiddleware.TelemetryStoreMiddleware},
 		)
 	},
 }

internal/engine/actions/actions.go

@@ -33,8 +33,8 @@ import (
 	"github.com/stacklok/minder/internal/engine/actions/remediate/pull_request"
 	enginerr "github.com/stacklok/minder/internal/engine/errors"
 	engif "github.com/stacklok/minder/internal/engine/interfaces"
-	"github.com/stacklok/minder/internal/providers"
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
+	provinfv1 "github.com/stacklok/minder/pkg/providers/v1"
 )
 
 // RuleActionsEngine is the engine responsible for processing all actions i.e., remediation and alerts
@@ -44,16 +44,19 @@ type RuleActionsEngine struct {
 }
 
 // NewRuleActions creates a new rule actions engine
-func NewRuleActions(p *minderv1.Profile, rt *minderv1.RuleType, pbuild *providers.ProviderBuilder,
+func NewRuleActions(
+	profile *minderv1.Profile,
+	ruletype *minderv1.RuleType,
+	provider provinfv1.Provider,
 ) (*RuleActionsEngine, error) {
 	// Create the remediation engine
-	remEngine, err := remediate.NewRuleRemediator(rt, pbuild)
+	remEngine, err := remediate.NewRuleRemediator(ruletype, provider)
 	if err != nil {
 		return nil, fmt.Errorf("cannot create rule remediator: %w", err)
 	}
 
 	// Create the alert engine
-	alertEngine, err := alert.NewRuleAlert(rt, pbuild)
+	alertEngine, err := alert.NewRuleAlert(ruletype, provider)
 	if err != nil {
 		return nil, fmt.Errorf("cannot create rule alerter: %w", err)
 	}
@@ -66,8 +69,8 @@ func NewRuleActions(p *minderv1.Profile, rt *minderv1.RuleType, pbuild *provider
 		// The on/off state of the actions is an integral part of the action engine
 		// and should be set upon creation.
 		actionsOnOff: map[engif.ActionType]engif.ActionOpt{
-			remEngine.Class():   remEngine.GetOnOffState(p),
-			alertEngine.Class(): alertEngine.GetOnOffState(p),
+			remEngine.Class():   remEngine.GetOnOffState(profile),
+			alertEngine.Class(): alertEngine.GetOnOffState(profile),
 		},
 	}, nil
 }

internal/engine/actions/alert/alert.go

@@ -18,21 +18,25 @@
 package alert
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/stacklok/minder/internal/engine/actions/alert/noop"
 	"github.com/stacklok/minder/internal/engine/actions/alert/security_advisory"
 	engif "github.com/stacklok/minder/internal/engine/interfaces"
-	"github.com/stacklok/minder/internal/providers"
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
+	provinfv1 "github.com/stacklok/minder/pkg/providers/v1"
 )
 
 // ActionType is the type of the alert engine
 const ActionType engif.ActionType = "alert"
 
 // NewRuleAlert creates a new rule alert engine
-func NewRuleAlert(rt *pb.RuleType, pbuild *providers.ProviderBuilder) (engif.Action, error) {
-	alertCfg := rt.Def.GetAlert()
+func NewRuleAlert(
+	ruletype *pb.RuleType,
+	provider provinfv1.Provider,
+) (engif.Action, error) {
+	alertCfg := ruletype.Def.GetAlert()
 	if alertCfg == nil {
 		return noop.NewNoopAlert(ActionType)
 	}
@@ -43,11 +47,11 @@ func NewRuleAlert(rt *pb.RuleType, pbuild *providers.ProviderBuilder) (engif.Act
 		if alertCfg.GetSecurityAdvisory() == nil {
 			return nil, fmt.Errorf("alert engine missing security-advisory configuration")
 		}
-		client, err := pbuild.GetGitHub()
+		client, err := provinfv1.As[provinfv1.GitHub](provider)
 		if err != nil {
-			return nil, fmt.Errorf("could not instantiate provider: %w", err)
+			return nil, errors.New("provider does not implement git trait")
 		}
-		return security_advisory.NewSecurityAdvisoryAlert(ActionType, rt.GetSeverity(), alertCfg.GetSecurityAdvisory(), client)
+		return security_advisory.NewSecurityAdvisoryAlert(ActionType, ruletype.GetSeverity(), alertCfg.GetSecurityAdvisory(), client)
 	}
 
 	return nil, fmt.Errorf("unknown alert type: %s", alertCfg.GetType())

internal/engine/actions/remediate/gh_branch_protect/gh_branch_protect_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/stacklok/minder/internal/providers/credentials"
 	"github.com/stacklok/minder/internal/providers/github/clients"
 	mock_ghclient "github.com/stacklok/minder/internal/providers/github/mock"
+	"github.com/stacklok/minder/internal/providers/ratecache"
 	"github.com/stacklok/minder/internal/providers/telemetry"
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 	provifv1 "github.com/stacklok/minder/pkg/providers/v1"
@@ -55,7 +56,7 @@ func testGithubProvider(baseURL string) (provifv1.GitHub, error) {
 		&pb.GitHubProviderConfig{
 			Endpoint: baseURL,
 		},
-		nil,
+		&ratecache.NoopRestClientCache{},
 		credentials.NewGitHubTokenCredential("token"),
 		clients.NewGitHubClientFactory(telemetry.NewNoopMetrics()),
 		"",

internal/engine/actions/remediate/pull_request/pull_request_test.go

@@ -43,6 +43,7 @@ import (
 	"github.com/stacklok/minder/internal/providers/credentials"
 	"github.com/stacklok/minder/internal/providers/github/clients"
 	mockghclient "github.com/stacklok/minder/internal/providers/github/mock"
+	"github.com/stacklok/minder/internal/providers/ratecache"
 	"github.com/stacklok/minder/internal/providers/telemetry"
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 	provifv1 "github.com/stacklok/minder/pkg/providers/v1"
@@ -92,7 +93,7 @@ func testGithubProvider() (provifv1.GitHub, error) {
 		&pb.GitHubProviderConfig{
 			Endpoint: ghApiUrl + "/",
 		},
-		nil,
+		&ratecache.NoopRestClientCache{},
 		credentials.NewGitHubTokenCredential("token"),
 		clients.NewGitHubClientFactory(telemetry.NewNoopMetrics()),
 		"",

internal/engine/actions/remediate/remediate.go

@@ -18,60 +18,64 @@
 package remediate
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/stacklok/minder/internal/engine/actions/remediate/gh_branch_protect"
 	"github.com/stacklok/minder/internal/engine/actions/remediate/noop"
 	"github.com/stacklok/minder/internal/engine/actions/remediate/pull_request"
 	"github.com/stacklok/minder/internal/engine/actions/remediate/rest"
 	engif "github.com/stacklok/minder/internal/engine/interfaces"
-	"github.com/stacklok/minder/internal/providers"
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
+	provinfv1 "github.com/stacklok/minder/pkg/providers/v1"
 )
 
 // ActionType is the type of the remediation engine
 const ActionType engif.ActionType = "remediate"
 
 // NewRuleRemediator creates a new rule remediator
-func NewRuleRemediator(rt *pb.RuleType, pbuild *providers.ProviderBuilder) (engif.Action, error) {
-	rem := rt.Def.GetRemediate()
-	if rem == nil {
+func NewRuleRemediator(
+	rt *pb.RuleType,
+	provider provinfv1.Provider,
+) (engif.Action, error) {
+	remediate := rt.Def.GetRemediate()
+	if remediate == nil {
 		return noop.NewNoopRemediate(ActionType)
 	}
 
 	// nolint:revive // let's keep the switch here, it would be nicer to extend a switch in the future
-	switch rem.GetType() {
+	switch remediate.GetType() {
 	case rest.RemediateType:
-		client, err := pbuild.GetHTTP()
+		client, err := provinfv1.As[provinfv1.REST](provider)
 		if err != nil {
-			return nil, fmt.Errorf("could not instantiate provider: %w", err)
+			return nil, errors.New("provider does not implement rest trait")
 		}
-		if rem.GetRest() == nil {
+		if remediate.GetRest() == nil {
 			return nil, fmt.Errorf("remediations engine missing rest configuration")
 		}
-		return rest.NewRestRemediate(ActionType, rem.GetRest(), client)
+		return rest.NewRestRemediate(ActionType, remediate.GetRest(), client)
 
 	case gh_branch_protect.RemediateType:
-		client, err := pbuild.GetGitHub()
+		client, err := provinfv1.As[provinfv1.GitHub](provider)
 		if err != nil {
-			return nil, fmt.Errorf("could not instantiate provider: %w", err)
+			return nil, errors.New("provider does not implement git trait")
 		}
-		if rem.GetGhBranchProtection() == nil {
+		if remediate.GetGhBranchProtection() == nil {
 			return nil, fmt.Errorf("remediations engine missing gh_branch_protection configuration")
 		}
-		return gh_branch_protect.NewGhBranchProtectRemediator(ActionType, rem.GetGhBranchProtection(), client)
+		return gh_branch_protect.NewGhBranchProtectRemediator(ActionType, remediate.GetGhBranchProtection(), client)
 
 	case pull_request.RemediateType:
-		client, err := pbuild.GetGitHub()
+		client, err := provinfv1.As[provinfv1.GitHub](provider)
 		if err != nil {
-			return nil, fmt.Errorf("could not instantiate provider: %w", err)
+			return nil, errors.New("provider does not implement git trait")
 		}
-		if rem.GetPullRequest() == nil {
+		if remediate.GetPullRequest() == nil {
 			return nil, fmt.Errorf("remediations engine missing pull request configuration")
 		}
 
-		return pull_request.NewPullRequestRemediate(ActionType, rem.GetPullRequest(), client)
+		return pull_request.NewPullRequestRemediate(ActionType, remediate.GetPullRequest(), client)
 	}
 
-	return nil, fmt.Errorf("unknown remediation type: %s", rem.GetType())
+	return nil, fmt.Errorf("unknown remediation type: %s", remediate.GetType())
 }