Skip to content

chore: integration: add test for pushing to cache repo that requires auth #233

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 12, 2024
Merged

chore: integration: add test for pushing to cache repo that requires auth #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd8b0fd
chore: integration: add test for pushing to cache repo that requires …
johnstcn Jun 12, 2024
be9c602
move gittest.BasicAuthMW to mwtest package
johnstcn Jun 12, 2024
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
7 changes: 4 additions & 3 deletions git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/coder/envbuilder"
"github.com/coder/envbuilder/internal/notcodersdk"
"github.com/coder/envbuilder/testutil/gittest"
"github.com/coder/envbuilder/testutil/mwtest"
"github.com/go-git/go-billy/v5"
"github.com/go-git/go-billy/v5/memfs"
"github.com/go-git/go-billy/v5/osfs"
Expand Down Expand Up @@ -82,7 +83,7 @@ func TestCloneRepo(t *testing.T) {
t.Run("AlreadyCloned", func(t *testing.T) {
srvFS := memfs.New()
_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "README.md", "Hello, world!", "Wow!"))
authMW := gittest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
authMW := mwtest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
srv := httptest.NewServer(authMW(gittest.NewServer(srvFS)))
clientFS := memfs.New()
// A repo already exists!
Expand All @@ -101,7 +102,7 @@ func TestCloneRepo(t *testing.T) {
t.Parallel()
srvFS := memfs.New()
_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "README.md", "Hello, world!", "Wow!"))
authMW := gittest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
authMW := mwtest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
srv := httptest.NewServer(authMW(gittest.NewServer(srvFS)))
clientFS := memfs.New()

