[ARVADOS] updated: 1.2.0-185-gbdd226bc9

Git user git at public.curoverse.com
Tue Oct 16 15:48:47 EDT 2018


Summary of changes:
 services/api/app/models/container.rb |  6 ++++++
 services/crunch-run/crunchrun.go     | 29 +++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 8 deletions(-)

       via  bdd226bc9ab3f9f957963171b8d9c3d4355f472c (commit)
      from  520ef5d733b60fe85c88e6779c7b5e204d5273c5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit bdd226bc9ab3f9f957963171b8d9c3d4355f472c
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Tue Oct 16 15:43:12 2018 -0400

    14262: Use container token for access to load Docker image
    
    Previously used Dispatcher token, which created a security race
    condition (you couldn't set a container image that you didn't have
    access to, but if your access was revoked it the meantime, the
    container would still run.)
    
    Also tweaked API server to allow a PDH for the container image spec
    with no further checking (so the API server doesn't have to go out and
    search the federation.)  This is no longer a security hazard since it
    is now using a user token and not the dispatcher token.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 527881ba6..8223bc569 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -248,6 +248,12 @@ class Container < ArvadosModel
   def self.resolve_container_image(container_image)
     coll = Collection.for_latest_docker_image(container_image)
     if !coll
+      # Allow bare pdh without any additional checking otherwise
+      # federated container requests won't work.
+      if loc = Keep::Locator.parse(container_image)
+        loc.strip_hints!
+        return loc.to_s
+      end
       raise ArvadosModel::UnresolvableContainerError.new "docker image #{container_image.inspect} not found"
     end
     coll.portable_data_hash
diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 44560b80a..ca2344113 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -121,7 +121,7 @@ type ContainerRunner struct {
 	SigChan         chan os.Signal
 	ArvMountExit    chan error
 	SecretMounts    map[string]arvados.Mount
-	MkArvClient     func(token string) (IArvadosClient, error)
+	MkArvClient     func(token string) (IArvadosClient, IKeepClient, error)
 	finalState      string
 	parentTemp      string
 
@@ -230,8 +230,17 @@ func (runner *ContainerRunner) LoadImage() (err error) {
 
 	runner.CrunchLog.Printf("Fetching Docker image from collection '%s'", runner.Container.ContainerImage)
 
+	tok, err := runner.ContainerToken()
+	if err != nil {
+		return fmt.Errorf("While getting container token (LoadImage): %v", err)
+	}
+	arvClient, kc, err := runner.MkArvClient(tok)
+	if err != nil {
+		return fmt.Errorf("While creating arv client (LoadImage): %v", err)
+	}
+
 	var collection arvados.Collection
-	err = runner.ArvClient.Get("collections", runner.Container.ContainerImage, nil, &collection)
+	err = arvClient.Get("collections", runner.Container.ContainerImage, nil, &collection)
 	if err != nil {
 		return fmt.Errorf("While getting container image collection: %v", err)
 	}
@@ -252,7 +261,7 @@ func (runner *ContainerRunner) LoadImage() (err error) {
 		runner.CrunchLog.Print("Loading Docker image from keep")
 
 		var readCloser io.ReadCloser
-		readCloser, err = runner.Kc.ManifestFileReader(manifest, img)
+		readCloser, err = kc.ManifestFileReader(manifest, img)
 		if err != nil {
 			return fmt.Errorf("While creating ManifestFileReader for container image: %v", err)
 		}
@@ -274,7 +283,7 @@ func (runner *ContainerRunner) LoadImage() (err error) {
 
 	runner.ContainerConfig.Image = imageID
 
-	runner.Kc.ClearBlockCache()
+	kc.ClearBlockCache()
 
 	return nil
 }
@@ -1643,7 +1652,7 @@ func (runner *ContainerRunner) fetchContainerRecord() error {
 		return fmt.Errorf("error getting container token: %v", err)
 	}
 
-	containerClient, err := runner.MkArvClient(containerToken)
+	containerClient, _, err := runner.MkArvClient(containerToken)
 	if err != nil {
 		return fmt.Errorf("error creating container API client: %v", err)
 	}
@@ -1683,13 +1692,17 @@ func NewContainerRunner(client *arvados.Client, api IArvadosClient, kc IKeepClie
 		}
 		return ps, nil
 	}
-	cr.MkArvClient = func(token string) (IArvadosClient, error) {
+	cr.MkArvClient = func(token string) (IArvadosClient, IKeepClient, error) {
 		cl, err := arvadosclient.MakeArvadosClient()
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		cl.ApiToken = token
-		return cl, nil
+		kc, err := keepclient.MakeKeepClient(cl)
+		if err != nil {
+			return nil, nil, err
+		}
+		return cl, kc, nil
 	}
 	var err error
 	cr.LogCollection, err = (&arvados.Collection{}).FileSystem(cr.client, cr.Kc)

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list