diff --git a/rkt/image/fetcher.go b/rkt/image/fetcher.go index b6eb0edf25..255bd131a9 100644 --- a/rkt/image/fetcher.go +++ b/rkt/image/fetcher.go @@ -90,10 +90,10 @@ func (f *Fetcher) addImageDeps(hash string, imgsl *list.List, seen map[string]st return fmt.Errorf("one of image ID's %q dependencies (image %q) is invalid: %v", hash, imgName, err) } appStr := app.String() - imgsl.PushBack(appStr) if _, ok := seen[appStr]; ok { - return fmt.Errorf("dependency %q specified multiple times in the dependency tree for image ID %q", appStr, hash) + continue } + imgsl.PushBack(app.String()) seen[appStr] = struct{}{} } return nil diff --git a/tests/rkt_auth_test.go b/tests/rkt_auth_test.go index 24c257a606..ae5921052a 100644 --- a/tests/rkt_auth_test.go +++ b/tests/rkt_auth_test.go @@ -143,30 +143,6 @@ func testAuthIgnoreSubdirectories(t *testing.T, server *taas.Server) { expectedRunRkt(ctx, t, server.URL, "oauth-subdirectories", authFailedDownload) } -func runServer(t *testing.T, auth taas.Type) *taas.Server { - actool := testutils.GetValueFromEnvOrPanic("ACTOOL") - gotool := testutils.GetValueFromEnvOrPanic("GO") - server, err := taas.NewServerWithPaths(auth, 20, actool, gotool) - if err != nil { - t.Fatalf("Could not start server: %v", err) - } - go serverHandler(t, server) - return server -} - -func serverHandler(t *testing.T, server *taas.Server) { - for { - select { - case msg, ok := <-server.Msg: - if ok { - t.Logf("server: %v", msg) - } - case <-server.Stop: - return - } - } -} - // expectedRunRkt tries to fetch and run a prog.aci from host within // given directory on host. Note that directory can be anything - it's // useful for ensuring that image name is unique and for descriptive diff --git a/tests/rkt_image_dependencies_test.go b/tests/rkt_image_dependencies_test.go new file mode 100644 index 0000000000..0c496bb69d --- /dev/null +++ b/tests/rkt_image_dependencies_test.go @@ -0,0 +1,226 @@ +// Copyright 2015 The rkt Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "flag" + "fmt" + "io/ioutil" + "os" + "strings" + "testing" + + taas "github.com/coreos/rkt/tests/test-auth-server/aci" + "github.com/coreos/rkt/tests/testutils" +) + +const ( + manifestDepsTemplate = ` +{ + "acKind" : "ImageManifest", + "acVersion" : "0.7.3", + "dependencies" : [ + DEPENDENCIES + ], + "labels" : [ + { + "name" : "version", + "value" : "VERSION" + }, + { + "name" : "arch", + "value" : "amd64" + }, + { + "value" : "linux", + "name" : "os" + } + ], + "app" : { + "user" : "0", + "exec" : [ + "/inspect", "--print-msg=HelloDependencies" + ], + "workingDirectory" : "/", + "group" : "0", + "environment" : [ + ] + }, + "name" : "IMG_NAME" +} +` +) + +// TestImageDependencies generates ACIs with a complex dependency tree and +// fetches them via the discovery mechanism. Some dependencies are already +// cached in the CAS, and some dependencies are fetched via the discovery +// mechanism. This is to reproduce the scenario in explained in: +// https://github.com/coreos/rkt/issues/1752#issue-117121841 +func TestImageDependencies(t *testing.T) { + tmpDir := createTempDirOrPanic("rkt-TestImageDeps-") + defer os.RemoveAll(tmpDir) + + ctx := testutils.NewRktRunCtx() + defer ctx.Cleanup() + + // TODO: we can avoid setting the port manually when appc/spec gains + // the ability to specify ports for discovery. + // See https://github.com/appc/spec/pull/110 + // + // httptest by default uses random ports. We override this via the + // "httptest.serve" flag. + // + // As long as we set the port via the "httptest.serve" flag, we have + // to use https rather than http because httptest.Start() would wait + // forever in "select {}", see + // https://golang.org/src/net/http/httptest/server.go?s=2768:2792#L92 + // + // This means this test must: + // - use https only + // - ignore tls errors with --insecure-options=tls + serverURL := flag.Lookup("httptest.serve") + if serverURL == nil { + panic("could not find the httptest.serve flag") + } + serverURL.Value.Set("127.0.0.1:443") + + server := runServer(t, taas.None) + if server == nil { + panic("could not start the https test server") + } + defer server.Close() + + // reset httptest.serve to "" so we don't influence other tests + serverURL.Value.Set("") + + baseImage := getInspectImagePath() + _ = importImageAndFetchHash(t, ctx, baseImage) + emptyImage := getEmptyImagePath() + fileSet := make(map[string]string) + + // Scenario from https://github.com/coreos/rkt/issues/1752#issue-117121841 + // + // A->B + // A->C + // A->D + // + // B: prefetched + // + // C->B + // C->E + // + // D->B + // D->E + + topImage := "localhost/image-a" + imageList := []struct { + shortName string + imageName string + deps string + version string + prefetch bool + + manifest string + fileName string + }{ + { + shortName: "a", + imageName: topImage, + deps: `{"imageName":"localhost/image-b"}, {"imageName":"localhost/image-c"}, {"imageName":"localhost/image-d"}`, + version: "1", + }, + { + shortName: "b", + imageName: "localhost/image-b", + deps: ``, + version: "1", + prefetch: true, + }, + { + shortName: "c", + imageName: "localhost/image-c", + deps: `{"imageName":"localhost/image-b"}, {"imageName":"localhost/image-e", "labels": [{"name": "version", "value": "1"}]}`, + version: "1", + }, + { + shortName: "d", + imageName: "localhost/image-d", + deps: `{"imageName":"localhost/image-b"}, {"imageName":"localhost/image-e", "labels": [{"name": "version", "value": "1"}]}`, + version: "1", + }, + { + shortName: "e", + imageName: "localhost/image-e", + deps: `{"imageName":"coreos.com/rkt-inspect"}`, + version: "1", + }, + } + + for i, _ := range imageList { + // We need a reference rather than a new copy from "range" + // because we modify the content + img := &imageList[i] + + img.manifest = manifestDepsTemplate + img.manifest = strings.Replace(img.manifest, "IMG_NAME", img.imageName, -1) + img.manifest = strings.Replace(img.manifest, "DEPENDENCIES", img.deps, -1) + img.manifest = strings.Replace(img.manifest, "VERSION", img.version, -1) + + tmpManifest, err := ioutil.TempFile(tmpDir, "manifest-"+img.shortName+"-") + if err != nil { + panic(fmt.Sprintf("Cannot create temp manifest: %v", err)) + } + defer os.Remove(tmpManifest.Name()) + if err := ioutil.WriteFile(tmpManifest.Name(), []byte(img.manifest), 0600); err != nil { + panic(fmt.Sprintf("Cannot write to temp manifest: %v", err)) + } + + baseName := "image-" + img.shortName + ".aci" + img.fileName = patchACI(emptyImage, baseName, "--manifest", tmpManifest.Name()) + defer os.Remove(img.fileName) + fileSet[baseName] = img.fileName + } + + server.UpdateFileSet(fileSet) + + for i := len(imageList) - 1; i >= 0; i-- { + img := imageList[i] + if img.prefetch { + t.Logf("Importing image %q: %q", img.imageName, img.fileName) + testImageShortHash := importImageAndFetchHash(t, ctx, img.fileName) + t.Logf("Imported image %q: %s", img.imageName, testImageShortHash) + } + } + + runCmd := fmt.Sprintf("%s --debug --insecure-options=image,tls run %s", ctx.Cmd(), topImage) + child := spawnOrFail(t, runCmd) + + expectedList := []string{ + "rkt: fetching image from https://localhost/localhost/image-a.aci", + "rkt: using image from local store for image name localhost/image-b", + "rkt: fetching image from https://localhost/localhost/image-c.aci", + "rkt: fetching image from https://localhost/localhost/image-d.aci", + "rkt: using image from local store for image name coreos.com/rkt-inspect", + "HelloDependencies", + } + + for _, expected := range expectedList { + if err := expectWithOutput(child, expected); err != nil { + t.Fatalf("Expected %q but not found: %v", expected, err) + } + } + + waitOrFail(t, child, true) +} diff --git a/tests/rkt_tests.go b/tests/rkt_tests.go index 54b13c3282..fac6a8c468 100644 --- a/tests/rkt_tests.go +++ b/tests/rkt_tests.go @@ -38,6 +38,7 @@ import ( "github.com/coreos/rkt/Godeps/_workspace/src/github.com/coreos/gexpect" "github.com/coreos/rkt/Godeps/_workspace/src/google.golang.org/grpc" "github.com/coreos/rkt/api/v1alpha" + taas "github.com/coreos/rkt/tests/test-auth-server/aci" "github.com/coreos/rkt/tests/testutils" ) @@ -175,13 +176,13 @@ func createTempDirOrPanic(dirName string) string { func importImageAndFetchHashAsGid(t *testing.T, ctx *testutils.RktRunCtx, img string, gid int) string { // Import the test image into store manually. - cmd := fmt.Sprintf("%s --insecure-options=image fetch %s", ctx.Cmd(), img) + cmd := fmt.Sprintf("%s --insecure-options=image,tls fetch %s", ctx.Cmd(), img) // TODO(jonboulle): non-root user breaks trying to read root-written // config directories. Should be a better way to approach this. Should // config directories be readable by the rkt group too? if gid != 0 { - cmd = fmt.Sprintf("%s --insecure-options=image fetch %s", ctx.CmdNoConfig(), img) + cmd = fmt.Sprintf("%s --insecure-options=image,tls fetch %s", ctx.CmdNoConfig(), img) } child, err := gexpect.Command(cmd) if err != nil { @@ -556,3 +557,27 @@ func newAPIClientOrFail(t *testing.T, address string) (v1alpha.PublicAPIClient, c := v1alpha.NewPublicAPIClient(conn) return c, conn } + +func runServer(t *testing.T, auth taas.Type) *taas.Server { + actool := testutils.GetValueFromEnvOrPanic("ACTOOL") + gotool := testutils.GetValueFromEnvOrPanic("GO") + server, err := taas.NewServerWithPaths(auth, 20, actool, gotool) + if err != nil { + t.Fatalf("Could not start server: %v", err) + } + go serverHandler(t, server) + return server +} + +func serverHandler(t *testing.T, server *taas.Server) { + for { + select { + case msg, ok := <-server.Msg: + if ok { + t.Logf("server: %v", msg) + } + case <-server.Stop: + return + } + } +} diff --git a/tests/test-auth-server/aci/server.go b/tests/test-auth-server/aci/server.go index 98fc2b6457..3166f761d9 100644 --- a/tests/test-auth-server/aci/server.go +++ b/tests/test-auth-server/aci/server.go @@ -18,6 +18,7 @@ import ( "crypto/tls" "encoding/base64" "fmt" + "io/ioutil" "net/http" "net/http/httptest" "os/exec" @@ -43,10 +44,11 @@ func (e *httpError) Error() string { } type serverHandler struct { - auth Type - stop chan<- struct{} - msg chan<- string - tools *aciToolkit + auth Type + stop chan<- struct{} + msg chan<- string + tools *aciToolkit + fileSet map[string]string } func (h *serverHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -107,6 +109,10 @@ func (h *serverHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } h.sendMsg(fmt.Sprintf("Trying to serve %q", r.URL.String())) switch filepath.Base(r.URL.Path) { + case "/": + indexHTML := `` + w.Write([]byte(indexHTML)) + h.sendMsg(fmt.Sprintf(" done.")) case "prog.aci": h.sendMsg(fmt.Sprintf(" serving")) if data, err := h.tools.prepareACI(); err != nil { @@ -117,8 +123,20 @@ func (h *serverHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.sendMsg(fmt.Sprintf(" done.")) } default: - h.sendMsg(fmt.Sprintf(" not found.")) - w.WriteHeader(http.StatusNotFound) + path, ok := h.fileSet[filepath.Base(r.URL.Path)] + if ok { + contents, err := ioutil.ReadFile(path) + if err == nil { + w.Write(contents) + h.sendMsg(fmt.Sprintf(" done.")) + } else { + h.sendMsg(fmt.Sprintf(" not found.")) + w.WriteHeader(http.StatusNotFound) + } + } else { + h.sendMsg(fmt.Sprintf(" not found.")) + w.WriteHeader(http.StatusNotFound) + } } } @@ -171,6 +189,10 @@ func (s *Server) Close() { close(s.handler.stop) } +func (s *Server) UpdateFileSet(fileSet map[string]string) { + s.handler.fileSet = fileSet +} + func NewServer(auth Type, msgCapacity int) (*Server, error) { return NewServerWithPaths(auth, msgCapacity, "actool", "go") } @@ -203,6 +225,7 @@ func NewServerWithPaths(auth Type, msgCapacity int, acTool, goTool string) (*Ser acTool: acTool, goTool: goTool, }, + fileSet: make(map[string]string), }, } server.http = httptest.NewUnstartedServer(server.handler)