Skip to content

Conversation

deanhuynh
Copy link
Contributor

@deanhuynh deanhuynh commented Oct 3, 2025

Adds a new mergeSettings helper to better identify drift across relevant resources (sources, destinations, destination_subscriptions, warehouses, profile_warehouses, and insert_functions). This helper now diffs settings present in the config with their actual backend values except if they are censored or the password field for warehouses which have special handling as their returned API values are different than the backend values.

Tested each resource along with relevant diffs.

Addresses #200

@deanhuynh deanhuynh requested a review from a team as a code owner October 3, 2025 20:20
@deanhuynh deanhuynh requested a review from Copilot October 3, 2025 20:41
Copy link
Contributor

@Copilot 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 introduces a new mergeSettings helper function to improve settings drift detection across Terraform resources. The main goal is to properly handle the difference between config-defined settings and backend-generated settings by merging only the relevant values.

Key changes:

  • Added a new mergeSettings utility function that preserves config-defined keys while detecting drift from backend values
  • Updated 6 resource types (sources, destinations, destination_subscriptions, warehouses, profile_warehouses, insert_functions) to use this new merge logic
  • Added special handling for warehouse password fields and censored secrets (indicated by bullet characters)

Reviewed Changes

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

Show a summary per file
File Description
internal/provider/utils.go Implements the core mergeSettings function with logic for handling censored secrets and warehouse passwords
internal/provider/utils_test.go Comprehensive test suite for the mergeSettings function covering various scenarios
internal/provider/warehouse_resource.go Updated to use mergeSettings with warehouse-specific handling
internal/provider/source_resource.go Updated to use mergeSettings for drift detection
internal/provider/profiles_warehouse_resource.go Updated to use mergeSettings with warehouse-specific handling
internal/provider/insert_function_instance_resource.go Updated to use mergeSettings for drift detection
internal/provider/destination_subscription_resource.go Updated to use mergeSettings for drift detection
internal/provider/destination_resource.go Updated to use mergeSettings for drift detection
go.mod Added testify dependency for test assertions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@deanhuynh deanhuynh merged commit 5edd692 into main Oct 3, 2025
17 checks passed
@deanhuynh deanhuynh deleted the APICS-3771/settings-updates branch October 3, 2025 21:57
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