1Password / connect-helm-charts

Official 1Password Helm Charts
https://developer.1password.com
MIT License
93 stars 74 forks source link

Remove need for NET_BROADCAST #81

Closed jpcoenen closed 2 years ago

jpcoenen commented 2 years ago

As requested in both https://github.com/1Password/connect/issues/22 and https://github.com/1Password/connect/issues/21, this removes the need for the containers to obtain the NET_BROADCAST capability by no longer depending on the auto-discovery of each other.

Bumps Connect to 1.5.0 to use the new OP_BUS_PEERS option.

verkaufer commented 2 years ago

Should we make a note about these static port assignments in the README?

jpcoenen commented 2 years ago

Should we make a note about these static port assignments in the README?

Good question. Afaik, these ports are internal to the pod created by this Helm chart only. So I am inclined to say: no, this is an internal detail that is practically never relevant for the user.

But I may be wrong there. Do you have a specific case in mind where this would be relevant to know?

verkaufer commented 2 years ago

Should we make a note about these static port assignments in the README?

Good question. Afaik, these ports are internal to the pod created by this Helm chart only. So I am inclined to say: no, this is an internal detail that is practically never relevant for the user.

But I may be wrong there. Do you have a specific case in mind where this would be relevant to know?

The only case I can think of is a customer preferring to use certain ports to adhere to any network firewall rules. We could defer this until a customer raises the issue, however.

Good to merge imo