From 5e256c64e7121d9d28e652940399416cd68d927d Mon Sep 17 00:00:00 2001 From: Encho Belezirev Date: Thu, 8 Feb 2018 15:53:11 +0200 Subject: [PATCH] Fix resolution of deploy-service url 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 --- util/archive_handler_test.go | 9 ++-- util/builders.go | 57 ++++++++++++++++++++++ util/builders_test.go | 42 ++++++++++++++++ util/deploy_service_url_calculator.go | 37 ++++++++++++-- util/deploy_service_url_calculator_test.go | 14 ++++-- util/fakes/fake_http_simple_executor.go | 16 ++++++ util/http_util.go | 25 ++++++++++ 7 files changed, 186 insertions(+), 14 deletions(-) create mode 100644 util/builders_test.go create mode 100644 util/fakes/fake_http_simple_executor.go create mode 100644 util/http_util.go diff --git a/util/archive_handler_test.go b/util/archive_handler_test.go index 31f3bb1..577aa5e 100644 --- a/util/archive_handler_test.go +++ b/util/archive_handler_test.go @@ -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" ) @@ -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")) }) }) @@ -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() { diff --git a/util/builders.go b/util/builders.go index e1d487d..c846032 100644 --- a/util/builders.go +++ b/util/builders.go @@ -1,6 +1,11 @@ package util import ( + "bytes" + "fmt" + "net/url" + "strings" + "github.com/SAP/cf-mta-plugin/clients/models" ) @@ -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 +} diff --git a/util/builders_test.go b/util/builders_test.go new file mode 100644 index 0000000..ca5363a --- /dev/null +++ b/util/builders_test.go @@ -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()) + }) + }) + + }) +}) diff --git a/util/deploy_service_url_calculator.go b/util/deploy_service_url_calculator.go index 030ba4b..9f91a27 100644 --- a/util/deploy_service_url_calculator.go +++ b/util/deploy_service_url_calculator.go @@ -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) { @@ -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 } @@ -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 +} diff --git a/util/deploy_service_url_calculator_test.go b/util/deploy_service_url_calculator_test.go index e116067..c18bd28 100644 --- a/util/deploy_service_url_calculator_test.go +++ b/util/deploy_service_url_calculator_test.go @@ -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" @@ -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")) }) }) @@ -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")) }) }) @@ -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)) }) @@ -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.")) }) diff --git a/util/fakes/fake_http_simple_executor.go b/util/fakes/fake_http_simple_executor.go new file mode 100644 index 0000000..b460fe9 --- /dev/null +++ b/util/fakes/fake_http_simple_executor.go @@ -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 +} diff --git a/util/http_util.go b/util/http_util.go new file mode 100644 index 0000000..9743405 --- /dev/null +++ b/util/http_util.go @@ -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 +}