update code for better use, addind delay and additional condition

[XeniaP]

Update Code

Change Summary

Adding delay in Check_Status to avoid response error on FSS Api
Adding 1 condition to handle the tags of S3 Buckets
Setting else condition to avoid duplicate tags for FSSMonited on add_tags method

PR Checklist

• I've read and followed the Contributing Guide.
• Documents/Readmes
  • Updated accordingly
  • [ x] Not required
• Plugins that have versioning
  • Version bumped and follows Semantic Versioning
  • [x ] Not required

Other Notes

Duplicate tags

API error response

[jessie-y-kuo]

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)

[jessie-y-kuo]

Could you explain a bit more about the response error mentioned here? and why you choose to use 10 seconds as the delay interval?

[jessie-y-kuo]

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)

[jessie-y-kuo]

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?

[jessie-y-kuo]

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

[jessie-y-kuo]

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)

[jessie-y-kuo]

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

[jessie-y-kuo]

Maybe consider logging sentences that are more readable:

Suggested change

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