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

Media.AmazonS3 requires AccessKeyId if NuGet package is used #12570

Open
quickstar opened this issue Oct 5, 2022 · 15 comments
Open

Media.AmazonS3 requires AccessKeyId if NuGet package is used #12570

quickstar opened this issue Oct 5, 2022 · 15 comments

Comments

@quickstar
Copy link

quickstar commented Oct 5, 2022

Describe the bug

I'm experiencing a very strange behaviour. I'm implementing the OrchardCore.Media.AmazonS3 storage provider. If I clone the OrchardCore repository and create a config entry like this:

"OrchardCore_Media_AmazonS3": {
	"Region": "eu-west-3",
	"Credentials": {
		"SecretKey": "*****",
		"AccessKey": "***"
	},
	"CreateBucket": false,
	"BucketName": "mybucket"
}

everthing seems to work just fine.

If I however create a new decoupledCMS project and add a plain MVC Project and add just the following NuGet packages

  <ItemGroup>
    <PackageReference Include="OrchardCore.Application.Cms.Core.Targets" Version="1.4.0" />
    <PackageReference Include="OrchardCore.Media.AmazonS3" Version="1.4.0" />
  </ItemGroup>

I get the following error message:

OrchardCore.Media.AmazonS3.Startup[0]
      S3 Media configuration validation failed with errors: AccessKeyId is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified fallback to local file storage.
OrchardCore.Media.AmazonS3.Startup: Error: S3 Media configuration validation failed with errors: AccessKeyId is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified fallback to local 
file storage.

Eventhough I use the exact same config as mentioned obove, any ideas what I might missing?

@hishamco
Copy link
Member

hishamco commented Oct 5, 2022

/cc @neglectedvalue

@neglectedvalue
Copy link
Contributor

Hello seems like the issue is in configuration files in decoupled mode, I was able to reproduce this bug with AWS and Azure modules.

  "OrchardCore": {
  "OrchardCore_Media_Azure":
  {
    "ConnectionString": "1212", 
    "ContainerName": "somecontainer", 
    "BasePath": "some/base/path", 
    "CreateContainer": false 
  }
}
fail: OrchardCore.Media.Azure.Startup[0]
      Azure Media Storage is enabled but not active because the 'ConnectionString' is missing or empty in application configuration.
fail: OrchardCore.Media.Azure.Startup[0]
      Azure Media Storage is enabled but not active because the 'ContainerName' is missing or empty in application configuration.

Upd:
So far after small investigation I can say that in decoupled mode Orchard doesn't use appsettings.json of the project but it uses only App_Data\Sites{TenantName}\appsettings.json , is it correct behavior @hishamco ?

@hishamco
Copy link
Member

hishamco commented Oct 6, 2022

@jtkech your though?

@quickstar
Copy link
Author

quickstar commented Oct 6, 2022

So far after small investigation I can say that in decoupled mode Orchard doesn't use appsettings.json of the project but it uses only App_Data\Sites{TenantName}\appsettings.json , is it correct behavior @hishamco ?

I guess it does use the appsettings.json from the project, but only partially because the media provider itself is recognized during the bootstrap process.

@neglectedvalue
Copy link
Contributor

I guess it does use the appsettings.json from the project, but only partially because the media provider itself is recognized during the bootstrap process.

What do you mean by media provider itself? If you are talking about S3/Azure media storages - they are using appsettings only to get storage configuration, their bootstrap happens only if you are enabled them in admin->features or created a custom recipe.

@hishamco
Copy link
Member

hishamco commented Oct 6, 2022

I remember the configuration could came from different sources, but there should be an order and fallback in case one of the configuration provider is missing

@quickstar
Copy link
Author

What do you mean by media provider itself? If you are talking about S3/Azure media storages - they are using appsettings only to get storage configuration, their bootstrap happens only if you are enabled them in admin->features or created a custom recipe.

Yes, sorry my bad. Off course you're right 🤦🏽‍♂️

@quickstar
Copy link
Author

Is this a general bug then, that in decoupled mode using the NuGet packages, the appsettings.json hierarchy is not respected?

@sebastienros
Copy link
Member

Decoupled (not front-end theme, no layout shapes) should have nothing to do with appsettings. Maybe you could debug the source to understand where this makes a difference.

@sebastienros sebastienros added this to the backlog milestone Oct 20, 2022
@jtkech
Copy link
Member

jtkech commented Oct 20, 2022

@quickstar

If you reference OC.Application.Cms.Core.Targets normally you don't need to reference OrchardCore.Media.AmazonS3. Then can you share some code to see how you activate the feature and then in which context you use it.

@Piedone
Copy link
Member

Piedone commented May 21, 2024

Closing due to no reply from the author.

@Piedone Piedone closed this as not planned Won't fix, can't repro, duplicate, stale May 21, 2024
@quickstar
Copy link
Author

quickstar commented May 28, 2024

@Piedone, I didn't answer because I thought it's out of my hand. Since @neglectedvalue was able to reproduce it:

So far after small investigation I can say that in decoupled mode Orchard doesn't use appsettings.json of the project but it uses only App_Data\Sites{TenantName}\appsettings.json , is it correct behavior @hishamco ?

@hishamco and @jtkech should probably decide what the correct behavior should be.

@Piedone
Copy link
Member

Piedone commented May 28, 2024

Please follow up from here: #12570 (comment)

JT, sadly, won't help anymore: #14954.

@Piedone Piedone reopened this May 28, 2024
@quickstar
Copy link
Author

quickstar commented May 28, 2024

JT, sadly, won't help anymore: #14954.

My condolences 🫶

In my opinion, the correct behavior should be as follows:

  • Primary Source: Use the App_Data\Sites{TenantName}\appsettings.json file if a setting is found in it.
  • Fallback Source: If the setting is not found in the tenant-specific file, fallback to the project's appsettings.json file.

If a setting is present in both appsettings.json files, the more specific App_Data\Sites\{TenantName}\appsettings.json should take precedence.

@Piedone
Copy link
Member

Piedone commented May 28, 2024

That's what indeed should happen.

What you see is I think less related to NuGet, and should more lie in how the app is initialized (and thus how it loads configuration). Can you share your Program.cs and compare it with the one in OC's OrchardCore.Cms.Web?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants