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

refactor: enhance clarity by changing 'parse_secret_string_as_json' option to 'json #2179

Merged
merged 2 commits into from
Dec 18, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,27 @@ def get_secret_as_dict(self, secret_id, options={}, role=None):
The secret payload from AWS is EITHER a string OR a binary blob.

If the secret contains a string payload ("SecretString"):
- if the `parse_secret_string_as_json` option is True (default):
- if the `json` option is True (default):
{SecretString} will be parsed as a JSON. If successfully parsed, AND the JSON contains a
top-level object, each entry K/V in the object will also be converted to an entry in the result. V will
always be casted to a string (if not already a string).
- If `parse_secret_string_as_json` option is False:
{SecretString} will be returned as a single entry in the result, with the key being the secret_id.
- If `json` option is False:
Copy link
Collaborator

@saikonen saikonen Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we're updating the docstring to match behaviour, could you also add for the json=False case that the returned secret key is either the secret_id OR the value set by options={"env_var_name": custom_env_var_name}. I leave the phrasing up to you :)

see L149:153 for that code path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @saikonen Now i think the docstring is aligned with the actual behavior and provides a clear explanation of the json=False case.

{SecretString} will be returned as a single entry in the result, where the key is either:
- the `secret_id`, OR
- the value set by `options={"env_var_name": custom_env_var_name}`.

Otherwise, the secret contains a binary blob payload ("SecretBinary"). In this case
- The result dic contains '{SecretName}': '{SecretBinary}', where {SecretBinary} is a base64-encoded string
Otherwise, if the secret contains a binary blob payload ("SecretBinary"):
- The result dict contains '{SecretName}': '{SecretBinary}', where {SecretBinary} is a base64-encoded string.

All keys in the result are sanitized to be more valid environment variable names. This is done on a best effort
All keys in the result are sanitized to be more valid environment variable names. This is done on a best-effort
basis. Further validation is expected to be done by the invoking @secrets decorator itself.

:param secret_id: ARN or friendly name of the secret
:param options: unused
:param role: AWS IAM Role ARN to assume before reading the secret
:return: dict of environment variables. All keys and values are strings.
:param secret_id: ARN or friendly name of the secret.
:param options: Dictionary of additional options. E.g., `options={"env_var_name": custom_env_var_name}`.
:param role: AWS IAM Role ARN to assume before reading the secret.
:return: Dictionary of environment variables. All keys and values are strings.
"""

import botocore
from metaflow.plugins.aws.aws_client import get_aws_client

Expand Down
Loading