storage: pass encryption options to libroachccl.

[mberhault]

Release Note: none.

CCL-only per-store encryption options are serialized and passed to
libroach through DBOptions.

Upon DBOpen, a hook is run extracting the options in CCL code, doing
nothing in OSS code.
This will eventually initialize encryption-related objects.

A few other things:

• hide encryption flag, it's not ready for use
• generate pb.{cc,h} for CCL protobuf and store in separate path
• print out encryption flags and error out when passed

[cockroach-teamcity]

This change is 

[mberhault]

Should this be $(LIBROACH_SRC_DIR)/ccl/protos instead? There's no reason to put it in the base directory.

[mberhault]

either way it kind of stutters. We can have protosccl/ccl/... or ccl/protos/ccl/.... I'm not sure I care enough about generated files though.

[mberhault]

eep! I'm still playing with hooks. I'd rather avoid unsafe if I can.

[mberhault]

I think we may want to split CCL protos out of GO_PROTOS, otherwise we're building with them even in OSS code.

[mberhault]

Removed the stdout logging for all but the "found extra options" when encryption flags are passed.
Made the flags hidden and DBOpen fails when present with "encryption not implemented" (after printing the parsed values).

[mberhault]

This is ready for review.

[bdarnell]

---

Reviewed 10 of 17 files at r1, 1 of 6 files at r3, 5 of 5 files at r4.
Review status: all files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.

---

c-deps/libroach/db.cc, line 49 at r4 (raw file):

// OpenHook does nothing in OSS mode.
__attribute__((weak)) DBStatus OpenHook(const DBOptions opts) { return kSuccess; }

s/OpenHook/DBOpenHook/? (or dbOpenHook to match the lowerCase names below)

Should this fail if extra_options is non-empty?

---

pkg/ccl/baseccl/encryption_options.proto, line 33 at r4 (raw file):

// EncryptionOptions defines the per-store encryption options.
message EncryptionOptions {

Unless EncryptionOptions are the only CCL options we ever have that need this treatment, the use of a field named ExtraOptions to pass EncryptionOptions is awkward. I would put a more generic ExtraOptions protobuf around EncryptionOptions to give us space for other CCL options in the future.

---

Comments from Reviewable

[mberhault]

Review status: all files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.

---

c-deps/libroach/db.cc, line 49 at r4 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

s/OpenHook/DBOpenHook/? (or dbOpenHook to match the lowerCase names below)

Should this fail if extra_options is non-empty?

Renamed to DBOpenHook to match the DBOpen function.

I prefer to have the OSS code be blissfully ignorant of extras_options. In the encryption case, we have plenty of safeguards already (encryption flag doesn't exist, registry will have unknown file type).

---

pkg/ccl/baseccl/encryption_options.proto, line 33 at r4 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Unless EncryptionOptions are the only CCL options we ever have that need this treatment, the use of a field named ExtraOptions to pass EncryptionOptions is awkward. I would put a more generic ExtraOptions protobuf around EncryptionOptions to give us space for other CCL options in the future.

I started out that way but adding a whole extra message we don't need seemed premature.
Since this is only used at runtime to communicate between go/cgo and not persisted or used across processes, we can add a new message that contains the field EncryptionOptions encryption_options anytime we like.

---

Comments from Reviewable

[mberhault]

Review status: 10 of 16 files reviewed at latest revision, 5 unresolved discussions.

---

c-deps/libroach/db.cc, line 49 at r4 (raw file):

Previously, mberhault (marc) wrote…

Renamed to DBOpenHook to match the DBOpen function.

I prefer to have the OSS code be blissfully ignorant of extras_options. In the encryption case, we have plenty of safeguards already (encryption flag doesn't exist, registry will have unknown file type).

Nevermind, erroring out just in case someone tries to pass them through. You're right, the extra check doesn't hurt.
I'll add tests for this once #20594 lands.

---

Comments from Reviewable

[bdarnell]

Review status: 10 of 16 files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.

---

pkg/ccl/baseccl/encryption_options.proto, line 33 at r4 (raw file):

Previously, mberhault (marc) wrote…

I started out that way but adding a whole extra message we don't need seemed premature.
Since this is only used at runtime to communicate between go/cgo and not persisted or used across processes, we can add a new message that contains the field EncryptionOptions encryption_options anytime we like.

Ack, I forgot that this was only used in-process so it can be changed freely.

---

Comments from Reviewable