Skip to content

feat: Added encryption_spec_key_name to all relevant resources#256

Merged
ivanmkc merged 22 commits into
googleapis:devfrom
ivanmkc:imkc--cmek-support2
Mar 11, 2021
Merged

feat: Added encryption_spec_key_name to all relevant resources#256
ivanmkc merged 22 commits into
googleapis:devfrom
ivanmkc:imkc--cmek-support2

Conversation

@ivanmkc

@ivanmkc ivanmkc commented Mar 3, 2021

Copy link
Copy Markdown
Contributor

The encryption key can be passed into aiplatform.init which will be applied to all applicable resources:

aiplatform.init(
            project=_TEST_PROJECT,
            encryption_spec_key_name=_TEST_DEFAULT_ENCRYPTION_KEY_NAME,
        )

Alternatively, it can be passed directly into the resource itself on creation. This will override the aiplatform default.

  1. Added CMEK support to:
  • Dataset
  • ImageDataset
  • TabularDataset
  • TextDataset
  • VideoDataset
  • Endpoint
  • Batch Prediction Job
  • Model
  • Training Pipeline
  • _TrainingJob
  • _CustomTrainingJob
  • CustomPythonPackageTrainingJob
  • CustomContainerTrainingJob
  • CustomTrainingJob
  • AutoMLTabularTrainingJob
  • AutoMLImageTrainingJob
  • AutoMLTextTrainingJob
  1. Added tests:
  • Dataset
  • ImageDataset
  • TabularDataset
  • TextDataset
  • VideoDataset
  • Endpoint
  • Batch Prediction Job
  • Model
  • Training Pipeline
  • _TrainingJob
  • _CustomTrainingJob
  • CustomPythonPackageTrainingJob
  • CustomContainerTrainingJob
  • CustomTrainingJob
  • AutoMLTabularTrainingJob
  • AutoMLImageTrainingJob
  • AutoMLTextTrainingJob

Currently Unused:

  • Custom Job
  • Hyper-parameter Tuning

Fixes b/178500930

@ivanmkc ivanmkc requested review from a team March 3, 2021 23:16
@ivanmkc ivanmkc changed the base branch from master to dev March 3, 2021 23:16
@ivanmkc ivanmkc requested a review from sasha-gitg March 3, 2021 23:16
@ivanmkc ivanmkc changed the title Added encryption_spec_key_name to Dataset (and subclasses) and Endpoint [WIP] Added encryption_spec_key_name to Dataset (and subclasses) and Endpoint Mar 3, 2021
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Mar 3, 2021
Comment thread google/cloud/aiplatform/datasets/dataset.py
@ivanmkc ivanmkc force-pushed the imkc--cmek-support2 branch from c0efeb7 to 7729595 Compare March 3, 2021 23:39
@ivanmkc ivanmkc force-pushed the imkc--cmek-support2 branch from cd16b6f to 7646caf Compare March 8, 2021 22:08
@ivanmkc ivanmkc changed the title [WIP] Added encryption_spec_key_name to Dataset (and subclasses) and Endpoint feat: Added encryption_spec_key_name to Dataset (and subclasses) and Endpoint Mar 8, 2021
@ivanmkc

ivanmkc commented Mar 8, 2021

Copy link
Copy Markdown
Contributor Author

I've modified existing tests to handle CMEK for the cases where it's passed into aiplatform.init and where it's passed into the job itself. Let me know if any of that is weird or could be improved.

Comment thread google/cloud/aiplatform/initializer.py
@ivanmkc ivanmkc changed the title feat: Added encryption_spec_key_name to Dataset (and subclasses) and Endpoint feat: Added encryption_spec_key_name to all relevant resources Mar 8, 2021
Comment thread google/cloud/aiplatform/datasets/dataset.py
Comment thread google/cloud/aiplatform/datasets/dataset.py Outdated
Comment thread google/cloud/aiplatform/datasets/dataset.py Outdated
Comment thread google/cloud/aiplatform/datasets/dataset.py Outdated
credentials (auth_credentials.Credentials):
Custom credentials to use to run call training service. Overrides
credentials set in aiplatform.init.
training_pipeline_encryption_spec_key_name (Optional[str]):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd possibly remove pipeline from this name so it's consistent with the class name. If we end up changing this the XTrainingPipeline instead of XTrainignJob, we can add it back. You can leave the docstring as is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread google/cloud/aiplatform/training_jobs.py Outdated
Comment thread google/cloud/aiplatform/training_jobs.py Outdated
Comment thread google/cloud/aiplatform/datasets/dataset.py Outdated
Comment thread google/cloud/aiplatform/initializer.py
Comment thread google/cloud/aiplatform/initializer.py Outdated
Comment thread google/cloud/aiplatform/jobs.py Outdated
Comment thread google/cloud/aiplatform/models.py Outdated
Comment thread google/cloud/aiplatform/models.py Outdated
Comment thread google/cloud/aiplatform/models.py
@ivanmkc ivanmkc force-pushed the imkc--cmek-support2 branch from 267feeb to 25c3ef4 Compare March 10, 2021 19:18
@ivanmkc

ivanmkc commented Mar 10, 2021

Copy link
Copy Markdown
Contributor Author

@sasha-gitg Fixed according to comments and then improved some docstrings.

@sasha-gitg sasha-gitg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks Ivan!

self, encryption_spec_key_name: Optional[str]
) -> Optional[gca_encryption_spec.EncryptionSpec]:
"""Creates a gca_encryption_spec.EncryptionSpec instance from the given key name.
If the provided key name is None, it uses the default key name if provided.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs Args and Returns section.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to improve my docstring game. Thanks

@ivanmkc ivanmkc merged commit d75da52 into googleapis:dev Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants