Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add delete provider command #2922

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/cli/app/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var ProviderCmd = &cobra.Command{
func init() {
app.RootCmd.AddCommand(ProviderCmd)
// Flags for all subcommands
// TODO: remove the provider flag from here and add it only to the subcommands that need it
ProviderCmd.PersistentFlags().StringP("provider", "p", ghclient.Github, "Name of the provider, i.e. github")
ProviderCmd.PersistentFlags().StringP("project", "j", "", "ID of the project")
}
Expand Down
81 changes: 81 additions & 0 deletions cmd/cli/app/provider/provider_delete.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
//
// 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 provider

import (
"context"

"github.com/spf13/cobra"
"github.com/spf13/viper"
"google.golang.org/grpc"

"github.com/stacklok/minder/internal/util/cli"
minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
)

var deleteCmd = &cobra.Command{
Use: "delete",
Short: "Delete a given provider available in a specific project",
Long: `The minder provider delete command deletes a given provider available in a specific project.`,
RunE: cli.GRPCClientWrapRunE(DeleteProviderCommand),
}

func init() {
ProviderCmd.AddCommand(deleteCmd)

deleteCmd.Flags().StringP("name", "n", "", "Name of the provider to delete")
deleteCmd.Flags().StringP("id", "i", "", "ID of the provider to delete")
// We allow deleting by name or ID but not both. One of them must be specified.
deleteCmd.MarkFlagsMutuallyExclusive("name", "id")
deleteCmd.MarkFlagsOneRequired("name", "id")
}

// DeleteProviderCommand deletes the provider in a specific project
func DeleteProviderCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn) error {
client := minderv1.NewProvidersServiceClient(conn)

project := viper.GetString("project")
name := viper.GetString("name")
id := viper.GetString("id")

eleftherias marked this conversation as resolved.
Show resolved Hide resolved
// No longer print usage on returned error, since we've parsed our inputs
// See https://github.com/spf13/cobra/issues/340#issuecomment-374617413
cmd.SilenceUsage = true

if id != "" {
resp, err := client.DeleteProviderByID(ctx, &minderv1.DeleteProviderByIDRequest{
Context: &minderv1.Context{
Project: &project,
},
Id: id,
})
if err != nil {
return cli.MessageAndError("Error deleting provider by id", err)
}
cmd.Println("Successfully deleted provider with id:", resp.Id)
} else {
// delete provider by name
resp, err := client.DeleteProvider(ctx, &minderv1.DeleteProviderRequest{
Context: &minderv1.Context{Provider: &name, Project: &project},
})
if err != nil {
return cli.MessageAndError("Error deleting provider by name", err)
}
cmd.Println("Successfully deleted provider with name:", resp.Name)
}

return nil
}
2 changes: 1 addition & 1 deletion database/mock/store.go

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

3 changes: 2 additions & 1 deletion database/query/providers.sql
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,5 @@ UPDATE providers
WHERE id = sqlc.arg('id') AND project_id = sqlc.arg('project_id');

-- name: DeleteProvider :exec
DELETE FROM providers WHERE id = $1;
DELETE FROM providers
WHERE id = $1 AND project_id = sqlc.arg('project_id');
1 change: 1 addition & 0 deletions docs/docs/ref/cli/minder_provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ minder provider [flags]
### SEE ALSO

* [minder](minder.md) - Minder controls the hosted minder service
* [minder provider delete](minder_provider_delete.md) - Delete a given provider available in a specific project
* [minder provider enroll](minder_provider_enroll.md) - Enroll a provider within the minder control plane
* [minder provider get](minder_provider_get.md) - Get a given provider available in a specific project
* [minder provider list](minder_provider_list.md) - List the providers available in a specific project
Expand Down
40 changes: 40 additions & 0 deletions docs/docs/ref/cli/minder_provider_delete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
title: minder provider delete
---
## minder provider delete

Delete a given provider available in a specific project

### Synopsis

The minder provider delete command deletes a given provider available in a specific project.

```
minder provider delete [flags]
```

### Options

```
-h, --help help for delete
-i, --id string ID of the provider to delete
-n, --name string Name of the provider to delete
```

### Options inherited from parent commands

