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

update code for better use, addind delay and additional condition #150

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

update code for better use, addind delay and additional condition #150

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 15 additions & 4 deletions deployment/aws-python-storage-stack-automation/storage_stack_lifecycle.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ Resources:

def lambda_handler(event, context):
# get secret
print(event)
secrets = boto3.client('secretsmanager').get_secret_value(SecretId=fss_key)
sm_data = json.loads(secrets["SecretString"])
new_api_format = sm_data["c1apikey"]
Expand Down Expand Up @@ -215,15 +216,19 @@ Resources:
response = s3_client.get_bucket_tagging(Bucket=bucket_name)
tags = response["TagSet"]
tag_status = tags
print("tag_Status1: ", tag_status)
Comment on lines 218 to +219
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe consider logging sentences that are more readable:

Suggested change
tag_status = tags
print("tag_Status1: ", tag_status)
print("Bucket tag set: ", tags)

except ClientError:
no_tags = "does not have tags"
tag_status = no_tags
if tag_status == "does not have tags":
print("does not have tags")
add_tag(s3_client, bucket_name, tag_list=[])
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
else:
Comment on lines 220 to 227
Copy link
Contributor

Choose a reason for hiding this comment

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

We can simplify the code by taking out the if condition when tag_status == "does not have tags" and just directly execute the actions inside the exception section like so:

Suggested change
except ClientError:
no_tags = "does not have tags"
tag_status = no_tags
if tag_status == "does not have tags":
print("does not have tags")
add_tag(s3_client, bucket_name, tag_list=[])
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
else:
except ClientError:
add_tag(s3_client, bucket_name, tag_list=[])
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
return 0

print("tag_Status2: ", tag_status)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we want to check the updated bucket tag after new tags were added, maybe consider adding the log at the end of add_tag() instead?

def add_tag(s3_client, bucket_name, tag_list):
    tag_list.append({'Key':'FSSMonitored', 'Value': 'Yes'})
    print(f"Bucket: {bucket_name} lacks an FSSMonitored tag; adding")
    s3_client.put_bucket_tagging(
        Bucket=bucket_name,
        Tagging={"TagSet": tag_list},
    )
    print("Updated bucket tag set: ", tag_list)

for tags in tag_status:
if tags["Key"] == "FSSMonitored":
print(tags["Value"].lower())
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think we need this since the tag set logs will print out the detailed tag structure and values already.

if tags["Value"].lower() == "no":
# if tag FSSMonitored is no; quit
print(
Expand All @@ -235,8 +240,12 @@ Resources:
elif tags["Value"].lower() != "yes":
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
break
Comment on lines 240 to 242
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, if the user gave an invalid FSSMonitored tag value like FSSMonitored: heehee, this plugin would still deploy the storage stack, right? In this case, should we also update their invalid FSSMonitored value to Yes instead of leaving it as heehee?

add_tag(s3_client, bucket_name, tag_list=tag_status)
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
elif tags["Value"].lower() == "yes":
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
break
else:
add_tag(s3_client, bucket_name, tag_list=tag_status) #move this inside if to avoid duplicate tags
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
Comment on lines +246 to +248
Copy link
Contributor

Choose a reason for hiding this comment

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

By moving the add_tag() and add_storage() inside the for loop means that every tag inside tag_status array that has a key != "FSSMonitored" will trigger the add_tag() and add_storage() once. I don't think this is what we wanted.

Suggested change
else:
add_tag(s3_client, bucket_name, tag_list=tag_status) #move this inside if to avoid duplicate tags
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)
add_tag(s3_client, bucket_name, tag_list=tag_status) #move this inside if to avoid duplicate tags
add_storage(cloud_one_api_key, bucket_name, ext_id, account_id, stack_id, kms_arn)

def add_tag(s3_client, bucket_name, tag_list):
tag_list.append({'Key':'FSSMonitored', 'Value': 'Yes'})
print(f"Bucket: {bucket_name} lacks an FSSMonitored tag; adding")
Expand Down Expand Up @@ -310,10 +319,11 @@ Resources:
check_status(cloud_one_api_key, url)
#check storage stack status
def check_status(cloud_one_api_key, url):
time.sleep(10) #Adding delay to avoid response error in API
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 explain a bit more about the response error mentioned here? and why you choose to use 10 seconds as the delay interval?

#gather stack status
st_call = http.request('GET', url , headers = {'Authorization': cloud_one_api_key, 'Api-Version': 'v1'})
headers = {'Authorization': cloud_one_api_key, 'Api-Version': 'v1'}
st_call = http.request('GET', url, headers=headers)
status = json.loads(st_call.data.decode('utf-8'))['status']
print("Status: " + status)
while status == 'creating':
st_call = http.request('GET', url , headers = {'Authorization': cloud_one_api_key, 'Api-Version': 'v1'})
Copy link
Contributor

Choose a reason for hiding this comment

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

This part could also use the headers variable you declared as well.

Suggested change
st_call = http.request('GET', url , headers = {'Authorization': cloud_one_api_key, 'Api-Version': 'v1'})
st_call = http.request('GET', url, headers=headers)

status = json.loads(st_call.data.decode('utf-8'))['status']
Expand All @@ -322,6 +332,7 @@ Resources:
print('Deployed Successfully')
else:
print('Deployment Failed')

TracingConfig:
Mode: Active
MemorySize: 128
Expand Down