-
Notifications
You must be signed in to change notification settings - Fork 0
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
Allow users to opt-out of changing instance name tags #3
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
route53 = boto3.client('route53') | ||
|
||
HOSTNAME_TAG_NAME = "asg:hostname_pattern" | ||
UPDATE_INSTANCE_TAG_NAME = "asg:update_instance_name" | ||
|
||
LIFECYCLE_KEY = "LifecycleHookName" | ||
ASG_KEY = "AutoScalingGroupName" | ||
|
@@ -45,21 +46,25 @@ def fetch_ip_from_route53(hostname, zone_id): | |
return ip_address | ||
|
||
# Fetches relevant tags from ASG | ||
# Returns tuple of hostname_pattern, zone_id | ||
# Returns dict of tags | ||
def fetch_tag_metadata(asg_name): | ||
logger.info("Fetching tags for ASG: %s", asg_name) | ||
|
||
tag_value = autoscaling.describe_tags( | ||
tags = autoscaling.describe_tags( | ||
Filters=[ | ||
{'Name': 'auto-scaling-group','Values': [asg_name]}, | ||
{'Name': 'key','Values': [HOSTNAME_TAG_NAME]} | ||
{'Name': 'key','Values': [HOSTNAME_TAG_NAME, UPDATE_INSTANCE_TAG_NAME]} | ||
], | ||
MaxRecords=1 | ||
)['Tags'][0]['Value'] | ||
Comment on lines
+53
to
59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: The 'Tags' key is accessed without checking if it exists or if it's empty. This could lead to an IndexError if no tags are found. |
||
|
||
logger.info("Found tags for ASG %s: %s", asg_name, tag_value) | ||
tag_values = { | ||
tag['Key']: tag['Value'] for tag in tags | ||
} | ||
Comment on lines
+61
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: This dictionary comprehension assumes that 'tags' is a list of dictionaries with 'Key' and 'Value' keys. However, 'tags' is actually the value of the first tag found. This will likely cause a runtime error. |
||
|
||
return tag_value.split("@") | ||
logger.info("Found tags for ASG %s: %s", asg_name, tag_values) | ||
|
||
return tag_values | ||
|
||
# Builds a hostname according to pattern | ||
def build_hostname(hostname_pattern, instance_id): | ||
|
@@ -119,13 +124,16 @@ def process_message(message): | |
asg_name = message['AutoScalingGroupName'] | ||
instance_id = message['EC2InstanceId'] | ||
|
||
hostname_pattern, zone_id = fetch_tag_metadata(asg_name) | ||
asg_tag_metadata = fetch_tag_metadata(asg_name) | ||
|
||
hostname_pattern, zone_id = asg_tag_metadata[HOSTNAME_TAG_NAME].split("@") | ||
hostname = build_hostname(hostname_pattern, instance_id) | ||
|
||
if operation == "UPSERT": | ||
ip = fetch_ip_from_ec2(instance_id) | ||
|
||
update_name_tag(instance_id, hostname) | ||
if asg_tag_metadata.get(UPDATE_INSTANCE_TAG_NAME, "true") == "true": | ||
update_name_tag(instance_id, hostname) | ||
Comment on lines
+135
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: The default value 'true' is a string, not a boolean. Consider using a boolean default value instead. |
||
else: | ||
ip = fetch_ip_from_route53(hostname, zone_id) | ||
|
||
|
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.
style: Consider adding a comment explaining the purpose and expected values of UPDATE_INSTANCE_TAG_NAME for better code documentation.