From a90aab4665b9f6d212770303f1e074c7f6724542 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 18 Sep 2014 17:16:10 -0400 Subject: [PATCH] Switch to using straight docker IDs instead of a hashing scheme --- endpoints/api/__init__.py | 7 ------- endpoints/api/image.py | 31 +++++++++++++++++-------------- endpoints/api/repository.py | 6 ++---- endpoints/api/tag.py | 5 ++++- static/js/app.js | 20 ++++++++++---------- static/js/controllers.js | 12 ++++++------ static/js/graphing.js | 22 +++++++++++----------- test/test_api_usage.py | 2 +- 8 files changed, 51 insertions(+), 54 deletions(-) diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 16a2c99e9..2f5e2045e 100644 --- a/endpoints/api/__init__.py +++ b/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 diff --git a/endpoints/api/image.py b/endpoints/api/image.py index 74cb1183f..3a6c62507 100644 --- a/endpoints/api/image.py +++ b/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): - if aid == '': + def docker_id(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 - # hashed 'internal' IDs. - ancestors = [internal_id(a) for a in image.ancestors.split('/')] + # Calculate the ancestors string, with the DBID's replaced with the docker IDs. + ancestors = [docker_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//image//changes') diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index 6de830b84..4e85a35b5 100644 --- a/endpoints/api/repository.py +++ b/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, - calculate_internal_id) + request_error, require_scope, Unauthorized, NotFound, InvalidRequest) 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, - 'internal_id': calculate_internal_id(tag.image.id) + 'image_id': tag.image.docker_image_id } organization = None diff --git a/endpoints/api/tag.py b/endpoints/api/tag.py index f9210881c..779b821ae 100644 --- a/endpoints/api/tag.py +++ b/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] } diff --git a/static/js/app.js b/static/js/app.js index b52e2f688..bf3376d94 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -5757,7 +5757,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 '; } @@ -5767,15 +5767,15 @@ quayApp.directive('tagSpecificImagesView', function () { var forAllTagImages = function(tag, callback, opt_cutoff) { if (!tag) { return; } - if (!$scope.imageByInternalId) { - $scope.imageByInternalId = []; + if (!$scope.imageByDockerId) { + $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; } @@ -5784,7 +5784,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; @@ -5810,7 +5810,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; }; @@ -5820,8 +5820,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)) { - delete toDelete[internal_id]; + for (var id in getIdsForTag(currentTag)) { + delete toDelete[id]; } } } @@ -5830,7 +5830,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); } } diff --git a/static/js/controllers.js b/static/js/controllers.js index 5373cf67a..792ac8bbf 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -493,7 +493,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) { @@ -681,9 +681,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. @@ -693,7 +693,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); } @@ -782,10 +782,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. diff --git a/static/js/graphing.js b/static/js/graphing.js index 74bd6aab5..795904747 100644 --- a/static/js/graphing.js +++ b/static/js/graphing.js @@ -307,8 +307,8 @@ ImageHistoryTree.prototype.setHighlightedPath_ = function(image) { this.markPath_(this.currentNode_, false); } - var imageByInternalId = this.imageByInternalId_; - var currentNode = imageByInternalId[image.internal_id]; + var imageByDockerId = this.imageByDockerId_; + 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) { diff --git a/test/test_api_usage.py b/test/test_api_usage.py index c7c285d79..b5c7959ab 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -1205,6 +1205,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 @@ -1212,7 +1213,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,