-
-
Notifications
You must be signed in to change notification settings - Fork 332
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.
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
feat: make it possible to use ssm parameters as slack_webhook_url #107
feat: make it possible to use ssm parameters as slack_webhook_url #107
Conversation
@@ -47,7 +56,9 @@ def default_notification(subject, message): | |||
# Send a message to a slack channel | |||
def notify_slack(subject, message, region): | |||
slack_url = os.environ['SLACK_WEBHOOK_URL'] | |||
if not slack_url.startswith("http"): | |||
if 'SLACK_WEBHOOK_URL_IS_SSM_PARAM' in os.environ and os.environ['SLACK_WEBHOOK_URL_IS_SSM_PARAM'] == 'True': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if 'SLACK_WEBHOOK_URL_IS_SSM_PARAM' in os.environ and os.environ['SLACK_WEBHOOK_URL_IS_SSM_PARAM']:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would propose a slightly more "pythontic" way (less chained conditionals):
slack_webhook_url = os.environ.get('SLACK_WEBHOOK_URL')
slack_webhook_ssm_path = os.environ.get('SLACK_WEBHOOK_URL_SSM_PATH')
if slack_webhook_url:
slack_url = decrypt(slack_url)
elif slack_webhook_ssm_path:
slack_url = get_ssm_param(slack_url)
else:
raise Exception('Either `SLACK_WEBHOOK_URL` or `SLACK_WEBHOOK_URL_SSM_PATH` value must be provided')
adding the .get()
will try to retrieve the env var and if not it will by default return None
. You can also add a fallback value of your choosing as well like os.environ.get('MY_ENV_VAR', 17)
@@ -15,6 +15,15 @@ def decrypt(encrypted_url): | |||
except Exception: | |||
logging.exception("Failed to decrypt URL with KMS") | |||
|
|||
def get_ssm_param(ssm_param): | |||
try: | |||
ssm = boto3.client("ssm") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would move the client init outside of the function to cache it and open connection each time :D
WithDecryption=True | ||
).get("Parameter").get("Value") | ||
except Exception: | ||
logging.exception("Failed to get URL from SSM") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably need to raise an exception here and possible change the message to Failed to get Slack webhook URL from SSM
This PR has been automatically marked as stale because it has been open 30 days |
This PR was automatically closed because of stale in 10 days |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
with this change lambda can pick up the url from an ssm parameter.
new variable: slack_webhook_url_is_ssm_param
in case the ssm parameter is encrypted the kms key must also be specified.
Motivation and Context
make it completely avoidable for the url to appear in the state
Breaking Changes
none that I know of
How Has This Been Tested?
already using it in aws