Skip to content
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.

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

feat!: Use / as the default path for the first app #748

Merged
merged 1 commit into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 cf-custom-resources/lib/alb-rule-priority-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

const aws = require('aws-sdk');

// priorityForRootRule is the max priority number that's always set for the listener rule that matches the root path "/"
const priorityForRootRule = "50000"

// These are used for test purposes only
let defaultResponseURL;

Expand Down Expand Up @@ -88,8 +91,9 @@ const calculateNextRulePriority = async function (listenerArn) {
if (rules.length > 0) {
// Take the max rule priority, and add 1 to it.
const rulePriorities = rules.map(rule => {
if (rule.Priority === "default") {
// We treat the default rule as having priority 0
if (rule.Priority === "default" || rule.Priority === priorityForRootRule) {
// Ignore the root rule's priority since it has to be always the max value.
// Ignore the default rule's prority since it's the same as 0.
return 0
}
return parseInt(rule.Priority);
Expand Down
40 changes: 40 additions & 0 deletions cf-custom-resources/test/alb-rule-priority-generator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,46 @@ describe('ALB Rule Priority Generator', () => {
});
});

test('Create operation returns rule priority 1 when the MAX rule is present', () => {

const describeRulesFake = sinon.fake.resolves(
{
"Rules": [
{
"Priority": "50000",
"Conditions": [],
"RuleArn": "arn:aws:elasticloadbalancing:us-west-2:000000000:listener-rule/app/rule",
"IsDefault": false,
"Actions": [
{
"TargetGroupArn": "arn:aws:elasticloadbalancing:us-west-2:000000000:targetgroup/tg",
"Type": "forward"
}
]
}
]
});

AWS.mock('ELBv2', 'describeRules', describeRulesFake);
const request = nock(ResponseURL).put('/', body => {
return body.Status === 'SUCCESS' && body.Data.Priority == 1;
}).reply(200);

return LambdaTester(albRulePriorityHandler.nextAvailableRulePriorityHandler)
.event({
RequestType: 'Create',
RequestId: testRequestId,
ResourceProperties: {
ListenerArn: testALBListenerArn
}
})
.expectResolve(() => {
sinon.assert.calledWith(describeRulesFake, sinon.match({
ListenerArn: testALBListenerArn,
}));
expect(request.isDone()).toBe(true);
});
});

test('Create operation returns rule priority max + 1', () => {
// This set of rules has the default, 3 and 5 rule priorities. We don't try to fill
Expand Down
8 changes: 3 additions & 5 deletions e2e/addons/addons_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,10 @@ var _ = Describe("addons flow", func() {
// Make a POST request to the API to store the user name in DynamoDB.
route := app.Routes[0]
Expect(route.Environment).To(Equal("test"))
Expect(route.URL).To(HaveSuffix(appName))
Eventually(func() (int, error) {
resp, fetchErr := http.Post(fmt.Sprintf("%s/%s", route.URL, projectName), "application/json", nil)
resp, fetchErr := http.Post(fmt.Sprintf("%s/%s/%s", route.URL, appName, projectName), "application/json", nil)
return resp.StatusCode, fetchErr
}, "10s", "1s").Should(Equal(201))
}, "30s", "1s").Should(Equal(201))
})

It("should be able to retrieve the results", func() {
Expand All @@ -175,11 +174,10 @@ var _ = Describe("addons flow", func() {
// Make a GET request to the API to retrieve the list of user names from DynamoDB.
route := app.Routes[0]
Expect(route.Environment).To(Equal("test"))
Expect(route.URL).To(HaveSuffix(appName))
var resp *http.Response
var fetchErr error
Eventually(func() (int, error) {
resp, fetchErr = http.Get(route.URL))
resp, fetchErr = http.Get(fmt.Sprintf("%s/hello", route.URL))
return resp.StatusCode, fetchErr
}, "10s", "1s").Should(Equal(200))

Expand Down
6 changes: 2 additions & 4 deletions e2e/init/front-end/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
FROM nginx

COPY nginx.conf /etc/nginx/nginx.conf

RUN mkdir -p /www/data/front-end
COPY index.html /www/data/front-end
RUN mkdir -p /www/data/
COPY index.html /www/data/
17 changes: 0 additions & 17 deletions e2e/init/front-end/nginx.conf

This file was deleted.

1 change: 0 additions & 1 deletion e2e/init/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ var _ = Describe("init flow", func() {
It("should return a valid route", func() {
Expect(len(app.Routes)).To(Equal(1))
Expect(app.Routes[0].Environment).To(Equal("test"))
Expect(app.Routes[0].URL).To(HaveSuffix(appName))
Eventually(func() (int, error) {
resp, fetchErr := http.Get(app.Routes[0].URL)
return resp.StatusCode, fetchErr
Expand Down
2 changes: 1 addition & 1 deletion e2e/multi-app-project/front-end/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ module github.com/aws/amazon-ecs-cli-v2/e2e/multi-app-project/front-end

go 1.13

require github.com/julienschmidt/httprouter v1.3.0 // indirect
require github.com/julienschmidt/httprouter v1.3.0
14 changes: 2 additions & 12 deletions e2e/multi-app-project/front-end/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ import (
"github.com/julienschmidt/httprouter"
)

// HealthCheck just returns true if the service is up.
func HealthCheck(w http.ResponseWriter, req *http.Request, ps httprouter.Params) {
log.Println("🚑 healthcheck ok!")
w.WriteHeader(http.StatusOK)
}

// SimpleGet just returns true no matter what
func SimpleGet(w http.ResponseWriter, req *http.Request, ps httprouter.Params) {
log.Println("Get Succeeded")
Expand Down Expand Up @@ -45,11 +39,7 @@ func ServiceDiscoveryGet(w http.ResponseWriter, req *http.Request, ps httprouter

func main() {
router := httprouter.New()
router.GET("/front-end/", SimpleGet)
router.GET("/front-end/service-discovery-test", ServiceDiscoveryGet)

// Health Check
router.GET("/", HealthCheck)

router.GET("/", SimpleGet)
router.GET("/service-discovery-test", ServiceDiscoveryGet)
log.Fatal(http.ListenAndServe(":80", router))
}
22 changes: 16 additions & 6 deletions e2e/multi-app-project/multi_app_project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,21 @@ var _ = Describe("Multiple App Project", func() {
// Call each environment's endpoint and ensure it returns a 200
route := app.Routes[0]
Expect(route.Environment).To(Equal("test"))
Expect(route.URL).To(HaveSuffix(appName))
resp, fetchErr := http.Get(route.URL)
Expect(fetchErr).NotTo(HaveOccurred())
Expect(resp.StatusCode).To(Equal(200))
// Since the front end was added first, it should have no suffix.
if appName == "front-end" {
Expect(route.URL).ToNot(HaveSuffix(appName))
}
// Since the back end was added second, it should have app appended
// to the name
if appName == "back-end" {
Expect(route.URL).To(HaveSuffix(appName))
}
var resp *http.Response
var fetchErr error
Eventually(func() (int, error) {
resp, fetchErr = http.Get(route.URL)
return resp.StatusCode, fetchErr
}, "10s", "1s").Should(Equal(200))

// Read the response - our deployed apps should return a body with their
// name as the value.
Expand Down Expand Up @@ -186,8 +197,7 @@ var _ = Describe("Multiple App Project", func() {
// to the backend, and pipe the backend response to us.
route := app.Routes[0]
Expect(route.Environment).To(Equal("test"))
Expect(route.URL).To(HaveSuffix(appName))
resp, fetchErr := http.Get(fmt.Sprintf("http://%s/service-discovery-test/", route.URL))
resp, fetchErr := http.Get(fmt.Sprintf("%s/service-discovery-test/", route.URL))
Expect(fetchErr).NotTo(HaveOccurred())
Expect(resp.StatusCode).To(Equal(200))

Expand Down
1 change: 0 additions & 1 deletion e2e/multi-env-project/multi_env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ var _ = Describe("Multiple Env Project", func() {
for _, env := range []string{"test", "prod"} {
route := envRoutes[env]
Expect(route.Environment).To(Equal(env))
Expect(route.URL).To(HaveSuffix(appName))
Eventually(func() (int, error) {
resp, fetchErr := http.Get(route.URL)
return resp.StatusCode, fetchErr
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ require (
github.com/fatih/structs v1.1.0
github.com/gobuffalo/packd v1.0.0
github.com/gobuffalo/packr/v2 v2.8.0
github.com/golang/mock v1.4.2
github.com/golang/mock v1.4.3
github.com/golang/protobuf v1.3.2 // indirect
github.com/google/uuid v1.1.1
github.com/hinshun/vt10x v0.0.0-20180809195222-d55458df857c // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ github.com/golang/mock v1.4.1 h1:ocYkMQY5RrXTYgXl7ICpV0IXwlEQGwKIsery4gyXa1U=
github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.2 h1:fXIkPzOBCwDUPvYmOPzETABgbqpYlYNigQ2o64eMr5c=
github.com/golang/mock v1.4.2/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/mock v1.4.3 h1:GV+pQPG/EUUbkh47niozDcADz6go/dUwhVzdUQHIVRw=
github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.1 h1:YF8+flBXS5eO826T4nzqPrxfhQThhXl0YzfuUPu4SBg=
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down
35 changes: 27 additions & 8 deletions internal/pkg/cli/app_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,10 @@ func (o *initAppOpts) Execute() error {
}

func (o *initAppOpts) createManifest() (string, error) {
props := &manifest.LBFargateManifestProps{
AppManifestProps: &manifest.AppManifestProps{
AppName: o.AppName,
Dockerfile: o.DockerfilePath,
},
Port: o.AppPort,
manifest, err := o.createLoadBalancedAppManifest()
if err != nil {
return "", err
}
props.Path = o.AppName
manifest := manifest.NewLoadBalancedFargateManifest(props)
manifestPath, err := o.ws.WriteAppManifest(manifest, o.AppName)
if err != nil {
return "", err
Expand All @@ -210,6 +205,30 @@ func (o *initAppOpts) createManifest() (string, error) {
return relPath, nil
}

func (o *initAppOpts) createLoadBalancedAppManifest() (*manifest.LBFargateManifest, error) {
props := &manifest.LBFargateManifestProps{
AppManifestProps: &manifest.AppManifestProps{
AppName: o.AppName,
Dockerfile: o.DockerfilePath,
},
Port: o.AppPort,
Path: "/",
}
existingApps, err := o.appStore.ListApplications(o.ProjectName())
if err != nil {
return nil, err
}
// We default to "/" for the first app, but if there's another
// load balanced web app, we use the app name as the default, instead.
for _, existingApp := range existingApps {
if existingApp.Type == manifest.LoadBalancedWebApplication && existingApp.Name != o.AppName {
props.Path = o.AppName
break
}
}
return manifest.NewLoadBalancedFargateManifest(props), nil
}

func (o *initAppOpts) createAppInProject(projectName string) error {
if err := o.appStore.CreateApplication(&archer.Application{
Project: projectName,
Expand Down
Loading