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

Review of reported errors from local run CAMARA linter to confirm conformance with API Design Guidelines for Traffic_Influence.yaml #191

Closed
sergiofranciscoortiz opened this issue Feb 6, 2024 · 14 comments · Fixed by #231

Comments

@sergiofranciscoortiz
Copy link
Collaborator

After running locally CAMARA linter scripts indicated in #170, although using draft spectral , along with files in lint_function_folder, following errors/warnings appear:

   5:6   warning  info-contact                          Info object must have "contact" object.                                                                                                          info
  124:7    error  oas3-valid-schema-example             "example" property must match format "ipv4"                                                                                                      paths
125:23    error  camara-parameter-casing-convention    /traffic-influences should be kebab-case: must be kebab case                                                                                     paths./traffic-influences
134:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences.get.tags[0]
137:20     hint  camara-operationid-casing-convention  Operation Id must be in Camel case "must be camel case"                                                                                          paths./traffic-influences.get.operationId
160:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences.post.tags[0]
164:20     hint  camara-operationid-casing-convention  Operation Id must be in Camel case "must be camel case"                                                                                          paths./traffic-influences.post.operationId
201:44    error  camara-parameter-casing-convention    /traffic-influences/{trafficInfluenceID} should be kebab-case: must be kebab case                                                                paths./traffic-influences/{trafficInfluenceID}
202:16    error  camara-http-methods                   Invalid HTTP method for '#/paths/~1traffic-influences~1{trafficInfluenceID}/parameters'. Must be one of get, put, post, delete, patch, options.  paths./traffic-influences/{trafficInfluenceID}.parameters
  210:9  warning  operation-description                 Operation "description" must be present and non-empty string.                                                                                    paths./traffic-influences/{trafficInfluenceID}.get
  210:9  warning  operation-operationId                 Operation must have "operationId".                                                                                                               paths./traffic-influences/{trafficInfluenceID}.get
213:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences/{trafficInfluenceID}.get.tags[0]
235:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences/{trafficInfluenceID}.patch.tags[0]
238:20     hint  camara-operationid-casing-convention  Operation Id must be in Camel case "must be camel case"                                                                                          paths./traffic-influences/{trafficInfluenceID}.patch.operationId
276:12  warning  operation-description                 Operation "description" must be present and non-empty string.                                                                                    paths./traffic-influences/{trafficInfluenceID}.delete
276:12  warning  operation-operationId                 Operation must have "operationId".                                                                                                               paths./traffic-influences/{trafficInfluenceID}.delete
279:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences/{trafficInfluenceID}.delete.tags[0]
324:31  warning  camara-parameters-descriptions        Parameter description is missing or empty: "onTrafficInfluenceChanged.description" property must be truthy                                       components.callbacks.onTrafficInfluenceChanged
324:31    error  oas3-valid-schema-example             "example" property must match format "ipv4"                                                                                                      components.callbacks.onTrafficInfluenceChanged
350:22  warning  camara-parameters-descriptions        Parameter description is missing or empty: "TrafficInfluence.description" property must be truthy                                                components.schemas.TrafficInfluence
350:22  warning  camara-parameters-descriptions        Parameter description is missing or empty: "TrafficInfluence" property must be truthy                                                            components.schemas.TrafficInfluence
402:27  warning  camara-parameters-descriptions        Parameter description is missing or empty: "PatchTrafficInfluence.description" property must be truthy                                           components.schemas.PatchTrafficInfluence
406:28  warning  camara-parameters-descriptions        Parameter description is missing or empty: "trafficInfluenceID.description" property must be truthy                                              components.schemas.PatchTrafficInfluence.properties.trafficInfluenceID
408:23  warning  camara-parameters-descriptions        Parameter description is missing or empty: "apiConsumerId.description" property must be truthy                                                   components.schemas.PatchTrafficInfluence.properties.apiConsumerId
410:23  warning  camara-parameters-descriptions        Parameter description is missing or empty: "applicationId.description" property must be truthy                                                   components.schemas.PatchTrafficInfluence.properties.applicationId
412:15  warning  camara-parameters-descriptions        Parameter description is missing or empty: "state.description" property must be truthy                                                           components.schemas.PatchTrafficInfluence.properties.state
415:26  warning  camara-parameters-descriptions        Parameter description is missing or empty: "PostTrafficInfluence.description" property must be truthy                                            components.schemas.PostTrafficInfluence
419:28  warning  camara-parameters-descriptions        Parameter description is missing or empty: "trafficInfluenceID.description" property must be truthy                                              components.schemas.PostTrafficInfluence.properties.trafficInfluenceID
421:15  warning  camara-parameters-descriptions        Parameter description is missing or empty: "state.description" property must be truthy                                                           components.schemas.PostTrafficInfluence.properties.state
424:34  warning  camara-parameters-descriptions        Parameter description is missing or empty: "TrafficInfluenceNotification.description" property must be truthy                                    components.schemas.TrafficInfluenceNotification
462:29  warning  camara-parameters-descriptions        Parameter description is missing or empty: "NetworkAccessIdentifier" property must be truthy                                                     components.schemas.NetworkAccessIdentifier
462:29  warning  camara-parameters-descriptions        Parameter description is missing or empty: "NetworkAccessIdentifier.description" property must be truthy                                         components.schemas.NetworkAccessIdentifier
476:16    error  oas3-valid-schema-example             "example" property must match format "ipv4"                                                                                                      components.schemas.Ipv4Address.example
509:17  warning  camara-parameters-descriptions        Parameter description is missing or empty: "ErrResponse.description" property must be truthy                                                     components.schemas.ErrResponse
512:16  warning  camara-parameters-descriptions        Parameter description is missing or empty: "status.description" property must be truthy                                                          components.schemas.ErrResponse.properties.status
515:17  warning  camara-parameters-descriptions        Parameter description is missing or empty: "message.description" property must be truthy                                                         components.schemas.ErrResponse.properties.message
518:15  warning  camara-parameters-descriptions        Parameter description is missing or empty: "ErrorInfo.description" property must be truthy                                                       components.schemas.ErrorInfo
592:24  warning  oas3-unused-component                 Potentially unused component has been detected.                                                                                                  components.responses.SessionNotFound404

To be double checked if correction is needed in yaml to avoid those errors/warnings ( at least the kebab case one seems a false positive). Most of them are warnings for missing parameter descriptions.

@FabrizioMoggio
Copy link
Collaborator

I'm not sure on how to retrieve the error on the YAML.

First, which version of the TI API YAML was used?

Then, let's use this as an example:

"124:7 error oas3-valid-schema-example "example" property must match format "ipv4" "

What does 124:7 means? I supposed that 124 was the line in the YAML, but I couldn't find a match.

@FabrizioMoggio
Copy link
Collaborator

About: "125:23"

URLs are already in Kebab-case

/traffic-influences

@sergiofranciscoortiz
Copy link
Collaborator Author

