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
+152 −5
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 @@ -64,6 +64,7 @@
| GCPCustomAPIEndpointsInstall| | | <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> |
| GCPDualStackInstall| | | <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> |
| GatewayAPIWithoutOLM| | | <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> |
| HTTPSLogFormat| | | <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> |
| 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> |
Expand Down
8 changes: 8 additions & 0 deletions features/features.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1020,4 +1020,12 @@ var (
enhancementPR("https://github.com/openshift/enhancements/pull/1910").
enable(inDevPreviewNoUpgrade(), inTechPreviewNoUpgrade()).
mustRegister()

FeatureGateHTTPSLogFormat = newFeatureGate("HTTPSLogFormat").
reportProblemsToJiraComponent("Networking/router").
contactPerson("rohara").
productScope(ocpSpecific).
enhancementPR("https://github.com/openshift/enhancements/pull/1832").
enable(inDevPreviewNoUpgrade(), inTechPreviewNoUpgrade()).
mustRegister()
Comment on lines +1024 to +1030
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

types_file="$(fd '^types_ingress\.go$' | head -n1)"
if [[ -z "${types_file}" ]]; then
  echo "types_ingress.go not found" >&2
  exit 1
fi

echo "== AccessLogging definition from ${types_file} =="
python - "$types_file" <<'PY'
from pathlib import Path
import re, sys

text = Path(sys.argv[1]).read_text()
match = re.search(r'type\s+AccessLogging\s+struct\s*\{.*?\n\}', text, re.S)
if not match:
    raise SystemExit("AccessLogging struct not found")
print(match.group(0))
PY

echo
echo "== Related feature-gate annotations and log-format fields =="
rg -n -C2 'HTTPSLogFormat|httpsLogFormat|tcpLogFormat|\+openshift:.*FeatureGate' "$types_file"

Repository: openshift/api

Length of output: 167

🏁 Script executed:

# Find where AccessLogging is defined
rg -n 'type\s+AccessLogging' --type go

# Also search for the feature gate in context
rg -n 'HTTPSLogFormat' --type go

# Look for any ingress-related API types files
fd 'types.*ingress' --type f

Repository: openshift/api

Length of output: 263

🏁 Script executed:

# Get the AccessLogging struct definition and surrounding context
sed -n '1490,1550p' operator/v1/types_ingress.go

Repository: openshift/api

Length of output: 2592

🏁 Script executed:

# Search for FeatureGate annotations in the ingress types file
rg -n 'FeatureGate' operator/v1/types_ingress.go -B2 -A2

Repository: openshift/api

Length of output: 39

🏁 Script executed:

# Search for FeatureGate pattern across operator/v1/
rg -n 'FeatureGate' operator/v1/ -B1 -A1 | head -50

# Also check if there are any gated fields using a different pattern
rg -n '\+openshift:' operator/v1/types_ingress.go | head -20

Repository: openshift/api

Length of output: 3591

🏁 Script executed:

# Find the Ingress CRD in generated manifests
rg -n 'name: ingress' operator/v1/zz_generated.featuregated-crd-manifests.yaml -A 50 | head -100

# Also search for any field-level feature gate examples
rg -n '\+openshift.*FeatureGate' . --type go -B2 -A2 | head -60

Repository: openshift/api

Length of output: 6179

🏁 Script executed:

# Check type-level annotations for AccessLogging
sed -n '1488,1495p' operator/v1/types_ingress.go

Repository: openshift/api

Length of output: 257

Add feature gate annotation to the new log-format fields.

The HTTPSLogFormat gate is registered in features.go, but the corresponding fields in the AccessLogging struct lack the required field-level annotation. Without +openshift:enable:FeatureGate=HTTPSLogFormat markers on TcpLogFormat, HttpLogFormat, and HttpsLogFormat fields in operator/v1/types_ingress.go, the API will accept these fields regardless of the gate status.

Add the annotation to each field:

// +openshift:enable:FeatureGate=HTTPSLogFormat
// +optional
TcpLogFormat string `json:"tcpLogFormat,omitempty"`

(Similarly for HttpLogFormat and HttpsLogFormat.)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@features/features.go` around lines 1024 - 1030, Add the feature-gate field
annotation to the AccessLogging struct fields so the fields are only accepted
when the FeatureGateHTTPSLogFormat gate is enabled: add the comment line
"+openshift:enable:FeatureGate=HTTPSLogFormat" immediately above the
TcpLogFormat, HttpLogFormat, and HttpsLogFormat field declarations (keeping the
existing "+optional" and the json `omitempty` tags intact) in the AccessLogging
type so the API server enforces the HTTPSLogFormat gate defined by
FeatureGateHTTPSLogFormat.

)
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.

36 changes: 35 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,29 @@ type AccessLogging struct {
// +required
Destination LoggingDestination `json:"destination"`

// tcpLogFormat specifies the format of the log message for a TCP
// request.
//
// If this field is empty, log messages use the implementation's default
// TCP log format. For HAProxy's default TCP log format, see the
// HAProxy documentation:
// http//docs.haproxy.org/2.8/configuration.html#8.2.2
Copy link

@coderabbitai coderabbitai bot Mar 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix broken HAProxy doc links and typo in field docs.

Line 1504 and Line 1520 use http//... (missing :), and Line 1531 has messsage typo. These will propagate into generated API docs.

Suggested doc fix 
-	// http//docs.haproxy.org/2.8/configuration.html#8.2.2
+	// http://docs.haproxy.org/2.8/configuration.html#8.2.2
...
-	// http//docs.haproxy.org/2.8/configuration.html#8.2.3
+	// http://docs.haproxy.org/2.8/configuration.html#8.2.3
...
-	// httpsLogFormat specifies the format of the log messsage for an HTTPS
+	// httpsLogFormat specifies the format of the log message for an HTTPS

Also applies to: 1520-1520, 1531-1531

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@operator/v1/types_ingress.go` at line 1504, Fix the broken HAProxy doc links
and the typo in the field docs by replacing the malformed
"http//docs.haproxy.org/2.8/configuration.html#8.2.2" occurrences with
"http://docs.haproxy.org/2.8/configuration.html#8.2.2" and correct the
misspelling "messsage" to "message" wherever they appear in the ingress type
comments (search for the literal strings "http//docs.haproxy.org" and "messsage"
in operator/v1/types_ingress.go to locate the affected struct field docs and
update them).

//
// Note that this format only applies to TCP connections. It only affects
// the log format for TLS passthrough connections.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=1024
// +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 +1528,24 @@ type AccessLogging struct {
// +optional
HttpLogFormat string `json:"httpLogFormat,omitempty"`

// httpsLogFormat specifies the format of the log messsage for an HTTPS
// request.
//
// If this field is empty, log messages use the implementation's default
// HTTPS log format. 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.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=1024
// +optional
HttpsLogFormat string `json:"httpsLogFormat,omitempty"`

// httpCaptureHeaders defines HTTP headers that should be captured in
// access logs. If this field is empty, no headers are captured.
//
Expand Down
34 changes: 33 additions & 1 deletion operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,14 +1660,31 @@ 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 messsage for an HTTPS
request.

If this field is empty, log messages use the implementation's default
HTTPS log format. 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 @@ -1685,6 +1702,21 @@ spec:
- Log
- Ignore
type: string
tcpLogFormat:
description: |-
tcpLogFormat specifies the format of the log message for a TCP
request.

If this field is empty, log messages use the implementation's default
TCP log format. 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 format only applies to TCP connections. It only affects
the log format for TLS passthrough connections.
maxLength: 1024
minLength: 1
type: string
required:
- destination
type: object
Expand Down
34 changes: 33 additions & 1 deletion ...ated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1654,14 +1654,31 @@ 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 messsage for an HTTPS
request.

If this field is empty, log messages use the implementation's default
HTTPS log format. 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 @@ -1679,6 +1696,21 @@ spec:
- Log
- Ignore
type: string
tcpLogFormat:
description: |-
tcpLogFormat specifies the format of the log message for a TCP
request.

If this field is empty, log messages use the implementation's default
TCP log format. 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 format only applies to TCP connections. It only affects
the log format for TLS passthrough connections.
maxLength: 1024
minLength: 1
type: string
required:
- destination
type: object
Expand Down
4 changes: 3 additions & 1 deletion operator/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

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

3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "ImageModeStatusReporting"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-Hypershift-DevPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "HighlyAvailableArbiter"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "ImageModeStatusReporting"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-Hypershift-TechPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "HighlyAvailableArbiter"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "HyperShiftOnlyDynamicResourceAllocation"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "HighlyAvailableArbiter"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "HyperShiftOnlyDynamicResourceAllocation"
},
Expand Down
3 changes: 3 additions & 0 deletions payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-TechPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@
{
"name": "GatewayAPIWithoutOLM"
},
{
"name": "HTTPSLogFormat"
},
{
"name": "HighlyAvailableArbiter"
},
Expand Down