Allow reflection of middlemail-rabbitmq secret

[ankur512512]

This will allow the reflection of middlemail-rabbitmq secret in other namespaces.

Please note that this annotation will not automatically create this secret in other namespaces but it will allow other namespaces to fetch the latest value from this secret using another annotation, whenever the source secret changes, using reflector operator.

[ahsan0608]

LGTM! Feel free to merge when you're ready!

[saraedum]

Since this is a public repo, I think the setup here should not be that opinionated. Could we just pull in some annotations from the values instead? (Like all the bitnami charts do it? e.g., search for "annotations" here: https://github.com/bitnami/charts/tree/main/bitnami/postgresql#parameters)

[ankur512512]

@saraedum yeah that makes sense, thanks for the review.

I have pushed the changes, please let me know if it looks good now.

[saraedum]

This change goes in the right direction but I don't think it fixes the issue.

The helm hook annotations are not opinionated. They should be kept as they were. The other annotations should usually be redis.secret.annotations and rabbitmq.secret.annotations or something like that. However, they should be empty to give the user a way to inject their own annotations (and not put the opinionated reflector annotation there that is particular to your setup.)

[ankur512512]

@saraedum Ah, okay. Got your point !!

Fixed it now.

P.S. I am only touching rabbitmq secret as of now because I need that one only for my setup.

[ankur512512]

This change goes in the right direction but I don't think it fixes the issue.

The helm hook annotations are not opinionated. They should be kept as they were. The other annotations should usually be redis.secret.annotations and rabbitmq.secret.annotations or something like that. However, they should be empty to give the user a way to inject their own annotations (and not put the opinionated reflector annotation there that is particular to your setup.)

Hi, can you please review again and approve if you feel it looks okay.

[saraedum]

I think you misunderstood my point here:

However, they should be empty to give the user a way to inject their own annotations (and not put the opinionated reflector annotation there that is particular to your setup.)

The values file should not ship such annotations. They are too opinionated to be in a chart that others might consider upstream.

[ankur512512]

I think you misunderstood my point here:

However, they should be empty to give the user a way to inject their own annotations (and not put the opinionated reflector annotation there that is particular to your setup.)

The values file should not ship such annotations. They are too opinionated to be in a chart that others might consider upstream.

Sorry, I understood your point but I actually forgot to delete them before pushing after testing.
Should be good now. Let me know if it looks okay.

[saraedum]

lgtm

[ankur512512]

lgtm

Hi, could you please approve and merge? I don't have the permissions to do that.