Skip to content

fix(include/outputs): Whitelist logic reformulated to exactly match input#1029

Merged
toniblyx merged 3 commits intomasterfrom
PRWLR-38-whitelist-does-not-match-exact-s-3-bucket-961
Feb 4, 2022
Merged

fix(include/outputs): Whitelist logic reformulated to exactly match input#1029
toniblyx merged 3 commits intomasterfrom
PRWLR-38-whitelist-does-not-match-exact-s-3-bucket-961

Conversation

@n4ch04
Copy link
Copy Markdown
Contributor

@n4ch04 n4ch04 commented Feb 3, 2022

Context

#961

Description

Logic of whitelisting matching reformulated to exactly macth input

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@n4ch04 n4ch04 requested a review from jfagoagas February 3, 2022 12:17
@n4ch04 n4ch04 self-assigned this Feb 3, 2022
@n4ch04 n4ch04 changed the title fix(inlcude/outputs) Whitelist logic reformulated to exactly match input fix(inlcude/outputs): Whitelist logic reformulated to exactly match input Feb 3, 2022
@n4ch04 n4ch04 changed the title fix(inlcude/outputs): Whitelist logic reformulated to exactly match input fix(include/outputs): Whitelist logic reformulated to exactly match input Feb 3, 2022
Comment thread include/outputs Outdated
@@ -169,14 +169,17 @@ textFail(){
colorcode="$BAD"
while read -r i; do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this i?

I think a more expressive name would be better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed, variable is called excluded_item instead

Comment thread include/outputs Outdated
# Checked value is the second field of log message divided by spaces
checked_value=$(awk '{print $2}' <<< $1)
echo
if [[ ${ignore_check_name} != "${CHECK_NAME}" ]]; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ ${ignore_check_name} != "${CHECK_NAME}" ]]; then
if [[ "${ignore_check_name}" != "${CHECK_NAME}" ]]; then

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Included

Comment thread include/outputs Outdated
fi
if [[ $1 =~ ${resource_value} ]]; then
# To set WARNING flag both values must be exactly the same
if [[ ${checked_value} == ${resource_value} ]]; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ ${checked_value} == ${resource_value} ]]; then
if [[ "${checked_value}" == "${resource_value}" ]]; then

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Included

Comment thread include/outputs Outdated
# To set WARNING flag both values must be exactly the same
if [[ ${checked_value} == ${resource_value} ]]; then
level="WARNING"
colorcode="$WARNING"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
colorcode="$WARNING"
colorcode="${WARNING}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Included

Comment thread include/outputs Outdated
resource_value=$(awk -F ":" '{print $2}' <<< $i)
# Checked value is the second field of log message divided by spaces
checked_value=$(awk '{print $2}' <<< $1)
echo
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this echo necessary here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Deleted

@n4ch04 n4ch04 requested a review from jfagoagas February 3, 2022 15:06
Copy link
Copy Markdown
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

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

Good job @n4ch04 !!

@toniblyx toniblyx merged commit 30ce253 into master Feb 4, 2022
@toniblyx toniblyx deleted the PRWLR-38-whitelist-does-not-match-exact-s-3-bucket-961 branch February 4, 2022 17:07
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