Switch to using straight docker IDs instead of a hashing scheme

endpoints/api/__init__.py

@@ -1,7 +1,6 @@
 import logging
 import json
 import datetime
-import hashlib
 
 from flask import Blueprint, request, make_response, jsonify, session
 from flask.ext.restful import Resource, abort, Api, reqparse
@@ -345,12 +344,6 @@ def log_action(kind, user_or_orgname, metadata=None, repo=None):
                    metadata=metadata, repository=repo)
 
 
-def calculate_internal_id(dbid):
-  """ Returns an 'internal id' we can send to the frontend that represents the
-      given database ID, but without leaking its actual value.
-  """
-  return hashlib.sha1("internal-db-" + str(dbid)).hexdigest()
-
 import endpoints.api.billing
 import endpoints.api.build
 import endpoints.api.discovery

endpoints/api/image.py

@@ -4,27 +4,26 @@ from collections import defaultdict
 
 from app import storage as store
 from endpoints.api import (resource, nickname, require_repo_read, RepositoryParamResource,
-                           format_date, NotFound, calculate_internal_id)
+                           format_date, NotFound)
 from data import model
 from util.cache import cache_control_flask_restful
 
 
-def image_view(image):
+def image_view(image, image_map):
   extended_props = image
   if image.storage and image.storage.id:
     extended_props = image.storage
 
   command = extended_props.command
 
-  def internal_id(aid):
+  def docker_id(aid):
-    if aid == '':
+    if not aid:
       return ''
 
-    return calculate_internal_id(aid)
+    return image_map[aid]
 
-  # Calculate the ancestors string, with the DBID's replaced with the
+  # Calculate the ancestors string, with the DBID's replaced with the docker IDs.
-  # hashed 'internal' IDs.
+  ancestors = [docker_id(a) for a in image.ancestors.split('/')]
-  ancestors = [internal_id(a) for a in image.ancestors.split('/')]
   ancestors_string = '/'.join(ancestors)
 
   return {
@@ -35,10 +34,7 @@ def image_view(image):
     'size': extended_props.image_size,
     'locations': list(image.storage.locations),
     'uploading': image.storage.uploading,
-
     'ancestors': ancestors_string,
-
-    'internal_id': calculate_internal_id(image.id),
     'sort_index': len(image.ancestors)
   }
 
@@ -57,14 +53,16 @@ class RepositoryImageList(RepositoryParamResource):
     for tag in all_tags:
       tags_by_image_id[tag.image.docker_image_id].append(tag.name)
 
+    image_map = {}
+    for image in all_images:
+      image_map[str(image.id)] = image.docker_image_id
 
     def add_tags(image_json):
       image_json['tags'] = tags_by_image_id[image_json['id']]
       return image_json
 
-
     return {
-      'images': [add_tags(image_view(image)) for image in all_images]
+      'images': [add_tags(image_view(image, image_map)) for image in all_images]
     }
 
 
@@ -79,7 +77,12 @@ class RepositoryImage(RepositoryParamResource):
     if not image:
       raise NotFound()
 
-    return image_view(image)
+    # Lookup all the ancestor images for the image.
+    image_map = {}
+    for current_image in model.get_parent_images(namespace, repository, image):
+      image_map[str(current_image.id)] = image.docker_image_id
+
+    return image_view(image, image_map)
 
 
 @resource('/v1/repository/<repopath:repository>/image/<image_id>/changes')

endpoints/api/repository.py

@@ -7,8 +7,7 @@ from data import model
 from endpoints.api import (truthy_bool, format_date, nickname, log_action, validate_json_request,
                            require_repo_read, require_repo_write, require_repo_admin,
                            RepositoryParamResource, resource, query_param, parse_args, ApiResource,
-                           request_error, require_scope, Unauthorized, NotFound, InvalidRequest,
+                           request_error, require_scope, Unauthorized, NotFound, InvalidRequest)
-                           calculate_internal_id)
 from auth.permissions import (ModifyRepositoryPermission, AdministerRepositoryPermission,
                               CreateRepositoryPermission, ReadRepositoryPermission)
 from auth.auth_context import get_authenticated_user
@@ -169,8 +168,7 @@ class Repository(RepositoryParamResource):
     def tag_view(tag):
       return {
         'name': tag.name,
-        'image_id': tag.image.docker_image_id,
+        'image_id': tag.image.docker_image_id
-        'internal_id': calculate_internal_id(tag.image.id)
       }
 
     organization = None

endpoints/api/tag.py

@@ -85,11 +85,14 @@ class RepositoryTagImages(RepositoryParamResource):
       raise NotFound()
 
     parent_images = model.get_parent_images(namespace, repository, tag_image)
+    image_map = {}
+    for image in parent_images:
+      image_map[str(image.id)] = image.docker_image_id
 
     parents = list(parent_images)
     parents.reverse()
     all_images = [tag_image] + parents
 
     return {
-      'images': [image_view(image) for image in all_images]
+      'images': [image_view(image, image_map) for image in all_images]
     }

static/js/app.js

@@ -6058,7 +6058,7 @@ quayApp.directive('tagSpecificImagesView', function () {
         }
  
         var currentTag = $scope.repository.tags[$scope.tag];
-        if (image.internal_id == currentTag.internal_id) {
+        if (image.id == currentTag.image_id) {
           classes += 'tag-image ';
         }
         
@@ -6068,15 +6068,15 @@ quayApp.directive('tagSpecificImagesView', function () {
       var forAllTagImages = function(tag, callback, opt_cutoff) {
         if (!tag) { return; }
 
-        if (!$scope.imageByInternalId) {
+        if (!$scope.imageByDockerId) {
-          $scope.imageByInternalId = [];
+          $scope.imageByDockerId = [];
           for (var i = 0; i < $scope.images.length; ++i) {
             var currentImage = $scope.images[i];
-            $scope.imageByInternalId[currentImage.internal_id] = currentImage;
+            $scope.imageByDockerId[currentImage.id] = currentImage;
           }
         }
 
-        var tag_image = $scope.imageByInternalId[tag.internal_id];
+        var tag_image = $scope.imageByDockerId[tag.image_id];
         if (!tag_image) {
           return;
         }
@@ -6085,7 +6085,7 @@ quayApp.directive('tagSpecificImagesView', function () {
 
         var ancestors = tag_image.ancestors.split('/').reverse();
         for (var i = 0; i < ancestors.length; ++i) {
-          var image = $scope.imageByInternalId[ancestors[i]];
+          var image = $scope.imageByDockerId[ancestors[i]];
           if (image) {
             if (image == opt_cutoff) {
               return;
@@ -6111,7 +6111,7 @@ quayApp.directive('tagSpecificImagesView', function () {
         var getIdsForTag = function(currentTag) {
           var ids = {};
           forAllTagImages(currentTag, function(image) {
-            ids[image.internal_id] = true;
+            ids[image.id] = true;
           }, $scope.imageCutoff);
           return ids;
         };
@@ -6121,8 +6121,8 @@ quayApp.directive('tagSpecificImagesView', function () {
         for (var currentTagName in $scope.repository.tags) {
           var currentTag = $scope.repository.tags[currentTagName];
           if (currentTag != tag) {
-            for (var internal_id in getIdsForTag(currentTag)) {
+            for (var id in getIdsForTag(currentTag)) {
-              delete toDelete[internal_id];
+              delete toDelete[id];
             }
           }
         }
@@ -6131,7 +6131,7 @@ quayApp.directive('tagSpecificImagesView', function () {
         var images = [];
         for (var i = 0; i < $scope.images.length; ++i) {
           var image = $scope.images[i];
-          if (toDelete[image.internal_id]) {
+          if (toDelete[image.id]) {
             images.push(image);
           }
         }

static/js/controllers.js

@@ -497,7 +497,7 @@ function RepoCtrl($scope, $sanitize, Restangular, ImageMetadataService, ApiServi
   };
 
   $scope.findImageForTag = function(tag) {
-    return tag && $scope.imageByInternalId && $scope.imageByInternalId[tag.internal_id];
+    return tag && $scope.imageByDockerId && $scope.imageByDockerId[tag.image_id];
   };
 
   $scope.createOrMoveTag = function(image, tagName, opt_invalid) {
@@ -685,9 +685,9 @@ function RepoCtrl($scope, $sanitize, Restangular, ImageMetadataService, ApiServi
   };
 
   var forAllTagImages = function(tag, callback) {
-    if (!tag || !$scope.imageByInternalId) { return; }
+    if (!tag || !$scope.imageByDockerId) { return; }
 
-    var tag_image =  $scope.imageByInternalId[tag.internal_id];
+    var tag_image =  $scope.imageByDockerId[tag.image_id];
     if (!tag_image) { return; }
 
     // Callback the tag's image itself.
@@ -697,7 +697,7 @@ function RepoCtrl($scope, $sanitize, Restangular, ImageMetadataService, ApiServi
     if (!tag_image.ancestors) { return; }
     var ancestors = tag_image.ancestors.split('/');
     for (var i = 0; i < ancestors.length; ++i) {
-      var image = $scope.imageByInternalId[ancestors[i]];
+      var image = $scope.imageByDockerId[ancestors[i]];
       if (image) {
         callback(image);
       }
@@ -786,10 +786,10 @@ function RepoCtrl($scope, $sanitize, Restangular, ImageMetadataService, ApiServi
       $scope.specificImages = [];
 
       // Build various images for quick lookup of images.
-      $scope.imageByInternalId = {};
+      $scope.imageByDockerId = {};
       for (var i = 0; i < $scope.images.length; ++i) {
         var currentImage = $scope.images[i];
-        $scope.imageByInternalId[currentImage.internal_id] = currentImage;
+        $scope.imageByDockerId[currentImage.id] = currentImage;
       }
 
       // Dispose of any existing tree.

static/js/graphing.js

@@ -307,8 +307,8 @@ ImageHistoryTree.prototype.setHighlightedPath_ = function(image) {
     this.markPath_(this.currentNode_, false);
   }
 
-  var imageByInternalId = this.imageByInternalId_;
+  var imageByDockerId = this.imageByDockerId_;
-  var currentNode = imageByInternalId[image.internal_id];
+  var currentNode = imageByDockerId[image.id];
   if (currentNode) {
     this.markPath_(currentNode, true);
     this.currentNode_ = currentNode;
@@ -386,7 +386,7 @@ ImageHistoryTree.prototype.buildRoot_ = function() {
   var formatted = {"name": "No images found"};
 
   // Build a node for each image.
-  var imageByInternalId = {};
+  var imageByDockerId = {};
   for (var i = 0; i < this.images_.length; ++i) {
     var image = this.images_[i];
     var imageNode = {
@@ -395,9 +395,9 @@ ImageHistoryTree.prototype.buildRoot_ = function() {
       "image": image,
       "tags": image.tags
     };
-    imageByInternalId[image.internal_id] = imageNode;
+    imageByDockerId[image.id] = imageNode;
   }
-  this.imageByInternalId_ = imageByInternalId;
+  this.imageByDockerId_ = imageByDockerId;
 
   // For each node, attach it to its immediate parent. If there is no immediate parent,
   // then the node is the root.
@@ -408,10 +408,10 @@ ImageHistoryTree.prototype.buildRoot_ = function() {
     // Skip images that are currently uploading.
     if (image.uploading) { continue; }
 
-    var imageNode = imageByInternalId[image.internal_id];
+    var imageNode = imageByDockerId[image.id];
     var ancestors = this.getAncestors_(image);
     var immediateParent = ancestors[ancestors.length - 1];
-    var parent = imageByInternalId[immediateParent];
+    var parent = imageByDockerId[immediateParent];
     if (parent) {	   
       // Add a reference to the parent. This makes walking the tree later easier.
       imageNode.parent = parent;
@@ -442,7 +442,7 @@ ImageHistoryTree.prototype.buildRoot_ = function() {
     // Skip images that are currently uploading.
     if (image.uploading) { continue; }
 
-    var imageNode = imageByInternalId[image.internal_id];
+    var imageNode = imageByDockerId[image.id];
     maxChildCount = Math.max(maxChildCount, this.determineMaximumChildCount_(imageNode));
   }
 
@@ -573,7 +573,7 @@ ImageHistoryTree.prototype.setTag_ = function(tagName) {
     return;
   }
 
-  var imageByInternalId = this.imageByInternalId_;
+  var imageByDockerId = this.imageByDockerId_;
   
   // Save the current tag.
   var previousTagName = this.currentTag_;
@@ -596,10 +596,10 @@ ImageHistoryTree.prototype.setTag_ = function(tagName) {
     // Skip images that are currently uploading.
     if (image.uploading) { continue; }
 
-    var imageNode = this.imageByInternalId_[image.internal_id];
+    var imageNode = this.imageByDockerId_[image.id];
     var ancestors = this.getAncestors_(image);
     var immediateParent = ancestors[ancestors.length - 1];
-    var parent = imageByInternalId[immediateParent];
+    var parent = imageByDockerId[immediateParent];
     if (parent && imageNode.highlighted) {
       var arr = parent.children;
       if (parent._children) {

test/test_api_usage.py

@@ -1429,6 +1429,7 @@ class TestListAndGetImage(ApiTestCase):
                                 params=dict(repository=ADMIN_ACCESS_USER + '/simple'))
 
     assert len(json['images']) > 0
+
     for image in json['images']:
       assert 'id' in image
       assert 'tags' in image
@@ -1436,7 +1437,6 @@ class TestListAndGetImage(ApiTestCase):
       assert 'comment' in image
       assert 'command' in image
       assert 'ancestors' in image
-      assert 'internal_id' in image
       assert 'size' in image
 
       ijson = self.getJsonResponse(RepositoryImage,