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

chore(lint): add values.schema.json validation strict #89

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

jon-whit
Copy link
Collaborator

@jon-whit jon-whit commented Nov 29, 2023

Description

Add additionalProperties: false to the top level values.schema.json specification so that we avoid introducing new values into the values.yaml file without having explicit documentation for it.

As part of ☝️ it was brought to my attention that we didn't have some of the common Helm chart values specified in the values.schema.json, so coverage has been added for those undocumented values.

References

Added entries for the top-level subchart properties to the values.schema.json file so we can do additionalProperties: false and still pass the Helm linter. See helm/helm#10392 for more info.

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@jon-whit jon-whit force-pushed the strict-values-schema-json branch 2 times, most recently from 1ff6825 to 9d60600 Compare November 29, 2023 17:57
@jpadilla
Copy link
Member

@jon-whit when this is ready to get merged, can we bump openfga to v1.4.0?

@jon-whit jon-whit marked this pull request as ready for review January 4, 2024 16:38
@jon-whit jon-whit requested a review from a team as a code owner January 4, 2024 16:38
@jon-whit jon-whit force-pushed the strict-values-schema-json branch from b4be0dc to c2e851c Compare January 4, 2024 16:43
@jon-whit jon-whit force-pushed the strict-values-schema-json branch from c2e851c to f317e8f Compare January 4, 2024 16:52
@jon-whit jon-whit enabled auto-merge (squash) January 4, 2024 16:54
Copy link
Member

@rhamzeh rhamzeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the initContainer change breaking?

@jon-whit
Copy link
Collaborator Author

jon-whit commented Jan 4, 2024

Is the initContainer change breaking?

@raghd It is, but the prior values definition seems quite misleading. The migrations job image isn't the k8s-wait-for image, that's the init container image for the migrations job. So that yaml definition, to me, was quite misleading.

@rhamzeh
Copy link
Member

rhamzeh commented Jan 12, 2024

@jon-whit - yeah the new way may be better, but let's document that in the release notes, because anyone who was previously working with it will now have to update to a different config

@jon-whit jon-whit merged commit 5b81239 into main Feb 12, 2024
4 checks passed
@jon-whit jon-whit deleted the strict-values-schema-json branch February 12, 2024 15:31
@jon-whit jon-whit mentioned this pull request Mar 7, 2024
4 tasks
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

Successfully merging this pull request may close these issues.

3 participants