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

Expose the CALM CLI as a RESTful API #689

Open
Budlee opened this issue Dec 17, 2024 · 13 comments
Open

Expose the CALM CLI as a RESTful API #689

Budlee opened this issue Dec 17, 2024 · 13 comments
Assignees
Labels
Milestone

Comments

@Budlee
Copy link
Member

Budlee commented Dec 17, 2024

Feature Request

Description of Problem:

The problem that I face is that I am working with dockerized Java appliation and would like to use the calm validation that has been created within the CALM cli.
Currently there is no easy way to access the CLI from a dockerized application as it requires the cli to be added to the container then calling to the system process to run the cli and looking at the output.
This is a less than optimal solution.

Potential Solutions:

I would like to propose is to add an option to the calm cli labelled server.

calm server -p 8080

This new option would then spin up a simple server to host the calm cli validation behaviour as an API endpoint that can be called by other applications.

The validation endpoint as an API will provide better interopoability for other services that want to utilise what has been built.
Additionally the process of dockerizing and running such an application would be more straight forward, it would be great id the CLI could be put onto dockerhub
I see this being used as a sidecar that an application can call locally.

calm server -p 8080
java myApp --validate-calm localhost:8080

TAG: @jpgough-ms @rocketstack-matt

@rocketstack-matt
Copy link
Member

@Thels have you already picked this up? If so can you assign to yourself, thanks!

@Thels
Copy link
Member

Thels commented Dec 19, 2024

I did play around with spinning up an expressJS server as part of a calm server command - and that's went fine ( and I can raise a PR for that small amount of work ). But ran into some problems with trying to test it in the cli.spec.ts - but a proper implementation tackling this issue would need;

  • OpenAPI support - likely moving to https://hono.dev/ as it has great OpenAPI support and is very Express like.
  • Definition of what API's we want to expose ( for example do we want to expose all formats of output validation or only JSON )

I'll take another stab at this tomorrow as my day is looking quite empty, at least if it's formalizing the "starting point" I mentioned at the top of this PR.

@Thels Thels self-assigned this Dec 19, 2024
@Thels
Copy link
Member

Thels commented Dec 19, 2024

@Thels have you already picked this up? If so can you assign to yourself, thanks!

But fair point on the assignee - fixed it to myself now.

@Budlee
Copy link
Member Author

Budlee commented Jan 10, 2025

