From 766cc3fed8646be806ee692418b2fd92ebfee814 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Mon, 11 Nov 2019 21:09:53 +0100 Subject: [PATCH 01/16] added new secret support --- sdk/python/kfp/utils.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 sdk/python/kfp/utils.py diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py new file mode 100644 index 00000000000..e90923ba2e5 --- /dev/null +++ b/sdk/python/kfp/utils.py @@ -0,0 +1,38 @@ +from typing import List + +def use_secret(secret_name:List=None, volume_name:List=None, secret_volume_mount_path:List=None): + + """An operator that configures the container to use a secret. + + This assumes that the secret is created and availabel in the k8s cluster + the user are expected to set the volume names in order to allow for multiple + volumes to be mounted. + """ + params = [secret_name, volume_name, secret_volume_mount_path] + param_names = ["secret_name", "volume_name", "secret_volume_mount_path"] + for param, param_name in params, param_names: + if param is None: + raise ValueError("'{}' needs to be specified, is: {}".format(param_name, param)) + if type(param) is not list: + raise ValueError("Parameter {} needs to be a list".format(param_name)) + + def _use_secret(task): + from kubernetes import client as k8s_client + task = task.add_volume( + k8s_client.V1Volume( + name=volume_name, + secret=k8s_client.V1SecretVolumeSource( + secret_name=secret_name, + ) + ) + ) + task.container \ + .add_volume_mount( + k8s_client.V1VolumeMount( + name=volume_name, + mount_path=secret_volume_mount_path, + ) + ) + return task + + return use_secret \ No newline at end of file From 3c9b54bd5b260e875e5bdf469eea2abb464d59c5 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Thu, 21 Nov 2019 18:06:30 +0100 Subject: [PATCH 02/16] updated the documentation and env settings --- sdk/python/kfp/utils.py | 63 ++++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py index e90923ba2e5..866f6aea32f 100644 --- a/sdk/python/kfp/utils.py +++ b/sdk/python/kfp/utils.py @@ -1,38 +1,61 @@ -from typing import List - -def use_secret(secret_name:List=None, volume_name:List=None, secret_volume_mount_path:List=None): - - """An operator that configures the container to use a secret. +def use_secret(secret_name=None, volume_name=None, secret_volume_mount_path=None, env_variable=None, secret_file_path_in_volume=None): + """ + An operator that configures the container to use a secret. - This assumes that the secret is created and availabel in the k8s cluster - the user are expected to set the volume names in order to allow for multiple - volumes to be mounted. + This assumes that the secret is created and availabel in the k8s cluster. + + Keyword Arguments: + secret_name {String} -- The k8s secret name (default: {None}) + volume_name {String} -- The pod volume name (default: {None}) + secret_volume_mount_path {String} -- The path to the secret that is mounted (default: {None}) + env_variable {String} -- Env variable pointing to the mounted secret file. Requires both the env_variable and secret_file_path_in_volume to be defined. + The value is the path to the secret (default: {None}) + secret_file_path_in_volume {String} -- The path to the secret in the volume. This will be the value of env_variable. + Both env_variable and secret_file_path_in_volume needs to be set if any env variable should be created (default: {None}) + + Raises: + ValueError: If not the necessary variables (secret_name, volume_name", secret_volume_mount_path) are supplied. + Or only one of env_variable and secret_file_path_in_volume are supplied + + Returns: + [ContainerOperator] -- Returns the container operator after it has been modified. """ + + if not volume_name: + volumen_name = secret_name + "_volume" + params = [secret_name, volume_name, secret_volume_mount_path] param_names = ["secret_name", "volume_name", "secret_volume_mount_path"] - for param, param_name in params, param_names: + for param, param_name in zip(params, param_names): if param is None: raise ValueError("'{}' needs to be specified, is: {}".format(param_name, param)) - if type(param) is not list: - raise ValueError("Parameter {} needs to be a list".format(param_name)) + if (env_variable and not secret_file_path_in_volume) or (secret_file_path_in_volume and not env_variable): + raise ValueError("Both {} and {} needs to be supplied together or not at all".format(env_variable, secret_file_path_in_volume)) + def _use_secret(task): + import os from kubernetes import client as k8s_client task = task.add_volume( k8s_client.V1Volume( name=volume_name, secret=k8s_client.V1SecretVolumeSource( - secret_name=secret_name, + secret_name=secret_name ) ) - ) - task.container \ - .add_volume_mount( - k8s_client.V1VolumeMount( - name=volume_name, - mount_path=secret_volume_mount_path, - ) + ).add_volume_mount( + k8s_client.V1VolumeMount( + name=volume_name, + mount_path=secret_volume_mount_path ) + ) + if env_variable: + task.container.add_env_variable( + k8s_client.V1EnvVar( + name=env_variable, + value=os.path.join(secret_volume_mount_path, secret_file_path_in_volume), + ) + ) return task - return use_secret \ No newline at end of file + return _use_secret \ No newline at end of file From 17fbf845e7bad57fb11b799eb119f50b7fe3ce69 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Fri, 22 Nov 2019 11:24:06 +0100 Subject: [PATCH 03/16] updated after feedback --- sdk/python/kfp/utils.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py index 866f6aea32f..ce3bd155d4b 100644 --- a/sdk/python/kfp/utils.py +++ b/sdk/python/kfp/utils.py @@ -1,13 +1,13 @@ -def use_secret(secret_name=None, volume_name=None, secret_volume_mount_path=None, env_variable=None, secret_file_path_in_volume=None): +def use_secret(secret_name:str=None, volume_name:str=None, secret_volume_mount_path:str=None, env_variable:str=None, secret_file_path_in_volume:str=None): """ An operator that configures the container to use a secret. This assumes that the secret is created and availabel in the k8s cluster. Keyword Arguments: - secret_name {String} -- The k8s secret name (default: {None}) + secret_name {String} -- [Required] The k8s secret name (default: {None}) volume_name {String} -- The pod volume name (default: {None}) - secret_volume_mount_path {String} -- The path to the secret that is mounted (default: {None}) + secret_volume_mount_path {String} -- [Required] The path to the secret that is mounted (default: {None}) env_variable {String} -- Env variable pointing to the mounted secret file. Requires both the env_variable and secret_file_path_in_volume to be defined. The value is the path to the secret (default: {None}) secret_file_path_in_volume {String} -- The path to the secret in the volume. This will be the value of env_variable. @@ -21,8 +21,8 @@ def use_secret(secret_name=None, volume_name=None, secret_volume_mount_path=None [ContainerOperator] -- Returns the container operator after it has been modified. """ - if not volume_name: - volumen_name = secret_name + "_volume" + if secret_name: + volume_name = volume_name or secret_name + '_volume' params = [secret_name, volume_name, secret_volume_mount_path] param_names = ["secret_name", "volume_name", "secret_volume_mount_path"] @@ -30,7 +30,7 @@ def use_secret(secret_name=None, volume_name=None, secret_volume_mount_path=None if param is None: raise ValueError("'{}' needs to be specified, is: {}".format(param_name, param)) - if (env_variable and not secret_file_path_in_volume) or (secret_file_path_in_volume and not env_variable): + if bool(env_variable) != bool(secret_file_path_in_volume): raise ValueError("Both {} and {} needs to be supplied together or not at all".format(env_variable, secret_file_path_in_volume)) def _use_secret(task): From 2b53468d1c9ce4d2122b09cd75ecdce063a719a0 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Fri, 22 Nov 2019 16:37:35 +0100 Subject: [PATCH 04/16] added tests --- sdk/python/tests/dsl/utils_tests.py | 67 +++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 sdk/python/tests/dsl/utils_tests.py diff --git a/sdk/python/tests/dsl/utils_tests.py b/sdk/python/tests/dsl/utils_tests.py new file mode 100644 index 00000000000..1f625c529c4 --- /dev/null +++ b/sdk/python/tests/dsl/utils_tests.py @@ -0,0 +1,67 @@ +from kfp.dsl import ContainerOp +from kfp.utils import use_secret +import unittest +import inspect + + +class TestAddSecrets(unittest.TestCase): + def test_default_use_secret(self): + spec = inspect.getfullargspec(use_secret) + assert len(spec.defaults) == 5 + for spec in spec.defaults: + assert spec == None + + + def test_use_default_use_secret(self): + op1 = ContainerOp(name='op1', image='image') + secret_name = "my-secret" + secret_path = "/here/are/my/secret" + op1 = op1.apply(use_secret(secret_name=secret_name, + secret_volume_mount_path=secret_path)) + assert type(op1.container.env) == type(None) + + container_dict = op1.container.to_dict() + volume_mounts = container_dict["volume_mounts"][0] + assert type(volume_mounts)==dict + assert volume_mounts["name"] == secret_name + '_volume' + assert volume_mounts["mount_path"] == secret_path + + + def test_use_default_use_secret(self): + op1 = ContainerOp(name='op1', image='image') + secret_name = "my-secret" + secret_path = "/here/are/my/secret" + volume_name = "my_volume" + op1 = op1.apply(use_secret(secret_name=secret_name, + secret_volume_mount_path=secret_path, + volume_name = volume_name)) + assert type(op1.container.env) == type(None) + + container_dict = op1.container.to_dict() + volume_mounts = container_dict["volume_mounts"][0] + assert type(volume_mounts)==dict + assert volume_mounts["name"] == volume_name + assert volume_mounts["mount_path"] == secret_path + + + def test_use_set_env_ues_secret(self): + op1 = ContainerOp(name='op1', image='image') + secret_name = "my-secret" + secret_path = "/here/are/my/secret/" + env_variable = "MY_SECRET" + secret_file_path_in_volume = "secret.json" + op1 = op1.apply(use_secret(secret_name=secret_name, + secret_volume_mount_path=secret_path, + env_variable=env_variable, + secret_file_path_in_volume=secret_file_path_in_volume)) + assert len(op1.container.env) ==1 + + container_dict = op1.container.to_dict() + volume_mounts = container_dict["volume_mounts"][0] + assert type(volume_mounts)==dict + assert volume_mounts["name"] == secret_name + '_volume' + assert volume_mounts["mount_path"] == secret_path + env_dict = op1.container.env[0].to_dict() + assert env_dict["name"] == env_variable + assert env_dict["value"] == secret_path + secret_file_path_in_volume + From 9523d011ed55b2a31c1e308eeb08832569498ed6 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Fri, 22 Nov 2019 16:43:16 +0100 Subject: [PATCH 05/16] nameing issue fixed --- sdk/python/tests/dsl/utils_tests.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/python/tests/dsl/utils_tests.py b/sdk/python/tests/dsl/utils_tests.py index 1f625c529c4..98afdf7bff4 100644 --- a/sdk/python/tests/dsl/utils_tests.py +++ b/sdk/python/tests/dsl/utils_tests.py @@ -27,7 +27,7 @@ def test_use_default_use_secret(self): assert volume_mounts["mount_path"] == secret_path - def test_use_default_use_secret(self): + def test_use_set_volume_use_secret(self): op1 = ContainerOp(name='op1', image='image') secret_name = "my-secret" secret_path = "/here/are/my/secret" @@ -64,4 +64,3 @@ def test_use_set_env_ues_secret(self): env_dict = op1.container.env[0].to_dict() assert env_dict["name"] == env_variable assert env_dict["value"] == secret_path + secret_file_path_in_volume - From 6e32a3bc1228b106691ca2949fd086daa08929f8 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Sun, 24 Nov 2019 21:13:05 +0100 Subject: [PATCH 06/16] renamed test to follow unittest standard --- .../dsl/{utils_tests.py => test_utils.py} | 39 +++++++------------ 1 file changed, 14 insertions(+), 25 deletions(-) rename sdk/python/tests/dsl/{utils_tests.py => test_utils.py} (63%) diff --git a/sdk/python/tests/dsl/utils_tests.py b/sdk/python/tests/dsl/test_utils.py similarity index 63% rename from sdk/python/tests/dsl/utils_tests.py rename to sdk/python/tests/dsl/test_utils.py index 98afdf7bff4..80454d66f0c 100644 --- a/sdk/python/tests/dsl/utils_tests.py +++ b/sdk/python/tests/dsl/test_utils.py @@ -5,12 +5,6 @@ class TestAddSecrets(unittest.TestCase): - def test_default_use_secret(self): - spec = inspect.getfullargspec(use_secret) - assert len(spec.defaults) == 5 - for spec in spec.defaults: - assert spec == None - def test_use_default_use_secret(self): op1 = ContainerOp(name='op1', image='image') @@ -18,14 +12,12 @@ def test_use_default_use_secret(self): secret_path = "/here/are/my/secret" op1 = op1.apply(use_secret(secret_name=secret_name, secret_volume_mount_path=secret_path)) - assert type(op1.container.env) == type(None) - + self.assertEqual(type(op1.container.env), type(None)) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] - assert type(volume_mounts)==dict - assert volume_mounts["name"] == secret_name + '_volume' - assert volume_mounts["mount_path"] == secret_path - + self.assertEqual(type(volume_mounts),dict) + self.assertEqual(volume_mounts["name"],secret_name + '_volume') + self.assertEqual(volume_mounts["mount_path"], secret_path) def test_use_set_volume_use_secret(self): op1 = ContainerOp(name='op1', image='image') @@ -35,14 +27,12 @@ def test_use_set_volume_use_secret(self): op1 = op1.apply(use_secret(secret_name=secret_name, secret_volume_mount_path=secret_path, volume_name = volume_name)) - assert type(op1.container.env) == type(None) - + self.assertEqual( type(op1.container.env), type(None)) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] - assert type(volume_mounts)==dict - assert volume_mounts["name"] == volume_name - assert volume_mounts["mount_path"] == secret_path - + self.assertEqual( type(volume_mounts), dict) + self.assertEqual( volume_mounts["name"], volume_name) + self.assertEqual( volume_mounts["mount_path"], secret_path) def test_use_set_env_ues_secret(self): op1 = ContainerOp(name='op1', image='image') @@ -54,13 +44,12 @@ def test_use_set_env_ues_secret(self): secret_volume_mount_path=secret_path, env_variable=env_variable, secret_file_path_in_volume=secret_file_path_in_volume)) - assert len(op1.container.env) ==1 - + self.assertEqual( len(op1.container.env), 1) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] - assert type(volume_mounts)==dict - assert volume_mounts["name"] == secret_name + '_volume' - assert volume_mounts["mount_path"] == secret_path + self.assertEqual(type(volume_mounts), dict) + self.assertEqual(volume_mounts["name"], secret_name + '_volume') + self.assertEqual(volume_mounts["mount_path"], secret_path) env_dict = op1.container.env[0].to_dict() - assert env_dict["name"] == env_variable - assert env_dict["value"] == secret_path + secret_file_path_in_volume + self.assertEqual(env_dict["name"], env_variable) + self.assertEqual(env_dict["value"], secret_path + secret_file_path_in_volume) \ No newline at end of file From d86cd612bc9b74cb400bb64a912546d1c6a76c88 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Sun, 24 Nov 2019 21:13:39 +0100 Subject: [PATCH 07/16] updated after feedback --- sdk/python/kfp/utils.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py index ce3bd155d4b..ea8453a9f68 100644 --- a/sdk/python/kfp/utils.py +++ b/sdk/python/kfp/utils.py @@ -1,17 +1,17 @@ -def use_secret(secret_name:str=None, volume_name:str=None, secret_volume_mount_path:str=None, env_variable:str=None, secret_file_path_in_volume:str=None): +def use_secret(secret_name:str, secret_volume_mount_path:str, volume_name:str=None, env_variable:str=None, secret_file_path_in_volume:str=None): """ An operator that configures the container to use a secret. This assumes that the secret is created and availabel in the k8s cluster. Keyword Arguments: - secret_name {String} -- [Required] The k8s secret name (default: {None}) - volume_name {String} -- The pod volume name (default: {None}) - secret_volume_mount_path {String} -- [Required] The path to the secret that is mounted (default: {None}) + secret_name {String} -- [Required] The k8s secret name. + volume_name {String} -- The pod volume name. + secret_volume_mount_path {String} -- [Required] The path to the secret that is mounted. env_variable {String} -- Env variable pointing to the mounted secret file. Requires both the env_variable and secret_file_path_in_volume to be defined. - The value is the path to the secret (default: {None}) + The value is the path to the secret. secret_file_path_in_volume {String} -- The path to the secret in the volume. This will be the value of env_variable. - Both env_variable and secret_file_path_in_volume needs to be set if any env variable should be created (default: {None}) + Both env_variable and secret_file_path_in_volume needs to be set if any env variable should be created. Raises: ValueError: If not the necessary variables (secret_name, volume_name", secret_volume_mount_path) are supplied. @@ -24,12 +24,6 @@ def use_secret(secret_name:str=None, volume_name:str=None, secret_volume_mount_p if secret_name: volume_name = volume_name or secret_name + '_volume' - params = [secret_name, volume_name, secret_volume_mount_path] - param_names = ["secret_name", "volume_name", "secret_volume_mount_path"] - for param, param_name in zip(params, param_names): - if param is None: - raise ValueError("'{}' needs to be specified, is: {}".format(param_name, param)) - if bool(env_variable) != bool(secret_file_path_in_volume): raise ValueError("Both {} and {} needs to be supplied together or not at all".format(env_variable, secret_file_path_in_volume)) From 7a11fd487d154f0fed1c890aa8e6552876710909 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Sun, 24 Nov 2019 21:19:28 +0100 Subject: [PATCH 08/16] the new test after renaming --- sdk/python/kfp/test_utils.py | 58 ++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 sdk/python/kfp/test_utils.py diff --git a/sdk/python/kfp/test_utils.py b/sdk/python/kfp/test_utils.py new file mode 100644 index 00000000000..f1daac7e66b --- /dev/null +++ b/sdk/python/kfp/test_utils.py @@ -0,0 +1,58 @@ +from kfp.dsl import ContainerOp +from utils import use_secret +import unittest +import inspect + + +class TestAddSecrets(unittest.TestCase): + + def test_use_default_use_secret(self): + op1 = ContainerOp(name='op1', image='image') + secret_name = "my-secret" + secret_path = "/here/are/my/secret" + op1 = op1.apply(use_secret(secret_name=secret_name, + secret_volume_mount_path=secret_path)) + self.assertEqual(type(op1.container.env), type(None)) + container_dict = op1.container.to_dict() + volume_mounts = container_dict["volume_mounts"][0] + self.assertEqual(type(volume_mounts),dict) + self.assertEqual(volume_mounts["name"],secret_name + '_volume') + self.assertEqual(volume_mounts["mount_path"], secret_path) + + def test_use_set_volume_use_secret(self): + op1 = ContainerOp(name='op1', image='image') + secret_name = "my-secret" + secret_path = "/here/are/my/secret" + volume_name = "my_volume" + op1 = op1.apply(use_secret(secret_name=secret_name, + secret_volume_mount_path=secret_path, + volume_name = volume_name)) + self.assertEqual( type(op1.container.env), type(None)) + container_dict = op1.container.to_dict() + volume_mounts = container_dict["volume_mounts"][0] + self.assertEqual( type(volume_mounts), dict) + self.assertEqual( volume_mounts["name"], volume_name) + self.assertEqual( volume_mounts["mount_path"], secret_path) + + def test_use_set_env_ues_secret(self): + op1 = ContainerOp(name='op1', image='image') + secret_name = "my-secret" + secret_path = "/here/are/my/secret/" + env_variable = "MY_SECRET" + secret_file_path_in_volume = "secret.json" + op1 = op1.apply(use_secret(secret_name=secret_name, + secret_volume_mount_path=secret_path, + env_variable=env_variable, + secret_file_path_in_volume=secret_file_path_in_volume)) + self.assertEqual( len(op1.container.env), 1) + container_dict = op1.container.to_dict() + volume_mounts = container_dict["volume_mounts"][0] + self.assertEqual(type(volume_mounts), dict) + self.assertEqual(volume_mounts["name"], secret_name + '_volume') + self.assertEqual(volume_mounts["mount_path"], secret_path) + env_dict = op1.container.env[0].to_dict() + self.assertEqual(env_dict["name"], env_variable) + self.assertEqual(env_dict["value"], secret_path + secret_file_path_in_volume) + +if __name__ == '__main__': + unittest.main() From ab0a51102cefb50f847b05f3650c5a143bd56b93 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Sun, 24 Nov 2019 21:20:21 +0100 Subject: [PATCH 09/16] added the test to main --- sdk/python/tests/dsl/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/python/tests/dsl/main.py b/sdk/python/tests/dsl/main.py index 0122876a9ea..1675564bffe 100644 --- a/sdk/python/tests/dsl/main.py +++ b/sdk/python/tests/dsl/main.py @@ -29,6 +29,7 @@ import volume_op_tests import pipeline_volume_tests import volume_snapshotop_tests +import test_utils if __name__ == '__main__': @@ -54,6 +55,7 @@ suite.addTests( unittest.defaultTestLoader.loadTestsFromModule(volume_snapshotop_tests) ) + suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(test_utils)) runner = unittest.TextTestRunner() if not runner.run(suite).wasSuccessful(): From fa5f69aa79e93dd1a5d947c114ff3fb23a6019c6 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Tue, 26 Nov 2019 10:12:22 +0100 Subject: [PATCH 10/16] updates after feedback --- sdk/python/kfp/test_utils.py | 58 ------------------------------ sdk/python/kfp/utils.py | 5 +-- sdk/python/tests/dsl/test_utils.py | 24 ++++++------- 3 files changed, 15 insertions(+), 72 deletions(-) delete mode 100644 sdk/python/kfp/test_utils.py diff --git a/sdk/python/kfp/test_utils.py b/sdk/python/kfp/test_utils.py deleted file mode 100644 index f1daac7e66b..00000000000 --- a/sdk/python/kfp/test_utils.py +++ /dev/null @@ -1,58 +0,0 @@ -from kfp.dsl import ContainerOp -from utils import use_secret -import unittest -import inspect - - -class TestAddSecrets(unittest.TestCase): - - def test_use_default_use_secret(self): - op1 = ContainerOp(name='op1', image='image') - secret_name = "my-secret" - secret_path = "/here/are/my/secret" - op1 = op1.apply(use_secret(secret_name=secret_name, - secret_volume_mount_path=secret_path)) - self.assertEqual(type(op1.container.env), type(None)) - container_dict = op1.container.to_dict() - volume_mounts = container_dict["volume_mounts"][0] - self.assertEqual(type(volume_mounts),dict) - self.assertEqual(volume_mounts["name"],secret_name + '_volume') - self.assertEqual(volume_mounts["mount_path"], secret_path) - - def test_use_set_volume_use_secret(self): - op1 = ContainerOp(name='op1', image='image') - secret_name = "my-secret" - secret_path = "/here/are/my/secret" - volume_name = "my_volume" - op1 = op1.apply(use_secret(secret_name=secret_name, - secret_volume_mount_path=secret_path, - volume_name = volume_name)) - self.assertEqual( type(op1.container.env), type(None)) - container_dict = op1.container.to_dict() - volume_mounts = container_dict["volume_mounts"][0] - self.assertEqual( type(volume_mounts), dict) - self.assertEqual( volume_mounts["name"], volume_name) - self.assertEqual( volume_mounts["mount_path"], secret_path) - - def test_use_set_env_ues_secret(self): - op1 = ContainerOp(name='op1', image='image') - secret_name = "my-secret" - secret_path = "/here/are/my/secret/" - env_variable = "MY_SECRET" - secret_file_path_in_volume = "secret.json" - op1 = op1.apply(use_secret(secret_name=secret_name, - secret_volume_mount_path=secret_path, - env_variable=env_variable, - secret_file_path_in_volume=secret_file_path_in_volume)) - self.assertEqual( len(op1.container.env), 1) - container_dict = op1.container.to_dict() - volume_mounts = container_dict["volume_mounts"][0] - self.assertEqual(type(volume_mounts), dict) - self.assertEqual(volume_mounts["name"], secret_name + '_volume') - self.assertEqual(volume_mounts["mount_path"], secret_path) - env_dict = op1.container.env[0].to_dict() - self.assertEqual(env_dict["name"], env_variable) - self.assertEqual(env_dict["value"], secret_path + secret_file_path_in_volume) - -if __name__ == '__main__': - unittest.main() diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py index ea8453a9f68..bb755150f70 100644 --- a/sdk/python/kfp/utils.py +++ b/sdk/python/kfp/utils.py @@ -22,8 +22,9 @@ def use_secret(secret_name:str, secret_volume_mount_path:str, volume_name:str=No """ if secret_name: - volume_name = volume_name or secret_name + '_volume' - + volume_name = volume_name or secret_name + "_volume" + else: + volume_name = "_volume" if bool(env_variable) != bool(secret_file_path_in_volume): raise ValueError("Both {} and {} needs to be supplied together or not at all".format(env_variable, secret_file_path_in_volume)) diff --git a/sdk/python/tests/dsl/test_utils.py b/sdk/python/tests/dsl/test_utils.py index 80454d66f0c..6b2aee8f959 100644 --- a/sdk/python/tests/dsl/test_utils.py +++ b/sdk/python/tests/dsl/test_utils.py @@ -7,35 +7,35 @@ class TestAddSecrets(unittest.TestCase): def test_use_default_use_secret(self): - op1 = ContainerOp(name='op1', image='image') + op1 = ContainerOp(name="op1", image="image") secret_name = "my-secret" secret_path = "/here/are/my/secret" op1 = op1.apply(use_secret(secret_name=secret_name, - secret_volume_mount_path=secret_path)) + secret_volume_mount_path=secret_path)) self.assertEqual(type(op1.container.env), type(None)) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] - self.assertEqual(type(volume_mounts),dict) - self.assertEqual(volume_mounts["name"],secret_name + '_volume') + self.assertEqual(type(volume_mounts), dict) + self.assertEqual(volume_mounts["name"], secret_name + "_volume") self.assertEqual(volume_mounts["mount_path"], secret_path) def test_use_set_volume_use_secret(self): - op1 = ContainerOp(name='op1', image='image') + op1 = ContainerOp(name="op1", image="image") secret_name = "my-secret" secret_path = "/here/are/my/secret" volume_name = "my_volume" op1 = op1.apply(use_secret(secret_name=secret_name, secret_volume_mount_path=secret_path, - volume_name = volume_name)) - self.assertEqual( type(op1.container.env), type(None)) + volume_name=volume_name)) + self.assertEqual(type(op1.container.env), type(None)) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] - self.assertEqual( type(volume_mounts), dict) - self.assertEqual( volume_mounts["name"], volume_name) - self.assertEqual( volume_mounts["mount_path"], secret_path) + self.assertEqual(type(volume_mounts), dict) + self.assertEqual(volume_mounts["name"], volume_name) + self.assertEqual(volume_mounts["mount_path"], secret_path) def test_use_set_env_ues_secret(self): - op1 = ContainerOp(name='op1', image='image') + op1 = ContainerOp(name="op1", image="image") secret_name = "my-secret" secret_path = "/here/are/my/secret/" env_variable = "MY_SECRET" @@ -48,7 +48,7 @@ def test_use_set_env_ues_secret(self): container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] self.assertEqual(type(volume_mounts), dict) - self.assertEqual(volume_mounts["name"], secret_name + '_volume') + self.assertEqual(volume_mounts["name"], secret_name + "_volume") self.assertEqual(volume_mounts["mount_path"], secret_path) env_dict = op1.container.env[0].to_dict() self.assertEqual(env_dict["name"], env_variable) From 15b80609dc8080deccc5061b14459c40ad78698f Mon Sep 17 00:00:00 2001 From: NikeNano Date: Tue, 26 Nov 2019 10:14:40 +0100 Subject: [PATCH 11/16] added licensce agreement --- sdk/python/kfp/utils.py | 14 ++++++++++++++ sdk/python/tests/dsl/test_utils.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py index bb755150f70..784c7bab389 100644 --- a/sdk/python/kfp/utils.py +++ b/sdk/python/kfp/utils.py @@ -1,3 +1,17 @@ +# Copyright 2019 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + def use_secret(secret_name:str, secret_volume_mount_path:str, volume_name:str=None, env_variable:str=None, secret_file_path_in_volume:str=None): """ An operator that configures the container to use a secret. diff --git a/sdk/python/tests/dsl/test_utils.py b/sdk/python/tests/dsl/test_utils.py index 6b2aee8f959..3ce5b08f13a 100644 --- a/sdk/python/tests/dsl/test_utils.py +++ b/sdk/python/tests/dsl/test_utils.py @@ -1,3 +1,17 @@ +# Copyright 2019 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + from kfp.dsl import ContainerOp from kfp.utils import use_secret import unittest From a7c46d9f26c33925f76f8f3a72923ba6a022eb38 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Tue, 26 Nov 2019 10:16:52 +0100 Subject: [PATCH 12/16] removed space --- sdk/python/tests/dsl/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/tests/dsl/test_utils.py b/sdk/python/tests/dsl/test_utils.py index 3ce5b08f13a..1d3e8313cce 100644 --- a/sdk/python/tests/dsl/test_utils.py +++ b/sdk/python/tests/dsl/test_utils.py @@ -58,7 +58,7 @@ def test_use_set_env_ues_secret(self): secret_volume_mount_path=secret_path, env_variable=env_variable, secret_file_path_in_volume=secret_file_path_in_volume)) - self.assertEqual( len(op1.container.env), 1) + self.assertEqual(len(op1.container.env), 1) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] self.assertEqual(type(volume_mounts), dict) From 68a403ca891fffdb10d34eba643453942cccf329 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Wed, 27 Nov 2019 13:55:52 +0100 Subject: [PATCH 13/16] updated the volume named to be generated --- sdk/python/kfp/utils.py | 14 ++++++++------ sdk/python/tests/dsl/test_utils.py | 9 ++------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py index 784c7bab389..6e123abc9b6 100644 --- a/sdk/python/kfp/utils.py +++ b/sdk/python/kfp/utils.py @@ -12,7 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -def use_secret(secret_name:str, secret_volume_mount_path:str, volume_name:str=None, env_variable:str=None, secret_file_path_in_volume:str=None): +import random +import string + +def use_secret(secret_name:str, secret_volume_mount_path:str, env_variable:str=None, secret_file_path_in_volume:str=None): """ An operator that configures the container to use a secret. @@ -20,7 +23,6 @@ def use_secret(secret_name:str, secret_volume_mount_path:str, volume_name:str=No Keyword Arguments: secret_name {String} -- [Required] The k8s secret name. - volume_name {String} -- The pod volume name. secret_volume_mount_path {String} -- [Required] The path to the secret that is mounted. env_variable {String} -- Env variable pointing to the mounted secret file. Requires both the env_variable and secret_file_path_in_volume to be defined. The value is the path to the secret. @@ -35,10 +37,10 @@ def use_secret(secret_name:str, secret_volume_mount_path:str, volume_name:str=No [ContainerOperator] -- Returns the container operator after it has been modified. """ - if secret_name: - volume_name = volume_name or secret_name + "_volume" - else: - volume_name = "_volume" + volume_name = ''.join(random.choices(string.ascii_lowercase + string.digits, k=10)) + "_volume" + for param, param_name in zip([secret_name, secret_volume_mount_path],["secret_name","secret_volume_mount_path"]): + if param == "": + raise ValueError(f"The '{param_name}' must not be empty") if bool(env_variable) != bool(secret_file_path_in_volume): raise ValueError("Both {} and {} needs to be supplied together or not at all".format(env_variable, secret_file_path_in_volume)) diff --git a/sdk/python/tests/dsl/test_utils.py b/sdk/python/tests/dsl/test_utils.py index 1d3e8313cce..8935c402787 100644 --- a/sdk/python/tests/dsl/test_utils.py +++ b/sdk/python/tests/dsl/test_utils.py @@ -30,25 +30,21 @@ def test_use_default_use_secret(self): container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] self.assertEqual(type(volume_mounts), dict) - self.assertEqual(volume_mounts["name"], secret_name + "_volume") self.assertEqual(volume_mounts["mount_path"], secret_path) def test_use_set_volume_use_secret(self): op1 = ContainerOp(name="op1", image="image") secret_name = "my-secret" secret_path = "/here/are/my/secret" - volume_name = "my_volume" op1 = op1.apply(use_secret(secret_name=secret_name, - secret_volume_mount_path=secret_path, - volume_name=volume_name)) + secret_volume_mount_path=secret_path)) self.assertEqual(type(op1.container.env), type(None)) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] self.assertEqual(type(volume_mounts), dict) - self.assertEqual(volume_mounts["name"], volume_name) self.assertEqual(volume_mounts["mount_path"], secret_path) - def test_use_set_env_ues_secret(self): + def test_use_set_env_use_secret(self): op1 = ContainerOp(name="op1", image="image") secret_name = "my-secret" secret_path = "/here/are/my/secret/" @@ -62,7 +58,6 @@ def test_use_set_env_ues_secret(self): container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] self.assertEqual(type(volume_mounts), dict) - self.assertEqual(volume_mounts["name"], secret_name + "_volume") self.assertEqual(volume_mounts["mount_path"], secret_path) env_dict = op1.container.env[0].to_dict() self.assertEqual(env_dict["name"], env_variable) From 61d17c4e2433a294fb6bfb5ea6b4200aab86b211 Mon Sep 17 00:00:00 2001 From: NikeNano Date: Thu, 28 Nov 2019 11:13:52 +0100 Subject: [PATCH 14/16] secret_name as volume name and updated test --- sdk/python/kfp/utils.py | 10 +++++++--- sdk/python/tests/dsl/test_utils.py | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/utils.py index 6e123abc9b6..f4294f4f8ad 100644 --- a/sdk/python/kfp/utils.py +++ b/sdk/python/kfp/utils.py @@ -37,10 +37,14 @@ def use_secret(secret_name:str, secret_volume_mount_path:str, env_variable:str=N [ContainerOperator] -- Returns the container operator after it has been modified. """ - volume_name = ''.join(random.choices(string.ascii_lowercase + string.digits, k=10)) + "_volume" - for param, param_name in zip([secret_name, secret_volume_mount_path],["secret_name","secret_volume_mount_path"]): + secret_name = str(secret_name) + if '{{' in secret_name: + volume_name = ''.join(random.choices(string.ascii_lowercase + string.digits, k=10)) + "_volume" + else: + volume_name = secret_name + for param, param_name in zip([secret_name, secret_volume_mount_path],["secret_name","secret_volume_mount_path"]): if param == "": - raise ValueError(f"The '{param_name}' must not be empty") + raise ValueError("The '{}' must not be empty".format(param_name)) if bool(env_variable) != bool(secret_file_path_in_volume): raise ValueError("Both {} and {} needs to be supplied together or not at all".format(env_variable, secret_file_path_in_volume)) diff --git a/sdk/python/tests/dsl/test_utils.py b/sdk/python/tests/dsl/test_utils.py index 8935c402787..7c0268ea6dd 100644 --- a/sdk/python/tests/dsl/test_utils.py +++ b/sdk/python/tests/dsl/test_utils.py @@ -29,6 +29,7 @@ def test_use_default_use_secret(self): self.assertEqual(type(op1.container.env), type(None)) container_dict = op1.container.to_dict() volume_mounts = container_dict["volume_mounts"][0] + self.assertEqual(volume_mounts["name"], secret_name) self.assertEqual(type(volume_mounts), dict) self.assertEqual(volume_mounts["mount_path"], secret_path) From 31b072aeae541b3da0d23f1affc96fad2ecff4ea Mon Sep 17 00:00:00 2001 From: NikeNano Date: Tue, 3 Dec 2019 15:18:03 +0100 Subject: [PATCH 15/16] updated the file structure --- sdk/python/kfp/dsl/extensions/__init__.py | 0 sdk/python/kfp/{utils.py => dsl/extensions/kubernetes.py} | 0 sdk/python/tests/dsl/extensions/__init__.py | 0 .../tests/dsl/{test_utils.py => extensions/test_kubernetes.py} | 2 +- 4 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 sdk/python/kfp/dsl/extensions/__init__.py rename sdk/python/kfp/{utils.py => dsl/extensions/kubernetes.py} (100%) create mode 100644 sdk/python/tests/dsl/extensions/__init__.py rename sdk/python/tests/dsl/{test_utils.py => extensions/test_kubernetes.py} (98%) diff --git a/sdk/python/kfp/dsl/extensions/__init__.py b/sdk/python/kfp/dsl/extensions/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/sdk/python/kfp/utils.py b/sdk/python/kfp/dsl/extensions/kubernetes.py similarity index 100% rename from sdk/python/kfp/utils.py rename to sdk/python/kfp/dsl/extensions/kubernetes.py diff --git a/sdk/python/tests/dsl/extensions/__init__.py b/sdk/python/tests/dsl/extensions/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/sdk/python/tests/dsl/test_utils.py b/sdk/python/tests/dsl/extensions/test_kubernetes.py similarity index 98% rename from sdk/python/tests/dsl/test_utils.py rename to sdk/python/tests/dsl/extensions/test_kubernetes.py index 7c0268ea6dd..af929203cdf 100644 --- a/sdk/python/tests/dsl/test_utils.py +++ b/sdk/python/tests/dsl/extensions/test_kubernetes.py @@ -13,7 +13,7 @@ # limitations under the License. from kfp.dsl import ContainerOp -from kfp.utils import use_secret +from kfp.dsl.extensions.kubernetes import use_secret import unittest import inspect From d72a18aa577c9d6d3fc9f671ff9edc2ad95a646e Mon Sep 17 00:00:00 2001 From: NikeNano Date: Tue, 3 Dec 2019 17:04:17 +0100 Subject: [PATCH 16/16] fixed build --- sdk/python/setup.py | 1 + sdk/python/tests/dsl/main.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/python/setup.py b/sdk/python/setup.py index 46f51e5c08c..2fc67a01bef 100644 --- a/sdk/python/setup.py +++ b/sdk/python/setup.py @@ -70,6 +70,7 @@ def find_version(*file_path_parts): 'kfp.components.structures.kubernetes', 'kfp.containers', 'kfp.dsl', + 'kfp.dsl.extensions', 'kfp.notebook', ], classifiers=[ diff --git a/sdk/python/tests/dsl/main.py b/sdk/python/tests/dsl/main.py index 1675564bffe..7296d72bfd1 100644 --- a/sdk/python/tests/dsl/main.py +++ b/sdk/python/tests/dsl/main.py @@ -29,7 +29,7 @@ import volume_op_tests import pipeline_volume_tests import volume_snapshotop_tests -import test_utils +import extensions.test_kubernetes as test_kubernetes if __name__ == '__main__': @@ -55,7 +55,7 @@ suite.addTests( unittest.defaultTestLoader.loadTestsFromModule(volume_snapshotop_tests) ) - suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(test_utils)) + suite.addTests(unittest.defaultTestLoader.loadTestsFromModule(test_kubernetes)) runner = unittest.TextTestRunner() if not runner.run(suite).wasSuccessful():