From 4a347b8ff197087f607476033b9125934d216502 Mon Sep 17 00:00:00 2001
From: Penghao He <penghaoh@amazon.com>
Date: Thu, 8 Jul 2021 09:09:29 -0700
Subject: [PATCH] chore(stack): error if target sidecar container has no port
 (#2571)

<!-- Provide summary of changes -->
After #2565, it is possible that a sidecar container has no port open. However, we should error out early if that sidecar container is the target container for LB.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
---
 .../test/custom-domain-app-runner-test.js     |  6 ++++-
 .../deploy/cloudformation/stack/lb_web_svc.go |  5 +++-
 .../cloudformation/stack/lb_web_svc_test.go   | 25 +++++++++++--------
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/cf-custom-resources/test/custom-domain-app-runner-test.js b/cf-custom-resources/test/custom-domain-app-runner-test.js
index 0722ba7b973..40682b89fca 100644
--- a/cf-custom-resources/test/custom-domain-app-runner-test.js
+++ b/cf-custom-resources/test/custom-domain-app-runner-test.js
@@ -10,16 +10,18 @@ const LambdaTester = require("lambda-tester").noVersionCheck();
 const {handler, domainStatusPendingVerification, waitForDomainStatusPendingAttempts, waitForDomainStatusActiveAttempts, withSleep, reset, withDeadlineExpired} = require("../lib/custom-domain-app-runner");
 const sinon = require("sinon");
 const nock = require("nock");
+let origLog = console.log;
 
 describe("Custom Domain for App Runner Service During Create", () => {
     const [mockServiceARN, mockCustomDomain, mockHostedZoneID, mockResponseURL, mockPhysicalResourceID, mockLogicalResourceID] =
         ["mockService", "mockDomain", "mockHostedZoneID", "https://mock.com/", "mockPhysicalResourceID", "mockLogicalResourceID", ];
 
     beforeEach(() => {
+        // Prevent logging.
+        console.log = function () {};
         withSleep(_ => {
             return Promise.resolve();
         });
-
         withDeadlineExpired(_ => {
             return new Promise((resolve) => {
                 setTimeout(resolve, 1000);
@@ -28,6 +30,8 @@ describe("Custom Domain for App Runner Service During Create", () => {
     });
 
     afterEach(() => {
+        // Restore logger
+        console.log = origLog;
         AWS.restore();
         reset();
     });
diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go
index 2a8cf68363e..a5bc24a40d5 100644
--- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go
+++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go
@@ -200,10 +200,13 @@ func (s *LoadBalancedWebService) loadBalancerTarget() (targetContainer *string,
 	if mftTargetContainer != nil {
 		sidecar, ok := s.manifest.Sidecars[*mftTargetContainer]
 		if ok {
+			if sidecar.Port == nil {
+				return nil, nil, fmt.Errorf("target container %s doesn't expose any port", *mftTargetContainer)
+			}
 			targetContainer = mftTargetContainer
 			targetPort = sidecar.Port
 		} else {
-			return nil, nil, fmt.Errorf("target container %s doesn't exist", *s.manifest.TargetContainer)
+			return nil, nil, fmt.Errorf("target container %s doesn't exist", *mftTargetContainer)
 		}
 	}
 	return
diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go
index ee6934ae1be..177fa33727b 100644
--- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go
+++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go
@@ -359,15 +359,14 @@ func TestLoadBalancedWebService_Parameters(t *testing.T) {
 			Enable: aws.Bool(true),
 		},
 	}
-	testLBWebServiceManifestWithBadSidecar := manifest.NewLoadBalancedWebService(&manifest.LoadBalancedWebServiceProps{
-		WorkloadProps: &manifest.WorkloadProps{
-			Name:       "frontend",
-			Dockerfile: "frontend/Dockerfile",
-		},
-		Path: "frontend",
-		Port: 80,
-	})
-	testLBWebServiceManifestWithBadSidecar.TargetContainer = aws.String("xray")
+	testLBWebServiceManifestWithBadSidecarName := manifest.NewLoadBalancedWebService(baseProps)
+	testLBWebServiceManifestWithBadSidecarName.TargetContainer = aws.String("xray")
+
+	testLBWebServiceManifestWithBadSidecarPort := manifest.NewLoadBalancedWebService(baseProps)
+	testLBWebServiceManifestWithBadSidecarPort.TargetContainer = aws.String("xray")
+	testLBWebServiceManifestWithBadSidecarPort.Sidecars = map[string]*manifest.SidecarConfig{
+		"xray": {},
+	}
 	expectedParams := []*cloudformation.Parameter{
 		{
 			ParameterKey:   aws.String(WorkloadAppNameParamKey),
@@ -554,10 +553,16 @@ func TestLoadBalancedWebService_Parameters(t *testing.T) {
 		},
 		"with bad sidecar container": {
 			httpsEnabled: true,
-			manifest:     testLBWebServiceManifestWithBadSidecar,
+			manifest:     testLBWebServiceManifestWithBadSidecarName,
 
 			expectedErr: fmt.Errorf("target container xray doesn't exist"),
 		},
+		"with target sidecar container with empty port": {
+			httpsEnabled: true,
+			manifest:     testLBWebServiceManifestWithBadSidecarPort,
+
+			expectedErr: fmt.Errorf("target container xray doesn't expose any port"),
+		},
 		"with bad count": {
 			httpsEnabled: true,
 			manifest:     testLBWebServiceManifestWithBadCount,