Uploaded image for project: 'OpenShift Bugs'
  1. OpenShift Bugs
  2. OCPBUGS-35767

Missing validation on network config's spec.defaultNetwork.ovnKubernetesConfig.policyAuditConfig.destination field

XMLWordPrintable

    • Moderate
    • None
    • False
    • Hide

      None

      Show
      None

      Description of problem

      The PolicyAuditConfig definition has an incorrect kubebuilder tag on the Destination field:

      	// +kubebuilder:pattern='^libc$|^null$|^udp:(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]):([0-9]){0,5}$|^unix:(\/[^\/ ]*)+([^\/\s])$'
      

      Instead of kubebuilder:pattern, it should have kubebuilder:validation:Pattern.

      With the incorrect kubebuilder tag, the generated CRD yaml manifest is missing the intended validation:

                                destination:
                                  default: "null"
                                  description: 'destination is the location for policy log
                                    messages. Regardless of this config, persistent logs
                                    will always be dumped to the host at /var/log/ovn/ however
                                    Additionally syslog output may be configured as follows.
                                    Valid values are: - "libc" -> to use the libc syslog()
                                    function of the host node''s journdald process - "udp:host:port"
                                    -> for sending syslog over UDP - "unix:file" -> for
                                    using the UNIX domain socket directly - "null" -> to
                                    discard all messages logged to syslog The default is
                                    "null"'
                                  type: string
      

      Note that there is no pattern: stanza in the yaml.

      Version-Release number of selected component (if applicable)

      The erroneous kubebuilder tag was introduced in OpenShift 4.8.

      How reproducible

      100%.

      Steps to Reproduce

      1. Compare the PolicyAuditConfig field in the Go definition at https://github.com/openshift/api/blob/master/operator/v1/types_network.go with the destination field in the generated CRD yaml manifest at https://github.com/openshift/api/blob/master/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks.crd.yaml.

      Actual results

      The Go definition defines a pattern that is missing from the yaml definition.

      Expected results

      The Go definition and yaml should match.

      Additional info

      I noticed the erroneous kubebuilder tag while looking for something unrelated in the API definitions. I don't know whether it would be better to fix the validation, or whether that would be a breaking API change, in which case it might be better to remove the illusory validation.

      Affected Platforms

      As far as I know, this is not causing any CI failures or customer issues, but it seems worth calling out as the Go definition is misleading, and the lack of validation could increase the risk of customer issues resulting from configuring invalid values. I won't be offended if you decide to close this issue as "Won't fix". However, if you decide not to fix the validation, I'd suggest at least removing the erroneous kubebuilder tag, or adding a // + developer comment that we know this validation isn't really enforced, lest someone be misled by the Go definition.

              mkennell@redhat.com Martin Kennelly
              mmasters1@redhat.com Miciah Masters
              Anurag Saxena Anurag Saxena
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Created:
                Updated:
                Resolved: