Use real command guard and cached client, fix bug with orphan resourc…

…e name (#818)

cmd/botkube/main.go

@@ -399,7 +399,7 @@ func getK8sClients(cfg *rest.Config) (dynamic.Interface, discovery.DiscoveryInte
 
 	discoCacheClient := memory.NewMemCacheClient(discoveryClient)
 	mapper := restmapper.NewDeferredDiscoveryRESTMapper(discoCacheClient)
-	return dynamicK8sCli, discoveryClient, mapper, nil
+	return dynamicK8sCli, discoCacheClient, mapper, nil
 }
 
 func reportFatalErrFn(logger logrus.FieldLogger, reporter analytics.Reporter) func(ctx string, err error) error {

pkg/execute/factory.go

@@ -64,6 +64,7 @@ type AnalyticsReporter interface {
 type CommandGuard interface {
 	GetAllowedResourcesForVerb(verb string, allConfiguredResources []string) ([]kubectl.Resource, error)
 	GetResourceDetails(verb, resourceType string) (kubectl.Resource, error)
+	FilterSupportedVerbs(allVerbs []string) []string
 }
 
 // NewExecutorFactory creates new DefaultExecutorFactory.
@@ -88,14 +89,14 @@ func NewExecutorFactory(params DefaultExecutorFactoryParams) *DefaultExecutorFac
 			params.AnalyticsReporter,
 		),
 		kubectlCmdBuilder: NewKubectlCmdBuilder(
-			params.Log.WithField("component", "Notifier Executor"),
+			params.Log.WithField("component", "Kubectl Command Builder"),
 			params.Merger,
 			kcExecutor,
 			params.NamespaceLister,
-			// TODO: Pass params.CommandGuard,
+			params.CommandGuard,
 		),
 		editExecutor: NewEditExecutor(
-			params.Log.WithField("component", "Notifier Executor"),
+			params.Log.WithField("component", "Botkube Edit Executor"),
 			params.AnalyticsReporter,
 			params.CfgManager,
 			params.Cfg,

pkg/execute/fake_kc_guard.go → pkg/execute/fake_kc_guard_test.go

@@ -1,39 +1,32 @@
-package execute
+package execute_test
+
+import "github.com/kubeshop/botkube/pkg/execute/kubectl"
 
 // FakeCommandGuard provides functionality to resolve correlations between kubectl verbs and resource types.
-// TODO: Currently, we use dumb implementation that handles only a happy path, just for test purposes.
-//
-//	This will be replaced by https://github.com/kubeshop/botkube/issues/786.
+// It's used for test purposes.
 type FakeCommandGuard struct{}
 
-// Resource holds additional details about the K8s resource type.
-type Resource struct {
-	Name                    string
-	Namespaced              bool
-	SlashSeparatedInCommand bool
-}
-
 // FilterSupportedVerbs filters out unsupported verbs by the interactive commands.
 func (f *FakeCommandGuard) FilterSupportedVerbs(allVerbs []string) []string {
 	return allVerbs
 }
 
 // GetAllowedResourcesForVerb returns allowed resources types for a given verb.
-func (f *FakeCommandGuard) GetAllowedResourcesForVerb(selectedVerb string, allConfiguredResources []string) ([]Resource, error) {
+func (f *FakeCommandGuard) GetAllowedResourcesForVerb(selectedVerb string, allConfiguredResources []string) ([]kubectl.Resource, error) {
 	_, found := resourcelessVerbs[selectedVerb]
 	if found {
 		return nil, nil
 	}
 
 	// special case for 'logs'
 	if selectedVerb == "logs" {
-		return []Resource{
+		return []kubectl.Resource{
 			staticResourceMapping["deployments"],
 			staticResourceMapping["pods"],
 		}, nil
 	}
 
-	var out []Resource
+	var out []kubectl.Resource
 	for _, name := range allConfiguredResources {
 		res, found := staticResourceMapping[name]
 		if !found {
@@ -45,25 +38,25 @@ func (f *FakeCommandGuard) GetAllowedResourcesForVerb(selectedVerb string, allCo
 }
 
 // GetResourceDetails returns resource details.
-func (f *FakeCommandGuard) GetResourceDetails(verb, resourceType string) Resource {
+func (f *FakeCommandGuard) GetResourceDetails(verb, resourceType string) (kubectl.Resource, error) {
 	if verb == "logs" {
-		return Resource{
+		return kubectl.Resource{
 			Name:                    resourceType,
 			Namespaced:              true,
 			SlashSeparatedInCommand: true,
-		}
+		}, nil
 	}
 
 	res, found := staticResourceMapping[resourceType]
 	if found {
-		return res
+		return res, nil
 	}
 
 	// fake data about resource
-	return Resource{
+	return kubectl.Resource{
 		Name:       resourceType,
 		Namespaced: true,
-	}
+	}, nil
 }
 
 var resourcelessVerbs = map[string]struct{}{
@@ -73,7 +66,7 @@ var resourcelessVerbs = map[string]struct{}{
 	"cluster-info":  {},
 }
 
-var staticResourceMapping = map[string]Resource{
+var staticResourceMapping = map[string]kubectl.Resource{
 	// namespace-scoped:
 	"deployments":  {Name: "deployments", Namespaced: true},
 	"pods":         {Name: "pods", Namespaced: true},

pkg/execute/kubectl/guard.go

@@ -40,7 +40,8 @@ var (
 
 	// additionalResourceVerbs contains map of per-resource verbs which are not returned by K8s API, but should be supported.
 	additionalResourceVerbs = map[string][]string{
-		"nodes": {"cordon", "uncordon", "drain"},
+		"nodes": {"cordon", "uncordon", "drain", "top"},
+		"pods":  {"top"},
 	}
 
 	// additionalResourcelessVerbs contains map of per-resource verbs which are not returned by K8s API, but should be supported.
@@ -84,6 +85,7 @@ var (
 		"scale":        {},
 		"wait":         {},
 		"proxy":        {},
+		"run":          {},
 	}
 )
 
@@ -135,6 +137,11 @@ func (g *CommandGuard) GetAllowedResourcesForVerb(verb string, allConfiguredReso
 
 // GetResourceDetails returns a Resource struct for a given resource type and verb.
 func (g *CommandGuard) GetResourceDetails(selectedVerb, resourceType string) (Resource, error) {
+	_, found := resourcelessVerbs[selectedVerb]
+	if found {
+		return Resource{}, nil
+	}
+
 	resMap, err := g.GetServerResourceMap()
 	if err != nil {
 		return Resource{}, err

pkg/execute/kubectl/guard_test.go

@@ -100,30 +100,52 @@ func TestCommandGuard_GetAllowedResourcesForVerb(t *testing.T) {
 }
 
 func TestCommandGuard_GetResourceDetails_HappyPath(t *testing.T) {
-	// given
-	expectedRes := kubectl.Resource{
-		Name:                    "pods",
-		Namespaced:              true,
-		SlashSeparatedInCommand: false,
-	}
-	fakeDisco := &fakeDisco{
-		list: []*v1.APIResourceList{
-			{GroupVersion: "v1", APIResources: []v1.APIResource{
-				{Name: "pods", Namespaced: true, Kind: "Pod", Verbs: []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"}},
-			}},
+	testCases := []struct {
+		Name         string
+		SelectedVerb string
+		ResourceType string
+		ResourceMap  map[string]v1.APIResource
+
+		ExpectedResult     kubectl.Resource
+		ExpectedErrMessage string
+	}{
+		{
+			Name:         "Namespaced",
+			SelectedVerb: "get",
+			ResourceType: "pods",
+			ExpectedResult: kubectl.Resource{
+				Name:                    "pods",
+				Namespaced:              true,
+				SlashSeparatedInCommand: false,
+			},
+		},
+		{
+			Name:           "Verb is resourceless",
+			SelectedVerb:   "api-versions",
+			ResourceType:   "",
+			ExpectedResult: kubectl.Resource{},
 		},
 	}
-	logger, _ := logtest.NewNullLogger()
-
-	cmdGuard := kubectl.NewCommandGuard(logger, fakeDisco)
-
-	// when
+	for _, tc := range testCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			logger, _ := logtest.NewNullLogger()
+			fakeDisco := &fakeDisco{
+				list: []*v1.APIResourceList{
+					{GroupVersion: "v1", APIResources: []v1.APIResource{
+						{Name: "pods", Namespaced: true, Kind: "Pod", Verbs: []string{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"}},
+					}},
+				},
+			}
+			cmdGuard := kubectl.NewCommandGuard(logger, fakeDisco)
 
-	res, err := cmdGuard.GetResourceDetails("get", "pods")
+			// when
+			result, err := cmdGuard.GetResourceDetails(tc.SelectedVerb, tc.ResourceType)
 
-	// then
-	require.NoError(t, err)
-	assert.Equal(t, expectedRes, res)
+			// then
+			require.NoError(t, err)
+			assert.Equal(t, tc.ExpectedResult, result)
+		})
+	}
 }
 
 func TestCommandGuard_GetServerResourceMap_HappyPath(t *testing.T) {
@@ -219,6 +241,17 @@ func TestCommandGuard_GetResourceDetailsFromMap(t *testing.T) {
 				SlashSeparatedInCommand: false,
 			},
 		},
+		{
+			Name:         "Additional top verb",
+			SelectedVerb: "top",
+			ResourceType: "nodes",
+			ResourceMap:  resMap,
+			ExpectedResult: kubectl.Resource{
+				Name:                    "nodes",
+				Namespaced:              false,
+				SlashSeparatedInCommand: false,
+			},
+		},
 		{
 			Name:               "Resource doesn't exist",
 			SelectedVerb:       "get",