diff --git a/controllers/accesslogpolicy_controller.go b/controllers/accesslogpolicy_controller.go index 77bdb5e8..fd5f9bee 100644 --- a/controllers/accesslogpolicy_controller.go +++ b/controllers/accesslogpolicy_controller.go @@ -128,21 +128,6 @@ func (r *accessLogPolicyReconciler) reconcile(ctx context.Context, req ctrl.Requ r.eventRecorder.Event(alp, corev1.EventTypeNormal, k8s.ReconcilingEvent, "Started reconciling") - if alp.Spec.TargetRef.Group != gwv1beta1.GroupName { - message := fmt.Sprintf("The targetRef's Group must be \"%s\" but was \"%s\"", - gwv1beta1.GroupName, alp.Spec.TargetRef.Group) - r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, message) - return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonInvalid, message) - } - - validKinds := []string{"Gateway", "HTTPRoute", "GRPCRoute"} - if !slices.Contains(validKinds, string(alp.Spec.TargetRef.Kind)) { - message := fmt.Sprintf("The targetRef's Kind must be \"Gateway\", \"HTTPRoute\", or \"GRPCRoute\""+ - " but was \"%s\"", alp.Spec.TargetRef.Kind) - r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, message) - return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonInvalid, message) - } - if !alp.DeletionTimestamp.IsZero() { return r.reconcileDelete(ctx, alp) } else { @@ -175,6 +160,21 @@ func (r *accessLogPolicyReconciler) reconcileUpsert(ctx context.Context, alp *an return err } + if alp.Spec.TargetRef.Group != gwv1beta1.GroupName { + message := fmt.Sprintf("The targetRef's Group must be \"%s\" but was \"%s\"", + gwv1beta1.GroupName, alp.Spec.TargetRef.Group) + r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, message) + return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonInvalid, message) + } + + validKinds := []string{"Gateway", "HTTPRoute", "GRPCRoute"} + if !slices.Contains(validKinds, string(alp.Spec.TargetRef.Kind)) { + message := fmt.Sprintf("The targetRef's Kind must be \"Gateway\", \"HTTPRoute\", or \"GRPCRoute\""+ + " but was \"%s\"", alp.Spec.TargetRef.Kind) + r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, message) + return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonInvalid, message) + } + 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"+ diff --git a/pkg/gateway/model_build_access_log_subscription.go b/pkg/gateway/model_build_access_log_subscription.go index 2a8b2bba..71448a44 100644 --- a/pkg/gateway/model_build_access_log_subscription.go +++ b/pkg/gateway/model_build_access_log_subscription.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/aws/aws-sdk-go/aws" "sigs.k8s.io/controller-runtime/pkg/client" anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" @@ -57,26 +58,29 @@ type accessLogSubscriptionModelBuildTask struct { } func (t *accessLogSubscriptionModelBuildTask) run(ctx context.Context) error { + var eventType = core.CreateEvent + if t.accessLogPolicy.DeletionTimestamp != nil { + eventType = core.DeleteEvent + } else if _, ok := t.accessLogPolicy.Annotations[anv1alpha1.AccessLogSubscriptionAnnotationKey]; ok { + eventType = core.UpdateEvent + } + sourceType := model.ServiceSourceType if t.accessLogPolicy.Spec.TargetRef.Kind == "Gateway" { sourceType = model.ServiceNetworkSourceType } sourceName, err := utils.TargetRefToLatticeResourceName(t.accessLogPolicy.Spec.TargetRef, t.accessLogPolicy.Namespace) - if err != nil { + if err != nil && eventType != core.DeleteEvent { return err } destinationArn := t.accessLogPolicy.Spec.DestinationArn if destinationArn == nil { - return fmt.Errorf("access log policy's destinationArn cannot be nil") - } - - var eventType = core.CreateEvent - if t.accessLogPolicy.DeletionTimestamp != nil { - eventType = core.DeleteEvent - } else if _, ok := t.accessLogPolicy.Annotations[anv1alpha1.AccessLogSubscriptionAnnotationKey]; ok { - eventType = core.UpdateEvent + if eventType != core.DeleteEvent { + return fmt.Errorf("access log policy's destinationArn cannot be nil") + } + destinationArn = aws.String("") } var status *model.AccessLogSubscriptionStatus diff --git a/pkg/gateway/model_build_access_log_subscription_test.go b/pkg/gateway/model_build_access_log_subscription_test.go index b6e4f78b..5ca24864 100644 --- a/pkg/gateway/model_build_access_log_subscription_test.go +++ b/pkg/gateway/model_build_access_log_subscription_test.go @@ -276,6 +276,67 @@ func Test_BuildAccessLogSubscription(t *testing.T) { onlyCompareSpecs: true, expectedError: nil, }, + { + description: "Delete event skips targetRef validation", + input: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: apimachineryv1.ObjectMeta{ + Namespace: namespace, + Name: name, + DeletionTimestamp: &apimachineryv1.Time{}, + Annotations: map[string]string{ + anv1alpha1.AccessLogSubscriptionAnnotationKey: "arn:aws:vpc-lattice:us-west-2:123456789012:accesslogsubscription/als-12345678901234567", + }, + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + DestinationArn: aws.String(s3DestinationArn), + TargetRef: &gwv1alpha2.PolicyTargetReference{ + Kind: "Foo", + Name: name, + }, + }, + }, + expectedOutput: &lattice.AccessLogSubscription{ + Spec: lattice.AccessLogSubscriptionSpec{ + SourceType: lattice.ServiceSourceType, + SourceName: "", + DestinationArn: s3DestinationArn, + ALPNamespacedName: expectedNamespacedName, + EventType: core.DeleteEvent, + }, + }, + onlyCompareSpecs: true, + expectedError: nil, + }, + { + description: "Delete event skips destinationArn validation", + input: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: apimachineryv1.ObjectMeta{ + Namespace: namespace, + Name: name, + DeletionTimestamp: &apimachineryv1.Time{}, + Annotations: map[string]string{ + anv1alpha1.AccessLogSubscriptionAnnotationKey: "arn:aws:vpc-lattice:us-west-2:123456789012:accesslogsubscription/als-12345678901234567", + }, + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.PolicyTargetReference{ + Kind: gatewayKind, + Name: name, + }, + }, + }, + expectedOutput: &lattice.AccessLogSubscription{ + Spec: lattice.AccessLogSubscriptionSpec{ + SourceType: lattice.ServiceNetworkSourceType, + SourceName: name, + DestinationArn: "", + ALPNamespacedName: expectedNamespacedName, + EventType: core.DeleteEvent, + }, + }, + onlyCompareSpecs: true, + expectedError: nil, + }, } for _, tt := range tests {