privatezone接入 #604

forcoder · 2018-08-15T15:34:15Z

privatezone接入

CLAassistant · 2018-08-15T15:34:23Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

qiaozhou seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

CLAassistant · 2018-08-15T15:34:23Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

qiaozhou seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

xiaozhu36 · 2018-08-16T03:27:20Z

alicloud/config.go

+func (c *Config) pvtzConn() (*pvtz.Client, error) {
+	return pvtz.NewClientWithAccessKey(c.RegionId, c.AccessKey, c.SecretKey)
+}
+


现在terraform要支持专有云，所以在build client之前要获取用户指定的endpoint；其次，使用NewClientWithOptions来构造新的client

xiaozhu36 · 2018-08-16T03:33:21Z

alicloud/data_source_alicloud_pvtz_zone.go

+				Optional: true,
+			},
+
+			"pvtz_zones": &schema.Schema{


直接使用zones就好，不用“pvtz_zones”

这个是要和region以及zones区分开来，要不会混淆的

不会混淆的，用户在引用是是通过data.alicloud_pvtz_zones.xxx.zones.0.id来引用的

另外，文件data source的文件名应该改为data_source_alicloud_pvtz_zones.go

xiaozhu36 · 2018-08-16T03:33:53Z

alicloud/data_source_alicloud_pvtz_zone.go

+						"zone_name": {
+							Type:     schema.TypeString,
+							Computed: true,
+						},


修改zone_id为id，zone_name 为name

zone_id之类不用和openapi的统一对齐吗？

不需要，如果要和API对其，就没发看了，现在咱们的API都存在很多不规范，现在我所在的团队正太规范化。另外，terraform内部保证对其就好了

xiaozhu36 · 2018-08-16T03:35:54Z

alicloud/data_source_alicloud_pvtz_zone.go

+							Type:     schema.TypeString,
+							Computed: true,
+						},
+						"update_timestamp": {


只保留create_time和update_time就好，去掉create_timestamp和update_timestamp，另外，修改create_time为creation_time

这个不用和openapi返回结果对齐是吧？

xiaozhu36 · 2018-08-16T03:38:28Z

alicloud/data_source_alicloud_pvtz_zone.go

+	var pvtzZones []pvtz.Zone
+	pvtzZoneBindVpcs := make(map[string][]map[string]interface{})
+
+	for true {


93行，直接使用for { 就好了

xiaozhu36 · 2018-08-16T06:25:30Z

alicloud/resource_alicloud_pvtz_zone_record.go

+	request := pvtz.CreateDescribeZoneRecordsRequest()
+	_, zoneId, rr, _ := getRecordIdAndZoneIdAndRr(d, meta)
+	request.ZoneId = zoneId
+	request.Keyword = rr


我认为zoneID和recordId就可以确定一个record

recordId就可以直接确认一个record，但是他没有根据recordId查询record详情的接口,所以需要根据zoneId+rr来查询

这个地方不一定要查询到唯一的一个record，根据zoneId查询到一组record之后，可以根据recordId进行过滤，这样可以省去rr

xiaozhu36 · 2018-08-16T06:26:24Z

alicloud/resource_alicloud_pvtz_zone_record.go

+
+	request := pvtz.CreateDeleteZoneRecordRequest()
+	recordIdStr, zoneId, rr, _ := getRecordIdAndZoneIdAndRr(d, meta)
+	recordId, _ := strconv.Atoi(recordIdStr)


208行如果有报错，要及时报出来

xiaozhu36 · 2018-08-16T06:27:15Z

alicloud/resource_alicloud_pvtz_zone_record.go

+	if len(parts) != 3 {
+		return "", "", "", fmt.Errorf("invalid resource id")
+	}
+	return parts[0], parts[1], parts[2], nil


getRecordIdAndZoneIdAndRr 和 splitRecordIdAndZoneIdAndRr完全可以合并成一个方法

因为我有场景不是直接使用getRecordIdAndZoneIdAndRr，而是需要使用splitRecordIdAndZoneIdAndRr，所以拆分成两个方法

xiaozhu36 · 2018-08-16T06:28:18Z

alicloud/resource_alicloud_pvtz_zone_record_test.go

+			return fmt.Errorf("No Record ID is set")
+		}
+		recordIdStr, zoneId, rr, _ := splitRecordIdAndZoneIdAndRr(rs.Primary.ID)
+		recordId, err := strconv.Atoi(recordIdStr)


106行，有错要先报错

xiaozhu36 · 2018-08-16T06:31:00Z

alicloud/service_alicloud_pvtz.go

+	response, err := conn.DescribeZoneInfo(request)
+	if err != nil {
+		return
+	}


可以参考其他service，在查询后构造NotFoundError，比如ZoneNotExists可以放在15行

能给个具体的参考产品？

任何一个都行，比如vpc

xiaozhu36 · 2018-08-21T16:20:55Z

alicloud/resource_alicloud_pvtz_zone.go

+		}
+	}
+
+	d.Partial(false)


115行可以删掉了

xiaozhu36 · 2018-08-21T16:22:48Z

alicloud/resource_alicloud_pvtz_zone_attachment.go

