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

feat: support for trusting private CA #139

Merged

Conversation

ricardoapl
Copy link
Contributor

Resolves #138

@k3rnelpan1c-dev k3rnelpan1c-dev added the enhancement New feature or request label Sep 17, 2023
@k3rnelpan1c-dev
Copy link
Owner

hmm, I am never sure if I like this approach or building a minimal custom image with the CA included.
There are advantages and disadvantages for both, but I will look into your PR in detail as soon as I can.

Copy link
Owner

@k3rnelpan1c-dev k3rnelpan1c-dev left a comment

Choose a reason for hiding this comment

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

overall, looks good to me.

Since Node, unlike Java, can directly read extra CAs directly without the need of updating a trust-store, I see nothing wrong with doing this.

reasoning for the comment
I have wasted too many hours at work with our self signed CA and the ppl behind it that I am very cautious when it comes to this "feature"
that and quite a few other "official" charts with Java backed services use init container ... which is a wast of time and resources when you scale up and down frequently.
Thereby I generally advocate for building a custom image with the proper trust in cases where you need to change/patch anything to the likes of a trust-store, that not being the case here, I approve of this method.

Thank you for your contribution and sorry for the odd review comment.

uptime-kuma/readme.adoc Show resolved Hide resolved
@k3rnelpan1c-dev k3rnelpan1c-dev merged commit 5d13f00 into k3rnelpan1c-dev:main Sep 24, 2023
@ricardoapl
Copy link
Contributor Author

Thank you for taking the time to look into this pull request, I really appreciate it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support self-signed certificates
2 participants