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.

Sign up for GitHub

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

Jump to bottom

fix: Change lower case from bash variable expansion to tr #1064

Merged
merged 3 commits into from Mar 15, 2022
Merged

fix: Change lower case from bash variable expansion to tr #1064

merged 3 commits into from Mar 15, 2022

Conversation

lazize
Copy link
Contributor

@lazize lazize commented Mar 9, 2022

Context

Bash variable expansion doesn't work on old bash version from macOS

Description

Change lower case transformation from bash variable expansion to tr '[:upper:]' '[:lower:]' command.
Fix issue #1058

License

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

@lazize lazize requested a review from a team as a code owner March 9, 2022 11:47
@lazize
Copy link
Contributor Author

lazize commented Mar 9, 2022

extra715
extra780
extra785
extra7101
check_sample

@lazize lazize changed the title fix(extra715): Change lower case from bash variable expansion to tr fix: Change lower case from bash variable expansion to tr Mar 9, 2022
@jfagoagas jfagoagas added severity/high Bug capable of collapsing large parts of the execution. status/waiting-for-revision Waiting for maintainer's revision labels Mar 10, 2022
n4ch04
n4ch04 suggested changes Mar 14, 2022
View reviewed changes
checks/check_extra7101 Outdated
continue
fi
if [[ "${AUDIT_LOGS_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain AUDIT_LOGS enabled" "$regx" "$domain"
if [[ $(echo "${AUDIT_LOGS_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this to
tr '[:upper:]' '[:lower:]' <<< "${AUDIT_LOGS_ENABLED}"

checks/check_extra715 Outdated
if [[ "${SEARCH_SLOWLOG_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain SEARCH_SLOW_LOGS enabled" "$regx" "$domain"
read -r SEARCH_SLOWLOG_ENABLED INDEX_SLOWLOG_ENABLED <<< "$SLOWLOG_ENABLED" && {
if [[ $(echo "${SEARCH_SLOWLOG_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous
tr '[:upper:]' '[:lower:]' <<< "${SEARCH_SLOWLOG_ENABLED}"

checks/check_extra715 Outdated
if [[ "${SEARCH_SLOWLOG_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain SEARCH_SLOW_LOGS enabled" "$regx" "$domain"
read -r SEARCH_SLOWLOG_ENABLED INDEX_SLOWLOG_ENABLED <<< "$SLOWLOG_ENABLED" && {
if [[ $(echo "${SEARCH_SLOWLOG_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous
tr '[:upper:]' '[:lower:]' <<< "${SEARCH_SLOWLOG_ENABLED}"

checks/check_extra715 Outdated
if [[ "${INDEX_SLOWLOG_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain INDEX_SLOW_LOGS enabled" "$regx" "$domain"

if [[ $(echo "${INDEX_SLOWLOG_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous
tr '[:upper:]' '[:lower:]' <<< "${INDEX_SLOWLOG_ENABLED}"

checks/check_extra715 Outdated
if [[ "${INDEX_SLOWLOG_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain INDEX_SLOW_LOGS enabled" "$regx" "$domain"

if [[ $(echo "${INDEX_SLOWLOG_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous
tr '[:upper:]' '[:lower:]' <<< "${INDEX_SLOWLOG_ENABLED}"

checks/check_extra780 Outdated
continue
fi
if [[ "${CHECK_IF_COGNITO_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain has Amazon Cognito authentication for Kibana enabled" "$regx" "$domain"
if [[ $(echo "${CHECK_IF_COGNITO_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous
tr '[:upper:]' '[:lower:]' <<< "${CHECK_IF_COGNITO_ENABLED}"

checks/check_extra780 Outdated
continue
fi
if [[ "${CHECK_IF_COGNITO_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain has Amazon Cognito authentication for Kibana enabled" "$regx" "$domain"
if [[ $(echo "${CHECK_IF_COGNITO_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous
tr '[:upper:]' '[:lower:]' <<< "${CHECK_IF_COGNITO_ENABLED}"

checks/check_extra785 Outdated
if [[ ${update_status,,} != "false" ]];then
textInfo "$regx: Amazon ES domain $domain v$es_version has updates available" "$regx" "$domain"
read -r update_status es_version <<< "${CHECK_IF_UPDATE_AVAILABLE_AND_VERSION}" &&
if [[ $(echo "${update_status}" | tr '[:upper:]' '[:lower:]') != "false" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous:
tr '[:upper:]' '[:lower:]' <<< "${CHECK_IF_COGNITO_ENABLED}"

checks/check_extra785 Outdated
if [[ ${update_status,,} != "false" ]];then
textInfo "$regx: Amazon ES domain $domain v$es_version has updates available" "$regx" "$domain"
read -r update_status es_version <<< "${CHECK_IF_UPDATE_AVAILABLE_AND_VERSION}" &&
if [[ $(echo "${update_status}" | tr '[:upper:]' '[:lower:]') != "false" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in previous:
tr '[:upper:]' '[:lower:]' <<< "${CHECK_IF_COGNITO_ENABLED}"

@n4ch04
Copy link
Contributor

n4ch04 commented Mar 14, 2022

@lazize, I have requested a few changes more related with the style we are using in the project, please check it out and thank you for your contribution !!

@n4ch04 n4ch04 self-requested a review March 14, 2022 11:14
n4ch04
n4ch04 approved these changes Mar 15, 2022
View reviewed changes
checks/check_extra7101 Outdated
continue
fi
if [[ "${AUDIT_LOGS_ENABLED,,}" == "true" ]];then
textPass "$regx: Amazon ES domain $domain AUDIT_LOGS enabled" "$regx" "$domain"
if [[ $(echo "${AUDIT_LOGS_ENABLED}" | tr '[:upper:]' '[:lower:]') == "true" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this to
tr '[:upper:]' '[:lower:]' <<< "${AUDIT_LOGS_ENABLED}"

@n4ch04 n4ch04 merged commit e77cd6b into prowler-cloud:master Mar 15, 2022
@n4ch04
Copy link
Contributor

n4ch04 commented Mar 15, 2022

Thanks for all !! @lazize

@n4ch04 n4ch04 mentioned this pull request Mar 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n4ch04 n4ch04 n4ch04 approved these changes

@toniblyx toniblyx Awaiting requested review from toniblyx toniblyx was automatically assigned from prowler-cloud/prowler-team

@jfagoagas jfagoagas Awaiting requested review from jfagoagas jfagoagas was automatically assigned from prowler-cloud/prowler-team

@sergargar sergargar Awaiting requested review from sergargar sergargar was automatically assigned from prowler-cloud/prowler-team

Assignees
No one assigned
Labels
severity/high Bug capable of collapsing large parts of the execution. status/waiting-for-revision Waiting for maintainer's revision
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lazize @n4ch04 @jfagoagas