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

[bitnami/flux] Fix incorrect argument mapping and add missing events-addr argument #30523

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

erfan-272758
Copy link

Description

This pull request addresses two key issues in the current Bitnami Helm chart for FluxCD:

1. Fix Argument Mapping for Notification Controller

  • The Notification Controller uses two primary arguments to manage communication:
    • events-addr: The address where the controller listens for events from other controllers (e.g., helm-controller, kustomize-controller). Events received on this address trigger Alert objects.
    • receiverAddr: The address where the controller listens for webhooks from external sources (e.g., Git repositories). Requests sent to this address trigger Receiver objects.
  • Problem: Incorrect Mapping in Current Chart
    • The receiver key maps to receiverAddr, which is correct.
    • However, the webhook key is incorrectly mapped to events-addr.
  • Impact of the Incorrect Mapping:
    • Users configuring the chart with the webhook key would expect it to configure the webhook listener (receiverAddr), but instead, it configures the event listener (events-addr).
    • Similarly, the receiver key, which users would expect to configure the event listener (events-addr), instead configures the webhook listener (receiverAddr).
  • Solution (Applied in This PR):
    • webhook key → receiverAddr (Webhook listener).
    • receiver key → events-addr (Event listener).
  • This fix aligns the key mappings with their intended functionality, making the chart configuration more intuitive and accurate.

2. Add Missing events-addr Argument for Other Controllers

  • The events-addr argument was not initialized for the following controllers:
    • helm-controller
    • kustomize-controller
    • source-controller
    • image-automation-controller
    • image-reflector-controller
  • Solution:
    • This PR adds the events-addr argument for these controllers, with its value set to the address of the Notification Controller's service.

Impact

  • Fixes the misconfigured argument mapping for the Notification Controller, ensuring it works as intended.
  • Ensures all controllers properly communicate with the Notification Controller by initializing the events-addr argument.

Testing

  • Verified that the Notification Controller correctly handles events via events-addr and webhooks via receiverAddr.
  • Confirmed other controllers now correctly use the events-addr argument to communicate with the Notification Controller.

Checklist

  • Updated argument mappings for webhook and receiver keys in the Notification Controller.
  • Added the events-addr argument to the relevant controllers.
  • Verified changes function correctly across all controllers.

Notes

These changes ensure that all FluxCD controllers are correctly configured for communication and align with the expected behavior of the Notification Controller.

Signed-off-by: Bitnami Containers <[email protected]>
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Nov 19, 2024
@github-actions github-actions bot removed the triage Triage is needed label Nov 19, 2024
@github-actions github-actions bot removed the request for review from javsalgar November 19, 2024 18:19
@github-actions github-actions bot requested a review from migruiz4 November 19, 2024 18:19
@migruiz4 migruiz4 changed the title Fix Incorrect Argument Mapping and Add Missing events-addr Argument in Bitnami FluxCD Chart [bitnami/flux] Fix incorrect argument mapping and add missing events-addr argument Nov 29, 2024
Signed-off-by: Bitnami Containers <[email protected]>
@erfan-272758
Copy link
Author

Is there anyone?

@erfan-272758
Copy link
Author

@migruiz4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flux in-progress verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants