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 http client and tlsSecretRef #25

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Avarei
Copy link

@Avarei Avarei commented Jun 6, 2024

Description of your changes

Add http.Client to httpClient.
add mTLS support to CRDs using new tlsSecretRef field which expects a secret containing tls.crt, tls.key, and/or ca.crt

Fixes #21

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

  • added test in go
  • ran disposablerequest/v1alpha2 with and without tlsSecretRef, with and without ca.crt in the referenced secret. with and without insecureSkipTlsVerify set
  • I did NOT test with Request and DisposableRequest/v1alpha1

@Avarei
Copy link
Author

Avarei commented Jun 6, 2024

still need to redo all tests

@barunavo
Copy link
Contributor

barunavo commented Sep 10, 2024

@Avarei : Thanks for this PR, I am waiting for this too. I need it to have possibilities to talk to internal API using Certificate authentication.

Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com>
Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com>
Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com>
Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com>
Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com>
@Avarei
Copy link
Author

Avarei commented Sep 15, 2024

thanks @barunavo for bringing this back to my attention.
I've rebased the changes and added some tests for newClient and the mTLS functionality.
Im still quite new to writing tests so any feedback would be appreciated.

I neeeded to update golangci-lint since the previous version seemed to leak memory on every system I tried to run it.

I also tried this functionality in a cluster using disposablerequests and I could not find any issues so far.

I have not yet used the non-disposable requests and I hope that someone can take a look at them.

@Avarei Avarei marked this pull request as ready for review September 15, 2024 13:03
Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com>
@barunavo
Copy link
Contributor

barunavo commented Sep 16, 2024

Thanks for the PR @Avarei , may be I will try to take care of the non-disposable request, not sure when I find time.

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.

Allow setting TLSClientConfig
2 participants