Skip to content

Refactor code #47

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

Merged
merged 53 commits into from
May 8, 2025
Merged

Refactor code #47

merged 53 commits into from
May 8, 2025

Conversation

danischm
Copy link
Member

@danischm danischm commented Apr 16, 2025

Breaking change: refactor resource keys:

  • Use keys derived from identifying fields in all resources
    instead of only resources that are referred to by other resources.
    This way, inserting a resource in the middle of the list
    does not cause existing resources to be modified/recreated.
  • Add ${domain.name}/ to all keys.
  • Consistently use format() for keys instead of string interpolation.

Breaking change: replace net_switch_stacks_routing_interfaces_dhcp_{first,not_first} with a single net_switch_stacks_routing_interfaces_dhcp resource.

Apply default values consistently:

  • Apply defaults everywhere - many resources and nested fields did not have them applied.
  • Change defaults paths to always be rooted at domains. instead of having different roots for domain/organization/network resources.
  • Fix outdated paths to defaults: make the path the same as for the value (skipping arrays).
  • Map values from the YAML (and apply defaults) in one place - directly in the local variable, instead of doing part of that in the resource as well.
    • Convert empty array fields to null in the variable directly.
    • Remove the catch-all data field and map all nested values explicitly - this is necessary to apply defaults everywhere.
  • Remove redundant if try(...) after loops that result in empty arrays by default anyway.

Fix a lot of fields where schema differences were missed previously.

@vozhyk- vozhyk- self-assigned this Apr 17, 2025
@vozhyk- vozhyk- force-pushed the ds-refactoring branch 3 times, most recently from cd24b85 to 2df388d Compare April 23, 2025 13:12
Copy link
Collaborator

@vozhyk- vozhyk- left a comment

Choose a reason for hiding this comment

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

Adding some things I noticed when fixing this to pass the tests.

@vozhyk- vozhyk- force-pushed the ds-refactoring branch 3 times, most recently from 61b97df to 6b642c4 Compare April 29, 2025 15:49
@vozhyk- vozhyk- marked this pull request as ready for review April 29, 2025 15:57
@vozhyk- vozhyk- requested a review from jon-humphries April 29, 2025 15:57
vozhyk- added 23 commits May 8, 2025 16:55
…(and partially regenerate) fields

Remove duplication -
use a single resource for dhcp of `_first` and `_not_first`
routing interfaces. Get their IDs using a local mapping instead.
Remove the "depends_on", as the dependency should already be added
via interface IDs anyway.
- Apply defaults to ipv6_prefix_assignments.
- Fix `mandatory_dhcp{.enabled -> }` for the current schema.
…enerate) fields

Add missing log and tcp_established fields (schema 1.53).
@jon-humphries jon-humphries merged commit 7eb7f6a into main May 8, 2025
2 checks passed
@jon-humphries jon-humphries deleted the ds-refactoring branch May 8, 2025 15:29
vozhyk- added a commit that referenced this pull request May 9, 2025
…ore loop check

Without this check, the resource is created
even when `l3_firewall_rules` is not specified.
Then resource creation fails
because the `rules` field is mandatory in the provider.

This was mistakenly removed during manual (not generated) changes
in #47.
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.

3 participants