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

[e2e]: Add tests for dockerv2 #309

Merged
merged 7 commits into from Dec 23, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Expand Up @@ -12,6 +12,8 @@ cache:

before_script:
- docker pull k8s.gcr.io/coredns:1.6.2
- docker pull registry:2
- go get -u github.com/google/go-containerregistry/cmd/crane
stp-ip marked this conversation as resolved.
Show resolved Hide resolved

script:
- go vet
Expand Down
1 change: 1 addition & 0 deletions Makefile
Expand Up @@ -36,6 +36,7 @@ endtoend-test: docker-build
docker build -t $(IMAGE)-dirty .
cd e2e && \
docker build -t c.txtdirect.org/tester:dirty . && \
docker run -d -p 5000:5000 --name registry registry:2 && \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pin this to a specific docker registry version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The currently available tags are:

  • 2.7.1, 2.7, 2, latest
  • 2.6.2, 2.6

Since "2.7.1, 2.7, 2" are all the same image, is it needed to switch the tag?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question is how much we trust Docker not to break during minor versions.
We could be specific and use 2.7.1, but happy to keep it as is. Up to you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree that Docker may mess up something during minor versions.
Will change it to 2.7.1.

VERSION=$(VERSION)-dirty go run main.go

version:
Expand Down
10 changes: 0 additions & 10 deletions dockerv2.go
Expand Up @@ -79,16 +79,6 @@ func (d *Dockerv2) Redirect() error {
return nil
}

// ValidAgent checks the User-Agent header to be sure it's Docker-Client
func (d *Dockerv2) ValidAgent() bool {
if !strings.Contains(d.req.Header.Get("User-Agent"), "Docker-Client") {
log.Println("[txtdirect]: The request is not from docker client, fallback triggered.")
fallback(d.rw, d.req, "to", d.rec.Code, d.c)
return false
}
return true
}

func createDockerv2URI(to string, path string) (string, error) {
uri, err := url.Parse(to)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions e2e/Dockerfile
Expand Up @@ -4,4 +4,6 @@ RUN apk --no-cache add git

WORKDIR /e2e

RUN GO111MODULE=on go get -u github.com/google/go-containerregistry/cmd/crane
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be pinnned to a specific version I think.
Two options:

Create a go.mod you use to track your globally installed tools, such as in ~/global-tools/go.mod, and cd to that directory prior to running go get or go install for any globally installed tools.

Create a go.mod for each tool in separate directories, such as ~/tools/gorename/go.mod and ~/tools/goimports/go.mod, and cd to that appropriate directory prior to running go get or go install for the tool.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we control the versions using the @Version at the end of go get command and manage them only inside the Dockerfile? Since we're only using crane's binary, I'm not sure if go.mod is needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Simpler and same effect. +1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since crane doesn't have specific versions, I used 34fb8ff which is the hash for insecure flag PR's merge commit.


CMD [ "go" ]
150 changes: 95 additions & 55 deletions e2e/dockerv2/main.go
@@ -1,34 +1,38 @@
package main

import (
"errors"
"fmt"
"log"
"net/http"
"os/exec"
)

type data struct {
host string
path string
host string
path string
image string
}

type Kind string

const (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the abstraction of Kind?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thought it may look better than direct string comparison. Will change it if it's unnecessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes the overall code a bit harder to understand in my view as the string for each type wouldn't change frequently anyway. Ok to keep it as is, but feel like this might be an over optimization for nicer code > readability. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, It would've been useful if we had lots of types and not only pull and push. And since they're not gonna get changed at all I guess going with strings instead of Kind type is better rn.

Pull Kind = "pull"
Push Kind = "push"
)

type test struct {
name string
args data
expected string
referer bool
status int
headers http.Header
kind Kind
fallback bool
}

var tests = []test{
{
name: "Fallback to \"to=\" when Docker-Client header is not set",
args: data{
host: "fallback.dockerv2.dockerv2.example.com",
path: "/",
},
expected: "https://fallback-to.dockerv2.dockerv2.example.com",
},
{
name: "Fallback to \"root=\"",
args: data{
Expand All @@ -39,6 +43,7 @@ var tests = []test{
"User-Agent": []string{"Docker-Client"},
},
expected: "https://fallback-root.dockerv2.dockerv2.example.com",
fallback: true,
},
{
name: "Fallback to 404 when \"to=\" is not linked to a Docker image or registry",
Expand All @@ -49,69 +54,104 @@ var tests = []test{
headers: http.Header{
"User-Agent": []string{"Docker-Client"},
},
status: 404,
status: 404,
fallback: true,
},
{
name: "Pull an image from the registry",
args: data{
image: "pull.dockerv2.dockerv2.example.com/txtdirect",
},
kind: Pull,
},
}

func main() {
result := make(map[bool][]test)
for _, test := range tests {
client := &http.Client{
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
},
}

req, err := http.NewRequest("GET", fmt.Sprintf("http://%s%s", test.args.host, test.args.path), nil)
if err != nil {
result[false] = append(result[false], test)
log.Printf("[%s]: Couldn't create the request: %s", test.name, err.Error())
continue
}

req.Header = test.headers

resp, err := client.Do(req)
if err != nil {
result[false] = append(result[false], test)
log.Printf("[%s]: Couldn't send the request: %s", test.name, err.Error())
continue
if test.fallback {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the restructure on the fallback.

if err := fallbackTest(test, result); err != nil {
continue
}
}

// Check response's status code
if test.status != 0 {
if resp.StatusCode != test.status {
switch test.kind {
case Pull:
_, err := exec.Command("crane", "pull", "-i", test.args.image, "txtdirect_e2e").CombinedOutput()
if err != nil {
result[false] = append(result[false], test)
log.Printf("Couldn't pull the image from the custom Docker registry: %s", err.Error())
continue
}
_, err = exec.Command("rm", "./txtdirect_e2e").CombinedOutput()
if err != nil {
result[false] = append(result[false], test)
log.Printf("[%s]: Expected %d status code, got %d", test.name, test.status, resp.StatusCode)
log.Printf("Couldn't remove the pulled image: %s", err.Error())
continue
}
result[true] = append(result[true], test)
continue
}

// Check "Location" header for redirects
location, err := resp.Location()
result[true] = append(result[true], test)
}
log.Printf("TestCase: \"dockerv2\", Total: %d, Passed: %d, Failed: %d", len(tests), len(result[true]), len(result[false]))
}

if err == http.ErrNoLocation {
result[false] = append(result[false], test)
log.Printf("[%s]: Location header is empty", test.name)
continue
}
// fallbackTest handles the fallback tests and returns an empty non-nil error
// in case of failure.
func fallbackTest(test test, result map[bool][]test) error {
client := &http.Client{
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
},
}

if location.String() != test.expected {
result[false] = append(result[false], test)
log.Printf("[%s]: Expected %s, got %s", test.name, test.expected, location)
continue
}
req, err := http.NewRequest("GET", fmt.Sprintf("http://%s%s", test.args.host, test.args.path), nil)
if err != nil {
result[false] = append(result[false], test)
log.Printf("[%s]: Couldn't create the request: %s", test.name, err.Error())
return errors.New("")
}

// Check "Referer" header if enabled
if test.referer && resp.Header.Get("Referer") != req.Host {
req.Header = test.headers

resp, err := client.Do(req)
if err != nil {
result[false] = append(result[false], test)
log.Printf("[%s]: Couldn't send the request: %s", test.name, err.Error())
return errors.New("")
}

// Check response's status code
if test.status != 0 {
if resp.StatusCode != test.status {
result[false] = append(result[false], test)
log.Printf("Expected %s referer but got \"%s\"", req.Host, resp.Header.Get("Referer"))
continue
log.Printf("[%s]: Expected %d status code, got %d", test.name, test.status, resp.StatusCode)
return errors.New("")
}

result[true] = append(result[true], test)
return errors.New("")
}
log.Printf("TestCase: \"dockerv2\", Total: %d, Passed: %d, Failed: %d", len(tests), len(result[true]), len(result[false]))

// Check "Location" header for redirects
location, err := resp.Location()

if err == http.ErrNoLocation {
result[false] = append(result[false], test)
log.Printf("[%s]: Location header is empty", test.name)
return errors.New("")
}

if location.String() != test.expected {
result[false] = append(result[false], test)
log.Printf("[%s]: Expected %s, got %s", test.name, test.expected, location)
return errors.New("")
}

// Check "Referer" header if enabled
if test.referer && resp.Header.Get("Referer") != req.Host {
result[false] = append(result[false], test)
log.Printf("Expected %s referer but got \"%s\"", req.Host, resp.Header.Get("Referer"))
return errors.New("")
}
return nil
}
3 changes: 3 additions & 0 deletions e2e/dockerv2/zonefile
Expand Up @@ -12,6 +12,9 @@ $TTL 1h

example.com. IN A 172.20.10.2

pull.dockerv2.dockerv2.example.com. IN A 172.20.10.2
_redirect.pull.dockerv2.dockerv2.example.com. IN TXT "v=txtv0;to=http://172.20.10.3:5000/txtdirect;type=dockerv2"

fallback.dockerv2.dockerv2.example.com. IN A 172.20.10.2
_redirect.fallback.dockerv2.dockerv2.example.com. IN TXT "v=txtv0;to=https://fallback-to.dockerv2.dockerv2.example.com;root=https://fallback-root.dockerv2.dockerv2.example.com;type=dockerv2"

Expand Down
70 changes: 64 additions & 6 deletions e2e/main.go
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"
"regexp"
"strconv"
"strings"
)

const (
Expand Down Expand Up @@ -69,6 +70,10 @@ func main() {
log.Fatalf("[txtdirect_e2e]: Couldn't examine the logs and count the stats: %s", err.Error())
}

if err := d.RemoveNetwork(); err != nil {
log.Fatalf("[txtdirect_e2e]: Couldn't remove the network adaptor: %s", err.Error())
}

if d.stats.failed != 0 {
log.Fatalf("[txtdirect_e2e]: Tests failed because there were %d failed tests.", d.stats.failed)
}
Expand Down Expand Up @@ -162,10 +167,49 @@ func (d *dockerManager) StartContainers() error {
return fmt.Errorf("Couldn't create the TXTDirect container: %s", err.Error())
}

// Connect the Docker registry to the e2e network
if strings.Contains(d.dir, "dockerv2") {
_, err := exec.Command("docker", "network", "connect", "coretxtd", "registry").CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't connect Docker registry the network adaptor: %s", err.Error())
}

// Tag TXTDirect's image to use in custom registry
_, err = exec.Command("docker", "tag", txtdirectImage, "172.20.10.3:5000/txtdirect").CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't tag image to use in custom Docker registry: %s", err.Error())
}

_, err = exec.Command("docker", "save", "172.20.10.3:5000/txtdirect:latest", "-o", "txtdirect.tar").CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't export the image into a tarball: %s", err.Error())
}

// Push the TXTDirect image to the custom registry
_, err = exec.Command("crane", "push", "txtdirect.tar", "172.20.10.3:5000/txtdirect").CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't push the image to the custom Docker registry: %s", err.Error())
}

_, err = exec.Command("rm", "./txtdirect.tar").CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't remove the image tarball: %s", err.Error())
}
}

return nil
}

func (d *dockerManager) StopContainers() error {
if strings.Contains(d.dir, "dockerv2") {
_, err := exec.Command("docker",
"logs",
"e2e_txtdirect_container",
).CombinedOutput()
if err != nil {
return err
}
}
_, err := exec.Command("docker",
"container", "rm", "-f",
"e2e_coredns_container",
Expand All @@ -190,12 +234,15 @@ func (d *dockerManager) StopContainers() error {
return fmt.Errorf("Couldn't remove the tester container: %s", err.Error())
}

_, err = exec.Command("docker",
"network", "rm",
"coretxtd",
).CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't remove the TXTDirect container: %s", err.Error())
if strings.Contains(d.dir, "dockerv2") {
_, err = exec.Command("docker",
"network", "disconnect",
"coretxtd",
"registry",
).CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't disconnect the Docker registry container: %s", err.Error())
}
}

return nil
Expand Down Expand Up @@ -265,3 +312,14 @@ func (d *dockerManager) CountStats(stats []string) error {

return nil
}

func (d *dockerManager) RemoveNetwork() error {
_, err := exec.Command("docker",
"network", "rm",
"coretxtd",
).CombinedOutput()
if err != nil {
return fmt.Errorf("Couldn't remove the network adaptor: %s", err.Error())
}
return nil
}
5 changes: 0 additions & 5 deletions txtdirect.go
Expand Up @@ -248,11 +248,6 @@ func Redirect(w http.ResponseWriter, r *http.Request, c Config) error {

docker := NewDockerv2(w, r, rec, c)

// Fallback gets triggered if the User-Agent isn't valid
if !docker.ValidAgent() {
return nil
}

if err := docker.Redirect(); err != nil {
log.Printf("[txtdirect]: couldn't redirect to the requested container: %s", err.Error())
fallback(w, r, "to", rec.Code, c)
Expand Down