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

Upgrading kube2pulumi to use pulumi/pulumi@3.46.0 causes changes in YAML comments #82

Closed
phillipedwards opened this issue Nov 4, 2022 · 2 comments · Fixed by #85
Closed
Assignees
Labels
customer/feedback Feedback from customers kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@phillipedwards
Copy link
Member

phillipedwards commented Nov 4, 2022

What happened?

Upgrading kube2pulumi to use the latest Pulumi SDK v3.46.0 forces an upgrade from github.com/goccy/go-yaml v1.8.* to github.com/goccy/go-yaml v1.9.*. Unfortunately, go-yaml seems to have changed how comments are represented in yaml AST.

pkg/yaml2pcl/yaml2pcl.go:490:43: comment.Value undefined (type *ast.CommentGroupNode has no field or method Value)
pkg/yaml2pcl/yaml2pcl.go:506:11: comment.Prev undefined (type *ast.CommentGroupNode has no field or method Prev)
pkg/yaml2pcl/yaml2pcl.go:507:11: comment.Next undefined (type *ast.CommentGroupNode has no field or method Next)

Steps to reproduce

see above

Expected Behavior

no change in behavior

Actual Behavior

change in comments

Output of pulumi about

No response

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@phillipedwards phillipedwards added needs-triage Needs attention from the triage team kind/bug Some behavior is incorrect or out of spec labels Nov 4, 2022
@lblackstone lblackstone added customer/feedback Feedback from customers and removed needs-triage Needs attention from the triage team labels Nov 7, 2022
@lblackstone
Copy link
Member

I dug into this, and it unfortunately appears to be a substantial breaking change in the parsing library. That got me thinking that it might be possible to swap out this conversion logic and use the library underpinning our Pulumi YAML support instead. Let's investigate that option before trying to fix these low-level changes.

@lblackstone
Copy link
Member

After some further testing, I found that the breaking API change was in v1.9.0 of the go-yaml library, so I pinned the dependency to v1.8.10 to avoid the API change for now.

@lblackstone lblackstone self-assigned this Nov 16, 2022
@lblackstone lblackstone added this to the 0.81 milestone Nov 16, 2022
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer/feedback Feedback from customers kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants