SPKI: Add key configuration

[oncilla]

Adds:

• support for key configuration parsing.
• key template generation from topo file

---

This change is 

[scrye]

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @oncilla)

---

go/tools/scion-pki/internal/v2/conf/key.go, line 58 at r1 (raw file):

}

func (k Keys) Write(file string) error {

Missing docstring.

---

go/tools/scion-pki/internal/v2/conf/key.go, line 121 at r1 (raw file):

// keysMarshaler is used for toml encoding and decoding because the library only
// allows string has map keys.

"string has map keys" -> "string map keys".

---

go/tools/scion-pki/internal/v2/conf/key.go, line 122 at r1 (raw file):

// keysMarshaler is used for toml encoding and decoding because the library only
// allows string has map keys.
type keysMarshaler struct {

I would love to see some golden files tests for these, because they serve both as (1) documentation of file structure, (2) regression protection and (3) I can play around with edge cases during code review (if I suspect one might cause issues).

---

go/tools/scion-pki/internal/v2/conf/key.go, line 122 at r1 (raw file):

// keysMarshaler is used for toml encoding and decoding because the library only
// allows string has map keys.
type keysMarshaler struct {

Maybe rename to tomlKeys? keysMarshaler sounds like it implements encoding.Marshaler, but it's just an adapter struct.

---

go/tools/scion-pki/internal/v2/tmpl/keys.go, line 26 at r1 (raw file):

)

func genKeysTmpl(ia addr.IA, val conf.Validity, isd *conf.ISDCfg) conf.Keys {

This would also benefit from being in a golden file test, because it'll serve as documentation for a templated file in human readable format.

Maybe it can be included/called in the blackbox test suite for the marshaler?

If it's awkward/hard, don't bother, though.

[oncilla]

Reviewable status: 5 of 12 files reviewed, 5 unresolved discussions (waiting on @scrye)

---

go/tools/scion-pki/internal/v2/conf/key.go, line 58 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Missing docstring.

Done.

---

go/tools/scion-pki/internal/v2/conf/key.go, line 121 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

"string has map keys" -> "string map keys".

Done.

---

go/tools/scion-pki/internal/v2/conf/key.go, line 122 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Maybe rename to tomlKeys? keysMarshaler sounds like it implements encoding.Marshaler, but it's just an adapter struct.

Done.

---

go/tools/scion-pki/internal/v2/conf/key.go, line 122 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

I would love to see some golden files tests for these, because they serve both as (1) documentation of file structure, (2) regression protection and (3) I can play around with edge cases during code review (if I suspect one might cause issues).

Done.

---

go/tools/scion-pki/internal/v2/tmpl/keys.go, line 26 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

This would also benefit from being in a golden file test, because it'll serve as documentation for a templated file in human readable format.

Maybe it can be included/called in the blackbox test suite for the marshaler?

If it's awkward/hard, don't bother, though.

tbh, I don't think this is worth testing.
The only test I see here would be does the keys struct actually contain what is set here.
Essentially, you would write this function twice, and check that the result matches.

I'm not super enthusiastic about the current templating approach supported in v1 and not really sure where we go with templating.
For now, I just want to support templates from topo files to allow using scion-pki in the generator.

We can invest time in more full fledged templating, when we have the other features in order.

[scrye]

Reviewed 7 of 7 files at r2, 2 of 2 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @oncilla)

---

go/tools/scion-pki/internal/v2/conf/key.go, line 57 at r2 (raw file):

}

// Encode writes the encoded keys config to the writter

s/writer/writer. Also add period at the end.

---

go/tools/scion-pki/internal/v2/conf/key_test.go, line 28 at r2 (raw file):

	"github.com/scionproto/scion/go/tools/scion-pki/internal/v2/conf/testdata"
)

The golden file pattern can be streamlined a bit more to make it easily maintainable:

var update = flag.Bool("update", false, "set to true to regenerate golden files")

func TestGolden(t *testing.T) {
	var buf bytes.Buffer
	err := testdata.GoldenKeys.Encode(&buf)
	require.NoError(err)

	if *update {
		err := ioutil.WriteFile("testdata/keys.toml", buf.Bytes(), 0644)
		require.NoError(t, err)
	}

	// Checks are the same, raw bytes and object equality
}

---

go/tools/scion-pki/internal/v2/conf/key_test.go, line 37 at r2 (raw file):

func TestKeysEncode(t *testing.T) {
	var buf bytes.Buffer
	testdata.GoldenKeys.Encode(&buf)

Missing error check.

---

go/tools/scion-pki/internal/v2/tmpl/keys.go, line 26 at r1 (raw file):

Previously, Oncilla wrote…

tbh, I don't think this is worth testing.
The only test I see here would be does the keys struct actually contain what is set here.
Essentially, you would write this function twice, and check that the result matches.

I'm not super enthusiastic about the current templating approach supported in v1 and not really sure where we go with templating.
For now, I just want to support templates from topo files to allow using scion-pki in the generator.

We can invest time in more full fledged templating, when we have the other features in order.

Ack, sounds reasonable.

[oncilla]

Reviewable status: 10 of 12 files reviewed, 3 unresolved discussions (waiting on @oncilla and @scrye)

---

go/tools/scion-pki/internal/v2/conf/key_test.go, line 28 at r2 (raw file):

Previously, scrye (Sergiu Costea) wrote…

The golden file pattern can be streamlined a bit more to make it easily maintainable:

var update = flag.Bool("update", false, "set to true to regenerate golden files")

func TestGolden(t *testing.T) {
	var buf bytes.Buffer
	err := testdata.GoldenKeys.Encode(&buf)
	require.NoError(err)

	if *update {
		err := ioutil.WriteFile("testdata/keys.toml", buf.Bytes(), 0644)
		require.NoError(t, err)
	}

	// Checks are the same, raw bytes and object equality
}

Done.

---

go/tools/scion-pki/internal/v2/conf/key_test.go, line 37 at r2 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Missing error check.

Done.

[scrye]

Reviewed 2 of 2 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved