Skip to content

Commit 08778a4

Browse files
committed
Remove default port for sidecar container and minor refactoring
1 parent 692a80f commit 08778a4

8 files changed

Lines changed: 35 additions & 33 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func TestBackendService_Template(t *testing.T) {
7979
Value: hello`,
8080
}
8181
},
82-
wantedErr: fmt.Errorf("parse port mapping 80/80/80: %w", errors.New("cannot parse 80/80/80")),
82+
wantedErr: fmt.Errorf("parse port mapping 80/80/80: %w", errors.New("cannot parse port mapping from 80/80/80")),
8383
},
8484
"failed parsing svc template": {
8585
manifest: testBackendSvcManifest,

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ func (s *LoadBalancedWebService) Template() (string, error) {
111111
return content.String(), nil
112112
}
113113

114-
// Parameters returns the list of CloudFormation parameters used by the template.
115-
func (s *LoadBalancedWebService) Parameters() []*cloudformation.Parameter {
114+
func (s *LoadBalancedWebService) loadBalancerTarget() (*string, *string) {
116115
containerName := s.name
117116
containerPort := strconv.FormatUint(uint64(aws.Uint16Value(s.manifest.Image.Port)), 10)
118117
// Route load balancer traffic to main container by default.
@@ -128,10 +127,16 @@ func (s *LoadBalancedWebService) Parameters() []*cloudformation.Parameter {
128127
log.Infof("Target container %s doesn't exist. Use main container to route traffics instead.\n", *s.manifest.TargetContainer)
129128
}
130129
}
130+
return targetContainer, targetPort
131+
}
132+
133+
// Parameters returns the list of CloudFormation parameters used by the template.
134+
func (s *LoadBalancedWebService) Parameters() []*cloudformation.Parameter {
135+
targetContainer, targetPort := s.loadBalancerTarget()
131136
return append(s.svc.Parameters(), []*cloudformation.Parameter{
132137
{
133138
ParameterKey: aws.String(LBWebServiceContainerPortParamKey),
134-
ParameterValue: aws.String(containerPort),
139+
ParameterValue: aws.String(strconv.FormatUint(uint64(aws.Uint16Value(s.manifest.Image.Port)), 10)),
135140
},
136141
{
137142
ParameterKey: aws.String(LBWebServiceRulePathParamKey),

internal/pkg/manifest/backend_svc_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func TestBackendSvc_ApplyEnv(t *testing.T) {
259259
Sidecar: Sidecar{
260260
Sidecars: map[string]*SidecarConfig{
261261
"xray": {
262-
CredParam: aws.String("some arn"),
262+
CredsParam: aws.String("some arn"),
263263
},
264264
},
265265
},
@@ -323,9 +323,9 @@ func TestBackendSvc_ApplyEnv(t *testing.T) {
323323
Sidecar: Sidecar{
324324
Sidecars: map[string]*SidecarConfig{
325325
"xray": {
326-
Port: aws.String("2000/udp"),
327-
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
328-
CredParam: aws.String("some arn"),
326+
Port: aws.String("2000/udp"),
327+
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
328+
CredsParam: aws.String("some arn"),
329329
},
330330
},
331331
},

internal/pkg/manifest/lb_web_svc_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,9 @@ func TestLoadBalancedWebSvc_ApplyEnv(t *testing.T) {
151151
Sidecar: Sidecar{
152152
Sidecars: map[string]*SidecarConfig{
153153
"xray": {
154-
Port: aws.String("2000"),
155-
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
156-
CredParam: aws.String("some arn"),
154+
Port: aws.String("2000"),
155+
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
156+
CredsParam: aws.String("some arn"),
157157
},
158158
},
159159
},
@@ -229,9 +229,9 @@ func TestLoadBalancedWebSvc_ApplyEnv(t *testing.T) {
229229
Sidecar: Sidecar{
230230
Sidecars: map[string]*SidecarConfig{
231231
"xray": {
232-
Port: aws.String("2000/udp"),
233-
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
234-
CredParam: aws.String("some arn"),
232+
Port: aws.String("2000/udp"),
233+
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
234+
CredsParam: aws.String("some arn"),
235235
},
236236
},
237237
},

internal/pkg/manifest/svc.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ const (
1919
LoadBalancedWebServiceType = "Load Balanced Web Service"
2020
// BackendServiceType is a service that cannot be accessed from the internet but can be reached from other services.
2121
BackendServiceType = "Backend Service"
22-
23-
defaultSidecarPort = "80"
2422
)
2523

2624
// ServiceTypes are the supported service manifest types.
@@ -82,7 +80,7 @@ func (s *Sidecar) SidecarsOpts() ([]*template.SidecarOpts, error) {
8280
Image: config.Image,
8381
Port: port,
8482
Protocol: protocol,
85-
CredParam: config.CredParam,
83+
CredsParam: config.CredsParam,
8684
})
8785
}
8886
return sidecars, nil
@@ -92,7 +90,7 @@ func (s *Sidecar) SidecarsOpts() ([]*template.SidecarOpts, error) {
9290
type SidecarConfig struct {
9391
Port *string `yaml:"port"`
9492
Image *string `yaml:"image"`
95-
CredParam *string `yaml:"credentialsParameter"`
93+
CredsParam *string `yaml:"credentialsParameter"`
9694
}
9795

9896
// TaskConfig represents the resource boundaries and environment variables for the containers in the task.
@@ -170,8 +168,7 @@ func intpcopy(v *int) *int {
170168
// Valid sidecar portMapping example: 2000/udp, or 2000 (default to be tcp).
171169
func parsePortMapping(s *string) (port *string, protocol *string, err error) {
172170
if s == nil {
173-
// default port for sidecar container to be 80.
174-
return aws.String(defaultSidecarPort), nil, nil
171+
return nil, nil, nil
175172
}
176173
portProtocol := strings.Split(*s, "/")
177174
switch len(portProtocol) {
@@ -180,6 +177,6 @@ func parsePortMapping(s *string) (port *string, protocol *string, err error) {
180177
case 2:
181178
return aws.String(portProtocol[0]), aws.String(portProtocol[1]), nil
182179
default:
183-
return nil, nil, fmt.Errorf("cannot parse %s", *s)
180+
return nil, nil, fmt.Errorf("cannot parse port mapping from %s", *s)
184181
}
185182
}

internal/pkg/manifest/svc_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ environments:
8181
Sidecar: Sidecar{
8282
Sidecars: map[string]*SidecarConfig{
8383
"xray": {
84-
Port: aws.String("2000/udp"),
85-
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
86-
CredParam: aws.String("some arn"),
84+
Port: aws.String("2000/udp"),
85+
Image: aws.String("123456789012.dkr.ecr.us-east-2.amazonaws.com/xray-daemon"),
86+
CredsParam: aws.String("some arn"),
8787
},
8888
},
8989
},

internal/pkg/template/service.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ type ServiceNestedStackOpts struct {
3131

3232
// SidecarOpts holds configuration that's needed if the service has sidecar containers.
3333
type SidecarOpts struct {
34-
Name *string
35-
Image *string
36-
Port *string
37-
Protocol *string
38-
CredParam *string
34+
Name *string
35+
Image *string
36+
Port *string
37+
Protocol *string
38+
CredsParam *string
3939
}
4040

4141
// ServiceOpts holds optional data that can be provided to enable features in a service stack template.
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
{{range $sidecar := .Sidecars}}- Name: {{$sidecar.Name}}
2-
Image: {{$sidecar.Image}}
2+
Image: {{$sidecar.Image}}{{if $sidecar.Port}}
33
PortMappings:
44
- ContainerPort: {{$sidecar.Port}}{{if $sidecar.Protocol}}
5-
Protocol: {{$sidecar.Protocol}}{{end}}
5+
Protocol: {{$sidecar.Protocol}}{{end}}{{end}}
66
LogConfiguration:
77
LogDriver: awslogs
88
Options:
99
awslogs-region: !Ref AWS::Region
1010
awslogs-group: !Ref LogGroup
1111
awslogs-stream-prefix: copilot
12-
{{- if $sidecar.CredParam}}
12+
{{- if $sidecar.CredsParam}}
1313
RepositoryCredentials:
14-
CredentialsParameter: {{$sidecar.CredParam}}{{- end}}
14+
CredentialsParameter: {{$sidecar.CredsParam}}{{- end}}
1515
{{end}}

0 commit comments

Comments
 (0)