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

Update ztApi.ts - change default URL of the Local Zerotier URL #196

Merged
merged 2 commits into from
Nov 1, 2023
Merged

Update ztApi.ts - change default URL of the Local Zerotier URL #196

merged 2 commits into from
Nov 1, 2023

Conversation

tinola
Copy link
Contributor

@tinola tinola commented Nov 1, 2023

I think address http://zerotier:9993 may be a bit misleading, especially for the standalone version installations. My proposition is to use by default (if not specified in .env file ZT_ADDR variable) http://127.0.0.1:9993 which should work everywhere.

I think address http://zerotier:9993 may be a bit misleading, especially for the standalone version installations.
My proposition is to use by default (if not specified in .env file ZT_ADDR variable) http://127.0.0.1:9993 which should work everywhere.
@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

This should avoid problem as described here

@sinamics
Copy link
Owner

sinamics commented Nov 1, 2023

ZT_ADDR is not added to docker-compose env as default. Using http://127.0.0.1:9993 as fallback will give error in docker.
We could do something like :

import { isRunningInDocker } from "./docker";

const LOCAL_ZT_ADDR =
	process.env.ZT_ADDR || isRunningInDocker()
		? "http://zerotier:9993"
		: "http://127.0.0.1:9993";

When not docker enviroment use http://127.0.0.1:9993 else http://zerotier:9993

@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

Great! You're absolutely right. That will do trick :)

@sinamics
Copy link
Owner

sinamics commented Nov 1, 2023

Do you want me to add the changes, or will you add them yourself?

When not docker enviroment use http://127.0.0.1:9993 else http://zerotier:9993
@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

Just updated my PR. Please check.

@sinamics
Copy link
Owner

sinamics commented Nov 1, 2023

Perfect.
Just tested in docker and works just fine.

@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

Thanks for cooperation! Then you can merge into main :)

@sinamics sinamics merged commit 557389a into sinamics:main Nov 1, 2023
2 checks passed
@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

Well... we have to consider to change also in other files for visual experience in webUI, because I still see this:

obraz

Grepping for "http://zerotier:9993" I found http://zerotier:9993 in these files as well:

./prisma/migrations/20230811063619_user_options/migration.sql:    "localControllerUrl" TEXT DEFAULT 'http://zerotier:9993',
./prisma/schema.prisma:  localControllerUrl             String?           @default("http://zerotier:9993")
./prisma/seeds/user-option.seed.ts:                                     localControllerUrl: "http://zerotier:9993",
./src/pages/admin/controller/index.tsx:                                                         placeholder: me?.options?.localControllerUrl || "http://zerotier:9993",

excluding auto generated during build.

I think that cruical is src/pages/admin/controller/index.tsx file, but I can be wrong.

Could you add proper conditions?

@sinamics
Copy link
Owner

sinamics commented Nov 1, 2023

i will add a check during the registration if the user is in docker or not and update db accourdingly.

@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

OK, thanks! 👍 But what about existing users?...

@sinamics
Copy link
Owner

sinamics commented Nov 1, 2023

users who already uses ztnet as standalone, has probably already changed this value. I dont want to create a new migration that will force new values in db when they update.

@sinamics sinamics mentioned this pull request Nov 1, 2023
4 tasks
@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

Understood. Basically, I only mean the visual effect in webUI as a placeholder in the screenshot as I pasted above :)

@sinamics
Copy link
Owner

sinamics commented Nov 1, 2023

I have updated the placeholder so the correct value will be displayed

@sinamics
Copy link
Owner

sinamics commented Nov 1, 2023

i will take ta deeper dive into this later. I think there is a potensial bug when using the ZT_ADDR env variable.

@tinola
Copy link
Contributor Author

tinola commented Nov 1, 2023

You're right. I've the same issue... ztnet stopped to respect ZT_ADDR env variable from .env file.
I thought it only applies to me ;) uff...

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.

None yet

2 participants