Fix the Get Secret Bug from OS Environment Variable

[Future-Outlier]

Why are the changes needed?

In the current design of flytekit, we have two ways to retrieve secrets:

1. Get the secret from the OS's environment variables.
2. Get the secret from a specified file path. (Usually use kubectl to create secret value)

If you choose the first method, you will encounter an issue: the retrieved secret value needs to have whitespace characters removed.

This update aims to fix errors that occur when using flytekit.current_context().secrets.get along with the first method above, especially in Single Binary Mode.

What changes were proposed in this pull request?

Call strip method to remove whitespace characters in context manager get secret function.
Note: The type of variable from os environment is python pickle object, maybe that's why we have this error.
I've tried to return type of the secret value.
Use code like below.

token = flytekit.current_context().secrets.get
return type(token)

{
   "o0":{
      "type":"single (PythonPickle) blob",
      "uri":"s3://my-s3-bucket/data/81/f4d0ab375012340e7942-fjfcm5iy-0/004422d2313077a075ec20a2e765bcb9/e5d6226b55aa2670cc70e14f7ec76bda"
   }
}

How was this patch tested?

Setup process

0. Start flyte cluster using latest single binary mode.
1. Add a secret value in your local cluster.
   
2. Use pyflyte run remote to run the task.
3. Copy the Output value to clipboard.
   

Use Default Flytekit Image

pyflyte run --remote secret.py github_secret_task

Return Value

{"o0":"abc_jDDmlkadf8ASSD8DSDA5F66ds6saADSFFdas\n"}

Use Flytekit Image Built by this branch

pyflyte run --remote --image futureoutlier/flytekit-secret:0145 secret.py github_secret_task

Return Value

{"o0":"abc_jDDmlkadf8ASSD8DSDA5F66ds6saADSFFdas"}

Dockerfile (Use this branch)

FROM python:3.9-slim-buster
USER root
WORKDIR /root
ENV PYTHONPATH /root
RUN apt-get update && apt-get install build-essential -y
RUN apt-get install git -y

RUN pip install -U git+https://github.com/Future-Outlier/flytekit.git@6d194c6a18754c6b8d825d0bf3d347db97be56d2

Python File

import flytekit
from flytekit import Secret, task

@task(secret_requests=[Secret(key="token", group="github-api")])
def github_secret_task() -> str:
    return flytekit.current_context().secrets.get("github-api", "token")

Screenshots

Use Default Flytekit Image

This image shows the output from the default Flytekit image, which includes a trailing blank space at the end of the output string.

Use Built Image By This branch

This image displays the output using the new Flytekit image from this branch, where the trailing blank space at the end of the output string is removed.

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e107994) 85.71% compared to head (6d194c6) 81.04%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2077      +/-   ##
==========================================
- Coverage   85.71%   81.04%   -4.68%     
==========================================
  Files         313      176     -137     
  Lines       23396    17001    -6395     
  Branches     3501     3501              
==========================================
- Hits        20055    13779    -6276     
+ Misses       2733     2643      -90     
+ Partials      608      579      -29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[wild-endeavor]

I think this is a low-risk change so let's go ahead, but i don't understand what's happening. why is this happening now? why hasn't this come up before? it could be an issue in the secrets mounting bit? when you get the secret using kubectl, are we sure it doesn't have the newline?

[Future-Outlier]

I think this is a low-risk change so let's go ahead, but i don't understand what's happening. why is this happening now? why hasn't this come up before? it could be an issue in the secrets mounting bit? when you get the secret using kubectl, are we sure it doesn't have the newline?

I have tested the scenario in the single binary mode using kubectl, it works well, I think the reason why it won't have the error is that the code here (second option) will strip the whitespace value before returning the secret value.
https://github.com/flyteorg/flytekit/blob/master/flytekit/core/context_manager.py#L371

I think maybe we have encountered this issue before, that's why we will remove whitespace when using kubectl or other mounting mechanism.