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

fix(yaml) do not crash when the specified YAML array index does not exist #502

Merged
merged 3 commits into from
Feb 1, 2022

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Feb 1, 2022

Fix #439

This was a quick one that fixes a real bug (a case on limit).

Thanks to the amount of unit tests that @olblak did on the homemade YAML parser, it was easy to add a failing case, and then fix it.

Now, the following error is thrown when an out of bounds index is specified for YAML arrays:

ERROR: ✗ cannot find key "spec.containers[1].name" in the YAML file ".tmp/PodTemplates.yaml"

Test

To test this pull request, you can run the following commands:

$ make test-short # Fails without the fix on `pkg/plugins/yaml/main.go`
$ go build -o ./dist/updatecli && ./dist/updatecli diff --config ./.tmp/updatecli.yaml --values ~/workspace/jenkins-infra/kubernetes-management/updatecli/values.yaml

Additional Information

  • Also removed the "non cleaned" yaml file that was still commited (caught by @lemeurherve in his PR ❤️ ).
  • Took the opportunity to switch the Test_Replace() to the testify/assert instructions

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
@dduportal dduportal changed the title Gh 439 fix(yaml) Feb 1, 2022
@dduportal dduportal changed the title fix(yaml) fix(yaml) do not crash when the specified YAML array index does not exist Feb 1, 2022
@dduportal dduportal added bug Something isn't working resource-yaml Resource of kind YAML labels Feb 1, 2022
…xist

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
@dduportal dduportal enabled auto-merge (squash) February 1, 2022 16:46
@dduportal dduportal merged commit f4fd423 into updatecli:main Feb 1, 2022
@dduportal dduportal deleted the gh-439 branch February 1, 2022 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resource-yaml Resource of kind YAML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If a key doesn't exists updatecli crashes
3 participants