Review Feedback

This commit is contained in:
Paul Bellamy
2015-10-20 14:26:27 +01:00
parent 986abd24cd
commit a1466cb3fc
9 changed files with 30 additions and 31 deletions

1
.gitignore vendored
View File

@@ -43,6 +43,7 @@ docker/scope-probe
docker/docker*
docker/weave
docker/runsvinit
docker/ca-certificates.crt
experimental/bridge/bridge
experimental/demoprobe/demoprobe
experimental/fixprobe/fixprobe

View File

@@ -29,19 +29,22 @@ docker/weave:
curl -L git.io/weave -o docker/weave
chmod u+x docker/weave
$(SCOPE_EXPORT): $(APP_EXE) $(PROBE_EXE) $(DOCKER_DISTRIB) docker/weave $(RUNSVINIT) docker/Dockerfile docker/run-app docker/run-probe docker/entrypoint.sh
$(SCOPE_EXPORT): $(APP_EXE) $(PROBE_EXE) $(DOCKER_DISTRIB) docker/weave $(RUNSVINIT) docker/Dockerfile docker/run-app docker/run-probe docker/entrypoint.sh docker/ca-certificates.crt
@if [ -z '$(DOCKER_SQUASH)' ] ; then echo "Please install docker-squash by running 'make deps' (and make sure GOPATH/bin is in your PATH)." && exit 1 ; fi
cp $(APP_EXE) $(PROBE_EXE) docker/
cp $(DOCKER_DISTRIB) docker/docker.tgz
$(SUDO) docker build -t $(SCOPE_IMAGE) docker/
$(SUDO) docker save $(SCOPE_IMAGE):latest | sudo $(DOCKER_SQUASH) -t $(SCOPE_IMAGE) | tee $@ | $(SUDO) docker load
docker/ca-certificates.crt: /etc/ssl/certs/ca-certificates.crt
cp $? $@
$(RUNSVINIT): vendor/runsvinit/*.go
go build -o $@ github.com/weaveworks/scope/vendor/runsvinit
$(APP_EXE): app/*.go render/*.go report/*.go xfer/*.go
$(APP_EXE): app/*.go render/*.go report/*.go xfer/*.go common/sanitize/*.go
$(PROBE_EXE): probe/*.go probe/docker/*.go probe/kubernetes/*.go probe/endpoint/*.go probe/host/*.go probe/process/*.go probe/overlay/*.go report/*.go xfer/*.go
$(PROBE_EXE): probe/*.go probe/docker/*.go probe/kubernetes/*.go probe/endpoint/*.go probe/host/*.go probe/process/*.go probe/overlay/*.go report/*.go xfer/*.go common/sanitize/*.go common/exec/*.go
$(APP_EXE) $(PROBE_EXE):
go get -d -tags netgo ./$(@D)

View File

@@ -10,29 +10,29 @@ import (
// URL returns a function that sanitizes a URL string. It lets underspecified
// strings to be converted to usable URLs via some default arguments.
func URL(scheme string, port int, path string) func(string) string {
if scheme == "" {
scheme = "http://"
func URL(defaultScheme string, defaultPort int, defaultPath string) func(string) string {
if defaultScheme == "" {
defaultScheme = "http://"
}
return func(s string) string {
if s == "" {
return s // can't do much here
}
if !strings.HasPrefix(s, "http") {
s = scheme + s
if !strings.Contains(s, "://") {
s = defaultScheme + s
}
u, err := url.Parse(s)
if err != nil {
log.Printf("%q: %v", s, err)
return s // oh well
}
if port > 0 {
if _, _, err = net.SplitHostPort(u.Host); err != nil {
u.Host += fmt.Sprintf(":%d", port)
}
if _, port, err := net.SplitHostPort(u.Host); err != nil && defaultPort > 0 {
u.Host += fmt.Sprintf(":%d", defaultPort)
} else if port == "443" {
u.Scheme = "https"
}
if path != "" && u.Path != path {
u.Path = path
if defaultPath != "" && u.Path != defaultPath {
u.Path = defaultPath
}
return u.String()
}

View File

@@ -22,9 +22,12 @@ func TestSanitizeURL(t *testing.T) {
{"https://", 0, "", "foo", "https://foo"},
{"https://", 80, "", "foo", "https://foo:80"},
{"https://", 0, "some/path", "foo", "https://foo/some/path"},
{"https://", 0, "", "http://foo", "http://foo"}, // specified scheme beats default...
{"", 9999, "", "foo:80", "http://foo:80"}, // specified port beats default...
{"", 0, "/bar", "foo/baz", "http://foo/bar"}, // ...but default path beats specified!
{"https://", 0, "", "http://foo", "http://foo"}, // specified scheme beats default...
{"", 0, "", "https://foo", "https://foo"}, // https can be a specified scheme without default...
{"http://", 0, "", "https://foo", "https://foo"}, // https can be a specified scheme with default...
{"", 9999, "", "foo:80", "http://foo:80"}, // specified port beats default...
{"", 0, "/bar", "foo/baz", "http://foo/bar"}, // ...but default path beats specified!
{"", 0, "", "foo:443", "https://foo:443"}, // port 443 addrs default to https scheme
} {
if want, have := input.want, sanitize.URL(input.scheme, input.port, input.path)(input.input); want != have {
t.Errorf("sanitize.URL(%q, %d, %q)(%q): want %q, have %q", input.scheme, input.port, input.path, input.input, want, have)

View File

@@ -10,5 +10,6 @@ ADD ./weave /usr/bin/
COPY ./scope-app ./scope-probe ./runsvinit ./entrypoint.sh /home/weave/
COPY ./run-app /etc/service/app/run
COPY ./run-probe /etc/service/probe/run
COPY ./ca-certificates.crt /etc/ssl/certs/
EXPOSE 4040
ENTRYPOINT ["/home/weave/entrypoint.sh"]

View File

@@ -23,7 +23,7 @@ func main() {
)
flag.Parse()
_, publisher, err := xfer.NewHTTPPublisher(*publish, "demoprobe", "demoprobe")
_, publisher, err := xfer.NewHTTPPublisher(*publish, "demoprobe", "demoprobe", false)
if err != nil {
log.Fatal(err)
}

View File

@@ -34,7 +34,7 @@ func main() {
}
f.Close()
_, publisher, err := xfer.NewHTTPPublisher(*publish, "fixprobe", "fixprobe")
_, publisher, err := xfer.NewHTTPPublisher(*publish, "fixprobe", "fixprobe", false)
if err != nil {
log.Fatal(err)
}

View File

@@ -5,7 +5,6 @@ import (
"encoding/json"
"fmt"
"io"
"net"
"net/http"
"time"
@@ -26,16 +25,8 @@ var fastClient = &http.Client{
// NewHTTPPublisher returns an HTTPPublisher ready for use.
func NewHTTPPublisher(target, token, probeID string, insecure bool) (string, *HTTPPublisher, error) {
_, port, err := net.SplitHostPort(target)
if err != nil {
return "", nil, err
}
scheme := "http"
if port == "443" {
scheme = "https"
}
p := &HTTPPublisher{
url: sanitize.URL(scheme+"://", 0, "/api/report")(target),
url: sanitize.URL("", 0, "/api/report")(target),
token: token,
probeID: probeID,
client: http.DefaultClient,
@@ -45,7 +36,7 @@ func NewHTTPPublisher(target, token, probeID string, insecure bool) (string, *HT
allowInsecure(fastClient)
allowInsecure(p.client)
}
req, err := p.authorizedRequest("GET", sanitize.URL(scheme+"://", 0, "/api")(target), nil)
req, err := p.authorizedRequest("GET", sanitize.URL("", 0, "/api")(target), nil)
if err != nil {
return "", nil, err
}

View File

@@ -67,7 +67,7 @@ func TestHTTPPublisher(t *testing.T) {
s := httptest.NewServer(handlers.CompressHandler(handler))
defer s.Close()
_, p, err := xfer.NewHTTPPublisher(s.URL, token, id)
_, p, err := xfer.NewHTTPPublisher(s.URL, token, id, false)
if err != nil {
t.Fatal(err)
}