-
Notifications
You must be signed in to change notification settings - Fork 2
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
✨ Add the new exclude fields for serverless AWS integration. Mark old fields as deprecated. #138
Conversation
Signed-off-by: Preslav <[email protected]>
15f9938
to
f7d1a58
Compare
"ebs_volume_scan": schema.BoolAttribute{ | ||
MarkdownDescription: "Enable EBS volume scan.", | ||
Optional: true, | ||
}, | ||
"ebs_scan_options": schema.SingleNestedAttribute{ | ||
Required: true, | ||
Required: true, | ||
DeprecationMessage: "This field is deprecated and will be removed in the future.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is not missing a flag here that indicates the argument is deprecated? Or just with this message it will detect it as deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just by setting the message, there's no deprecated
field
"ebs_volume_scan": schema.BoolAttribute{ | ||
MarkdownDescription: "Enable EBS volume scan.", | ||
Optional: true, | ||
}, | ||
"ebs_scan_options": schema.SingleNestedAttribute{ | ||
Required: true, | ||
Required: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this argument is deprecated, can we not make it a required field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once removed, I'll approve!
|
||
Optional: | ||
|
||
- `ebs_volume_scan` (Boolean) Enable EBS volume scan. | ||
- `exclude_instance_ids_filter` (List of String) List of instance IDs to exclude. | ||
- `exclude_regions_filter` (List of String) List of regions to exclude. | ||
- `exclude_tags_filter` (Map of String) Excluded Tags filter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish we could have used a Set here to define a nested set of filters. Like:
filters {
exclude_tags = [""]
exclude_regions = [""]
exclude...
}
Maybe in a new major version we can do this.
Signed-off-by: Preslav <[email protected]>
No description provided.