fix: allow kms encryption upload for processing #1897

chuyang-deng · 2020-09-16T20:32:23Z

Issue #, if available:
#1892

Description of changes:
outputs customized paths has already been resolved.
This PR is to implement kms_key for the processing job input upload, it sets ServerSideEncryption to aws:kms if kms_key is present.

Testing done:
unit test
integ test

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

I have read the CONTRIBUTING doc
I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

sagemaker-bot · 2020-09-16T20:40:19Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 1ab207f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-16T20:59:15Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 1ab207f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-16T21:13:26Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 1ab207f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-16T22:08:53Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 82beaaf
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-16T22:09:10Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 82beaaf
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-16T22:17:27Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 82beaaf
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-16T22:17:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 82beaaf
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-16T23:03:34Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 82beaaf
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T00:29:41Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 82beaaf
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T15:42:12Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: d3a6d5f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T16:01:39Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: d3a6d5f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T16:15:26Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: d3a6d5f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T16:49:34Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: d3a6d5f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…into processing-output-path

…cessing-output-path

…aker-python-sdk into processing-output-path

sagemaker-bot · 2020-09-17T17:12:04Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: b2348fc
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T17:30:05Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: b2348fc
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T17:44:59Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: b2348fc
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T17:54:32Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 70b0847
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T18:15:22Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 70b0847
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T18:26:15Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 70b0847
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T19:01:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 70b0847
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T19:29:40Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 01c0490
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T19:41:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 01c0490
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

metrizable · 2020-09-17T19:37:13Z

src/sagemaker/processing.py

@@ -170,7 +174,9 @@ def _extend_processing_args(self, inputs, outputs, **kwargs):  # pylint: disable
        """Extend inputs and outputs based on extra parameters"""
        return inputs, outputs

-    def _normalize_args(self, job_name=None, arguments=None, inputs=None, outputs=None, code=None):
+    def _normalize_args(
+        self, job_name=None, arguments=None, inputs=None, kms_key=None, outputs=None, code=None


can we make the code and kms_key as adjacent args, preferably after code, since they are the args that are associated?

Suggested change

self, job_name=None, arguments=None, inputs=None, kms_key=None, outputs=None, code=None

self, job_name=None, arguments=None, inputs=None, outputs=None, code=None, kms_key=None

Yes, it makes more sense to put relevant args together.

metrizable · 2020-09-17T19:39:15Z

src/sagemaker/processing.py

@@ -439,6 +453,7 @@ def run(
            job_name=job_name,
            arguments=arguments,
            inputs=inputs,
+            kms_key=kms_key,


push this to after code?

metrizable · 2020-09-17T19:39:42Z

src/sagemaker/processing.py

@@ -113,6 +113,7 @@ def __init__(
    def run(
        self,
        inputs=None,
+        kms_key=None,


maybe later in the arg list. if a customer is relying on ordinal position rather naming args on invocation, then this would be a breaking change.

sagemaker-bot · 2020-09-17T19:56:04Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 01c0490
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T20:06:40Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 01c0490
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T22:01:50Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 01c0490
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T22:25:40Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 8727aa7
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T22:55:55Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 8727aa7
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T22:57:05Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 8727aa7
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-17T22:59:25Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 8727aa7
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-18T20:43:46Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 181dba9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-18T21:02:21Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 181dba9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-18T21:04:22Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-pr
Commit ID: 181dba9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-09-18T21:14:52Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 181dba9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

* fix: allow kms encryption upload for processing * update spark processing with kms keys= * move kms_key to end of arg list Co-authored-by: Chuyang Deng <[email protected]>

Somehow the kms key fix aws#1897 that was recently merged is not propagate throughout all the methods that require itt when uploading customer code for example, in _upload_code() #L581 which is called in _handle_user_code_url() #L554, which in turn is referenced in _include_code_in_inputs() #L496. _include_code_in_inputs() is then called by _normalize_args() #L208 . This pull request address this issue.

fix: allow kms encryption upload for processing

1ab207f

Merge branch 'master' into processing-output-path

82beaaf

Merge branch 'master' into processing-output-path

d3a6d5f

Chuyang Deng added 3 commits September 17, 2020 10:02

Merge branch 'master' of github.com:ChuyangDeng/sagemaker-python-sdk …

ff1a03d

…into processing-output-path

Merge branch 'master' of github.com:aws/sagemaker-python-sdk into pro…

a2be04d

…cessing-output-path

Merge branch 'processing-output-path' of github.com:ChuyangDeng/sagem…

b2348fc

…aker-python-sdk into processing-output-path

update spark processing with kms keys=

70b0847

ajaykarpur previously approved these changes Sep 17, 2020

View reviewed changes

Merge branch 'master' into processing-output-path

01c0490

metrizable suggested changes Sep 17, 2020

View reviewed changes

move kms_key to end of arg list

8727aa7

chuyang-deng dismissed ajaykarpur’s stale review via 8727aa7 September 17, 2020 22:17

metrizable approved these changes Sep 18, 2020

View reviewed changes

Merge branch 'master' into processing-output-path

181dba9

chuyang-deng merged commit 7bb613e into aws:master Sep 21, 2020

This was referenced Nov 18, 2020

fix: use the provided kms key to upload code Christian-kam/sagemaker-python-sdk#1

Merged

fix: use the provided kms key to upload user code #1992

Closed

fix: use the provided kms key to upload user code Christian-kam/sagemaker-python-sdk#2

Merged

Christian-kam mentioned this pull request Dec 1, 2020

fix: use the provided kms key to upload user code #2005

Closed

7 tasks

	self, job_name=None, arguments=None, inputs=None, kms_key=None, outputs=None, code=None
	self, job_name=None, arguments=None, inputs=None, outputs=None, code=None, kms_key=None

fix: allow kms encryption upload for processing #1897

fix: allow kms encryption upload for processing #1897

Uh oh!

Conversation

chuyang-deng commented Sep 16, 2020

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Sep 17, 2020

AWS CodeBuild CI Report

Uh oh!

metrizable Sep 17, 2020

Choose a reason for hiding this comment

Uh oh!

chuyang-deng Sep 17, 2020

Choose a reason for hiding this comment