-
Bug
-
Resolution: Obsolete
-
Normal
-
None
-
4.13, 4.12, 4.11, 4.10, 4.9, 4.8, 4.14, 4.15, 4.16, 4.17
-
Moderate
-
None
-
False
-
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.