Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Harden and simplify yaml stream decoding/encoding #1931

Merged
merged 4 commits into from Apr 15, 2019

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Apr 12, 2019

Flux choked on end-of-document markers (...).

To avoid complicating the existing multidoc parser (stolen from kubernetes) I abused go-yaml'sDecoder to obtain the raw documents from the stream by unmarshalling to an interface{} and marshalling again.

Also, I replaced our ad-hoc stream parsing and generation by a go-yaml Decoder and Encoder.

I, however, left the encoding part of fluxctl save untouched since yaml.Encoder generates different output (no start-of-document prefix) and users may be relying on it.

During testing I made sure that changes I made in fluxd's Export() are compatible with fluxctl save prior to this change.

Fixes #1925

@2opremio 2opremio requested a review from hiddeco April 12, 2019 19:07
@2opremio 2opremio changed the title Support contigous yaml EOD markers Support contigous yaml SOD markers Apr 12, 2019
@2opremio
Copy link
Contributor Author

2opremio commented Apr 12, 2019

Uhm, I think I got something wrong. The test seems to pass without the code changes ...
(I would had sworn it didn't)

@2opremio 2opremio changed the title Support contigous yaml SOD markers [WIP] Support contigous yaml SOD markers Apr 12, 2019
@2opremio 2opremio changed the title [WIP] Support contigous yaml SOD markers [WIP] Harden and simplify yaml stream parsing Apr 13, 2019
@2opremio 2opremio force-pushed the 1925-contigous-yaml-EOD branch 2 times, most recently from 9e0ed2c to cf189e2 Compare April 13, 2019 01:47
@2opremio 2opremio changed the title [WIP] Harden and simplify yaml stream parsing Harden and simplify yaml stream parsing Apr 13, 2019
2opremio pushed a commit to 2opremio/kubeyaml that referenced this pull request Apr 13, 2019
Technically it's correct to add an end-of-document mark to
the output. However, `kubectl` and `Flux` (before
fluxcd/flux#1931 ) choke on it.
2opremio pushed a commit to 2opremio/kubeyaml that referenced this pull request Apr 13, 2019
Technically it's correct to add an end-of-document mark to the output. However,
`kubectl` and `Flux` (before fluxcd/flux#1931 )
choke on it.

This change makes sure we always use `---` markers to separate documents and
that we never add `...`.
2opremio pushed a commit to 2opremio/kubeyaml that referenced this pull request Apr 13, 2019
Technically it's correct to add an end-of-document mark to the output. However,
`kubectl` and `Flux` (before fluxcd/flux#1931 )
choke on it.

This change makes sure we always use `---` markers to separate documents and
that we never add `...`.
2opremio pushed a commit to 2opremio/kubeyaml that referenced this pull request Apr 13, 2019
Technically it's correct to add an end-of-document mark to the output. However,
`kubectl` and `Flux` (before fluxcd/flux#1931 )
choke on it.

This change makes sure we always use `---` markers to separate documents and
that we never add `...`.
Alfonso Acosta added 2 commits April 13, 2019 05:52
Flux choked on end-of-document markers (`...`).

To avoid complicating the existing multidoc parser (stolen from kubernetes) I
abused go-yaml's Decoder to obtain the raw documents from the stream by
unmarshalling to an interface{} and marshalling again.
Use go-yaml's Decoder instead of our own
@2opremio 2opremio changed the title Harden and simplify yaml stream parsing Harden and simplify yaml stream decoding/encoding Apr 14, 2019
@2opremio 2opremio requested a review from hiddeco April 14, 2019 17:34
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Thanks Fons 🍪

@2opremio 2opremio merged commit 5ee497b into fluxcd:master Apr 15, 2019
@2opremio 2opremio deleted the 1925-contigous-yaml-EOD branch April 15, 2019 08:55
@squaremo
Copy link
Member

squaremo commented Apr 24, 2019

Q: Does this mean comments are no longer preserved when files are updated by flux?
(Genuine question; I have lost track of how the bytes are obtained and passed to kubeyaml)

@squaremo
Copy link
Member

A: no, because the kubeyaml-using code still just reads a whole file in, and writes a whole file out.

@squaremo squaremo added this to the v1.12.1 milestone Apr 24, 2019
@2opremio
Copy link
Contributor Author

Yep, exactly, sorry I didn't get to answer the question in time.

squaremo pushed a commit to squaremo/kubeyaml that referenced this pull request Apr 24, 2019
Technically it's correct to add an end-of-document mark to the output. However,
`kubectl` and `Flux` (before fluxcd/flux#1931 )
choke on it.

This change makes sure we always use `---` markers to separate documents and
that we never add `...`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weave cloud deploy page error with '---' at EOF
3 participants