Skip to content

Commit 4a347b8

Browse files
authored
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.
1 parent 210a6df commit 4a347b8

File tree

3 files changed

+24
-12
lines changed

3 files changed

+24
-12
lines changed

cf-custom-resources/test/custom-domain-app-runner-test.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,18 @@ const LambdaTester = require("lambda-tester").noVersionCheck();
1010
const {handler, domainStatusPendingVerification, waitForDomainStatusPendingAttempts, waitForDomainStatusActiveAttempts, withSleep, reset, withDeadlineExpired} = require("../lib/custom-domain-app-runner");
1111
const sinon = require("sinon");
1212
const nock = require("nock");
13+
let origLog = console.log;
1314

1415
describe("Custom Domain for App Runner Service During Create", () => {
1516
const [mockServiceARN, mockCustomDomain, mockHostedZoneID, mockResponseURL, mockPhysicalResourceID, mockLogicalResourceID] =
1617
["mockService", "mockDomain", "mockHostedZoneID", "https://mock.com/", "mockPhysicalResourceID", "mockLogicalResourceID", ];
1718

1819
beforeEach(() => {
20+
// Prevent logging.
21+
console.log = function () {};
1922
withSleep(_ => {
2023
return Promise.resolve();
2124
});
22-
2325
withDeadlineExpired(_ => {
2426
return new Promise((resolve) => {
2527
setTimeout(resolve, 1000);
@@ -28,6 +30,8 @@ describe("Custom Domain for App Runner Service During Create", () => {
2830
});
2931

3032
afterEach(() => {
33+
// Restore logger
34+
console.log = origLog;
3135
AWS.restore();
3236
reset();
3337
});

internal/pkg/deploy/cloudformation/stack/lb_web_svc.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,13 @@ func (s *LoadBalancedWebService) loadBalancerTarget() (targetContainer *string,
200200
if mftTargetContainer != nil {
201201
sidecar, ok := s.manifest.Sidecars[*mftTargetContainer]
202202
if ok {
203+
if sidecar.Port == nil {
204+
return nil, nil, fmt.Errorf("target container %s doesn't expose any port", *mftTargetContainer)
205+
}
203206
targetContainer = mftTargetContainer
204207
targetPort = sidecar.Port
205208
} else {
206-
return nil, nil, fmt.Errorf("target container %s doesn't exist", *s.manifest.TargetContainer)
209+
return nil, nil, fmt.Errorf("target container %s doesn't exist", *mftTargetContainer)
207210
}
208211
}
209212
return

internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -359,15 +359,14 @@ func TestLoadBalancedWebService_Parameters(t *testing.T) {
359359
Enable: aws.Bool(true),
360360
},
361361
}
362-
testLBWebServiceManifestWithBadSidecar := manifest.NewLoadBalancedWebService(&manifest.LoadBalancedWebServiceProps{
363-
WorkloadProps: &manifest.WorkloadProps{
364-
Name: "frontend",
365-
Dockerfile: "frontend/Dockerfile",
366-
},
367-
Path: "frontend",
368-
Port: 80,
369-
})
370-
testLBWebServiceManifestWithBadSidecar.TargetContainer = aws.String("xray")
362+
testLBWebServiceManifestWithBadSidecarName := manifest.NewLoadBalancedWebService(baseProps)
363+
testLBWebServiceManifestWithBadSidecarName.TargetContainer = aws.String("xray")
364+
365+
testLBWebServiceManifestWithBadSidecarPort := manifest.NewLoadBalancedWebService(baseProps)
366+
testLBWebServiceManifestWithBadSidecarPort.TargetContainer = aws.String("xray")
367+
testLBWebServiceManifestWithBadSidecarPort.Sidecars = map[string]*manifest.SidecarConfig{
368+
"xray": {},
369+
}
371370
expectedParams := []*cloudformation.Parameter{
372371
{
373372
ParameterKey: aws.String(WorkloadAppNameParamKey),
@@ -554,10 +553,16 @@ func TestLoadBalancedWebService_Parameters(t *testing.T) {
554553
},
555554
"with bad sidecar container": {
556555
httpsEnabled: true,
557-
manifest: testLBWebServiceManifestWithBadSidecar,
556+
manifest: testLBWebServiceManifestWithBadSidecarName,
558557

559558
expectedErr: fmt.Errorf("target container xray doesn't exist"),
560559
},
560+
"with target sidecar container with empty port": {
561+
httpsEnabled: true,
562+
manifest: testLBWebServiceManifestWithBadSidecarPort,
563+
564+
expectedErr: fmt.Errorf("target container xray doesn't expose any port"),
565+
},
561566
"with bad count": {
562567
httpsEnabled: true,
563568
manifest: testLBWebServiceManifestWithBadCount,

0 commit comments

Comments
 (0)