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

logservice audit support resource directory #3983

Merged
merged 1 commit into from Sep 11, 2021
Merged

logservice audit support resource directory #3983

merged 1 commit into from Sep 11, 2021

Conversation

lichengseu
Copy link

log audit service of log_service support resource directory

@CLAassistant
Copy link

CLA assistant check
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.


licheng 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.

@@ -61,7 +66,22 @@ func resourceAlicloudLogAuditUpdate(d *schema.ResourceData, meta interface{}) er

var variableMap = map[string]interface{}{}
mutiAccount := expandStringList(d.Get("multi_account").(*schema.Set).List())
if len(mutiAccount) > 0 {
resourceDirectoryType := d.Get("resource_directory_type").(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里用d.getOk 或者 判断下 resourceDirectoryType是否为 nil

Copy link
Member

Choose a reason for hiding this comment

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

这个地方需要按照如上的意见修改

Copy link
Author

Choose a reason for hiding this comment

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

这里d.Get()方法的实现里已经包了getok,所以不用担心nil的情况。

Copy link
Member

Choose a reason for hiding this comment

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

这个不一样,如果得到是 nil,那么直接转 string 会报错,改为 d.GetOk

Copy link
Author

Choose a reason for hiding this comment

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

image
这里d.Get()其实就是d.GetOK,如果这个参数是nil,会返回""的

Copy link
Member

Choose a reason for hiding this comment

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

这个地方是没有判断是否有 key 这个值存在的,返回的v有可能就是空,这个必须改

@@ -131,13 +151,29 @@ func resourceAlicloudLogAuditRead(d *schema.ResourceData, meta interface{}) erro
}
d.Set("display_name", displayName)
d.Set("aliuid", initMap["aliuid"].(string))
d.Set("multi_account", make([]string, 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

这块是不是有问题

Copy link
Member

Choose a reason for hiding this comment

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

这个有问题,不要设置空的值

Copy link
Author

Choose a reason for hiding this comment

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

已修改,去除该行

@@ -148,6 +184,7 @@ func resourceAlicloudLogAuditRead(d *schema.ResourceData, meta interface{}) erro
delete(initMap, "project")
delete(initMap, "logstore")
delete(initMap, "multi_account")
delete(initMap, "resource_directory")
d.Set("variable_map", initMap)
return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

在写一个针对这个字段的测试用例

Copy link
Author

Choose a reason for hiding this comment

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

测试用例已补充到alicloud/resource_alicloud_log_audit_test.go中


if len(resourceDirectoryType) > 0 {
if strings.ToLower(resourceDirectoryType) != "custom" && strings.ToLower(resourceDirectoryType) != "all" {
return WrapError(errors.New("Invalid resource_directory_type, must be one of 'custom' or 'all'"))
Copy link
Member

Choose a reason for hiding this comment

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

这个地方不要这么写,直接在schema中定义validation 就行

Copy link
Author

Choose a reason for hiding this comment

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

已修改

@@ -131,13 +151,29 @@ func resourceAlicloudLogAuditRead(d *schema.ResourceData, meta interface{}) erro
}
d.Set("display_name", displayName)
d.Set("aliuid", initMap["aliuid"].(string))
d.Set("multi_account", make([]string, 0))
Copy link
Member

Choose a reason for hiding this comment

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

这个有问题,不要设置空的值

@@ -61,7 +66,22 @@ func resourceAlicloudLogAuditUpdate(d *schema.ResourceData, meta interface{}) er

var variableMap = map[string]interface{}{}
mutiAccount := expandStringList(d.Get("multi_account").(*schema.Set).List())
if len(mutiAccount) > 0 {
resourceDirectoryType := d.Get("resource_directory_type").(string)
Copy link
Member

Choose a reason for hiding this comment

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

这个地方需要按照如上的意见修改

"resource_directory_type": "custom",
}),
),
},
},
})
}
Copy link
Member

Choose a reason for hiding this comment

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

请在文档中补充新的属性

Copy link
Author

Choose a reason for hiding this comment

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

已补充

@@ -61,7 +66,22 @@ func resourceAlicloudLogAuditUpdate(d *schema.ResourceData, meta interface{}) er

var variableMap = map[string]interface{}{}
mutiAccount := expandStringList(d.Get("multi_account").(*schema.Set).List())
if len(mutiAccount) > 0 {
resourceDirectoryType := d.Get("resource_directory_type").(string)
Copy link
Member

Choose a reason for hiding this comment

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

这个不一样,如果得到是 nil,那么直接转 string 会报错,改为 d.GetOk

@@ -277,6 +305,7 @@ The following arguments are supported:

- `k8s_ingress_ttl` - (Optional) K8s ingress log ttl. Default 180.
* `multi_account` - (Optional) Multi-account configuration, please fill in multiple aliuid.
* `resource_directory_type` - (Optional) Resource Directory type. Optional values are all or custom. If the value is custom, argument multi_account should be provided.
Copy link
Member

Choose a reason for hiding this comment

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

在括号中增加一个 Availeble的tag:(Optional, Available in 1.135.0+)

Copy link
Author

Choose a reason for hiding this comment

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

已增加

@@ -61,7 +66,22 @@ func resourceAlicloudLogAuditUpdate(d *schema.ResourceData, meta interface{}) er

var variableMap = map[string]interface{}{}
mutiAccount := expandStringList(d.Get("multi_account").(*schema.Set).List())
if len(mutiAccount) > 0 {
resourceDirectoryType := d.Get("resource_directory_type").(string)
Copy link
Member

Choose a reason for hiding this comment

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

这个地方是没有判断是否有 key 这个值存在的,返回的v有可能就是空,这个必须改

set default value for muti_account and rd_enabled

change keyword rd_enabled to rd_type

handle invalid rd_type

add exception handler for no type field

add testcase for resource_directory_type param

remove unnecessary set for multi_account

mv rd_type validation to schema

modify doc for new argument

add available tag for log_audit doc

use GetOK for arugument rd_type
@xiaozhu36 xiaozhu36 merged commit 8a6d422 into aliyun:master Sep 11, 2021
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.

None yet

4 participants