Skip to content

fix: respect network replacement configured through manifest #3352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 9 commits into from
Mar 14, 2022
Merged

fix: respect network replacement configured through manifest #3352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
07df8ce
fix typo
Lou1415926 Mar 11, 2022
fa0cef5
remove network config unmarshal logic
Lou1415926 Mar 11, 2022
832b62f
define placement options as const instead of var
Lou1415926 Mar 11, 2022
a5bfe18
fix unit test
Lou1415926 Mar 11, 2022
530e5af
addr fb: add function comment
Lou1415926 Mar 11, 2022
83739f9
addr fb: move comment to a different line
Lou1415926 Mar 11, 2022
db93a3c
addr fb: unexport platformp
Lou1415926 Mar 11, 2022
b26d258
fix static
Lou1415926 Mar 14, 2022
39c2d14
Merge branch 'mainline' into fix/manifest/vpc-placmenet-has-default
mergify[bot] Mar 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions internal/pkg/deploy/cloudformation/stack/rd_web_svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import (
"github.com/stretchr/testify/require"
)

var (
testPrivatePlacement = manifest.PrivateSubnetPlacement
)

var testRDWebServiceManifest = &manifest.RequestDrivenWebService{
Workload: manifest.Workload{
Name: aws.String(testServiceName),
Expand Down Expand Up @@ -221,7 +225,7 @@ func TestRequestDrivenWebService_Template(t *testing.T) {
}{
"should throw an error if env controller cannot be parsed": {
inManifest: func(mft manifest.RequestDrivenWebService) manifest.RequestDrivenWebService {
mft.Network.VPC.Placement = (*manifest.RequestDrivenWebServicePlacement)(&manifest.PrivateSubnetPlacement)
mft.Network.VPC.Placement = (*manifest.RequestDrivenWebServicePlacement)(&testPrivatePlacement)
return mft
},
mockDependencies: func(t *testing.T, ctrl *gomock.Controller, c *RequestDrivenWebService) {
Expand All @@ -243,7 +247,7 @@ func TestRequestDrivenWebService_Template(t *testing.T) {
"should be able to parse custom resource URLs when alias is enabled": {
inManifest: func(mft manifest.RequestDrivenWebService) manifest.RequestDrivenWebService {
mft.Alias = aws.String("convex.domain.com")
mft.Network.VPC.Placement = (*manifest.RequestDrivenWebServicePlacement)(&manifest.PrivateSubnetPlacement)
mft.Network.VPC.Placement = (*manifest.RequestDrivenWebServicePlacement)(&testPrivatePlacement)
return mft
},
inCustomResourceURLs: map[string]string{
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/deploy/cloudformation/stack/worker_svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ Outputs:

if tc.setUpManifest != nil {
tc.setUpManifest(conf)
conf.manifest.Network.VPC.Placement = &manifest.PrivateSubnetPlacement
conf.manifest.Network.VPC.Placement = &testPrivatePlacement
conf.manifest.Network.VPC.SecurityGroups = []string{"sg-1234"}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/manifest/backend_svc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func newDefaultBackendService() *BackendService {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/manifest/backend_svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestNewBackendSvc(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down Expand Up @@ -102,7 +102,7 @@ func TestNewBackendSvc(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down Expand Up @@ -152,7 +152,7 @@ func TestNewBackendSvc(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/manifest/job.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func newDefaultScheduledJob() *ScheduledJob {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/manifest/job_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func TestScheduledJob_ApplyEnv(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down Expand Up @@ -187,7 +187,7 @@ func TestScheduledJob_ApplyEnv(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/manifest/lb_web_svc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func newDefaultLoadBalancedWebService() *LoadBalancedWebService {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down
12 changes: 6 additions & 6 deletions internal/pkg/manifest/lb_web_svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestNewHTTPLoadBalancedWebService(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestNewHTTPLoadBalancedWebService(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down Expand Up @@ -454,7 +454,7 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
SecurityGroups: []string{"sg-123"},
},
},
Expand Down Expand Up @@ -615,7 +615,7 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
SecurityGroups: []string{"sg-456", "sg-789"},
},
},
Expand Down Expand Up @@ -686,7 +686,7 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
SecurityGroups: []string{"sg-456", "sg-789"},
},
},
Expand All @@ -710,7 +710,7 @@ func TestLoadBalancedWebService_ApplyEnv(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
SecurityGroups: []string{"sg-456", "sg-789"},
},
},
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/manifest/svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ environments:
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
TaskDefOverrides: []OverrideRule{
Expand Down Expand Up @@ -270,7 +270,7 @@ secrets:
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down Expand Up @@ -333,7 +333,7 @@ subscribe:
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
Subscribe: SubscribeConfig{
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/manifest/worker_svc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func newDefaultWorkerService() *WorkerService {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/manifest/worker_svc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestNewWorkerSvc(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down Expand Up @@ -112,7 +112,7 @@ func TestNewWorkerSvc(t *testing.T) {
},
Network: NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP(PublicSubnetPlacement),
},
},
},
Expand Down
40 changes: 13 additions & 27 deletions internal/pkg/manifest/workload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ const (
defaultDockerfileName = "Dockerfile"
)

// AWS VPC subnet placement options.
var (
const (
// AWS VPC subnet placement options.
PublicSubnetPlacement = Placement("public")
PrivateSubnetPlacement = Placement("private")

// All placement options.
subnetPlacements = []string{string(PublicSubnetPlacement), string(PrivateSubnetPlacement)}
)

// All placement options.
var subnetPlacements = []string{string(PublicSubnetPlacement), string(PrivateSubnetPlacement)}

// Error definitions.
var (
ErrAppRunnerInvalidPlatformWindows = errors.New("Windows is not supported for App Runner services")
Expand Down Expand Up @@ -73,7 +73,6 @@ func UnmarshalWorkload(in []byte) (WorkloadManifest, error) {
switch typeVal {
case LoadBalancedWebServiceType:
m = newDefaultLoadBalancedWebService()

case RequestDrivenWebServiceType:
m = newDefaultRequestDrivenWebService()
case BackendServiceType:
Expand Down Expand Up @@ -381,27 +380,6 @@ func (c *NetworkConfig) IsEmpty() bool {
return c.VPC.isEmpty()
}

// UnmarshalYAML ensures that a NetworkConfig always defaults to public subnets.
// If the user specified a placement that's not valid then throw an error.
func (c *NetworkConfig) UnmarshalYAML(value *yaml.Node) error {
type networkWithDefaults NetworkConfig
publicPlacement := Placement(PublicSubnetPlacement)
defaultVPCConf := vpcConfig{
Placement: &publicPlacement,
}
conf := networkWithDefaults{
VPC: defaultVPCConf,
}
if err := value.Decode(&conf); err != nil {
return err
}
if conf.VPC.isEmpty() { // If after unmarshaling the user did not specify VPC configuration then reset it to public.
conf.VPC = defaultVPCConf
}
*c = NetworkConfig(conf)
return nil
}

// Placement represents where to place tasks (public or private subnets).
type Placement string

Expand Down Expand Up @@ -565,3 +543,11 @@ func uint16P(n uint16) *uint16 {
}
return &n
}

Comment thread
Lou1415926 marked this conversation as resolved.
func placementP(p Placement) *Placement {
if p == "" {
return nil
}
placement := p
return &placement
}
6 changes: 2 additions & 4 deletions internal/pkg/manifest/workload_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,9 +627,7 @@ network:
vpc:
`,
wantedConfig: &NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
},
VPC: vpcConfig{},
},
},
"unmarshals successfully for public placement with security groups": {
Expand All @@ -643,7 +641,7 @@ network:
`,
wantedConfig: &NetworkConfig{
VPC: vpcConfig{
Placement: &PublicSubnetPlacement,
Placement: placementP("public"),
SecurityGroups: []string{"sg-1234", "sg-4567"},
},
},
Expand Down