🐛 Allow external redis/rabbitmq secret creation even if the broke…

[afreyermuth98]

What this PR does

This PR allows the chart to create the secret of your redis / rabbitmq even if it's not the broker.
Actually, this is blocking if we want to have a redis as cache and a rabbitmq as broker for example

Which issue(s) this PR fixes

Closes #2979

Checklist

• Unit, integration, and e2e (if applicable) tests updated
• Documentation added (or pr:no public docs PR label added if not required)
• CHANGELOG.md updated (or pr:no changelog PR label added if not required)

[CLAassistant]

All committers have signed the CLA.

[joeyorlando]

hi there @afreyermuth98 👋

Thanks for your contribution! Do you mind adding two things:

• a few helm unit tests which assert the scenario you are trying to fix
• add an entry to the root CHANGELOG.md under (Unreleased > Fixed)

Thank you! 👊

[afreyermuth98]

Hey !
It's done :D
Btw I've modified a bit my code as we only use rabbitmq as a broker I guess :)

[joeyorlando]

@afreyermuth98 lastly you just need to sign the CLA 🙂

#3903 (comment)

[afreyermuth98]

@joeyorlando Hmm weird i've signed it just after my commit 🤔

You have agreed to the CLA for grafana/oncall

[joeyorlando]

@joeyorlando Hmm weird i've signed it just after my commit 🤔

You have agreed to the CLA for grafana/oncall

looks to be fine now, thank you!

[joeyorlando]

@afreyermuth98 one last thing. One of the new helm unit tests is failing:

https://github.com/grafana/oncall/actions/runs/7971753982/job/21762105142#step:5:1

[afreyermuth98]

@joeyorlando Yeah I don't really understand why. The test fails because the password is not set but it's the normal functioning isn't it ? If I'm wrong I would be glad to be highlighted :)

UPDATE : it should be fixed

[joeyorlando]

what about the case where externalRedis and externalRabbitmq are set?

[afreyermuth98]

I'll do it ;)

[afreyermuth98]

Hey @joeyorlando I've modified the tests. It passed but it's not perfect and I would be glad to be highlighted.
Firstly, I had to comment my last lines of the broker_secret_test.yaml and I can't understand why it doesn't pass with.
Also, on secrets.yaml - L64 , I've put (.Values.externalRedis.host) as a condition to pass (as you put the host for sure if you set an externalRedis) but I would have prefered to have (.Values.externalRedis.password) as before but I've investigated for hours and I can't understand why the test doesn't pass, everything is ok for me.
Thanks by advance !

[joeyorlando]

Hey @joeyorlando I've modified the tests. It passed but it's not perfect and I would be glad to be highlighted. Firstly, I had to comment my last lines of the broker_secret_test.yaml and I can't understand why it doesn't pass with. Also, on secrets.yaml - L64 , I've put (.Values.externalRedis.host) as a condition to pass (as you put the host for sure if you set an externalRedis) but I would have prefered to have (.Values.externalRedis.password) as before but I've investigated for hours and I can't understand why the test doesn't pass, everything is ok for me. Thanks by advance !

One trick I just figured out is if you add a matchSnapshot assertion it will spit out the rendered .yaml as such:

# test file
- it: externalRedis.password and externalRabbitmq.password -> should create secret -redis-external and -rabbitmq-external
    templates:
      - engine/deployment.yaml
      - celery/deployment.yaml
    set:
      rabbitmq.enabled: false
      redis.enabled: false
      broker.type: rabbitmq
      externalRedis:
        host: redis.example.com
        username: user123
        password: abcd123
      externalRabbitmq:
        host: custom-host
        user: custom-user
        password: custom-password
    asserts:
      ... other assertions
      - matchSnapshot: {}
         template: secrets.yaml

then in ./helm/oncall/tests/__snapshot__ you will have a broker_secret_test.yaml.snap as such:

externalRedis.password and externalRabbitmq.password -> should create secret -redis-external and -rabbitmq-external:
  1: |
    apiVersion: v1
    data:
      MIRAGE_SECRET_KEY: aEV4RmNqYm5sdllPVUtqVmZ2ZG1oc3N4NXRQNGpuc0E2dzV5eXJrOA==
      SECRET_KEY: WjNGRVUzak9IMkM3TTBiQ0dyZUpDTmZ5dnQ0aEticGFFaEdXUk5IcQ==
    kind: Secret
    metadata:
      labels:
        app.kubernetes.io/instance: oncall
        app.kubernetes.io/managed-by: Helm
        app.kubernetes.io/name: oncall
        app.kubernetes.io/version: v1.3.99
        helm.sh/chart: oncall-1.3.99
      name: oncall
    type: Opaque
  2: |
    apiVersion: v1
    data:
      rabbitmq-password: Y3VzdG9tLXBhc3N3b3Jk
    kind: Secret
    metadata:
      name: oncall-rabbitmq-external
    type: Opaque
  3: |
    apiVersion: v1
    data:
      redis-password: YWJjZDEyMw==
    kind: Secret
    metadata:
      name: oncall-redis-external
    type: Opaque

turns out it was simply the documentIndex on the test you had commented out. It should be "2" instead of "1" 😄

[afreyermuth98]

Ooook I was wondering about this documentIndex but didn't find a conclusion about it ahah. Thanks a lot ! :D