From 63628678b85147fb7b27aec41e9870aef98e0f94 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 8 Sep 2014 15:02:26 -0400 Subject: [PATCH] Instead of sending DB IDs, send "internal IDs" which are DB IDs hashed. This way, we can still calculate the ancestors without hitting the DB further, but without leaking the size of the images table --- endpoints/api/__init__.py | 7 +++++++ endpoints/api/image.py | 21 ++++++++++++++++++--- endpoints/api/repository.py | 5 +++-- static/js/app.js | 22 +++++++++++----------- static/js/controllers.js | 12 ++++++------ static/js/graphing.js | 26 +++++++++++++------------- test/test_api_usage.py | 2 +- 7 files changed, 59 insertions(+), 36 deletions(-) diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 2f5e2045e..16a2c99e9 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -1,6 +1,7 @@ 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 @@ -344,6 +345,12 @@ 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 3060053ad..74cb1183f 100644 --- a/endpoints/api/image.py +++ b/endpoints/api/image.py @@ -4,7 +4,7 @@ from collections import defaultdict from app import storage as store from endpoints.api import (resource, nickname, require_repo_read, RepositoryParamResource, - format_date, NotFound) + format_date, NotFound, calculate_internal_id) from data import model from util.cache import cache_control_flask_restful @@ -15,16 +15,31 @@ def image_view(image): extended_props = image.storage command = extended_props.command + + def internal_id(aid): + if aid == '': + return '' + + return calculate_internal_id(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('/')] + ancestors_string = '/'.join(ancestors) + return { 'id': image.docker_image_id, 'created': format_date(extended_props.created), 'comment': extended_props.comment, 'command': json.loads(command) if command else None, - 'ancestors': image.ancestors, - 'dbid': image.id, '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) } diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index 17a35fea1..6de830b84 100644 --- a/endpoints/api/repository.py +++ b/endpoints/api/repository.py @@ -7,7 +7,8 @@ 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,7 +170,7 @@ class Repository(RepositoryParamResource): return { 'name': tag.name, 'image_id': tag.image.docker_image_id, - 'dbid': tag.image.id + 'internal_id': calculate_internal_id(tag.image.id) } organization = None diff --git a/static/js/app.js b/static/js/app.js index 0551df2dc..b52e2f688 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.dbid == currentTag.dbid) { + if (image.internal_id == currentTag.internal_id) { classes += 'tag-image '; } @@ -5767,15 +5767,15 @@ quayApp.directive('tagSpecificImagesView', function () { var forAllTagImages = function(tag, callback, opt_cutoff) { if (!tag) { return; } - if (!$scope.imageByDBID) { - $scope.imageByDBID = []; + if (!$scope.imageByInternalId) { + $scope.imageByInternalId = []; for (var i = 0; i < $scope.images.length; ++i) { var currentImage = $scope.images[i]; - $scope.imageByDBID[currentImage.dbid] = currentImage; + $scope.imageByInternalId[currentImage.internal_id] = currentImage; } } - var tag_image = $scope.imageByDBID[tag.dbid]; + var tag_image = $scope.imageByInternalId[tag.internal_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.imageByDBID[ancestors[i]]; + var image = $scope.imageByInternalId[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.dbid] = true; + ids[image.internal_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 dbid in getIdsForTag(currentTag)) { - delete toDelete[dbid]; + for (var internal_id in getIdsForTag(currentTag)) { + delete toDelete[internal_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.dbid]) { + if (toDelete[image.internal_id]) { images.push(image); } } @@ -5841,7 +5841,7 @@ quayApp.directive('tagSpecificImagesView', function () { return result; } - return b.dbid - a.dbid; + return b.sort_index - a.sort_index; }); $scope.tagSpecificImages = images; diff --git a/static/js/controllers.js b/static/js/controllers.js index 9131a0140..5373cf67a 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.imageByDBID && $scope.imageByDBID[tag.dbid]; + return tag && $scope.imageByInternalId && $scope.imageByInternalId[tag.internal_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.imageByDBID) { return; } + if (!tag || !$scope.imageByInternalId) { return; } - var tag_image = $scope.imageByDBID[tag.dbid]; + var tag_image = $scope.imageByInternalId[tag.internal_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.imageByDBID[ancestors[i]]; + var image = $scope.imageByInternalId[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.imageByDBID = {}; + $scope.imageByInternalId = {}; for (var i = 0; i < $scope.images.length; ++i) { var currentImage = $scope.images[i]; - $scope.imageByDBID[currentImage.dbid] = currentImage; + $scope.imageByInternalId[currentImage.internal_id] = currentImage; } // Dispose of any existing tree. diff --git a/static/js/graphing.js b/static/js/graphing.js index b18e5ddbe..74bd6aab5 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 imageByDBID = this.imageByDBID_; - var currentNode = imageByDBID[image.dbid]; + var imageByInternalId = this.imageByInternalId_; + var currentNode = imageByInternalId[image.internal_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 imageByDBID = {}; + var imageByInternalId = {}; 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 }; - imageByDBID[image.dbid] = imageNode; + imageByInternalId[image.internal_id] = imageNode; } - this.imageByDBID_ = imageByDBID; + this.imageByInternalId_ = imageByInternalId; // 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 = imageByDBID[image.dbid]; + var imageNode = imageByInternalId[image.internal_id]; var ancestors = this.getAncestors_(image); - var immediateParent = ancestors[ancestors.length - 1] * 1; - var parent = imageByDBID[immediateParent]; + var immediateParent = ancestors[ancestors.length - 1]; + var parent = imageByInternalId[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 = imageByDBID[image.dbid]; + var imageNode = imageByInternalId[image.internal_id]; maxChildCount = Math.max(maxChildCount, this.determineMaximumChildCount_(imageNode)); } @@ -573,7 +573,7 @@ ImageHistoryTree.prototype.setTag_ = function(tagName) { return; } - var imageByDBID = this.imageByDBID_; + var imageByInternalId = this.imageByInternalId_; // 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.imageByDBID_[image.dbid]; + var imageNode = this.imageByInternalId_[image.internal_id]; var ancestors = this.getAncestors_(image); - var immediateParent = ancestors[ancestors.length - 1] * 1; - var parent = imageByDBID[immediateParent]; + var immediateParent = ancestors[ancestors.length - 1]; + var parent = imageByInternalId[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 004f20651..c7c285d79 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -1212,7 +1212,7 @@ class TestListAndGetImage(ApiTestCase): assert 'comment' in image assert 'command' in image assert 'ancestors' in image - assert 'dbid' in image + assert 'internal_id' in image assert 'size' in image ijson = self.getJsonResponse(RepositoryImage,