Skip to content

Conversation

@j-vizcaino
Copy link
Contributor

What type of PR is this?

bug

Which issue does this PR fix:

Fixes #626

What does this PR do / Why do we need it:

When namespace is not specified in an AccessLogPolicy the controller panics instead of logging a comprehensive error message.

If an issue # is not available please add repro steps and logs from aws-gateway-controller showing the issue:

  • Create an AccessLogPolicy that refs an existing Gateway or HTTPRoute, but doesn't provide the namespace field in the targetRef
  • Controller panics with
{"level":"error","ts":"2025-05-21T13:15:30.212Z","logger":"runtime","caller":"controllers/accesslogpolicy_controller.go:185","msg":"Observed a panic","controller":"accesslogpolicy","controllerGroup":"application-networking.k8s.aws","controllerKind":"AccessLogPolicy","AccessLogPolicy":{"name":"test","namespace":"test"},"namespace":"test","name":"test","reconcileID":"0560da4f-34ea-45a9-a89e-23ffeb2441cf","panic":"runtime error: invalid memory address or nil pointer dereference","panicGoValue":"\"invalid memory address or nil pointer dereference\"","stacktrace":"goroutine 721 [running[]:\nk8s.io/apimachinery/pkg/util/runtime.logPanic({0x21fd168, 0xc0007a2db0}, {0x1b70b80, 0x316b6e0})\n\t/go/pkg/mod/k8s.io/apimachinery@v0.31.1/pkg/util/runtime/runtime.go:107 +0xbc\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).Reconcile.func1()\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:105 +0x112\npanic({0x1b70b80?, 0x316b6e0?})\n\t/usr/local/go/src/runtime/panic.go:785 +0x132\ngithub.com/aws/aws-application-networking-k8s/pkg/controllers.(*accessLogPolicyReconciler).reconcileUpsert(0xc00089f300, {0x21fd168, 0xc0007a2e10}, 0xc0009d9040)\n\t/workspace/pkg/controllers/accesslogpolicy_controller.go:185 +0x452\ngithub.com/aws/aws-application-networking-k8s/pkg/controllers.(*accessLogPolicyReconciler).reconcile(0xc00089f300, {0x21fd168, 0xc0007a2e10}, {{{0xc000847730?, 0xf?}, {0xc000847728?, 0x8?}}})\n\t/workspace/pkg/controllers/accesslogpolicy_controller.go:138 +0x1eb\ngithub.com/aws/aws-application-networking-k8s/pkg/controllers.(*accessLogPolicyReconciler).Reconcile(0xc00089f300, {0x21fd168?, 0xc0007a2db0?}, {{{0xc000847730?, 0x1df74e6?}, {0xc000847728?, 0x100?}}})\n\t/workspace/pkg/controllers/accesslogpolicy_controller.go:112 +0x12d\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).Reconcile(0xc0007a2d20?, {0x21fd168?, 0xc0007a2db0?}, {{{0xc000847730?, 0x0?}, {0xc000847728?, 0x0?}}})\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:116 +0xbf\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).reconcileHandler(0x220c500, {0x21fd1a0, 0xc000789c20}, {{{0xc000847730, 0x8}, {0xc000847728, 0x8}}})\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:303 +0x3a5\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).processNextWorkItem(0x220c500, {0x21fd1a0, 0xc000789c20})\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:263 +0x20e\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).Start.func2.2()\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:224 +0x85\ncreated by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2 in goroutine 254\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:220 +0x490\n"}

Or with a more readable backtrace

goroutine 721 [running[]:
k8s.io/apimachinery/pkg/util/runtime.logPanic({0x21fd168, 0xc0007a2db0}, {0x1b70b80, 0x316b6e0})
	/go/pkg/mod/k8s.io/apimachinery@v0.31.1/pkg/util/runtime/runtime.go:107 +0xbc
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).Reconcile.func1()
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:105 +0x112
panic({0x1b70b80?, 0x316b6e0?})
	/usr/local/go/src/runtime/panic.go:785 +0x132
github.com/aws/aws-application-networking-k8s/pkg/controllers.(*accessLogPolicyReconciler).reconcileUpsert(0xc00089f300, {0x21fd168, 0xc0007a2e10}, 0xc0009d9040)
	/workspace/pkg/controllers/accesslogpolicy_controller.go:185 +0x452
github.com/aws/aws-application-networking-k8s/pkg/controllers.(*accessLogPolicyReconciler).reconcile(0xc00089f300, {0x21fd168, 0xc0007a2e10}, {{{0xc000847730?, 0xf?}, {0xc000847728?, 0x8?}}})
	/workspace/pkg/controllers/accesslogpolicy_controller.go:138 +0x1eb
github.com/aws/aws-application-networking-k8s/pkg/controllers.(*accessLogPolicyReconciler).Reconcile(0xc00089f300, {0x21fd168?, 0xc0007a2db0?}, {{{0xc000847730?, 0x1df74e6?}, {0xc000847728?, 0x100?}}})
	/workspace/pkg/controllers/accesslogpolicy_controller.go:112 +0x12d
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).Reconcile(0xc0007a2d20?, {0x21fd168?, 0xc0007a2db0?}, {{{0xc000847730?, 0x0?}, {0xc000847728?, 0x0?}}})
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:116 +0xbf
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).reconcileHandler(0x220c500, {0x21fd1a0, 0xc000789c20}, {{{0xc000847730, 0x8}, {0xc000847728, 0x8}}})
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:303 +0x3a5
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).processNextWorkItem(0x220c500, {0x21fd1a0, 0xc000789c20})
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:263 +0x20e
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...[]).Start.func2.2()
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:224 +0x85
created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller[...]).Start.func2 in goroutine 254
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.19.1/pkg/internal/controller/controller.go:220 +0x490

Will this PR introduce any new dependencies?:

No

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

No

Does this PR introduce any user-facing change?:

Fixes panic when AccessLogPolicy doesn't specify a namespace in targetRef

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@ghost ghost enabled auto-merge May 23, 2025 21:01
@ghost ghost added this pull request to the merge queue May 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 23, 2025
@ghost ghost enabled auto-merge May 23, 2025 22:00
@ghost ghost added this pull request to the merge queue May 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 23, 2025
@ghost ghost enabled auto-merge May 24, 2025 01:26
@ghost ghost added this pull request to the merge queue May 24, 2025
Merged via the queue into aws:main with commit 4a816e5 May 24, 2025
2 checks passed
@j-vizcaino j-vizcaino deleted the fix-acces-log-ns-panic branch May 24, 2025 20:29
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using example AccessLogPolicy in readme results in nil pointer exception

1 participant