Skip to content

Commit

Permalink
fix(sdk): Verify binary data in step copy-results-artifacts. Fixes ku…
Browse files Browse the repository at this point in the history 
…beflow#984 (kubeflow#985)

* Add binary character check to copy-results-artifacts

* Update unit tests
  • Loading branch information
@maxdebayser
maxdebayser authored Jul 6, 2022
1 parent 52f06b6 commit bce9814
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 17 deletions.
4 changes: 3 additions & 1 deletion sdk/python/kfp_tekton/compiler/_data_passing_rewriter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,9 @@ def append_taskrun_params(task_name_append: str):
'TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)\n' +
'touch ' + dst + '\n' + # create an empty file by default.
'if [[ $TOTAL_SIZE -lt 3072 ]]; then\n' +
' cp ' + src + ' ' + dst + '\n' +
' if ! awk "/[^[:print:]]/{f=1} END{exit !f}" %s; then\n' % src +
' cp ' + src + ' ' + dst + '\n' +
' fi\n'
'fi\n'
)
_append_original_pr_name_env_to_step(copy_results_artifact_step)
Expand Down
4 changes: 3 additions & 1 deletion sdk/python/tests/compiler/testdata/artifact_outputs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.data.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.gcs-download.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data $(results.data.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.gcs-download.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data; then
cp $(workspaces.gcs-download.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data $(results.data.path)
fi
fi
onError: continue
env:
Expand Down
20 changes: 15 additions & 5 deletions sdk/python/tests/compiler/testdata/big_data_passing.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.output-text.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.repeat-line.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/output_text $(results.output-text.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.repeat-line.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/output_text; then
cp $(workspaces.repeat-line.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/output_text $(results.output-text.path)
fi
fi
onError: continue
env:
Expand Down Expand Up @@ -226,13 +228,17 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.odd-lines.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.split-text-lines.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/odd_lines $(results.odd-lines.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.split-text-lines.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/odd_lines; then
cp $(workspaces.split-text-lines.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/odd_lines $(results.odd-lines.path)
fi
fi
ARTIFACT_SIZE=`wc -c $(workspaces.split-text-lines.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/even_lines | awk '{print $1}'`
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.even-lines.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.split-text-lines.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/even_lines $(results.even-lines.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.split-text-lines.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/even_lines; then
cp $(workspaces.split-text-lines.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/even_lines $(results.even-lines.path)
fi
fi
onError: continue
env:
Expand Down Expand Up @@ -446,7 +452,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.numbers.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.write-numbers.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/numbers $(results.numbers.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.write-numbers.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/numbers; then
cp $(workspaces.write-numbers.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/numbers $(results.numbers.path)
fi
fi
onError: continue
env:
Expand Down Expand Up @@ -614,7 +622,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.output.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.sum-numbers.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Output $(results.output.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.sum-numbers.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Output; then
cp $(workspaces.sum-numbers.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Output $(results.output.path)
fi
fi
onError: continue
env:
Expand Down
8 changes: 6 additions & 2 deletions sdk/python/tests/compiler/testdata/create_component_from_func.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.output-dir.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.produce-dir-with-files-python-op.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/output_dir $(results.output-dir.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.produce-dir-with-files-python-op.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/output_dir; then
cp $(workspaces.produce-dir-with-files-python-op.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/output_dir $(results.output-dir.path)
fi
fi
onError: continue
env:
Expand Down Expand Up @@ -170,7 +172,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.subdir.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.get-subdirectory.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Subdir $(results.subdir.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.get-subdirectory.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Subdir; then
cp $(workspaces.get-subdirectory.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Subdir $(results.subdir.path)
fi
fi
onError: continue
env:
Expand Down
12 changes: 9 additions & 3 deletions sdk/python/tests/compiler/testdata/data_passing_pipeline_complete.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.data.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.produce-anything.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data $(results.data.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.produce-anything.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data; then
cp $(workspaces.produce-anything.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data $(results.data.path)
fi
fi
onError: continue
env:
Expand Down Expand Up @@ -212,7 +214,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.data.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.produce-something.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data $(results.data.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.produce-something.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data; then
cp $(workspaces.produce-something.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/data $(results.data.path)
fi
fi
onError: continue
env:
Expand Down Expand Up @@ -311,7 +315,9 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.output.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp $(workspaces.produce-string.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Output $(results.output.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" $(workspaces.produce-string.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Output; then
cp $(workspaces.produce-string.path)/artifacts/$ORIG_PR_NAME/$(context.taskRun.name)/Output $(results.output.path)
fi
fi
onError: continue
env:
Expand Down
8 changes: 6 additions & 2 deletions sdk/python/tests/compiler/testdata/many_results.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,17 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.param1.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp /tekton/home/tep-results/param1 $(results.param1.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" /tekton/home/tep-results/param1; then
cp /tekton/home/tep-results/param1 $(results.param1.path)
fi
fi
ARTIFACT_SIZE=`wc -c /tekton/home/tep-results/param3-b | awk '{print $1}'`
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.param3-b.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp /tekton/home/tep-results/param3-b $(results.param3-b.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" /tekton/home/tep-results/param3-b; then
cp /tekton/home/tep-results/param3-b $(results.param3-b.path)
fi
fi
onError: continue
env:
Expand Down
12 changes: 9 additions & 3 deletions sdk/python/tests/compiler/testdata/many_results_with_warnings.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,25 @@ spec:
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.param2.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp /tekton/home/tep-results/param2 $(results.param2.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" /tekton/home/tep-results/param2; then
cp /tekton/home/tep-results/param2 $(results.param2.path)
fi
fi
ARTIFACT_SIZE=`wc -c /tekton/home/tep-results/param3 | awk '{print $1}'`
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.param3.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp /tekton/home/tep-results/param3 $(results.param3.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" /tekton/home/tep-results/param3; then
cp /tekton/home/tep-results/param3 $(results.param3.path)
fi
fi
ARTIFACT_SIZE=`wc -c /tekton/home/tep-results/superlongnamesuperlongnamesuperlongnamesuperlongnamesuper | awk '{print $1}'`
TOTAL_SIZE=$( expr $TOTAL_SIZE + $ARTIFACT_SIZE)
touch $(results.superlongnamesuperlongnamesuperlongnamesuperlongnamesuper.path)
if [[ $TOTAL_SIZE -lt 3072 ]]; then
cp /tekton/home/tep-results/superlongnamesuperlongnamesuperlongnamesuperlongnamesuper $(results.superlongnamesuperlongnamesuperlongnamesuperlongnamesuper.path)
if ! awk "/[^[:print:]]/{f=1} END{exit !f}" /tekton/home/tep-results/superlongnamesuperlongnamesuperlongnamesuperlongnamesuper; then
cp /tekton/home/tep-results/superlongnamesuperlongnamesuperlongnamesuperlongnamesuper $(results.superlongnamesuperlongnamesuperlongnamesuperlongnamesuper.path)
fi
fi
onError: continue
env:
Expand Down

0 comments on commit bce9814

Please sign in to comment.