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: Handle empty or invalid Sumo base URL #2240

Merged
merged 9 commits into from
Apr 27, 2022
Merged

fix: Handle empty or invalid Sumo base URL #2240

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ed4892b
fix: Handle empty or invalid Sumo base URL
starzu-sumo Apr 22, 2022
12a4f28
fix: changelog update
starzu-sumo Apr 22, 2022
ccf127f
fix: Allow non-sumo URLs
starzu-sumo Apr 22, 2022
b4a1ff1
fix: Cleanup shellcheck warnings
starzu-sumo Apr 22, 2022
d9b94b8
fix: Tests adjustments
starzu-sumo Apr 22, 2022
d0e9ca9
fix: Minor bash syntax improvement
starzu-sumo Apr 26, 2022
bb5f620
fix: declare BASE_URL as local before value assignment
starzu-sumo Apr 27, 2022
693309b
fix: small shellcheck fix
starzu-sumo Apr 27, 2022
5413271
chore(changelog): update
sumo-drosiek Apr 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- fix: set source name and category in the FluentD output for events [#2222][#2222]
- fix: proper handling of empty `sumologic.endpoint` in the setup script
sumo-drosiek marked this conversation as resolved.
Show resolved Hide resolved
- docs: FluentD buffer size configuration [#2232][#2232]

[#2232]: https://github.com/SumoLogic/sumologic-kubernetes-collection/pull/2232
sumo-drosiek marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
24 changes: 22 additions & 2 deletions deploy/helm/sumologic/conf/setup/setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,28 @@ if [[ ${DEBUG_MODE,,} == ${DEBUG_MODE_ENABLED_FLAG} ]]; then
done
fi

# Fix URL to remove "v1" or "v1/"
export SUMOLOGIC_BASE_URL=${SUMOLOGIC_BASE_URL%v1*}
function fix_sumo_base_url() {
local BASE_URL=$SUMOLOGIC_BASE_URL

if [[ ! "$BASE_URL" =~ ^https:\/\/.*sumologic\.com\/api\/.*$ ]]; then
Copy link

Choose a reason for hiding this comment

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

We should just check if this is set instead of making sure if it's a sumologic.com subdomain. It's valid to put other urls here for testing - this is why integration tests are currently failing.

BASE_URL="https://api.sumologic.com/api/"
fi

OPTIONAL_REDIRECTION="$(curl -XGET -s -o /dev/null -D - \
-u "${SUMOLOGIC_ACCESSID}:${SUMOLOGIC_ACCESSKEY}" \
"${BASE_URL}"v1/collectors \
| grep -Fi location )"

if [[ ! $OPTIONAL_REDIRECTION =~ ^\s*$ ]]; then
BASE_URL=$( echo $OPTIONAL_REDIRECTION | sed -E 's/.*: (https:\/\/.*(au|ca|de|eu|fed|in|jp|us2)?\.sumologic\.com\/api\/).*/\1/' )
Copy link

Choose a reason for hiding this comment

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

Suggested change
BASE_URL=$( echo $OPTIONAL_REDIRECTION | sed -E 's/.*: (https:\/\/.*(au|ca|de|eu|fed|in|jp|us2)?\.sumologic\.com\/api\/).*/\1/' )
BASE_URL=$( echo "$OPTIONAL_REDIRECTION" | sed -E 's/.*: (https:\/\/.*(au|ca|de|eu|fed|in|jp|us2)?\.sumologic\.com\/api\/).*/\1/' )

Copy link
Contributor

Choose a reason for hiding this comment

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

"${OPTIONAL_REDIRECTION}"

fi

BASE_URL=${BASE_URL%v1*}

echo $BASE_URL
Copy link

Choose a reason for hiding this comment

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

Suggested change
echo $BASE_URL
echo "$BASE_URL"

Copy link
Contributor

@sumo-drosiek sumo-drosiek Apr 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

"${BASE_URL}"

}

export SUMOLOGIC_BASE_URL=$(fix_sumo_base_url)
# Support proxy for Terraform
export HTTP_PROXY=${HTTP_PROXY:=""}
export HTTPS_PROXY=${HTTPS_PROXY:=""}
Expand Down