feat!: moves object ids to top level attributes

[singhmj-1]

Description

The application description and deployment ids were nested under metadata and annotations fields, which in themselves are sub-fields in the respective definitions of app and deployment. This change moves them to the root-level objects i.e. they are accessible directly as AppDescription.Id and AppDeployment.Id .

Issues Addressed

#161

Change Type

Please select the relevant options:

• Fix (change that resolves an issue)
• [] New enhancement (change that adds specification content)
• Content edits (change that edits existing content)

Checklist

• I have read the CONTRIBUTING document.
• My changes adhere to the established patterns, and best practices.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ajcraig / name: Armand Craig (1032799)
• ✅ login: singhmj-1 / name: Manjinder (3de18e1, 6f59d2c, 83fc2c6, 95acc5f)

[ajcraig]

Couple items need addressed:
Desired state templates

• the id in desired state is actually 2 items.
  • 1. UUID of the app deployment managed via the WFM
  • 2. applicationId
• Appears the rendered markdowns are showing id at root, but annotations/application id in the metadata
• id doesn't have a descriptor
• appears the top-level attributes table header has disappeared.
• Annotations attributes table is showing both applicationId and id within the table. These, I believe, should be in the top-level attributes.

@Silvanoc was the intent of your issue to move both uuid and application id in the deploymentprofiles to root?

Application Description
Check the attribute tables. It is currently showing two id elements. One in top-level attributes and one in metadata attributes.

• Top-level doesn't have a descirption / type / and states N on required which would be a change from previous.

Examples look good in this section.

[singhmj-1]

@ajcraig Thanks for the detailed review! Responses to your points below:

Couple items need addressed: Desired state templates

• the id in desired state is actually 2 items.
  • 1. UUID of the app deployment managed via the WFM
  • 2. applicationId

Is there any reason that we want these to be together? The applicationId is a reference to the original application description, and can be kept at any level inside the App Deployment (though it would be okay to keep at root level, something like):

kind: ApplicationDeployment
id: depl-1111-12hj-0000-0000
applicationId: app1-1111-1111-1111-1111
metadata: ....

Or another example:

kind: ApplicationDeployment
id: depl-1111-12hj-0000-0000
applicationRef: # renamed it intentionally to show it is a reference
  id: app1-1111-1111-1111-1111
metadata: ....

---

• Appears the rendered markdowns are showing id at root, but annotations/application id in the metadata

I thought only uuid needs to be moved to the top. Should I wait for everyone's consensus here?

• id doesn't have a descriptor
• appears the top-level attributes table header has disappeared.
• Annotations attributes table is showing both applicationId and id within the table. These, I believe, should be in the top-level attributes.

Will check this again. Thanks for pointing out!

---

Application Description Check the attribute tables. It is currently showing two id elements. One in top-level attributes and one in metadata attributes.

• Top-level doesn't have a description / type / and states N on required which would be a change from previous.

Actually, I just copy pasted the fields, but the type info was missing even in the nested field, but somehow they are being picked up as string but not the root-level. Will fix it.

Plus, I didn't remove the nested id element and instead marked it as deprecated. @phil-abb left a comment and guided to remove the deprecated fields completely. Hence, I'll fix this.

[ajcraig]

@singhmj-1

Is there any reason that we want these to be together? The applicationId is a reference to the original application description, and can be kept at any level inside the App Deployment (though it would be okay to keep at root level, something like):

I didn't see silvano moving this outside of the metadata:annotations. So I think for ApplicationDeployment yaml this can stay where it is.

[phil-abb]

Suggested change

	description: UUID of the ApplicationDeployment
	description: Unique identifier for the application deployment

[phil-abb]

Suggested change

	The unique UUID from the ApplicationDeployment.
	Unique identifier for the application deployment.

[ajcraig]

Pending small fixes from Phil, approved. Looks good!
Also fixed my commit that was hanging on the CLA error.

[matlec]

Hey @singhmj-1! Thanks for moving this forward!

Re-reading #163, I think the current PR implements only part of what we reached there. The thread concluded that the nested id in ApplicationDeployment only existed because of CRD-conformance, and the spec isn't CRD-conformant anyway. So the agreement was to drop the CRD shape entirely, not just promote id.

Given that, these are the missing pieces I see from #163:

• applicationId in ApplicationDeployment is still nested under metadata.annotations. Since it's a reference to an ApplicationDescription (rather than its own identity), I'd suggest moving it under spec alongside other properties that already describe "what to deploy".
• name in ApplicationDeployment is still retained. I argued in Make ApplicationDeployment ID a top-level attribute #163 to remove it and saw no push back.
• namespace has the same shape of problem.

The final shape I envision for ApplicationDeployment:

apiVersion: application.margo.org/v1alpha1
kind: ApplicationDeployment
id: a3e2f5dc-912e-494f-8395-52cf3769bc06
spec:
  applicationId: com-northstartida-digitron-orchestrator
  deploymentProfile: ...
  parameters: ...

A side note on name and namespace: while looking into these, I realized our WFM Client implementation currently misinterprets metadata.name as the Helm release name and metadata.namespace as the Helm install target (helm install --namespace). That's on us to fix, however, the deeper issue is that the spec doesn't currently define fields for "Helm release name" or "Helm install target namespace" at all, which I'll open a separate issue for.

Edit: tracked in #183

[singhmj-1]

@matlec -- I agree with you and I actually raised it over here as well . If we see a consensus on #183 then I'll make those changes as well.