From a9000d882006f08a5cc281c96ecf7f2d871f2462 Mon Sep 17 00:00:00 2001 From: Eleftheria Stein-Kousathana Date: Tue, 2 Apr 2024 13:26:45 +0200 Subject: [PATCH] Remove provider from profile (#2850) Fix #2848 --- cmd/cli/app/profile/apply.go | 3 +- cmd/cli/app/profile/common.go | 15 ++---- cmd/cli/app/profile/create.go | 3 +- cmd/cli/app/profile/delete.go | 3 +- cmd/cli/app/profile/get.go | 3 +- cmd/cli/app/profile/list.go | 3 +- cmd/cli/app/profile/profile.go | 2 - cmd/cli/app/profile/status/status_get.go | 3 +- cmd/cli/app/profile/status/status_list.go | 3 +- cmd/cli/app/profile/table_render.go | 2 +- .../000043_profiles_drop_provider.down.sql | 29 ++++++++++ .../000043_profiles_drop_provider.up.sql | 26 +++++++++ database/query/profiles.sql | 4 +- docs/docs/ref/cli/minder_profile.md | 5 +- docs/docs/ref/cli/minder_profile_apply.md | 1 - docs/docs/ref/cli/minder_profile_create.md | 1 - docs/docs/ref/cli/minder_profile_delete.md | 1 - docs/docs/ref/cli/minder_profile_get.md | 1 - docs/docs/ref/cli/minder_profile_list.md | 1 - docs/docs/ref/cli/minder_profile_status.md | 1 - .../docs/ref/cli/minder_profile_status_get.md | 1 - .../ref/cli/minder_profile_status_list.md | 1 - internal/controlplane/handlers_profile.go | 54 ++++++------------- .../controlplane/handlers_profile_test.go | 18 +------ internal/db/models.go | 4 +- internal/db/profiles.sql.go | 17 +++--- internal/db/profiles_test.go | 24 ++++----- internal/engine/executor_test.go | 1 - internal/engine/profile.go | 7 +-- internal/marketplaces/service.go | 19 +++---- internal/marketplaces/service_test.go | 15 ++---- .../subscriptions/mock/subscription.go | 6 +-- .../marketplaces/subscriptions/service.go | 19 ++++--- .../subscriptions/service_test.go | 10 +--- internal/marketplaces/types/types.go | 40 -------------- internal/profiles/mock/fixtures/service.go | 8 +-- internal/profiles/mock/service.go | 16 +++--- internal/profiles/service.go | 28 +++------- internal/projects/create.go | 8 ++- internal/reconcilers/run_profile.go | 37 ++----------- internal/util/cli/table/simple/simple.go | 2 +- 41 files changed, 165 insertions(+), 280 deletions(-) create mode 100644 database/migrations/000043_profiles_drop_provider.down.sql create mode 100644 database/migrations/000043_profiles_drop_provider.up.sql delete mode 100644 internal/marketplaces/types/types.go diff --git a/cmd/cli/app/profile/apply.go b/cmd/cli/app/profile/apply.go index 9a2779e7e5..cd1a75f7fb 100644 --- a/cmd/cli/app/profile/apply.go +++ b/cmd/cli/app/profile/apply.go @@ -42,7 +42,6 @@ var applyCmd = &cobra.Command{ func applyCommand(_ context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error { client := minderv1.NewProfileServiceClient(conn) - provider := viper.GetString("provider") project := viper.GetString("project") f := viper.GetString("file") @@ -85,7 +84,7 @@ func applyCommand(_ context.Context, cmd *cobra.Command, conn *grpc.ClientConn) // cmd.Context() is the root context. We need to create a new context for each file // so we can avoid the timeout. - profile, err := ExecOnOneProfile(cmd.Context(), table, f, cmd.InOrStdin(), project, provider, applyFunc) + profile, err := ExecOnOneProfile(cmd.Context(), table, f, cmd.InOrStdin(), project, applyFunc) if err != nil { return cli.MessageAndError(fmt.Sprintf("error applying profile from %s", f), err) } diff --git a/cmd/cli/app/profile/common.go b/cmd/cli/app/profile/common.go index c5362c68a9..47a10e0b5d 100644 --- a/cmd/cli/app/profile/common.go +++ b/cmd/cli/app/profile/common.go @@ -29,7 +29,7 @@ import ( ) // ExecOnOneProfile is a helper function to execute a function on a single profile -func ExecOnOneProfile(ctx context.Context, t table.Table, f string, dashOpen io.Reader, project string, provider string, +func ExecOnOneProfile(ctx context.Context, t table.Table, f string, dashOpen io.Reader, project string, exec func(context.Context, string, *minderv1.Profile) (*minderv1.Profile, error), ) (*minderv1.Profile, error) { ctx, cancel := cli.GetAppContext(ctx, viper.GetViper()) @@ -41,7 +41,7 @@ func ExecOnOneProfile(ctx context.Context, t table.Table, f string, dashOpen io. } defer closer() - p, err := parseProfile(reader, project, provider) + p, err := parseProfile(reader, project) if err != nil { return nil, fmt.Errorf("error parsing profile: %w", err) } @@ -56,7 +56,7 @@ func ExecOnOneProfile(ctx context.Context, t table.Table, f string, dashOpen io. return profile, nil } -func parseProfile(r io.Reader, proj string, provider string) (*minderv1.Profile, error) { +func parseProfile(r io.Reader, proj string) (*minderv1.Profile, error) { p, err := engine.ParseYAML(r) if err != nil { return nil, fmt.Errorf("error reading profile from file: %w", err) @@ -71,14 +71,5 @@ func parseProfile(r io.Reader, proj string, provider string) (*minderv1.Profile, p.Context.Project = &proj } - // Override the YAML specified provider with the command line argument - if provider != "" { - if p.Context == nil { - p.Context = &minderv1.Context{} - } - - p.Context.Provider = &provider - } - return p, nil } diff --git a/cmd/cli/app/profile/create.go b/cmd/cli/app/profile/create.go index e619b0f653..b892eedda8 100644 --- a/cmd/cli/app/profile/create.go +++ b/cmd/cli/app/profile/create.go @@ -40,7 +40,6 @@ var createCmd = &cobra.Command{ func createCommand(_ context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error { client := minderv1.NewProfileServiceClient(conn) - provider := viper.GetString("provider") project := viper.GetString("project") f := viper.GetString("file") enableAlerts := viper.GetBool("enable-alerts") @@ -74,7 +73,7 @@ func createCommand(_ context.Context, cmd *cobra.Command, conn *grpc.ClientConn) } // cmd.Context() is the root context. We need to create a new context for each file // so we can avoid the timeout. - profile, err := ExecOnOneProfile(cmd.Context(), table, f, cmd.InOrStdin(), project, provider, createFunc) + profile, err := ExecOnOneProfile(cmd.Context(), table, f, cmd.InOrStdin(), project, createFunc) if err != nil { return cli.MessageAndError(fmt.Sprintf("error creating profile from %s", f), err) } diff --git a/cmd/cli/app/profile/delete.go b/cmd/cli/app/profile/delete.go index 2ccc760a73..172bb44d71 100644 --- a/cmd/cli/app/profile/delete.go +++ b/cmd/cli/app/profile/delete.go @@ -38,7 +38,6 @@ var deleteCmd = &cobra.Command{ func deleteCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error { client := minderv1.NewProfileServiceClient(conn) - provider := viper.GetString("provider") project := viper.GetString("project") id := viper.GetString("id") @@ -48,7 +47,7 @@ func deleteCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientCon // Delete profile _, err := client.DeleteProfile(ctx, &minderv1.DeleteProfileRequest{ - Context: &minderv1.Context{Provider: &provider, Project: &project}, + Context: &minderv1.Context{Project: &project}, Id: id, }) if err != nil { diff --git a/cmd/cli/app/profile/get.go b/cmd/cli/app/profile/get.go index 0bb7603d3f..0c05978d2a 100644 --- a/cmd/cli/app/profile/get.go +++ b/cmd/cli/app/profile/get.go @@ -42,7 +42,6 @@ var getCmd = &cobra.Command{ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error { client := minderv1.NewProfileServiceClient(conn) - provider := viper.GetString("provider") project := viper.GetString("project") format := viper.GetString("output") id := viper.GetString("id") @@ -57,7 +56,7 @@ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) cmd.SilenceUsage = true p, err := client.GetProfileById(ctx, &minderv1.GetProfileByIdRequest{ - Context: &minderv1.Context{Provider: &provider, Project: &project}, + Context: &minderv1.Context{Project: &project}, Id: id, }) if err != nil { diff --git a/cmd/cli/app/profile/list.go b/cmd/cli/app/profile/list.go index 53e8ad8725..ef81e1a477 100644 --- a/cmd/cli/app/profile/list.go +++ b/cmd/cli/app/profile/list.go @@ -41,7 +41,6 @@ var listCmd = &cobra.Command{ func listCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error { client := minderv1.NewProfileServiceClient(conn) - provider := viper.GetString("provider") project := viper.GetString("project") format := viper.GetString("output") @@ -55,7 +54,7 @@ func listCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) cmd.SilenceUsage = true resp, err := client.ListProfiles(ctx, &minderv1.ListProfilesRequest{ - Context: &minderv1.Context{Provider: &provider, Project: &project}, + Context: &minderv1.Context{Project: &project}, }) if err != nil { return cli.MessageAndError("Error getting profiles", err) diff --git a/cmd/cli/app/profile/profile.go b/cmd/cli/app/profile/profile.go index 831f8b8a18..27a75ff55a 100644 --- a/cmd/cli/app/profile/profile.go +++ b/cmd/cli/app/profile/profile.go @@ -20,7 +20,6 @@ import ( "github.com/spf13/cobra" "github.com/stacklok/minder/cmd/cli/app" - ghclient "github.com/stacklok/minder/internal/providers/github/oauth" ) // ProfileCmd is the root command for the profile subcommands @@ -36,6 +35,5 @@ var ProfileCmd = &cobra.Command{ func init() { app.RootCmd.AddCommand(ProfileCmd) // Flags for all subcommands - ProfileCmd.PersistentFlags().StringP("provider", "p", ghclient.Github, "Name of the provider, i.e. github") ProfileCmd.PersistentFlags().StringP("project", "j", "", "ID of the project") } diff --git a/cmd/cli/app/profile/status/status_get.go b/cmd/cli/app/profile/status/status_get.go index f01ccf01fb..3586153a3e 100644 --- a/cmd/cli/app/profile/status/status_get.go +++ b/cmd/cli/app/profile/status/status_get.go @@ -43,7 +43,6 @@ var getCmd = &cobra.Command{ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error { client := minderv1.NewProfileServiceClient(conn) - provider := viper.GetString("provider") project := viper.GetString("project") profileName := viper.GetString("name") entityId := viper.GetString("entity") @@ -56,7 +55,7 @@ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) } resp, err := client.GetProfileStatusByName(ctx, &minderv1.GetProfileStatusByNameRequest{ - Context: &minderv1.Context{Provider: &provider, Project: &project}, + Context: &minderv1.Context{Project: &project}, Name: profileName, Entity: &minderv1.EntityTypedId{ Id: entityId, diff --git a/cmd/cli/app/profile/status/status_list.go b/cmd/cli/app/profile/status/status_list.go index f7157cfd4d..28364aa6ea 100644 --- a/cmd/cli/app/profile/status/status_list.go +++ b/cmd/cli/app/profile/status/status_list.go @@ -41,7 +41,6 @@ var listCmd = &cobra.Command{ func listCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error { client := minderv1.NewProfileServiceClient(conn) - provider := viper.GetString("provider") project := viper.GetString("project") profileName := viper.GetString("name") format := viper.GetString("output") @@ -55,7 +54,7 @@ func listCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) } resp, err := client.GetProfileStatusByName(ctx, &minderv1.GetProfileStatusByNameRequest{ - Context: &minderv1.Context{Provider: &provider, Project: &project}, + Context: &minderv1.Context{Project: &project}, Name: profileName, All: detailed, RuleType: ruleType, diff --git a/cmd/cli/app/profile/table_render.go b/cmd/cli/app/profile/table_render.go index 99aa37b3d8..a5812da080 100644 --- a/cmd/cli/app/profile/table_render.go +++ b/cmd/cli/app/profile/table_render.go @@ -59,7 +59,7 @@ func NewProfileSettingsTable() table.Table { // RenderProfileSettingsTable renders the profile settings table func RenderProfileSettingsTable(p *minderv1.Profile, t table.Table) { - t.AddRow(p.GetId(), p.GetName(), p.GetContext().GetProvider(), p.GetAlert(), p.GetRemediate()) + t.AddRow(p.GetId(), p.GetName(), p.GetAlert(), p.GetRemediate()) } // NewProfileTable creates a new table for rendering profiles diff --git a/database/migrations/000043_profiles_drop_provider.down.sql b/database/migrations/000043_profiles_drop_provider.down.sql new file mode 100644 index 0000000000..cfd5c5e695 --- /dev/null +++ b/database/migrations/000043_profiles_drop_provider.down.sql @@ -0,0 +1,29 @@ +-- Copyright 2024 Stacklok, Inc +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. + +BEGIN; + +ALTER TABLE profiles + ADD CONSTRAINT profiles_provider_id_name_fkey + FOREIGN KEY (provider_id, provider) + REFERENCES providers(id, name) + ON DELETE CASCADE; + +ALTER TABLE profiles + ALTER COLUMN provider SET NOT NULL; + +ALTER TABLE profiles + ALTER COLUMN provider_id SET NOT NULL; + +COMMIT; \ No newline at end of file diff --git a/database/migrations/000043_profiles_drop_provider.up.sql b/database/migrations/000043_profiles_drop_provider.up.sql new file mode 100644 index 0000000000..02d2239de1 --- /dev/null +++ b/database/migrations/000043_profiles_drop_provider.up.sql @@ -0,0 +1,26 @@ +-- Copyright 2024 Stacklok, Inc +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. + +BEGIN; + +-- Drop the existing foreign key constraint on provider +ALTER TABLE profiles DROP CONSTRAINT profiles_provider_id_name_fkey; + +ALTER TABLE profiles + ALTER COLUMN provider DROP NOT NULL; + +ALTER TABLE profiles + ALTER COLUMN provider_id DROP NOT NULL; + +COMMIT; \ No newline at end of file diff --git a/database/query/profiles.sql b/database/query/profiles.sql index 8883b794cf..6081cbba54 100644 --- a/database/query/profiles.sql +++ b/database/query/profiles.sql @@ -1,15 +1,13 @@ -- name: CreateProfile :one INSERT INTO profiles ( - provider, project_id, remediate, alert, name, - provider_id, subscription_id, display_name, labels -) VALUES ($1, $2, $3, $4, $5, sqlc.arg(provider_id), sqlc.narg(subscription_id), sqlc.arg(display_name), COALESCE(sqlc.arg(labels)::text[], '{}'::text[])) RETURNING *; +) VALUES ($1, $2, $3, $4, sqlc.narg(subscription_id), sqlc.arg(display_name), COALESCE(sqlc.arg(labels)::text[], '{}'::text[])) RETURNING *; -- name: UpdateProfile :one UPDATE profiles SET diff --git a/docs/docs/ref/cli/minder_profile.md b/docs/docs/ref/cli/minder_profile.md index b338ab7445..d6f481a6ae 100644 --- a/docs/docs/ref/cli/minder_profile.md +++ b/docs/docs/ref/cli/minder_profile.md @@ -16,9 +16,8 @@ minder profile [flags] ### Options ``` - -h, --help help for profile - -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") + -h, --help help for profile + -j, --project string ID of the project ``` ### Options inherited from parent commands diff --git a/docs/docs/ref/cli/minder_profile_apply.md b/docs/docs/ref/cli/minder_profile_apply.md index a383283c4a..5d32425137 100644 --- a/docs/docs/ref/cli/minder_profile_apply.md +++ b/docs/docs/ref/cli/minder_profile_apply.md @@ -30,7 +30,6 @@ minder profile apply [flags] --identity-client string Identity server client ID (default "minder-cli") --identity-url string Identity server issuer URL (default "https://auth.stacklok.com") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/docs/docs/ref/cli/minder_profile_create.md b/docs/docs/ref/cli/minder_profile_create.md index af84c64209..3d2a209b4a 100644 --- a/docs/docs/ref/cli/minder_profile_create.md +++ b/docs/docs/ref/cli/minder_profile_create.md @@ -32,7 +32,6 @@ minder profile create [flags] --identity-client string Identity server client ID (default "minder-cli") --identity-url string Identity server issuer URL (default "https://auth.stacklok.com") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/docs/docs/ref/cli/minder_profile_delete.md b/docs/docs/ref/cli/minder_profile_delete.md index 6716c3d93c..d8ef3b122b 100644 --- a/docs/docs/ref/cli/minder_profile_delete.md +++ b/docs/docs/ref/cli/minder_profile_delete.md @@ -30,7 +30,6 @@ minder profile delete [flags] --identity-client string Identity server client ID (default "minder-cli") --identity-url string Identity server issuer URL (default "https://auth.stacklok.com") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/docs/docs/ref/cli/minder_profile_get.md b/docs/docs/ref/cli/minder_profile_get.md index 38574c6a6f..2949f531ea 100644 --- a/docs/docs/ref/cli/minder_profile_get.md +++ b/docs/docs/ref/cli/minder_profile_get.md @@ -31,7 +31,6 @@ minder profile get [flags] --identity-client string Identity server client ID (default "minder-cli") --identity-url string Identity server issuer URL (default "https://auth.stacklok.com") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/docs/docs/ref/cli/minder_profile_list.md b/docs/docs/ref/cli/minder_profile_list.md index ad11e45f88..25b6800ab6 100644 --- a/docs/docs/ref/cli/minder_profile_list.md +++ b/docs/docs/ref/cli/minder_profile_list.md @@ -30,7 +30,6 @@ minder profile list [flags] --identity-client string Identity server client ID (default "minder-cli") --identity-url string Identity server issuer URL (default "https://auth.stacklok.com") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/docs/docs/ref/cli/minder_profile_status.md b/docs/docs/ref/cli/minder_profile_status.md index e3a32fe865..94918074a5 100644 --- a/docs/docs/ref/cli/minder_profile_status.md +++ b/docs/docs/ref/cli/minder_profile_status.md @@ -31,7 +31,6 @@ minder profile status [flags] --identity-client string Identity server client ID (default "minder-cli") --identity-url string Identity server issuer URL (default "https://auth.stacklok.com") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/docs/docs/ref/cli/minder_profile_status_get.md b/docs/docs/ref/cli/minder_profile_status_get.md index 3bc05993e1..d02f54c5fe 100644 --- a/docs/docs/ref/cli/minder_profile_status_get.md +++ b/docs/docs/ref/cli/minder_profile_status_get.md @@ -33,7 +33,6 @@ minder profile status get [flags] -n, --name string Profile name to get profile status for -o, --output string Output format (one of json,yaml,table) (default "table") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/docs/docs/ref/cli/minder_profile_status_list.md b/docs/docs/ref/cli/minder_profile_status_list.md index 79e20ec9ad..4b32d0b607 100644 --- a/docs/docs/ref/cli/minder_profile_status_list.md +++ b/docs/docs/ref/cli/minder_profile_status_list.md @@ -34,7 +34,6 @@ minder profile status list [flags] -n, --name string Profile name to get profile status for -o, --output string Output format (one of json,yaml,table) (default "table") -j, --project string ID of the project - -p, --provider string Name of the provider, i.e. github (default "github") ``` ### SEE ALSO diff --git a/internal/controlplane/handlers_profile.go b/internal/controlplane/handlers_profile.go index 77e5d89070..48c59a89c2 100644 --- a/internal/controlplane/handlers_profile.go +++ b/internal/controlplane/handlers_profile.go @@ -51,20 +51,14 @@ func (s *Server) CreateProfile(ctx context.Context, entityCtx := engine.EntityFromContext(ctx) - // validate that project and provider are valid and exist in the db - err := entityCtx.Validate(ctx, s.store) + // validate that project is valid and exist in the db + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } - // TODO: This will be removed once we decouple providers from profiles - provider, err := s.providerStore.GetByName(ctx, entityCtx.Project.ID, in.GetContext().GetProvider()) - if err != nil { - return nil, providerError(err) - } - newProfile, err := db.WithTransaction(s.store, func(qtx db.ExtendQuerier) (*minderv1.Profile, error) { - return s.profiles.CreateProfile(ctx, entityCtx.Project.ID, provider, uuid.Nil, in, qtx) + return s.profiles.CreateProfile(ctx, entityCtx.Project.ID, uuid.Nil, in, qtx) }) if err != nil { // assumption: service layer is setting meaningful errors @@ -83,7 +77,7 @@ func (s *Server) DeleteProfile(ctx context.Context, in *minderv1.DeleteProfileRequest) (*minderv1.DeleteProfileResponse, error) { entityCtx := engine.EntityFromContext(ctx) - err := entityCtx.Validate(ctx, s.store) + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } @@ -120,7 +114,6 @@ func (s *Server) DeleteProfile(ctx context.Context, } // Telemetry logging - logger.BusinessRecord(ctx).Provider = profile.Provider logger.BusinessRecord(ctx).Project = profile.ProjectID logger.BusinessRecord(ctx).Profile = logger.Profile{Name: profile.Name, ID: profile.ID} @@ -132,7 +125,7 @@ func (s *Server) ListProfiles(ctx context.Context, req *minderv1.ListProfilesRequest) (*minderv1.ListProfilesResponse, error) { entityCtx := engine.EntityFromContext(ctx) - err := entityCtx.Validate(ctx, s.store) + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } @@ -164,7 +157,6 @@ func (s *Server) ListProfiles(ctx context.Context, } // Telemetry logging - logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name logger.BusinessRecord(ctx).Project = entityCtx.Project.ID return &resp, nil @@ -176,7 +168,7 @@ func (s *Server) GetProfileById(ctx context.Context, entityCtx := engine.EntityFromContext(ctx) - err := entityCtx.Validate(ctx, s.store) + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } @@ -196,7 +188,6 @@ func (s *Server) GetProfileById(ctx context.Context, } // Telemetry logging - logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name logger.BusinessRecord(ctx).Project = entityCtx.Project.ID logger.BusinessRecord(ctx).Profile = logger.Profile{Name: profile.Name, ID: parsedProfileID} @@ -240,17 +231,15 @@ func getRuleEvalEntityInfo( entityType *db.NullEntities, selector *uuid.NullUUID, rs db.ListRuleEvaluationsByProfileIdRow, - providerName string, ) map[string]string { l := zerolog.Ctx(ctx) - entityInfo := map[string]string{ - "provider": providerName, - } + entityInfo := map[string]string{} if rs.RepositoryID.Valid { // this is always true now but might not be when we support entities not tied to a repo entityInfo["repo_name"] = rs.RepoName entityInfo["repo_owner"] = rs.RepoOwner + entityInfo["provider"] = rs.Provider entityInfo["repository_id"] = rs.RepositoryID.UUID.String() } @@ -267,6 +256,7 @@ func getRuleEvalEntityInfo( entityInfo["artifact_id"] = artifact.ID.String() entityInfo["artifact_name"] = artifact.ArtifactName entityInfo["artifact_type"] = artifact.ArtifactType + entityInfo["provider"] = artifact.Provider } return entityInfo @@ -279,7 +269,7 @@ func (s *Server) GetProfileStatusByName(ctx context.Context, entityCtx := engine.EntityFromContext(ctx) - err := entityCtx.Validate(ctx, s.store) + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } @@ -351,13 +341,12 @@ func (s *Server) GetProfileStatusByName(ctx context.Context, ruleEvaluationStatuses = s.getRuleEvaluationStatuses( ctx, dbRuleEvaluationStatuses, dbProfileStatus.ID.String(), - dbEntity, selector, entityCtx.Provider.Name, + dbEntity, selector, ) // TODO: Add other entities once we have database entries for them } // Telemetry logging - logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name logger.BusinessRecord(ctx).Project = entityCtx.Project.ID logger.BusinessRecord(ctx).Profile = logger.Profile{Name: dbProfileStatus.Name, ID: dbProfileStatus.ID} @@ -378,7 +367,6 @@ func (s *Server) getRuleEvaluationStatuses( profileId string, dbEntity *db.NullEntities, selector *uuid.NullUUID, - providerName string, ) []*minderv1.RuleEvaluationStatus { ruleEvaluationStatuses := make( []*minderv1.RuleEvaluationStatus, 0, len(dbRuleEvaluationStatuses), @@ -387,7 +375,7 @@ func (s *Server) getRuleEvaluationStatuses( // Loop through the rule evaluation statuses and convert them to protobuf for _, dbRuleEvalStat := range dbRuleEvaluationStatuses { // Get the rule evaluation status - st, err := getRuleEvalStatus(ctx, s.store, profileId, dbEntity, selector, providerName, dbRuleEvalStat) + st, err := getRuleEvalStatus(ctx, s.store, profileId, dbEntity, selector, dbRuleEvalStat) if err != nil { l.Err(err).Msg("error getting rule evaluation status") continue @@ -407,7 +395,6 @@ func getRuleEvalStatus( profileID string, dbEntity *db.NullEntities, selector *uuid.NullUUID, - providerName string, dbRuleEvalStat db.ListRuleEvaluationsByProfileIdRow, ) (*minderv1.RuleEvaluationStatus, error) { l := zerolog.Ctx(ctx) @@ -443,7 +430,7 @@ func getRuleEvalStatus( Entity: string(dbRuleEvalStat.Entity), Status: string(dbRuleEvalStat.EvalStatus.EvalStatusTypes), Details: dbRuleEvalStat.EvalDetails.String, - EntityInfo: getRuleEvalEntityInfo(ctx, store, dbEntity, selector, dbRuleEvalStat, providerName), + EntityInfo: getRuleEvalEntityInfo(ctx, store, dbEntity, selector, dbRuleEvalStat), Guidance: guidance, LastUpdated: timestamppb.New(dbRuleEvalStat.EvalLastUpdated.Time), RemediationStatus: string(dbRuleEvalStat.RemStatus.RemediationStatusTypes), @@ -502,7 +489,7 @@ func (s *Server) GetProfileStatusByProject(ctx context.Context, entityCtx := engine.EntityFromContext(ctx) - err := entityCtx.Validate(ctx, s.store) + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } @@ -529,7 +516,6 @@ func (s *Server) GetProfileStatusByProject(ctx context.Context, } // Telemetry logging - logger.BusinessRecord(ctx).Provider = entityCtx.Provider.Name logger.BusinessRecord(ctx).Project = entityCtx.Project.ID return res, nil @@ -540,7 +526,7 @@ func (s *Server) PatchProfile(ctx context.Context, ppr *minderv1.PatchProfileReq patch := ppr.GetPatch() entityCtx := engine.EntityFromContext(ctx) - err := entityCtx.Validate(ctx, s.store) + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } @@ -628,19 +614,13 @@ func (s *Server) UpdateProfile(ctx context.Context, entityCtx := engine.EntityFromContext(ctx) - err := entityCtx.Validate(ctx, s.store) + err := entityCtx.ValidateProject(ctx, s.store) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "error in entity context: %v", err) } - // TODO: This will be removed once we decouple providers from profiles - provider, err := s.providerStore.GetByName(ctx, entityCtx.Project.ID, in.GetContext().GetProvider()) - if err != nil { - return nil, providerError(err) - } - updatedProfile, err := db.WithTransaction(s.store, func(qtx db.ExtendQuerier) (*minderv1.Profile, error) { - return s.profiles.UpdateProfile(ctx, entityCtx.Project.ID, provider, uuid.Nil, in, qtx) + return s.profiles.UpdateProfile(ctx, entityCtx.Project.ID, uuid.Nil, in, qtx) }) if err != nil { diff --git a/internal/controlplane/handlers_profile_test.go b/internal/controlplane/handlers_profile_test.go index b4a7c12445..bb92c22776 100644 --- a/internal/controlplane/handlers_profile_test.go +++ b/internal/controlplane/handlers_profile_test.go @@ -227,18 +227,6 @@ func TestCreateProfile(t *testing.T) { t.Fatalf("Error creating project: %v", err) } - // The provider is used in the profile definition - _, err = dbStore.CreateProvider(ctx, db.CreateProviderParams{ - Name: "github", - ProjectID: dbproj.ID, - Class: db.NullProviderClass{ProviderClass: db.ProviderClassGithub, Valid: true}, - Implements: []db.ProviderType{db.ProviderTypeGithub}, - AuthFlows: []db.AuthorizationFlow{db.AuthorizationFlowUserInput}, - Definition: []byte(`{}`), - }) - if err != nil { - t.Fatalf("Error creating provider: %v", err) - } _, err = dbStore.CreateRuleType(ctx, db.CreateRuleTypeParams{ Name: "rule_type_1", ProjectID: dbproj.ID, @@ -339,8 +327,7 @@ func TestCreateProfile(t *testing.T) { if tc.profile.GetContext() == nil { tc.profile.Profile.Context = &minderv1.Context{ - Project: proto.String(dbproj.ID.String()), - Provider: proto.String("github"), + Project: proto.String(dbproj.ID.String()), } if tc.result != nil { tc.result.GetProfile().Context = tc.profile.GetContext() @@ -348,8 +335,7 @@ func TestCreateProfile(t *testing.T) { } ctx := engine.WithEntityContext(context.Background(), &engine.EntityContext{ - Project: engine.Project{ID: dbproj.ID}, - Provider: engine.Provider{Name: "github"}, + Project: engine.Project{ID: dbproj.ID}, }) evts := &stubeventer.StubEventer{} s := &Server{ diff --git a/internal/db/models.go b/internal/db/models.go index 14e7cb57f0..63df266abb 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -486,13 +486,13 @@ type MigrationProfileBackfillLog struct { type Profile struct { ID uuid.UUID `json:"id"` Name string `json:"name"` - Provider string `json:"provider"` + Provider sql.NullString `json:"provider"` ProjectID uuid.UUID `json:"project_id"` Remediate NullActionType `json:"remediate"` Alert NullActionType `json:"alert"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` - ProviderID uuid.UUID `json:"provider_id"` + ProviderID uuid.NullUUID `json:"provider_id"` SubscriptionID uuid.NullUUID `json:"subscription_id"` DisplayName string `json:"display_name"` Labels []string `json:"labels"` diff --git a/internal/db/profiles.sql.go b/internal/db/profiles.sql.go index 93b2c67cfb..6f81a3ce9a 100644 --- a/internal/db/profiles.sql.go +++ b/internal/db/profiles.sql.go @@ -7,6 +7,7 @@ package db import ( "context" + "database/sql" "encoding/json" "time" @@ -62,25 +63,21 @@ func (q *Queries) CountProfilesByName(ctx context.Context, name string) (int64, const createProfile = `-- name: CreateProfile :one INSERT INTO profiles ( - provider, project_id, remediate, alert, name, - provider_id, subscription_id, display_name, labels -) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, COALESCE($9::text[], '{}'::text[])) RETURNING id, name, provider, project_id, remediate, alert, created_at, updated_at, provider_id, subscription_id, display_name, labels +) VALUES ($1, $2, $3, $4, $5, $6, COALESCE($7::text[], '{}'::text[])) RETURNING id, name, provider, project_id, remediate, alert, created_at, updated_at, provider_id, subscription_id, display_name, labels ` type CreateProfileParams struct { - Provider string `json:"provider"` ProjectID uuid.UUID `json:"project_id"` Remediate NullActionType `json:"remediate"` Alert NullActionType `json:"alert"` Name string `json:"name"` - ProviderID uuid.UUID `json:"provider_id"` SubscriptionID uuid.NullUUID `json:"subscription_id"` DisplayName string `json:"display_name"` Labels []string `json:"labels"` @@ -88,12 +85,10 @@ type CreateProfileParams struct { func (q *Queries) CreateProfile(ctx context.Context, arg CreateProfileParams) (Profile, error) { row := q.db.QueryRowContext(ctx, createProfile, - arg.Provider, arg.ProjectID, arg.Remediate, arg.Alert, arg.Name, - arg.ProviderID, arg.SubscriptionID, arg.DisplayName, pq.Array(arg.Labels), @@ -199,13 +194,13 @@ type GetEntityProfileByProjectAndNameParams struct { type GetEntityProfileByProjectAndNameRow struct { ID uuid.UUID `json:"id"` Name string `json:"name"` - Provider string `json:"provider"` + Provider sql.NullString `json:"provider"` ProjectID uuid.UUID `json:"project_id"` Remediate NullActionType `json:"remediate"` Alert NullActionType `json:"alert"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` - ProviderID uuid.UUID `json:"provider_id"` + ProviderID uuid.NullUUID `json:"provider_id"` SubscriptionID uuid.NullUUID `json:"subscription_id"` DisplayName string `json:"display_name"` Labels []string `json:"labels"` @@ -359,13 +354,13 @@ type GetProfileByProjectAndIDParams struct { type GetProfileByProjectAndIDRow struct { ID uuid.UUID `json:"id"` Name string `json:"name"` - Provider string `json:"provider"` + Provider sql.NullString `json:"provider"` ProjectID uuid.UUID `json:"project_id"` Remediate NullActionType `json:"remediate"` Alert NullActionType `json:"alert"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` - ProviderID uuid.UUID `json:"provider_id"` + ProviderID uuid.NullUUID `json:"provider_id"` SubscriptionID uuid.NullUUID `json:"subscription_id"` DisplayName string `json:"display_name"` Labels []string `json:"labels"` diff --git a/internal/db/profiles_test.go b/internal/db/profiles_test.go index 69d2e55fb5..d2e49c0495 100644 --- a/internal/db/profiles_test.go +++ b/internal/db/profiles_test.go @@ -15,16 +15,14 @@ import ( "github.com/stacklok/minder/internal/util/rand" ) -func createRandomProfile(t *testing.T, prov Provider, projectID uuid.UUID, labels []string) Profile { +func createRandomProfile(t *testing.T, projectID uuid.UUID, labels []string) Profile { t.Helper() seed := time.Now().UnixNano() arg := CreateProfileParams{ - Name: rand.RandomName(seed), - Provider: prov.Name, - ProviderID: prov.ID, - ProjectID: projectID, + Name: rand.RandomName(seed), + ProjectID: projectID, Remediate: NullActionType{ ActionType: "on", Valid: true, @@ -199,15 +197,15 @@ func TestProfileLabels(t *testing.T) { randomEntities := createTestRandomEntities(t) - health1 := createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{"stacklok:health"}) + health1 := createRandomProfile(t, randomEntities.proj.ID, []string{"stacklok:health"}) require.NotEmpty(t, health1) - health2 := createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{"stacklok:health", "obsolete"}) + health2 := createRandomProfile(t, randomEntities.proj.ID, []string{"stacklok:health", "obsolete"}) require.NotEmpty(t, health2) - obsolete := createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{"obsolete"}) + obsolete := createRandomProfile(t, randomEntities.proj.ID, []string{"obsolete"}) require.NotEmpty(t, obsolete) - p1 := createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{}) + p1 := createRandomProfile(t, randomEntities.proj.ID, []string{}) require.NotEmpty(t, p1) - p2 := createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{}) + p2 := createRandomProfile(t, randomEntities.proj.ID, []string{}) require.NotEmpty(t, p2) tests := []struct { @@ -475,7 +473,7 @@ func TestCreateProfileStatusStoredProcedure(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - profile := createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{}) + profile := createRandomProfile(t, randomEntities.proj.ID, []string{}) require.NotEmpty(t, profile) tt.ruleStatusSetupFn(profile, randomEntities) @@ -608,7 +606,7 @@ func TestCreateProfileStatusStoredDeleteProcedure(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - profile := createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{}) + profile := createRandomProfile(t, randomEntities.proj.ID, []string{}) require.NotEmpty(t, profile) delRepo := createRandomRepository(t, randomEntities.proj.ID, randomEntities.prov) @@ -843,7 +841,7 @@ func TestListRuleEvaluations(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - tt.profile = createRandomProfile(t, randomEntities.prov, randomEntities.proj.ID, []string{}) + tt.profile = createRandomProfile(t, randomEntities.proj.ID, []string{}) require.NotEmpty(t, tt.profile) tt.ruleStatusSetupFn(tt.profile, randomEntities) diff --git a/internal/engine/executor_test.go b/internal/engine/executor_test.go index bfad9dd7c7..0514769543 100644 --- a/internal/engine/executor_test.go +++ b/internal/engine/executor_test.go @@ -149,7 +149,6 @@ func TestExecutor_handleEntityEvent(t *testing.T) { Profile: db.Profile{ ID: profileID, Name: "test-profile", - Provider: providerName, ProjectID: projectID, CreatedAt: time.Now(), UpdatedAt: time.Now(), diff --git a/internal/engine/profile.go b/internal/engine/profile.go index ec7a4017d6..23af49d48a 100644 --- a/internal/engine/profile.go +++ b/internal/engine/profile.go @@ -29,7 +29,6 @@ import ( "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine/entities" "github.com/stacklok/minder/internal/util/jsonyaml" - "github.com/stacklok/minder/internal/util/ptr" pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" ) @@ -198,8 +197,7 @@ func MergeDatabaseListIntoProfiles[T db.ProfileRow](ppl []T) map[string]*pb.Prof Name: p.GetProfile().Name, DisplayName: displayName, Context: &pb.Context{ - Provider: ptr.Ptr[string](p.GetProfile().Provider), - Project: &project, + Project: &project, }, } @@ -253,8 +251,7 @@ func MergeDatabaseGetIntoProfiles(ppl []db.GetProfileByProjectAndIDRow) map[stri Name: p.Name, DisplayName: displayName, Context: &pb.Context{ - Provider: &p.Provider, - Project: &project, + Project: &project, }, } diff --git a/internal/marketplaces/service.go b/internal/marketplaces/service.go index e16a8a47e7..2f53c2ee96 100644 --- a/internal/marketplaces/service.go +++ b/internal/marketplaces/service.go @@ -20,9 +20,10 @@ import ( "context" "fmt" + "github.com/google/uuid" + "github.com/stacklok/minder/internal/db" sub "github.com/stacklok/minder/internal/marketplaces/subscriptions" - "github.com/stacklok/minder/internal/marketplaces/types" "github.com/stacklok/minder/pkg/mindpak" "github.com/stacklok/minder/pkg/mindpak/reader" "github.com/stacklok/minder/pkg/mindpak/sources" @@ -36,14 +37,14 @@ type Marketplace interface { // bundle and adds all rules from that bundle to the project. Subscribe( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundleID mindpak.BundleID, qtx db.Querier, ) error // AddProfile adds the specified profile from the bundle to the project. AddProfile( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundleID mindpak.BundleID, profileName string, qtx db.Querier, @@ -61,7 +62,7 @@ type marketplace struct { func (s *marketplace) Subscribe( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundleID mindpak.BundleID, qtx db.Querier, ) error { @@ -69,7 +70,7 @@ func (s *marketplace) Subscribe( if err != nil { return err } - if err = s.subscriptions.Subscribe(ctx, project, bundle, qtx); err != nil { + if err = s.subscriptions.Subscribe(ctx, projectID, bundle, qtx); err != nil { return fmt.Errorf("error while creating subscription: %w", err) } return nil @@ -77,7 +78,7 @@ func (s *marketplace) Subscribe( func (s *marketplace) AddProfile( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundleID mindpak.BundleID, profileName string, qtx db.Querier, @@ -87,7 +88,7 @@ func (s *marketplace) AddProfile( return err } - if err = s.subscriptions.CreateProfile(ctx, project, bundle, profileName, qtx); err != nil { + if err = s.subscriptions.CreateProfile(ctx, projectID, bundle, profileName, qtx); err != nil { return fmt.Errorf("error while creating profile in project: %w", err) } @@ -110,13 +111,13 @@ func (s *marketplace) getBundle(bundleID mindpak.BundleID) (reader.BundleReader, // This is used when the Marketplace functionality is disabled type noopMarketplace struct{} -func (_ *noopMarketplace) Subscribe(_ context.Context, _ types.ProjectContext, _ mindpak.BundleID, _ db.Querier) error { +func (_ *noopMarketplace) Subscribe(_ context.Context, _ uuid.UUID, _ mindpak.BundleID, _ db.Querier) error { return nil } func (_ *noopMarketplace) AddProfile( _ context.Context, - _ types.ProjectContext, + _ uuid.UUID, _ mindpak.BundleID, _ string, _ db.Querier, diff --git a/internal/marketplaces/service_test.go b/internal/marketplaces/service_test.go index 86b7f129ab..8df2731e1c 100644 --- a/internal/marketplaces/service_test.go +++ b/internal/marketplaces/service_test.go @@ -22,14 +22,12 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" - "github.com/stacklok/minder/internal/db" dbf "github.com/stacklok/minder/internal/db/fixtures" "github.com/stacklok/minder/internal/marketplaces" mockbundle "github.com/stacklok/minder/internal/marketplaces/bundles/mock" bsf "github.com/stacklok/minder/internal/marketplaces/bundles/mock/fixtures" "github.com/stacklok/minder/internal/marketplaces/subscriptions" ssf "github.com/stacklok/minder/internal/marketplaces/subscriptions/mock/fixtures" - "github.com/stacklok/minder/internal/marketplaces/types" "github.com/stacklok/minder/pkg/mindpak" "github.com/stacklok/minder/pkg/mindpak/sources" ) @@ -115,9 +113,9 @@ func testHarness(t *testing.T, method testMethod, scenarios []testScenario) { switch method { case subscribe: - err = marketplace.Subscribe(ctx, projectContext, bundleID, store) + err = marketplace.Subscribe(ctx, projectID, bundleID, store) case createProfile: - err = marketplace.AddProfile(ctx, projectContext, bundleID, profileName, store) + err = marketplace.AddProfile(ctx, projectID, bundleID, profileName, store) default: t.Fatalf("unknown method %d", method) } @@ -133,12 +131,9 @@ func testHarness(t *testing.T, method testMethod, scenarios []testScenario) { // use nil controller - we do not need to mock any methods on this type var ( - bundleReader = mockbundle.NewMockBundleReader(nil) - projectContext = types.ProjectContext{ - ID: uuid.New(), - Provider: &db.Provider{}, - } - bundleID = mindpak.ID("stacklok", "healthcheck") + bundleReader = mockbundle.NewMockBundleReader(nil) + projectID = uuid.New() + bundleID = mindpak.ID("stacklok", "healthcheck") ) const ( diff --git a/internal/marketplaces/subscriptions/mock/subscription.go b/internal/marketplaces/subscriptions/mock/subscription.go index 458ee1a664..f9951f41f9 100644 --- a/internal/marketplaces/subscriptions/mock/subscription.go +++ b/internal/marketplaces/subscriptions/mock/subscription.go @@ -13,8 +13,8 @@ import ( context "context" reflect "reflect" + uuid "github.com/google/uuid" db "github.com/stacklok/minder/internal/db" - types "github.com/stacklok/minder/internal/marketplaces/types" reader "github.com/stacklok/minder/pkg/mindpak/reader" gomock "go.uber.org/mock/gomock" ) @@ -43,7 +43,7 @@ func (m *MockSubscriptionService) EXPECT() *MockSubscriptionServiceMockRecorder } // CreateProfile mocks base method. -func (m *MockSubscriptionService) CreateProfile(arg0 context.Context, arg1 types.ProjectContext, arg2 reader.BundleReader, arg3 string, arg4 db.Querier) error { +func (m *MockSubscriptionService) CreateProfile(arg0 context.Context, arg1 uuid.UUID, arg2 reader.BundleReader, arg3 string, arg4 db.Querier) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateProfile", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(error) @@ -57,7 +57,7 @@ func (mr *MockSubscriptionServiceMockRecorder) CreateProfile(arg0, arg1, arg2, a } // Subscribe mocks base method. -func (m *MockSubscriptionService) Subscribe(arg0 context.Context, arg1 types.ProjectContext, arg2 reader.BundleReader, arg3 db.Querier) error { +func (m *MockSubscriptionService) Subscribe(arg0 context.Context, arg1 uuid.UUID, arg2 reader.BundleReader, arg3 db.Querier) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Subscribe", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(error) diff --git a/internal/marketplaces/subscriptions/service.go b/internal/marketplaces/subscriptions/service.go index 16a267c7c8..9d1eafa2b2 100644 --- a/internal/marketplaces/subscriptions/service.go +++ b/internal/marketplaces/subscriptions/service.go @@ -26,7 +26,6 @@ import ( "github.com/google/uuid" "github.com/stacklok/minder/internal/db" - "github.com/stacklok/minder/internal/marketplaces/types" profsvc "github.com/stacklok/minder/internal/profiles" "github.com/stacklok/minder/internal/ruletypes" minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" @@ -43,14 +42,14 @@ type SubscriptionService interface { // and bundle. It is a no-op if the project is already subscribed. Subscribe( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundle reader.BundleReader, qtx db.Querier, ) error // CreateProfile creates the specified profile from the bundle in the project. CreateProfile( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundle reader.BundleReader, profileName string, qtx db.Querier, @@ -75,7 +74,7 @@ func NewSubscriptionService( func (s *subscriptionService) Subscribe( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundle reader.BundleReader, qtx db.Querier, ) error { @@ -83,7 +82,7 @@ func (s *subscriptionService) Subscribe( _, err := qtx.GetSubscriptionByProjectBundle(ctx, db.GetSubscriptionByProjectBundleParams{ Namespace: metadata.Namespace, Name: metadata.Name, - ProjectID: project.ID, + ProjectID: projectID, }) // project already subscribed to bundle, skip if err == nil { @@ -102,7 +101,7 @@ func (s *subscriptionService) Subscribe( // create subscription subscription, err := qtx.CreateSubscription(ctx, db.CreateSubscriptionParams{ - ProjectID: project.ID, + ProjectID: projectID, BundleID: bundleID, CurrentVersion: metadata.Version, }) @@ -111,7 +110,7 @@ func (s *subscriptionService) Subscribe( } // populate all rule types from this bundle into the project - err = s.upsertBundleRules(ctx, qtx, project.ID, bundle, subscription.ID) + err = s.upsertBundleRules(ctx, qtx, projectID, bundle, subscription.ID) if err != nil { return fmt.Errorf("error while creating rules in project: %w", err) } @@ -120,13 +119,13 @@ func (s *subscriptionService) Subscribe( func (s *subscriptionService) CreateProfile( ctx context.Context, - project types.ProjectContext, + projectID uuid.UUID, bundle reader.BundleReader, profileName string, qtx db.Querier, ) error { // ensure project is subscribed to this bundle - subscription, err := s.findSubscription(ctx, qtx, project.ID, bundle.GetMetadata()) + subscription, err := s.findSubscription(ctx, qtx, projectID, bundle.GetMetadata()) if err != nil { return err } @@ -136,7 +135,7 @@ func (s *subscriptionService) CreateProfile( return fmt.Errorf("error while retrieving profile from bundle: %w", err) } - _, err = s.profiles.CreateProfile(ctx, project.ID, project.Provider, subscription.ID, profile, qtx) + _, err = s.profiles.CreateProfile(ctx, projectID, subscription.ID, profile, qtx) if err != nil { return fmt.Errorf("error while creating profile in project: %w", err) } diff --git a/internal/marketplaces/subscriptions/service_test.go b/internal/marketplaces/subscriptions/service_test.go index 3ea417e094..613c4b42dd 100644 --- a/internal/marketplaces/subscriptions/service_test.go +++ b/internal/marketplaces/subscriptions/service_test.go @@ -28,7 +28,6 @@ import ( dbf "github.com/stacklok/minder/internal/db/fixtures" brf "github.com/stacklok/minder/internal/marketplaces/bundles/mock/fixtures" "github.com/stacklok/minder/internal/marketplaces/subscriptions" - "github.com/stacklok/minder/internal/marketplaces/types" "github.com/stacklok/minder/internal/profiles" psf "github.com/stacklok/minder/internal/profiles/mock/fixtures" "github.com/stacklok/minder/internal/ruletypes" @@ -106,7 +105,7 @@ func TestSubscriptionService_Subscribe(t *testing.T) { querier := getQuerier(ctrl, scenario.DBSetup) svc := createService(ctrl, nil, scenario.RuleTypeSetup) - err := svc.Subscribe(ctx, projectContext, bundle, querier) + err := svc.Subscribe(ctx, projectID, bundle, querier) if scenario.ExpectedError == "" { require.NoError(t, err) } else { @@ -171,7 +170,7 @@ func TestSubscriptionService_CreateProfile(t *testing.T) { querier := getQuerier(ctrl, scenario.DBSetup) svc := createService(ctrl, scenario.ProfileSetup, nil) - err := svc.CreateProfile(ctx, projectContext, bundle, profileName, querier) + err := svc.CreateProfile(ctx, projectID, bundle, profileName, querier) if scenario.ExpectedError == "" { require.NoError(t, err) } else { @@ -190,11 +189,6 @@ var ( subscriptionID = uuid.New() projectID = uuid.New() bundleID = uuid.New() - provider = db.Provider{ - ID: uuid.New(), - Name: "github", - } - projectContext = types.NewProjectContext(projectID, &provider) ) func withNotFoundFindSubscription(mock dbf.DBMock) { diff --git a/internal/marketplaces/types/types.go b/internal/marketplaces/types/types.go deleted file mode 100644 index 543aad3560..0000000000 --- a/internal/marketplaces/types/types.go +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2024 Stacklok, Inc -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package types contains domain models for marketplaces -package types - -import ( - "github.com/google/uuid" - - "github.com/stacklok/minder/internal/db" -) - -// ProjectContext contains the information needed to create rule types and -// profiles in a project. -// This may be useful in various parts of the codebase outside of marketplaces. -type ProjectContext struct { - // Project ID - ID uuid.UUID - // Provider which profiles/rule types will be linked to - Provider *db.Provider -} - -// NewProjectContext is a convenience function for creating a ProjectContext -func NewProjectContext(id uuid.UUID, provider *db.Provider) ProjectContext { - return ProjectContext{ - ID: id, - Provider: provider, - } -} diff --git a/internal/profiles/mock/fixtures/service.go b/internal/profiles/mock/fixtures/service.go index 0f1edea9e6..35b7599b8a 100644 --- a/internal/profiles/mock/fixtures/service.go +++ b/internal/profiles/mock/fixtures/service.go @@ -46,24 +46,24 @@ var ( func WithSuccessfulCreateSubscriptionProfile(mock ProfileSvcMock) { mock.EXPECT(). - CreateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + CreateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&minderv1.Profile{}, nil) } func WithFailedCreateSubscriptionProfile(mock ProfileSvcMock) { mock.EXPECT(). - CreateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + CreateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, errDefault) } func WithSuccessfulUpdateSubscriptionProfile(mock ProfileSvcMock) { mock.EXPECT(). - UpdateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + UpdateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(&minderv1.Profile{}, nil) } func WithFailedUpdateSubscriptionProfile(mock ProfileSvcMock) { mock.EXPECT(). - UpdateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + UpdateProfile(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, errDefault) } diff --git a/internal/profiles/mock/service.go b/internal/profiles/mock/service.go index 249b2d8c0b..7dc400fcda 100644 --- a/internal/profiles/mock/service.go +++ b/internal/profiles/mock/service.go @@ -43,31 +43,31 @@ func (m *MockProfileService) EXPECT() *MockProfileServiceMockRecorder { } // CreateProfile mocks base method. -func (m *MockProfileService) CreateProfile(arg0 context.Context, arg1 uuid.UUID, arg2 *db.Provider, arg3 uuid.UUID, arg4 *v1.Profile, arg5 db.Querier) (*v1.Profile, error) { +func (m *MockProfileService) CreateProfile(arg0 context.Context, arg1, arg2 uuid.UUID, arg3 *v1.Profile, arg4 db.Querier) (*v1.Profile, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateProfile", arg0, arg1, arg2, arg3, arg4, arg5) + ret := m.ctrl.Call(m, "CreateProfile", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(*v1.Profile) ret1, _ := ret[1].(error) return ret0, ret1 } // CreateProfile indicates an expected call of CreateProfile. -func (mr *MockProfileServiceMockRecorder) CreateProfile(arg0, arg1, arg2, arg3, arg4, arg5 any) *gomock.Call { +func (mr *MockProfileServiceMockRecorder) CreateProfile(arg0, arg1, arg2, arg3, arg4 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateProfile", reflect.TypeOf((*MockProfileService)(nil).CreateProfile), arg0, arg1, arg2, arg3, arg4, arg5) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateProfile", reflect.TypeOf((*MockProfileService)(nil).CreateProfile), arg0, arg1, arg2, arg3, arg4) } // UpdateProfile mocks base method. -func (m *MockProfileService) UpdateProfile(arg0 context.Context, arg1 uuid.UUID, arg2 *db.Provider, arg3 uuid.UUID, arg4 *v1.Profile, arg5 db.Querier) (*v1.Profile, error) { +func (m *MockProfileService) UpdateProfile(arg0 context.Context, arg1, arg2 uuid.UUID, arg3 *v1.Profile, arg4 db.Querier) (*v1.Profile, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateProfile", arg0, arg1, arg2, arg3, arg4, arg5) + ret := m.ctrl.Call(m, "UpdateProfile", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(*v1.Profile) ret1, _ := ret[1].(error) return ret0, ret1 } // UpdateProfile indicates an expected call of UpdateProfile. -func (mr *MockProfileServiceMockRecorder) UpdateProfile(arg0, arg1, arg2, arg3, arg4, arg5 any) *gomock.Call { +func (mr *MockProfileServiceMockRecorder) UpdateProfile(arg0, arg1, arg2, arg3, arg4 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateProfile", reflect.TypeOf((*MockProfileService)(nil).UpdateProfile), arg0, arg1, arg2, arg3, arg4, arg5) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateProfile", reflect.TypeOf((*MockProfileService)(nil).UpdateProfile), arg0, arg1, arg2, arg3, arg4) } diff --git a/internal/profiles/service.go b/internal/profiles/service.go index c5b684abf3..fd82759614 100644 --- a/internal/profiles/service.go +++ b/internal/profiles/service.go @@ -51,7 +51,6 @@ type ProfileService interface { CreateProfile( ctx context.Context, projectID uuid.UUID, - provider *db.Provider, subscriptionID uuid.UUID, profile *minderv1.Profile, qtx db.Querier, @@ -62,7 +61,6 @@ type ProfileService interface { UpdateProfile( ctx context.Context, projectID uuid.UUID, - provider *db.Provider, subscriptionID uuid.UUID, profile *minderv1.Profile, qtx db.Querier, @@ -89,13 +87,11 @@ func NewProfileService(publisher events.Publisher) ProfileService { func (p *profileService) CreateProfile( ctx context.Context, projectID uuid.UUID, - provider *db.Provider, subscriptionID uuid.UUID, profile *minderv1.Profile, qtx db.Querier, ) (*minderv1.Profile, error) { // Telemetry logging - logger.BusinessRecord(ctx).Provider = provider.Name logger.BusinessRecord(ctx).Project = projectID rulesInProf, err := p.validator.ValidateAndExtractRules(ctx, qtx, projectID, profile) @@ -121,8 +117,6 @@ func (p *profileService) CreateProfile( } params := db.CreateProfileParams{ - Provider: provider.Name, - ProviderID: provider.ID, ProjectID: projectID, Name: profile.GetName(), DisplayName: displayName, @@ -155,12 +149,11 @@ func (p *profileService) CreateProfile( } logger.BusinessRecord(ctx).Profile = logger.Profile{Name: profile.Name, ID: newProfile.ID} - p.sendNewProfileEvent(provider.Name, projectID) + p.sendNewProfileEvent(projectID) profile.Id = ptr.Ptr(newProfile.ID.String()) profile.Context = &minderv1.Context{ - Provider: &newProfile.Provider, - Project: ptr.Ptr(newProfile.ProjectID.String()), + Project: ptr.Ptr(newProfile.ProjectID.String()), } profile.Remediate = ptr.Ptr(string(newProfile.Remediate.ActionType)) @@ -174,13 +167,11 @@ func (p *profileService) CreateProfile( func (p *profileService) UpdateProfile( ctx context.Context, projectID uuid.UUID, - provider *db.Provider, subscriptionID uuid.UUID, profile *minderv1.Profile, qtx db.Querier, ) (*minderv1.Profile, error) { // Telemetry logging - logger.BusinessRecord(ctx).Provider = provider.Name logger.BusinessRecord(ctx).Project = projectID rules, err := p.validator.ValidateAndExtractRules(ctx, qtx, projectID, profile) @@ -203,7 +194,7 @@ func (p *profileService) UpdateProfile( } // validate update - if err = validateProfileUpdate(oldDBProfile, profile, projectID, provider); err != nil { + if err = validateProfileUpdate(oldDBProfile, profile, projectID); err != nil { return nil, util.UserVisibleError(codes.InvalidArgument, "invalid profile update: %v", err) } @@ -273,15 +264,14 @@ func (p *profileService) UpdateProfile( profile.Id = ptr.Ptr(updatedProfile.ID.String()) profile.Context = &minderv1.Context{ - Provider: &updatedProfile.Provider, - Project: ptr.Ptr(updatedProfile.ProjectID.String()), + Project: ptr.Ptr(updatedProfile.ProjectID.String()), } profile.Remediate = ptr.Ptr(string(updatedProfile.Remediate.ActionType)) profile.Alert = ptr.Ptr(string(updatedProfile.Alert.ActionType)) // re-trigger profile evaluation - p.sendNewProfileEvent(provider.Name, projectID) + p.sendNewProfileEvent(projectID) return profile, nil } @@ -338,11 +328,10 @@ func createProfileRulesForEntity( } func (p *profileService) sendNewProfileEvent( - providerName string, projectID uuid.UUID, ) { // both errors in this case are considered non-fatal - msg, err := reconcilers.NewProfileInitMessage(providerName, projectID) + msg, err := reconcilers.NewProfileInitMessage(projectID) if err != nil { log.Printf("error creating reconciler event: %v", err) } @@ -409,7 +398,6 @@ func validateProfileUpdate( old *db.Profile, new *minderv1.Profile, projectID uuid.UUID, - provider *db.Provider, ) error { if old.Name != new.Name { return util.UserVisibleError(codes.InvalidArgument, "cannot change profile name") @@ -419,10 +407,6 @@ func validateProfileUpdate( return util.UserVisibleError(codes.InvalidArgument, "cannot change profile project") } - if old.Provider != provider.Name { - return util.UserVisibleError(codes.InvalidArgument, "cannot change profile provider") - } - if err := namespaces.ValidateLabelsUpdate(new.GetLabels(), old.Labels); err != nil { return util.UserVisibleError(codes.InvalidArgument, "labels update failed validation: %v", err) } diff --git a/internal/projects/create.go b/internal/projects/create.go index 240fe56e90..0e4a15954f 100644 --- a/internal/projects/create.go +++ b/internal/projects/create.go @@ -29,7 +29,6 @@ import ( "github.com/stacklok/minder/internal/config/server" "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/marketplaces" - "github.com/stacklok/minder/internal/marketplaces/types" github "github.com/stacklok/minder/internal/providers/github/oauth" pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" "github.com/stacklok/minder/pkg/mindpak" @@ -102,7 +101,7 @@ func ProvisionSelfEnrolledProject( } // Create GitHub provider - dbProvider, err := qtx.CreateProvider(ctx, db.CreateProviderParams{ + _, err = qtx.CreateProvider(ctx, db.CreateProviderParams{ Name: github.Github, ProjectID: project.ID, Class: db.NullProviderClass{ProviderClass: db.ProviderClassGithub, Valid: true}, @@ -117,13 +116,12 @@ func ProvisionSelfEnrolledProject( // Enable any default profiles and rule types in the project. // For now, we subscribe to a single bundle and a single profile. // Both are specified in the service config. - projectContext := types.NewProjectContext(project.ID, &dbProvider) bundleID := mindpak.ID(profilesCfg.Bundle.Namespace, profilesCfg.Bundle.Name) - if err := marketplace.Subscribe(ctx, projectContext, bundleID, qtx); err != nil { + if err := marketplace.Subscribe(ctx, project.ID, bundleID, qtx); err != nil { return nil, fmt.Errorf("unable to subscribe to bundle: %w", err) } for _, profileName := range profilesCfg.GetProfiles() { - if err := marketplace.AddProfile(ctx, projectContext, bundleID, profileName, qtx); err != nil { + if err := marketplace.AddProfile(ctx, project.ID, bundleID, profileName, qtx); err != nil { return nil, fmt.Errorf("unable to enable bundle profile: %w", err) } } diff --git a/internal/reconcilers/run_profile.go b/internal/reconcilers/run_profile.go index b3b8c2215f..b7769ecb2a 100644 --- a/internal/reconcilers/run_profile.go +++ b/internal/reconcilers/run_profile.go @@ -18,7 +18,6 @@ import ( "context" "database/sql" "encoding/json" - "errors" "fmt" "github.com/ThreeDotsLabs/watermill/message" @@ -30,7 +29,6 @@ import ( "github.com/stacklok/minder/internal/db" "github.com/stacklok/minder/internal/engine" "github.com/stacklok/minder/internal/engine/entities" - "github.com/stacklok/minder/internal/events" "github.com/stacklok/minder/internal/util" ) @@ -44,7 +42,7 @@ type ProfileInitEvent struct { } // NewProfileInitMessage creates a new repos init event -func NewProfileInitMessage(provider string, projectID uuid.UUID) (*message.Message, error) { +func NewProfileInitMessage(projectID uuid.UUID) (*message.Message, error) { evt := &ProfileInitEvent{ Project: projectID, } @@ -55,7 +53,6 @@ func NewProfileInitMessage(provider string, projectID uuid.UUID) (*message.Messa } msg := message.NewMessage(uuid.New().String(), evtStr) - msg.Metadata.Set(events.ProviderTypeKey, provider) return msg, nil } @@ -64,7 +61,6 @@ func NewProfileInitMessage(provider string, projectID uuid.UUID) (*message.Messa // for the project and sending a profile evaluation event for each one. func (r *Reconciler) handleProfileInitEvent(msg *message.Message) error { ctx := msg.Context() - prov := msg.Metadata.Get(events.ProviderTypeKey) var evt ProfileInitEvent if err := json.Unmarshal(msg.Payload, &evt); err != nil { @@ -81,40 +77,17 @@ func (r *Reconciler) handleProfileInitEvent(msg *message.Message) error { return nil } - projHierarchy, err := r.store.GetParentProjects(ctx, evt.Project) - if err != nil { - return fmt.Errorf("error getting project hierarchy: %w", err) - } - - provInfo, err := r.store.GetProviderByName(context.Background(), db.GetProviderByNameParams{ - Name: prov, - Projects: projHierarchy, - }) - if err != nil { - if errors.Is(err, sql.ErrNoRows) { - // We don't return the event since there's no use - // retrying it if the provider doesn't exist. - zerolog.Ctx(ctx).Error().Str("provider", prov).Msg("provider not found") - return nil - } - - return fmt.Errorf("error getting provider: %w", err) - } - ectx := &engine.EntityContext{ Project: engine.Project{ ID: evt.Project, }, - Provider: engine.Provider{ - Name: provInfo.Name, - }, } - zerolog.Ctx(ctx).Debug().Str("provider", prov).Msg("handling profile init event") + zerolog.Ctx(ctx).Debug().Msg("handling profile init event") if err := r.publishProfileInitEvents(ctx, ectx); err != nil { // We don't return an error since watermill will retry // the message. - zerolog.Ctx(ctx).Error().Str("provider", prov).Msg("error publishing profile events") + zerolog.Ctx(ctx).Error().Msg("error publishing profile events") return nil } @@ -141,7 +114,7 @@ func (r *Reconciler) publishProfileInitEvents( // protobufs are our API, so we always execute on these instead of the DB directly. repo := util.PBRepositoryFromDB(dbrepo) err := entities.NewEntityInfoWrapper(). - WithProvider(ectx.Provider.Name). + WithProvider(dbrepo.Provider). WithProjectID(ectx.Project.ID). WithRepository(repo). WithRepositoryID(dbrepo.ID). @@ -188,7 +161,7 @@ func (r *Reconciler) publishArtifactProfileInitEvents( } err = entities.NewEntityInfoWrapper(). - WithProvider(ectx.Provider.Name). + WithProvider(dbrepo.Provider). WithProjectID(ectx.Project.ID). WithArtifact(pbArtifact). WithRepositoryID(dbrepo.ID). diff --git a/internal/util/cli/table/simple/simple.go b/internal/util/cli/table/simple/simple.go index 68184b9274..602aee6504 100644 --- a/internal/util/cli/table/simple/simple.go +++ b/internal/util/cli/table/simple/simple.go @@ -94,7 +94,7 @@ func keyValueLayout(table *tablewriter.Table) { func profileSettingsLayout(table *tablewriter.Table) { defaultLayout(table) - table.SetHeader([]string{"ID", "Name", "Provider", "Alert", "Remediate"}) + table.SetHeader([]string{"ID", "Name", "Alert", "Remediate"}) table.SetColMinWidth(1, 50) }