```
--config string Config file (default is $PWD/config.yaml)
--grpc-host string Server host (default "api.stacklok.com")
--grpc-insecure Allow establishing insecure connections
--grpc-port int Server port (default 443)
--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

* [minder provider](minder_provider.md) - Manage providers within a minder control plane

29 changes: 29 additions & 0 deletions docs/docs/ref/proto.md

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

99 changes: 99 additions & 0 deletions internal/controlplane/handlers_providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"database/sql"
"errors"

"github.com/google/uuid"
"github.com/rs/zerolog"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand All @@ -31,6 +32,7 @@ import (
"github.com/stacklok/minder/internal/util"
cursorutil "github.com/stacklok/minder/internal/util/cursor"
minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
provinfv1 "github.com/stacklok/minder/pkg/providers/v1"
)

// GetProvider gets a given provider available in a specific project.
Expand Down Expand Up @@ -156,6 +158,103 @@ func (_ *Server) ListProviderClasses(
}, nil
}

// DeleteProvider deletes a provider by name from a specific project.
func (s *Server) DeleteProvider(
ctx context.Context,
_ *minderv1.DeleteProviderRequest,
) (*minderv1.DeleteProviderResponse, error) {
entityCtx := engine.EntityFromContext(ctx)
projectID := entityCtx.Project.ID
providerName := entityCtx.Provider.Name

if providerName == "" {
return nil, status.Errorf(codes.InvalidArgument, "provider name is required")
}

provider, err := s.providerStore.GetByNameInSpecificProject(ctx, projectID, providerName)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, util.UserVisibleError(codes.NotFound, "provider not found")
}
return nil, status.Errorf(codes.Internal, "error getting provider: %v", err)
}

err = s.deleteProvider(ctx, provider, projectID)
if err != nil {
return nil, status.Errorf(codes.Internal, "error deleting provider: %v", err)
}

return &minderv1.DeleteProviderResponse{
Name: providerName,
}, nil
}

// DeleteProviderByID deletes a provider by ID from a specific project.
func (s *Server) DeleteProviderByID(
ctx context.Context,
in *minderv1.DeleteProviderByIDRequest,
) (*minderv1.DeleteProviderByIDResponse, error) {
entityCtx := engine.EntityFromContext(ctx)
projectID := entityCtx.Project.ID

parsedProviderID, err := uuid.Parse(in.Id)
if err != nil {
return nil, util.UserVisibleError(codes.InvalidArgument, "invalid provider ID")
}

provider, err := s.providerStore.GetByID(ctx, parsedProviderID)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, util.UserVisibleError(codes.NotFound, "provider not found")
}
return nil, status.Errorf(codes.Internal, "error getting provider: %v", err)
}

err = s.deleteProvider(ctx, provider, projectID)
if err != nil {
return nil, status.Errorf(codes.Internal, "error deleting provider: %v", err)
}

return &minderv1.DeleteProviderByIDResponse{
Id: in.Id,
}, nil
}

func (s *Server) deleteProvider(ctx context.Context, provider *db.Provider, projectID uuid.UUID) error {
pbOpts := []providers.ProviderBuilderOption{
providers.WithProviderMetrics(s.provMt),
providers.WithRestClientCache(s.restClientCache),
}

p, err := providers.GetProviderBuilder(ctx, *provider, s.store, s.cryptoEngine, &s.cfg.Provider, pbOpts...)
if err != nil {
return status.Errorf(codes.Internal, "cannot get provider builder: %v", err)
}

// If the provider is a GitHub provider with a valid credential, delete all repositories associated with the provider
if p.Implements(db.ProviderTypeGithub) &&
providers.GetCredentialStateForProvider(ctx, *provider, s.store, s.cryptoEngine, &s.cfg.Provider) ==
provinfv1.CredentialStateSet {
client, err := p.GetGitHub()
if err != nil {
return status.Errorf(codes.Internal, "error creating github provider: %v", err)
}

// Delete all repositories associated with the provider and remove the webhooks
err = s.repos.DeleteRepositoriesByProvider(ctx, client, provider.Name, projectID)
if err != nil {
return status.Errorf(codes.Internal, "error deleting repositories: %v", err)
}
}
Comment on lines +235 to +248
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it should be a part of the DeleteProvider implementation, rather than in ControlPlane. e.g. GitHub providers should understand that they need to clean up webhooks, but other types of providers might have other types of cleanup they need to do.

(e.g. we talked about DeleteProvider for GHA un-installing the app from the org.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had written it like that initially, but then providerService needed to use the repositoryService which seemed wrong. We could also call the store directly to delete the repositories, but then there will be duplicate logic in the services.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's totally reasonable for different services to call eachother - some domain concepts are at a higher level of abstraction than others, and thus delegate to the services for the lower-level domain concepts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there were some recent changes that also now create an import cycle when I try that:

package github.com/stacklok/minder/internal/providers
	imports github.com/stacklok/minder/internal/repositories/github
	imports github.com/stacklok/minder/internal/logger
	imports github.com/stacklok/minder/internal/engine/actions/alert
	imports github.com/stacklok/minder/internal/engine/actions/alert/security_advisory
	imports github.com/stacklok/minder/internal/providers: import cycle not allowed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we can resolve the cycle, but it's not something I want to spend time too much time on now. Can we live with this logic in the controlplane for now and do a followup PR to refactor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm okay with that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the import cycle, I'd probably break the logger --> actions/alert import path, but happy to do that as a follow-up.


// Delete the provider itself
err = s.providers.DeleteProvider(ctx, provider)
if err != nil {
return status.Errorf(codes.Internal, "error deleting provider: %v", err)
}
return nil
}

func protobufProviderImplementsFromDB(ctx context.Context, p db.Provider) []minderv1.ProviderType {
impls := make([]minderv1.ProviderType, 0, len(p.Implements))
for _, i := range p.Implements {
Expand Down
Loading
Loading