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

Fix merging Kubernetes configuration #1253

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

mszostok
Copy link
Contributor

@mszostok mszostok commented Sep 12, 2023

Description

Changes proposed in this pull request:

  • I tried to add some linter to warns when assigning the address of the variable from range, but unfortunately, none were able to catch this because we are passing that pointer to a function. Even revive.

It's a valid candidate for cherry-picking into v1.4.1 release.

Testing

Unit tests

Writing manual compare was too slow, so first I used

dump = litter.Options{
	StripPackageNames:         true,
	HidePrivateFields:         false,
	HideZeroValues:            true,
	DisablePointerReplacement: true,
	Separator:                 " ",
}
dump.Sdump(router.table)

but this was not producing readable enough assets, so I decided to go with YAML, but for that I had to export fields. Because the struct is still private, it doesn't change anything.

Manual

  1. Create cluster: k3d cluster create
  2. Build k8s plugin: PLUGIN_TARGETS="kubernetes" make build-plugins-single
  3. Start server: env PLUGIN_SERVER_HOST=http://host.k3d.internal go run test/helpers/plugin_server.go
  4. Create Botkube config:
    communications:
      default-group:
        socketSlack:
          enabled: true
          channels:
            default:
              name: hakuna-matata
              bindings:
                sources:
                  - "k8s-all-events"
                executors: [ ]
          appToken: "xapp-"
          botToken: "xoxb-"
    
    sources:
      "k8s-all-events":
        botkube/kubernetes:
          enabled: true
          config:
            namespaces:
              include:
                - ".*"
            event:
              types:
                - delete
                - error
            resources:
              - type: v1/services
              - type: networking.k8s.io/v1/ingresses
              - type: v1/nodes
              - type: v1/namespaces
              - type: v1/persistentvolumes
              - type: v1/persistentvolumeclaims
              - type: v1/configmaps
                namespaces:
                  include:
                    - ".*"
                  exclude:
                    - "gitlab-runner"
              - type: rbac.authorization.k8s.io/v1/roles
              - type: rbac.authorization.k8s.io/v1/rolebindings
              - type: rbac.authorization.k8s.io/v1/clusterrolebindings
              - type: rbac.authorization.k8s.io/v1/clusterroles
              - type: apps/v1/daemonsets
              - type: batch/v1/jobs
              - type: apps/v1/deployments
              - type: apps/v1/statefulsets
    
    plugins:
      cacheDir: "/tmp/plugins"
    
      repositories:
        botkube:
          url: http://host.k3d.internal:3010/botkube.yaml
    
    
    settings:
      log:
        level: "debug"
        formatter: "text"
      clusterName: "labs"
      upgradeNotifier: false
    
    analytics:
      disable: true
    
    configWatcher:
      enabled: false
    
  5. Start Botkube: env BOTKUBE_CONFIG_PATHS="./helm/botkube/values.yaml,/tmp/debug-events.yaml" go run ./cmd/botkube-agent/main.go
  6. Create configmap in gitlab-runner ns and delete it, create configmap in default ns and try to delete it.
    kc create ns gitlab-runner
    
    kubectl create configmap my-config --from-literal=key1=config1 -n gitlab-runner                                                                                                                       
    kubectl create configmap my-config --from-literal=key1=config1 -n default
    
    kubectl delete cm my-config -n gitlab-runner
    kubectl delete cm my-config -n default

Related issue(s)

#1251

@mszostok mszostok added the bug Something isn't working label Sep 12, 2023
@mszostok mszostok requested review from PrasadG193 and a team as code owners September 12, 2023 19:40
@mszostok mszostok force-pushed the fix-k8s-event-merging branch 4 times, most recently from dbf1e63 to cf045e9 Compare September 12, 2023 19:54
Copy link

@josefkarasek josefkarasek left a comment

Choose a reason for hiding this comment

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

Good find, I bet it took some time. Well done 👍

@@ -169,25 +169,26 @@ func mergeResourceEvents(cfg *config.Config) mergedEvents {

func (r *Router) mergeEventRoutes(resource string, cfg *config.Config) map[config.EventType][]route {
out := make(map[config.EventType][]route)
for _, r := range cfg.Resources {
for idx := range cfg.Resources {
r := cfg.Resources[idx] // make sure that we work on a copy

Choose a reason for hiding this comment

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

🤯

@mszostok mszostok merged commit 16b5f04 into kubeshop:main Sep 13, 2023
14 checks passed
@mszostok mszostok deleted the fix-k8s-event-merging branch September 13, 2023 08:05
pkosiec pushed a commit to pkosiec/botkube that referenced this pull request Sep 14, 2023
pkosiec pushed a commit that referenced this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kubernetes nesting overrides don't work with multiple defined resources
2 participants