Skip to content

Conversation

@Stun3R
Copy link
Contributor

@Stun3R Stun3R commented May 11, 2021

What does it do?

It change the default configuration example for PostgreSQL database since it's wrong regarding SSL

Why is it needed?

Lot of people were in trouble trying to make strapi works with PostgreSQL on several Hosting platform (Qovery/Heroku...)
The default configuration was not right regarding the SSL configuration for knex

Related issue(s)/PR(s)

#281

@derrickmehaffy
Copy link
Member

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/help-with-s3-storage-and-cdn/4797/13

Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is actually wrong. The SSL option isn't used and the one removed either needs to be an object for that rejectUnauthorized (needed if SSL is self-signed) or it needs to be a boolean false if you want to disable SSL.

@Stun3R
Copy link
Contributor Author

Stun3R commented May 18, 2021

I believe this is actually wrong. The SSL option isn't used and the one removed either needs to be an object for that rejectUnauthorized (needed if SSL is self-signed) or it needs to be a boolean false if you want to disable SSL.

Yes I see, should we write two versions for this configuration since we can't set an object inside an ENV variable ?

I'll do some test if it's possible to only let the rejectUnauthorized to false by default and the hosting don't throw us the SSL error

@derrickmehaffy
Copy link
Member

I believe this is actually wrong. The SSL option isn't used and the one removed either needs to be an object for that rejectUnauthorized (needed if SSL is self-signed) or it needs to be a boolean false if you want to disable SSL.

Yes I see, should we write two versions for this configuration since we can't set an object inside an ENV variable ?

I'll do some test if it's possible to only let the rejectUnauthorized to false by default and the hosting don't throw us the SSL error

You can, for that you use env.json(): https://strapi.io/documentation/developer-docs/latest/setup-deployment-guides/configurations.html#casting-environment-variables

We do clarify in the settings:

ssl (boolean/object): For ssl database connection. Object is used to pass certificate files as strings.

https://strapi.io/documentation/developer-docs/latest/setup-deployment-guides/configurations.html#database

@Stun3R
Copy link
Contributor Author

Stun3R commented May 18, 2021

I believe this is actually wrong. The SSL option isn't used and the one removed either needs to be an object for that rejectUnauthorized (needed if SSL is self-signed) or it needs to be a boolean false if you want to disable SSL.

Yes I see, should we write two versions for this configuration since we can't set an object inside an ENV variable ?
I'll do some test if it's possible to only let the rejectUnauthorized to false by default and the hosting don't throw us the SSL error

You can, for that you use env.json(): https://strapi.io/documentation/developer-docs/latest/setup-deployment-guides/configurations.html#casting-environment-variables

We do clarify in the settings:

ssl (boolean/object): For ssl database connection. Object is used to pass certificate files as strings.

https://strapi.io/documentation/developer-docs/latest/setup-deployment-guides/configurations.html#database

So we should just keep the default ssl object :

ssl: {
    rejectUnauthorized: env.bool('DATABASE_SSL_SELF', false), // For self-signed certificates
 },

and add some details about the SSL server error because when we use the object knex consider the ssl options as true and we get the SSL server error ?

@derrickmehaffy
Copy link
Member

Exactly

@Stun3R
Copy link
Contributor Author

Stun3R commented May 18, 2021

Exactly

I fixed the change you requested me. What are you thought on "How can we add some details about the SSL server error ?" ?

@derrickmehaffy
Copy link
Member

Exactly

I fixed the change you requested me. What are you thought on "How can we add some details about the SSL server error ?" ?

We will need to add a notice in the documentation @pwizla thoughts?

@pwizla
Copy link
Collaborator

pwizla commented May 19, 2021

Exactly

I fixed the change you requested me. What are you thought on "How can we add some details about the SSL server error ?" ?

We will need to add a notice in the documentation @pwizla thoughts?

Thank you for the contribution @Stun3R and thank you for the explanations @derrickmehaffy 🙂

Indeed, I encourage you to add a

::: warning WARNING

callout in which you can describe the SSL server issue and how it can be solved, in a few sentences.

Thank you!

@Stun3R
Copy link
Contributor Author

Stun3R commented May 19, 2021

Exactly

I fixed the change you requested me. What are you thought on "How can we add some details about the SSL server error ?" ?

We will need to add a notice in the documentation @pwizla thoughts?

Thank you for the contribution @Stun3R and thank you for the explanations @derrickmehaffy 🙂

Indeed, I encourage you to add a

::: warning WARNING

callout in which you can describe the SSL server issue and how it can be solved, in a few sentences.

Thank you!

You're welcome ! 😄 It looks like the ::: warning WARNING inside tabs break it since it has to be closed by the same ::: than a tab 😞

@pwizla
Copy link
Collaborator

pwizla commented May 19, 2021

You're welcome ! 😄 It looks like the ::: warning WARNING inside tabs break it since it has to be closed by the same ::: than a tab 😞

Oh, I see! I ran into the same issue a few days ago 😄

You can use 5 colons to open and close the tabs, then 4 colons to open/close a specific tab, and then the regular 3 colons for the warning.

Like this:

::::: tabs
:::: tab topicOne
      
::: warning WARNING

:::

::::

:::: tab topicTwo
  
::::
:::::

VuePress needs at least 3 colons to understand it's a custom container, but then you can have more, and the number of : will define the hierarchy. You can find an example in the caddy-proxy doc for instance.

Hope this helps! 🙂

@vercel
Copy link

vercel bot commented May 19, 2021

@Stun3R is attempting to deploy a commit to the Strapi Test US Team on Vercel.

A member of the Team first needs to authorize it.

@Stun3R
Copy link
Contributor Author

Stun3R commented May 19, 2021

You're welcome ! 😄 It looks like the ::: warning WARNING inside tabs break it since it has to be closed by the same ::: than a tab 😞

Oh, I see! I ran into the same issue a few days ago 😄

You can use 5 colons to open and close the tabs, then 4 colons to open/close a specific tab, and then the regular 3 colons for the warning.

Like this:

::::: tabs
:::: tab topicOne
      
::: warning WARNING

:::

::::

:::: tab topicTwo
  
::::
:::::

VuePress needs at least 3 colons to understand it's a custom container, but then you can have more, and the number of : will define the hierarchy. You can find an example in the caddy-proxy doc for instance.

Hope this helps! 🙂

Thanks it helps a lot! I did the change and add the warning alert 🥰

@pwizla
Copy link
Collaborator

pwizla commented May 19, 2021

Awesome! 🤩
Thank you 🙂 It looks good to me, now.

I'll let @derrickmehaffy give his thoughts :-)

@pwizla pwizla self-requested a review May 19, 2021 08:43
@pwizla pwizla requested a review from derrickmehaffy May 19, 2021 08:44
@pwizla pwizla merged commit 9564a4c into strapi:main May 19, 2021
@derrickmehaffy
Copy link
Member

(looks fine to be btw)

@pwizla
Copy link
Collaborator

pwizla commented May 19, 2021

(looks fine to be btw)

I clicked on the merge button too fast, sorry @derrickmehaffy! 😬 😅

@Stun3R Stun3R deleted the dev/postgres-config-ssl branch May 19, 2021 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants