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

rpcserver: Adds ability to allow alternative dns names for TLS. #1476

Merged
merged 2 commits into from
Oct 11, 2018

Conversation

logicminds
Copy link
Contributor

  • Adds AltDNSNames config for passing additional hostnames to the
    TLS certificate generate method. Without this change
    there is no way to pass in alternative dns names for the rpc
    server.

  • Additionally this commit allows the user to set 'DCRD_ALT_DNSNAMES'
    as an environment variable when starting dcrd. This is useful for
    docker and other app runtimes that require apps to be 12 factor.

@davecgh
Copy link
Member

davecgh commented Oct 4, 2018

A quick note that this is failing CI due to a duplicate test definition.

@logicminds logicminds force-pushed the altdnsnames branch 3 times, most recently from 5cf14a9 to 1243a37 Compare October 4, 2018 05:47
@davecgh davecgh added this to the 1.4.0 milestone Oct 8, 2018
@@ -106,6 +110,12 @@ var primaryHarness *rpctest.Harness
func TestMain(m *testing.M) {
var err error

// Parse the -test.* flags before removing them from the command line
// arguments list, which we do to allow go-flags to succeed.
// See config_test.rb for more info
Copy link
Member

Choose a reason for hiding this comment

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

Probably .go instead of .rb.

@@ -115,7 +125,6 @@ func TestMain(m *testing.M) {
fmt.Println("unable to create primary harness: ", err)
os.Exit(1)
}

Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove this new line.

config_test.go Outdated
// arguments list, which we do to allow go-flags to succeed.
flag.Parse()
os.Args = os.Args[:1]
// Run the tests now that the testing package flags have been parsed.
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a stray comment.

config_test.go Show resolved Hide resolved
config_test.go Show resolved Hide resolved
defer os.Remove(certfile.Name())
err := genCertPair(certfile.Name(), keyfile.Name(), hostnames)
if err != nil {
t.Fatalf("certifcate was not created correctly")
Copy link
Member

Choose a reason for hiding this comment

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

Would be helpful to include the error here, so if it fails, we'll know why.

rpcserver_test.go Show resolved Hide resolved
@@ -165,3 +174,36 @@ func TestRpcServer(t *testing.T) {
currentTestNum++
}
}

func TestCertCreationWithHosts(t *testing.T) {
certfile, _ := ioutil.TempFile("", "certfile")
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest checking errors using t.Fatalf to display them and fail the test if non nil. Otherwise, it will be difficulty to figure out why the test is failing.

Same goes for the rest.

  * Adds AltDNSNames config for passing additional hostnames to the
    TLS certificate generate method. Without this change
    there is no way to pass in alternative dns names for the rpc
    server.

  * Additionally this commit allows the user to set 'DCRD_ALT_DNSNAMES'
    as an environment variable when starting dcrd.  This is useful for
    docker and other app runtimes that require apps to be 12 factor.
@davecgh davecgh merged commit ea54d03 into decred:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants