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

Integrate mycelium in zos #2342

Merged
merged 19 commits into from
Jun 10, 2024
Merged

Integrate mycelium in zos #2342

merged 19 commits into from
Jun 10, 2024

Conversation

Eslam-Nawara
Copy link
Contributor

@Eslam-Nawara Eslam-Nawara commented May 23, 2024

Description

Integrate mycelium in zos and allow flists to be served over mycelium

Changes

Add mycelium to networkd

Related Issues

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstring

@Eslam-Nawara Eslam-Nawara force-pushed the integrate-mycelium-in-zos branch 2 times, most recently from b260121 to 52be829 Compare May 28, 2024 12:45
@Eslam-Nawara Eslam-Nawara force-pushed the integrate-mycelium-in-zos branch from 52be829 to 70b73ee Compare May 28, 2024 13:17
@xmonader
Copy link
Collaborator

xmonader commented May 28, 2024

don't forget to add an ADR please

@Eslam-Nawara Eslam-Nawara force-pushed the integrate-mycelium-in-zos branch from 37da9be to f6fddb9 Compare May 30, 2024 13:03
@Eslam-Nawara Eslam-Nawara force-pushed the integrate-mycelium-in-zos branch from f6fddb9 to 29253a8 Compare May 30, 2024 13:35
@Eslam-Nawara Eslam-Nawara marked this pull request as ready for review May 30, 2024 13:47
@@ -148,9 +161,34 @@ func action(cli *cli.Context) error {
if err := dmzYgg.SetYggIP(ip, gw.IP); err != nil {
return errors.Wrap(err, "failed to set yggdrasil ip for dmz")
}

// set up mycelium in ndmz
dmzMy, err := mycelium.NewMyNamespace(dmz.Namespace())
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be extracted into another function


## Decision

Integrate mycelium in zos and allow zos host to have mycelium IPs, serve flists over mycelium, and support mycelium on zdbs
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it serve or mount flists over mycelium?

YggBridge = "br-ygg"
// MyBridge mycelium bridge
MyBridge = "br-my"
Copy link
Collaborator

Choose a reason for hiding this comment

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

MyCeliumBridge

}

ip, err := myc.SubnetFor(hw)
log.Info().Msgf("myc subnet is %s", ip)
Copy link
Collaborator

Choose a reason for hiding this comment

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

myc -> mycelium

@@ -114,13 +117,14 @@ func NewNetworker(identity *stubs.IdentityManagerStub, ndmz ndmz.DMZ, ygg *yggdr
portSet: set.NewInt(),

ygg: ygg,
myc: myc,
Copy link
Collaborator

Choose a reason for hiding this comment

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

mycelium:

"github.com/threefoldtech/zos/pkg/zinit"
)

func EnsureMycelium(ctx context.Context, privateKey ed25519.PrivateKey, ns MyceliumNamespace) (*MyServer, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

move this into mycelium.go and remove utils file

// this should return always the flist mountpoint. which is used
// as a base for all RW mounts.
sublog := log.With().Str("url", url).Str("storage", storage).Logger()
sublog.Info().Msg("request to mount flist")

hash, flistPath, err := f.downloadFlist(url)
hash, flistPath, err := f.downloadFlist(url, namespace)
Copy link
Member

Choose a reason for hiding this comment

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

You probably need to use this same namespace at line 262 below


key := x25519.EdPrivateKeyToX25519([]byte(privateKey))

return json.NewEncoder(f).Encode(key)
Copy link
Member

Choose a reason for hiding this comment

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

Why are u using json here. the priv key of mycelium is not json encoded. It's a binary file. i don't believe this file is loaded by mycelium and is either ignored is overridden.

If this ever worked, then i assume it's because mycelium just read key length bytes from the file and used that as seed for the key!

This is definitely an issue, because it means the key u generated here is not secure. because it's not the full "Random" key but only part of the key json encoded bytes.

Do you get what I mean ?

TunName string
Peers []string
PrivateKey ed25519.PrivateKey
PublicKey string
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

KeyFile string
TunName string
Peers []string
PrivateKey ed25519.PrivateKey
Copy link
Member

Choose a reason for hiding this comment

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

Why do you keep the private key as ed25519 while we need x25519. I think it's better to convert the key before storing it on the NodeConfig from the start instead of changing it on start

Peers []string
PrivateKey ed25519.PrivateKey
PublicKey string
NodeInfo map[string]interface{}
Copy link
Member

Choose a reason for hiding this comment

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

What is this and what is it used for?

We don't need to copy everything over from ygg. Ygg has a completely different configuration and even requires a config file with way more config than mycelium.

Please drop all attributes that are not needed.

myBin = "mycelium"
zinitService = "mycelium"
confPath = "/tmp/mycelium_priv_key.bin"
MyListenTCP = 9651
Copy link
Member

Choose a reason for hiding this comment

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

you will have to add this to the list of reserved ports of the system

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's added to reserved ports here

Comment on lines 109 to 115
func (s *MyServer) NodeID() (ed25519.PublicKey, error) {
if s.cfg.PublicKey == "" {
panic("EncryptionPublicKey empty")
}

return hex.DecodeString(s.cfg.PublicKey)
}
Copy link
Member

Choose a reason for hiding this comment

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

Not needed

return inspection, err
}

cmd := fmt.Sprintf(`exec ip netns exec %s %s inspect --json --key-file %s `, s.ns, bin, confPath)
Copy link
Member

Choose a reason for hiding this comment

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

Any idea why your command is a sh then u do exec and not the command directly!
I mean why not do

os.Command("ip", "netns", "exec", s.ns,  bin, "inspect", "--json", "--key-file", confPath)

?!

Address net.IP `json:"address"`
}

type Subnet [8]byte
Copy link
Member

Choose a reason for hiding this comment

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

this is not used


// SubnetFor return an IP address out of the node allocated subnet by hasing b and using it
// to generate the last 64 bits of the IPV6 address
func (m *MyceliumInspection) SubnetFor(b []byte) (net.IPNet, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I really think this should be called IPFor not SubnetFor even if it's called like that in ygg it's still wrong.

@Eslam-Nawara Eslam-Nawara force-pushed the integrate-mycelium-in-zos branch from 1fde494 to be8bc71 Compare June 3, 2024 07:21
@Eslam-Nawara Eslam-Nawara force-pushed the integrate-mycelium-in-zos branch from f45a0e1 to 14ebe83 Compare June 10, 2024 09:06
@muhamadazmy muhamadazmy merged commit fd7d15f into main Jun 10, 2024
24 checks passed
@muhamadazmy muhamadazmy deleted the integrate-mycelium-in-zos branch June 10, 2024 11:34
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