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

Validation in API layer #3950

Closed
rossjones opened this issue Nov 14, 2023 · 4 comments · Fixed by #4136
Closed

Validation in API layer #3950

rossjones opened this issue Nov 14, 2023 · 4 comments · Fixed by #4136
Assignees
Labels
type/bug Type: Something is not working as expected
Milestone

Comments

@rossjones
Copy link
Contributor

Although we do normalize inputs, there's an oversight on the normalization of the optional publisher field if the job is posted directly to the API.

❯ curl  -H "Content-Type: application/json"  -d "@job.json" -X POST http://127.0.0.1:20000/api/v1/orchestrator/jobs
{
  "error": "code=400, message=1 error occurred:\n\t* task main validation failed: 1 error occurred:\n\t* publisher validation failed: nil spec config\n\n\n\n",
  "message": "1 error occurred:\n\t* task main validation failed: 1 error occurred:\n\t* publisher validation failed: nil spec config\n\n\n\n"
}

where the JSON file looks like the following (note, no publisher).

{
  "Job": {
    "Name": "LogProcessorDuckDB",
    "Type": "batch",
    "Count": 1,
    "Namespace": "logging",
    "Constraints": [],
    "Tasks": [
      {
        "Name": "main",
        "Engine": {
          "Type": "docker",
          "Params": {
            "Image": "expanso/nginx-access-log-processor:1.0.0",
            "Parameters": [
              "--log-path",
              "/logs/**",
              "--start-time",
              "-1 day",
              "--query",
              "SELECT * FROM logs ORDER BY timestamp LIMIT 10",
              "-json"
            ]
          }
        },
        "Resources": {
          "CPU": "0.1",
          "Memory": "512MB"
        },
        "InputSources": [
          {
            "Target": "/logs",
            "Source": {
              "Type": "s3",
              "Params": {
                "Bucket": "bacalhau-usecases",
                "Key": "log-orchestration/part3_12/",
                "Region": "eu-west-1"
              }
            }
          }
        ]
      }
    ]
  }
}
@aronchick
Copy link
Collaborator

Interesting - we should do this (and think about broader frameworks for doing this kind of validation). Working on the swagger stuff right now, would that help?

@rossjones
Copy link
Contributor Author

In this case I think it was just an oversight on the server-side validation, so that it was happening in the client, but not also at the server.

@wdbaruni wdbaruni added the type/bug Type: Something is not working as expected label Apr 15, 2024
@wdbaruni
Copy link
Member

I confirm this is still an issue:

# hello.json
{
  "Job": {
    "Type": "batch",
    "Count": 1,
    "Tasks": [
      {
        "Name": "main",
        "Engine": {
          "Type": "docker",
          "Params": {
            "Image": "ubuntu",
            "Parameters": ["echo", "hello"]
          }
        }
      }
    ]
  }
}
 curl -X PUT localhost:1234/api/v1/orchestrator/jobs -H "Content-Type: application/json" -d @hello.json
{
  "error": "code=400, message=task main validation failed: publisher validation failed: nil spec config",
  "message": "task main validation failed: publisher validation failed: nil spec config"
}

@wdbaruni wdbaruni self-assigned this Apr 15, 2024
@wdbaruni wdbaruni added this to the v1.4.0 milestone Apr 15, 2024
@wdbaruni wdbaruni transferred this issue from another repository Apr 21, 2024
@wdbaruni wdbaruni transferred this issue from another repository Apr 21, 2024
@frrist
Copy link
Member

frrist commented Apr 23, 2024

So we don't forget. I did a bit of code spelunking with cuelang to try and address this in #3735. The idea was to define a schema in cue that we could use to validate job specs against. The resulting work allowed users to validate a job.yaml file against the cue schema via a command like: cue vet -d "#Job" testdata/jobs/docker-output.yaml pkg/models/job-schema.cue and have it return detailed errors on which fields were invalid by line number in the job spec. cuelang has a go sdk (cue is written in go) so it would be fairly straightforward to port this functionality into the bacalhau CLI or API to provide client and server side validation.

@MichaelHoepler MichaelHoepler changed the title Missing validation in API layer Validation in API layer May 24, 2024
@wdbaruni wdbaruni modified the milestones: v1.4.0, v1.5.0 Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Type: Something is not working as expected
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants