diff --git a/CHANGES/255.bugfix b/CHANGES/255.bugfix new file mode 100644 index 000000000..c264e8d12 --- /dev/null +++ b/CHANGES/255.bugfix @@ -0,0 +1 @@ +Added validation to some json input parameters. diff --git a/mypy.ini b/mypy.ini index 44204b26e..f927ec968 100644 --- a/mypy.ini +++ b/mypy.ini @@ -13,3 +13,6 @@ ignore_missing_imports = True [mypy-click_shell.*] ignore_missing_imports = True + +[mypy-schema.*] +ignore_missing_imports = True diff --git a/pulpcore/cli/common/generic.py b/pulpcore/cli/common/generic.py index e2fee84b8..0b750148b 100644 --- a/pulpcore/cli/common/generic.py +++ b/pulpcore/cli/common/generic.py @@ -15,6 +15,7 @@ ) import click +import schema as s from pulpcore.cli.common.context import ( DEFAULT_LIMIT, @@ -210,12 +211,21 @@ def load_json_callback( return json_object -def create_content_json_callback(content_ctx: Type[PulpContentContext]) -> Any: +def create_content_json_callback( + content_ctx: Type[PulpContentContext], schema: s.Schema = None +) -> Any: def _callback( ctx: click.Context, param: click.Parameter, value: Optional[str] ) -> Optional[List[PulpContentContext]]: new_value = load_json_callback(ctx, param, value) if new_value is not None: + if schema is not None: + try: + schema.validate(new_value) + except s.SchemaError as e: + raise click.ClickException( + "Validation of '{}' failed: {}".format(param.name, str(e)) + ) pulp_ctx = ctx.find_object(PulpContext) assert pulp_ctx is not None return [content_ctx(pulp_ctx, entity=unit) for unit in new_value] diff --git a/pulpcore/cli/file/repository.py b/pulpcore/cli/file/repository.py index 3ba9b78dd..7cd122462 100644 --- a/pulpcore/cli/file/repository.py +++ b/pulpcore/cli/file/repository.py @@ -2,6 +2,7 @@ from typing import Any, Dict, List, Optional import click +import schema as s from pulpcore.cli.common.context import ( EntityFieldDefinition, @@ -59,6 +60,19 @@ def _content_callback(ctx: click.Context, param: click.Parameter, value: Any) -> return value +CONTENT_LIST_SCHEMA = s.Schema([{"sha256": str, "relative_path": s.And(str, len)}]) + + +def _content_list_callback(ctx: click.Context, param: click.Parameter, value: Any) -> Any: + result = load_json_callback(ctx, param, value) + if result is None: + return None + try: + return CONTENT_LIST_SCHEMA.validate(result) + except s.SchemaError as e: + raise click.ClickException("Validation of '{}' failed: {}".format(param.name, str(e))) + + @click.group() @click.option( "-t", @@ -100,14 +114,16 @@ def repository(ctx: click.Context, pulp_ctx: PulpContext, repo_type: str) -> Non callback=_content_callback, ), ] -content_json_callback = create_content_json_callback(PulpFileContentContext) +content_json_callback = create_content_json_callback( + PulpFileContentContext, schema=CONTENT_LIST_SCHEMA +) modify_options = [ click.option( "--add-content", callback=content_json_callback, help=_( """JSON string with a list of objects to add to the repository. - Each object should consist of the following keys: "sha256", "relative_path".. + Each object must contain the following keys: "sha256", "relative_path". The argument prefixed with the '@' can be the path to a JSON file with a list of objects.""" ), ), @@ -116,7 +132,7 @@ def repository(ctx: click.Context, pulp_ctx: PulpContext, repo_type: str) -> Non callback=content_json_callback, help=_( """JSON string with a list of objects to remove from the repository. - Each object should consist of the following keys: "sha256", "relative_path".. + Each object must contain the following keys: "sha256", "relative_path". The argument prefixed with the '@' can be the path to a JSON file with a list of objects.""" ), ), @@ -247,22 +263,22 @@ def remove( @click.option( "--add-content", default="[]", - callback=load_json_callback, + callback=_content_list_callback, expose_value=True, help=_( """JSON string with a list of objects to add to the repository. - Each object should consist of the following keys: "sha256", "relative_path".. + Each object must contain the following keys: "sha256", "relative_path". The argument prefixed with the '@' can be the path to a JSON file with a list of objects.""" ), ) @click.option( "--remove-content", default="[]", - callback=load_json_callback, + callback=_content_list_callback, expose_value=True, help=_( """JSON string with a list of objects to remove from the repository. - Each object should consist of the following keys: "sha256", "relative_path".. + Each object must contain the following keys: "sha256", "relative_path". The argument prefixed with the '@' can be the path to a JSON file with a list of objects.""" ), ) diff --git a/setup.py b/setup.py index 23ea55ac1..e37cb84d0 100644 --- a/setup.py +++ b/setup.py @@ -45,6 +45,7 @@ "click>=7.1.2,<9.0.0", "packaging", "PyYAML~=5.3", + "schema==0.7.4", "requests~=2.24", "toml==0.10.2", ], diff --git a/tests/scripts/pulp_file/test_content_bulk.sh b/tests/scripts/pulp_file/test_content_bulk.sh index e8838c270..fed3edfc8 100755 --- a/tests/scripts/pulp_file/test_content_bulk.sh +++ b/tests/scripts/pulp_file/test_content_bulk.sh @@ -28,6 +28,18 @@ expect_succ pulp file content create --sha256 "$sha256_3" --relative-path upload expect_succ pulp file repository create --name "cli_test_file_repository" +# Test invalid json input +expect_fail pulp file repository content modify --repository "cli_test_file_repository" --add-content "{\"sha256\":\"$sha256_1\",\"relative_path\":\"upload_test/test_1.txt\"}" +echo "${ERROUTPUT}" | grep -q "should be instance of 'list'" +expect_fail pulp file repository content modify --repository "cli_test_file_repository" --add-content "[{\"relative_path\":\"upload_test/test_1.txt\"}]" +echo "${ERROUTPUT}" | grep -q "Missing key: 'sha256'" +expect_fail pulp file repository content modify --repository "cli_test_file_repository" --add-content "[{\"sha256\":\"$sha256_1\",\"relative_path\":\"upload_test/test_1.txt\", \"sha128\":\"abcd\"}]" +echo "${ERROUTPUT}" | grep -q "Wrong key 'sha128' in" +expect_fail pulp file repository content modify --repository "cli_test_file_repository" --add-content "[{\"sha256\":1234567890,\"relative_path\":\"upload_test/test_1.txt\"}]" +echo "${ERROUTPUT}" | grep -q "should be instance of 'str'" +expect_fail pulp file repository content modify --repository "cli_test_file_repository" --add-content "[{\"sha256\":\"$sha256_1\",\"relative_path\":\"\"}]" +echo "${ERROUTPUT}" | grep -q "Key 'relative_path' error:" + # Old content commands # Add content using JSON string expect_succ pulp file repository modify --name "cli_test_file_repository" --add-content "[{\"sha256\":\"$sha256_1\",\"relative_path\":\"upload_test/test_1.txt\"},{\"sha256\":\"$sha256_2\",\"relative_path\":\"upload_test/test_2.txt\"},{\"sha256\":\"$sha256_3\",\"relative_path\":\"upload_test/test_3.txt\"}]" @@ -62,3 +74,4 @@ test "$(echo "$OUTPUT" | jq -r length)" -eq "3" expect_succ pulp file repository content modify --repository "cli_test_file_repository" --remove-content "@remove_content.json" expect_succ pulp file repository content list --repository "cli_test_file_repository" test "$(echo "$OUTPUT" | jq -r length)" -eq "0" +