Skip to content

Update msteams webhook URLs to use mandatory secrets#2204

Open
jovial wants to merge 4 commits intostackhpc/2025.1from
bugfix/mandatory-webhooks
Open

Update msteams webhook URLs to use mandatory secrets#2204
jovial wants to merge 4 commits intostackhpc/2025.1from
bugfix/mandatory-webhooks

Conversation

@jovial
Copy link
Contributor

@jovial jovial commented Mar 10, 2026

Current default never makes sense.

Current default never makes sense.
@jovial jovial requested a review from a team as a code owner March 10, 2026 13:21
@jovial jovial requested a review from jackhodgkiss March 10, 2026 13:23
Alex-Welsh
Alex-Welsh previously approved these changes Mar 10, 2026
Copy link
Member

@Alex-Welsh Alex-Welsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
One suggestion but it's fine if you want to ignore it. I'm happy to merge as is

- name: 'msteamsv2-notifications'
msteamsv2_configs:
- webhook_url: '{{ secrets_msteams_notification_channel_url | default('https://prod-01.westeurope.logic.azure.com/workflows/') }}'
- webhook_url: '{{ secrets_msteams_notification_channel_url | ansible.builtin.mandatory }}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it'd be nice to also include a message here to make it clear what the variable is and how to acquire one

Suggested change
- webhook_url: '{{ secrets_msteams_notification_channel_url | ansible.builtin.mandatory }}'
- webhook_url: "{{ secrets_msteams_notification_channel_url | ansible.builtin.mandatory(msg='secrets_msteams_notification_channel_url must be defined when xyz is enabled and can be found by looking in foo place.' }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Granted that is better than the standard:

fatal: [localhost]: FAILED! => {"msg": "Mandatory variable 'secrets_msteams_notification_channel_url'  not defined."}

Alex-Welsh
Alex-Welsh previously approved these changes Mar 10, 2026
@Alex-Welsh Alex-Welsh enabled auto-merge (squash) March 10, 2026 14:08
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates an example configuration file for Prometheus Alertmanager. The change replaces default, non-functional webhook URLs for Microsoft Teams notifications with a requirement that these URLs be provided as mandatory secrets. This is a good practice as it ensures users configure their specific endpoints, improving the robustness and security of the setup. The change is correct and well-implemented using Ansible's mandatory filter. I have no issues to report.

@jovial
Copy link
Contributor Author

jovial commented Mar 11, 2026

Darn, it looks like kayobe is copying the example file so needs the variable defined. I will try and prevent that, but probably explains why there was a default 😆

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible.errors.AnsibleFilterError: secrets_msteams_notification_channel_url is undefined. Please add this variable to secrets.yml when using the Microsoft Teams Alertmanager integration.
failed: [localhost] (item={'src': '/stack/kayobe-automation-env/src/kayobe-config/etc/kayobe/kolla/config/prometheus/prometheus-alertmanager.msteamsv2.yml.example', 'dest': '/stack/kayobe-automation-env/src/kayobe-config/etc/kolla/config/prometheus/prometheus-alertmanager.msteamsv2.yml.example', 'params': []}) => 
    ansible_loop_var: item
    changed: false
    item:
        dest: /stack/kayobe-automation-env/src/kayobe-config/etc/kolla/config/prometheus/prometheus-alertmanager.msteamsv2.yml.example
        params: []
        src: /stack/kayobe-automation-env/src/kayobe-config/etc/kayobe/kolla/config/prometheus/prometheus-alertmanager.msteamsv2.yml.example
    msg: 'AnsibleFilterError: secrets_msteams_notification_channel_url is undefined. Please
        add this variable to secrets.yml when using the Microsoft Teams Alertmanager integration.'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants