Skip to content

Conversation

@caubut-charter
Copy link
Contributor

What type of PR is this?

  • subproject management

What this PR does / why we need it:

Release.

Which issue(s) this PR fixes:

Fixes #72 , #34

Does this PR introduce a breaking change?

  • Yes
  • No

Special notes for reviewers:

Changelog input

Additional documentation

@github-actions
Copy link

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.03s
✅ OPENAPI spectral 2 0 5.01s
✅ REPOSITORY git_diff yes no 0.0s
✅ REPOSITORY secretlint yes no 0.76s
✅ YAML yamllint 5 0 1.0s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@hdamker
Copy link
Contributor

hdamker commented Jul 15, 2025

@caubut-charter Thanks for opening this PR for review.

For a release-candidate version within the Fall25 meta-release cycle it is quite late and not yet complete. There are no test definitions for the API(s) and they are mandatory for a release-candidate. There are also various issues which would need to be resolved.

Alternative could be to declare the version within the release as 0.1.0-alpha.1 (as currently in the main branch) and continue the release-cycle outside of the meta-release.

Some formal points:

  • the initial release will be r1.1 (as in the PR title) ... the CHANGELOG has currently a section "1.0"
  • the release PR should set the version, the files in main should have wip as version until the release (we want to avoid that someone is taking the current YAML file from main and assumes that this is the released version)
  • there are multiple YAML files in /code/API_Definitions, their purpose is unclear
    • redocly.yaml has to be removed, it is not an API definitons
    • capabilities.yaml is incomplete, has wrong version (0.1.0 without extension), wrong API name in server URL (same as the main API), and it is not mentioned within the CHANGELOG.md
    • purpose of YAML files in "Domain" is unclear - please be aware that always the complete repository will be released
  • CHANGELOG.md and Readiness Checklist are referring still to Commonalities 0.5 (r2.3), while the API is referring to 0.6 (currently r3.2)

@hdamker
Copy link
Contributor

hdamker commented Jul 15, 2025

Beyond the formal points above I have run two reviews with help of Claude instead of a manual review: technically for compliance with the Design Guidelines of CAMARA and a design review against the recommendation within the CAMARA Design Guide.

The issues found in the first (technical) are quite easy to address (and should be done even for an alpha version release):

  • remove forbidden contact field
  • add missing externalDocs
  • correct server URL format
  • correct commonalities version format
  • correct service site path parameter reference

Further issues which are overlapping with the design review:

  • overly complex scope patterns
  • extensive custom error codes
  • discriminator mapping format
  • BaseDevice schema duplication

Please consider for the custom error codes:

A {{SPECIFIC_CODE}}, unless it may have traversal scope (i.e. re-usable among different APIs), SHALL follow this scheme for a specific API: {{API_NAME}}.{{SPECIFIC_CODE}}

Here is the full report: nam-camara-compliance-review.md -- please consider that not all points might be relevant for your API (e.g. pagination if there only a few instances expected).

One additional question from my side: it is intentional that only the endpoints for /isolated-networks are mandatory to be implemented while all other features (service sites, devices, reboot requests) are optional extensions (as they have 501 as possible response)?

Copy link
Contributor

@hdamker hdamker left a comment

Choose a reason for hiding this comment

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

I'm just seen that at least one of the comments which I wrote got lost (maybe I haven't submitted it before closing the browser).

To be very clear: the current state of the API is from perspective of Release Management NOT ready for pre-release with a release-candidate version. The issues raised within #73 (comment) must be resolved before (including at least basic test definitions), also the first set of issues raised in #73 (comment)

The alternative is to pre-release an alpha version (without test definitions). But also for that several of the points should be addressed before release creation.

@hdamker
Copy link
Contributor

hdamker commented Jul 16, 2025

And here is - just for your later consideration - the result of the automated design review (I removed some points which I didn't consider as relevant): nam-camara-design-review.md

The summary:

While the API demonstrates good documentation and security awareness, it deviates significantly from CAMARA design patterns in several areas:

  1. Critical Design Issues:

    • Misuse of PATCH for relationship management
    • Overly complex scope hierarchy
    • Conditional response bodies
  2. Recommended Actions:

    • Redesign bulk operations to use standard REST patterns
    • Simplify scope model
    • Clarify resource ownership and relationships
    • Remove scope-dependent response variations
  3. Positive Aspects to Maintain:

    • Comprehensive documentation
    • Rich examples
    • Strong security model (though simplification needed)

The API would benefit from significant design refactoring to align with CAMARA patterns before moving beyond alpha status.

@tanjadegroot
Copy link

Given the above review comments, Release Management recommends doing an alpha release outside the Fall25 meta-release, and prepare the public release for Spring26.

this means

  • updating the API release tracker with the label camara-spring26
  • create technical PRs to address the comments (and you can re-use or close this PR)
  • you can request an alpha release review if you want once ready.

@RandyLevensalor
Copy link
Contributor

@hdamker @tanjadegroot thanks for the review and feedback. We have our next meeting on Friday and will review the feedback and respond with a plan to address the feedback after we discuss there.

@hdamker
Copy link
Contributor

hdamker commented Jan 18, 2026

@camaraproject/network-access-management_codeowners I suggest that this PR is outdated and should be closed

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.

Prepare r1.1

6 participants