From df5a6aabe2d280dd8b574f7621f51fd07f1d0c53 Mon Sep 17 00:00:00 2001 From: Charlton Austin Date: Wed, 29 Mar 2017 12:54:15 -0400 Subject: [PATCH] fix(buildman, endpoint): added in fix upload gzip and dockerfile --- buildman/component/buildcomponent.py | 16 +++++++- .../component/test/test_buildcomponent.py | 12 ++++++ endpoints/api/build.py | 40 +++++++++++-------- endpoints/api/test/test_build.py | 14 +++++++ 4 files changed, 63 insertions(+), 19 deletions(-) create mode 100644 endpoints/api/test/test_build.py diff --git a/buildman/component/buildcomponent.py b/buildman/component/buildcomponent.py index 31b9b70bb..52ac7abfb 100644 --- a/buildman/component/buildcomponent.py +++ b/buildman/component/buildcomponent.py @@ -125,11 +125,11 @@ class BuildComponent(BaseComponent): # password: The password for pulling the base image (if any). # TODO: Charlie Tuesday, March 28, 2017 come back and clean up build_subdir. - dockerfile_path = os.path.relpath(build_config.get('build_subdir'), build_config.get('context')) + context, dockerfile_path = self.extract_dockerfile_args(build_config) build_arguments = { 'build_package': build_job.get_build_package_url(self.user_files), - 'context': build_config.get('context'), + 'context': context, 'dockerfile_path': dockerfile_path, 'repository': repository_name, 'registry': self.registry_hostname, @@ -174,6 +174,18 @@ class BuildComponent(BaseComponent): # by this point, so it makes sense to have a timeout. self._last_heartbeat = datetime.datetime.utcnow() + BUILD_HEARTBEAT_DELAY + @staticmethod + def extract_dockerfile_args(build_config): + dockerfile_path = build_config.get('build_subdir', '') + context = build_config.get('context', '') + if not (dockerfile_path == '' or context == ''): + # This should not happen and can be removed when we centralize validating build_config + if ".." in os.path.relpath(dockerfile_path, context): + return os.path.split(dockerfile_path) + dockerfile_path = os.path.relpath(dockerfile_path, context) + + return context, dockerfile_path + @staticmethod def _commit_sha(build_config): """ Determines whether the metadata is using an old schema or not and returns the commit. """ diff --git a/buildman/component/test/test_buildcomponent.py b/buildman/component/test/test_buildcomponent.py index 2fad4dfe7..a434dd092 100644 --- a/buildman/component/test/test_buildcomponent.py +++ b/buildman/component/test/test_buildcomponent.py @@ -21,3 +21,15 @@ def test_path_is_dockerfile(input, expected_path, expected_file): actual_path, actual_file = BuildComponent.name_and_path(input) assert actual_path == expected_path assert actual_file == expected_file + +@pytest.mark.parametrize('build_config,context,dockerfile_path', [ + ({}, "", ""), + ({'build_subdir': "/builddir/Dockerfile"}, "", "/builddir/Dockerfile"), + ({'context': "/builddir"}, "/builddir", ""), + ({'context': "/builddir", 'build_subdir': "/builddir/Dockerfile"}, "/builddir", "Dockerfile"), + ({'context': "/some_other_dir/Dockerfile", 'build_subdir': "/builddir/Dockerfile"}, "/builddir", "Dockerfile"), +]) +def test_extract_dockerfile_args(build_config, context, dockerfile_path): + actual_context, actual_dockerfile_path = BuildComponent.extract_dockerfile_args(build_config) + assert context == actual_context + assert dockerfile_path == actual_dockerfile_path diff --git a/endpoints/api/build.py b/endpoints/api/build.py index 1a0d1c89c..3abca20a3 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -1,29 +1,27 @@ """ Create, list, cancel and get status/logs of repository builds. """ -import os -from urlparse import urlparse - -import logging -import json import datetime import hashlib +import json +import logging +import os +from urlparse import urlparse from flask import request from app import userfiles as user_files, build_logs, log_archive, dockerfile_build_queue +from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermission, + AdministerRepositoryPermission, AdministerOrganizationPermission, + SuperUserPermission) from buildtrigger.basehandler import BuildTriggerHandler +from data import database +from data import model +from data.buildlogs import BuildStatusRetrievalError from endpoints.api import (RepositoryParamResource, parse_args, query_param, nickname, resource, require_repo_read, require_repo_write, validate_json_request, ApiResource, internal_only, format_date, api, path_param, require_repo_admin, abort, disallow_for_app_repositories) -from endpoints.exception import Unauthorized, NotFound, InvalidRequest from endpoints.building import start_build, PreparedBuild, MaximumBuildsQueuedException -from data import database -from data import model -from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermission, - AdministerRepositoryPermission, AdministerOrganizationPermission, - SuperUserPermission) - -from data.buildlogs import BuildStatusRetrievalError +from endpoints.exception import Unauthorized, NotFound, InvalidRequest from util.names import parse_robot_username @@ -246,8 +244,7 @@ class RepositoryBuildList(RepositoryParamResource): if scheme != 'http' and scheme != 'https': raise InvalidRequest('Invalid Archive URL: Must be http or https') - subdir = request_json['subdirectory'] if 'subdirectory' in request_json else '' - context = request_json['context'] if 'context' in request_json else os.path.dirname(subdir) + context, subdir = self.get_dockerfile_context(request_json) tags = request_json.get('docker_tags', ['latest']) pull_robot_name = request_json.get('pull_robot', None) @@ -311,8 +308,17 @@ class RepositoryBuildList(RepositoryParamResource): } return resp, 201, headers - - + @staticmethod + def get_dockerfile_context(request_json): + context = request_json['context'] if 'context' in request_json else os.path.sep + if 'subdirectory' in request_json: + subdir = request_json["subdirectory"] + else: + if context.endswith(os.path.sep): + subdir = context + "Dockerfile" + else: + subdir = context + os.path.sep + "Dockerfile" + return context, subdir @resource('/v1/repository//build/') @path_param('repository', 'The full path of the repository. e.g. namespace/name') diff --git a/endpoints/api/test/test_build.py b/endpoints/api/test/test_build.py new file mode 100644 index 000000000..27592193e --- /dev/null +++ b/endpoints/api/test/test_build.py @@ -0,0 +1,14 @@ +import pytest + +from endpoints.api.build import RepositoryBuildList + + +@pytest.mark.parametrize('request_json,subdir,context', [ + ({}, "/Dockerfile", "/"), + ({"context": "/some_context"}, "/some_context/Dockerfile", "/some_context"), + ({"subdirectory": "/some_subdir/Dockerfile"}, "/some_subdir/Dockerfile", "/"), +]) +def test_extract_dockerfile_args(request_json, subdir, context): + actual_context, actual_subdir = RepositoryBuildList.get_dockerfile_context(request_json) + assert subdir == actual_subdir + assert context == actual_context