SPKI: Prototype TRC generation

[oncilla]

Add support for generating prototype TRCs based on the TRC configuration
files.

The version to generate can be provided via command line flag. If no
version is specified, the tool searches for the newest TRC configuration
file and uses its version.

This change also removes the old code for generating TRCs for a clean
slate approach.

---

This change is 

[scrye]

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

---

go/tools/scion-pki/internal/v2/trcs/loader.go, line 34 at r1 (raw file):

}

func (l loader) LoadConfigs(asMap map[addr.ISD][]addr.IA) (map[addr.ISD]conf.TRC2, error) {

Can we limit asMap to just a slice of addr.ISD? It's a bit confusing that so much information is being passed around without it being used.

---

go/tools/scion-pki/internal/v2/trcs/loader.go, line 62 at r1 (raw file):

	}
	re := regexp.MustCompile(`trc-v(\d*)\.toml$`)
	var max uint64

Please extract computation of scrypto.Version(max) to a function.

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 35 at r1 (raw file):

)

// signedMeta keeps track of the version.

"of the TRC version."

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 104 at r1 (raw file):

// loadPubKeys loads all public keys necessary for the given configuration.
func (g protoGen) loadPubKeys(isd addr.ISD,
	cfg conf.TRC2) (map[addr.AS]map[trc.KeyType]keyconf.Key, error) {

First return type is too complex. Defining map[trc.KeyType]keyconf.Key as a type would help a lot. It would also make the code below a lot more readable.

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 108 at r1 (raw file):

	keys := make(map[addr.AS]map[trc.KeyType]keyconf.Key)
	for as, primary := range cfg.PrimaryASes {
		keys[as] = make(map[trc.KeyType]keyconf.Key)

The function is a bit hard to follow at the moment. It would be nice if this big loop would look like:

for as, primary := range cfg.PrimaryASes {
   km := make(KeyMap)
   km[trc.IssuingKey], err = foo1()
   if err {
   }
   km[trc.OfflineKey], err = foo2()
   if err {
   }
   keys[as] = km

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 294 at r1 (raw file):

}

func sortedAttributes(attrs []trc.Attribute) []trc.Attribute {

sortedAttributes sounds like isSortedAttributes, which hints to a boolean return. It's also a noun which is awkward for a function.

sortAttributes would be clearer.

[oncilla]

Reviewable status: 31 of 36 files reviewed, 6 unresolved discussions (waiting on @oncilla and @scrye)

---

go/tools/scion-pki/internal/v2/trcs/loader.go, line 34 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Can we limit asMap to just a slice of addr.ISD? It's a bit confusing that so much information is being passed around without it being used.

Done.

---

go/tools/scion-pki/internal/v2/trcs/loader.go, line 62 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Please extract computation of scrypto.Version(max) to a function.

Done.

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 35 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

"of the TRC version."

Done.

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 104 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

First return type is too complex. Defining map[trc.KeyType]keyconf.Key as a type would help a lot. It would also make the code below a lot more readable.

Done.

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 108 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

The function is a bit hard to follow at the moment. It would be nice if this big loop would look like:

for as, primary := range cfg.PrimaryASes {
   km := make(KeyMap)
   km[trc.IssuingKey], err = foo1()
   if err {
   }
   km[trc.OfflineKey], err = foo2()
   if err {
   }
   keys[as] = km

I do not want zero values for non-issuing and non-voting ASes in the map.
I hope it is more readable now.

---

go/tools/scion-pki/internal/v2/trcs/prototype.go, line 294 at r1 (raw file):

Previously, scrye (Sergiu Costea) wrote…

sortedAttributes sounds like isSortedAttributes, which hints to a boolean return. It's also a noun which is awkward for a function.

sortAttributes would be clearer.

Done.

[scrye]

Reviewed 5 of 5 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @oncilla)

---

go/tools/scion-pki/internal/v2/trcs/loader.go, line 68 at r3 (raw file):

}

func (l loader) findMax(files []string) (scrypto.Version, error) {

Specify which Max in the name, e.g., findMaxVersion, because max, err := l.findMax(files) isn't very descriptive.

[oncilla]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @scrye)

---

go/tools/scion-pki/internal/v2/trcs/loader.go, line 68 at r3 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Specify which Max in the name, e.g., findMaxVersion, because max, err := l.findMax(files) isn't very descriptive.

Done.

[scrye]

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

[scrye]

Reviewed 1 of 1 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved