Skip to content

chore: handle charts that do not have a Chart.lock file when generating the values schema file#327

Merged
rm3l merged 3 commits intoredhat-developer:mainfrom
rm3l:chore/generate_value_schema_json_from_tpl_even_if_chart_does_not_have_chart_lockfile
Mar 11, 2026
Merged

chore: handle charts that do not have a Chart.lock file when generating the values schema file#327
rm3l merged 3 commits intoredhat-developer:mainfrom
rm3l:chore/generate_value_schema_json_from_tpl_even_if_chart_does_not_have_chart_lockfile

Conversation

@rm3l
Copy link
Member

@rm3l rm3l commented Mar 11, 2026

Description of the change

values.schema.json generation was based solely on the presence of a Chart.lock file, which may not be the case for all the charts here.

Which issue(s) does this PR fix or relate to

How to test changes / Special notes to the reviewer

Run the pre-commit hooks.

Checklist

  • For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
  • For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Run pre-commit run --all-files to run the hooks and then push any resulting changes. The pre-commit Workflow will enforce this and warn you if needed.
  • JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
  • Tests pass using the Chart Testing tool and the ct lint command.
  • If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

@sonarqubecloud
Copy link

@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Template Escaping

The Jinja2 template is created with autoescape enabled while rendering a JSON template. Depending on the template contents, this can HTML-escape characters (e.g., quotes) and produce invalid JSON or unexpected schema output. Validate that generated values.schema.json is identical to the expected output for charts using string values containing special characters, and consider disabling autoescape or using a JSON-safe rendering approach.

with open(template_path, "r", encoding="utf-8") as f:
    my_schema_template = Template(f.read(), autoescape=True)

return json.loads(my_schema_template.render(my_chart_yaml))
Chart Discovery

Charts are discovered by rglob(Chart.yaml) and using the parent directory. This may unintentionally include nested/dependency charts or vendored charts if present in the repo, causing schema generation in unintended locations. Validate the expected set of chart directories and consider filtering (e.g., only under charts/) if necessary.

charts = [p.parent for p in Path(".").rglob(CHART_YAML)]
📄 References
  1. redhat-developer/rhdh-chart/charts/orchestrator-infra/chart_schema.yaml [1-33]
  2. redhat-developer/rhdh-chart/charts/backstage/chart_schema.yaml [1-33]
  3. redhat-developer/rhdh-chart/charts/orchestrator-infra/values.yaml [37-41]
  4. redhat-developer/rhdh-chart/charts/orchestrator-software-templates/values.yaml [1-12]
  5. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27]
  6. redhat-developer/rhdh-chart/charts/backstage/values.yaml [342-356]
  7. redhat-developer/rhdh-chart/charts/backstage/ci/with-test-pod-disabled-values.yaml [1-12]
  8. redhat-developer/rhdh-chart/charts/backstage/templates/tests/test-secret.yaml [1-3]

@rhdh-qodo-merge rhdh-qodo-merge bot added the enhancement New feature or request label Mar 11, 2026
@rhdh-qodo-merge
Copy link

PR Type

Enhancement


Description

  • Changed schema generation to use Chart.yaml instead of Chart.lock file

  • Added graceful handling for charts without template files

  • Updated pre-commit hook to avoid passing multiple files unnecessarily

  • Bumped orchestrator-infra chart version and regenerated schema


File Walkthrough

Relevant files
Enhancement
jsonschema-dereference.py
Switch schema generation to use Chart.yaml                             

.pre-commit/jsonschema-dereference.py

  • Changed file detection from Chart.lock to Chart.yaml for chart
    discovery
  • Updated template_schema() function to accept Chart.yaml data instead
    of lock file
  • Added existence check for template file with informative logging when
    missing
  • Returns None when template file doesn't exist to skip schema
    generation
+18/-8   
Configuration changes
.pre-commit-config.yaml
Disable filename passing for jsonschema hook                         

.pre-commit-config.yaml

  • Added pass_filenames: false flag to jsonschema-dereference hook
  • Prevents passing individual files to script that already parses all
    charts
+1/-0     
Chart.yaml
Bump orchestrator-infra chart version                                       

charts/orchestrator-infra/Chart.yaml

  • Bumped version from 0.5.1 to 0.5.2
+1/-1     
Documentation
README.md
Update documentation with new chart version                           

charts/orchestrator-infra/README.md

  • Updated version badge from 0.5.1 to 0.5.2
  • Updated helm install command example with new version
+2/-2     
Miscellaneous
values.schema.json
Regenerate and reformat values schema                                       

charts/orchestrator-infra/values.schema.json

  • Fixed $id URL from backstage to orchestrator-infra
  • Reformatted JSON with consistent 4-space indentation
  • Reorganized property definitions for better readability
+124/-122

@rhdh-qodo-merge
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid catching overly broad exceptions

Replace except BaseException with except Exception to avoid catching
system-level exceptions like KeyboardInterrupt.

.pre-commit/jsonschema-dereference.py [88-90]

-except BaseException as e:
+except Exception as e:
     print(f"Could not process schema for '{chart}': {e}")
     errors.append(e)

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that catching BaseException is bad practice and proposes the standard Exception class, which improves script robustness and predictability.

Medium
Skip when values.yaml missing

Add a check for the existence of values.yaml and skip processing the chart if
the file is not found to prevent errors.

.pre-commit/jsonschema-dereference.py [80]

-values = read_yaml(chart / VALUES_FILE)
+values_path = chart / VALUES_FILE
+if not values_path.exists():
+    print(f"INFO: '{values_path}' not found; skipping chart '{chart}'")
+    continue
+values = read_yaml(values_path)
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential FileNotFoundError if values.yaml is missing and provides a reasonable fix to improve the script's robustness by skipping the chart.

Low
  • More

@rm3l rm3l merged commit 38e1390 into redhat-developer:main Mar 11, 2026
5 of 6 checks passed
@rm3l rm3l deleted the chore/generate_value_schema_json_from_tpl_even_if_chart_does_not_have_chart_lockfile branch March 11, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant