Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/alicloud_db_instance: fix_add_ModifyDBInstanceConfig #7944

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chaitiansheng0524
Copy link
Contributor

No description provided.

@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch 2 times, most recently from 5e18672 to 2bd60d2 Compare December 6, 2024 03:24
@chaitiansheng0524 chaitiansheng0524 changed the title resource/alicloud_db_instance: fix_add_ModifyDBInstanceConfig resource/alicloud_db_instance: fix_Add_ModifyDBInstanceConfig Dec 6, 2024
@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch 2 times, most recently from 23bbda7 to 95e2679 Compare December 11, 2024 05:48
engine := d.Get("engine").(string)
encryptionKey := d.Get("encryption_key").(string)

// 检查 engine 不是 PostgreSQL、MySQL 或 SQLServer
Copy link
Collaborator

Choose a reason for hiding this comment

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

无效的注释去掉;工具类函数需要增加单测覆盖

Copy link
Contributor Author

Choose a reason for hiding this comment

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

无效注释已去掉,已放回资源类。

@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch 3 times, most recently from 02110e7 to f3ac1e2 Compare December 11, 2024 07:29
@chaitiansheng0524 chaitiansheng0524 changed the title resource/alicloud_db_instance: fix_Add_ModifyDBInstanceConfig resource/alicloud_db_instance: fix_add_ModifyDBInstanceConfig Dec 11, 2024
@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch from f3ac1e2 to 6bf98c6 Compare December 11, 2024 07:40
@@ -1017,6 +1017,16 @@ func TestAccAliCloudRdsDBInstance_SQLServer(t *testing.T) {
}),
),
},
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

新增字段recovery_model和pg_bouncer_enabled,当前只覆盖了更新操作,没有在创建操作指定的单测,需要在别的单测里面指定覆盖一下。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已在新增中覆盖

@@ -397,7 +397,20 @@ func resourceAliCloudDBInstance() *schema.Resource {
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
return d.Get("engine").(string) != "PostgreSQL" && d.Get("engine").(string) != "MySQL" && d.Get("engine").(string) != "SQLServer"
engine := d.Get("engine").(string)
encryptionKey := d.Get("encryption_key").(string)
Copy link
Collaborator

Choose a reason for hiding this comment

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

encryption_key 是非必填的,这里直接断言会不会异常?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

encryption_key参数是非必填的且在未启用加密时,查询实例接口并不会返回这个参数,但是经过本地测试,不会出现断言异常:
image
Uploading image.png…

@@ -1763,6 +1853,8 @@ func resourceAliCloudDBInstanceRead(d *schema.ResourceData, meta interface{}) er
} else if len(slaveZones) == 1 {
d.Set("zone_id_slave_a", slaveZones[0].(map[string]interface{})["ZoneId"])
}
recoveryModel := instance["Extra"].(map[string]interface{})["RecoveryModel"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

接口上一定会返回这个结构吗,不返回的情况下这里的断言会直接抛出异常

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的
6ba01322115ee99f2b86476cd9e8a7d5

* `encryption_key` - (Optional, Available since 1.109.0) The key id of the KMS. Used for encrypting a disk if not null. Only for PostgreSQL, MySQL and SQLServer.
When the instance is PostgreSQL, this parameter can be used to enable, modify, and disable cloud disk encryption.Value range:
- ServiceKey: Enable disk encryption using the service-managed key (Default Service CMK) automatically generated by Alibaba Cloud RDS.
- <密钥>: Use a custom key to enable cloud disk encryption or change the current key. For example: 494c98ce-f2b5-48ab-96ab-36c986b6****.
Copy link
Collaborator

Choose a reason for hiding this comment

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

为什么有中文

Copy link
Contributor Author

Choose a reason for hiding this comment

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

本意想代表此处可以填自定义密钥
已翻译成英文。

@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch 2 times, most recently from 65a5204 to 0ef5412 Compare December 16, 2024 07:35
Copy link
Collaborator

@ChenHanZhang ChenHanZhang left a comment

Choose a reason for hiding this comment

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

approved

@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch 3 times, most recently from 878a2ef to e20b5fb Compare December 18, 2024 03:25
@@ -3179,6 +3212,7 @@ func TestAccAliCloudRdsDBInstance_SQLServer_2019_ServerlessHA(t *testing.T) {
"serverless_config.#": "1",
"serverless_config.0.max_capacity": "8",
"serverless_config.0.min_capacity": "2",
"recovery_model": "simple",
Copy link
Collaborator

Choose a reason for hiding this comment

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

单测不完善,既然现在encryption_key是可查的,那就应该在ImportStateVerifyIgnore中将encryption_key删掉,并在每个单测步骤中,只要Config涉及到encryption_key设置的,都应该在Check中CHECKSET的encryption_key值
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修复

@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch from e20b5fb to 8ccc958 Compare December 18, 2024 05:51
@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_add_ModifyDBInstanceConfig branch from 8ccc958 to 41d48c0 Compare December 18, 2024 08:27
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