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

Auto correct the IP if the operator set their IP to localhost when opt-in #136

Conversation

wmb-software-consulting
Copy link
Contributor

@wmb-software-consulting wmb-software-consulting commented Dec 18, 2023

Why are these changes needed?

We are seeing more operators setting their Node IP to localhost.
Such Nodes won't be reachable from public. We should auto correct the IP (from localhost to current IP with IP checker) when the operator registers.

Checks

  • I've made sure the lint is passing in this PR.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@wmb-software-consulting wmb-software-consulting changed the title Auto correct the IP if the operator set their IP to localhost when op… Auto correct the IP if the operator set their IP to localhost when opt-in Dec 18, 2023
node/plugin/config.go Outdated Show resolved Hide resolved
@jianoaix
Copy link
Contributor

@shrimalmadhur This can affect the operator setup or experience.

node/plugin/cmd/main.go Outdated Show resolved Hide resolved
core/state.go Outdated Show resolved Hide resolved
PubIPProviderFlag = cli.StringFlag{
Name: "public-ip-provider",
Usage: "The ip provider service used to obtain a operator's public IP [seeip (default), ipify)",
Required: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be required? @shrimalmadhur

Copy link
Contributor

Choose a reason for hiding this comment

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

oh umm - I don't think we should put it as required - can we make this as optional and by default point to seeip?

We can re-use the .env one in the setup

core/state.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jianoaix jianoaix left a comment

Choose a reason for hiding this comment

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

LGTM

core/state.go Show resolved Hide resolved
core/test/core_test.go Show resolved Hide resolved
node/plugin/cmd/main.go Outdated Show resolved Hide resolved
@wmb-software-consulting wmb-software-consulting merged commit e8d8b80 into Layr-Labs:master Dec 21, 2023
4 checks passed
@wmb-software-consulting wmb-software-consulting deleted the feat/auto-correct-operator-ip branch December 21, 2023 00:20
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