Modify the problem in the code review

Author: cyberHermanwang

tencentcloud/data_source_tc_ckafka_topics.go

@@ -130,7 +130,7 @@ func dataSourceTencentCloudCkafkaTopics() *schema.Resource {
 							Description: "Clear log policy, log clear mode. delete: logs are deleted according to the storage time, compact: logs are compressed according to the key, compact, delete: logs are compressed according to the key and will be deleted according to the storage time.",
 						},
 						"unclean_leader_election_enable": {
-							Type:        schema.TypeInt,
+							Type:        schema.TypeBool,
 							Computed:    true,
 							Description: "Whether to allow unsynchronized replicas to be selected as leader, false: not allowed, true: allowed, not allowed by default.",
 						},
@@ -181,6 +181,10 @@ func dataSourceTencentCloudCkafkaTopicRead(d *schema.ResourceData, meta interfac
 	ids := make([]string, 0, len(topicDetails))
 
 	for _, topic := range topicDetails {
+		uncleanLeaderElectionEnable := false
+		if *topic.Config.UncleanLeaderElectionEnable == int64(1) {
+			uncleanLeaderElectionEnable = true
+		}
 		instance := map[string]interface{}{
 			"topic_name":                     topic.TopicName,
 			"topic_id":                       topic.TopicId,
@@ -196,7 +200,7 @@ func dataSourceTencentCloudCkafkaTopicRead(d *schema.ResourceData, meta interfac
 			"retention":                      topic.Config.Retention,
 			"sync_replica_min_num":           topic.Config.MinInsyncReplicas,
 			"clean_up_policy":                topic.Config.CleanUpPolicy,
-			"unclean_leader_election_enable": topic.Config.UncleanLeaderElectionEnable,
+			"unclean_leader_election_enable": uncleanLeaderElectionEnable,
 			"max_message_bytes":              topic.Config.MaxMessageBytes,
 			"segment":                        topic.Config.SegmentMs,
 			"segment_bytes":                  topic.Config.SegmentBytes,

tencentcloud/data_source_tc_ckafka_topics_test.go

@@ -17,11 +17,24 @@ func TestAccTencentCloudCkafkaTopicDataSource(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckKafkaTopicInstanceExists("tencentcloud_ckafka_topic.kafka_topic"),
 					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.kafka_topics", "instance_id", "ckafka-f9ife4zz"),
-					resource.TestCheckResourceAttrSet("data.tencentcloud_ckafka_topics.kafka_topics", "instance_list.#"),
+					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.kafka_topics", "instance_list.#", "1"),
 					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.kafka_topics", "instance_list.0.topic_name", "ckafkaTopic-tf-test"),
 					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.kafka_topics", "instance_list.0.partition_num", "1"),
 					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.kafka_topics", "instance_list.0.replica_num", "2"),
 					resource.TestCheckResourceAttrSet("data.tencentcloud_ckafka_topics.kafka_topics", "instance_list.0.create_time"),
+					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.kafka_topics", "instance_list.0.note", "test topic"),
+					resource.TestCheckResourceAttr("tencentcloud_ckafka_topic.kafka_topic", "enable_white_list", "1"),
+					resource.TestCheckResourceAttr("tencentcloud_ckafka_topic.kafka_topic", "ip_white_list.#", "1"),
+					resource.TestCheckResourceAttr("tencentcloud_ckafka_topic.kafka_topic", "ip_white_list.0", "192.168.1.1"),
+					resource.TestCheckResourceAttr("tencentcloud_ckafka_topic.kafka_topic", "clean_up_policy", "delete"),
+					resource.TestCheckResourceAttr("tencentcloud_ckafka_topic.kafka_topic", "sync_replica_min_num", "1"),
+					resource.TestCheckResourceAttrSet("tencentcloud_ckafka_topic.kafka_topic", "unclean_leader_election_enable"),
+					resource.TestCheckResourceAttr("tencentcloud_ckafka_topic.kafka_topic", "segment", "3600000"),
+					resource.TestCheckResourceAttr("tencentcloud_ckafka_topic.kafka_topic", "retention", "60000"),
+					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.foo", "instance_list.#", "1"),
+					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.foo", "instance_list.0.partition_num", "1"),
+					resource.TestCheckResourceAttr("data.tencentcloud_ckafka_topics.foo", "instance_list.0.replica_num", "2"),
+					resource.TestCheckResourceAttrSet("data.tencentcloud_ckafka_topics.foo", "instance_list.0.create_time"),
 				),
 			},
 		},
@@ -30,14 +43,26 @@ func TestAccTencentCloudCkafkaTopicDataSource(t *testing.T) {
 
 const testAccTencentCloudCkafkaTopicDataSourceConfig = `
 resource "tencentcloud_ckafka_topic" "kafka_topic" {
-   instance_id                   = "ckafka-f9ife4zz"
-   topic_name                = "ckafkaTopic-tf-test"
-   replica_num                   = 2
-   partition_num              = 1
+	instance_id						= "ckafka-f9ife4zz"
+	topic_name						= "ckafkaTopic-tf-test"
+	replica_num						= 2
+	partition_num					= 1
+	note							= "test topic"
+	enable_white_list           	= 1
+	ip_white_list               	= ["192.168.1.1"]
+	clean_up_policy					= "delete"
+	sync_replica_min_num 			= 1
+	unclean_leader_election_enable	= false
+	segment							= 3600000
+	retention						= 60000
 }
 
 data "tencentcloud_ckafka_topics" "kafka_topics" {
 	instance_id						= tencentcloud_ckafka_topic.kafka_topic.instance_id
 	topic_name						= tencentcloud_ckafka_topic.kafka_topic.topic_name
 }
+
+data "tencentcloud_ckafka_topics" "foo" {
+	instance_id						= tencentcloud_ckafka_topic.kafka_topic.instance_id
+}
 `

tencentcloud/resource_tc_ckafka_topic.go

@@ -217,6 +217,9 @@ func resourceTencentCloudCkafkaTopicCreate(d *schema.ResourceData, meta interfac
 	request.ReplicaNum = helper.IntInt64(d.Get("replica_num").(int))
 	request.EnableWhiteList = helper.BoolToInt64Ptr(whiteListSwitch)
 	if whiteListSwitch {
+		if len(ipWhiteList) == 0 {
+			return fmt.Errorf("this Topic %s Create Failed, reason: ip whitelist switch is on, ip whitelist cannot be empty", topicName)
+		}
 		request.IpWhiteList = ipWhiteList
 	}
 	request.MinInsyncReplicas = &syncReplicaMinNum
@@ -249,8 +252,7 @@ func resourceTencentCloudCkafkaTopicCreate(d *schema.ResourceData, meta interfac
 	if err != nil {
 		return err
 	}
-
-	if len(ipWhiteList) > 0 {
+	if len(ipWhiteList) > 0 && whiteListSwitch {
 		err = ckafkcService.AddCkafkaTopicIpWhiteList(ctx, instanceId, topicName, ipWhiteList)
 		if err != nil {
 			return err
@@ -369,30 +371,33 @@ func resourceTencentCloudCkafkaTopicUpdate(d *schema.ResourceData, meta interfac
 	}
 	//update ip white List
 	if whiteListSwitch {
-		if d.HasChange("ip_white_list") {
-			oldInterface, newInterface := d.GetChange("ip_white_list")
-			oldIpWhiteListInterface := oldInterface.([]interface{})
-			newIpWhiteListInterface := newInterface.([]interface{})
-			var oldIpWhiteList, newIpWhiteList []*string
-			for _, value := range oldIpWhiteListInterface {
-				oldIpWhiteList = append(oldIpWhiteList, helper.String(value.(string)))
-			}
-			for _, value := range newIpWhiteListInterface {
-				newIpWhiteList = append(newIpWhiteList, helper.String(value.(string)))
-			}
+		oldInterface, newInterface := d.GetChange("ip_white_list")
+		oldIpWhiteListInterface := oldInterface.([]interface{})
+		newIpWhiteListInterface := newInterface.([]interface{})
+		var oldIpWhiteList, newIpWhiteList []*string
+		for _, value := range oldIpWhiteListInterface {
+			oldIpWhiteList = append(oldIpWhiteList, helper.String(value.(string)))
+		}
+		for _, value := range newIpWhiteListInterface {
+			newIpWhiteList = append(newIpWhiteList, helper.String(value.(string)))
+		}
+		if len(oldIpWhiteList) > 0 {
 			error := ckafkcService.RemoveCkafkaTopicIpWhiteList(ctx, instanceId, topicName, oldIpWhiteList)
 			if error != nil {
 				return fmt.Errorf("IP whitelist Modification failed, reason[%s]\n", error.Error())
 			}
-			error = ckafkcService.AddCkafkaTopicIpWhiteList(ctx, instanceId, topicName, newIpWhiteList)
-			if error != nil {
-				return fmt.Errorf("IP whitelist Modification failed, reason[%s]\n", error.Error())
-			}
+		}
+		if len(newIpWhiteList) == 0 {
+			return fmt.Errorf("this Topic %s Create Failed, reason: ip whitelist switch is on, ip whitelist cannot be empty", topicName)
+		}
+		error := ckafkcService.AddCkafkaTopicIpWhiteList(ctx, instanceId, topicName, newIpWhiteList)
+		if error != nil {
+			return fmt.Errorf("IP whitelist Modification failed, reason[%s]\n", error.Error())
 		}
 	} else {
 		//IP whiteList Switch not turned on, and the ip whitelist cannot be modified
 		if d.HasChange("ip_white_list") {
-			return fmt.Errorf("This Topic %s IP whitelist Modification failed, reason: The Ip Whitelist Switch is not turned on.", topicName)
+			return fmt.Errorf("this Topic %s IP whitelist Modification failed, reason: The Ip Whitelist Switch is not turned on", topicName)
 		}
 	}
 	err := ckafkcService.ModifyCkafkaTopicAttribute(ctx, request)

tencentcloud/resource_tc_ckafka_topic_test.go

@@ -69,15 +69,6 @@ func testAccTencentCloudKafkaTopicDestory(s *terraform.State) error {
 		if len(split) < 2 {
 			continue
 		}
-		//check ckafka instance
-		has, err := ckafkcService.DescribeCkafkaById(ctx, split[0])
-		if err != nil {
-			return err
-		}
-		//ckafka does not exist
-		if !has {
-			return nil
-		}
 		//check ckafka topic
 		_, has, error := ckafkcService.DescribeCkafkaTopicByName(ctx, split[0], split[1])
 		if error != nil {

tencentcloud/service_tencentcloud_ckafka.go

@@ -6,10 +6,9 @@ import (
 	"log"
 	"strings"
 
-	"github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common/errors"
-
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	ckafka "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/ckafka/v20190819"
+	"github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common/errors"
 	"github.com/terraform-providers/terraform-provider-tencentcloud/tencentcloud/connectivity"
 	"github.com/terraform-providers/terraform-provider-tencentcloud/tencentcloud/internal/helper"
 	"github.com/terraform-providers/terraform-provider-tencentcloud/tencentcloud/ratelimit"
@@ -455,7 +454,18 @@ func (me *CkafkaService) DescribeCkafkaTopics(ctx context.Context, instanceId st
 
 	request.Offset = &offset
 	request.Limit = &limit
-
+	//check ckafka exist
+	ckafkaExist, error := me.DescribeCkafkaById(ctx, instanceId)
+	if error != nil {
+		if sdkErr, ok := error.(*errors.TencentCloudSDKError); ok {
+			if sdkErr.Code == CkafkaInstanceNotFound {
+				return
+			}
+		}
+	}
+	if !ckafkaExist {
+		return
+	}
 	for {
 		ratelimit.Check(request.GetAction())
 		response, err := me.client.UseCkafkaClient().DescribeTopicDetail(request)
@@ -489,6 +499,7 @@ func (me *CkafkaService) CreateCkafkaTopic(ctx context.Context, request *ckafka.
 	}
 	if response == nil || response.Response == nil || response.Response.Result == nil {
 		errRet = fmt.Errorf("TencentCloud SDK return nil response, %s", request.GetAction())
+		return
 	}
 	if *response.Response.Result.TopicId == "" {
 		errRet = fmt.Errorf("TencentCloud SDK returns empty ckafka topic ID, %s", request.GetAction())
@@ -500,18 +511,6 @@ func (me *CkafkaService) CreateCkafkaTopic(ctx context.Context, request *ckafka.
 
 func (me *CkafkaService) DescribeCkafkaTopicByName(ctx context.Context, instanceId string, topicName string) (topic *ckafka.TopicDetail, has bool, errRet error) {
 	var topicList []*ckafka.TopicDetail
-	//check ckafka exist
-	ckafkaExist, error := me.DescribeCkafkaById(ctx, instanceId)
-	if error != nil {
-		if sdkErr, ok := error.(*errors.TencentCloudSDKError); ok {
-			if sdkErr.Code == CkafkaInstanceNotFound {
-				return nil, false, error
-			}
-		}
-	}
-	if !ckafkaExist {
-		return nil, false, error
-	}
 	//check ckafka topic exist
 	err := resource.Retry(readRetryTimeout, func() *resource.RetryError {
 		list, err := me.DescribeCkafkaTopics(ctx, instanceId, topicName)
@@ -529,9 +528,7 @@ func (me *CkafkaService) DescribeCkafkaTopicByName(ctx context.Context, instance
 		if *v.TopicName == topicName {
 			has = true
 			topic = v
-			return
-		} else {
-			continue
+			break
 		}
 	}
 	return
@@ -556,6 +553,7 @@ func (me *CkafkaService) DescribeCkafkaTopicAttributes(ctx context.Context, inst
 	}
 	if response == nil || response.Response == nil || response.Response.Result == nil {
 		errRet = fmt.Errorf("TencentCloud SDK return nil response, %s", request.GetAction())
+		return
 	}
 	topicInfo = response.Response.Result
 	return
@@ -575,22 +573,21 @@ func (me *CkafkaService) AddCkafkaTopicIpWhiteList(ctx context.Context, instaneI
 	request.IpWhiteList = whiteIpList
 	ratelimit.Check(request.GetAction())
 	var response *ckafka.CreateTopicIpWhiteListResponse
-	err := resource.Retry(readRetryTimeout, func() *resource.RetryError {
+	errRet = resource.Retry(readRetryTimeout, func() *resource.RetryError {
 		resp, e := me.client.UseCkafkaClient().CreateTopicIpWhiteList(request)
 		if e != nil {
 			return retryError(e)
 		}
 		response = resp
 		return nil
 	})
-	if err != nil {
-		errRet = err
+	if errRet != nil {
 		return
 	}
 	if response == nil || response.Response == nil || response.Response.Result == nil {
 		errRet = fmt.Errorf("TencentCloud SDK return nil response, %s", request.GetAction())
 	}
-	return err
+	return errRet
 }
 
 func (me *CkafkaService) RemoveCkafkaTopicIpWhiteList(ctx context.Context, instaneId string, topicName string, whiteIpList []*string) (errRet error) {
@@ -607,25 +604,23 @@ func (me *CkafkaService) RemoveCkafkaTopicIpWhiteList(ctx context.Context, insta
 	request.IpWhiteList = whiteIpList
 	ratelimit.Check(request.GetAction())
 	var response *ckafka.DeleteTopicIpWhiteListResponse
-	err := resource.Retry(readRetryTimeout, func() *resource.RetryError {
+	errRet = resource.Retry(readRetryTimeout, func() *resource.RetryError {
 		resp, e := me.client.UseCkafkaClient().DeleteTopicIpWhiteList(request)
 		if e != nil {
 			return retryError(e)
 		}
 		response = resp
 		return nil
 	})
-	if err != nil {
-		errRet = err
+	if errRet != nil {
 		return
 	}
 	if response == nil || response.Response == nil || response.Response.Result == nil {
 		errRet = fmt.Errorf("TencentCloud SDK return nil response, %s", request.GetAction())
 	}
-	return err
+	return errRet
 }
 
-//get ckafka instance by instanceId.
 func (me *CkafkaService) DescribeCkafkaById(ctx context.Context, instanceId string) (has bool, errRet error) {
 	logId := getLogId(ctx)
 	request := ckafka.NewDescribeInstancesDetailRequest()
@@ -645,9 +640,7 @@ func (me *CkafkaService) DescribeCkafkaById(ctx context.Context, instanceId stri
 	for _, v := range resp.Response.Result.InstanceList {
 		if *v.InstanceId == instanceId {
 			has = true
-			return
-		} else {
-			continue
+			break
 		}
 	}
 	return
@@ -695,7 +688,7 @@ func (me *CkafkaService) DeleteCkafkaTopic(ctx context.Context, instanceId strin
 		if err != nil {
 			return retryError(err)
 		}
-		if topicList != nil || len(topicList) != 0 {
+		if len(topicList) != 0 {
 			errRet = fmt.Errorf("this Topic %s Delete Failed, timeout", name)
 		}
 		return nil