Skip to content

Commit

Permalink
Fix resolution of deploy-service url
Browse files Browse the repository at this point in the history
When there are more than one shared domains, presented in the landscape,
the logic of finding the correct one, was assuming that the first one is
the correct. However, this is not true all the times as the first one
could not be the correct one. This commit introduces a mechanism for
pinging the domains till the right one responds with 200 OK status code.

BUG: LMCROSSITXSADEPLOY-833
  • Loading branch information
Encho Belezirev authored and enchobelezirev committed Feb 8, 2018
1 parent be6fb78 commit 5e256c6
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 14 deletions.
9 changes: 5 additions & 4 deletions util/archive_handler_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package util
package util_test

import (
"os"
"path/filepath"

"github.com/SAP/cf-mta-plugin/util"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)
Expand All @@ -13,13 +14,13 @@ var _ = Describe("ArchiveHandler", func() {
var mtaArchiveFilePath, _ = filepath.Abs("../test_resources/commands/mtaArchive.mtar")
Context("with valid mta archive", func() {
It("should extract and return the id from deployment descriptor", func() {
Expect(GetMtaIDFromArchive(mtaArchiveFilePath)).To(Equal("test"))
Expect(util.GetMtaIDFromArchive(mtaArchiveFilePath)).To(Equal("test"))
})
})
Context("with valid mta archive and no deployment descriptor provided", func() {
It("should return error", func() {
mtaArchiveFilePath, _ = filepath.Abs("../test_resources/util/mtaArchiveNoDescriptor.mtar")
_, err := GetMtaIDFromArchive(mtaArchiveFilePath)
_, err := util.GetMtaIDFromArchive(mtaArchiveFilePath)
Expect(err).To(MatchError("Could not get MTA id from archive"))
})
})
Expand All @@ -30,7 +31,7 @@ var _ = Describe("ArchiveHandler", func() {
mtaArchiveFilePath, _ = filepath.Abs("test.mtar")
})
It("should return error for not a valid zip archive", func() {
_, err := GetMtaIDFromArchive(mtaArchiveFilePath)
_, err := util.GetMtaIDFromArchive(mtaArchiveFilePath)
Expect(err).To(MatchError("zip: not a valid zip file"))
})
AfterEach(func() {
Expand Down
57 changes: 57 additions & 0 deletions util/builders.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package util

import (
"bytes"
"fmt"
"net/url"
"strings"

"github.com/SAP/cf-mta-plugin/clients/models"
)

Expand Down Expand Up @@ -31,3 +36,55 @@ func (pb *ProcessBuilder) Parameter(parameterID string, value string) *ProcessBu
func (pb *ProcessBuilder) Build() *models.Operation {
return &pb.operation
}

const hostAndSchemeSeparator = "://"

type UriBuilder struct {
host string
scheme string
path string
}

func NewUriBuilder() *UriBuilder {
return &UriBuilder{host: "", scheme: "", path: ""}
}

func (builder *UriBuilder) SetScheme(scheme string) *UriBuilder {
builder.scheme = scheme
return builder
}

func (builder *UriBuilder) SetHost(host string) *UriBuilder {
builder.host = host
return builder
}

func (builder *UriBuilder) SetPath(path string) *UriBuilder {
builder.path = path
return builder
}

func (builder *UriBuilder) Build() (string, error) {
if builder.scheme == "" || builder.host == "" {
return "", fmt.Errorf("The host or scheme could not be empty")
}
stringBuilder := bytes.Buffer{}
stringBuilder.WriteString(builder.scheme)
stringBuilder.WriteString(hostAndSchemeSeparator)
stringBuilder.WriteString(builder.host)
stringBuilder.WriteString(getPath(builder.path))
builtUrl := stringBuilder.String()
parsedUrl, err := url.Parse(builtUrl)
if err != nil {
return "", err
}
return parsedUrl.String(), nil
}

func getPath(path string) string {
if strings.HasPrefix(path, "/") {
return path
}

return "/" + path
}
42 changes: 42 additions & 0 deletions util/builders_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package util_test

import (
"github.com/SAP/cf-mta-plugin/util"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

var _ = Describe("UriBuilderTest", func() {
Describe("BuildUriTest", func() {
const hostName = "test-host"
const scheme = "test-scheme"
const path = "test-path"

Context("no scheme and host are provided", func() {
It("should return an error", func() {
uriBuilder := util.NewUriBuilder().SetPath(path)
_, err := uriBuilder.Build()

Expect(err).Should(MatchError("The host or scheme could not be empty"))
})
})
Context("when scheme and path are provided", func() {
It("should return an error", func() {
uriBuilder := util.NewUriBuilder().SetScheme(scheme).SetPath(path)
_, err := uriBuilder.Build()

Expect(err).Should(MatchError("The host or scheme could not be empty"))
})
})
Context("when scheme, host and path are provided", func() {
It("should the built uri", func() {
uriBuilder := util.NewUriBuilder().SetHost(hostName).SetScheme(scheme).SetPath(path)
uri, err := uriBuilder.Build()

Expect(uri).To(Equal("test-scheme://test-host/test-path"))
Expect(err).To(BeNil())
})
})

})
})
37 changes: 32 additions & 5 deletions util/deploy_service_url_calculator.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,24 @@ import (
)

const deployServiceHost = "deploy-service"
const defaultDeployServiceHostHttpScheme = "https"
const defaultDeployServiceEndpoint = "/public/ping"

type DeployServiceURLCalculator interface {
ComputeDeployServiceURL() (string, error)
}

type deployServiceURLCalculatorImpl struct {
cliConnection plugin.CliConnection
cliConnection plugin.CliConnection
httpGetExecutor HttpSimpleGetExecutor
}

func NewDeployServiceURLCalculator(cliConnection plugin.CliConnection) DeployServiceURLCalculator {
return deployServiceURLCalculatorImpl{cliConnection: cliConnection}
return deployServiceURLCalculatorImpl{cliConnection: cliConnection, httpGetExecutor: NewSimpleGetExecutor()}
}

func NewDeployServiceURLCalculatorWithHttpExecutor(cliConnection plugin.CliConnection, httpGetExecutor HttpSimpleGetExecutor) DeployServiceURLCalculator {
return deployServiceURLCalculatorImpl{cliConnection: cliConnection, httpGetExecutor: httpGetExecutor}
}

func (c deployServiceURLCalculatorImpl) ComputeDeployServiceURL() (string, error) {
Expand All @@ -27,7 +34,7 @@ func (c deployServiceURLCalculatorImpl) ComputeDeployServiceURL() (string, error
return "", err
}

sharedDomain, err := findSharedDomain(currentSpace)
sharedDomain, err := c.findSharedDomain(currentSpace)
if err != nil {
return "", err
}
Expand All @@ -47,11 +54,31 @@ func (c deployServiceURLCalculatorImpl) getCurrentSpace() (plugin_models.GetSpac
return c.cliConnection.GetSpace(currentSpace.Name)
}

func findSharedDomain(space plugin_models.GetSpace_Model) (plugin_models.GetSpace_Domains, error) {
func (c deployServiceURLCalculatorImpl) findSharedDomain(space plugin_models.GetSpace_Model) (plugin_models.GetSpace_Domains, error) {
for _, domain := range space.Domains {
if domain.Shared {
return domain, nil
if c.isCorrectDomain(domain.Name) {
return domain, nil
}
}
}
return plugin_models.GetSpace_Domains{}, fmt.Errorf("Could not find any shared domains in space: %s", space.Name)
}

func (c deployServiceURLCalculatorImpl) isCorrectDomain(domainName string) bool {
statusCode, err := c.httpGetExecutor.ExecuteGetRequest(buildDeployServiceUrl(domainName))
if err != nil {
return false
}

return statusCode == 200
}

func buildDeployServiceUrl(domainName string) string {
uriBuilder := NewUriBuilder()
uri, err := uriBuilder.SetScheme(defaultDeployServiceHostHttpScheme).SetPath(defaultDeployServiceEndpoint).SetHost(deployServiceHost + "." + domainName).Build()
if err != nil {
return ""
}
return uri
}
14 changes: 9 additions & 5 deletions util/deploy_service_url_calculator_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package util
package util_test

import (
cli_fakes "github.com/SAP/cf-mta-plugin/cli/fakes"
"github.com/SAP/cf-mta-plugin/util"
fakes "github.com/SAP/cf-mta-plugin/util/fakes"
plugin_models "github.com/cloudfoundry/cli/plugin/models"

. "github.com/onsi/ginkgo"
Expand All @@ -26,7 +28,8 @@ var _ = Describe("DeployServiceURLCalculator", func() {
plugin_models.GetSpace_Domains{Name: "test.ondemand.com", Shared: true},
},
}, nil).Build()
deployServiceURLCalculator := NewDeployServiceURLCalculator(cliConnection)
fakeHttpExecutor := fakes.NewFakeHttpGetExecutor(200, nil)
deployServiceURLCalculator := util.NewDeployServiceURLCalculatorWithHttpExecutor(cliConnection, fakeHttpExecutor)
Expect(deployServiceURLCalculator.ComputeDeployServiceURL()).To(Equal("deploy-service.test.ondemand.com"))
})
})
Expand All @@ -44,7 +47,8 @@ var _ = Describe("DeployServiceURLCalculator", func() {
plugin_models.GetSpace_Domains{Name: "test2.ondemand.com", Shared: true},
},
}, nil).Build()
deployServiceURLCalculator := NewDeployServiceURLCalculator(cliConnection)
fakeHttpExecutor := fakes.NewFakeHttpGetExecutor(200, nil)
deployServiceURLCalculator := util.NewDeployServiceURLCalculatorWithHttpExecutor(cliConnection, fakeHttpExecutor)
Expect(deployServiceURLCalculator.ComputeDeployServiceURL()).To(Equal("deploy-service.test1.ondemand.com"))
})
})
Expand All @@ -60,7 +64,7 @@ var _ = Describe("DeployServiceURLCalculator", func() {
plugin_models.GetSpace_Domains{Name: "custom.test.ondemand.com", Shared: false},
},
}, nil).Build()
deployServiceURLCalculator := NewDeployServiceURLCalculator(cliConnection)
deployServiceURLCalculator := util.NewDeployServiceURLCalculator(cliConnection)
_, err := deployServiceURLCalculator.ComputeDeployServiceURL()
Expect(err).Should(MatchError("Could not find any shared domains in space: " + spaceName))
})
Expand All @@ -70,7 +74,7 @@ var _ = Describe("DeployServiceURLCalculator", func() {
cliConnection := cli_fakes.NewFakeCliConnectionBuilder().
CurrentSpace("", "", nil).
Build()
deployServiceURLCalculator := NewDeployServiceURLCalculator(cliConnection)
deployServiceURLCalculator := util.NewDeployServiceURLCalculator(cliConnection)
_, err := deployServiceURLCalculator.ComputeDeployServiceURL()
Expect(err).Should(MatchError("No space targeted, use 'cf target -s SPACE' to target a space."))
})
Expand Down
16 changes: 16 additions & 0 deletions util/fakes/fake_http_simple_executor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package fakes

import "github.com/SAP/cf-mta-plugin/util"

type fakeHttpGetExecutor struct{
statusCode int
err error
}

func NewFakeHttpGetExecutor(statusCode int, err error) util.HttpSimpleGetExecutor{
return &fakeHttpGetExecutor{statusCode: statusCode, err: err}
}

func (f fakeHttpGetExecutor) ExecuteGetRequest(url string) (int, error) {
return f.statusCode, f.err
}
25 changes: 25 additions & 0 deletions util/http_util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package util

import (
"net/http"
)

type HttpSimpleGetExecutor interface {
ExecuteGetRequest(url string) (int, error)
}

type SimpleGetExecutor struct {
}

func NewSimpleGetExecutor() SimpleGetExecutor {
return SimpleGetExecutor{}
}

func (executor SimpleGetExecutor) ExecuteGetRequest(url string) (int, error) {
resp, err := http.Get(url)
if err != nil {
return -1, err
}
defer resp.Body.Close()
return resp.StatusCode, nil
}

0 comments on commit 5e256c6

Please sign in to comment.