[ISV-5500] Validate ci.yaml file with json schema #781
Conversation
| "description": "Whether the operator already have FBC support or not", | ||
| "type": "boolean" | ||
| }, | ||
| "catalog_mapping": { |
There was a problem hiding this comment.
This section is part of a separate story. But now when you added it you can keep it.
| "type": "object", | ||
| "properties": { | ||
| "enabled": { | ||
| "description": "Whether the operator already have FBC support or not", |
There was a problem hiding this comment.
Let's rephrase it.. Flag that determines whether the operator uses FBC delivery method. The original message might be misleading since FBC is not a feature that the operator offers.
| "required": [ "catalog_mapping" ] | ||
| } | ||
| }, | ||
| "required": [ "fbc" ] |
There was a problem hiding this comment.
The FBC flag is not required.
| "description": "json schema validating the ci.yaml", | ||
| "type": "object", | ||
| "additionaProperties": true, | ||
| "properties": { |
There was a problem hiding this comment.
There are couple of other properties that we use from the ci file.
- updateGraph (str, optional)
- reviewers (list of strings, optional)
- merge (bool, optional)
- cert_project_id (str, optional)
Please add the missing properties to the schema.
There was a problem hiding this comment.
Updated. As I was adding these fields, I realized that the updateGraph property is also valid for enum as we need to have only two values semver-mode or replaces-mode using for that. Should I add the enum for this property?
There was a problem hiding this comment.
The place where you added the additional fields is not correct. You put them under fbc object however they are on the root level of the file.
There was a problem hiding this comment.
Let's keep it simple for now and don't add any additional enums.
| "type": "object", | ||
| "properties": { | ||
| "template_name": { | ||
| "description": "Name of the catalog template file (required)", |
There was a problem hiding this comment.
What's the pattern for adding (required) to the description? It is a bit inconsistent. Ether removed it from the description since it is already marked as required in the schema or add it to all descriptions for all required fields.
| }, | ||
| "minItems": 1 | ||
| }, | ||
| "type": { |
There was a problem hiding this comment.
The type is enum with set of allowed values. Please add it there. "enum": ["olm.template.basic", "olm.semver"]
| @@ -167,6 +167,26 @@ def check_bundle_release_config(bundle: Bundle) -> Iterator[CheckResult]: | |||
| ) | |||
|
|
|||
|
|
|||
| def check_validate_schema_ci_config( | |||
There was a problem hiding this comment.
The ci.yaml file is associated with the operator. Please move this check under the operator test suite.
|
@Allda I updated the PR with all suggested changes. Could you please review them? |
| } | ||
| }, | ||
| "minItems": 1, | ||
| "required": [ "catalog_mapping" ] |
There was a problem hiding this comment.
The catalog mapping is not a mandatory field. It depends on a fbc.enabled flag. Please keep it optional in the schema.
| "required": [ "catalog_mapping" ] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Please add the missing empty line.
…inst the json schema.
Merge Request Checklists