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 configuration merging when using existingCommunicationsSecretName #1274

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

pkosiec
Copy link
Member

@pkosiec pkosiec commented Sep 22, 2023

Description

Changes proposed in this pull request:

  • Fix configuration merging when using existingCommunicationsSecretName
  • Do not create unused ConfigMaps when using remote config

Testing

See the instruction in #1273 and test the values:

existingCommunicationsSecretName: "comm-config.yaml"
settings:
  clusterName: "devops.azure.akelius.com"
sources:
  k8s-custom:
    botkube/kubernetes:
      config:
        event:
          types:
            - error
        namespaces:
          include:
            - kube-infrastructure
        resources:
          - type: v1/pods
      context:
        rbac:
          group:
            prefix: ""
            static:
              values:
                - botkube-plugins-default
            type: Static
      enabled: true
    displayName: Kubernetes Errors Kube Infra

resources:
  limits:
    cpu: 100m
    memory: 512Mi
  requests:
    cpu: 10m
    memory: 136Mi

Also, test the following scenarios:

  • Remote config

  • Using default Communications secret - you can simply use

communications:
  'default-group':
    socketSlack:
      enabled: true
      channels:
        'default':
          name: 'k8s_devops_azure'
          bindings:
            sources:
              - k8s-custom
      botToken: 'xoxb-'
      appToken: 'xapp-'
settings:
  clusterName: "devops.azure.akelius.com"
sources:
  k8s-custom:
    botkube/kubernetes:
      config:
        event:
          types:
            - error
        namespaces:
          include:
            - kube-infrastructure
        resources:
          - type: v1/pods
      context:
        rbac:
          group:
            prefix: ""
            static:
              values:
                - botkube-plugins-default
            type: Static
      enabled: true
    displayName: Kubernetes Errors Kube Infra

resources:
  limits:
    cpu: 100m
    memory: 512Mi
  requests:
    cpu: 10m
    memory: 136Mi

Related issue(s)

Resolves #1273

@pkosiec pkosiec added the bug Something isn't working label Sep 22, 2023
@pkosiec pkosiec marked this pull request as ready for review September 25, 2023 08:18
@pkosiec pkosiec requested review from PrasadG193 and a team as code owners September 25, 2023 08:19
@josefkarasek josefkarasek requested review from josefkarasek and removed request for mszostok September 25, 2023 08:23
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.

lgtm 👍

@pkosiec pkosiec merged commit fdb389c into kubeshop:main Sep 26, 2023
16 checks passed
@pkosiec pkosiec deleted the fix-merging-existing-config branch September 26, 2023 14:51
@voriol
Copy link
Contributor

voriol commented Feb 8, 2024

When updating my helm chart to the latest version, 1.8.0 I got the following error:

Error: template: sre-botkube/charts/botkube/templates/persistent-config.yaml:6:23: executing "sre-botkube/charts/botkube/templates/persistent-config.yaml" at <index $secret.data "comm_config.yaml">: error calling index: index of untyped nil

Reviewing the latest changes I see that the line it refers to was changed in this commit

fdb389c#diff-e694b58be4de3f2b3d147e65317458bbbc0f5319bd07c703d80dc3b05bb5d1b7

Specifically, on line 6:

{{- $data := b64dec (index $secret.data "comm_config.yaml") -}}

This is the secret content:

apiVersion: v1
kind: Secret
metadata:
  name: botkube-communications
stringData:
  comm_config.yaml: |
    communications:
      default-group:
        ## Settings for Slack with Socket Mode.
        ...etc

I am doing something wrong?

Thanks

@pkosiec
Copy link
Member Author

pkosiec commented Feb 8, 2024

Hey @voriol,
As you can see in the line above, the existing Secret's data field is used: {{- $secret := lookup "v1" "Secret" .Release.Namespace .Values.existingCommunicationsSecretName | default dict }}.
you provided a snippet with the stringData, so I assume this is how you create the Secret. We'd need to check the actual Secret in the cluster, as the stringData map is encoded to data.

  • Are you sure the existingCommunicationsSecretName value in Helm chart is valid?
  • Can you get the secret with name defined in the existingCommunicationsSecretName via kubectl and see if the data map contains comm_config.yaml?

If that needs further investigation, please create a separate issue and mention me there, I'll try to help. Cheers!

@voriol
Copy link
Contributor

voriol commented Feb 8, 2024

Hi @pkosiec,

I think I have all the ingredients for it to work, that's why I don't understand where the problem could be.

  • helm dependencies:
dependencies:
- name: botkube
  version: "1.8.0"
  repository: "https://charts.botkube.io/"
  • values:
botkube:
  existingCommunicationsSecretName: "botkube-communications"
  • secret / data map:
$ k get secret botkube-communications -n botkube -o jsonpath='{.data}' 
{"comm_config.yaml":"base64encoded..."

Thanks!

@pkosiec
Copy link
Member Author

pkosiec commented Feb 9, 2024

@voriol Let's continue our discussion in #1376 👍

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.

Wrong source bindings when using existingCommunicationsSecretName
3 participants