Move embedded API types to common/v1

[hasheddan]

Description of your changes

Moves the core embedded API types out of the core/v1alpha1 directory and
into common/v1. These types are used by many mature APIs and are now
recognized as stable. This new package should be imported with alias
xpv1 by convention.

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

make

[negz]

Thanks @hasheddan!

Do you think it's worth keeping the v1alpha1 types around (i.e. duplicating rather than moving them) and marking them deprecated? Doing so might ease the transition for community providers a little.

[negz]

Also - a naming nit - should we consider apis/runtime rather than apis/core? Does upstream Kubernetes have some convention we could follow?

[hasheddan]

@negz feels like the closest thing in upstream kubernetes would be meta/v1 so not sure how that translates here 🤔 We could just call them meta. If we keep around this v1alpha1 types aren't we going to need to support them across the rest of the code base as well? I agree that it would be nice if we could follow a deprecation schedule, but I'm not sure it is worth that burden in this case.

[negz]

@negz feels like the closest thing in upstream kubernetes would be meta/v1 so not sure how that translates here 🤔 We could just call them meta.

That does seem close (i.e. its a versioned package of types that aren't in and of themselves first class API objects), but in this case our types are fairly all over the show - not really strictly related to metadata. That said, the precedent of the meta/v1 package does now have me wondering whether it would be wise for us to keep versioning these after all - i.e. core/v1?

I'm tempted to bikeshed the package name a little bit here. We tend to import it as runtimev1alpha1 when there are import name conflicts. If we go with core/v1 there would likely be conflicts with the upstream core/v1 package which is usually imported as corev1. I wonder if just apis/crossplane/v1 would make sense? We could import it as xpv1 (Or just xp if we decide to drop the version after all).

If we keep around this v1alpha1 types aren't we going to need to support them across the rest of the code base as well?

I may not be following correctly, but if you're suggesting that keeping the v1alpha1 types here means we'd have to do the same elsewhere I don't think that's the case. That said I'm not bullish on duplicating rather than copying. Folks would still have to update their code eventually - the former option just gives them a little extra notice.

[hasheddan]

That said, the precedent of the meta/v1 package does now have me wondering whether it would be wise for us to keep versioning these after all.

I don't necessarily think the fact that they are versioned upstream means that they have to be versioned here. Our use case is a bit different in that the consumers of our embedded types are also typically consuming other packages in crossplane-runtime that use them, so being pinned to a crossplane-runtime version is mostly enough to account for versioning compatibility. However, I have secret plans to make some of the Crisscross stuff part of core Crossplane eventually, in which case the reconciler and the types would be separated by network calls, and at that point it would be much more similar to how meta/v1 is used. I think apis/crossplane/v1 sounds good to me.

I may not be following correctly, but if you're suggesting that keeping the v1alpha1 types here means we'd have to do the same elsewhere I don't think that's the case.

I am not sure how useful it would be to just keep the v1alpha1 types without being able to use a lot of the packages in crossplane-runtime that work with them, such as setting conditions. It seems like folks likely would not be able to update because of the changes in those package, making keeping the v1alpha1 types around potentially more confusing than just removing them.

[muvaf]

I'm tempted to bikeshed the package name a little bit here. We tend to import it as runtimev1alpha1 when there are import name conflicts. If we go with core/v1 there would likely be conflicts with the upstream core/v1 package which is usually imported as corev1. I wonder if just apis/crossplane/v1 would make sense? We could import it as xpv1 (Or just xp if we decide to drop the version after all).

I'm not sure whether crossplane-runtime should be concerned about that conflict. It feels like it's fair to assume that consumers know that this is a crossplane import and they need to take care of conflict, if any, just like other packages. Also, apis/crossplane/v1 sounds like stuttering to me that we'd have import github.com/crosspalne/crossplane-runtime/apis/crossplane/v1 where it's used. So, I'm leaning towards common/v1(slight preference) or core/v1.

Regarding versioning, I think keeping the version would help us to use the same names for the structs when we make a change since the new one and deprecated one wouldn't have to live in the same package. I feel kind of indifferent about whether that's worth to keep the version. I'm OK either way with slight preference of keeping the versions just because kubernetes does.

[hasheddan]

@negz any follow-up thoughts here? I am good with either common/v1 or core/v1 also.

[negz]

I agree that crossplane/v1 stutters, but generic names like core, common and util are considered to be antipatterns in Go so it feels like we'd be doing something undiomatic either way; a stuttering package name or a meaningless package name.

To me core/v1 seems like the worst name of all because it's both meaningless and requires us to figure out a way to differentiate it from the existing import corev1 k8s.io/api/core/v1 convention. I suspect we'd end up importing the package as runtimev1 (like we do today with runtimev1alpha1). If we're going to do that, it should be named apis/runtime/v1.

common/v1 is slightly better - I could read it as "common Crossplane types" which is pretty much what the package is. It's almost specifically types that are used to write a Crossplane provider, with the exception of some reference types and conditions that are used by c/c types too.

I continue to mostly prefer crossplane/v1 because I feel like it's more immediately obvious to a casual reader that xpv1.TypedReference (for example) is a common Crossplane subtype, whereas commonv1.TypedReference would require me to go check my imports to see whether apis/common was a Crossplane thing, a Kubernetes thing, or some other thing.

Edit: In retrospect it could be fine to go with a mixed approach in which the package is crossplane-runtime/apis/core/v1 and we just import it based on the module name rather than the (single type) package name - i.e. import xpv1 github.com/crossplane-runtime/apis/core/v1.

[hasheddan]

@muvaf @negz I update to use common/v1 here as seemed you both prefer common to core and I also changed all imports to alias xpv1. PTAL

[negz]

LGTM - thanks for updating this early @hasheddan.