"webhook.enabled" means "webhook disabled"

[zmatthias]

Hello,

in line 264 values.yaml: https://github.com/8gears/n8n-helm-chart/blob/master/values.yaml
.Values.scaling.webhook.enabed is set to false:

 webhook:
    enabled: false
    count: 1

However, in line 95-97 _helpers.tpl: https://github.com/8gears/n8n-helm-chart/blob/master/templates/_helpers.tpl
the environment variable N8N_DISABLE_PRODUCTION_MAIN_PROCESS is set to true, if .Values.scaling.webhook.enabled is set to true:

{{- if .Values.scaling.webhook.enabled }}
- name: "N8N_DISABLE_PRODUCTION_MAIN_PROCESS"
  value: "true"
{{ end }}

Description of N8N_DISABLE_PRODUCTION_MAIN_PROCESS:

Disable production webhooks from main process. This helps ensure no HTTP traffic load to main process when using webhook-specific processes.

so seemlingly "enabling" webhooks actually disables them for production use. Debugging this caused us hours of headache.

[Vad1mo]

hmm, I see, what would you suggest solving the situation, and save many people a lot of debug time?

[zmatthias]

For example: rename .Values.scaling.webhook.enabed to .Values.scaling.webhook.disabled in values.yaml and in _helpers.tpl

---

By the way, test webhooks are not affected by this setting. Only production webhooks are. So a more descriptive name might be: .Values.scaling.productionWebhook.disabled. But that's more a matter of taste.

[Vad1mo]

This has a few flaws. Every service has the notation service.enabled. This is used here and is practice in other charts. It is also advised to do the same in general programming

It is also not backwards compatible

Any other ideas?

[swarnat]

This is only 50% a view of this topic.

Because when you set .Values.scaling.webhook.enabled you disable Webhooks from Main process. So you are true, this disables the processing on this instance.
But you enable the processing on a different Webhook instance, which is described on this section of docs:
https://docs.n8n.io/hosting/scaling/queue-mode/#webhook-processors
and deployed here:
https://github.com/8gears/n8n-helm-chart/blob/master/templates/deployment.webhooks.yaml

This is completely optional, so you don't need to activate this scaling feature and process the webhooks on main instance.
But in this moment misses > 1 instances in this moment.
In my eyes, the "enabled" attribute is correct. Maybe the "webhook" name can be changed to something like "webbhook_distribution".

[zmatthias]

Alright, I see. Then I think a simple comment in values.yaml would be sufficient and shouldn't do any harm :)

Edit: Sorry, I didn't intend to close the issue.

[Vad1mo]

thanks for the positive discussion and resolution. I updated the values file referencing this discussion and @swarnat explanation and impact.
https://github.com/8gears/n8n-helm-chart/blob/master/values.yaml#L262