pipelines
pipelines copied to clipboard
WIP: fix(Backend+SDK): Update kubernetes.use_secret_as_volume to accept secret name dynamically at runtime
Description of your changes: Resolves https://github.com/kubeflow/pipelines/issues/10914
Attempting to pass the secret_name parameter in the dsl compile this way:
from kfp import dsl
from kfp import kubernetes
@dsl.component(base_image="python:3.10")
def print_secret():
with open('/mnt/my_vol') as f:
print(f.read())
@dsl.pipeline
def pipeline(secret_name: str = 'my-secret'):
task = print_secret()
kubernetes.use_secret_as_volume(task,
secret_name=secret_name,
mount_path='/mnt/my_vol')
results in this error:
File "/home/ddalvi/PycharmProjects/pythonProject2/main.py", line 23, in pipeline
kubernetes.use_secret_as_volume(task,
File "/home/ddalvi/miniconda3/envs/kfp_pipe_secret_env/lib/python3.11/site-packages/kfp/kubernetes/secret.py", line 81, in use_secret_as_volume
secret_as_vol = pb.SecretAsVolume(
^^^^^^^^^^^^^^^^^^
TypeError: bad argument type for built-in operation
The secret_name parameter was originally expected to be a string (str). However, in this case, it was being provided as a an object of the PipelineParameterChannel class , leading to a type mismatch error when the code tried to use it as a string.
Updated secret_name to accept either a str or PipelineParameterChannel using Union[str, PipelineParameterChannel] and added a check to convert PipelineParameterChannel to its string representation.
Furthermore,
- Implemented support for dynamically specifying secret names in the
use_secret_as_volumefunction. - Updated driver code to handle secret name substitution at runtime based on input parameters.
- Introduced a
{{secret_name}}template string representation for secret names in the compiled DSL. - Added a test to validate secret name template creation in IR.
Refer to this comment to understand why the driver code change is required.
Please refer to a PoC wherein we've listed out different test cases - four different pairs of DSL compile code and resulting pipeline yaml IRs with which we tested this update.
Collaborated with @gregsheremeta for this contribution, it was a joint effort. Thanks Greg for your contributions! :)
Checklist:
- [x] The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.
/lgtm
I also verified it, so /verified :)
@chensun mind taking a quick look?
Please create a test for this scenario
good call
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: amadhusu Once this PR has been reviewed and has the lgtm label, please assign chensun for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@chensun could you please take a look and let us know if this templating method looks good to you? Thanks!
/rerun-all
/lgtm
@chensun could you please take a look? Thanks!
New changes are detected. LGTM label has been removed.
/rerun-all
@chensun could you please take a look and let us know if this templating method looks good to you? Thanks!
@chensun bumping :)
cc: @gregsheremeta @rimolive
I've tested the implementation and it works as intended, however I think it's worth getting input from @chensun on the approach, I've left a comment here as well.
closing in lieu of https://github.com/kubeflow/pipelines/pull/11621