Implement GetExternalTags

[muvaf]

Description of your changes

Implement GetExternalTags to return Crossplane tags for managed resource controllers to tag their external resources.

Makes it easier to implement https://github.com/crossplane/crossplane/blob/master/design/one-pager-managed-resource-api-design.md#external-resource-labeling

Checklist

I have:

• Run make reviewable to ensure this PR is ready for review.
• Ensured this PR contains a neat, self documenting set of commits.
• Updated any relevant documentation, examples, or release notes.
• Updated the RBAC permissions in clusterrole.yaml to include any new types.

[muvaf]

I thought I opened the PR but seems like forgot to click the button :(

@negz @hasheddan I was thinking about adding the following labels, too, to make it possible do more detailed resource filterin in cloud provider:

• crossplane.io/class
• crossplane.io/provider

What do you guys think?

[hasheddan]

This looks pretty good so far! For the other two proposed tags:

1. Class tag: I am not sure how much longer are current class structure will be relevant so might hold off on that one.
2. Provider tag: Assuming this would be the name of the literal provider object? If so I think that could be useful!

[muvaf]

Class tag: I am not sure how much longer are current class structure will be relevant so might hold off on that one.

Well, be it class or some other thing, I think we'll have something that serves as resource class. So, it might be worth adding it now and change the string if the name changes.

Provider tag: Assuming this would be the name of the literal provider object? If so I think that could be useful!

Yes, it's literal object name.

[hasheddan]

That sounds fine. Another thing that makes class different is that statically provisioned managed resources do not have a class ref while the other 3 tags are values that are always present.

[muvaf]

Another thing that makes class different is that statically provisioned managed resources do not have a class ref while the other 3 tags are values that are always present.

Yeah, I imagine we'd just skip class label in that case.

I'll circle back to API patterns doc and add these if everyone is onboard. @negz what do you think?

[negz]

Thanks @muvaf!

[negz]

Can we rely on this field always being set? I recall it was being automatically cleared by some clients, apparently intentionally, which is why most of our crossplane-runtime functions that need a GVK require we pass either a GVK or a scheme to determine one. kubernetes-sigs/controller-runtime#528 indicates that the old behaviour I described may now be gone.

[muvaf]

I will test it and write here. I simply relied on the fact that all those functions returned non-pointer objects.

[muvaf]

Verified that it does return GVK correctly.

[negz]

Nice! This might allow us to simplify some of our functions in pkg/meta (and possibly pkg/resource?) that assume they can't rely on GetObjectKind to actually return the kind of an object.

[muvaf]

@hasheddan @negz Heads-up, I'm replacing / with - as it seems / in tag key is only accepted by AWS and hyphen is accepted by all three. See the restrictions for each:

• AWS https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html#tag-restrictions
• GCP https://cloud.google.com/compute/docs/labeling-resources#label_format
• Azure https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/tag-resources#limitations

[muvaf]

Alright, . is not supported by GCP. What do you think about crossplane-kind, crossplane-class etc ?

[hasheddan]

@muvaf those sound good to me!

[negz]

Looks like this needs to be rebased - I'll merge once the branches align.

[muvaf]

@negz ready to go! I added a small change to make kind lowercase(checked kinds are not case-sensitive) to comply with GCP more easily without breaking anything.