+				ForceNew: true,
+			},
+			"vpc_ids": &schema.Schema{
+				Type:     schema.TypeList,


vpc_ids的类型最好申明为set，否则当vpc_ids内的顺序改变时，该资源会出现diff

xiaozhu36 · 2018-08-21T16:27:42Z

alicloud/resource_alicloud_pvtz_zone_record.go

+	request := pvtz.CreateDescribeZoneRecordsRequest()
+	_, zoneId, rr, _ := getRecordIdAndZoneIdAndRr(d, meta)
+	request.ZoneId = zoneId
+	request.Keyword = rr


这个地方不一定要查询到唯一的一个record，根据zoneId查询到一组record之后，可以根据recordId进行过滤，这样可以省去rr

xiaozhu36 · 2018-08-21T16:30:23Z

alicloud/resource_alicloud_pvtz_zone.go

+		}
+
+		if _, err := client.DescribePvtzZoneInfo(d.Id()); err != nil {
+			if NotFoundError(err) || IsExceptedError(err, ZoneNotExists) {


135行直接判断NotFoundError(err)就够了，因为DescribePvtzZoneInfo中应对IsExceptedError(err, ZoneNotExists)进行了处理

xiaozhu36 · 2018-08-21T16:31:21Z

website/docs/d/pvtz_zone_records.html.markdown

+    Provides a list of Private Zone Records which owned by an Alicloud account.
+---
+
+# alicloud\_pvtz_zone_records


第9行修改为：alicloud_pvtz_zone_records
下同

xiaozhu36 · 2018-08-21T16:32:14Z

website/docs/r/pvtz_zone_record.html.markdown

+
+The following attributes are exported:
+
+* `record_id` - The ID of the Private Zone Record.


49行我觉得应该是id，不是record_id

xiaozhu36 · 2018-08-21T16:33:05Z

website/docs/r/pvtz_zone_attachment.html.markdown

+
+* `zone_id` - (Required, Forces new resource) The name of the Private Zone Record.
+* `vpc_ids` - (Required) The id List of the VPC, for example:["vpc-1","vpc-2"].
+


出参中有一个id，要说明

xiaozhu36 · 2018-08-21T16:34:15Z

website/docs/d/pvtz_zone_records.html.markdown

+
+The following attributes are exported:
+
+* `record_id` - ID of the Private Zone Record.


应该是ID，部署record_id 吧

xiaozhu36 · 2018-08-21T16:35:18Z

alicloud/validators.go

@@ -1196,6 +1196,17 @@ func validateNatGatewaySpec(v interface{}, k string) (ws []string, errors []erro
 	return
 }

+func validatePrivateZoneRecordPriority(v interface{}, k string) (ws []string, errors []error) {


这个函数完全可以用validateIntegerInRange替换掉

xiaozhu36 · 2018-08-22T08:47:02Z

alicloud/config.go

+func (c *Config) pvtzConn() (*pvtz.Client, error) {
+	endpoint := LoadEndpoint(c.RegionId, PVTZCode)
+	if endpoint != "" {
+		endpoints.AddEndpointMapping(c.RegionId, string(PVTZCode), endpoint)


既然 private zone是中心化的产品，那么就加一条逻辑吧：当endpoint为空时，endpoint为：pvtz.aliyuncs.com

xiaozhu36 · 2018-08-22T08:51:51Z

alicloud/resource_alicloud_pvtz_zone_attachment.go

+			return fmt.Errorf("bindZoneVpc error:%#v", err)
+		}
+
+		d.SetId(args.ZoneId)


83行就不用再set id了

xiaozhu36 · 2018-08-22T08:56:28Z

alicloud/resource_alicloud_pvtz_zone_attachment.go

+		}
+
+		if _, err := client.DescribePvtzZoneInfo(d.Id()); err != nil {
+			if NotFoundError(err) || IsExceptedError(err, ZoneNotExists) {


136如果是检查ZoneNotExists，有NotFoundError就够了

xiaozhu36 · 2018-08-22T08:58:17Z

website/alicloud.erb

@@ -85,6 +85,12 @@
                        <li<%= sidebar_current("docs-alicloud-datasource-ram-users") %>>


vendor的变化也要提交上来

xiaozhu36 requested changes Aug 16, 2018

View reviewed changes

forcoder force-pushed the master branch 2 times, most recently from 29acb65 to 7f5858e Compare August 21, 2018 01:24

xiaozhu36 requested changes Aug 21, 2018

View reviewed changes

forcoder force-pushed the master branch from 7f5858e to 15f9ceb Compare August 22, 2018 02:56

xiaozhu36 requested changes Aug 22, 2018

View reviewed changes

forcoder force-pushed the master branch from 15f9ceb to ba2f023 Compare August 22, 2018 09:28

privatezone 接入terraform provider

b51360d

forcoder force-pushed the master branch from ba2f023 to b51360d Compare August 22, 2018 09:56

xiaozhu36 approved these changes Aug 22, 2018

View reviewed changes

xiaozhu36 merged commit d8e8737 into alibaba:master Aug 22, 2018


		The following attributes are exported:

		* `record_id` - The ID of the Private Zone Record.


		* `zone_id` - (Required, Forces new resource) The name of the Private Zone Record.
		* `vpc_ids` - (Required) The id List of the VPC, for example:["vpc-1","vpc-2"].


		The following attributes are exported:

		* `record_id` - ID of the Private Zone Record.

		@@ -85,6 +85,12 @@
		<li<%= sidebar_current("docs-alicloud-datasource-ram-users") %>>

privatezone接入 #604

privatezone接入 #604

Conversation

forcoder commented Aug 15, 2018 • edited

CLAassistant commented Aug 15, 2018 • edited

CLAassistant commented Aug 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forcoder Aug 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forcoder commented Aug 15, 2018 •

edited

CLAassistant commented Aug 15, 2018 •

edited

forcoder Aug 16, 2018 •

edited