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

Improve Docker setup + Certbot + Redis DB #30

Merged
merged 32 commits into from Jun 24, 2019

Conversation

Projects
None yet
4 participants
@agutsal
Copy link

commented Jun 10, 2019

Fix #25

@agutsal agutsal referenced this pull request Jun 10, 2019

Open

Improve Docker setup #25

Arsen A. Gutsal added some commits Jun 10, 2019

Arsen A. Gutsal
Some work has been done on Gitcoin Improve Docker Setup task
Installable dependencies modifications according to certbot website

now runs on 5001 port

copying build directory to Docker image

runs webapp in 5 seconds after redis
@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

Add the following to the .gitignore file

ssl/*.pem
ssl/certbot/*
!ssl/certbot/.keep
Show resolved Hide resolved Dockerfile Outdated

@agutsal agutsal changed the title WIP: Some work has been done on Gitcoin Improve Docker Setup task Work has been done on Gitcoin Improve Docker Setup task Jun 12, 2019

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 12, 2019

Please, review @pedrouid

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Hey @agutsal, I tested this in a Digital Ocean droplet but certbot was forcing me to choose a domain which I didn't pick.

I used @janus fork of your branch so that I could test both changes at the same time since most of his changes are on the app side. I believe your changes are the ones related to the ssl certificates.

Screenshot 2019-06-21 14 22 33

Basically I saw the messages display above. It asks which names to activate but only allows for 11cee86c.ngrok.io while I was tying to assign it to testing.walletconnect.org.

I have forked @janus fork to add the ssl, nginx and systemd directories that were missing. You can review my changes on this PR: janus#1

However in order to replicate my bug, here are the steps.

  1. Clone forked respository (pedrouid/node-walletconnect-bridge)
  2. Install Docker and Make
  3. Run make setup URL=testing.walletconnect.org
  4. Run make build (this step was successful)
  5. Run make run

I was able to do the whole process until I hit ssl certificate generation problem on make run. Let me know what you think it is. Thanks

@janus

This comment has been minimized.

Copy link

commented Jun 21, 2019

Hey @agutsal, I just compared your file and @pedrouid on nginx configuration, I found the followings:
1)
listen 80;
server_name bridge.mydomain.com;

listen 80;
server_name localhost:5001;

listen 443 ssl;
server_name bridge.mydomain.com;

listen 443 ssl;
server_name localhost:5001;

@pedrouid , Did the redis side of the app work? And should I merge your PR?

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

@janus lets wait for @pedrouid answer.

@pedrouid Thanks for so detailed explanation.
11cee86c.ngrok.io been cached somewhere inside github tree it seems. Lemme check and fix.

agutsal and others added some commits Jun 24, 2019

Merge pull request #1 from janus/master
Add redis data store capability
@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

@pedrouid tested locally - still attempts to use 11cee86c.ngrok.io

I'm new to certbot but found that:
image

Now we should find where that forewer cert is saved. Tried different emails and key.pem, cert.pem files removed. Any advice?

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

Seems that was my mistake, I had to use wildcard cert for *.ngrok.io:
image

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

it redirects to https://discordapp.com/channels/492410046307631105/492410046307631107 and stuck. Allow me 15 mins to rest. Will keep trying after that

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

My Discord is also not loading for some reason

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Let's use the WalletConnect Gitter which is unused

https://gitter.im/WalletConnect/Lobby

@pgporada

This comment has been minimized.

Copy link

commented Jun 24, 2019

Hi team,

I think I found the problem referenced on https://community.letsencrypt.org/t/certbot-keeps-suggesting-ngrok-domain/96456

The default nginx configuration has an ngrok address at the following line. https://github.com/WalletConnect/node-walletconnect-bridge/pull/30/files#diff-5c29fc0b85522159c4dce0482195bdd8R10

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Hi team,

I think I found the problem referenced on https://community.letsencrypt.org/t/certbot-keeps-suggesting-ngrok-domain/96456

The default nginx configuration has an ngrok address at the following line. https://github.com/WalletConnect/node-walletconnect-bridge/pull/30/files#diff-5c29fc0b85522159c4dce0482195bdd8R10

Wow I can't believe we missed that, thanks a lot for your help! 🙏

@pgporada

This comment has been minimized.

Copy link

commented Jun 24, 2019

Anytime, best of luck out there! That combined with the changes to the the Makefile at https://github.com/WalletConnect/node-walletconnect-bridge/pull/30/files#diff-b67911656ef5d18c4ae36cb6741b7965R23 I think are what puts the ngrok domain into the container.

pedrouid added some commits Jun 24, 2019

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

@pgporada thanks guys
@pedrouid what to put here, man? lets do it and finish it.

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

I already fixed it
agutsal#2

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

OK. So you're testing now?

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Yes, the Docker setup works fine, but the RedisDB implementation is not working yet so I can't merge this yet

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

@janus have you tested it? ^
@pedrouid plz, define the problem

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

I'm fixing them now, but @janus please pay more attention next time because there were typos plus all redis calls are asynchronous but you didn't adapt them in the rest of the app.

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

@pedrouid thx man, and sorry for 11cee86c.ngrok.io issue - it's totally mine. Just forgot I've modified defaultConf file

@pedrouid pedrouid changed the base branch from master to improved-docker Jun 24, 2019

@pedrouid

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Hey @agutsal, can you merge agutsal#2?

@agutsal

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

done

@pedrouid pedrouid merged commit 31eee4b into WalletConnect:improved-docker Jun 24, 2019

1 check passed

security/snyk - package.json (pedrouid) No new issues
Details
@janus

This comment has been minimized.

Copy link

commented Jun 24, 2019

I'm fixing them now, but @janus please pay more attention next time because there were typos plus all redis calls are asynchronous but you didn't adapt them in the rest of the app

Sorry for the inconvenience ... I initially asked for test data so that I could test it. It was untested ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.