Hi Fabrizio, apparently draft version had a bug checking Kebab-case in path. Links for linter files from commonalities (referenced in in #170) are working now (updated in main branch). We have rerun it with current Traffic_Influence.yaml ( the one in progress in #167

And current output is the following:

   5:6   warning  info-contact                          Info object must have "contact" object.                                                                                                          info
  123:7    error  oas3-valid-schema-example             "example" property must match format "ipv4"                                                                                                      paths
133:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences.get.tags[0]
136:20     hint  camara-operationid-casing-convention  Operation Id must be in Camel case "must be camel case"                                                                                          paths./traffic-influences.get.operationId
159:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences.post.tags[0]
162:20     hint  camara-operationid-casing-convention  Operation Id must be in Camel case "must be camel case"                                                                                          paths./traffic-influences.post.operationId
200:16    error  camara-http-methods                   Invalid HTTP method for '#/paths/~1traffic-influences~1{trafficInfluenceID}/parameters'. Must be one of get, put, post, delete, patch, options.  paths./traffic-influences/{trafficInfluenceID}.parameters
  208:9  warning  camara-routes-description             Functionality method description Warning: Each method should have description.                                                                   paths./traffic-influences/{trafficInfluenceID}.get
  208:9  warning  operation-description                 Operation "description" must be present and non-empty string.                                                                                    paths./traffic-influences/{trafficInfluenceID}.get
  208:9  warning  operation-operationId                 Operation must have "operationId".                                                                                                               paths./traffic-influences/{trafficInfluenceID}.get
211:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences/{trafficInfluenceID}.get.tags[0]
233:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences/{trafficInfluenceID}.patch.tags[0]
236:20     hint  camara-operationid-casing-convention  Operation Id must be in Camel case "must be camel case"                                                                                          paths./traffic-influences/{trafficInfluenceID}.patch.operationId
274:12  warning  camara-routes-description             Functionality method description Warning: Each method should have description.                                                                   paths./traffic-influences/{trafficInfluenceID}.delete
274:12  warning  operation-description                 Operation "description" must be present and non-empty string.                                                                                    paths./traffic-influences/{trafficInfluenceID}.delete
274:12  warning  operation-operationId                 Operation must have "operationId".                                                                                                               paths./traffic-influences/{trafficInfluenceID}.delete
277:11  warning  operation-tag-defined                 Operation tags must be defined in global tags.                                                                                                   paths./traffic-influences/{trafficInfluenceID}.delete.tags[0]
322:31  warning  camara-properties-descriptions        Property description is missing or empty: "onTrafficInfluenceChanged.description" property must be truthy                                        components.callbacks.onTrafficInfluenceChanged
322:31    error  oas3-valid-schema-example             "example" property must match format "ipv4"                                                                                                      components.callbacks.onTrafficInfluenceChanged
348:22  warning  camara-properties-descriptions        Property description is missing or empty: "TrafficInfluence.description" property must be truthy                                                 components.schemas.TrafficInfluence
348:22  warning  camara-properties-descriptions        Property description is missing or empty: "TrafficInfluence" property must be truthy                                                             components.schemas.TrafficInfluence
398:27  warning  camara-properties-descriptions        Property description is missing or empty: "PatchTrafficInfluence.description" property must be truthy                                            components.schemas.PatchTrafficInfluence
402:28  warning  camara-properties-descriptions        Property description is missing or empty: "trafficInfluenceID.description" property must be truthy                                               components.schemas.PatchTrafficInfluence.properties.trafficInfluenceID
404:23  warning  camara-properties-descriptions        Property description is missing or empty: "apiConsumerId.description" property must be truthy                                                    components.schemas.PatchTrafficInfluence.properties.apiConsumerId
406:15  warning  camara-properties-descriptions        Property description is missing or empty: "appId.description" property must be truthy                                                            components.schemas.PatchTrafficInfluence.properties.appId
408:15  warning  camara-properties-descriptions        Property description is missing or empty: "state.description" property must be truthy                                                            components.schemas.PatchTrafficInfluence.properties.state
411:26  warning  camara-properties-descriptions        Property description is missing or empty: "PostTrafficInfluence.description" property must be truthy                                             components.schemas.PostTrafficInfluence
415:28  warning  camara-properties-descriptions        Property description is missing or empty: "trafficInfluenceID.description" property must be truthy                                               components.schemas.PostTrafficInfluence.properties.trafficInfluenceID
417:15  warning  camara-properties-descriptions        Property description is missing or empty: "state.description" property must be truthy                                                            components.schemas.PostTrafficInfluence.properties.state
420:34  warning  camara-properties-descriptions        Property description is missing or empty: "TrafficInfluenceNotification.description" property must be truthy                                     components.schemas.TrafficInfluenceNotification
458:29  warning  camara-properties-descriptions        Property description is missing or empty: "NetworkAccessIdentifier" property must be truthy                                                      components.schemas.NetworkAccessIdentifier
458:29  warning  camara-properties-descriptions        Property description is missing or empty: "NetworkAccessIdentifier.description" property must be truthy                                          components.schemas.NetworkAccessIdentifier
472:16    error  oas3-valid-schema-example             "example" property must match format "ipv4"                                                                                                      components.schemas.Ipv4Address.example
510:17  warning  camara-properties-descriptions        Property description is missing or empty: "ErrResponse.description" property must be truthy                                                      components.schemas.ErrResponse
513:16  warning  camara-properties-descriptions        Property description is missing or empty: "status.description" property must be truthy                                                           components.schemas.ErrResponse.properties.status
516:17  warning  camara-properties-descriptions        Property description is missing or empty: "message.description" property must be truthy                                                          components.schemas.ErrResponse.properties.message
519:15  warning  camara-properties-descriptions        Property description is missing or empty: "ErrorInfo.description" property must be truthy                                                        components.schemas.ErrorInfo
593:24  warning  oas3-unused-component                 Potentially unused component has been detected.                                                                                                  components.responses.SessionNotFound404

@sergiofranciscoortiz
Copy link
Collaborator Author

Just 3 errors:

  • It seems to be needed to put the get method inmediately after the path definition ( in this case appears parameters: first)
200:16    error  camara-http-methods                   Invalid HTTP method for '#/paths/~1traffic-influences~1{trafficInfluenceID}/parameters'. Must be one of get, put, post, delete, patch, options.  paths./traffic-influences/{trafficInfluenceID}.parameters
  /traffic-influences/{trafficInfluenceID}:
    parameters:
      - name: trafficInfluenceID
        in: path
        description: Identifier of the specific TrafficInfluence resource to be retrieved, modified or deleted. It is the value used to fill trafficInfluenceID parameter
        required: true
        schema:
          type: string
   
    get:
  • Example Ipv4. It seems that it is complaining of the example example: "192.168.0.1/24" it should be enough to remove /24 to comply
    I don´t know why it appears two references, but it seem to be corrected just with that change. We noticed that we are using different namig for schemas in EdgeCloud_LCM for ipv4 (/components/schemas/Ipv4Addr) while in TI it is used ( /components/schemas/Ipv4Address) but that is something we could review in the future.
322:31    error  oas3-valid-schema-example             "example" property must match format "ipv4"
472:16    error  oas3-valid-schema-example             "example" property must match format "ipv4"                                                                                                      components.schemas.Ipv4Address.example

The rest are just warnings or hints.

BTW: I agree that some times is confusing/not accurate the line:column number on the output.

@FabrizioMoggio
Copy link
Collaborator

FabrizioMoggio commented Feb 21, 2024

About error 200:16. According to my understanding, the tool doesn't recognize some methods as GET, PATCH and DELETE. Even if they actually are GET, PATCH and DELETE. I suppose the reason is that we defined "parameters" as child of "paths" rather than child of the "methods". Is this a prescription from the CAMARA Guidelines? Because our implementation is correct according to the OpenAPI Specifications (https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#path-item-object). We have used the "parameters" description as child of "path". With our solution we avoid duplicating the parameter definition for the different methods (Get, Patch and Delete). With the suggested approach we should define the same parameter three times in the code with the risk of code misalignment.

@FabrizioMoggio
Copy link
Collaborator

many warning are not correct. For example 408:15.

The warning states that we have not described the property "state" in "PatchTrafficInfluence". Actually, we used the keyword "allOff" importing all the properties from "/components/schemas/TrafficInfluence" where the property "state" is described. Actually, if you see the code in swagger, the property "state" in "PatchTrafficInfluence" is correctly described.

FabrizioMoggio added a commit to FabrizioMoggio/EdgeCloud that referenced this issue Feb 26, 2024
- alignement of parameters with EdgeCloud_LCM: applicationId changed into appId and instanceId changed into appInstanceId
- modified reference to CAMARA Authorization guidelines link

- Telco Edge Site changed in Edge Cloud Zone (camaraproject#163)

(camaraproject#191)

- added: info-contact
- Device: IPV4 and IPV6 changed to represent just one IP. Net mask is no more valid.
- global tags definition
- adopted lowerCamelCase for OperationId
- added descriptions for Delte and Get (for specific resource) methods
- added missing operationid
- improvement of callback definition
- added "description" to the TrafficInfluence resource
- added "description" to the PatchTrafficInfluence resource
- added "description" to TrafficInfluenceNotification
- added "description" to NetworkAccessIdentifier
- added "description" to ErrResponse
- added "description" to message
- added "description" to status
- added "description" to ErrorInfo
- removed unused error code SessionNotFound404
FabrizioMoggio added a commit to FabrizioMoggio/EdgeCloud that referenced this issue Feb 26, 2024
- alignement of parameters with EdgeCloud_LCM: applicationId changed into appId and instanceId changed into appInstanceId
- modified reference to CAMARA Authorization guidelines link

- Telco Edge Site changed in Edge Cloud Zone (camaraproject#163)

(camaraproject#191)

- added: info-contact
- Device: IPV4 and IPV6 changed to represent just one IP. Net mask is no more valid.
- global tags definition
- adopted lowerCamelCase for OperationId
- added descriptions for Delte and Get (for specific resource) methods
- added missing operationid
- improvement of callback definition
- added "description" to the TrafficInfluence resource
- added "description" to the PatchTrafficInfluence resource
- added "description" to TrafficInfluenceNotification
- added "description" to NetworkAccessIdentifier
- added "description" to ErrResponse
- added "description" to message
- added "description" to status
- added "description" to ErrorInfo
- removed unused error code SessionNotFound404
@FabrizioMoggio
Copy link
Collaborator

Linting rules (spectral) have been updated to be compatible with TI API methods definition.

https://github.com/camaraproject/Commonalities/pull/169/files

@sergiofranciscoortiz is the new spectral.yaml automatically used by Edge Cloud MegaLinter or do we need to update it?

@javierlozallu
Copy link
Collaborator

@FabrizioMoggio I'm not sure if MegaLinter uses the new spectral.yaml but since passing the Linter test is mandatory, you could ask at commonalities if passing MegaLinter is enough to cover it or we need to use spectral.yaml locally.

P.S. Sergio is no longer part of the project, I'm covering for him in case you need anything or if I miss any important information regarding EdgeCloud Apis.😄

@FabrizioMoggio
Copy link
Collaborator

I include @rartych in the loop to ask him how to move forward considering camaraproject/Commonalities#169. Rafal, how do we update Edge Cloud MegaLinter to include that PR?

@rartych
Copy link
Collaborator

rartych commented Apr 17, 2024

You need just to update the https://github.com/camaraproject/EdgeCloud/blob/main/.spectral.yml (replace it with the new version from: https://github.com/camaraproject/Commonalities/blob/main/artifacts/linting_rules/.spectral.yml )
We should expect the update of linting rules to cover Commonalities 0.4.0 requirements.
If anyone have idea how to automatically propagate changes in linting rules to all repos, please propose it in Commonalities.

@FabrizioMoggio
Copy link
Collaborator

@javierlozallu I will wait for the spectral update to update the TI PR (#167) with a new YAML that fixes the all the MegaLinter errors. Please let me know here when it is done.

@javierlozallu
Copy link
Collaborator

Meanwhile, @gunjald as mentioned in the meeting, could you ask Commonalities for a tool to test common parameters across APIs and also, as you mentioned, if we can configure MegaLinter to test a specific set of YAMLs instead of the whole repository? Please update us here or create a new discussion if you think it is better to have it separate.

@rartych
Copy link
Collaborator

rartych commented Apr 19, 2024

MegaLinter has rather static configuration with the filters for files included/excluded for the linting. I guess you would like to lint files for given API depending on the content of PR.
The short-term solution can be Manually running linting workflow - if you prepare separate workflows for each API using spectral_oas_lint.yml as template and changing its last line. Then the respective action can be triggered manually after merging PR, to check for errors and warnings.

@javierlozallu
Copy link
Collaborator

@javierlozallu I will wait for the spectral update to update the TI PR (#167) with a new YAML that fixes the all the MegaLinter errors. Please let me know here when it is done.

Hi @FabrizioMoggio, I have created PR #231 to update spectral.yaml with the latest version. Please check it to merge.

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 a pull request may close this issue.

4 participants