- Notifications
You must be signed in to change notification settings - Fork 68
Description
using this sample accesslogpolicy
aws-application-networking-k8s/docs/api-types/access-log-policy.md
Lines 37 to 47 in 086bcb0
| ```yaml | |
| apiVersion: application-networking.k8s.aws/v1alpha1 | |
| kind: AccessLogPolicy | |
| metadata: | |
| name: my-access-log-policy | |
| spec: | |
| destinationArn: "arn:aws:logs:us-west-2:123456789012:log-group:myloggroup:*" | |
| targetRef: | |
| group: gateway.networking.k8s.io | |
| kind: HTTPRoute | |
| name: inventory |
results in an NPE
this is because in the controller:
aws-application-networking-k8s/pkg/controllers/accesslogpolicy_controller.go
Lines 177 to 183 in 086bcb0
| targetRefNamespace := k8s.NamespaceOrDefault(alp.Spec.TargetRef.Namespace) | |
| if targetRefNamespace != alp.Namespace { | |
| message := fmt.Sprintf("The targetRef's namespace, \"%s\", does not match the Access Log Policy's"+ | |
| " namespace, \"%s\"", string(*alp.Spec.TargetRef.Namespace), alp.Namespace) | |
| r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, message) | |
| return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonInvalid, message) | |
| } |
namespace is nullchecked, but then the nil value is reused in string(*alp.Spec.TargetRef.Namespace)
I looked at all other objects and it seems that they do not require the operator to specify target ref namespace. I'm wondering why in this accesslogpolicy we require target ref namespace instead of just defaulting it to the access log policy's namespace if it's missing. This is also the only place that uses k8s.NamespaceOrDefault for now.
I would make a contribution to fix this but I'm not sure if the maintainers would prefer
- for this NPE to be fixed by not referencing the nil in the log
- or for the default namespace to be assumed