After the discussion in the Office Hours call (#776) we want to re-scope this issue. @jpgough-ms shall we close this open a new one or just edit the title and provide and update in the initial description.
If you think we should just open a new one then just close this one

@Thels
Copy link
Member

Thels commented Jan 10, 2025

Discussed briefly on a call;

  • Currently validate CLI requires both an architecture and a pattern file path. @Budlee would like to be able to have the architecture be just JSON in the body. So we need to think about saving it down as part of the server into a temporary location.
  • The 0.01 release for the CLI Server will simply expose the ability to validate with a known pattern file - but the architecture being in JSON. With a simple /api/v1/validate endpoint.

@jpgough-ms
Copy link
Member

After the discussion in the Office Hours call (#776) we want to re-scope this issue. @jpgough-ms shall we close this open a new one or just edit the title and provide and update in the initial description. If you think we should just open a new one then just close this one

@Budlee perhaps just repurpose this one, as after discussion it can be changed to represent more accurately what we are trying to do with the history/context?

@Budlee
Copy link
Member Author

Budlee commented Jan 16, 2025

After the discussion in the Office Hours call (#776) we want to re-scope this issue. @jpgough-ms shall we close this open a new one or just edit the title and provide and update in the initial description. If you think we should just open a new one then just close this one

@Budlee perhaps just repurpose this one, as after discussion it can be changed to represent more accurately what we are trying to do with the history/context?

Updated the ticket to be more representative

@Budlee
Copy link
Member Author

Budlee commented Jan 16, 2025

Proposed workings of the Validate endpoint

When looking at the CALM cli Validation command I think there are four key values -p, -i, -m -f

 -p, --pattern <pattern>                         Path to the pattern file to use. May be a file path or a URL.
  -i, --instantiation <instantiation>             Path to the pattern instantiation file to use. May be a file path or a URL.
  -m, --metaSchemasLocation <metaSchemaLocation>  The location of the directory of the meta schemas to be loaded (default: "/usr/local/lib/node_modules/@finos/calm-cli/dist/calm/meta")
  -f, --format <format>                           The format of the output (choices: "json", "junit", default: "json")

As this proposal is worked through it should answer how these 4 are specified/resolved in the cli server operation

My proposal on the validation endpoint as part of the CALM CLI Server is to allow the API Consumer to pass in a pattern instantiation that should be validated.

POST /calm/validate
content-type: JSON
Accept: JSON
{
  "calm": "instantiation"
}

This POST resolves three of the required arguments:

  1. -i it allows the user to specify the CALM instantiation
  2. -f The Accept header specifies the returned format type JSON, we can then later support JUnit type later on (though I'm not sure how this would look).
  3. -pIf the instantiation is part of a pattern then the pattern should be listed in the CALM under the "$schema" type.

As the pattern is listed in the calm instantiation it will need to be looked up.
To avoid any security risk of the cli-server reaching out to another server to retrieve a specification I believe the metaSchemas should patterns should be made available to the server on startup in folders that we want to use.

clam server --schema-locations folderSchemasDraft2024.10,folderPatterns,folderSchemasDraft2024.12

This solves the arguments of -m and the list of patterns -p that I want the CLI to support.

When/If the CLI is dockerized then it becomes straight forward to mount the folders we want into the container.

Endpoint reasoning

I think that if we use the endpoint /calm/validate we can then extend the other functionality to /calm/<CLI-Command>.
Currently what I see is that all of the commands that we may want to expose are functional calls

I'd appreciate this and others thoughts on this idea of the implementation.

TAG: @rocketstack-matt @jpgough-ms @Thels @willosborne

@Thels
Copy link
Member

Thels commented Jan 16, 2025

The only potential issue I see with what you're proposing above is the --schema-locations element. Right now we pass in the single location, so this would need to be changed/adapted ( although I assume this is likely covered under #790 ? ). But for now if we just wanted it to be a single folder to get things moving that could also work and removes that dependency for now.

Is there any reason why you would want to pass in the CALM folders at start-up as opposed to having them part of the payload? Not against it just curious if you considered it as an alternative.

Going onto specifics of validate, that may fall out of the scope of this issue but could impact the vision.

  • Validate supports validating an architecture ( @grahampacker-ms - didn't we rename instantiation -> architecture? Or am I making that up ) against a pattern. Or simply validating it in isolation without a pattern. ( @lbulanti-ms recently did this ) This means that there would need to be a new issue raised to have the $schema pointer followed. This would probably need to be piped through Typescript CALM Module Refactoring - Visualizer, Doc Publisher, Schema Directory, CALM Model #790 also given that longer term we would want it to resolve a CALMHub protocol.
  • Format is a funny one, the CLI currently does some transformation on the "validation outcome payload" from the validate command to put it into another format. Even when it says "JSON" yes it's outputting the JSON string but it's doing it in a "pretty" way. I think for a v0.0.01 or whatever of this CLI we probably just want to have it be JSON only with no flexibility.

@willosborne
Copy link
Member

I think this is a good approach - pre-load the pattern file in by having it configured as a metaschema location.
It certainly makes the process of sending the pattern files a lot simpler - otherwise I guess we need to

  • pass arch, pattern, etc as sub-parts of the payload in JSON
  • use multipart uploads to send multiple files
    ...neither of which I like.

We definitely need to rework schema-location to take multiple options, and we should also apply this rework to the other commands to keep things consistent.

I also agree with @Thels - let's just do JSON-only for now. The pretty output is just not something we'd need on the server, and the JUnit format is for test evidence in CI so probably doesn't overlap all that much with the server in terms of usage

@Budlee
Copy link
Member Author

Budlee commented Jan 17, 2025

Firstly thanks @willosborne and @Thels for the feedback

@Thels, I don't like having to pass in the pattern with the payload as I think it's more secure that you are defining upfront all the patterns available to be used and not allowing "junk" to be used to validate against.

Let's go with one folder for the time being and just output JSON.

@Thels i can have a crack at implementing what I am proposing if that's okay with you.

@rocketstack-matt are you happy with this and having it as an experimental feature that could go in?

@Budlee
Copy link
Member Author

Budlee commented Jan 22, 2025

Hey,

I've been building the server and I have found a couple of issues with the validation and the schemaDirectory.
I am coding around them, however, I want to share them as they may want to be fixed before my change.

  1. The SchemaDirectory has a Process.exit(1) coded into the schema loading.

     } catch (err) { 
         if (err.code === 'ENOENT') {
             this.logger.error('Schema Path not found: ', dir, ', error: ', err);
         } else {
             this.logger.error(err);
         }
         process.exit(1)
     }

    Fix (the function calling it honours the return of false and does process.exit)

     } catch (err) { 
         if (err.code === 'ENOENT') {
             this.logger.error('Schema Path not found: ', dir, ', error: ', err);
         } else {
             this.logger.error(err);
         }
         return false
     }
     return true
  2. The schemaDirectory loadSchemas function fails when loadSchema does not load a scheama.
    When loading the individual scheams in the loadSchama it can return undefined, however, there is no check for undefined

     for (const schemaPath of schemaPaths) {
         const schema = await this.loadSchema(schemaPath);
         map.set(schema['$id'], schema);
     }

    Fix

     console.info("Found schema: " + schema)
     if (schema){
         map.set(schema['$id'], schema);
     }
  3. loadSchema can not load yaml files
    Though LoadScheams has a file filter for yaml and json the loadscheama is converting the yaml to JSON.
    THough yaml should be compatible with JSON I am seeing it error out, though it may be bad YAML.

     const parsed = JSON.parse(str);

    Fix

     if (!schemaPath.endsWith(".json")){
         this.logger.warn("yaml/yml schema loading is not currently supported")
         return
     }   
  4. The biggest issue I see is the ajv
    This ajv is looking up the schema's and reaching out.
    It is not doing what is expected and using the scheama directory.
    In my case I see it trying to call out to get the calm draft in github.

    You can validate this at home by running the calm CLI locally and disconnecting from the internet.

    Currently I have no fix.
    If someone that has worked with ajv can help with this it would be appreciated

@willosborne, @jpgough-ms , @Thels, @Bula365

@willosborne
Copy link
Member

  1. this makes sense. process.exit should not be called by the directory, so I'm happy with this change. Part of me wonders if we should instead just let the exception propagate up - but can always come back to this when we refactor.

  2. this is to catch the case where the schema path has nothing in it? is this even a use case we want to support? surely setting this to an empty folder will likely indicate an error in how they're using the tool?

  3. AFAIK we don't formally support yaml anyway, so i'm okay with disabling it outright for now. @jpgough-ms @rocketstack-matt am I right in thinking this?

  4. it's really strange (and concerning) that AJV is calling out. I thought it wasn't meant to do that, that URLs are just identifiers. @lbulanti-ms @grahampacker-ms does that surprise you?
    In general the way AJV loads schemas is quite annoying; it has its own internal directory of loaded schemas. I want to make it more integrated and use the same schema directory loading mechanism as the other commands soon but have not got around to it; but it should still not be doing any remote lookups.

@LeighFinegold LeighFinegold added this to the 0.1 milestone Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants