Openfoodnetwork: [v3] Images are not displaying on BE

[TheoGallez]

Description

There is no more image on hubs of www.OpenFoodNetwork.be.
No image product
No image logo entreprise
Nos image banner entreprise.

Expected Behavior

amazone server ?

Severity

very important!

[sauloperez]

Solved! here comes what I did

First, the DB seemed showed the appropriate values although I didn't check whether API keys were correct. I assumed they weren't changed.

openfoodnetwork=> select * from spree_preferences where key ilike '%s3%';
-[ RECORD 1 ]--------------------------------------------
id         | 61
value      | ofn-belgium
created_at | 2018-10-29 20:08:34.890521
updated_at | 2018-11-01 21:05:49.400378
key        | /spree/app_configuration/s3_bucket
value_type | string
-[ RECORD 2 ]--------------------------------------------
id         | 58
value      | t
created_at | 2018-10-29 20:08:34.873126
updated_at | 2019-05-09 16:00:07.339224
key        | /spree/app_configuration/use_s3
value_type | boolean
-[ RECORD 3 ]--------------------------------------------
id         | 62
value      | --- https                                   +
           | ...                                         +
           |
created_at | 2018-10-29 20:08:34.896605
updated_at | 2020-06-25 09:05:38.83934
key        | /spree/app_configuration/s3_protocol
value_type | string
-[ RECORD 4 ]--------------------------------------------
id         | 116
value      | {"Cache-Control":"max-age=31557600"}
created_at | 2018-11-01 21:05:49.35094
updated_at | 2018-11-01 21:05:49.35094
key        | /spree/app_configuration/s3_headers
value_type | string

(...)

I also double checked there were no old preferences entries using the former key format and that the migration was ran.

Then, seeing that use_s3 above was set to true I went straight with a Rails.cache.clear and a unicorn and DJ restart. It was the bit that had to be wrong. No luch :sweat: . I was a bit surprised about the return value. Why an array and not a plain true? although I haven't found any change on that command in Rails 4 it looks it didn't take any effect :thinking:

The rails console was still returning false:

irb(main):001:0> Spree::Config.use_s3                                                                                                                                                           
=> false
irb(main):002:0> Rails.cache.read('/spree/app_configuration/use_s3')                                                                                                                            
=> false 

But others matched the DB values, including the s3_secret:

irb(main):005:0> Rails.cache.read('/spree/app_configuration/s3_bucket')
=> "ofn-belgium"
irb(main):008:0> Rails.cache.read('/spree/app_configuration/s3_headers')
=> "{\"Cache-Control\":\"max-age=31557600\"}"
(...)

Finally, I tried manually enabling USE AMAZON S3 FOR IMAGES from /admin/image_settings/edit and after a Unicorn and DJ restart, we got them back.

Although I doubt it, I'm wondering if there was any manual intervention. Did you change any configuration by any chance @TheoGallez ?

I really suspect the duplicate ofn-install configs and the UI settings might the root cause here. It's been problematic in the past the fact that they can be set from these two sources.

[sauloperez]

I can't find a proper explanation yet and nothing seems out of normal. Maybe the explanation might ring a bell to someone?

use_s3 is configured at boot time in https://github.com/openfoodfoundation/openfoodnetwork/blob/9e730e6445231c16396de2da7c33bff26c0aed74/config/initializers/spree.rb#L40 if the env var S3_BUCKET is set. That's not the case right now. The instance must have https://github.com/openfoodfoundation/ofn-install/blob/31833e59f76170b4fa2fb6e6acf65b77034c7521/roles/app/templates/application.yml.j2#L43 defined. The DB had use_s3 properly set to true.

Maybe time ago a manual configuration change wasn't followed by a unicorn and DJ restart and the cache wasn't updated? Does Spree update the cached value? it might not.

I'd really to the bottom of all this reproducing it in staging. Even checking combinations of manually setting the S3_BUCKET and what I described above. In any case, IMO we really need to lower the complexity of this by removing these settings from the UI leaving it to ofn-install only. These are not meant to be touched by a manager but a dev.

[luisramos0]

I think this could still be an effect of https://github.com/openfoodfoundation/openfoodnetwork/issues/5657
That t value in the db, when you update it through the UI it becomes: --- true
I wonder if the v3 code understands the preference values saved in the v2 format.

I think the next step is to test this in staging as we tested #5660. I am moving this to "dev test ready", ok?

I agree we need to remove these configs from the admin UI.

[Matt-Yorkley]

https://github.com/openfoodfoundation/ofn-secrets/pull/40

[luisramos0]

I was about to test this but then I went to staging AU and I can see AU staging was upgraded to v3 without any config changes and the S3 images are working fine there.
Going to ofn-secrets there's a big difference between au staging and be-prod, the key s3_images_bucket, so Matt's PR above will fix the problem in BE. Only au-prod and au-staging have this s3_images_bucket config defined :warning: we need to update all secrets of all servers that are using S3: FR and UK at least.

Additionally I think we need to create an issue and remove all configs from the "Configuration" menu that are changed by ofn-install and can't really be changed without dev intervention, for example, images location and urls.. #5695

[sauloperez]

Thank you @luisramos0 ! That's the issue we really need.

[luisramos0]

I think this is dev ready, we need to add s3_images_bucket to ofn-secrets.

[Matt-Yorkley]

I made a PR for that about 5 days ago...

[sauloperez]

Already done in https://github.com/openfoodfoundation/ofn-secrets/pull/40 and merged. It will need provisioning though.

[luisramos0]

That PR is for BE, we need to do that for all the instances that use S3 and dont have that setting yet.

[Matt-Yorkley]

I'll update the secrets for the other servers now :+1:

[sauloperez]

Thanks @Matt-Yorkley !

[Matt-Yorkley]

The PR for this last little cleanup job is in code review in another repo, so I'll close this for now. The main issue is resolved, right?