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

Fix handling of HTTP CA #1742

Merged
merged 4 commits into from
Sep 20, 2019
Merged

Conversation

charith-elastic
Copy link
Contributor

More details can be found in #1614.
Briefly, this PR does the following:

  • Treat the presence of ca.crt key in the secret as a signal to override the appropriate *.certificate_authorities setting of the associated stack application.
  • Add support for PKCS#8 encoded private keys.
  • Update the documentation to clarify how custom certificates can be used.
  • Update/add unit tests

Fixes #1614

Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

LGTM! Maybe add tests for the private key support?

return nil, fmt.Errorf("expected an RSA private key but got %t", key)
}

return rsaKey, nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Am I mistaken or are we missing unit tests for this? Maybe an opportunity to add one (or two)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I didn't add any unit tests is because these functions are just thin wrappers over standard library functions. Happy to add tests if you think differently.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK

Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

LGTM

docs/accessing-services.asciidoc Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v1.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve handling public CA for ES certificate in Kibana
3 participants