rpk: sample config to include empty structs

[r-vasquez]

Cover letter

Sample config file had empty properties:
pandaproxy and schema_registry; rpk strips them
out due to the omitempty struct tag.

In the past viper assumed the presence of a
property was enough to include it in the file,
that didn't respect the omitempty tag.

Fixes #5494

Release notes

• The old default redpanda.yaml incorrectly specified the pandaproxy and schema_registry, and rpk had a bug that always added those sections by default. rpk will no longer add those sections no matter what, but the yaml file also now includes the sections properly by default. Using a new rpk on an old default yaml will no longer add the missing pandaproxy and schema_registry sections.

[BenPope]

If the {} are now required, does that mean that we still have a backwards compatibility issue?

[twmb]

Somewhat.

Old versions are already converted to using {} through old rpk. The only problem will be if somebody uses an old redpanda.yaml that has never been touched by rpk, with the new rpk.

[twmb]

Can we actually change this to a specific dedicated test case that loads the conf/redpanda.yaml ? That will ensure we catch drift if the default redpanda.yaml changes -- modifying that file will require fixing this test.

[r-vasquez]

Changed to read from redpanda/conf/redpanda.yaml

[twmb]

Awesome, ty!

[BenPope]

The old default redpanda.yaml incorrectly specified the pandaproxy and schema_registry, and rpk had a bug that always added those sections by default. rpk will no longer add those sections no matter what

It was my impression that an empty tag is valid yaml.

The pandaproxy and schema_registry top-level tags were intentionally added to enable those subsystems by default. Perhaps the omitempty was incorrect.

I wonder if the change in behaviour now requires changes to other examples and documentation?

[twmb]

Per the yaml spec,

https://yaml.org/spec/1.2-old/spec.html#id2805071
https://yaml.org/type/null.html

a blank tag is null. If redpanda is reading null and enabling the schema registry, it's likely a bug.

The main thing we're trying to address is to not add configuration when invoking rpk commands that change the redpanda.yaml file. Previously, if you set a random field, then the pandaproxy and schema_registry would always be added even if you explicitly disabled them previously (by removing the fields entirely). We fixed that, which then showed our default sample configuration was incorrect. This was never noticed because of the old behavior that always added the defaults back. This new PR ensures the defaults are correct.

[BenPope]

a blank tag is null. If redpanda is reading null and enabling the schema registry, it's likely a bug.

That was the intended behaviour, that is the current behaviour. Everything has a useful default. We can argue whether it was implemented incorrectly. I'm just pointing out the change in behaviour, and that it might require changes elsewhere.

[twmb]

That sounds as if redpanda has a custom yaml decoder that is hooking into the decoding process. How can a program know if null is significant (enable this), vs not present?

Using,

type Foo struct {
    Field *string
}

how can a program know after deserialiation whether field: was in the configuration file, or if it was completely missing?

Requiring null to be significant in pandaproxy makes it impossible to know if the field is completely missing if acting on the configuration after it has been decoded. The only way to support significant nulls is to keep a raw yaml object and then ask if a key is present -- something that I've never seen because of never seeing this use case.

We can look into whether docs needs an update as a part of this.

[twmb]

All pages mentioning pandaproxy::

https://docs.redpanda.com/docs/reference/node-configuration-sample/ <- has full specification of pandaproxy
https://docs.redpanda.com/docs/21.11/cluster-administration/configuration/ <- same (older doc)
https://docs.redpanda.com/docs/data-management/data-transform/ <- already uses {}

All pages referencing schema_registry::

https://docs.redpanda.com/docs/21.11/deployment/kubernetes-additional-config/ <- has additional configuration knob
https://docs.redpanda.com/docs/21.11/cluster-administration/configuration/ <- has full specification
https://docs.redpanda.com/docs/reference/node-configuration-sample/ <- same
https://docs.redpanda.com/docs/data-management/data-transform/ <- already uses {}
https://redpanda.com/blog/ably-redpanda-real-time-data-pipeline <- same