validate: Add constraint system to enforce constraints on arguments
Sometimes we need to take request parameters that have to meet
some set of constraints to be valid. This is only enforceable
at runtime, so write constraint and check_constraint decorators
to absorb some of the boiler plate of doing these checks.
BUG=b:200472226
TEST=run_tests
Change-Id: I800b50d962a3825727e34fa8008984a40bbabd1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/3173098
Tested-by: Sean McAllister <smcallis@google.com>
Commit-Queue: Sean McAllister <smcallis@google.com>
Reviewed-by: Alex Klein <saklein@chromium.org>
diff --git a/api/controller/test.py b/api/controller/test.py
index f5168d9..58f0315 100644
--- a/api/controller/test.py
+++ b/api/controller/test.py
@@ -9,6 +9,7 @@
import functools
import os
+import string
from chromite.api import controller
from chromite.api import faux
@@ -180,11 +181,56 @@
))
+@validate.constraint('valid docker tag')
+def _ValidDockerTag(tag):
+ """Check that a string meets requirements for Docker tag naming."""
+ # Tags can't start with period or dash
+ if tag[0] in '.-':
+ return "tag can't begin with '.' or '-'"
+
+ # Tags can only consist of [a-zA-Z0-9-_.]
+ allowed_chars = set(string.ascii_letters+string.digits+'-_.')
+ invalid_chars = set(tag) - allowed_chars
+ if invalid_chars:
+ return 'saw one or more invalid characters: [{}]'.format(
+ ''.join(invalid_chars),
+ )
+
+ # Finally, max tag length is 128 characters
+ if len(tag) > 128:
+ return 'maximum tag length is 128 characters'
+
+
+@validate.constraint('valid docker label key')
+def _ValidDockerLabelKey(key):
+ """Check that a string meets requirements for Docker tag naming."""
+
+ # Label keys should start and end with a lowercase letter
+ lowercase = set(string.ascii_lowercase)
+ if not (key[0] in lowercase and key[-1] in lowercase):
+ return "label key doesn't start and end with lowercase letter"
+
+ # Label keys can have lower-case alphanumeric characters, period and dash
+ allowed_chars = set(string.ascii_lowercase+string.digits+'-.')
+ invalid_chars = set(key) - allowed_chars
+ if invalid_chars:
+ return 'saw one or more invalid characters: [{}]'.format(
+ ''.join(invalid_chars),
+ )
+
+ # Repeated . and - aren't allowed
+ for char in '.-':
+ if char*2 in key:
+ return "'{}' can\'t be repeated in label key".format(char)
+
+
@faux.success(_BuildTestServiceContainersResponse)
@faux.error(_BuildTestServiceContainersFailedResponse)
@validate.require('build_target.name')
@validate.require('chroot.path')
@validate.require('version')
+@validate.check_constraint('tags', _ValidDockerTag)
+@validate.check_constraint('labels', _ValidDockerLabelKey)
@validate.validation_complete
def BuildTestServiceContainers(input_proto, output_proto, _config):
"""Builds docker containers for all test services and pushes them to gcr.io"""
diff --git a/api/controller/test_unittest.py b/api/controller/test_unittest.py
index dcd06a6..5d4aba6 100644
--- a/api/controller/test_unittest.py
+++ b/api/controller/test_unittest.py
@@ -278,6 +278,64 @@
os.path.join(input_msg.result_path, 'unit_tests.tar'))
+class DockerConstraintsTest(cros_test_lib.MockTestCase):
+ """Tests for Docker argument constraints."""
+
+ def assertValid(self, output):
+ return output is None
+
+ def assertInvalid(self, output):
+ return not self.assertValid(output)
+
+ def testValidDockerTag(self):
+ """Check logic for validating docker tag format."""
+ # pylint: disable=protected-access
+
+ invalid_tags = [
+ '.invalid-tag',
+ '-invalid-tag',
+ 'invalid-tag;',
+ 'invalid'*100,
+ ]
+
+ for tag in invalid_tags:
+ self.assertInvalid(test_controller._ValidDockerTag(tag))
+
+ valid_tags = [
+ 'valid-tag',
+ 'valid-tag-',
+ 'valid.tag.',
+ ]
+
+ for tag in valid_tags:
+ self.assertValid(test_controller._ValidDockerTag(tag))
+
+
+ def testValidDockerLabelKey(self):
+ """Check logic for validating docker label key format."""
+ # pylint: disable=protected-access
+
+ invalid_keys = [
+ 'Invalid-keY',
+ 'Invalid-key',
+ 'invalid-keY',
+ 'iNVALID-KEy',
+ 'invalid_key',
+ 'invalid-key;',
+ ]
+
+ for key in invalid_keys:
+ self.assertInvalid(test_controller._ValidDockerLabelKey(key))
+
+ valid_keys = [
+ 'chromeos.valid-key',
+ 'chromeos.valid-key-2',
+ ]
+
+ for key in valid_keys:
+ self.assertValid(test_controller._ValidDockerLabelKey(key))
+
+
class BuildTestServiceContainers(cros_test_lib.MockTestCase,
api_config.ApiConfigMixin):
"""Tests for the BuildTestServiceContainers function."""
@@ -735,8 +793,11 @@
def testShouldCallBundleCodeCoverageLlvmJsonForEachValidArtifact(self):
"""Test BundleCodeCoverageLlvmJson is called on each valid artifact."""
- BundleCodeCoverageLlvmJson_mock = self.PatchObject(
- test_service, 'BundleCodeCoverageLlvmJson', return_value='test')
+ BundleCodeCoverageLlvmJson_mock = (
+ self.PatchObject(
+ test_service,
+ 'BundleCodeCoverageLlvmJson',
+ return_value='test'))
test_controller.GetArtifacts(
common_pb2.ArtifactsByService.Test(output_artifacts=[
@@ -761,9 +822,9 @@
def testShouldReturnValidResult(self):
"""Test result contains paths and code_coverage_llvm_json type."""
self.PatchObject(test_service, 'BundleCodeCoverageLlvmJson',
- return_value='test')
+ return_value='test')
self.PatchObject(test_service, 'BuildTargetUnitTestTarball',
- return_value='unit_tests.tar')
+ return_value='unit_tests.tar')
result = test_controller.GetArtifacts(
common_pb2.ArtifactsByService.Test(output_artifacts=[