fix: critical and high-severity defects from full provider review - #79
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the critical and high-severity defects found by a full review of the provider. Each fix ships with a regression test, and every test was mutation-checked: the defect was reintroduced one at a time to confirm the test actually fails without the fix.
only_groupscould delete every managed rule groupdetermineGroupsToManagefiltersonly_groupsagainst the group names found in the configuration and returns only what matched —nilwhen nothing does. Update then computesgroupsToDelete = difference(oldGroups, newManagedGroups), which with an empty set is every group the resource manages: they are deleted from Mimir, the create loop skips all of them, and the apply reports success.CustomizeDiffduplicated the same logic and did not guard it either.A typo in
only_groupswas therefore enough to silently destroy a namespace's rule groups. Names that match no group are now rejected at plan time, before anything is deleted.Two provider panics
flattenSnsSigV4Configdereferenced*sigV4Configwith no nil check while its caller invoked it unconditionally, unlike the guardedhttp_configbeside it. Anysns_configsstored without asigv4key — a config written by mimirtool or by hand — crashed the plugin onterraform importor on a data source read.parseTimesplit on:and indexed[1]unconditionally.start_timeandend_timeare both optional andvalidateTimereturns early on the empty string, sotimes { start_time = "03:00" }passed validation and then panicked during plan.Configuration silently dropped
wechat_configs.api_urlwas read fromdata["api_url_url"], a key no schema declares, so the branch never ran: the configured endpoint never reached Mimir and every plan re-added the attribute.oauth2.tls_configwas declared in the schema and present in the Go struct but never read by expand nor written by flatten, silently weakening TLS on the token request and leaving a permanent diff.Empty
sigv4block no longer emittedexpandSnsSigV4Configallocated the struct before its guard, so the pointer was never nil,yaml:"sigv4,omitempty"could not apply, andsigv4: {}was written for everysns_configsblock — read back as a block the user never wrote, producing a plan that never converges. Allocation now happens inside the guard, matching every other nested block.