Skip to content

INC-3971 Fix TF intermittent false diff detection issue #767

Open
Cynthia Qin (cqin-confluent) wants to merge 2 commits intomasterfrom
INC-3971-fix
Open

INC-3971 Fix TF intermittent false diff detection issue #767
Cynthia Qin (cqin-confluent) wants to merge 2 commits intomasterfrom
INC-3971-fix

Conversation

@cqin-confluent
Copy link
Member

@cqin-confluent Cynthia Qin (cqin-confluent) commented Jul 30, 2025

Release Notes

New Features

  • [Briefly describe new features introduced in this PR].

Bug Fixes

  • This change fix the occasional issue of TF detect false diffs for confluent_kafka_topic resources configs

Examples

  • [Briefly describe any Terraform configuration example updates in this PR].

Checklist

  • I can successfully build and use a custom Terraform provider binary for Confluent.
  • I have verified my PR with real Confluent Cloud resources in a pre-prod or production environment, or both.
  • I have attached manual Terraform verification results or screenshots in the Test & Review section below.
  • I have included appropriate Terraform acceptance or unit tests for any new resource, data source, or functionality.
  • I confirm that this PR introduces no breaking changes or backward compatibility issues.
  • I have updated the corresponding documentation and include relevant examples for this PR.
  • I have indicated the potential customer impact if something goes wrong in the Blast Radius section below.
  • I have put checkmarks below confirming that the feature associated with this PR is enabled in:
    • Confluent Cloud prod
    • Confluent Cloud stag
    • Check this box if the feature is enabled for certain organizations only

What

This change fix the intermittent phantom issue of TF falsely detecting diffs in confluent_kafka_topic resources configs on existing topics and prompt for in-place updates despite the topic haven't been modified

Blast Radius

Confluent Cloud customer who are updating confluent_kafka_topic resource config will be blocked.

References

More context on the TF drift issue

INC-3971 channel

Test & Review

❯ TF_ACC=1 go test ./... -v -timeout 5m -run='TestAccTopic'
?       github.com/confluentinc/terraform-provider-confluent    [no test files]
--- PASS: TestAccTopicWithEnhancedProviderBlock (6.15s)
--- PASS: TestAccTopic (6.29s)
--- PASS: TestAccTopicPartition (8.60s)
--- PASS: TestAccTopicConfigDiffSuppression (0.00s)
PASS
ok      github.com/confluentinc/terraform-provider-confluent/internal/provider  31.597s

Copilot AI review requested due to automatic review settings July 30, 2025 20:33
@cqin-confluent Cynthia Qin (cqin-confluent) requested a review from a team as a code owner July 30, 2025 20:33
Copy link

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 addresses an intermittent issue where Terraform falsely detects configuration diffs for confluent_kafka_topic resources, causing unnecessary in-place updates on existing topics that haven't been modified.

Key Changes

  • Added a DiffSuppressFunc to the topic config schema that suppresses diffs when configuration keys exist in the old state but are removed from the new configuration
  • Added comprehensive unit tests to verify the diff suppression logic works correctly

Reviewed Changes

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

File Description
internal/provider/resource_kafka_topic.go Added DiffSuppressFunc to config schema to prevent false positive diffs
internal/provider/resource_kafka_topic_test.go Added unit tests to verify diff suppression behavior

return true
}

// when old value exists but new value is empty, check if key is in new config and suppress diff if not
Copy link

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

The comment has inconsistent spacing and should end with a period for consistency: "// When old value exists but new value is empty, check if key is in new config and suppress diff if not."

Suggested change
// when old value exists but new value is empty, check if key is in new config and suppress diff if not
// When old value exists but new value is empty, check if the key is in the new config and suppress the diff if not.

Copilot uses AI. Check for mistakes.
configKey = strings.TrimPrefix(k, "config.")
}

if newConfigs, ok := d.GetOk(paramConfigs); ok {
Copy link

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

The logic should handle the case where d.GetOk(paramConfigs) returns false. Consider adding an explicit return statement or handling the else case to make the intention clearer.

Copilot uses AI. Check for mistakes.
@sonarqube-confluent
Copy link

Passed

Analysis Details

1 Issue

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 1 Code Smell

Coverage and Duplications

  • Coverage 100.00% Coverage (80.10% Estimated after merge)
  • Duplications No duplication information (0.10% Estimated after merge)

Project ID: terraform-provider-confluent

View in SonarQube

Copy link
Contributor

@linouk23 Kostya Linou (linouk23) left a comment

Choose a reason for hiding this comment

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

Could you also attach the actual responses from the Kafka REST API? I’m curious why we can’t see topic settings with empty values in the first place.

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