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.

Sign up for GitHub

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

Generate kube config based on plugin RBAC #1020

Merged
merged 15 commits into from
Mar 23, 2023
Merged

Generate kube config based on plugin RBAC #1020

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
72653f1
generate kube config
Mar 20, 2023
069937a
distribute kubeConfig as `[]bytes`
Mar 21, 2023
6a567d5
handle helm executor
Mar 21, 2023
f85e722
implement fn
Mar 21, 2023
cfd4e9d
working helm executor
Mar 21, 2023
d52875d
implement sources
Mar 22, 2023
8841af2
update values.yaml
Mar 22, 2023
c7f7bcb
include generated grpc resource
Mar 22, 2023
a47d83d
fix tests
Mar 22, 2023
387907d
process chart
Mar 22, 2023
750fdef
use default namespace
Mar 22, 2023
61f7920
update e2e values.yaml
Mar 22, 2023
0dd2fe5
WIP: channel membership
Mar 23, 2023
75352fb
revert UserGroups
Mar 23, 2023
670b48b
rename kubeConfigBytes field
Mar 23, 2023
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/botkube/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ func run(ctx context.Context) error {
CommandGuard: cmdGuard,
PluginManager: pluginManager,
BotKubeVersion: botkubeVersion,
RestCfg: kubeConfig,
AuditReporter: auditReporter,
},
)
Expand Down
2 changes: 1 addition & 1 deletion internal/source/dispatcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (d *Dispatcher) Dispatch(dispatch PluginDispatch) error {
Context: source.StreamInputContext{
IsInteractivitySupported: dispatch.isInteractivitySupported,
ClusterName: dispatch.cfg.Settings.ClusterName,
KubeConfig: dispatch.cfg.Settings.Kubeconfig,
Copy link
Author

@josefkarasek josefkarasek Mar 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This might break existing functionality. We might as well remove kube config from config

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 I think it would be great to keep the existing functionality of passing external kubeconfig. This functionality is great to connect to an external cluster. So Botkube can reside e.g. on cluster A, but it can listen for events + execute commands from cluster B.

Options I see:

  1. have kubeconfig as it were before, which overrides kubeconfig for all (sources, executors) and we ignore context.rbac 🤔 That would need to load the file from path, right?
  2. load external kubeconfig as a base for generated kubeconfigs (with modifying impersonate-related properties)
  3. add context.kubeconfig.path property which takes custom kubeconfig as a path and injects it in a given plugin.

I'd prefer third option I think 🤔 Let's chat about this in our team.

Copy link
Collaborator

@mszostok mszostok Mar 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

2 cents from my side: I like that option however, even Argo Workflow which is quite popular project, it doesn't support external/multi-cluster (they have issue opened for more that 2years now). If someone wants to have such functionality, they can always use https://github.com/virtual-kubelet/virtual-kubelet#providers. Of course, it brings additional maintenance cost, but it's also a more powerful and “safer” option.

Maybe at the end it just brings not necessary complexity to the Botkube itself? Probably it's a question to @brampling whether we should leave that functionality or not.

However, if we will decide to leave that functionality, it would be good to name it like context.remote.kubeconfig? so it is easier to distinguish current cluster RBAC usage vs external one.

KubeConfig: []byte(dispatch.cfg.Settings.Kubeconfig),
},
})
if err != nil {
Expand Down
22 changes: 4 additions & 18 deletions internal/source/kubernetes/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"k8s.io/client-go/rest"
"k8s.io/client-go/restmapper"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/tools/clientcmd/api"
)

// Client Kubernetes client
Expand All @@ -23,23 +22,10 @@ type Client struct {
}

// NewClient initializes Kubernetes client
func NewClient(kubeConfigPath string) (*Client, error) {
var kubeConfig *rest.Config
if kubeConfigPath == "" {
config, err := rest.InClusterConfig()
if err != nil {
return nil, fmt.Errorf("while loading in cluster config. %v", err)
}
kubeConfig = config
} else {
config, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
&clientcmd.ClientConfigLoadingRules{ExplicitPath: kubeConfigPath},
&clientcmd.ConfigOverrides{ClusterInfo: api.Cluster{}},
).ClientConfig()
if err != nil {
return nil, fmt.Errorf("while loading dynamic config. %v", err)
}
kubeConfig = config
func NewClient(kubeConfigBytes []byte) (*Client, error) {
kubeConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeConfigBytes)
if err != nil {
return nil, fmt.Errorf("while reading kube config. %v", err)
}
dynamicCli, discoveryCli, mapper, err := getK8sClients(kubeConfig)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/source/kubernetes/source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Source struct {
commandGuard *command.CommandGuard
filterEngine filterengine.FilterEngine
clusterName string
kubeConfig string
kubeConfig []byte
messageBuilder *MessageBuilder
isInteractivitySupported bool
}
Expand Down
128 changes: 69 additions & 59 deletions pkg/api/executor/executor.pb.go
View file
Open in desktop

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

4 changes: 4 additions & 0 deletions pkg/api/executor/grpc_adapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ type (
// IsInteractivitySupported is set to true only if communication platform supports interactive Messages.
IsInteractivitySupported bool

// KubeConfig is the path to kubectl configuration file.
josefkarasek marked this conversation as resolved.
Show resolved Hide resolved
KubeConfig []byte

// SlackState represents modal state. It's available only if:
// - IsInteractivitySupported is set to true,
// - and interactive actions were used in the response Message.
Expand Down Expand Up @@ -192,6 +195,7 @@ func (p *grpcServer) Execute(ctx context.Context, request *ExecuteRequest) (*Exe
Context: ExecuteInputContext{
SlackState: &slackState,
IsInteractivitySupported: request.Context.IsInteractivitySupported,
KubeConfig: request.Context.KubeConfig,
},
})
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/api/source/grpc_adapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type (
IsInteractivitySupported bool

// KubeConfig is the path to kubectl configuration file.
KubeConfig string
KubeConfig []byte

// ClusterName is the name of underlying Kubernetes cluster which is provided by end user.
ClusterName string
Expand Down Expand Up @@ -126,7 +126,7 @@ func (p *grpcClient) Stream(ctx context.Context, in StreamInput) (StreamOutput,
Configs: in.Configs,
Context: &StreamContext{
IsInteractivitySupported: in.Context.IsInteractivitySupported,
KubeConfig: in.Context.KubeConfig,
KubeConfigBytes: in.Context.KubeConfig,
ClusterName: in.Context.ClusterName,
},
}
Expand Down Expand Up @@ -218,7 +218,7 @@ func (p *grpcServer) Stream(req *StreamRequest, gstream Source_StreamServer) err
Configs: req.Configs,
Context: StreamInputContext{
IsInteractivitySupported: req.Context.IsInteractivitySupported,
KubeConfig: req.Context.KubeConfig,
KubeConfig: req.Context.KubeConfigBytes,
ClusterName: req.Context.ClusterName,
},
})
Expand Down
Loading