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

json schema wrongly required fields #1416

Closed
ilyagorban-codefresh opened this issue Nov 7, 2021 · 10 comments
Closed

json schema wrongly required fields #1416

ilyagorban-codefresh opened this issue Nov 7, 2021 · 10 comments
Labels
bug Something isn't working

Comments

@ilyagorban-codefresh
Copy link

ilyagorban-codefresh commented Nov 7, 2021

Describe the bug
While validating using the jsonschema with kubeconform tool (I tried other json validators as well), I found couple of inconsistencies (not required required fields):

  • (EventSource): For field (root): Must validate one and only one schema (oneOf) - For field spec.calendar.interval-time: schedule is required
  • (EventSource): For field (root): Must validate one and only one schema (oneOf) - For field spec.github.push: id is required - For field spec.github.push: owner is required - For field spec.github.push: repository is required

To Reproduce
Yaml with calendar' schedule requirement:

apiVersion: argoproj.io/v1alpha1
kind: EventSource
metadata:
  name: calendar-eventsource
spec:
  eventBusName: some-eventbus
  calendar:
    interval-time:
      interval: 8760h

Yaml with github push id,repo,owner requirement

apiVersion: argoproj.io/v1alpha1
kind: EventSource
metadata:
  name: git-source
spec:
  replicas: 2
  eventBusName: some-eventbus
  service:
    ports:
      - port: 80
        targetPort: "80"
  github:
    push:
      repositories:
        - owner: organization-name
          names:
            - repo-name
      webhook:
        endpoint: /git-source/
        port: "80"
        method: POST
        url: https://some-url
      events:
        - push
      apiToken:
        name: github-token
        key: token
      insecure: true
      active: true
      contentType: json

Steps to reproduce the behavior:

  1. install kubeconform tool
  2. launch in terminal where you saved the yamls above: kubeconform -schema-location https://raw.githubusercontent.com/argoproj/argo-events/master/api/jsonschema/schema.json -strict -output tap .
  3. See error

Expected behavior
Absence of errors on validation

Environment (please complete the following information):

  • Argo: v3.2.0-rc4
  • Argo Events: v1.5.0

Additional context


Message from the maintainers:

If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

@ilyagorban-codefresh ilyagorban-codefresh added the bug Something isn't working label Nov 7, 2021
@MosenzonTal
Copy link

👍

@daniel-codefresh
Copy link
Member

@whynowy This happens because the schema is based on our swagger.json, and all of the fields that were mentioned above are not marked as optional. Do you think we can mark them as optional at least until they are completely removed in v1.6?
Anyway you can assign this one to me

@whynowy
Copy link
Member

whynowy commented Nov 8, 2021

@whynowy This happens because the schema is based on our swagger.json, and all of the fields that were mentioned above are not marked as optional. Do you think we can mark them as optional at least until they are completely removed in v1.6? Anyway you can assign this one to me

That seems to be the only way. Feel free to assigned to yourself, thanks @daniel-codefresh!

@daniel-codefresh
Copy link
Member

daniel-codefresh commented Nov 9, 2021

@whynowy heres the related PR: #1421

@ilyagorban-codefresh
Copy link
Author

I can confirm that it stopped to show errors, but absence of interval in calendar now does not trigger errors at all.

apiVersion: argoproj.io/v1alpha1
kind: EventSource
metadata:
  name: calendar-eventsource
spec:
  eventBusName: some-eventbus
  calendar:
    interval-time:
      blabla: 8760h

@daniel-codefresh
Copy link
Member

@ilyagorban-codefresh That's because interval isn't a mandatory field, for example this is a completely valid Calendar eventsource:

apiVersion: argoproj.io/v1alpha1
kind: EventSource
metadata:
  name: calendar-eventsource
spec:
  eventBusName: some-eventbus
  calendar:
    interval-time:
      schedule: "30 * * * *"

@ilyagorban-codefresh
Copy link
Author

@daniel-codefresh I thought about oneOf example. Although I don't know how it is possible to set in open API to be converted to json schema

@ilyagorban-codefresh
Copy link
Author

@daniel-codefresh
Copy link
Member

@ilyagorban-codefresh Yes, oneOf would've been a great option, but again the json schema is generated automatically based on the swagger schema and unfortunately oneOf isn't supported in swagger v2, nor by the k8s openapi generators that we're using, therefore I think there's probably no quick or maintainable solution for this ATM, without needing to change the whole schema generation process.

@ilyagorban-codefresh
Copy link
Author

You are right, so we can close the ticket - thank you for your help!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants