Feature Minio S3 Enhancement

[SantoshDeepak1]

Feature Minio S3 committed

Description

This pull request (PR) introduces the integration of Minio's S3 capabilities with Eclipse EDC, enabling the transfer of assets from a source S3 provider to a target S3 consumer.

Closes #34

[SantoshDeepak1]

Current Issue

Currently, after a successful transfer of assets from the source to the target, an exception is being logged in the source dataplane.

Failed to send callback request
org.eclipse.edc.spi.http.EdcHttpClientException: Server response to Request{method=POST, url=http://localhost:8083/control/transferprocess/18d14e52-7796-4e01-add3-7f25802352d5/complete} was not one of [200, 204] but was 405: 

I attempted to override callback address to the transfer payload as described ("MXD Management API_S3_MINIO.json"), but it seems to have no effect, as the system is still using a default callback address.

[paullatzelsperger]

the minio deployment should be a terraform module rather than a static config that contains hardocded bob and alice. Much like the connector module is done.

It could even be a module, that is used in turn by the connector module

[SantoshDeepak1]

Hello @paullatzelsperger , could you provide more details regarding your comment?
Should the deployment of Minio be structured as a Terraform module, similar to how the connector module has been implemented?
In the current setup, there is a single Minio server where two distinct buckets are created, one for Alice and one for Bob. Are you suggesting that we should have separate Minio servers for each connector, with each server hosting assets for different participants, such as Alice and Bob?

[paullatzelsperger]

@SantoshDeepak1 Like I said: for every connector there should be a separate MinIO deployment (~server) that hosts the bucket(s). This should be implemented as terraform module, so that the same module declaration can be used for bob and alice. please use appropriate variables to distinguish them.

Are you suggesting that we should have separate Minio servers for each connector, with each server hosting assets for different participants, such as Alice and Bob?

this is what i'm suggesting.

BTW there are even dedicated MinIO providers for terraform (check here or here), that might be easier than a raw K8S deployment.

[SantoshDeepak1]

Yes, @paullatzelsperger, opting for MinIO providers makes the process more straightforward. However, in issue #34, you suggested utilizing kubernetes_deployment. Currently, we are shifting our strategy to incorporate terraform providers.

[paullatzelsperger]

Yes, @paullatzelsperger, opting for MinIO providers makes the process more straightforward. However, in issue #34, you suggested utilizing kubernetes_deployment. Currently, we are shifting our strategy to incorporate terraform providers.

either one is fine i think, so long as it is re-usable

[SantoshDeepak1]

Hello @paullatzelsperger, I've implemented the code based on your recommendations. Could you please review it and let me know if I'm heading in the correct direction?

[SantoshDeepak1]

Hello @paullatzelsperger ,

I have incorporated all your suggestions; please review the changes. According to @hemantxpatel , there is no requirement to include accessKeyId, secretAccessKey, and endpointOverride in the payload . Based on my observations, these parameters are not necessary when creating assets on the provider side. However, these parameters become necessary when a consumer requests to transfer assets from the provider; otherwise, it leads to a NoSuchBucketException. Kindly examine the code and advise if this behavior is intentional or if adjustments are needed in the payload.

While creating asset from the provider the payload is

{
    "@context": {},
    "asset": {
        "@type": "Asset",
        "@id": "3",
        "properties": {
            "description": "Product EDC Demo Asset 3- S3"
        }
    },
    "properties": {
        "version": "1.0",
        "type": "AmazonS3",
        "name": "document_alice",
        "id": "document_alice",
        "contenttype": "text/plain"
    },
    "dataAddress": {
        "@type": "DataAddress",
        "type": "AmazonS3",
        "blobname": "document.txt",
        "region":"us-east-1",
        "bucketName": "alice-bucket",
        "keyName":"document.txt"
    }
}

While transfer request from consumer the payload is

{
    "@context": {
       "@vocab": "https://w3id.org/edc/v0.0.1/ns/"
    },
    "assetId": "3",
    "connectorAddress": "{{ALICE_PROVIDER_PROTOCOL_DSP}}",
    "connectorId": "BPNL000000000002",
    "contractId": "37e5556e-c43c-4d7a-941e-21d40284af0f",
    "dataDestination": {
        "type": "AmazonS3",
        "keyName":"document.txt",
        "bucketName": "bob-bucket",
        "region":"us-east-1",
        "endpointOverride":"http://10.244.0.38:9000",
        "accessKeyId":"qwerty123",
        "secretAccessKey":"qwerty123"
    },
    "protocol": "dataspace-protocol-http"
}

[paullatzelsperger]

@SantoshDeepak1 Please take a look at the EDC Samples to see what data must be included when creating assets, and when requesting a transfer. The sample does create assets on Azure Blob, but AWS S3 works analogously

[SantoshDeepak1]

Hi @paullatzelsperger,

Thanks for sharing the resource. While examining the transfer payload in the sample, I noticed that it doesn't explicitly mention the accessKeyId and secretAccessKey. During the creation of the connector using the helm chart, I override the following section, and the provider utilizes these values when pulling from the S3 store. However, the transfer consumer encounters difficulty storing these values in their own S3 store without explicitly specifying accessKeyId and secretAccessKey. The example you provided uses the AWS CLI and stores them in a vault. Therefore, I'm seeking a method to transmit these secrets to the connector via a vault.

In the connector module main.tf file line no 65

dataplane: {
  aws: {
    endpointOverride: "http://${var.minio-config.minio-url}",
    accessKeyId: var.minio-config.minio-username,
    secretAccessKey: var.minio-config.minio-password
   }
}

[SantoshDeepak1]

Hello @paullatzelsperger ,

I've made an attempt using the examples provided in the resources you shared. However, our situation involves Minio being recreated after each Terraform destroy and apply cycle, posing challenges in setting a temporary session token or access key in the vault. This issue results in the following exception:

software.amazon.awssdk.services.s3.model.S3Exception: The Access Key Id you provided does not exist in our records

Minio doesn't support AWS temporary session tokens. There are discussions and issues raised regarding this limitation:

GitHub Issue 2444

The workaround mentioned in the ticket involves editing the mc configuration, but attempts to do so resulted in a "permission denied" error due to it being a read-only file.

Can you please suggest.

[gitguardian]

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

---

<sup><sub>🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!</sub></sup>

[hemantxpatel]

I have refactored the code. Some of the major changes includes:

• Instead of creating minIO separately for Alice and Bob, connector module uses minIO module to deploy a MinIO instance for itself.
• Moved tutorial from main README.md to a file under docs folder.
• Current tutorial requires actual AWS access key / secret during initiate transfer due to MinIO limitations. I have raised an issue Enhance S3 to S3 Transfer Tutorial using AWS Temp Credentials (AWS STS Token) #169 to fix it in case we have some workaround or MinIO adds support.

[hemantxpatel]

I messed up this branch as I was trying to create a branch with both azurite and S3 changes. Will open a new PR.