Initial Problem

The initial Slack report is that AzureResult has a code snippet like this (link to code here):

class AzureResult(Result):
    def __init__(
        self,
        container: str,
        connection_string: str = None,
        connection_string_secret: str = None,
        **kwargs: Any
    ) -> None:
        self.container = container
        self.connection_string = connection_string or os.getenv(
            "AZURE_STORAGE_CONNECTION_STRING"
        )
        self.connection_string_secret = connection_string_secret
        super().__init__(**kwargs)

So the __init__ method is actually loading in an environment variable when a result is defined, when the Flow is serialized, this environment variable is actually stored leading into issues (and security concerns) when accessing the Result during Flow execution.

Initial Look

Does it actually get serialized? After digging with @peytonrunyan, it seems that it actually does, but we have to explain a bit where. First, doing

@task(result=AzureResult(...))
def abc():
    return 1

does not actually include the result when doing flow.serialize() so we are safe when it's defined on the task level. We are more concerned with it defined on the flow level.

On the flow level, doing

with Flow(..., result = AzureResult(...)) as flow:

also is not part of the serialized Flow. This can be confirmed by looking at the FlowSchema here. Results are indeed not serialized.

But Then!

However, the initial Slack report by @zyzil mentions that AzureStorage is using the AzureResult here. There is code that looks like:

class Azure(Storage):
    def __init__(
        self,
        ...
        **kwargs: Any
    ) -> None:
       .....
        self.connection_string_secret = connection_string_secret
        result = AzureResult(
            connection_string_secret=self.connection_string_secret,
            container=container,
        )
        super().__init__(result=result, stored_as_script=stored_as_script, **kwargs)

Notice that AzureResult is attached to the Storage object. When AzureResult is created, aconnection_string_secret is passed and connection_string is left None. Because it is None, the environment variable is pulled in the Result, and then the Result is added to storage.

Thoughts on Fixes

The least invasive non-breaking change should just be editing the AzureResult class in the __init__ method to avoid the environment variable pulling here and then defer it somewhere inside the initialize_service method. On first glance, this should be non-breaking and will be more consistent with S3 and GCS.

It seems a bit weird we even accept a connection_string that is sensitive on the __init__. We might want to remove that, but @peytonrunyan correctly points out it would be a breaking change so we should just leave it as is.