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

Make public_ip and domain_name top level config keys #24

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

bakhtin
Copy link
Contributor

@bakhtin bakhtin commented Nov 28, 2024

📝 Summary

  • Make public_ip and domain_name top level config keys
  • Orderflow Proxy p2p bundles port will be behind HAProxy. Move it to localhost. HAProxy will listen on 443.

⛱ Motivation and Context

Multiple services require public_ip and dns_name keys for TLS cert generation. To not duplicate move them to top level.

📚 References


✅ I have run these commands

  • make lint
  • make test
  • go mod tidy

@bakhtin bakhtin requested a review from metachris November 28, 2024 12:12
"public_listen_addr": "0.0.0.0:5544",
"cert_hosts": "1.2.3.4,localhost,127.0.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the rationale for replacing cert_hosts with a fixed IP and DNS entry? should be fine anyway, just curious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is easier to compose a string with CN names for the certificate from the individual components like IP address and DNS name when needed. Top level IP address and DNS name may come in handy for some other services (though we don't have a use case right now).

@bakhtin bakhtin merged commit d80e618 into main Dec 2, 2024
2 checks passed
@bakhtin bakhtin deleted the ab-top-level-ip branch December 2, 2024 12:09
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.

2 participants