Expand Down Expand Up @@ -134,7 +135,7 @@ func TestCloneRepo(t *testing.T) {
t.Parallel()
srvFS := memfs.New()
_ = gittest.NewRepo(t, srvFS, gittest.Commit(t, "README.md", "Hello, world!", "Wow!"))
authMW := gittest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
authMW := mwtest.BasicAuthMW(tc.srvUsername, tc.srvPassword)
srv := httptest.NewServer(authMW(gittest.NewServer(srvFS)))

authURL, err := url.Parse(srv.URL)
Expand Down
176 changes: 156 additions & 20 deletions integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/coder/envbuilder"
"github.com/coder/envbuilder/devcontainer/features"
"github.com/coder/envbuilder/testutil/gittest"
"github.com/coder/envbuilder/testutil/mwtest"
"github.com/coder/envbuilder/testutil/registrytest"
clitypes "github.com/docker/cli/cli/config/types"
"github.com/docker/docker/api/types"
Expand Down Expand Up @@ -776,7 +777,7 @@ func TestPrivateRegistry(t *testing.T) {
t.Parallel()
// Even if something goes wrong with auth,
// the pull will fail as "scratch" is a reserved name.
image := setupPassthroughRegistry(t, "scratch", &registryAuth{
image := setupPassthroughRegistry(t, "scratch", &setupPassthroughRegistryOptions{
Username: "user",
Password: "test",
})
Expand All @@ -795,7 +796,7 @@ func TestPrivateRegistry(t *testing.T) {
})
t.Run("Auth", func(t *testing.T) {
t.Parallel()
image := setupPassthroughRegistry(t, "envbuilder-test-alpine:latest", &registryAuth{
image := setupPassthroughRegistry(t, "envbuilder-test-alpine:latest", &setupPassthroughRegistryOptions{
Username: "user",
Password: "test",
})
Expand Down Expand Up @@ -827,7 +828,7 @@ func TestPrivateRegistry(t *testing.T) {
t.Parallel()
// Even if something goes wrong with auth,
// the pull will fail as "scratch" is a reserved name.
image := setupPassthroughRegistry(t, "scratch", &registryAuth{
image := setupPassthroughRegistry(t, "scratch", &setupPassthroughRegistryOptions{
Username: "user",
Password: "banana",
})
Expand Down Expand Up @@ -857,38 +858,43 @@ func TestPrivateRegistry(t *testing.T) {
})
}

type registryAuth struct {
type setupPassthroughRegistryOptions struct {
Username string
Password string
Upstream string
}

func setupPassthroughRegistry(t *testing.T, image string, auth *registryAuth) string {
func setupPassthroughRegistry(t *testing.T, image string, opts *setupPassthroughRegistryOptions) string {
t.Helper()
dockerURL, err := url.Parse("http://localhost:5000")
if opts.Upstream == "" {
// Default to local test registry
opts.Upstream = "http://localhost:5000"
}
upstreamURL, err := url.Parse(opts.Upstream)
require.NoError(t, err)
proxy := httputil.NewSingleHostReverseProxy(dockerURL)
proxy := httputil.NewSingleHostReverseProxy(upstreamURL)

// The Docker registry uses short-lived JWTs to authenticate
// anonymously to pull images. To test our MITM auth, we need to
// generate a JWT for the proxy to use.
registry, err := name.NewRegistry("localhost:5000")
registry, err := name.NewRegistry(upstreamURL.Host)
require.NoError(t, err)
proxy.Transport, err = transport.NewWithContext(context.Background(), registry, authn.Anonymous, http.DefaultTransport, []string{})
require.NoError(t, err)

srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
r.Host = "localhost:5000"
r.URL.Host = "localhost:5000"
r.URL.Scheme = "http"
r.Host = upstreamURL.Host
r.URL.Host = upstreamURL.Host
r.URL.Scheme = upstreamURL.Scheme

if auth != nil {
if opts != nil {
user, pass, ok := r.BasicAuth()
if !ok {
w.Header().Set("WWW-Authenticate", "Basic realm=\"Access to the staging site\", charset=\"UTF-8\"")
w.WriteHeader(http.StatusUnauthorized)
return
}
if user != auth.Username || pass != auth.Password {
if user != opts.Username || pass != opts.Password {
w.WriteHeader(http.StatusUnauthorized)
return
}
Expand Down Expand Up @@ -1008,7 +1014,7 @@ func TestPushImage(t *testing.T) {
})

// Given: an empty registry
testReg := setupInMemoryRegistry(t)
testReg := setupInMemoryRegistry(t, setupInMemoryRegistryOpts{})
testRepo := testReg + "/test"
ref, err := name.ParseReference(testRepo + ":latest")
require.NoError(t, err)
Expand Down Expand Up @@ -1062,7 +1068,7 @@ func TestPushImage(t *testing.T) {
})

// Given: an empty registry
testReg := setupInMemoryRegistry(t)
testReg := setupInMemoryRegistry(t, setupInMemoryRegistryOpts{})
testRepo := testReg + "/test"
ref, err := name.ParseReference(testRepo + ":latest")
require.NoError(t, err)
Expand Down Expand Up @@ -1101,6 +1107,130 @@ func TestPushImage(t *testing.T) {
require.NoError(t, err)
})

t.Run("CacheAndPushAuth", func(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

review: I may look into making this more of a table-driven test, as it's getting pretty unwieldy

Copy link
Contributor

Choose a reason for hiding this comment

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

I was gonna suggest the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had a try and there's a bit too much difference behaviour between the various test cases to really make it worthwhile IMO compared with the cognitive overhead of DRYing this.

t.Parallel()

srv := createGitServer(t, gitServerOptions{
files: map[string]string{
".devcontainer/Dockerfile": fmt.Sprintf("FROM %s\nRUN date --utc > /root/date.txt", testImageAlpine),
".devcontainer/devcontainer.json": `{
"name": "Test",
"build": {
"dockerfile": "Dockerfile"
},
}`,
},
})

// Given: an empty registry
opts := setupInMemoryRegistryOpts{
Username: "testing",
Password: "testing",
}
remoteAuthOpt := remote.WithAuth(&authn.Basic{Username: opts.Username, Password: opts.Password})
testReg := setupInMemoryRegistry(t, opts)
testRepo := testReg + "/test"
regAuthJSON, err := json.Marshal(envbuilder.DockerConfig{
AuthConfigs: map[string]clitypes.AuthConfig{
testRepo: {
Username: opts.Username,
Password: opts.Password,
},
},
})
require.NoError(t, err)
ref, err := name.ParseReference(testRepo + ":latest")
require.NoError(t, err)
_, err = remote.Image(ref, remoteAuthOpt)
require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push")

// When: we run envbuilder with GET_CACHED_IMAGE
_, err = runEnvbuilder(t, options{env: []string{
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("CACHE_REPO", testRepo),
envbuilderEnv("GET_CACHED_IMAGE", "1"),
}})
require.ErrorContains(t, err, "error probing build cache: uncached command")
// Then: it should fail to build the image and nothing should be pushed
_, err = remote.Image(ref, remoteAuthOpt)
require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push")

// When: we run envbuilder with PUSH_IMAGE set
_, err = runEnvbuilder(t, options{env: []string{
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("CACHE_REPO", testRepo),
envbuilderEnv("PUSH_IMAGE", "1"),
envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString(regAuthJSON)),
}})
require.NoError(t, err)

// Then: the image should be pushed
_, err = remote.Image(ref, remoteAuthOpt)
require.NoError(t, err, "expected image to be present after build + push")

// Then: re-running envbuilder with GET_CACHED_IMAGE should succeed
_, err = runEnvbuilder(t, options{env: []string{
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("CACHE_REPO", testRepo),
envbuilderEnv("GET_CACHED_IMAGE", "1"),
envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString(regAuthJSON)),
}})
require.NoError(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any more specificity we could add here? The absence of an error is fine, but validating the cached image exists explicitly would be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

see L1168/1169

})

t.Run("CacheAndPushAuthFail", func(t *testing.T) {
t.Parallel()

srv := createGitServer(t, gitServerOptions{
files: map[string]string{
".devcontainer/Dockerfile": fmt.Sprintf("FROM %s\nRUN date --utc > /root/date.txt", testImageAlpine),
".devcontainer/devcontainer.json": `{
"name": "Test",
"build": {
"dockerfile": "Dockerfile"
},
}`,
},
})

// Given: an empty registry
opts := setupInMemoryRegistryOpts{
Username: "testing",
Password: "testing",
}
remoteAuthOpt := remote.WithAuth(&authn.Basic{Username: opts.Username, Password: opts.Password})
testReg := setupInMemoryRegistry(t, opts)
testRepo := testReg + "/test"
ref, err := name.ParseReference(testRepo + ":latest")
require.NoError(t, err)
_, err = remote.Image(ref, remoteAuthOpt)
require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push")

// When: we run envbuilder with GET_CACHED_IMAGE
_, err = runEnvbuilder(t, options{env: []string{
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("CACHE_REPO", testRepo),
envbuilderEnv("GET_CACHED_IMAGE", "1"),
}})
require.ErrorContains(t, err, "error probing build cache: uncached command")
// Then: it should fail to build the image and nothing should be pushed
_, err = remote.Image(ref, remoteAuthOpt)
require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push")

// When: we run envbuilder with PUSH_IMAGE set
_, err = runEnvbuilder(t, options{env: []string{
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("CACHE_REPO", testRepo),
envbuilderEnv("PUSH_IMAGE", "1"),
}})
// Then: it should fail with an Unauthorized error
require.ErrorContains(t, err, "401 Unauthorized", "expected unauthorized error using no auth when cache repo requires it")

// Then: the image should not be pushed
_, err = remote.Image(ref, remoteAuthOpt)
require.ErrorContains(t, err, "NAME_UNKNOWN", "expected image to not be present before build + push")
})

t.Run("CacheAndPushMultistage", func(t *testing.T) {
// Currently fails with:
// /home/coder/src/coder/envbuilder/integration/integration_test.go:1417: "error: unable to get cached image: error fake building stage: failed to optimize instructions: failed to get files used from context: failed to get fileinfo for /.envbuilder/0/root/date.txt: lstat /.envbuilder/0/root/date.txt: no such file or directory"
Expand All @@ -1122,7 +1252,7 @@ COPY --from=a /root/date.txt /date.txt`, testImageAlpine, testImageAlpine),
})

// Given: an empty registry
testReg := setupInMemoryRegistry(t)
testReg := setupInMemoryRegistry(t, setupInMemoryRegistryOpts{})
testRepo := testReg + "/test"
ref, err := name.ParseReference(testRepo + ":latest")
require.NoError(t, err)
Expand Down Expand Up @@ -1224,11 +1354,17 @@ COPY --from=a /root/date.txt /date.txt`, testImageAlpine, testImageAlpine),
})
}

func setupInMemoryRegistry(t *testing.T) string {
type setupInMemoryRegistryOpts struct {
Username string
Password string
}

func setupInMemoryRegistry(t *testing.T, opts setupInMemoryRegistryOpts) string {
t.Helper()
tempDir := t.TempDir()
testReg := registry.New(registry.WithBlobHandler(registry.NewDiskBlobHandler(tempDir)))
regSrv := httptest.NewServer(testReg)
regHandler := registry.New(registry.WithBlobHandler(registry.NewDiskBlobHandler(tempDir)))
authHandler := mwtest.BasicAuthMW(opts.Username, opts.Password)(regHandler)
regSrv := httptest.NewServer(authHandler)
t.Cleanup(func() { regSrv.Close() })
regSrvURL, err := url.Parse(regSrv.URL)
require.NoError(t, err)
Expand Down Expand Up @@ -1274,7 +1410,7 @@ type gitServerOptions struct {
func createGitServer(t *testing.T, opts gitServerOptions) *httptest.Server {
t.Helper()
if opts.authMW == nil {
opts.authMW = gittest.BasicAuthMW(opts.username, opts.password)
opts.authMW = mwtest.BasicAuthMW(opts.username, opts.password)
}
commits := make([]gittest.CommitFunc, 0)
for path, content := range opts.files {
Expand Down
15 changes: 0 additions & 15 deletions testutil/gittest/gittest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,18 +249,3 @@ func WriteFile(t *testing.T, fs billy.Filesystem, path, content string) {
err = file.Close()
require.NoError(t, err)
}

Copy link
Member Author

Choose a reason for hiding this comment

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

review: moved as this isn't specific to testing git stuff

func BasicAuthMW(username, password string) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if username != "" || password != "" {
authUser, authPass, ok := r.BasicAuth()
if !ok || username != authUser || password != authPass {
w.WriteHeader(http.StatusUnauthorized)
return
}
}
next.ServeHTTP(w, r)
})
}
}
18 changes: 18 additions & 0 deletions testutil/mwtest/auth_basic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package mwtest

import "net/http"

func BasicAuthMW(username, password string) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if username != "" || password != "" {
authUser, authPass, ok := r.BasicAuth()
if !ok || username != authUser || password != authPass {
w.WriteHeader(http.StatusUnauthorized)
return
}
}
next.ServeHTTP(w, r)
})
}
}