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

Opt-in to sending explicit payloads to ARM #3060

Merged
merged 21 commits into from
Jun 22, 2023

Conversation

theunrepentantgeek
Copy link
Member

@theunrepentantgeek theunrepentantgeek commented Jun 9, 2023

What this PR does / why we need it:

As identified in #2914, some Azure Resource Providers work more as a PATCH than a PUT, allowing unwarrented drift to occur without correction by ASO.

This PR adds the option $payloadType so we can opt-in certain groups for being more explicit with the payloads they send to ARM.

Closes #2914

Special notes for your reviewer:

I've left the door open for configuration of this at the object and property level, but given the behaviour we've seen so far, configuration at the group level seemed appropriate.

How does this PR make you feel:
gif

Copilot Summary

copilot:summary

Copilot Walkthrough

copilot:walkthrough

@theunrepentantgeek
Copy link
Member Author

Let's see if CoPilot works ...

copilot:summary

copilot:walkthrough

@theunrepentantgeek
Copy link
Member Author

/prbot review

@theunrepentantgeek theunrepentantgeek force-pushed the feature/arm-omitempty branch 4 times, most recently from b1d505c to 2cb356b Compare June 13, 2023 22:37
#
# Valid values are:
# omitempty - only non-empty properties are included (default)
# collection - always include collections, even if empty; other properties only if specified.
Copy link
Member

Choose a reason for hiding this comment

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

payloadType: collection doesn't read super descriptively to me. Wondering if there's a better name? Maybe payloadType: explicitCollection (to match the explicit phrasing below?)

Or payloadType: alwaysEmitCollections

@codecov-commenter
Copy link

codecov-commenter commented Jun 22, 2023

Codecov Report

Merging #3060 (2f323f8) into main (d8f6813) will increase coverage by 0.04%.
The diff coverage is 71.66%.

@@            Coverage Diff             @@
##             main    #3060      +/-   ##
==========================================
+ Coverage   54.21%   54.25%   +0.04%     
==========================================
  Files        1400     1400              
  Lines      601177   601252      +75     
==========================================
+ Hits       325920   326231     +311     
+ Misses     221858   221628     -230     
+ Partials    53399    53393       -6     
Impacted Files Coverage Δ
...1api20210401/storage_account_spec_arm_types_gen.go 33.33% <ø> (ø)
...torage_accounts_blob_service_spec_arm_types_gen.go 33.33% <ø> (ø)
...unts_blob_services_container_spec_arm_types_gen.go 33.33% <ø> (ø)
...e_accounts_management_policy_spec_arm_types_gen.go 33.33% <ø> (ø)
...ccounts_queue_services_queue_spec_arm_types_gen.go 33.33% <ø> (ø)
...1api20220901/storage_account_spec_arm_types_gen.go 33.33% <ø> (ø)
...torage_accounts_blob_service_spec_arm_types_gen.go 33.33% <ø> (ø)
...unts_blob_services_container_spec_arm_types_gen.go 33.33% <ø> (ø)
...accounts_file_services_share_spec_arm_types_gen.go 33.33% <ø> (ø)
...e_accounts_management_policy_spec_arm_types_gen.go 33.33% <ø> (ø)
... and 16 more

... and 37 files with indirect coverage changes

@theunrepentantgeek theunrepentantgeek enabled auto-merge (squash) June 22, 2023 21:51
@theunrepentantgeek theunrepentantgeek merged commit ebbb765 into main Jun 22, 2023
8 checks passed
@theunrepentantgeek theunrepentantgeek deleted the feature/arm-omitempty branch June 22, 2023 23:22
@theunrepentantgeek theunrepentantgeek added this to the v2.2.0 milestone Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Bug: Storage Account resource does not detect drift in networkAcls if the original is empty
4 participants