EnvoyGateway crashes (nil pointer) due to inappropriate AccessLog settings

[ardikabs]

Description:
Envoy Gateway crashes (nil pointer) when defining EnvoyProxy with AccessLog enabled using the File sink type. It happened when we didn't provide the file content under spec.telemetry.accessLog.settings.sinks.

Repro steps:
Apply below EnvoyProxy manifest:

apiVersion: config.gateway.envoyproxy.io/v1alpha1
kind: EnvoyProxy
metadata:
  name: default
  namespace: envoy-gateway-system
spec:
  telemetry:
    accessLog:
      settings:
      - format:
          json:
            host: '%REQ(:AUTHORITY)%'
          type: JSON
        sinks:
        - type: File

Environment:
Envoy Gateway v0.5.0 and latest

Logs:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1d668af]

goroutine 72 [running]:
github.com/envoyproxy/gateway/internal/gatewayapi.processAccessLog(0xc001218780)
        /home/runner/work/gateway/gateway/internal/gatewayapi/listener.go:193 +0x28f
github.com/envoyproxy/gateway/internal/gatewayapi.(*Translator).ProcessListeners(0xc0008a1bf0?, {0xc0000128c8?, 0x1, 0x1}, 0x3c00900?, 0x55?, 0xc000bb1500)
        /home/runner/work/gateway/gateway/internal/gatewayapi/listener.go:52 +0x5fe
github.com/envoyproxy/gateway/internal/gatewayapi.(*Translator).Translate(0xc000083fc0?, 0xc000bb1500)
        /home/runner/work/gateway/gateway/internal/gatewayapi/translator.go:121 +0xbd
github.com/envoyproxy/gateway/internal/gatewayapi/runner.(*Runner).subscribeAndTranslate.func1({{0xc000080e40?, 0x16?}, 0x0?, 0xc000bb1500?})
        /home/runner/work/gateway/gateway/internal/gatewayapi/runner/runner.go:78 +0x16e
github.com/envoyproxy/gateway/internal/message.HandleSubscription[...](0x2769c40, 0xc0008a1f98?)
        /home/runner/work/gateway/gateway/internal/message/watchutil.go:36 +0x148
github.com/envoyproxy/gateway/internal/gatewayapi/runner.(*Runner).subscribeAndTranslate(0xc0000f5280, {0x2749548?, 0xc00013c5f0?})
        /home/runner/work/gateway/gateway/internal/gatewayapi/runner/runner.go:52 +0x75
created by github.com/envoyproxy/gateway/internal/gatewayapi/runner.(*Runner).Start
        /home/runner/work/gateway/gateway/internal/gatewayapi/runner/runner.go:46 +0x272

[ardikabs]

It seems to me, that the code lacks of nil pointer check before proceeding with the access log file type in lines 177 - 178. Thus when file is not provided, both 182 and 193 will face nil pointer panic.

gateway/internal/gatewayapi/listener.go

Lines 177 to 196 in 3999cdd

	case configv1a1.ProxyAccessLogSinkTypeFile:
	switch accessLog.Format.Type {
	case configv1a1.ProxyAccessLogFormatTypeText:
	al := &ir.TextAccessLog{
	Format: accessLog.Format.Text,
	Path: sink.File.Path,
	}
	irAccessLog.Text = append(irAccessLog.Text, al)
	case configv1a1.ProxyAccessLogFormatTypeJSON:
	if len(accessLog.Format.JSON) == 0 {
	// TODO: use a default JSON format if not specified?
	continue
	}
	al := &ir.JSONAccessLog{
	JSON: accessLog.Format.JSON,
	Path: sink.File.Path,
	}
	irAccessLog.JSON = append(irAccessLog.JSON, al)
	}

[zirain]

I recall all of these check should be part of validation check, maybe I just miss implement it.

[ardikabs]

hi @zirain , interested to pick this one up. However need some clarity, about the validation check you refer, was that should be part of openapi schema or validation in the code level?