fix: Handle empty or invalid Sumo base URL

[starzu-sumo]

Description

It's allowed to left sumologic.endpoint empty. In such case SUMOLOGIC_BASE_URL is empty and breaks logic of get_remaining_fields and other API calls.

Checklist

• Changelog updated

Testing performed

• Redeploy fluentd and fluentd-events pods
• Confirm events, logs, and metrics are coming in

[starzu-sumo]

Tested with the following script:

#!/bin/bash

function fix_sumo_base_url() {
  local BASE_URL=$SUMOLOGIC_BASE_URL

  if [[ ! "$BASE_URL" =~ ^https:\/\/.*sumologic\.com\/api\/.*$ ]]; then
    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/' )
  fi

  BASE_URL=${BASE_URL%v1*}

  echo $BASE_URL
}

TESTS=( \
  "" \
  "https://blabla.com/api/" \
  "http://api.sumologic.com/api/" \
  "https://sumologic.com/" \
  "https://api.jp.sumologic.com/api/" \
  "https://api.sumologic.com/api/" \
  "https://api.de.sumologic.com/api/" \
  "https://api.de.sumologic.com/api/v1/" \
  "https://api.de.sumologic.com/api/v1/collectors" \
)

for TEST in ${TESTS[@]}; do
  SUMOLOGIC_BASE_URL=$TEST
  echo $SUMOLOGIC_BASE_URL
  echo $(fix_sumo_base_url)
  echo "----------------------"
done

unset SUMOLOGIC_BASE_URL
echo "unset"
echo $(fix_sumo_base_url)

Output with keys from de deployment:

https://blabla.com/api/
https://api.de.sumologic.com/api/
----------------------
http://api.sumologic.com/api/
https://api.de.sumologic.com/api/
----------------------
https://sumologic.com/
https://api.de.sumologic.com/api/
----------------------
https://api.jp.sumologic.com/api/
https://api.de.sumologic.com/api/
----------------------
https://api.sumologic.com/api/
https://api.de.sumologic.com/api/
----------------------
https://api.de.sumologic.com/api/
https://api.de.sumologic.com/api/
----------------------
https://api.de.sumologic.com/api/v1/
https://api.de.sumologic.com/api/
----------------------
https://api.de.sumologic.com/api/v1/collectors
https://api.de.sumologic.com/api/
----------------------
unset
https://api.de.sumologic.com/api/

[swiatekm]

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.

[swiatekm]

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/' )

[sumo-drosiek]

"${OPTIONAL_REDIRECTION}"

[swiatekm]

Suggested change

	echo $BASE_URL
	echo "$BASE_URL"

[sumo-drosiek]

"${BASE_URL}"

[sumo-drosiek]

@starzu-sumo overall LGTM, but please run shellcheck against generated sh file?

[starzu-sumo]

From shellcheck:

[Line 8:](javascript:setPosition(8, 26))
if [[ ${DEBUG_MODE,,} == ${DEBUG_MODE_ENABLED_FLAG} ]]; then
                         ^-- [SC2053](https://github.com/koalaman/shellcheck/wiki/SC2053) (warning): Quote the right-hand side of == in [[ ]] to prevent glob matching.
 
[Line 39:](javascript:setPosition(39, 8))
export SUMOLOGIC_BASE_URL=$(fix_sumo_base_url)
       ^-- [SC2155](https://github.com/koalaman/shellcheck/wiki/SC2155) (warning): Declare and assign separately to avoid masking return values.
 
[Line 47:](javascript:setPosition(47, 14))
    readonly RESPONSE="$(curl -XGET -s \
             ^-- [SC2155](https://github.com/koalaman/shellcheck/wiki/SC2155) (warning): Declare and assign separately to avoid masking return values.
 
[Line 58:](javascript:setPosition(58, 14))
    readonly RESPONSE=$(get_remaining_fields)
             ^-- [SC2155](https://github.com/koalaman/shellcheck/wiki/SC2155) (warning): Declare and assign separately to avoid masking return values.
 
[Line 71:](javascript:setPosition(71, 14))
    readonly REMAINING=$(jq -e '.remaining' <<< "${RESPONSE}")
             ^-- [SC2155](https://github.com/koalaman/shellcheck/wiki/SC2155) (warning): Declare and assign separately to avoid masking return values.
 
[Line 89:](javascript:setPosition(89, 14))
    readonly FIELDS_RESPONSE="$(curl -XGET -s \
             ^-- [SC2155](https://github.com/koalaman/shellcheck/wiki/SC2155) (warning): Declare and assign separately to avoid masking return values.

[sumo-drosiek]

Please fix it 😅

I believe first is obvious, the second is explained here

[sumo-drosiek]

Thank you!

[sumo-drosiek]

@starzu-sumo Sorry for another comment, I was just checking before merge
Didn't lint complain about this line? 😨

[starzu-sumo]

Nope, do you want to change it to the following anyway?

BASE_URL=$SUMOLOGIC_BASE_URL
local BASE_URL

[sumo-drosiek]

I was thinking rather about "${SUMOLOGIC_BASE_URL}"

[sumo-drosiek]

but I believe both changes should be added

[sumo-drosiek]

@starzu-sumo thank you for the change and patience, I created an issue which will help to resolve the bash syntax. Ideally I would ask you to do it manually for now, but as we have an issue we will resolve it sooner or later. So, it's up to you :)

#2248