Refactor getTagCreatedByUid() to distinguish multiple Cloud Orchestrator access

@sekinet

Thank you for the refactoring. It looks good to me.

@xiaohua-guan

What do you think?

@yas @sekinet

I posted my comment. Please check it.

@sekinet

Thank you for the update. I tested the patch and had the following error.

An error occurred when calling K8s API: PUT /namespaces/admin-project-20210322-005514
Status Code: 422
Error reason: Invalid
Message: Namespace "admin-project-20210322-005514" is invalid: metadata.annotations: Invalid value: "k8s_created_by_uid__5f711c16-73c7-4326-89cc-95dc4e36a3c0__cloudopt_k8s_1": name part must be no more than 63 characters
The Namespace admin-project-20210322-005514 could not be updated.

I think we need to consider the length of UUID. e.g.

• Shorten the tag key prefix from k8s_created_by_uid to k8s_created_uid.
• Just use the last hex such as 95dc4e36a3c0 in our case.
• Restrict and validate the length of the name in K8s cloud service provider creation form by considering the tag prefix and UUID.

@sekinet

Thank you for the update. I have one comment. Thanks

@sekinet

Thank you for the update --- Did you update / push the patch into your working branch? I couldn't find the change that I requested.

As I commented above, I suggest the following:

1. Since getTagCreatedByUid is common, so we should move it to CloudContentEntityTrait::getTagCreatedByUid($bundle, $cloud_context). ($bundle should be basically equal to a module name.)
2. We can change the description in each cloud service provider form in e.g. k8s_form_cloud_config_k8s_add_form_alter(). We need to implement in the tag supported modules such as aws_cloud, k8s, and openstack.

@sekinet

Thank you for the update. I posted my comments. Please check the ones. Thanks

@sekinet

Thank you for the update. I posted my comments above. Basically we can use a global service container such as \Drupal::service('cloud') in *.module so that we don't have to introduce the new method CloudContentEntityTrait::getCloudService().

@sekinet

As per our discussion, please let us change the status to Needs work to double-check AWS tag creation.

@sekinet

Thank you for the update. I tested the patch and it looks good to me. I'll change the status of this issue to RTBC. Please let us hold this patch until we create the 4.x branch. Thanks

@sekinet

Thank you for adding the test cases. It looks good to me but one thing that I want ask you is to keep the blank lines in SecurityGroupTest for the code readability if you don't have any specific reasons that you removed the ones. Thanks

@sekinet

Thank you for the update. It looks good to me to now. I'll change the status of this issue to RTBC. Please let us hold this patch until we create the 4.x branch. Thanks

@sekinet

Thank you for the update. It looks good.

@xiaohua-guan

Could you please review the patch? Thanks

@yas @sekinet

It looks good to me now. Thanks.

@sekinet

Thank you for the update. It looks good to me.

@baldwinlouie

What do you think? Thanks

@yas and @sekinet

This is a great patch. I have the above comments.

@baldwinlouie

Thank you for your reivew.

@sekinet

Thank you for the update. It looks good now. I'll change the status to RTBC. I'll merge the patch to 4.x branch when it's ready.

@sekinet

The patch is now behind against HEAD. Could you please rebase and resolve the conflicts? I'll change the status to Needs work. Thanks

@sekinet

Thank you for the update. It looks good. I'll change the status to RTBC. I'll merge the patch to 4.x branch when it's ready.

@sekinet

@all

The patch is now behind against HEAD on 4.x branch. I'll take over to fix and change the status to Needs work. Thanks

@sekinet

Thank you for the rebase. It looks good. The tests have been passed successfully, so I'll merge the patch to 4.x and close this issue as Fixed.