Skip to content

NE-2117: Add httpsLogFormat and tcpLogFormat to API #2773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rohara wants to merge 1 commit into
base: master
Choose a base branch
from
Open

NE-2117: Add httpsLogFormat and tcpLogFormat to API #2773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
| HyperShiftOnlyDynamicResourceAllocation| <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | |
| ImageModeStatusReporting| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
| IngressControllerDynamicConfigurationManager| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
| IngressControllerHTTPSLogFormat| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
| IrreconcilableMachineConfig| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
| KMSEncryption| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
| MachineAPIMigration| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> |
Expand Down
20 changes: 14 additions & 6 deletions features/features.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ var (
mustRegister()

FeatureGateNoOverlayMode = newFeatureGate("NoOverlayMode").
reportProblemsToJiraComponent("Networking/ovn-kubernetes").
contactPerson("pliurh").
productScope(ocpSpecific).
enhancementPR("https://github.com/openshift/enhancements/pull/1859").
enable(inDevPreviewNoUpgrade(), inTechPreviewNoUpgrade()).
mustRegister()
reportProblemsToJiraComponent("Networking/ovn-kubernetes").
Copy link
Copy Markdown
Contributor

@yuqi-zhang yuqi-zhang Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, did something automatically add extra spaces here?

contactPerson("pliurh").
productScope(ocpSpecific).
enhancementPR("https://github.com/openshift/enhancements/pull/1859").
enable(inDevPreviewNoUpgrade(), inTechPreviewNoUpgrade()).
mustRegister()

FeatureGateEVPN = newFeatureGate("EVPN").
reportProblemsToJiraComponent("Networking/ovn-kubernetes").
Expand Down Expand Up @@ -665,6 +665,14 @@ var (
enable(inDevPreviewNoUpgrade(), inTechPreviewNoUpgrade()).
mustRegister()

FeatureGateIngressControllerHTTPSLogFormat = newFeatureGate("IngressControllerHTTPSLogFormat").
reportProblemsToJiraComponent("Networking / router").
contactPerson("rohara").
productScope(ocpSpecific).
enhancementPR("https://github.com/openshift/enhancements/pull/1832").
enable(inDevPreviewNoUpgrade(), inTechPreviewNoUpgrade()).
mustRegister()

FeatureGateMinimumKubeletVersion = newFeatureGate("MinimumKubeletVersion").
reportProblemsToJiraComponent("Node").
contactPerson("haircommander").
Expand Down
16 changes: 15 additions & 1 deletion openapi/generated_openapi/zz_generated.openapi.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 67 additions & 0 deletions ...or/v1/tests/ingresscontrollers.operator.openshift.io/IngressControllerHTTPSLogFormat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "IngressController"
crdName: ingresscontrollers.operator.openshift.io
featureGates:
- IngressControllerHTTPSLogFormat
tests:
onCreate:
- name: Should be able to create a minimal ingresscontroller with tcpLogFormat
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
logging:
access:
destination:
type: Container
logEmptyRequests: "Ignore"
tcpLogFormat: "%ci:%cp [%t] %ft %b/%s %Tw/%Tc/%Tt %B %ts"
expected: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
httpEmptyRequestsPolicy: Respond
idleConnectionTerminationPolicy: Immediate
closedClientConnectionPolicy: Continue
logging:
access:
destination:
type: Container
logEmptyRequests: "Ignore"
tcpLogFormat: "%ci:%cp [%t] %ft %b/%s %Tw/%Tc/%Tt %B %ts"
- name: Should be able to create a minimal ingresscontroller with httpsLogFormat
initial: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
logging:
access:
destination:
type: Container
logEmptyRequests: "Ignore"
httpsLogFormat: "%ci:%cp [%tr] %ft %b/%s %TR/%Tw/%Tc/%Tr/%Ta %ST %B %CC"
expected: |
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
httpEmptyRequestsPolicy: Respond
idleConnectionTerminationPolicy: Immediate
closedClientConnectionPolicy: Continue
logging:
access:
destination:
type: Container
logEmptyRequests: "Ignore"
httpsLogFormat: "%ci:%cp [%tr] %ft %b/%s %TR/%Tw/%Tc/%Tr/%Ta %ST %B %CC"
44 changes: 43 additions & 1 deletion operator/v1/types_ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1495,13 +1495,35 @@ type AccessLogging struct {
// +required
Destination LoggingDestination `json:"destination"`

// tcpLogFormat specifies the format of the log message for a TCP
// connection.
//
// If this field is empty, log messages use the implementation's default
// TCP log format. When set, the value must be between 1 and 1024
// characters long.For HAProxy's default TCP log format, see the
Copy link
Copy Markdown
Contributor

@yuqi-zhang yuqi-zhang Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few inconsistencies in the godoc (no space between long.For, some extra whitespace, etc.). Could you clean that up?

// HAProxy documentation:
// http://docs.haproxy.org/2.8/configuration.html#8.2.2
//
// Note that this log message is used for the initial TCP connection
// associated with a TLS session. Note that for edge-terminated and
// reencrypt routes, you may see a second log message for the HTTP
// request in addition to the log message for the TCP connection. Use
// the httpsLogFormat option to customize the log format for the HTTPS
// request.
//
// +kubebuilder:validation:MinLength=1
Copy link
Copy Markdown
Contributor

@yuqi-zhang yuqi-zhang Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For validations such as these, please have their corresponding requirements explicit in the godoc (example: // When set, the value must be between 1 and 1024 characters long.)

// +kubebuilder:validation:MaxLength=1024
// +openshift:enable:FeatureGate=IngressControllerHTTPSLogFormat
// +optional
TcpLogFormat string `json:"tcpLogFormat,omitempty"`

// httpLogFormat specifies the format of the log message for an HTTP
// request.
//
// If this field is empty, log messages use the implementation's default
// HTTP log format. For HAProxy's default HTTP log format, see the
// HAProxy documentation:
// http://cbonte.github.io/haproxy-dconv/2.0/configuration.html#8.2.3
// http://docs.haproxy.org/2.8/configuration.html#8.2.3
//
// Note that this format only applies to cleartext HTTP connections
// and to secure HTTP connections for which the ingress controller
Expand All @@ -1512,6 +1534,26 @@ type AccessLogging struct {
// +optional
HttpLogFormat string `json:"httpLogFormat,omitempty"`

// httpsLogFormat specifies the format of the log message for an HTTPS
// request.
//
// If this field is empty, log messages use the implementation's default
// HTTPS log format. When set, the value must be between 1 and 1024
// characters long.For HAProxy's default HTTPS log format, see the
Copy link
Copy Markdown
Contributor

@yuqi-zhang yuqi-zhang Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same spacing inconsistency as above

// HAProxy documentation:
// http://docs.haproxy.org/2.8/configuration.html#8.2.4
//
// Note that this format only applies to HTTPS connections for which the
// ingress controller terminates encryption (that is, edge-terminated or
// reencrypt connections). It does not affect the log format for TLS
// passthrough connections.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=1024
// +openshift:enable:FeatureGate=IngressControllerHTTPSLogFormat
// +optional
HttpsLogFormat string `json:"httpsLogFormat,omitempty"`
Copy link
Copy Markdown
Contributor

@yuqi-zhang yuqi-zhang Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, this should be HTTPSLogFormat (and TcpLogFormat should be TCPLogFormat) based on naming conventions, which generally is followed in this file, except for some reason the existing HttpLogFormat field.

I think the best behaviour is probably make the new fields formatted correct, even if it's not "consistent"


// httpCaptureHeaders defines HTTP headers that should be captured in
// access logs. If this field is empty, no headers are captured.
//
Expand Down
40 changes: 39 additions & 1 deletion ...zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1661,14 +1661,32 @@ spec:
If this field is empty, log messages use the implementation's default
HTTP log format. For HAProxy's default HTTP log format, see the
HAProxy documentation:
http://cbonte.github.io/haproxy-dconv/2.0/configuration.html#8.2.3
http://docs.haproxy.org/2.8/configuration.html#8.2.3

Note that this format only applies to cleartext HTTP connections
and to secure HTTP connections for which the ingress controller
terminates encryption (that is, edge-terminated or reencrypt
connections). It does not affect the log format for TLS passthrough
connections.
type: string
httpsLogFormat:
description: |-
httpsLogFormat specifies the format of the log message for an HTTPS
request.

If this field is empty, log messages use the implementation's default
HTTPS log format. When set, the value must be between 1 and 1024
characters long.For HAProxy's default HTTPS log format, see the
HAProxy documentation:
http://docs.haproxy.org/2.8/configuration.html#8.2.4

Note that this format only applies to HTTPS connections for which the
ingress controller terminates encryption (that is, edge-terminated or
reencrypt connections). It does not affect the log format for TLS
passthrough connections.
maxLength: 1024
minLength: 1
type: string
logEmptyRequests:
default: Log
description: |-
Expand All @@ -1686,6 +1704,26 @@ spec:
- Log
- Ignore
type: string
tcpLogFormat:
description: |-
tcpLogFormat specifies the format of the log message for a TCP
connection.

If this field is empty, log messages use the implementation's default
TCP log format. When set, the value must be between 1 and 1024
characters long.For HAProxy's default TCP log format, see the
HAProxy documentation:
http://docs.haproxy.org/2.8/configuration.html#8.2.2

Note that this log message is used for the initial TCP connection
associated with a TLS session. Note that for edge-terminated and
reencrypt routes, you may see a second log message for the HTTP
request in addition to the log message for the TCP connection. Use
the httpsLogFormat option to customize the log format for the HTTPS
request.
maxLength: 1024
minLength: 1
type: string
required:
- destination
type: object
Expand Down
2 changes: 1 addition & 1 deletion ...ator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-Default.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,7 @@ spec:
If this field is empty, log messages use the implementation's default
HTTP log format. For HAProxy's default HTTP log format, see the
HAProxy documentation:
http://cbonte.github.io/haproxy-dconv/2.0/configuration.html#8.2.3
http://docs.haproxy.org/2.8/configuration.html#8.2.3

Note that this format only applies to cleartext HTTP connections
and to secure HTTP connections for which the ingress controller
Expand Down
40 changes: 39 additions & 1 deletion ...enerated.crd-manifests/0000_50_ingress_00_ingresscontrollers-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1661,14 +1661,32 @@ spec:
If this field is empty, log messages use the implementation's default
HTTP log format. For HAProxy's default HTTP log format, see the
HAProxy documentation:
http://cbonte.github.io/haproxy-dconv/2.0/configuration.html#8.2.3
http://docs.haproxy.org/2.8/configuration.html#8.2.3

Note that this format only applies to cleartext HTTP connections
and to secure HTTP connections for which the ingress controller
terminates encryption (that is, edge-terminated or reencrypt
connections). It does not affect the log format for TLS passthrough
connections.
type: string
httpsLogFormat:
description: |-
httpsLogFormat specifies the format of the log message for an HTTPS
request.

If this field is empty, log messages use the implementation's default
HTTPS log format. When set, the value must be between 1 and 1024
characters long.For HAProxy's default HTTPS log format, see the
HAProxy documentation:
http://docs.haproxy.org/2.8/configuration.html#8.2.4

Note that this format only applies to HTTPS connections for which the
ingress controller terminates encryption (that is, edge-terminated or
reencrypt connections). It does not affect the log format for TLS
passthrough connections.
maxLength: 1024
minLength: 1
type: string
logEmptyRequests:
default: Log
description: |-
Expand All @@ -1686,6 +1704,26 @@ spec:
- Log
- Ignore
type: string
tcpLogFormat:
description: |-
tcpLogFormat specifies the format of the log message for a TCP
connection.

If this field is empty, log messages use the implementation's default
TCP log format. When set, the value must be between 1 and 1024
characters long.For HAProxy's default TCP log format, see the
HAProxy documentation:
http://docs.haproxy.org/2.8/configuration.html#8.2.2

Note that this log message is used for the initial TCP connection
associated with a TLS session. Note that for edge-terminated and
reencrypt routes, you may see a second log message for the HTTP
request in addition to the log message for the TCP connection. Use
the httpsLogFormat option to customize the log format for the HTTPS
request.
maxLength: 1024
minLength: 1
type: string
required:
- destination
type: object
Expand Down
2 changes: 1 addition & 1 deletion operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers-OKD.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,7 @@ spec:
If this field is empty, log messages use the implementation's default
HTTP log format. For HAProxy's default HTTP log format, see the
HAProxy documentation:
http://cbonte.github.io/haproxy-dconv/2.0/configuration.html#8.2.3
http://docs.haproxy.org/2.8/configuration.html#8.2.3

Note that this format only applies to cleartext HTTP connections
and to secure HTTP connections for which the ingress controller
Expand Down
Loading