Skip to content

Conversation

@n4ch04
Copy link
Contributor

@n4ch04 n4ch04 commented Feb 8, 2022

Context

With last change to exactly match elements included in the whitelist and log info returned whitelist feature was broken not whitelisting multitude of items

Description

Fix the feature, rolling back to regular expression checking to find if whitelisted element is included into log message

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 review from a team, jfagoagas and toniblyx February 8, 2022 12:38
@n4ch04 n4ch04 added the status/waiting-for-revision Waiting for maintainer's revision label Feb 8, 2022
toniblyx
toniblyx previously approved these changes Feb 8, 2022
Copy link
Member

@toniblyx toniblyx left a comment

Choose a reason for hiding this comment

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

LGTM

resource_value=$(awk -F ":" '{print $2}' <<< $excluded_item)
# Checked value is the second field of log message divided by spaces
checked_value=$(awk '{print $2}' <<< $1)
checked_value=$1
Copy link
Member

@jfagoagas jfagoagas Feb 8, 2022

Choose a reason for hiding this comment

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

Please link $1 to a local variable with a proper name to better follow the execution flow.

Also, quote variables like "${varName}"

Copy link
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.

LGTM

@n4ch04 n4ch04 merged commit 3097ba6 into master Feb 14, 2022
@n4ch04 n4ch04 deleted the PRWLR-151-fix-whitelist-checking branch February 14, 2022 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/waiting-for-revision Waiting for maintainer's revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants