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 support for mutual authentication via TLS #146

Merged
merged 5 commits into from
Aug 9, 2024

Conversation

shearn89
Copy link
Contributor

Prior to this change, there was no way to pass in a client certificate
if you had set up Nexus behind a load balancer with mutual auth
configured.

This change exposes some new configuration values that allow you to pass
in a path to a file on disk containing your client key/cert (which can
be in a single file) and optionally a Root CA.

shearn89 added 3 commits July 25, 2024 10:43
Prior to this change, there was no way to pass in a client certificate
if you had set up Nexus behind a load balancer with mutual auth
configured.

This change exposes some new configuration values that allow you to pass
in a path to a file on disk containing your client key/cert (which can
be in a single file) and optionally a Root CA.
@shearn89
Copy link
Contributor Author

One thing I noted was the lack of a logging library across the client - I didn't want to introduce one, but would have liked to add some helpful error messages for when files can't be found etc. If that's desirable, let me know.

Copy link
Member

@anmoel anmoel left a comment

Choose a reason for hiding this comment

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

lgtm
lgtm

@anmoel
Copy link
Member

anmoel commented Aug 8, 2024

@shearn89 please rebase your fork, so the tests are working again

@anmoel anmoel self-requested a review August 8, 2024 09:16
nexus3/pkg/client/client.go Outdated Show resolved Hide resolved
nexus3/pkg/client/client.go Outdated Show resolved Hide resolved
@shearn89
Copy link
Contributor Author

shearn89 commented Aug 9, 2024

Couldn't get Docker to behave on my laptop so haven't been able to run the full test suite, but the client test passes. If needed I can rebase/squash etc before merge - let me know if there's any further changes needed! @anmoel

Copy link
Member

@anmoel anmoel left a comment

Choose a reason for hiding this comment

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

lgtm
lgtm

@anmoel anmoel merged commit 1da3496 into datadrivers:main Aug 9, 2024
5 checks passed
@anmoel
Copy link
Member

anmoel commented Aug 9, 2024

@shearn89 do you need this only in the go client or also in the terraform provider?

@shearn89
Copy link
Contributor Author

shearn89 commented Aug 9, 2024

I'll need it in the TF provider but I have an internal fork that I used to avoid waiting on PRs, so I'll clean that up and raise a PR in that repo shortly. Thanks!

@shearn89 shearn89 deleted the feature/support-mtls branch August 9, 2024 14:37
@anmoel
Copy link
Member

anmoel commented Aug 9, 2024

@shearn89 release v1.12.0 deployed

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