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

Artifacts cannot be stored in object-lock enabled S3 buckets #11182

Open
3 tasks done
akloss-cibo opened this issue Jun 5, 2023 · 10 comments · May be fixed by #14140
Open
3 tasks done

Artifacts cannot be stored in object-lock enabled S3 buckets #11182

akloss-cibo opened this issue Jun 5, 2023 · 10 comments · May be fixed by #14140
Labels
area/artifacts S3/GCP/OSS/Git/HDFS etc P3 Low priority type/bug

Comments

@akloss-cibo
Copy link

akloss-cibo commented Jun 5, 2023

Pre-requisites

  • I have double-checked my configuration
  • I can confirm the issues exists when I tested with :latest
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what you expected to happen?

Artifacts cannot be stored into AWS S3 buckets with Object Lock enabled.

I believe this may be fixed by minio/minio#15433

Version

3.4.7 and latest (and probably others)

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  annotations:
    iam.amazonaws.com/role: AWSIAMROLENAME
    workflows.argoproj.io/pod-name-format: v1
  name: wf-artifact-test
spec:
  entrypoint: wf-artifact-test
  templates:
  - name: wf-artifact-test
    metadata:
      annotations:
        iam.amazonaws.com/role: '{{workflow.annotations.iam.amazonaws.com/role}}'
    podSpecPatch: |
      containers:
      - name: wait
        image: argoproj/argoexec:latest
    container:
      name: wf-artifact-test
      image: busybox
      command:
      - /bin/sh
      - -c
      - |
        echo '{}' > /out/test.json
        sleep 1
        cat /out/test.json
      resources:
        requests:
          cpu: 100m
          memory: &memory 300Mi
        limits:
          memory: *memory
      volumeMounts:
      - name: out
        mountPath: /out
    volumes:
    - name: out
      emptyDir: {}
    outputs:
      artifacts:
      - archive:
          none: {}
        name: test-artifact
        path: /out/test.json
        s3:
          bucket: object-lock-enabled-bucket
          endpoint: s3.amazonaws.com
          key: test/test-artifact.json

Logs from the workflow controller

(none)

Logs from in your workflow's wait container

time="2023-06-05T20:02:44.014Z" level=warning msg="Non-transient error: Content-MD5 OR x-amz-checksum- HTTP header is required for Put Object requests with Object Lock parameters"
time="2023-06-05T20:02:44.014Z" level=error msg="executor error: failed to put file: Content-MD5 OR x-amz-checksum- HTTP header is required for Put Object requests with Object Lock parameters"
time="2023-06-05T20:02:44.114Z" level=fatal msg="failed to put file: Content-MD5 OR x-amz-checksum- HTTP header is required for Put Object requests with Object Lock parameters"
@juliev0
Copy link
Contributor

juliev0 commented Jun 8, 2023

@akloss-cibo do you want to try updating to that version of minio in that case and contribute the fix if it's working?

@juliev0 juliev0 added the P3 Low priority label Jun 8, 2023
@akloss-cibo
Copy link
Author

It looks like dependabot keeps minio up to date (see commit 760299f). I tried using argoproj/argoexec@sha256:e237c40aadcfd5d3d6b8325d4eba79e53fd7085cccd8dc6eefd48049966d873c and it still fails. Perhaps Argo Workflows is using one of the cases that minio doesn't handle correctly.

minio/minio#16480 seems related as well.

@juliev0
Copy link
Contributor

juliev0 commented Jun 8, 2023

thanks for checking

@harshavardhana
Copy link

Client must set Content-MD5 in this case and the problem is solved for all S3 providers. Is there a specific reason argo-cd is trying to look for the exact error returned in this case?

@harshavardhana
Copy link

harshavardhana commented Aug 21, 2023

This is what mc does if set the content-md5 if the bucket has ObjectLockEnabled.

@stale

This comment was marked as resolved.

@stale stale bot added the problem/stale This has not had a response in some time label Sep 17, 2023
@terrytangyuan terrytangyuan removed the problem/stale This has not had a response in some time label Sep 20, 2023
@agilgur5 agilgur5 added the area/artifacts S3/GCP/OSS/Git/HDFS etc label Oct 5, 2023
@tooptoop4
Copy link
Contributor

would u like to raise PR @harshavardhana @akloss-cibo ?

@akloss-cibo
Copy link
Author

I have hacked together a solution that does seem to work for us. The only required change is

diff --git a/s3/s3.go b/s3/s3.go
index e7fda4b..7ebd146 100644
--- a/s3/s3.go
+++ b/s3/s3.go
@@ -219,7 +219,7 @@ func (s *s3client) PutFile(bucket, key, path string) error {
                return err
        }

-       _, err = s.minioClient.FPutObject(s.ctx, bucket, key, path, minio.PutObjectOptions{ServerSideEncryption: encOpts})
+       _, err = s.minioClient.FPutObject(s.ctx, bucket, key, path, minio.PutObjectOptions{SendContentMd5: true, ServerSideEncryption: encOpts})
        if err != nil {
                return err
        }
%

in argoprog/pkg. This is not a very elegant solution.

To my eye, augmenting S3ClientOpts to include an option for this seems reasonable. I'm not a skilled golang engineer so if anyone wants to send me down a different path first, please speak up.

akloss-cibo added a commit to cibotech/argoproj-pkg that referenced this issue Jan 21, 2025
This checksum is required if a bucket has Object Lock enabled by
default.

addresses argoproj/argo-workflows#11182
akloss-cibo added a commit to cibotech/argoproj-pkg that referenced this issue Jan 21, 2025
This checksum is required if a bucket has Object Lock enabled by
default.

addresses argoproj/argo-workflows#11182

Signed-off-by: Alec Kloss <[email protected]>
@akloss-cibo
Copy link
Author

I'm not quite sure how to get someone's attention to look at argoproj/pkg#790... help?

akloss-cibo added a commit to cibotech/argoproj-pkg that referenced this issue Jan 23, 2025
This checksum is required if a bucket has Object Lock enabled by
default.

addresses argoproj/argo-workflows#11182

Signed-off-by: Alec Kloss <[email protected]>
@akloss-cibo
Copy link
Author

akloss-cibo commented Jan 24, 2025

I have this working on-site with some images I built but I'm stuck in the make pre-commit -B; this bit of the docker part exit code 1s with no other messages:

% docker run --rm --user 501:20 -v /Users/alec/go/src/github.com/argoproj/argo-workflows/sdks/java/client:/wd --workdir /wd openapitools/openapi-generator-cli:v5.2.1 \
                generate -v \
                -i /wd/swagger.json \
                -g java \
                -o /wd \
                -p hideGenerationTimestamp=true \
                -p serializationLibrary=jsonb \
                -p dateLibrary=java8 \
                --api-package io.argoproj.workflow.apis \
                --invoker-package io.argoproj.workflow \
                --model-package io.argoproj.workflow.models \
                --skip-validate-spec \
                --group-id io.argoproj.workflow \
                --artifact-id argo-client-java \
                --artifact-version 0.0.0-SNAPSHOT \
                --import-mappings Time=java.time.Instant \
                --import-mappings Affinity=io.kubernetes.client.openapi.models.V1Affinity \
                --import-mappings ConfigMapKeySelector=io.kubernetes.client.openapi.models.V1ConfigMapKeySelector \
                --import-mappings Container=io.kubernetes.client.openapi.models.V1Container \
                --import-mappings ContainerPort=io.kubernetes.client.openapi.models.V1ContainerPort \
                --import-mappings EnvFromSource=io.kubernetes.client.openapi.models.V1EnvFromSource \
                --import-mappings EnvVar=io.kubernetes.client.openapi.models.V1EnvVar \
                --import-mappings HostAlias=io.kubernetes.client.openapi.models.V1HostAlias \
                --import-mappings Lifecycle=io.kubernetes.client.openapi.models.V1Lifecycle \
                --import-mappings ListMeta=io.kubernetes.client.openapi.models.V1ListMeta \
                --import-mappings LocalObjectReference=io.kubernetes.client.openapi.models.V1LocalObjectReference \
                --import-mappings ObjectMeta=io.kubernetes.client.openapi.models.V1ObjectMeta \
                --import-mappings ObjectReference=io.kubernetes.client.openapi.models.V1ObjectReference \
                --import-mappings PersistentVolumeClaim=io.kubernetes.client.openapi.models.V1PersistentVolumeClaim \
                --import-mappings PodDisruptionBudgetSpec=io.kubernetes.client.openapi.models.V1beta1PodDisruptionBudgetSpec \
                --import-mappings PodDNSConfig=io.kubernetes.client.openapi.models.V1PodDNSConfig \
                --import-mappings PodSecurityContext=io.kubernetes.client.openapi.models.V1PodSecurityContext \
                --import-mappings Probe=io.kubernetes.client.openapi.models.V1Probe \
                --import-mappings ResourceRequirements=io.kubernetes.client.openapi.models.V1ResourceRequirements \
                --import-mappings SecretKeySelector=io.kubernetes.client.openapi.models.V1SecretKeySelector \
                --import-mappings SecurityContext=io.kubernetes.client.openapi.models.V1SecurityContext \
                --import-mappings Toleration=io.kubernetes.client.openapi.models.V1Toleration \
                --import-mappings Volume=io.kubernetes.client.openapi.models.V1Volume \
                --import-mappings VolumeDevice=io.kubernetes.client.openapi.models.V1VolumeDevice \
                --import-mappings VolumeMount=io.kubernetes.client.openapi.models.V1VolumeMount \
                --generate-alias-as-model
% echo $?
1
%

Help?

akloss-cibo added a commit to cibotech/argoproj-argo-workflows that referenced this issue Jan 31, 2025
This change will allow objects to be written to buckets with S3 Object
Lock enabled.

fixes argoproj#11182

Signed-off-by: Alec Kloss <[email protected]>
@akloss-cibo akloss-cibo linked a pull request Jan 31, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/artifacts S3/GCP/OSS/Git/HDFS etc P3 Low priority type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants