Bug 1847921

Didn't get a chance to look into this yet. It's next on my queue.

Created attachment 1698491 [details]
YAML with empty data structures

IMO, this is not a console bug. It was an intentional decision to prune empty data structures when toggling between editors and before submitting. I've attached a yaml file that shows what happens if we don't. (spoiler, you end up with a 400+ line YAML file that only defines 10 properties). I think the root of this bug is actually in the CRD validation schema.

Why it's a schema validation issue:
From what I gather, resource creation is failing validation because spec.kafka.listeners is defined as a required property in the schema. This means that even if the CR doesn't define any kafka listeners, an empty object has to be defined in order to pass validation. If the CRD schema is designed to allow spec.kafka.listeners to be empty, it probably shouldn't require that field. If, on the other hand, spec.kafka.listeners SHOULD be a required property, then at least one of its properties should also be required. If that required property is object, it should have a required property, and repeat recursively until you reach a primitive-type property.

I can't think of a case where an empty object property would need to be required, but that's not to say that it couldn't exist. If this is such a case, then there will need to be some further discussion about how we allow users to define empty data structures from the form view, and how we selectively sync empty data structures between the form and yaml views.

Why the schema issue wasn't caught before:
This behavior is only now manifesting in 4.5 because prior to this release, we landed on the YAML view and did not support nested fields in the form view. So these empty values were populated in the YAML view, but could not be edited in the form view, and therefore remained untouched when switching from YAML to form and back. In 4.5, we land on the form view and have added support for nested fields. So spec.kafka.listeners and all of its nested properties are now editable in the form view. With nested fields, we have to assume that when a user doesn't change anything within an empty or undefined nested structure, it will remain undefined. If instead we automatically assign an empty data structure to that field, then the YAML editor would become filled with empty data structures, which clutters up the YAML view for CRs with large schemas. For example, this is what spec.kafka.listeners looks like in the YAML view if we leave empty object definitions instead of pruning them:

listeners:
  external:
    authentication:
      clientSecret: {}
    configuration:
      bootstrap:
        dnsAnnotations: {}
      brokerCertChainAndKey: {}
    overrides:
      bootstrap:
        dnsAnnotations: {}
  plain:
    authentication:
      clientSecret: {}
  tls:
    authentication:
      clientSecret: {}
    configuration:
      brokerCertChainAndKey: {}

I think you are missing the point. There is a big difference between a defined but empty property and a property which is not defined in the CR YAML / JSON. So having empty listeners is not the same as having listeners with some properties defined but empty. You are right that the CRD definition could maybe use the anyOf keyword for the objects inside listener to indicate that at least one of the properties inside is required. But that would still leave them valid as empty objects - so I guess that would not solve the UI issue. Or would it?

I also do not think we are asking you to generate empty objects for every field in the schema. I think it would be good if the user can specify empty objects from the form (e.g. using some checkbox or something) but mainly the UI should respect the examples which are valid and which worked completely fine with the previous version of OCP.

Jakub, yeah, I totally understand that empty is not the same as undefined. I guess I was wondering if in this specific case those two things actually convey different information. Regardless, I see your point about respecting the example if it is valid. Regardless of whether there is a logical flaw in the schema, if the example is valid, we should respect it. Let me look a little more into this.

Fix is in progress.

Created attachment 1700969 [details]
Verification screenshot - form view

Created attachment 1700970 [details]
Verification screenshot - yaml view

Attached the screenshots, both yaml view and form view have the fields for `spec.kafka.listeners.plain:{}` object.
This could be Verified on 4.6.0-0.nightly-2020-07-14-004201

This is not fixed. When I changed the `alm-example` section of the CSV to use empty objects for the `listeners.plain` and `listeners.tls` these empty objects do not appear in the YAML view for creating the Kafka Operand. Here is the section of the CSV

--
   alm-examples: |-
      [
        {
          "apiVersion":"kafka.strimzi.io/v1beta1",
          "kind":"Kafka",
          "metadata":{
            "name":"my-cluster"
          },
          "spec":{
            "kafka":{
                "version":"2.5.0",
                "replicas":3,
                "listeners":{
                  "plain":{
                  },
                  "tls":{
                  }
                },
--

Tested on 4.6.0-0.nightly-2020-07-14-004201

Created attachment 1700986 [details]
YAML view in which the `spec.listeners.plain:{}` is still missing

Simon, that nightly doesn't have the fix. You need at least 4.6.0-0.nightly-2020-07-14-035247

Thanks, I misinterpreted the fix version - tested on 4.6.0-0.nightly-2020-07-15-031221 and it looks good.

*** Bug 1854152 has been marked as a duplicate of this bug. ***

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (OpenShift Container Platform 4.6 GA Images), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:4196