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

"Managed" YAML files should store JSON patches, not Unix diffs #75

Open
aiyengar2 opened this issue Jan 28, 2022 · 3 comments
Open

"Managed" YAML files should store JSON patches, not Unix diffs #75

aiyengar2 opened this issue Jan 28, 2022 · 3 comments
Assignees
Labels
enhancement New feature or request team/mapps

Comments

@aiyengar2
Copy link
Contributor

Since the Chart.yaml and values.yaml files are prone to conflicts on rebases but are guaranteed to be real YAML files (e.g. they won't have any weird go templating stuff in them, unlike other Helm files), the diff we store for these two root files should be a JSON patch instead of a Unix diff.

Since it's far more unlikely for a JSON patch applied on the base chart to encounter a conflict than a Unix diff, this should make the process of applying rebases far simpler and remove the caveat the scripts have with working with these "Managed Files"

@aiyengar2 aiyengar2 self-assigned this Jan 28, 2022
@aiyengar2 aiyengar2 added the enhancement New feature or request label Jan 28, 2022
@aiyengar2
Copy link
Contributor Author

Will require a charts-build-scripts minor release since it will involve migrating existing Chart.yaml.patch files to a new format that is incompatible with previous versions of the script. Also will require non-trivial updates to introduce special diff logic.

@aiyengar2 aiyengar2 added this to the v2.x - Backlog milestone Jan 28, 2022
@aiyengar2
Copy link
Contributor Author

To support apiVersion: v1 Helm charts, we should also support requirements.yaml using JSON patches

@aiyengar2 aiyengar2 changed the title Chart.yaml and values.yaml should store JSON patches, not Unix diffs Managed YAML files should store JSON patches, not Unix diffs Jan 28, 2022
@aiyengar2 aiyengar2 changed the title Managed YAML files should store JSON patches, not Unix diffs "Managed" YAML files should store JSON patches, not Unix diffs Jan 28, 2022
@aiyengar2 aiyengar2 modified the milestones: v2.x - Backlog, 0.4.x Jan 28, 2022
@aiyengar2
Copy link
Contributor Author

#9 might be a good issue to prioritize with this; we could make the default such that certain files automatically use yaml_diff while others default to unified_diff, but by exposing it as a possible override in the Chart.yaml, we could make it easier to opt-in to this.

@deniseschannon deniseschannon removed this from the 0.4.x milestone Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request team/mapps
Projects
None yet
Development

No branches or pull requests

4 participants