fix(queries): support all valid CloudWatch Logs retention periods #7450
Conversation
|
Sorry, I do not understand what the check is asking me to do. Happy to fix with guidance. |
|
Hi @jamesbascle, how are you? Thanks for your contribution 🙌 About the "title-check" GitHub action: it was failing because the first letter after the double points ( We also have this query for two other platforms besides Crossplane:
Would you like to update them too, or would you prefer that I handle it? |
|
Hey! I'll try to update those tomorrow when I'm back at my computer. Thanks
…On Sat, Apr 26, 2025, 6:02 PM Rui Araújo Gomes ***@***.***> wrote:
Assigned #7450 <#7450> to
@jamesbascle <https://github.com/jamesbascle>.
—
Reply to this email directly, view it on GitHub
<#7450 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA76RBJJENQ4ZSXISSGXRR323P65TAVCNFSM6AAAAAB34MPFPKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJXGQYTQNZZHAYTQNA>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
|
@cx-rui-araujo The unit test runner failed on this...I don't really know what i'm looking at here, but it doesn't seem related to the change I made. Any advice? |
|
Hi @jamesbascle, We usually add specific tests for queries, but in this case, I checked and we don't cover all valid values individually (for example, /test/negative.yaml). |
There was a problem hiding this comment.
LGTM!
As @cx-rui-araujo explained, I don't see a reason why we should be testing the full list of values, but we could improve the unit tests with better use cases... Either way, we can do that in the future; doesn't need to be added in this PR.
Thanks for your contribution!
There was a problem hiding this comment.
LGTM!
I'm merging this PR.
These changes will be included in the next KICS release (v2.1.8).
We just released a version last week, so the next one will probably be mid to late next month (May).
@jamesbascle, Thanks for your contribution!
…eckmarx#7450) * Support all valid CloudWatch Logs retention periods Pulled from latest documentation @ https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group * fix AWS CWL retention period valid values for terraform and ansible --------- Co-authored-by: jb <git@noxowa.com>
Pulled from latest documentation @ https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group
Closes #
Reason for Proposed Changes
Proposed Changes
I submit this contribution under the Apache-2.0 license.