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

Migrate to SnakeYAML Engine #4836

Merged
merged 3 commits into from Feb 6, 2023

Conversation

manusa
Copy link
Member

@manusa manusa commented Feb 6, 2023

Description

Supersedes closes #4753

Migrate from SnakeYAML to SnakeYAML Engine

Originally posted by @asomov in #4753

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@manusa manusa changed the title Feature/upgrade snakeyaml reviewed Migrate to SnakeYAML Engine Feb 6, 2023
@manusa manusa added this to the 6.5.0 milestone Feb 6, 2023
@sonarcloud
Copy link

sonarcloud bot commented Feb 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@asomov
Copy link
Contributor

asomov commented Feb 6, 2023

@manusa do you expect anything from me or you can continue ?

@manusa
Copy link
Member Author

manusa commented Feb 6, 2023

@manusa do you expect anything from me or you can continue ?

Hi @asomov,

I created a new PR because I couldn't push to your branch, the CI failures are unrelated to these changes. I'll merge once they're green.

Thanks a lot for these changes 🙌

@manusa manusa merged commit 50ab1cb into fabric8io:master Feb 6, 2023
@manusa manusa deleted the feature/upgrade-snakeyaml-reviewed branch February 6, 2023 15:44
@asomov
Copy link
Contributor

asomov commented Feb 6, 2023

@manusa can you please clarify why a dependency to SnakeYAML was added back ? I removed it to use the SnakeYAML Engine

@manusa
Copy link
Member Author

manusa commented Feb 6, 2023

The dependency is only added for the Karaf Bundle.

Jackson Dataformat YAML has a transitive dependency to SnakeYAML (2.14.2 -> 1.33). Karaf requires to add the bundle to the container in order to be able to run the test application.

So yes, our internal usage of SnakeYAML has now shifted to SnakeYAML Engine thanks to your changes. However, I guess Jackson is still using the regular SnakeYAML.

I'm also assuming that Jackson does a responsible use of SnakeYAML internally.

@asomov
Copy link
Contributor

asomov commented Feb 7, 2023

@manusa thank you for the clarification.

I'm also assuming that Jackson does a responsible use of SnakeYAML internally.

correct assumption (but the quasi-security tooling still creates a false positive for Jackson)

Jackson already migrated to SnakewYAML Engine in version 3.0

@manusa
Copy link
Member Author

manusa commented Feb 7, 2023

Jackson already migrated to SnakewYAML Engine in version 3.0

I guess someone had something to do with that :) FasterXML/jackson-dataformats-text#106

Looking forward for the 3.0 release

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.

None yet

3 participants