AccessLogSubscriptionSynthesizer returns 1 error instead of debug log…

…ging each error + test cleanup
aws · Oct 13, 2023 · 5f422ae · 5f422ae
1 parent 32784fb
commit 5f422ae
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 48 deletions.
diff --git a/pkg/deploy/lattice/access_log_subscription_synthesizer.go b/pkg/deploy/lattice/access_log_subscription_synthesizer.go
@@ -2,6 +2,8 @@ package lattice
 
 import (
 	"context"
+	"fmt"
+	"strings"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -38,22 +40,26 @@ func (s *accessLogSubscriptionSynthesizer) Synthesize(ctx context.Context) error
 		return err
 	}
 
-	var ret error = nil
+	var errs []string
 	for _, als := range accessLogSubscriptions {
 		if !als.Spec.IsDeleted {
 			s.log.Debugf("Started creating or updating access log subscription %s", als.ID())
 			_, err := s.accessLogSubscriptionManager.Create(ctx, als)
 			if err != nil {
-				s.log.Debugf("Synthesizing access log subscription %s failed due to %s", als.ID(), err)
-				ret = err
+				errMsg := fmt.Sprintf("synthesizing access log subscription %s failed due to: %s", als.ID(), err)
+				errs = append(errs, errMsg)
 			}
 		} else {
 			s.log.Debugf("Started deleting access log subscription %s", als.ID())
 			// TODO
 		}
 	}
 
-	return ret
+	if len(errs) > 0 {
+		return fmt.Errorf(strings.Join(errs, "; "))
+	}
+
+	return nil
 }
 
 func (s *accessLogSubscriptionSynthesizer) PostSynthesize(ctx context.Context) error {

diff --git a/pkg/deploy/lattice/access_log_subscription_synthesizer_test.go b/pkg/deploy/lattice/access_log_subscription_synthesizer_test.go
@@ -17,57 +17,65 @@ import (
 	"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
 )
 
-func Test_Synthesize_AccessLogSubscriptionSpecIsNotDeleted_CreatesAccessLogSubscription(t *testing.T) {
-	ctx := context.TODO()
-	c := gomock.NewController(t)
-	defer c.Finish()
-	mockManager := NewMockAccessLogSubscriptionManager(c)
-	k8sClient := mockclient.NewMockClient(c)
-	builder := gateway.NewAccessLogSubscriptionModelBuilder(gwlog.FallbackLogger, k8sClient)
-	input := &anv1alpha1.AccessLogPolicy{
-		Spec: anv1alpha1.AccessLogPolicySpec{
-			DestinationArn: aws.String(s3DestinationArn),
-			TargetRef: &v1alpha2.PolicyTargetReference{
-				Kind: "Gateway",
-				Name: "TestName",
-			},
-		},
+func TestSynthesizeAccessLogSubscription(t *testing.T) {
+	setup := func() (
+		context.Context,
+		*gomock.Controller,
+		*MockAccessLogSubscriptionManager,
+		*mockclient.MockClient,
+		gateway.AccessLogSubscriptionModelBuilder,
+	) {
+		ctx := context.TODO()
+		c := gomock.NewController(t)
+		mockManager := NewMockAccessLogSubscriptionManager(c)
+		k8sClient := mockclient.NewMockClient(c)
+		builder := gateway.NewAccessLogSubscriptionModelBuilder(gwlog.FallbackLogger, k8sClient)
+		return ctx, c, mockManager, k8sClient, builder
 	}
 
-	stack, accessLogSubscription, _ := builder.Build(context.Background(), input)
+	t.Run("SpecIsNotDeleted_CreatesAccessLogSubscription", func(t *testing.T) {
+		ctx, c, mockManager, k8sClient, builder := setup()
+		defer c.Finish()
+		input := &anv1alpha1.AccessLogPolicy{
+			Spec: anv1alpha1.AccessLogPolicySpec{
+				DestinationArn: aws.String(s3DestinationArn),
+				TargetRef: &v1alpha2.PolicyTargetReference{
+					Kind: "Gateway",
+					Name: "TestName",
+				},
+			},
+		}
 
-	k8sClient.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-	mockManager.EXPECT().Create(ctx, accessLogSubscription).Return(&lattice.AccessLogSubscriptionStatus{}, nil).AnyTimes()
+		stack, accessLogSubscription, _ := builder.Build(context.Background(), input)
 
-	synthesizer := NewAccessLogSubscriptionSynthesizer(gwlog.FallbackLogger, k8sClient, mockManager, stack)
-	err := synthesizer.Synthesize(ctx)
-	assert.Nil(t, err)
-}
+		k8sClient.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
+		mockManager.EXPECT().Create(ctx, accessLogSubscription).Return(&lattice.AccessLogSubscriptionStatus{}, nil).AnyTimes()
 
-func Test_Synthesize_AccessLogSubscriptionSpecIsNotDeletedButErrorOccurs_ReturnsError(t *testing.T) {
-	ctx := context.TODO()
-	c := gomock.NewController(t)
-	defer c.Finish()
-	mockManager := NewMockAccessLogSubscriptionManager(c)
-	k8sClient := mockclient.NewMockClient(c)
-	builder := gateway.NewAccessLogSubscriptionModelBuilder(gwlog.FallbackLogger, k8sClient)
-	input := &anv1alpha1.AccessLogPolicy{
-		Spec: anv1alpha1.AccessLogPolicySpec{
-			DestinationArn: aws.String(s3DestinationArn),
-			TargetRef: &v1alpha2.PolicyTargetReference{
-				Kind: "Gateway",
-				Name: "TestName",
+		synthesizer := NewAccessLogSubscriptionSynthesizer(gwlog.FallbackLogger, k8sClient, mockManager, stack)
+		err := synthesizer.Synthesize(ctx)
+		assert.Nil(t, err)
+	})
+
+	t.Run("SpecIsNotDeletedButErrorOccurs_ReturnsError", func(t *testing.T) {
+		ctx, c, mockManager, k8sClient, builder := setup()
+		defer c.Finish()
+		input := &anv1alpha1.AccessLogPolicy{
+			Spec: anv1alpha1.AccessLogPolicySpec{
+				DestinationArn: aws.String(s3DestinationArn),
+				TargetRef: &v1alpha2.PolicyTargetReference{
+					Kind: "Gateway",
+					Name: "TestName",
+				},
 			},
-		},
-	}
-	expectedError := errors.New("")
+		}
 
-	stack, accessLogSubscription, _ := builder.Build(context.Background(), input)
+		stack, accessLogSubscription, _ := builder.Build(context.Background(), input)
 
-	k8sClient.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
-	mockManager.EXPECT().Create(ctx, accessLogSubscription).Return(nil, expectedError).AnyTimes()
+		k8sClient.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
+		mockManager.EXPECT().Create(ctx, accessLogSubscription).Return(nil, errors.New("mock error")).AnyTimes()
 
-	synthesizer := NewAccessLogSubscriptionSynthesizer(gwlog.FallbackLogger, k8sClient, mockManager, stack)
-	err := synthesizer.Synthesize(ctx)
-	assert.Equal(t, expectedError, err)
+		synthesizer := NewAccessLogSubscriptionSynthesizer(gwlog.FallbackLogger, k8sClient, mockManager, stack)
+		err := synthesizer.Synthesize(ctx)
+		assert.NotNil(t, err)
+	})
 }