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

Feature: opportunity to set TLS easily by default with ca-certs autogeneration if needed #71

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

Conversation

YarikRevich
Copy link

This PR partly solves issue(#69). It addes opportunity to easily set TLS settings using values.yaml file by default.

@vishnu-narayanan
Copy link
Member

@YarikRevich Thank you for the PR. Is this in a state where we can test this out?

@vishnu-narayanan
Copy link
Member

@YarikRevich are you still working on this?

@YarikRevich
Copy link
Author

Yes, I'm still working on this.

@YarikRevich YarikRevich changed the title WIP: opportunity to set TLS easily by default Feature: opportunity to set TLS easily by default Aug 10, 2022
@YarikRevich YarikRevich changed the title Feature: opportunity to set TLS easily by default Feature: opportunity to set TLS easily by default with ca-certs autogeneration if needed Aug 10, 2022
@YarikRevich
Copy link
Author

@vishnu-narayanan Please, test my PR out

@vishnu-narayanan
Copy link
Member

@YarikRevich Could you please share instructions on how I can test this out?

@YarikRevich
Copy link
Author

YarikRevich commented Aug 10, 2022

@vishnu-narayanan Sure, test is devidid into two parts. Before all use should set "ingress.enabled" to "true" in values.yaml.
1.(Test TLS with own certificates) Uncomment rows from 76 to 81 and then put to "ingress.tls[0].key" your own key and to "ingress.tls[0].crt" your own crt. Then run "helm install . --timeout 30m" being in charts directory. Afterwards, you'll be able to access resource with "https://chart-example.local/"
2.(Test TLS with certificate autogeneration) Uncomment rows from 76 to 81 and then delete "ingress.tls[0].key" and "ingress.tls[0].crt" items. If though one of them is not present, autogeneration will be used. Afterwards, you'll be able to access resource with "https://chart-example.local/" and you can check, that TLS certificate is generated by Kubernetes certificate generator

{{/*
Create chart name and version as used by the chart label.
*/}}
{{- define "custom-metrics.chart" -}}
Copy link
Member

Choose a reason for hiding this comment

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

custom-metrics?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, forgot to remove unused parts of configuration

@vishnu-narayanan
Copy link
Member

vishnu-narayanan commented Aug 11, 2022

@YarikRevich I am not able to test this.

  1. Are you expecting an ingress-controller to be present?
  2. At which point is the user expected to point DNS?

@YarikRevich
Copy link
Author

YarikRevich commented Aug 11, 2022

@vishnu-narayanan User should specify DNS name via values.yaml

hosts:
- host: "chart-example.local"
paths:
- path: /
pathType: Prefix
backend:
service:
name: "chatwoot"
port:
number: 3000
# It's highly recommended to use TLS protocol to secure
# your activities. Before specifying cert and key you should
# set ".Values.ingress.enabled" to "true"
# tls:
# - crt: "" # If either 'crt' or 'key' is not set, automatic ca-cert generation will be used
# key: ""
# label: "chart-example"
# hosts:
# - "chart-example.local"

@sojan-official
Copy link
Member

@vishnu-narayanan can we look into the next steps for this PR ?

@vishnu-narayanan
Copy link
Member

@YarikRevich Sorry for the delay here. This got left behind due to other priorites. Will pick this up once the team has enough bandwidth.

@sojan-official sojan-official added the community PRs from the community label Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community PRs from the community on-hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants