Skip to content

Initialize EAM /appinstances callback (2)#407

Open
clundie-CL wants to merge 13 commits intocamaraproject:mainfrom
cablelabs:EAM-Callback
Open

Initialize EAM /appinstances callback (2)#407
clundie-CL wants to merge 13 commits intocamaraproject:mainfrom
cablelabs:EAM-Callback

Conversation

@clundie-CL
Copy link

What type of PR is this?

enhancement/feature

What this PR does / why we need it:

This PR continues work from #373 (originally opened by @travishepworth ) to add callback support for POST /appinstances. Travis's PR was opened from a fork on his personal GitHub account and since I don't have permissions to push commits to this repo, I have elected to recreate his branch on the CableLabs' fork and submit this companion PR.

This update addresses reviewer feedback (#373 (comment)) by expanding the terminationReason enum to include comprehensive termination scenarios that prevent infinite retry loops and provide developers with actionable diagnostic information.

Key additions:

  • Adds CAMARA-standard termination reasons: SUBSCRIPTION_DELETED, NETWORK_TERMINATED
  • Adds application-specific failure reasons: DELIVERY_FAILURE (for repeated sink endpoint failures), RESOURCE_DELETED (for deleted app instances/deployments)
  • Adds inline documentation describing each termination reason

This aligns with CAMARA CloudEvents standards and webhook API best practices, ensuring developers receive clear signals about why subscriptions terminated and whether they should retry.

Which issue(s) this PR fixes:

Fixes #370

Special notes for reviewers:

This PR addresses the review feedback from #373 regarding the need for expanded termination reasons with diagnostic information. The termination reasons now follow the CAMARA CloudEvents subscription template standard and include reasons that prevent clients from retrying operations that will always fail due to application-level issues.

The implementation remains backward-compatible. No breaking changes to existing callback behavior.

Changelog input

Add callback response support to the EAM POST /appinstances endpoint with comprehensive subscription termination reasons following CAMARA standards

#### Additional documentation

Updated terminationReason enum in CloudEvent schema with seven comprehensive termination scenarios including SUBSCRIPTION_EXPIRED, MAX_EVENTS_REACHED, ACCESS_TOKEN_EXPIRED, SUBSCRIPTION_DELETED, NETWORK_TERMINATED, DELIVERY_FAILURE, and RESOURCE_DELETED. Each reason includes inline documentation for developer clarity.

@clundie-CL clundie-CL changed the title Eam callback Initialize EAM /appinstances callback (2) Feb 2, 2026
@JoseMConde
Copy link
Collaborator

Hi @hdamker , do you know why the megaLinter stopped after 15 minutes??
Thanks!!

@hdamker
Copy link
Collaborator

hdamker commented Feb 3, 2026

Hi @hdamker , do you know why the megaLinter stopped after 15 minutes??
Thanks!!

The question is rather why it ran 15 minutes, that's much too long ... that can be a timeout.
Yesterday evening had GitHub a major outage of their runners ... have you checked the GitHub status?

Just restarted the job and it failed after a minute ;-)

@clundie-CL
Copy link
Author

@hdamker Thanks for taking a look and providing some insights.

@JoseMConde and @gunjald PR should be ready now that the linter error has been addressed.

the server will return a callback response at the specified URL.

**WARNING**: This callback endpoint must be exposed on the listener side as `POST {$request.body#/subscriptionRequest/sink}`
operationId: EdgeApplicationManagementDeploymentCallback
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think as an enhancement of the documentation we need to slightly update the description which is not linked with this issue itself. The terms like application deployment, application onboarding and application instantiation seems to have some overlaps which can be improved. So we may have another issue created to take the discussion outside of this issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let you and codeowners decide if there's a TODO here that is blocking approval of this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you @gunjald , we can create a new Issue trying to improve the documentation, could you please create for me??
In my view @clundie-CL , I think this shouldn't block the approval for this PR, so we can manage in a different issue and don't block this PR.

format: date-time
example: 2023-01-17T13:18:23.682Z
description: The subscription expiration time (in date-time format) requested by the API consumer. It must follow [RFC 3339](https://datatracker.ietf.org/doc/html/rfc3339#section-5.6) and must have time zone.
subscriptionMaxEvents:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we allow for some max limit that the platform can impose? Otherwise I may put an arbitrary big number which could mean unlimited events?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I am the one to answer this question — feels like codeowners should weigh in or please let me know if there's a change you want here. My read on this is that if you put an arbitrary big number with the intent of achieving unlimited events, perhaps the better path is just omitting this field as it does not appear to be required. To me, omitting subscriptionMaxEvents implies API Consumer is ok with any number of events. However, maybe this behavior should be documented.

In terms of what an API Provider might want to impose might be platform specific and API Consumers would want to discover that ahead of time or at runtime. There's an emerging CAMARA API that covers this use case that may be of note: https://camaraproject.org/capabilities-and-runtime-restrictions/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gunjald, @clundie-CL,

According to the latest commonalities, for integers, the minimum and maximum ranges, as well as the format, must be specified. This means that this must be specified not only for subscriptionMaxEvents but for all other integers as well. https://github.com/camaraproject/Commonalities/blob/main/documentation/CAMARA-API-Design-Guide.md#22-data-definitions

My suggestion is that we define the maximum for the subscriptionMaxEvents field and for all other fields in a separate issue. I will open it to start the discussion on this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DLondonoD Thanks for the Commonalities reference. That makes it clear.

@JoseMConde
Copy link
Collaborator

Hi @clundie-CL , could you please check the last Deepak's comments??

@clundie-CL
Copy link
Author

@JoseMConde

Hi @clundie-CL , could you please check the last Deepak's comments??

I have responded, but overall I would note that due to fact I am picking up this PR from Travis, I might not be the best person to discuss what should or shouldn't be changed for this PR. I see my role here as helping usher any explicit modifications the codeowners would require in order to complete this PR. I'd have to defer to codeowner direction for what needs changed to gather approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EAM API: Support Callbacks for POST /appinstance

6 participants