Skip to content

Commit

Permalink
chore(stack): error if target sidecar container has no port (#2571)
Browse files Browse the repository at this point in the history
<!-- 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.
  • Loading branch information
iamhopaul123 authored Jul 8, 2021
1 parent 210a6df commit 4a347b8
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 12 deletions.
6 changes: 5 additions & 1 deletion cf-custom-resources/test/custom-domain-app-runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -28,6 +30,8 @@ describe("Custom Domain for App Runner Service During Create", () => {
});

afterEach(() => {
// Restore logger
console.log = origLog;
AWS.restore();
reset();
});
Expand Down
5 changes: 4 additions & 1 deletion internal/pkg/deploy/cloudformation/stack/lb_web_svc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 15 additions & 10 deletions internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 4a347b8

Please sign in to comment.