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

SPKI: Generate issuer and AS config for topo #3363

Merged
merged 2 commits into from
Nov 13, 2019

Conversation

oncilla
Copy link
Contributor

@oncilla oncilla commented Nov 12, 2019

This PR adds the capability to generate issuer and AS certificate
config files for a given topology description.

Additionally, adds missing tests for generated keys.toml.


This change is Reviewable

@oncilla oncilla added the c/tooling SCION network tools label Nov 12, 2019
@oncilla oncilla added this to the Q4S3.D milestone Nov 12, 2019
Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

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


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

}

func (g topoGen) genIssuerCerts(topo topoFile, cfg map[addr.ISD]conf.TRC2) error {

cfg argument is not used. It could maybe be used to determine the TRCVersion for the issuer cert. Else just drop it.


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

}

func (g topoGen) genASCerts(topo topoFile, cfg map[addr.ISD]conf.TRC2) error {

cfg argument is not used.


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

				assert.Equal(t, algo, meta.Algorithm)
				assert.Equal(t, g.Validity, meta.Validity)

extra empty line. I'd move it one line below.

Copy link
Contributor Author

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @lukedirtwalker)


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

Previously, lukedirtwalker (Lukas Vogel) wrote…

cfg argument is not used. It could maybe be used to determine the TRCVersion for the issuer cert. Else just drop it.

That was the plan initially, but then I dropped it because it is always 1 anyway.


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

Previously, lukedirtwalker (Lukas Vogel) wrote…

cfg argument is not used.

Done.


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

Previously, lukedirtwalker (Lukas Vogel) wrote…

extra empty line. I'd move it one line below.

Done.

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

:lgtm:

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

This PR adds the capability to generate issuer and AS certificate
config files for a given topology description.

Additionally, adds missing tests for generated keys.toml.
@oncilla oncilla merged commit 825268a into scionproto:master Nov 13, 2019
@oncilla oncilla deleted the pub-spki-tmpl-cert branch November 13, 2019 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/tooling SCION network tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants