Skip to content

Comments

Add support for diff overrides in handle_updates; fix updating default_firewall_ids when None#731

Merged
lgarber-akamai merged 7 commits intolinode:devfrom
lgarber-akamai:fix/firewall-settings-update-null
Dec 4, 2025
Merged

Add support for diff overrides in handle_updates; fix updating default_firewall_ids when None#731
lgarber-akamai merged 7 commits intolinode:devfrom
lgarber-akamai:fix/firewall-settings-update-null

Conversation

@lgarber-akamai
Copy link
Contributor

@lgarber-akamai lgarber-akamai commented Dec 3, 2025

📝 Description

This pull request introduces a new diff_overrides field on the handle_updates helper, which allows modules to override the default diffing logic for fields that may require custom diff logic.

Integration tests run: https://github.com/linode/ansible_linode/actions/runs/19909274376/job/57073583771

✔️ How to Test

The following test steps assume you have pulled down this PR locally and run make install.

Integration Testing

NOTE: This test was previously failing only when the user has a null entry in the default_firewall_ids field.

make test-int TEST_ARGS="-v firewall_settings"

@lgarber-akamai lgarber-akamai requested a review from a team as a code owner December 3, 2025 21:16
@lgarber-akamai lgarber-akamai added the improvement for improvements in existing functionality in the changelog. label Dec 3, 2025
@lgarber-akamai lgarber-akamai requested review from jriddle-linode and removed request for a team December 3, 2025 21:16
@lgarber-akamai lgarber-akamai added the bugfix for any bug fixes in the changelog. label Dec 3, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for custom diffing logic in the handle_updates helper function and fixes an issue with updating default_firewall_ids when it contains None values.

Key changes:

  • Introduces a diff_overrides parameter to handle_updates allowing modules to provide custom comparison logic for specific fields
  • Updates firewall settings module to use custom diff logic that preserves None values in default_firewall_ids
  • Adjusts integration test to properly handle null values in firewall settings

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
plugins/module_utils/linode_helper.py Refactored handle_updates to support custom diff functions via diff_overrides parameter
plugins/modules/firewall_settings.py Added custom diff override for default_firewall_ids field using matching_keys_eq
tests/integration/targets/firewall_settings/tasks/main.yaml Updated test to handle null values in default_firewall_ids field

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lgarber-akamai and others added 2 commits December 3, 2025 16:21
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lgarber-akamai lgarber-akamai force-pushed the fix/firewall-settings-update-null branch from e01939c to 8830f01 Compare December 3, 2025 21:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

Tests passed on my end with null values, nice work!

@lgarber-akamai lgarber-akamai merged commit 8bbc66f into linode:dev Dec 4, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix for any bug fixes in the changelog. improvement for improvements in existing functionality in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants