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

Implement use of linting rule set for Blockchain Public Address API #38

Closed
PedroDiez opened this issue Dec 12, 2023 · 10 comments · Fixed by #40, #41 or #42
Closed

Implement use of linting rule set for Blockchain Public Address API #38

PedroDiez opened this issue Dec 12, 2023 · 10 comments · Fixed by #40, #41 or #42
Assignees
Labels
subproject management Indicating issues with subproject repository or release management process

Comments

@PedroDiez
Copy link
Collaborator

Problem description
Implement use of linting rule set for Blockchain Public Address API.
what we have to do is well documented in Commonalities:
camaraproject/Commonalities#110
camaraproject/Commonalities#74

Expected action
Check our API with linting rule set
provide feedback to commonalities team

Additional context
cc: @rartych who asked project volunteer to perform this action.

@PedroDiez PedroDiez added the subproject management Indicating issues with subproject repository or release management process label Dec 12, 2023
@PedroDiez PedroDiez self-assigned this Dec 12, 2023
@PedroDiez
Copy link
Collaborator Author

25/ENE/24: Being able to apply Linter. Next step is to check linter output and made adjustments in two directions:

  • The ones that apply in API spec
  • The refinements in linter configuration for BlockcahinPublicAddress applicability

@PedroDiez
Copy link
Collaborator Author

PedroDiez commented Jan 29, 2024

LINTER RULES OUTPUT

Run spectral lint code/API_definitions/blockchain_public_address.yaml --verbose --ruleset .spectral.yml
spectral lint code/API_definitions/blockchain_public_address.yaml --verbose --ruleset .spectral.yml
shell: /usr/bin/bash -e {0}
env:
APPLY_FIXES: all
APPLY_FIXES_EVENT: pull_request
APPLY_FIXES_MODE: commit
Found 65 rules (56 enabled)
Linting /home/runner/work/BlockchainPublicAddress/BlockchainPublicAddress/code/API_definitions/blockchain_public_address.yaml
warning camara-reserved-words Reserved words found in input: Consider avoiding the use of reserved word 'public' /blockchain-public-addresses
warning camara-reserved-words Reserved words found in input: Consider avoiding the use of reserved word 'public' /blockchain-public-addresses/{id}

/home/runner/work/BlockchainPublicAddress/BlockchainPublicAddress/code/API_definitions/blockchain_public_address.yaml
46:7 warning camara-path-param-id Path Parameter Naming Warning: Use 'resource_id' instead of just 'id' in path parameters. paths
47:32 error camara-parameter-casing-convention /blockchain-public-addresses should be kebab-case: must be kebab case paths./blockchain-public-addresses
58:13 warning oas3-operation-security-defined "blockchain-public-address:read" must be listed among scopes. paths./blockchain-public-addresses.get.security[0].openId[0]
114:13 warning oas3-operation-security-defined "blockchain-public-address:create" must be listed among scopes. paths./blockchain-public-addresses.post.security[0].openId[0]
152:37 error camara-parameter-casing-convention /blockchain-public-addresses/{id} should be kebab-case: must be kebab case paths./blockchain-public-addresses/{id}
163:13 warning oas3-operation-security-defined "blockchain-public-address:delete" must be listed among scopes. paths./blockchain-public-addresses/{id}.delete.security[0].openId[0]
191:12 warning camara-parameters-descriptions Parameter description is missing or empty: "openId.description" property must be truthy components.securitySchemes.openId
207:13 warning camara-path-param-id Path Parameter Naming Warning: Use 'resource_id' instead of just 'id' in path parameters. components.parameters.Id.name
311:15 warning camara-parameters-descriptions Parameter description is missing or empty: "ErrorInfo.description" property must be truthy components.schemas.ErrorInfo
344:21 error oas3-valid-media-example "value" property must have required property "status" components.responses.InvalidArgumentForBlockchain400.content.application/json.examples.InvalidArgument.value
348:21 error oas3-valid-media-example "value" property must have required property "status" components.responses.InvalidArgumentForBlockchain400.content.application/json.examples.InvalidBlockchainNetworkId.value
352:21 error oas3-valid-media-example "value" property must have required property "status" components.responses.InvalidArgumentForBlockchain400.content.application/json.examples.InvalidCurrencyForBlockchain.value
356:21 error oas3-valid-media-example "value" property must have required property "status" components.responses.InvalidArgumentForBlockchain400.content.application/json.examples.RequiredCurrencyforBlockchain.value
373:21 error oas3-valid-media-example "value" property must have required property "status" components.responses.PermissionDeniedForBlockchain403.content.application/json.examples.PermissionDenied.value
377:21 error oas3-valid-media-example "value" property must have required property "status" components.responses.PermissionDeniedForBlockchain403.content.application/json.examples.NotAllowedBlockchainNetworkId.value

