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

Add TLS and mTLS #97

Merged
merged 2 commits into from
Aug 6, 2020
Merged

Add TLS and mTLS #97

merged 2 commits into from
Aug 6, 2020

Conversation

jasonodonnell
Copy link
Contributor

@jasonodonnell jasonodonnell commented Aug 5, 2020

Currently the action does not support TLS or mTLS. This adds new parameters to support these features:

Parameter Default Description
caCertificate null Base64 encoded CA certificate the server certificate was signed with.
clientCertificate null Base64 encoded client certificate the action uses to authenticate with Vault when mTLS is enabled.
clientKey null Base64 encoded client key the action uses to authenticate with Vault when mTLS is enabled.
tlsSkipVerify false When set to true, disables verification of server certificates when testing the action.

This current implementation expects that the client key is not password protected. This could be added in the future if needed.

I made some other tweaks while I was in here:

  • Expanded and organized docker-compose.yaml for the various tests
  • Updated test formatting
  • Added TLS testing infrastructure. The cert/keys added to this repo are valid for 100 years.
  • Set secrets on the repo that contain the certificate/key tests will use.
  • Updated got to 11.5.1. This is required for the custom TLS settings.

All of these tests are run when a pull request is opened. See the checks below for the specific test outputs.

@jasonodonnell jasonodonnell force-pushed the tls branch 2 times, most recently from 8fdfc0d to f3483ab Compare August 5, 2020 19:49
@jasonodonnell jasonodonnell marked this pull request as ready for review August 5, 2020 20:36
Copy link
Contributor

@tomhjp tomhjp left a comment

Choose a reason for hiding this comment

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

Nice 👍 LGTM, although my JavaScript knowledge is pretty shallow.

src/action.js Outdated Show resolved Hide resolved
src/action.js Outdated Show resolved Hide resolved
Copy link

@Valarissa Valarissa left a comment

Choose a reason for hiding this comment

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

I enjoy how deliciously go-like this JS is, lol. LGTM.

@jasonodonnell jasonodonnell merged commit b3a0228 into master Aug 6, 2020
@jasonodonnell jasonodonnell deleted the tls branch August 6, 2020 18:30
@RichiCoder1
Copy link
Contributor

Whoop! Nice.

I enjoy how deliciously go-like this JS is, lol. LGTM.

True to this action's history of being a mish-mash of language and pattern influences 😁

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.

4 participants