Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kubernetes_platform] Error calling kubernetes.use_secret_as_env and use_secret_as_volume with secret name as parameter #10914

Open
revit13 opened this issue Jun 17, 2024 · 6 comments · May be fixed by #11039 or #11121
Assignees
Milestone

Comments

@revit13
Copy link
Contributor

revit13 commented Jun 17, 2024

We get the following error running the program from the example with the change of passing the secret name as a parameter:

e-packages/kfp/kubernetes/secret.py", line 48, in use_secret_as_env
    secret_as_env = pb.SecretAsEnv(
TypeError: bad argument type for built-in operation

Environment

  • KFP version:
    2.2.0

Steps to reproduce

run the following example:

from kfp import dsl
from kfp import kubernetes

@dsl.component(base_image="python:3.10")
def print_secret():
    import os
    print(os.environ['my-secret'])

@dsl.pipeline
def pipeline(secret_name: str = 'my-secret'):
    task = print_secret()
    kubernetes.use_secret_as_env(task,
                                 secret_name=secret_name,
                                 secret_key_to_env={'password': 'SECRET_VAR'})

It also happens when running the following program:

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')

Expected result

not produce an error

Materials and Reference


Impacted by this bug? Give it a 👍.

@revit13 revit13 changed the title [kubernetes_platform] Error calling kubernetes.use_secret_as_env with secret name as parameter [kubernetes_platform] Error calling kubernetes.use_secret_as_env and use_secret_as_volume with secret name as parameter Jun 17, 2024
@roytman
Copy link
Contributor

roytman commented Jun 19, 2024

As a workaround, we add hard-coded names of CM/Secrets in pipelines, so moving to a new CM/Secret requires updating/compilation and deployment of a pipeline.

@DharmitD
Copy link
Contributor

/assign @DharmitD

@DharmitD
Copy link
Contributor

DharmitD commented Jul 16, 2024

@diegolovison and I tested out the first example with kfp sdk version 2.2.0 and kfp-kubernetes version 1.0.0. We noticed that the environment variable setting is incorrect in the example, find details here. With that correction, we were able to successfully run the first example and spin up a Pipeline and a run.

Second scenario is the one that is failing, this is the error observed on attempting to complle the second example:
TypeError: Cannot set kfp_kubernetes.SecretAsVolume.secret_name to {{channel:task=;name=secret_name;type=String;}}: {{channel:task=;name=secret_name;type=String;}} has type <class 'kfp.dsl.pipeline_channel.PipelineParameterChannel'>, but expected one of: (<class 'bytes'>, <class 'str'>) for field SecretAsVolume.secret_name

Will investigate this error further and debug it.
cc: @revit13 @roytman

gregsheremeta added a commit to gregsheremeta/data-science-pipelines that referenced this issue Aug 11, 2024
proof of concept for supporting parameter inputs in kfp-kubernetes.
We've added support first to secret.py.

Ref: kubeflow#10534
Ref: kubeflow#10914
@DharmitD
Copy link
Contributor

DharmitD commented Aug 12, 2024

Created a solution in this PR to update use_secret_as_volume function to have the secret name passed in as a parameter, but we noticed that the resulting IR yaml created isn't grabbing the secret name as we expect it to do.

@gregsheremeta and I further investigated and realized that the attempt to pass in a secret name as a parameter must be to have the secret name dynamically specified at runtime, and not just have it hardcoded in the DSL compile every time. This would require an additional backend driver code change, we've implemented the driver + SDK code change in a PoC PR. In this PoC, we have defined a string representation for secret name - {{my-secret}} that gets defined in the compiler DSL whenever use_secret_as_volume function is called with secret name specified as a parameter. The resulting IR yaml generated out of this DSL compile has this string representation / template value for secret name:

platforms:
  kubernetes:
    deploymentSpec:
      executors:
        exec-comp:
          secretAsVolume:
          - mountPath: /mnt/my_secret
            optional: false
            secretName: '{{my_secret}}'

This {{my_secret}} string representation gets substituted with the actual secret name when specified at runtime.

We've covered multiple test cases on how secret name could be specified in the use_secret_as_volume function during the DSL compile step, and how it could also be updated at runtime. This is a WIP, we're planning to add some error handling and add these test cases to the existing KFP kubernetes test suite.

We will present this PoC in the upcoming KFP community meeting.
cc: @revit13 @roytman @chensun

@gregsheremeta
Copy link
Contributor

I think we've also partially implemented #10534. Note that @Tomcli suggests

{{tasks.taskname.param_name}} for referring task parameters and {{loops.inputs.param_name}} for loop parameters

so we might want to go from {{param_name}} to {{tasks.taskname.param_name}} so that we can include loop parameters in this solution, as well as any other runtime scoped sets of parameters that we may not yet be aware of.

@gregsheremeta
Copy link
Contributor

We talked about this in the KFP community call today, and @chensun suggested an alternative approach to using templating. He's going to post an example here (thanks Chen!)

@HumairAK HumairAK added this to the KFP 2.4.0 milestone Oct 9, 2024
cmadam added a commit to IBM/data-prep-kit that referenced this issue Nov 14, 2024
Signed-off-by: Constantin M Adam <[email protected]>
cmadam added a commit to IBM/data-prep-kit that referenced this issue Nov 14, 2024
Signed-off-by: Constantin M Adam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment