TrueCloudLab/frostfs-node

Fork 27

Move `SignMessage` away from `control/server` package #1092

New issue

Closed

opened 2024-04-12 08:45:59 +00:00 by fyrchik · 3 comments

fyrchik commented

2024-04-12 08:45:59 +00:00

Owner

This makes it necessary to import server package in client code just to sign the message.

func SignMessage(key *ecdsa.PrivateKey, msg SignedMessage) error {

cc @alexvanin

This makes it necessary to import server package in client code just to sign the message. https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/40781b3a20240185b2b76e6fc676196ded1d9395/pkg/services/control/server/sign.go#L73 cc @alexvanin

fyrchik added the

frostfs-node

label 2024-04-12 08:45:59 +00:00

fyrchik added this to the v0.39.0 milestone 2024-04-12 08:46:03 +00:00

achuprov was assigned by fyrchik

2024-04-22 08:03:23 +00:00

fyrchik commented

2024-04-22 08:28:55 +00:00

Author

Owner

It could be in the control package itself, which is imported in control/server anyway.

It could be in the `control` package itself, which is imported in `control/server` anyway.

achuprov added the

internal

label 2024-04-22 09:49:00 +00:00

achuprov commented

2024-04-23 16:08:04 +00:00

Member

Moving SignMessage to the control package would cause a cyclic import of packages. It might then be necessary to create a separate package.
@alexvanin Can you describe the issue in more detail?

Moving `SignMessage` to the control package would cause a cyclic import of packages. It might then be necessary to create a separate package. @alexvanin Can you describe the issue in more detail?

alexvanin commented

2024-05-02 11:59:42 +00:00

Owner

@alexvanin Can you describe the issue in more detail?

Sure. In my project I import two dependencies.

Dependency	Reason
git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control/server	Use `SignMessage` to sign Control API requests to update policy chains
git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool/tree	Use tree pool to send requests to Tree Service

Those dependencies also use these packages:

git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/tree
git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool/tree/service

And those packages contain init function to initializae gRPC structure which leads to runtime error in the application

panic: proto: file "pkg/services/tree/types.proto" is already registered
        previously from: "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool/tree/service"
        currently from:  "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/tree"
See https://protobuf.dev/reference/go/faq#namespace-conflict

My fix was to copy sign.go file to my repo and avoid package import.

> @alexvanin Can you describe the issue in more detail? Sure. In my project I import two dependencies. | Dependency | Reason | | -- | -- | | git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control/server | Use `SignMessage` to sign Control API requests to update policy chains | | git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool/tree | Use tree pool to send requests to Tree Service | Those dependencies also use these packages: - git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/tree - git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool/tree/service And those packages contain `init` function to initializae gRPC structure which leads to runtime error in the application ``` panic: proto: file "pkg/services/tree/types.proto" is already registered previously from: "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool/tree/service" currently from: "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/tree" See https://protobuf.dev/reference/go/faq#namespace-conflict ``` My fix was to copy `sign.go` file to my repo and avoid package import.

fyrchik modified the milestone from v0.39.0 to vNext

2024-05-14 14:03:42 +00:00

achuprov referenced this issue from a pull request that will close it,

2024-05-14 14:23:31 +00:00

node: Move SignMessage to separate package #1132

fyrchik modified the milestone from vNext to v0.40.0

2024-05-14 14:29:30 +00:00

fyrchik closed this issue

2024-05-16 10:35:50 +00:00

fyrchik referenced this issue from a commit

2024-05-16 10:35:50 +00:00

[#1092] control: Move SignMessage to separate package