search

cloudflare / roughtime

A secure clock-synchronization protocol for when rough is enough.
https://developers.cloudflare.com/time-services/roughtime/
Apache License 2.0
135 stars 29 forks source link

Vendor error when using client library #5

Closed bluecmd closed 6 years ago

bluecmd commented 6 years ago

Hi,

I'm trying to use your server library, but running into vendoring issues.

You're expecting a roughtime.googlesource.com/go/config.Server as input to e.g. roughtime.Get. When I'm trying to use it like this:

package ttime

import (
    "log"
    "time"

    "github.com/cloudflare/roughtime"
    "golang.org/x/sync/errgroup"
    "roughtime.googlesource.com/go/config"
)

const (
    KEY_TYPE_ED25519 = "ed25519"
    roughtimeAttempts = 3
    roughtimeTimeout = 15 * time.Second
)

type RoughtimeServer struct {
    Protocol      string
    Address       string
    PublicKey     string
    PublicKeyType string
}

type NtpServer string

func getOneRoughtime(rs []RoughtimeServer) (*roughtime.Roughtime) {
    var g errgroup.Group
    cr := make(chan *roughtime.Roughtime, len(rs))
    for _, r := range rs {
        srv := &config.Server{
            Name: r.Address,
            PublicKeyType: r.PublicKeyType,
            PublicKey: []byte(r.PublicKey),
            Addresses: []config.ServerAddress{
                config.ServerAddress{Protocol: r.Protocol, Address: r.Address},
            },}
        g.Go(func() error {
            res, err := roughtime.Get(srv, roughtimeAttempts, roughtimeTimeout, nil)
            if err != nil {
                log.Printf("Failed to get roughtime from %s (skipping): %v", err)
                return err
            }
            cr <- res
            return nil
        })
    }

    go func() {
        // In the case of an error, put a nil in on the queue to ensure that
        // this function never deadlocks
        if err := g.Wait(); err != nil {
            cr <- nil
        }
    }()

    return <-cr
}

With roughtime.googlesource.com/go/config in my vendor directory, I get this:

./roughtime.go:43:29: cannot use srv (type *"github.com/u-root/u-bmc/vendor/roughtime.googlesource.com/go/config".Server) as type *"github.com/cloudflare/roughtime/vendor/roughtime.googlesource.com/go/config".Server in argument to roughtime.Get

Do you have any ideas on how to solve this?

cjpatton commented 6 years ago

Indeed, I'm really sorry about this ugliness. This should be resolved by this PR https://github.com/cloudflare/roughtime/pull/6 once it lands.

In the meantime, checkout this branch, then import "github.com/cloudflare/roughtime/config" instead of "roughtime.googlesource.com/.../config". Hope that helps!

bluecmd commented 6 years ago

Super! :) I've worked around it by taking all your code and temporarily imported it and hacked it together in my own fork, but I'll make sure to sync back to your version when it is fixed.

gabbifish commented 6 years ago

PR #6 has been merged--importing "github.com/cloudflare/roughtime/config" should now work for you on master branch. :) Feel-free to open another issue if other vendoring issues crop up. I'm going to contact the google folks to see if they can make their roughtime-library go-gettable so we can properly vendor it.

gabbifish commented 6 years ago

Update: I fixed vendoring such that importing from "roughtime.googlesource.com/roughtime.git/go/protocol" and "roughtime.googlesource.com/roughtime.git/go/config" should now work. Let me know if there are any other issues!

bluecmd commented 5 years ago 
[bluecmd]$ git diff .                                                                  [ttime] [14:39:49]
diff --git a/pkg/bmc/ttime/roughtime.go b/pkg/bmc/ttime/roughtime.go
index fd2a462..b65e702 100644
--- a/pkg/bmc/ttime/roughtime.go
+++ b/pkg/bmc/ttime/roughtime.go
@@ -11,9 +11,9 @@ import (
        "time"

        "github.com/beevik/ntp"
-       "github.com/u-root/u-bmc/pkg/roughtime"
-       "github.com/u-root/u-bmc/pkg/roughtime/upstream/config"
+       "github.com/cloudflare/roughtime"
        "golang.org/x/sync/errgroup"
+       "roughtime.googlesource.com/roughtime.git/go/config"
 )

 const (
[bluecmd]$ go get                                                                      [ttime] [14:39:51]
# github.com/u-root/u-bmc/pkg/bmc/ttime
./roughtime.go:52:29: cannot use srv (type *"roughtime.googlesource.com/roughtime.git/go/config".Server) as type *"github.com/cloudflare/roughtime/vendor/roughtime.googlesource.com/go/config".Server in argument to roughtime.Get

:( Any ideas?

bluecmd commented 5 years ago

Ping @gabbifish, not sure how visible a single comment is in a closed issue :)

gabbifish commented 5 years ago

Hi there! I'm so sorry for the delay. Have you re-pulled our repository since my last PR? Perhaps you're working on an old version of cloudflare/roughtime--the path github.com/cloudflare/roughtime/vendor/roughtime.googlesource.com/go/config should no longer exist in the most recent version of the cloudflare/roughtime repository. (It is now github.com/cloudflare/roughtime/vendor/roughtime.googlesource.com/roughtime.git/go/config/).

bluecmd commented 5 years ago

I apparently hadn't, which I do apologize for, but the error remains (slighty modified):

[bluecmd]$ git reflog
ba5e314 (HEAD -> master, origin/master, origin/HEAD) HEAD@{0}: pull --ff-only: Fast-forward
a00510d HEAD@{1}: clone: from https://github.com/cloudflare/roughtime
[bluecmd]$ pwd
/home/bluecmd/go/src/github.com/cloudflare/roughtime

Trying to build my thing:

[bluecmd]$ git diff
diff --git a/pkg/bmc/ttime/roughtime.go b/pkg/bmc/ttime/roughtime.go
index fd2a462..b65e702 100644
--- a/pkg/bmc/ttime/roughtime.go
+++ b/pkg/bmc/ttime/roughtime.go
@@ -11,9 +11,9 @@ import (
        "time"

        "github.com/beevik/ntp"
-       "github.com/u-root/u-bmc/pkg/roughtime"
-       "github.com/u-root/u-bmc/pkg/roughtime/upstream/config"
+       "github.com/cloudflare/roughtime"
        "golang.org/x/sync/errgroup"
+       "roughtime.googlesource.com/roughtime.git/go/config"
 )

 const (
[bluecmd]$ pwd
/home/bluecmd/go/src/github.com/u-root/u-bmc/pkg/bmc/ttime
[bluecmd]$ go get

# github.com/u-root/u-bmc/pkg/bmc/ttime
./roughtime.go:52:29: cannot use srv (type *"roughtime.googlesource.com/roughtime.git/go/config".Server) as type *"github.com/cloudflare/roughtime/vendor/roughtime.googlesource.com/roughtime.git/go/config".Server in argument to roughtime.Get

Using go1.11 FWIW.

Line in question is: https://github.com/u-root/u-bmc/blob/master/pkg/bmc/ttime/roughtime.go#L52

gabbifish commented 5 years ago

Huh, I just cloned your repo, added your changes, and go get-ed without issue. I'm also on go1.11:

Gabbis-Work-MacBook:ttime git diff
index b29e57c..0000000
diff --git a/pkg/bmc/ttime/roughtime.go b/pkg/bmc/ttime/roughtime.go
index fd2a462..b65e702 100644
--- a/pkg/bmc/ttime/roughtime.go
+++ b/pkg/bmc/ttime/roughtime.go
@@ -11,9 +11,9 @@ import (
        "time"

        "github.com/beevik/ntp"
-       "github.com/u-root/u-bmc/pkg/roughtime"
-       "github.com/u-root/u-bmc/pkg/roughtime/upstream/config"
+       "github.com/cloudflare/roughtime"
        "golang.org/x/sync/errgroup"
+       "roughtime.googlesource.com/roughtime.git/go/config"
 )

 const (
Gabbis-Work-MacBook:ttime gabbifisher$ go get
roughtime.go:14:2: found packages roughtime (client.go) and ttime (vendor.go) in /Users/gabbifisher/go/src/github.com/cloudflare/roughtime

The way your repository is vendored, it looks like you have dep installed. Just in case, have you tried running dep init in the u-root/u-bmc directory?

bluecmd commented 5 years ago

I disabled vendoring in my repository because another dependency doesn't work with vendoring.

Was the only thing you did a clean git clone and go get in that folder?

I'll try with a clean go src and get back to you. I appreciate the help here, I really do.

gabbifish commented 5 years ago

Yup! I did a clean git clone of u-root/u-bmc, applied your changes, and ran go get. Let me know if that works for you!

bluecmd commented 5 years ago

Hello again. I removed my go/src/ and did go get github.com/u-root/u-bmc/pkg/bmc/ttime. Same result for me again.

[bluecmd]$ find ~/go/src/ -maxdepth 2
/home/bluecmd/go/src/
/home/bluecmd/go/src/roughtime.googlesource.com
/home/bluecmd/go/src/roughtime.googlesource.com/roughtime.git
/home/bluecmd/go/src/github.com
/home/bluecmd/go/src/github.com/cloudflare
/home/bluecmd/go/src/github.com/u-root
/home/bluecmd/go/src/github.com/beevik
/home/bluecmd/go/src/golang.org
/home/bluecmd/go/src/golang.org/x
[bluecmd]$ git status --ignored                                                                                                                                                                   [ttime] [02:02:27]
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   roughtime.go

no changes added to commit (use "git add" and/or "git commit -a")
[bluecmd]$ pwd
/home/bluecmd/go/src/github.com/u-root/u-bmc/pkg/bmc/ttime
[bluecmd]$ go version
go version go1.11 linux/amd64
[bluecmd]$ go get
# github.com/u-root/u-bmc/pkg/bmc/ttime
./roughtime.go:52:29: cannot use srv (type *"roughtime.googlesource.com/roughtime.git/go/config".Server) as type *"github.com/cloudflare/roughtime/vendor/roughtime.googlesource.com/roughtime.git/go/config".Server in argument to roughtime.Get

I'm really scratching my head here that you're not seeing the same.

Edit:

[bluecmd]$ git diff
diff --git a/pkg/bmc/ttime/roughtime.go b/pkg/bmc/ttime/roughtime.go
index fd2a462..b65e702 100644
--- a/pkg/bmc/ttime/roughtime.go
+++ b/pkg/bmc/ttime/roughtime.go
@@ -11,9 +11,9 @@ import (
        "time"

        "github.com/beevik/ntp"
-       "github.com/u-root/u-bmc/pkg/roughtime"
-       "github.com/u-root/u-bmc/pkg/roughtime/upstream/config"
+       "github.com/cloudflare/roughtime"
        "golang.org/x/sync/errgroup"
+       "roughtime.googlesource.com/roughtime.git/go/config"
 )

 const (
gabbifish commented 5 years ago

I think I know what's going on... So, the way golang vendoring works is that only dependencies from the root vendoring directories are included. So if you're trying to pull roughtime.googlesource.com/roughtime.git/go/config from the vendor directory in the github.com/cloudflare/roughtime repository, golang will not look for it in github.com/cloudflare/roughtime/vendor.

Instead, golang will only look at u-bmc/vendor to find the dependency. Because roughtime.googlesource.com/roughtime.git/go/config is not in the u-bmc/vendor directory, the go compiler will not find this package.

The way to fix this is to add roughtime.googlesource.com/roughtime.git/go/config to your Gopkg.* to include it in your vendor directory.

Now, the reason my clone of u-root/u-bmc worked is because I had github.com/cloudflare/roughtime in my go/src directory, so the go compiler on my computer found the package there.

I checked that my suggested fix worked. I deleted my local src/github.com/cloudflare/roughtime directory and ran dep init in u-root/u-bmc. The google roughtime package was included in the root vendor directory, and go get worked :)

bluecmd commented 5 years ago

Right, and that will work when I can use vendoring - but due to a bug in u-root which is a dependency of u-bmc, I cannot use vendoring. Besides, isn't it a quite weird dependency to require vendoring of your clients? :/

bluecmd commented 5 years ago

@gabbifish Ping! Did you see my question above? My experience is that having vendored dependencies in libraries cause these kind of issues snowballing the need to vendor.

gabbifish commented 5 years ago

Hi there! Apologies for the delay, I was out of town for a conference. Hmm, I think fixing vendoring in u-bmc is important and would definitely solve the problem, but another thing I can do is see if your use case of roughtime.googlesource.com/roughtime.git/go/config is a feature we want to add to cloudflare/roughtime. If so, this could allow you to drop the import roughtime.googlesource.com/roughtime.git/go/config call and should fix the issue. I'll take a look into this.

bluecmd commented 5 years ago

Fwiw, the vendoring issue on my side is in a dependency of u-bmc, so that as well out of my control - they are working on fixing the issue, but unclear how long it will take. So I'm stuck between a rock and a hard place.