@PedroDiez
Copy link
Collaborator Author

PedroDiez commented Jan 29, 2024

Considerations/Feedback:

    1. warning camara-reserved-words Reserved words found in input: Consider avoiding the use of reserved word 'public' /blockchain-public-addresses. NOTE: Action is adjust linter to skip this in Blockchain, no sense to trigger warning. Up to CAMARA Commonalities WG to consider a default rule
    1. 46:7 warning camara-path-param-id Path Parameter Naming Warning: Use 'resource_id' instead of just 'id' in path parameters. paths: OK to the warning, {id} can be kept. No additional action
    1. 47:32 error camara-parameter-casing-convention /blockchain-public-addresses should be kebab-case: must be kebab case paths./blockchain-public-addresses: report to Commonalities as the endpoint path is already kebab-case. Seems it needs adjustments
    1. 58:13 warning oas3-operation-security-defined "blockchain-public-address:read" must be listed among scopes. paths./blockchain-public-addresses.get.security[0].openId[0]: Not sure what to do with this kind of warning.
    1. 191:12 warning camara-parameters-descriptions Parameter description is missing or empty: "openId.description" property must be truthy components.securitySchemes.openId: Not sure what to do with this kind of message. Maybe set a description: openId security schema (to be applied in the same way in all APIs)?
    1. 311:15 warning camara-parameters-descriptions Parameter description is missing or empty: "ErrorInfo.description" property must be truthy components.schemas.ErrorInfo: Not sure what to do with this kind of message. Maybe set a description: Error Model for CAMARA APIs (to be applied in the same way in all APIs)?
    1. 344:21 error oas3-valid-media-example "value" property must have required property "status": These kind of errors need to be fixed in API Spec. Trigger new PR to cover this

@PedroDiez
Copy link
Collaborator Author

@rartych, any consideration in how to proceed with topics iv, v, and vi?

@rartych
Copy link
Collaborator

rartych commented Jan 29, 2024

Thanks @PedroDiez, my comments:

ii. https://github.com/camaraproject/Commonalities/blob/API-linting-Implementation-Guideline/documentation/API-design-guidelines.md#34-path-parameters-use

  1. It is recommended that the identifier have a similar morphology on all endpoints. For example, “xxxxId”, where xxx is the name of the entity it references:
    /users/{userId}
    /accounts/{accountId}

iii. was false positive - corrected in final version of linting rules
iv. rule removed as not fully compatible with OpenIdConnect
v. and vi. if adding description does not make sense then ignore the warning

@PedroDiez
Copy link
Collaborator Author

PedroDiez commented Feb 2, 2024

After Linter execution with configuration as per: camaraproject/Commonalities@29e4e9d

OutPut:

Need to check output in detail:
https://github.com/PedroDiez/BlockchainPublicAddress/actions/runs/7759672690/job/21164236006?pr=1

Seems False-Positive case (kebab-case checking) is working well, so fixed cc @rartych

@PedroDiez
Copy link
Collaborator Author

PedroDiez commented Feb 2, 2024

Regarding Errors:

cc @rartych

@PedroDiez
Copy link
Collaborator Author

Now Linter has good refinement. Pending errors are format related "ones"

code/API_definitions/blockchain_public_address.yaml
Error: 10:1 [trailing-spaces] trailing spaces
Error: 58:11 [indentation] wrong indentation: expected 12 but found 10
Error: 114:11 [indentation] wrong indentation: expected 12 but found 10
Error: 163:11 [indentation] wrong indentation: expected 12 but found 10
Error: 241:1 [trailing-spaces] trailing spaces
Error: 257:17 [trailing-spaces] trailing spaces
Error: 293:1 [trailing-spaces] trailing spaces
Error: 309:17 [trailing-spaces] trailing spaces

cc @rartych, @grgpapadopoulos I will generate new PR to fix them.

Regarding the working of linter rules is fine for Blockchain Public Address

@PedroDiez
Copy link
Collaborator Author

Test with current specification after format updates and newest linter configuration from Commonalities

@PedroDiez
Copy link
Collaborator Author

After latest check:

❌ Linted [YAML] files with [yamllint]: Found 1 error(s) - (0.33s) (expand for details)

  • Using [yamllint v1.32.0] https://megalinter.io/7.3.0/descriptors/yaml_yamllint
  • MegaLinter key: [YAML_YAMLLINT]
  • Rules config: [/github/workspace/.yamllint.yaml]
  • Number of files analyzed: [1]
    --Error detail:
    code/API_definitions/blockchain_public_address.yaml
    Error: 293:1 [trailing-spaces] trailing spaces

Need to check spec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subproject management Indicating issues with subproject repository or release management process
Projects
None yet
2 participants