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

TrustStore: Generate certificates for tests #3493

Merged
merged 3 commits into from
Dec 9, 2019

Conversation

oncilla
Copy link
Contributor

@oncilla oncilla commented Dec 9, 2019

This change adds certificate generation for tests.


This change is Reviewable

@oncilla oncilla added the c/CPPKI SCION Control-plane PKI label Dec 9, 2019
@oncilla oncilla added this to the Q4S5.D milestone Dec 9, 2019
@oncilla oncilla requested a review from karampok December 9, 2019 08:57
Copy link
Contributor

@karampok karampok 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)

a discussion (no related file):

  1. Who/When should run this command? Why we cannot store them as testdata?
  2. Given I generated the crypto.tar that includes the new thing, which command should I run go test or bazel test, which part of the code do I test?


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 25 at r1 (raw file):

done

$1 v2 certs issuer -d $TMP "*-*" > /dev/null

Maybe one line comment which file is created after that command?


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 26 at r1 (raw file):

$1 v2 certs issuer -d $TMP "*-*" > /dev/null
$1 v2 certs chain -d $TMP "*-*" > /dev/null

ditto

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 @karampok)

a discussion (no related file):

Previously, karampok (Konstantinos) wrote…
  1. Who/When should run this command? Why we cannot store them as testdata?
  2. Given I generated the crypto.tar that includes the new thing, which command should I run go test or bazel test, which part of the code do I test?
  1. Bazel should run it when running the tests. (it already does) It cannot be committed, because the crypto material expires at some point.
  2. After this PR, an armada of pull requests will be opened that will use the new crypto. If you really want to "test" it, unpack the tar and do scion-pki v2 certs human path/to/tar/ISD*/AS*/certs/*.crt.


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 25 at r1 (raw file):

Previously, karampok (Konstantinos) wrote…

Maybe one line comment which file is created after that command?

Done.


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 26 at r1 (raw file):

Previously, karampok (Konstantinos) wrote…

ditto

Done.

Copy link
Contributor

@karampok karampok left a comment

Choose a reason for hiding this comment

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

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


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 5 at r2 (raw file):

# usage: gen_crypto_tar.sh <scion-pki> <output-file>
#
# This script is run by bazel to generate the testsdata for the trust store

is run maybe bazel runs this script.
For my knowledge where do I configure bazel to actually run the script? which bazel file?


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 7 at r2 (raw file):

# This script is run by bazel to generate the testsdata for the trust store
# tests. Crypto material needs to generate dynamically and cannot be commited
# to the tree because it expires. To use the regualr go toolchain, create the

We cannot put to expire in X years? and update as the golden special unit test?


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 18 at r2 (raw file):

# Generate config files for the default topology.
$1 v2 tmpl topo -d $TMP ./topology/Default.topo > /dev/null

(nit) I would have

set -euo pipefail
# doc at https://coderwall.com/p/fkfaqq/safer-bash-scripts-with-set-euxo-pipefail

PKIBIN=${1:-./bin/scion-pk}  
OUTDIR=${2:-../go/lib/infra/modules/trust/v2/testda}

PKIBIN v2 ....

tar -C TMP $OUTDIR

to enable just run the script without args

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 @karampok)


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 5 at r2 (raw file):

Previously, karampok (Konstantinos) wrote…

is run maybe bazel runs this script.
For my knowledge where do I configure bazel to actually run the script? which bazel file?

go/lib/infra/modules/trust/v2/testdata/BUILD.bazel:

cmd = "$(location :gen_crypto_tar.sh) $(location //go/tools/scion-pki:scion-pki) $@",

go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 7 at r2 (raw file):

Previously, karampok (Konstantinos) wrote…

We cannot put to expire in X years? and update as the golden special unit test?

because we will forget :)


go/lib/infra/modules/trust/v2/testdata/gen_crypto_tar.sh, line 18 at r2 (raw file):

Previously, karampok (Konstantinos) wrote…

(nit) I would have

set -euo pipefail
# doc at https://coderwall.com/p/fkfaqq/safer-bash-scripts-with-set-euxo-pipefail

PKIBIN=${1:-./bin/scion-pk}  
OUTDIR=${2:-../go/lib/infra/modules/trust/v2/testda}

PKIBIN v2 ....

tar -C TMP $OUTDIR

to enable just run the script without args

Done.

Copy link
Contributor

@karampok karampok left a comment

Choose a reason for hiding this comment

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

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

@oncilla oncilla merged commit d19b378 into scionproto:master Dec 9, 2019
@oncilla oncilla deleted the pub-trust-test-gen branch December 9, 2019 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/CPPKI SCION Control-plane PKI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants