Make `ApplicationDeployment` ID a top-level attribute

[Silvanoc]

Description

Currently the ID of an ApplicationDeployment is "hidden" as the attribute id of the 3rd-level object Annotations.

Additionally "annotations" are typically data without native structure and semantics. They are mostly used as extension points. But the ID of an ApplicationDeployment has very clear semantics (it's an identifier!) and structure (it's a UUID!).

Motivation

An object identifier should have visibility at the highest object level.

Proposed Solution (optional)

Promote the Annotations attribute id to become the ApplicationDeployment attribute id.

[Silvanoc]

@matlec any opinion on this?

[phil-abb]

@Silvanoc / @matlec / @ajcraig - the ApplicationDeployment was originally defined to be a valid CRD out of the box. The only fields allowed at the top level for a CRD are:

• apiVersion (required)
• kind (required)
• metadata (required)
• spec (optional)
• status (optional)

So, if the decision is made to change this, then the ApplicationDeployment would no longer be a valid CRD. This may be fine; it's just something that needs to be considered if this change is made.

[Silvanoc]

@Silvanoc / @matlec / @ajcraig - the ApplicationDeployment was originally defined to be a valid CRD out of the box. The only fields allowed at the top level for a CRD are:

* apiVersion (required)

* kind (required)

* metadata (required)

* spec (optional)

* status (optional)

So, if the decision is made to change this, then the ApplicationDeployment would no longer be a valid CRD. This may be fine; it's just something that needs to be considered if this change is made.

Now I know at least the reason for that IMO convoluted data structure 😄

I personally don't see the need to stick to K8s CRDs, but I might be missing some good reason to be CRD-conform.

If the decision is to remain CRD-conform, then we should:

1. Be consistently conform on the different resources. Probably something like "all resources, except manifests (class names ...Manifest), are to be modeled K8s CRD-conform".
2. Document it very prominently. I haven't found any reference to this fact in the whole specification.

Remaining CRD-conform would be IMO the only reason for not having the ID at top-level. So if we stick to CRD-conformance, then this issue, #162 and #161 can be closed.

[matlec]

@phil-abb thanks for pointing this out. In general, I share @Silvanoc's sentiment that id does not belong to annotations. Do you remember the rationale for having this particular schema bound to the Kubernetes API conventions?

[phil-abb]

@matlec / @Silvanoc - The only point I recall is that, as a CRD, it can be used as is on Kubernetes without any changes or conversions if someone were to implement this using an operator pattern. I don't recall if there are any strong opinions either way.

I think it's convenient, but I don't have a strong opinion.

[matlec]

@phil-abb / @Silvanoc assuming that the original intent was to support implementing a WFM client as an operator, I'm still having trouble to understand the shape of ApplicationDeployment:

1. In Kubernetes, metadata.name would be the unique (namespace-scoped) identifier of the CR. Today, the spec does not list this as a requirement for the WFM. Also, metadata.annotations.id then seems to be redundant.
2. If we actually wanted to support the operator pattern, we'd probably need to define .status as well (and embed the contents of the DeploymentStatusManifest). Otherwise, how would an operator that acts upon the contents of .spec notify its status to the WFM? Today, it would need to post this information to the /api/v1/clients/{clientId}/deployments/{deploymentId}/status endpoint, essentially negating the benefit of using a CRD in the first place.

That said, I don't have a strong opinion on this topic either.

[Silvanoc]

That said, I don't have a strong opinion on this topic either.

I do have a strong opinion on this topic 😄

• Either we are fully CRD-conform and we need to consider @matlec comments.
• Or we aren't and then we can simplify the data model.

[ajcraig]

Don't remember having a strong integration reason into K8s with this decision. Probably something legacy that no one has done a deep dive, so APPRECIATE IT SILVANO!

I guess we are speaking on proposals at this stage, assuming a SUP or PR to spec will follow.

At first I thought maybe metadata.uid would be a good fit, but further research describes that as an id that kubernetes auto generates.
This then makes me question the metadata.name attribute. our description for this attribute is as follows:

• "when deploying to kubernetes, the manifests name. The name is chosen by the workload orch vendor and is not displayed anywhere".
• K8s description: "A client-provided string that refers to an object in a URL, such as /api/v1/pods/some-name.

Conclusion: Deployment specifications are a layer above a single deployment environment i.e.: Kubernetes, so we shouldn't follow their standard if we aren't applying it correctly.

So! I want to expand this issue to elevate as Silvano said id, but also name, as ApplicationDeployment attributes.

• new description for name:
  • Human readable user assigned name, within the Workload Fleet Manager associated with this particular deployment.
  • Type: String.

[phil-abb]

@Silvanoc / @matlec
I don't see any reason not to propose a simplification for this. With all the other changes made to how the desired state works, making something generic makes more sense, probably.

[ajcraig]

That said, I don't have a strong opinion on this topic either.

I do have a strong opinion on this topic 😄

• Either we are fully CRD-conform and we need to consider @matlec comments.
• Or we aren't and then we can simplify the data model.

I vote simplify the data model knowing the ApplicationDeployments are utilized for more than Kubernetes deployments.