Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align on Commonalities' subscription-model by using sink and sinkCredentials #335

Merged
merged 10 commits into from
Aug 7, 2024

Conversation

maxl2287
Copy link
Collaborator

@maxl2287 maxl2287 commented Aug 5, 2024

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature

What this PR does / why we need it:

Aligns on Commonalities' updated subscription model.

Which issue(s) this PR fixes:

Fixes #332

@maxl2287
Copy link
Collaborator Author

maxl2287 commented Aug 5, 2024

@hdamker shell I also update the BadRequest with the error-examples?

    Generic400:
      description: Invalid input
      headers:
        x-correlator:
          $ref: "#/components/headers/x-correlator"
      content:
        application/json:
          schema:
            $ref: "#/components/schemas/ErrorInfo"
          examples:
            InvalidArgument:
              value:
                status: 400
                code: INVALID_ARGUMENT
                message: "Schema validation failed at  ..."
            InvalidCredential:
              value:
                status: 400
                code: INVALID_CREDENTIAL
                message: Only Access token is supported
            InvalidToken:
              value:
                status: 400
                code: INVALID_TOKEN
                message: Only bearer token is supported

Copy link
Collaborator

@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.

sinkCredentials are currently within the Design Guidelines quite restricted. Added here to the descriptions.

code/API_definitions/quality-on-demand.yaml Outdated Show resolved Hide resolved
code/API_definitions/quality-on-demand.yaml Outdated Show resolved Hide resolved
code/API_definitions/quality-on-demand.yaml Outdated Show resolved Hide resolved
@maxl2287 maxl2287 force-pushed the feature/align-subscription-model branch from d3cab0b to 4f80ca5 Compare August 5, 2024 11:28
maxl2287 and others added 3 commits August 5, 2024 13:32
Co-authored-by: Herbert Damker <herbert.damker@telekom.de>
Co-authored-by: Herbert Damker <herbert.damker@telekom.de>
Co-authored-by: Herbert Damker <herbert.damker@telekom.de>
Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

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

You have to correct as well: bearerFormat: "{$request.body#/webhook/notificationAuthToken}" -> bearerFormat: "{$request.body#/sinkCredential.credentialType}"

@jlurien
Copy link
Collaborator

jlurien commented Aug 5, 2024

sinkCredentials are currently within the Design Guidelines quite restricted. Added here to the descriptions.

In QoD Provisioning I have adjusted the previous text to:

    * **Notification URL and token**:
    Developers may provide a callback URL (`sink`) on which notifications about all status change events (eg. provisioning termination) can be received from the service provider. This is an optional parameter. The notification will be sent as a CloudEvent compliant message. If `sink` is included, it is RECOMMENDED for the client to provide as well the `sinkCredential` property to protect the notification enpoint. In the current version,`sinkCredential.credentialType` MUST be set to `ACCESSTOKEN` if provided.

cc @maxl2287

Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

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

I have added also 410 as response to
callbacks:
notifications:
"{$request.body#/sink}":

As in the artifact because it has an explict reference to notifications:

Generic410:
  description: Gone
  headers:
    x-correlator:
      $ref: "#/components/headers/x-correlator"
  content:
    application/json:
      schema:
        $ref: "#/components/schemas/ErrorInfo"
      examples:
        GENERIC_410_GONE:
          description: Use in notifications flow to allow API Consumer to indicate that its callback is no longer available
          value:
            status: 410
            code: GONE
            message: Access to the target resource is no longer available.

@hdamker
Copy link
Collaborator

hdamker commented Aug 5, 2024

@maxl2287 The additions of Generic410 and Generic422 got mixed up ... may I ask to resolve the conflict?

@hdamker
Copy link
Collaborator

hdamker commented Aug 5, 2024

@hdamker shell I also update the BadRequest with the error-examples?

@maxl2287 the current Generic400 "BadRequest" is a copy from line 131ff of Commonalities/blob/main/artifacts/CAMARA_common.yaml and fits for the other endpoints.

INVALID_CREDENTIAL and INVALID_TOKEN are specific for createSession only - so I suppose you have to introduce and additional CreateSessionBadRequest400 similar as in the event-subscription-template?

@jlurien also relevant for #299

@hdamker hdamker mentioned this pull request Aug 5, 2024
…e/align-subscription-model

# Conflicts:
#	code/API_definitions/quality-on-demand.yaml
@hdamker hdamker merged commit 452efd6 into camaraproject:main Aug 7, 2024
1 check passed
hdamker added a commit that referenced this pull request Aug 23, 2024
Release notes for r1.2 including:
Editorial changes or some entries from r1.1
Added PR #335 which was missed to be mentioned in r1.1
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.

Alignment of notification web hook to CloudEvent subscription model
3 participants