Can not edit topics with picture with S3 backend

dirien · June 14, 2021, 5:45am

Hi everybody,

we stumbled over a strange issue.

Our setting:

Discourse: 2.8.0.beta2
S3: Minio
CDN: Myra

The S3 is configured:

root@assets-prod-app01:~# sudo -u minio mc admin policy info minio readwrite-discourse
{
 "Version": "2012-10-17",
 "Statement": [
  {
   "Effect": "Allow",
   "Action": [
    "s3:ListBucketMultipartUploads",
    "s3:GetBucketLocation",
    "s3:ListBucket"
   ],
   "Resource": [
    "arn:aws:s3:::discourse"
   ]
  },
  {
   "Effect": "Allow",
   "Action": [
    "s3:*"
   ],
   "Resource": [
    "arn:aws:s3:::discourse/*"
   ]
  }
 ]
}

But we get all the time, when we edit following error message

In the logs files we get

Aws::S3::Errors::NotImplemented (A header you provided implies functionality that is not implemented)
lib/file_store/s3_store.rb:279:in `update_ACL'
lib/file_store/s3_store.rb:209:in `update_upload_ACL'
...

Is there a setting we did not consider, or is Minio not compatible with discourse?

Any hint would be nice, as we get a little bit pressure from our users

Thanks in advance

dirien · June 22, 2021, 4:24am

Is this setting so uncommon?

I would appriciate some direction on what to look at in more detail or what to change? @Falco At the moment we are quite stuck…

Falco · June 22, 2021, 4:55pm

Did you enable the site setting secure_uploads? That won’t work with MinIO at all.

dirien · June 22, 2021, 5:20pm

Hi @Falco,

unfortunatly not…

Falco · June 22, 2021, 9:01pm

Is the S3 setup configured as the documentation at Using Object Storage for Uploads (S3 & Clones) ?

dirien · June 23, 2021, 4:44am

Yepp, really letter for letter

dirien · July 1, 2021, 5:03am

Sorry to bother, but could you spot anything?

dirien · July 8, 2021, 5:32am

Hi, do you have any ideas? Sorry to bother…

appcomqa · July 8, 2021, 3:23pm

Hey,

Today I upgraded to 2.8.0.beta2, same error:

just select secure_media, minio work again.

Falco · July 8, 2021, 3:50pm

@pmusaraj / @martin looks like a regression caused by

github.com/discourse/discourse

FIX: Update upload secure status when revising posts

discourse:master ← discourse:issue/update-upload-secure-status-on-post-revision

opened 02:33AM - 21 May 21 UTC

martin-brennan

+36 -0

When uploads are created from the composer (editing or creating a post), for si…tes with secure uploads enabled we assume security by default and that new upload is set to secure. When the post is created, we then check whether the post uploads _actually_ need to be secure and adjust accordingly. We were not doing this when revising a post, so when a new upload was created when editing a post in a public topic, the secure status stayed true erroneously causing issues with image previews, among other things.

which didn’t take into account the same stuff that was fixed by

So the post_creator does:

github.com

discourse/discourse/blob/master/lib/post_creator.rb#L427-L429

    
      
          def update_uploads_secure_status
            @post.update_uploads_secure_status(source: "post creator") if SiteSetting.secure_media?
          end

and models/post:

github.com

discourse/discourse/blob/master/lib/post_revisor.rb#L232

    
      
            )
          end
          
          
# WARNING: do not pull this into the transaction
          # it can fire events in sidekiq before the post is done saving
          # leading to corrupt state
          QuotedPost.extract_from(@post)
          
          
# This must be done before post_process_post, because that uses
          # post upload security status to cook URLs.
          @post.update_uploads_secure_status(source: "post revisor")
          
          
post_process_post
          
          
update_topic_word_counts
          alert_users
          publish_changes
          grant_badge
          
          
TopicLink.extract_from(@post)

martin · July 9, 2021, 12:18am

There has already been a fix for this that was done for performance reasons but should also fix your problem:

~~Though I will make a fix to the job so that it exits early if secure media is disabled.~~

Actually, I cannot do the above because we want post uploads to be marked not secure if secure media is disabled or if the post has been moved to a different category. Instead I will add a fix to capture the Aws::S3::Errors::NotImplemented error when we run Discourse.store.update_upload_ACL(self). That way clients that do not support the private ACLs should have no problem.

Edit: I merged this fix today, hopefully it will help FIX: Handle storage providers not implementing ACLs by martin-brennan · Pull Request #13675 · discourse/discourse · GitHub. I think we are cutting a new beta next week as well.

Falco · July 12, 2021, 11:00am

This topic was automatically closed after 3 days. New replies are no longer allowed.