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

object_store 0.9.1 breaks Azure Storage Emulator Config #5475

Closed
aersam opened this issue Mar 6, 2024 · 5 comments
Closed

object_store 0.9.1 breaks Azure Storage Emulator Config #5475

aersam opened this issue Mar 6, 2024 · 5 comments
Labels
question Further information is requested

Comments

@aersam
Copy link

aersam commented Mar 6, 2024

delta-rs is using this config to connect to Azure Storage Emulator:

dict(
        AZURE_STORAGE_ACCOUNT_NAME="devstoreaccount1",
        AZURE_STORAGE_ACCOUNT_KEY="Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==",
        AZURE_STORAGE_CONTAINER_NAME="deltars",
        AZURE_STORAGE_USE_EMULATOR="true",
        AZURE_STORAGE_USE_HTTP="true",
    )

this is passed via python, but it basically just sets the configs in object_store.

In 0.9.1, this started to cause an authentication error:

E       OSError: Generic MicrosoftAzure error: Client error with status 403 Forbidden: <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
E       <Error>
E         <Code>AuthorizationFailure</Code>
E         <Message>Server failed to authenticate the request. Make sure the value of the Authorization header is formed correctly including the signature.
E       RequestId:44a1c5ff-8a6b-491a-a890-e07d0d823e87
E       Time:2024-03-05T11:31:55.140Z</Message>
E       </Error>

Can you check this?

Since there was a change in Azure Authentication in object_store 0.9.1, is there some different config required? I was assuming a bug since it's only a minor version increase and the config looks good

@tustvold
Copy link
Contributor

tustvold commented Mar 6, 2024

This is likely related to #5425

@andrebsguedes could you take a look please

Edit: is it possible you have other credentials configured, e.g. in the environment? Previously these would have been being ignored

Edit edit: the config we use for integration testing against azurite can be found here - https://github.com/apache/arrow-rs/blob/master/.github%2Fworkflows%2Fobject_store.yml#L106

@andrebsguedes
Copy link
Contributor

@aersam The current version of object_store respects other kind of credentials that were being ignored before. So if your test code is passing a dummy SAS token or bearer token it will fail to authenticate.

I saw this kind of pattern on your repo, could that be an example of this?

@ion-elgreco
Copy link

ion-elgreco commented Mar 21, 2024

@andrebsguedes @tustvold the test fails in our python integration tests. I checked but we don't have any env variables set for azure while this test is being executed. Also it shouldn't be a dummy SAS token, since we create one by calling azurite:

    endpoint_url = (
        f"http://localhost:10000/{azurite_creds['AZURE_STORAGE_ACCOUNT_NAME']}"
    )
    env = os.environ.copy()
    env.update(azurite_creds)
    env["AZURE_STORAGE_CONNECTION_STRING"] = (
        "DefaultEndpointsProtocol=http;"
        f"AccountName={azurite_creds['AZURE_STORAGE_ACCOUNT_NAME']};"
        f"AccountKey={azurite_creds['AZURE_STORAGE_ACCOUNT_KEY']};"
        f"BlobEndpoint={endpoint_url};"
    )
    output = subprocess.run(
        [
            "az",
            "storage",
            "container",
            "generate-sas",
            "--name",
            azurite_creds["AZURE_STORAGE_CONTAINER_NAME"],
            "--permissions",
            "dlrw",
        ],
        env=env,
        capture_output=True,
    )

    creds = {key: value for key, value in azurite_creds.items() if "KEY" not in key}
    creds["SAS_TOKEN"] = output.stdout.decode()

@giacomorebecchi
Copy link

Hi @ion-elgreco, I deep-dived into this as I'm looking to upgrade my deltalake dep in python, but some integration tests using azurite with code-generated SAS tokens started failing.

Basically, in object-store==0.9.0, when use_emulator="true", an account key is always passed, and the SAS token is ignored:

let (is_emulator, storage_url, auth, account) = if self.use_emulator.get()? {
let account_name = self
.account_name
.unwrap_or_else(|| EMULATOR_ACCOUNT.to_string());
// Allow overriding defaults. Values taken from
// from https://docs.rs/azure_storage/0.2.0/src/azure_storage/core/clients/storage_account_client.rs.html#129-141
let url = url_from_env("AZURITE_BLOB_STORAGE_URL", "http://127.0.0.1:10000")?;
let key = match self.access_key {
Some(k) => AzureAccessKey::try_new(&k)?,
None => AzureAccessKey::try_new(EMULATOR_ACCOUNT_KEY)?,
};

whereas, in object-store==0.9.1, the actual SAS token is passed:
let (is_emulator, storage_url, auth, account) = if self.use_emulator.get()? {
let account_name = self
.account_name
.unwrap_or_else(|| EMULATOR_ACCOUNT.to_string());
// Allow overriding defaults. Values taken from
// from https://docs.rs/azure_storage/0.2.0/src/azure_storage/core/clients/storage_account_client.rs.html#129-141
let url = url_from_env("AZURITE_BLOB_STORAGE_URL", "http://127.0.0.1:10000")?;
let credential = if let Some(k) = self.access_key {
AzureCredential::AccessKey(AzureAccessKey::try_new(&k)?)
} else if let Some(bearer_token) = self.bearer_token {
AzureCredential::BearerToken(bearer_token)
} else if let Some(query_pairs) = self.sas_query_pairs {
AzureCredential::SASToken(query_pairs)
} else if let Some(sas) = self.sas_key {
AzureCredential::SASToken(split_sas(&sas)?)
} else {
AzureCredential::AccessKey(AzureAccessKey::try_new(EMULATOR_ACCOUNT_KEY)?)
};
self.client_options = self.client_options.with_allow_http(true);
(true, url, static_creds(credential), account_name)

Thus, I suspect that the issue you are encountering is rather linked to this issue:

I'll try to implement the fix to create a valid SAS token in my repo, and, if it works, I will try to open a PR also on delta-rs!

@ion-elgreco
Copy link

@giacomorebecchi Cool! Just ping me when you push a PR in Delta-RS 😄

@tustvold tustvold added question Further information is requested and removed bug labels May 29, 2024
ion-elgreco pushed a commit to delta-io/delta-rs that referenced this issue Jun 14, 2024
# Description
Reintroduce integration tests using Azurite and SAS tokens.

These tests were skipped believing that the mistake was due to the
upgrade of version in `object-store>=0.9.1". Actually, what
`object-store` started doing from that version on, is really using the
SAS token for the first time. For more details, see:
- apache/arrow-rs#5475 (comment)

I changed the command to create the SAS token using the
`azure-storage-blob` library (adding it to the dev deps), rather than
the azure CLI, and removed the pytest markers to skip the tests.

@ion-elgreco pinging you as promised, sorry if this took more time than
expected!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants