Python KMS Apiary P1 samples #779

ryanmats · 2017-02-02T04:52:42Z

Drafts of KMS Apiary Python P1 Samples

Had a few linting issues with really long lines - not sure how to fix those. Other than those, I addressed all the linting issues.

…n-docs-samples into python-kms-samples

theacodes

These should go in the api-client folder. (I know this isn't yet consistent in this repo)

theacodes · 2017-02-02T17:56:01Z

kms/api/functions.py

+
+
+# [START kms_create_keyring]
+def create_keyring(project_id, location, keyring):


docstrings throughout, please.

e.g.:

"""Creates a keyring in the given location."""

I might be useful to list the locations that are available.

theacodes · 2017-02-02T17:56:08Z

kms/api/functions.py

+import argparse
+import base64
+
+# Imports the Google APIs client library


This comment isn't necessary.

theacodes · 2017-02-02T17:59:26Z

kms/api/functions.py

+    # Creates an API client for the KMS API.
+    kms_client = discovery.build('cloudkms', 'v1beta1')
+
+    # The resource name of the location associated with the KeyRing.


nit: Does KeyRing need to be cased as such?

The cloud.google.com documentation refers to them as "CryptoKey" "KeyRing" etc. (https://cloud-dot-devsite.googleplex.com/kms/docs/creating-keys) and so does the Python API library documentation (https://developers.google.com/resources/api-libraries/documentation/cloudkms/v1beta1/python/latest/cloudkms_v1beta1.projects.locations.keyRings.cryptoKeys.html#getIamPolicy).

The canonical sample also uses this casing in comments.

theacodes · 2017-02-02T17:59:39Z

kms/api/functions.py

+
+    # Create KeyRing
+    request = kms_client.projects().locations().keyRings().create(
+        parent=parent, body={}, keyRingId=keyring)


Why is body empty here?

We need body for the request, where one can include optional specification fields (https://developers.google.com/resources/api-libraries/documentation/cloudkms/v1beta1/python/latest/cloudkms_v1beta1.projects.locations.keyRings.cryptoKeys.html#create). The canonical sample doesn't include any of the optional fields.

theacodes · 2017-02-02T18:00:12Z

kms/api/functions.py

+        parent=parent, body={}, keyRingId=keyring)
+    response = request.execute()
+
+    print 'Created KeyRing {}.'.format(response["name"])


You need to use print-as-a-function for this to work on Python 3.

(In general, you should be developing with Python 3.6, let me know if you need help setting that up)

theacodes · 2017-02-02T18:05:38Z

kms/api/functions.py

+    response = request.execute()
+
+    # Write the encrypted text to a file.
+    o = open(outfile, 'w')


use a context manager here as well.

Also, is the ciphertext base64 encoded? If not, you should open the file as binary rb.

theacodes · 2017-02-02T18:06:18Z

kms/api/functions.py

+
+
+# [START kms_decrypt]
+def decrypt(project_id, location, keyring, cryptokey, infile, outfile):


Apply my same comments from encrypt to this function.

theacodes · 2017-02-02T18:07:21Z

kms/api/functions.py

+        project_id, location, keyring, cryptokey)
+
+    # Get the current IAM policy and add the new member to it.
+    policy_request = kms_client.projects().locations().keyRings().cryptoKeys().getIamPolicy(resource=parent)


Holy cow. How is this done in other languages?

talked with Brent and it looks like the API handles duplicates and everything, so I changed this so now I just append the new role / member pair to the end of the bindings.

theacodes · 2017-02-02T18:08:27Z

kms/api/functions_test.py

+
+
+# Your Google Cloud Platform project ID
+PROJECT_ID = 'YOUR_PROJECT_ID'


This is available from the cloud_config fixture.

theacodes · 2017-02-02T18:08:46Z

kms/api/functions_test.py

+# Your Google Cloud Platform CryptoKey name
+CRYPTOKEY = 'sample-key-4'
+
+# An infile for text to be encrypted


Use pytest's tmpfile fixture for files, please.

…n-docs-samples into python-kms-samples

theacodes · 2017-02-04T01:43:26Z

kms/api-client/functions.py

+
+# [START kms_create_keyring]
+def create_keyring(project_id, location, keyring):
+    """Creates a KeyRing in the given location. Potential locations include: 


Summary should be one sentence.

theacodes · 2017-02-04T01:44:29Z

kms/api-client/functions.py

+# [START kms_create_keyring]
+def create_keyring(project_id, location, keyring):
+    """Creates a KeyRing in the given location. Potential locations include: 
+    global, asia-east1, europe-west1, us-central1, and us-east1."""


Since these can change, better to just link to the documentation.

can't seem to find any documentation that lists out all locations. gonna just say "(e.g. global)."

theacodes · 2017-02-04T01:46:17Z

kms/api-client/functions.py

+
+    # Create a CryptoKey for the given KeyRing.
+    request = kms_client.projects().locations().keyRings().cryptoKeys().create(
+        parent=parent, body={"purpose": 'ENCRYPT_DECRYPT'},


single quotes everywhere (except for docstrings), please.

theacodes · 2017-02-04T01:47:22Z

kms/api-client/functions.py

+    # Read text from the input file.
+    with io.open(plaintext_file_name, 'rb') as plaintext_file:
+        plaintext = plaintext_file.read()
+        encoded_text = base64.b64encode(plaintext)


You can de-dent everything after the call to read(). You no longer need the file open after getting its contents.

theacodes · 2017-02-04T01:52:05Z

kms/api-client/functions.py

+    kms_client = discovery.build('cloudkms', 'v1beta1')
+
+    # Construct the resource name of the CryptoKeyVersion.
+    name = 'projects/{}/locations/{}/'.format(project_id, location)


do this to break the string up over multiple lines:

name = ( 'projects/..........' '/....' '.....')

Python will automatically concatenate them.

theacodes · 2017-02-04T01:53:27Z

kms/api-client/functions.py

@@ -0,0 +1,292 @@
+#!/usr/bin/env python


name this file snippets.py.

theacodes · 2017-02-04T01:54:26Z

kms/api-client/functions_test.py

+import functions
+
+
+# Your Google Cloud Platform Key Location


The audience for these comments is us, the maintainers. So maybe it makes sense to either remove or revise them?

Revised them. Kept a few comments for my own reference in the future.

theacodes · 2017-02-04T01:54:47Z

kms/api-client/functions_test.py

+    functions.create_cryptokey(
+        cloud_config.project, LOCATION, KEYRING, CRYPTOKEY)
+    out, _ = capsys.readouterr()
+    expected = 'Created CryptoKey projects/{}/'.format(cloud_config.project)


see my previous comment about breaking up long strings.

theacodes · 2017-02-04T01:55:25Z

kms/api-client/functions_test.py

+        encrypted_file_name, decrypted_file_name)
+
+    # Make sure the decrypted text matches the original text.
+    decrypted_text = open(decrypted_file_name).read()


if you hadn't already cast to a str, you could do decrypted_file.read().

changed (now setting up variables like 'plaintext_file', and then calling encrypt / decrypt with str(plaintext_file) )

theacodes · 2017-02-04T01:56:00Z

kms/api-client/functions_test.py

+    assert 'Saved decrypted text to {}.'.format(decrypted_file_name) in out
+
+
+def test_encrypt_decrypt_cli(capsys, cloud_config, tmpdir):


This test isn't necessary - we don't typically test the argparse section.

…n-docs-samples into python-kms-samples

theacodes · 2017-02-07T00:01:11Z

kms/api-client/snippets.py

+        resource=parent, body={'policy': policy_response})
+    request.execute()
+
+    print_msg = (


Don't do this, you can put this all within the print statement.

bshaffer · 2017-02-07T00:18:15Z

The README should be updated to include snippets.py I believe

Change-Id: Ib79dc1345c9c40547f3fd4e9c3c9a48963a3b399

…ython-docs-samples into python-kms-samples

Change-Id: Icf4a66083f56d6f51be76ba1cf3b5dc8daf2c4c1

Change-Id: I2fbaa55092ef8787f1423d499aa310cab258c0c1

theacodes · 2017-02-07T00:21:37Z

The README should be updated to include snippets.py I believe

Done.

theacodes · 2017-02-07T00:22:11Z

@ryanmats You still need to add the remaining commands for the rest of the samples in snippets. Currently there's just encrypt, decrypt, and other.

…atform/python-docs-samples into python-kms-samples

theacodes · 2017-02-07T16:54:40Z

@ryanmats can you run the nox readmegen session for this sample? After that, feel free to merge.

…n-docs-samples into python-kms-samples

theacodes · 2017-02-07T21:06:49Z

@dpebot will you merge when travis passes?

dpebot · 2017-02-07T21:06:50Z

Okay! I'll merge when all statuses are green and all reviewers approve.

…-samples#779) * Draft of first half of KMS samples * reversed wrong change * KMS Apiary Python samples - P1 * Few minor style issues * Adding back in space i accidentally deleted * Addressed all code review comments * Renamed api directory to api-client * Addressed more code review comments * Formatting change * Fix quickstart test Change-Id: Ib79dc1345c9c40547f3fd4e9c3c9a48963a3b399 * Update readme Change-Id: Icf4a66083f56d6f51be76ba1cf3b5dc8daf2c4c1 * Add readme Change-Id: I2fbaa55092ef8787f1423d499aa310cab258c0c1 * Added parsers * Final minor changes to parsers * Added autogenerated README * Changed snippets_test keyring name and cryptokey name

* Draft of first half of KMS samples * reversed wrong change * KMS Apiary Python samples - P1 * Few minor style issues * Adding back in space i accidentally deleted * Addressed all code review comments * Renamed api directory to api-client * Addressed more code review comments * Formatting change * Fix quickstart test Change-Id: Ib79dc1345c9c40547f3fd4e9c3c9a48963a3b399 * Update readme Change-Id: Icf4a66083f56d6f51be76ba1cf3b5dc8daf2c4c1 * Add readme Change-Id: I2fbaa55092ef8787f1423d499aa310cab258c0c1 * Added parsers * Final minor changes to parsers * Added autogenerated README * Changed snippets_test keyring name and cryptokey name

…-samples#779) * Draft of first half of KMS samples * reversed wrong change * KMS Apiary Python samples - P1 * Few minor style issues * Adding back in space i accidentally deleted * Addressed all code review comments * Renamed api directory to api-client * Addressed more code review comments * Formatting change * Fix quickstart test Change-Id: Ib79dc1345c9c40547f3fd4e9c3c9a48963a3b399 * Update readme Change-Id: Icf4a66083f56d6f51be76ba1cf3b5dc8daf2c4c1 * Add readme Change-Id: I2fbaa55092ef8787f1423d499aa310cab258c0c1 * Added parsers * Final minor changes to parsers * Added autogenerated README * Changed snippets_test keyring name and cryptokey name

ryanmats added 6 commits January 31, 2017 00:52

Draft of first half of KMS samples

9f67a99

reversed wrong change

6d82309

KMS Apiary Python samples - P1

5529d40

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

13c7c74

…n-docs-samples into python-kms-samples

Few minor style issues

491b6d9

Adding back in space i accidentally deleted

21f812c

ryanmats requested a review from theacodes February 2, 2017 04:52

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 2, 2017

theacodes suggested changes Feb 2, 2017

View reviewed changes

ryanmats added 3 commits February 3, 2017 15:53

Addressed all code review comments

2a92880

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

17c735c

…n-docs-samples into python-kms-samples

Renamed api directory to api-client

9cf08a4

theacodes suggested changes Feb 4, 2017

View reviewed changes

ryanmats added 2 commits February 6, 2017 14:33

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

d8d15db

…n-docs-samples into python-kms-samples

Addressed more code review comments

816877a

theacodes reviewed Feb 7, 2017

View reviewed changes

Formatting change

51ef300

Jon Wayne Parrott added 4 commits February 6, 2017 16:19

Fix quickstart test

1be099c

Change-Id: Ib79dc1345c9c40547f3fd4e9c3c9a48963a3b399

Merge branch 'python-kms-samples' of github.com:GoogleCloudPlatform/p…

eb07c44

…ython-docs-samples into python-kms-samples

Update readme

8e9f3d0

Change-Id: Icf4a66083f56d6f51be76ba1cf3b5dc8daf2c4c1

Add readme

14fb5c0

Change-Id: I2fbaa55092ef8787f1423d499aa310cab258c0c1

ryanmats added 3 commits February 6, 2017 18:35

Added parsers

b44fdfd

Merge branch 'python-kms-samples' of https://github.com/GoogleCloudPl…

6ee4bd9

…atform/python-docs-samples into python-kms-samples

Final minor changes to parsers

ce75417

theacodes approved these changes Feb 7, 2017

View reviewed changes

Added autogenerated README

842af60

Merge branch 'master' of https://github.com/GoogleCloudPlatform/pytho…

599341f

…n-docs-samples into python-kms-samples

dpebot added the automerge Merge the pull request once unit tests and other checks pass. label Feb 7, 2017

dpebot self-assigned this Feb 7, 2017

Changed snippets_test keyring name and cryptokey name

1c53533

dpebot merged commit f7179af into master Feb 7, 2017

dpebot deleted the python-kms-samples branch February 7, 2017 21:15

rsamborski mentioned this pull request Nov 8, 2022

migrate code from googleapis/python-kms #8470

Merged

8 tasks



		# [START kms_create_keyring]
		def create_keyring(project_id, location, keyring):



		# [START kms_decrypt]
		def decrypt(project_id, location, keyring, cryptokey, infile, outfile):



		# Your Google Cloud Platform project ID
		PROJECT_ID = 'YOUR_PROJECT_ID'

		assert 'Saved decrypted text to {}.'.format(decrypted_file_name) in out


		def test_encrypt_decrypt_cli(capsys, cloud_config, tmpdir):

Python KMS Apiary P1 samples #779

Python KMS Apiary P1 samples #779

Conversation

ryanmats commented Feb 2, 2017 • edited by dpebot Loading

theacodes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bshaffer commented Feb 7, 2017

theacodes commented Feb 7, 2017

theacodes commented Feb 7, 2017

theacodes commented Feb 7, 2017

theacodes commented Feb 7, 2017

dpebot commented Feb 7, 2017

ryanmats commented Feb 2, 2017 •

edited by dpebot

Loading