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

Metadata Schema Missing additionalProperties: false Constraint #3770

Open
theosanderson opened this issue Feb 26, 2025 · 0 comments
Open

Metadata Schema Missing additionalProperties: false Constraint #3770

theosanderson opened this issue Feb 26, 2025 · 0 comments

Comments

@theosanderson
Copy link
Member

Metadata Schema Missing additionalProperties: false Constraint

Issue Description

The Loculus Helm Chart values.yaml JSON schema is not correctly validating additional properties in the metadata definition. When adding a new property includeInDownloadsByDefault to a metadata field, the schema validation did not catch this addition as an error, even though it wasn't defined in the schema.

Root Cause

The metadata definition in the schema is missing the additionalProperties: false constraint. Without this constraint, JSON Schema validation allows any additional properties not explicitly defined in the schema, which can lead to silent additions of unintended properties.

Current Behavior

metadata:
 - name: sampleCollectionDate
   displayName: Collection date
   header: Sample details
   ingest: ncbiCollectionDate
   order: 10
   includeInDownloadsByDefault: true  # This property had not defined in the schema but was accepted
   preprocessing:
     function: parse_date_into_range
     inputs: ...

The above configuration is accepted by the schema validator even though includeInDownloadsByDefault wasn't defined in the schema.

Expected Behavior

The schema should reject any undefined properties until they are properly added to the schema definition. This would ensure consistent configuration and prevent typos or misunderstandings.

Proposed Solution

Add additionalProperties: false to the metadata definition in the schema:

"metadata": {
  "type": "object",
  "additionalProperties": false",  // Add this line
  "properties": {
    // existing properties
    "includeInDownloadsByDefault": {
      "groups": ["metadata"],
      "type": "boolean",
      "description": "Whether this field should be included in data downloads by default."
    }
  },
  "required": ["name"]
}

Impact

This change would make the schema more strict and help prevent silent additions of unintended properties, improving the reliability of configurations and making schema errors more discoverable.

Related

Note that other sections like ingest already have additionalProperties: false constraints which work properly to prevent undefined properties.

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

1 participant