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

FoundationDB backend TLS support and housekeeping #5800

Merged
merged 6 commits into from
Jan 8, 2019

Conversation

jblache
Copy link
Contributor

@jblache jblache commented Nov 15, 2018

TLS support was added to FoundationDB in 5.2.4; this PR adds the necessary bits for TLS support to the FoundationDB Vault backend, along with documentation updates.

Some housekeeping is included for good measure. Of note, the FoundationDB Go bindings got vendored in an update, which breaks the build and doesn't work in practice. Revert that and add the proper ignores.

FoundationDB bindings are tightly coupled to the server version and
client library version used in a specific deployment. Bindings need
to be installed using the fdb-go-install.sh script, as documented in
the foundationdb backend documentation.

err := opts.SetTLSCaPath(tlsCAFile)
if err != nil {
return nil, errwrap.Wrapf("failed to set TLS CA bundble path: {{err}}", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, errwrap.Wrapf("failed to set TLS CA bundble path: {{err}}", err)
return nil, errwrap.Wrapf("failed to set TLS CA bundle path: {{err}}", err)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Fixed :)

TLS support appeared in FoundationDB 5.2.4, raising the minimum API version
for TLS-aware FoundationDB code to 520.
@briankassouf briankassouf added this to the 1.1 milestone Dec 12, 2018
@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Dec 20, 2018
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

@jblache thank you for working on this! The code looks fantastic. Just need to figure out the right way to go about vendoring.

tlsKeyFile, hasKeyFile := conf["tls_key_file"]
tlsCAFile, hasCAFile := conf["tls_ca_file"]

tlsEnabled := hasCertFile && hasKeyFile && hasCAFile
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the clarity with which you named this variable!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! 🎩

@@ -1,6 +1,6 @@
{
"comment": "",
"ignore": "test",
"ignore": "test github.com/apple/foundationdb/bindings/go/src/fdb/",
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks Dec 20, 2018

Choose a reason for hiding this comment

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

Something about the vendoring changes doesn't seem quite right. I notice that none of the other test dependencies are listed in this line, only below in this area. Also, if I test, for instance, CockroachDB on this branch, I get:

=== RUN   TestCockroachDBBackend
--- PASS: TestCockroachDBBackend (29.67s)
PASS

But if I try to test this db, I get:

foundationdb.go:20:2: cannot find package "github.com/apple/foundationdb/bindings/go/src/fdb/directory" in any of:
	/home/tbex/go/src/github.com/jblache/vault/vendor/github.com/apple/foundationdb/bindings/go/src/fdb/directory (vendor tree)
	/usr/local/go/src/github.com/apple/foundationdb/bindings/go/src/fdb/directory (from $GOROOT)
	/home/tbex/go/src/github.com/apple/foundationdb/bindings/go/src/fdb/directory (from $GOPATH)
foundationdb.go:21:2: cannot find package "github.com/apple/foundationdb/bindings/go/src/fdb/subspace" in any of:
	/home/tbex/go/src/github.com/jblache/vault/vendor/github.com/apple/foundationdb/bindings/go/src/fdb/subspace (vendor tree)
	/usr/local/go/src/github.com/apple/foundationdb/bindings/go/src/fdb/subspace (from $GOROOT)
	/home/tbex/go/src/github.com/apple/foundationdb/bindings/go/src/fdb/subspace (from $GOPATH)
foundationdb.go:22:2: cannot find package "github.com/apple/foundationdb/bindings/go/src/fdb/tuple" in any of:
	/home/tbex/go/src/github.com/jblache/vault/vendor/github.com/apple/foundationdb/bindings/go/src/fdb/tuple (vendor tree)
	/usr/local/go/src/github.com/apple/foundationdb/bindings/go/src/fdb/tuple (from $GOROOT)
	/home/tbex/go/src/github.com/apple/foundationdb/bindings/go/src/fdb/tuple (from $GOPATH)

I do notice that some of Cockroach DB's files are currently in the vendor directory so I'm wondering if fully stripping them out for foundationdb is the right way to solve the build issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The FoundationDB-related code won't build without installing the bindings first, using the fdb-go-install.sh script. The bindings cannot be vendored for several reasons:

  • they need generated code that isn't checked into git;
  • they need the client library and the versions need to match;
  • the version of the bindings to build with needs to be carefully selected depending on your particular installation, due to features availability and stepped-upgrade requirements.

In short, there is no one-size-fits-all, but, if you follow the backend's documentation, a simple invocation of the fdb-go-install.sh script will get you going for development and test purposes. For production deployments, more FoundationDB knowledge is required, and hopefully I've documented enough of that to at least get folks started on the right path.

In the current state of master, anyone trying to build Vault with FoundationDB will be met with an arcane build failure that they may not be able to get themselves out of :(

@chrishoffman chrishoffman modified the milestones: 1.1, 1.0.2 Jan 7, 2019
Copy link
Contributor

@kalafut kalafut left a comment

Choose a reason for hiding this comment

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

LGTM

@kalafut kalafut merged commit 29471c8 into hashicorp:master Jan 8, 2019
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.

5 participants