Add wrapper for serde_yaml

[siegfriedweber]

Description

• Add wrapper for serde_yaml which adheres to the YAML specification. Operators should use the yaml module instead of serde_yaml.
• Update serde_yaml to version 0.9.

This pull request solves the problem that serde_yaml in version 0.9 does not emit leading dashes anymore (see dtolnay/serde-yaml#266) which does not conform to the YAML specification (see https://yaml.org/spec/1.2.2/#22-structures). This becomes especially a problem when concatenating YAMLs which is done in many operators as follows:

Command::Crd => println!(
    "{}{}{}",
    serde_yaml::to_string(&SupersetCluster::crd())?,
    serde_yaml::to_string(&SupersetDB::crd())?,
    serde_yaml::to_string(&DruidConnection::crd())?
),

However, I did not move the concatenation to operator-rs, so it must still be done in the operator:

Command::Crd => {
    let yamls = vec![
        yaml::to_string(&SupersetCluster::crd())?,
        yaml::to_string(&SupersetDB::crd())?,
        yaml::to_string(&DruidConnection::crd())?,
    ];
    println!("{}", yamls.concat());
}

It can be discussed if this step should also be moved into operator-rs.

According to the usage in the operators (https://github.com/search?l=&q=org%3Astackabletech+%22serde_yaml%3A%3A%22+language%3ARust&type=code), it should be sufficient to forward only Error, from_str, and to_string from serde_yaml.

The implementation of yaml::to_string could be improved to use less space but with increased code complexity.

Review Checklist

• Code contains useful comments
• (Integration-)Test cases added (or not applicable)
• Documentation added (or not applicable)
• Changelog updated (or not applicable)
• Cargo.toml only contains references to git tags (not specific commits or branches)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

[nightkr]

Oh, I would have called it serialize_crds or something, since that's the actual use-case we're trying to solve here.

Otherwise a more descriptive name might be documents_to_string or something? to_string doesn't really make it clear to me what the difference is compared to serde_yaml::to_string.

[soenkeliebau]

Code looks good, ran tests, looks good to me overall.

Not sure I agree with the upstreams change, but it is what it is :)

[soenkeliebau]

I think strictly speaking the --- are only mandatory if the file starts with directives, to delimit these from the actual content of the file.
So arguably serde_yaml adheres to the spec, even though I have to say that I do not agree with the upstream change.
Besides that, it might be worthwhile just adding one sentence about what exactly the difference is between this and serde_yaml to make it easier to understand.

[siegfriedweber]

In YAML 1.0 and 1.1, a YAML stream can start with an "implicit document" (without dashes) or an "explicit document" (with dashes), see https://yaml.org/spec/1.0/index.html#id2561559 and https://yaml.org/spec/1.1/current.html#id898785.

It is the same in YAML 1.2 but "implicit documents" are called "bare documents", see https://yaml.org/spec/1.2.2/#913-bare-documents.

So serde_yaml is compliant with the YAML specification. I will change my pull request.

[soenkeliebau]

Is it worth opening an issue against yamllint for this?

[siegfriedweber]

There is already a closed issue with this topic: adrienverge/yamllint#302

[siegfriedweber]

Pull request closed in favor of #450