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

RFC: CI framework #33

Closed
SOPHIA2401 opened this issue May 5, 2022 · 5 comments
Closed

RFC: CI framework #33

SOPHIA2401 opened this issue May 5, 2022 · 5 comments

Comments

@SOPHIA2401
Copy link
Contributor

SOPHIA2401 commented May 5, 2022

Is your feature request related to a problem? Please describe.
We don't have an automated testing mechanism for our Smithy models to guarantee that they are accurate representations of OpenSearch service APIs. To validate these smithy models, we have to manually match the PR raised by the contributor with the OpenSearch documentation. This is a time-consuming and error-prone process.
As a result, implementing a continuous integration workflow will be extremely helpful in ensuring that the smithy models accurately reflects the API implementation.

Describe the solution you'd like.
We propose to use Dredd framework for validating our smithy models API description document against backend implementation of the API. We will make Dredd part of our continuous integration workflow using GitHub actions. CI workflow will ensure that the application code we are developing doesn’t violate the contract between the API implementation and the API documentation.

Additional context

Working of Continuous Integration workflow:
When a contributor raises a PR (pull request) for the Smithy specification, GitHub will listen for it and take an automated action using GitHub actions, which will trigger a CI workflow.

Further in the CI workflow:

  1. The OpenSearch domain endpoint will be linked, and the appropriate environment will be set up.
  2. The Dredd framework will iteratively go through our OpenAPI specification file, validating that the documented responses satisfy the implemented responses for all APIs using a Python script.
  3. If tests pass and the PR is approved, we can merge the PR; otherwise, all the steps will be repeated again whenever the contributor raises an updated PR.

Requirements:

  1. OpenAPI specification: This is a yaml script file that Dredd will use as a test case for API implementation. URL parameters, request headers, query parameters, request body parameters to be passed and the API's accepted response body parameters will all be included in this file. We need to mention examples in this file for all the required field.
    Example: While implementing Create Index API here, we have passed “books” as an example index URL parameter.
paths:
  /{index}:
    put:
      description: Creates index mappings.
      operationId: PutCreateIndex
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/PutCreateIndexRequestContent'
      parameters:
        - name: index
          in: path
          schema:
            type: string
            pattern: ^[^+_\-\.][^\\, /*?"<>| ,#\nA-Z]+$
          required: true
          example: books

Smithy models can easily be configured to translate into OpenAPI format by specifying

  1. OpenAPI plugin in the smithy-build.json file
  2. OpenAPI dependency in build.gradle.kts file.
 // Plugin  
    "plugins": {
        "openapi": {
            "service": "OpenSearch#OpenSearch",
            "protocol": "aws.protocols#restJson1"
        }
    }
    
 // Dependency
  classpath("software.amazon.smithy:smithy-openapi:1.21.0")
  1. Hooks file: This is the file used to write blocks of code that run before or after an API implementation to ensure that the setup and tear down procedures for any API are performed.
    Below is the snippet for set up process before implementing “DELETE” API.
//Hooks for Delete Index

hooks.before("/{index} > DELETE > 200 > application/json",function(transactions){
   transactions.expected.headers['Content-Type'] =  "application/json; charset=UTF-8";
     
       // Create an index with non-default settings.
       var index_name = "books";
       var settings = {
         settings: {
           index: {
             number_of_shards: 4,
             number_of_replicas: 3,
           },
         },
       };
     
       var response = client.indices.create({
         index: index_name,
         body: settings,
       });
     
       hooks.log("Index created before implementing Delete Index API.");

});
  1. Driver-Script file: This file will be responsible for running a python-script when invoked by CI workflow, to iteratively call all the OpenAPI documentation present for API implementation and will store the details of total tests passed/failed.

  2. Config file: This file will be used to define our GitHub Actions workflow. This will be written in YAML and live inside our GitHub repository in the .github/workflows directory.

Test Directory structure:
Let’s say we are creating IDLs for Ping, Create index and Delete APIs. this is how our test directory will look like.

test
├── driver-code.py
└── models
      ├── _global
      │     └── ping
      │          ├── hooks.js
      │          └── OpenSearchModel.yaml           
      └── indices
             ├── create
             │     ├── hooks.js
             │     └── OpenSearchModel.yaml 
             └── delete 
                   ├── hooks.js
                   └── OpenSearchModel.yaml 

Benefits:

  1. The CI system allows us to detect bugs more quickly.
  2. We can save time on manual verification of Smithy model generated by the contributor by using OpenSearch documentation for the service APIs.
  3. Using a continuous integration workflow, we can improve software quality.

Limitations:

  1. The Smithy model's OpenAPI specification file will be used as a test file. As a result, the contributor has to submit the OpenAPI specification file and set the default examples for all the fields that are required.
  2. For the set up and teardown processes of API implementation, the contributor needs to write a hooks file.
@dblock
Copy link
Member

dblock commented May 5, 2022

Would this still be needed if we generated the API implementation in OpenSearch from spec? That’s one option in opensearch-project/OpenSearch#3090

@pgtgrly
Copy link
Contributor

pgtgrly commented May 6, 2022

@dblock This will be needed initially to create the definitions of all the APIs to ensure that the definitions match the actual implementation as they are not coupled.
Once we couple the classes generated from the definition to the core for serialisation and deserailaisation, this CI framework can be deprecated.

@dblock
Copy link
Member

dblock commented May 6, 2022

Why not skip this step and do opensearch-project/OpenSearch#3090?

@sachetalva
Copy link
Collaborator

I think having a CI framework to validate the smithy models being created for the API specification would be very useful to find out missing parameters / request-response structures. We have already found few issues in the models while testing out using dredd framework.
Having this CI framework will ensure that the API models being created are matching the actual implementation and would help in reducing churn when these models are used to generate the rest api handler side code as part of opensearch-project/OpenSearch#3090
We can possibly deprecate the CI step later if it is not needed or it could also be repurposed it to ensure there is no unintended regression in the API specification.

@SOPHIA2401
Copy link
Contributor Author

Implemented #35

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

No branches or pull requests

4 participants