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.

Sign up for GitHub

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

Jump to bottom

feat(istio): add configurable TLS version #624

Merged
merged 1 commit into from
Aug 1, 2024
Merged

feat(istio): add configurable TLS version #624

merged 1 commit into from
Aug 1, 2024
+4 −1

Conversation

jamestexas
Copy link
Contributor

@jamestexas jamestexas commented Jul 31, 2024 •
edited
Loading

Description

  • Introduced support for configuring the minimum TLS version for UDS Core.
  • Added supportTLSV1_2 boolean flag in values file to allow selection between TLS 1.2 and TLS 1.3.
  • Updated Helm templates to conditionally set minProtocolVersion based on supportTLSV1_2 value.
  • Ensured backward compatibility by defaulting to TLS 1.3.
  • Commented and documented the new configuration option for clarity.

Related Issue

Fixes #599

Relates to #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@jamestexas jamestexas force-pushed the feature/tls-1.2-support-599 branch 2 times, most recently from d5a0ee2 to d67d022 Compare July 31, 2024 22:20
@jamestexas
Copy link
Contributor Author

I originally ran into issues with this (before using the ternary) where the lack of tls.supportTLSV1_2 define made it error out when attempting to deploy it. This change added logic to where "if supportTLSV1_2 is both defined and true" it'll use TLS 1.2. It being set as false (like it is here) retains the existing behavior of defaulting to 1.3, and I feel it makes it self documenting. Let me know if it's preferred to remove it.

Additionally I wasn't sure if more tests need-be added here, but if they do let me know!

@jamestexas jamestexas marked this pull request as ready for review July 31, 2024 22:46
mjnagel
mjnagel reviewed Aug 1, 2024
View reviewed changes
src/istio/chart/templates/gateway.yaml Outdated Show resolved Hide resolved
src/istio/values/config-passthrough.yaml Outdated Show resolved Hide resolved
src/istio/values/config-admin.yaml Outdated Show resolved Hide resolved
@jamestexas
feat(istio): add configurable TLS version
f80d4b8 
- Introduced support for configuring the minimum TLS version for UDS Core.
- Added 'supportTLSV1_2' boolean flag in values file to allow selection between TLS 1.2 and TLS 1.3.
- Updated Helm templates to conditionally set 'minProtocolVersion' based on 'supportTLSV1_2' value.
- Ensured backward compatibility by defaulting to TLS 1.3.
- Commented and documented the new configuration option for clarity.

Closes #599

Signed-off-by: jamestexas <jamestexasgardner@gmail.com>
@jamestexas jamestexas force-pushed the feature/tls-1.2-support-599 branch from d67d022 to f80d4b8 Compare August 1, 2024 16:52
@jamestexas jamestexas requested a review from mjnagel August 1, 2024 17:13
mjnagel
mjnagel approved these changes Aug 1, 2024
View reviewed changes
Copy link
Contributor

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for the contribution @jamestexas !

@mjnagel mjnagel merged commit cd2b87e into main Aug 1, 2024
17 checks passed
@mjnagel mjnagel deleted the feature/tls-1.2-support-599 branch August 1, 2024 18:17
@github-actions github-actions bot mentioned this pull request Aug 1, 2024
Merged
mjnagel pushed a commit that referenced this pull request Aug 2, 2024
@github-actions
chore(main): release 0.25.0 (#595)
6b1f89b 
🤖 I have created a release *beep* *boop*
---


##
[0.25.0](v0.24.1...v0.25.0)
(2024-08-02)


### ⚠ BREAKING CHANGES

* change metric server to optional
(#611)

### Features

* add json logging for keycloak
([#610](#610))
([29ed934](29ed934))
* **istio:** add configurable TLS version
([#624](#624))
([cd2b87e](cd2b87e))


### Bug Fixes

* account for keycloak HA ports
([#619](#619))
([434f349](434f349))
* add google saml to slim-dev
([#613](#613))
([f2164e1](f2164e1))
* address network policy generation inter-namespace bug
([#564](#564))
([9b14c2c](9b14c2c))
* reference root scope
([#633](#633))
([5de6915](5de6915))


### Miscellaneous

* change metric server to optional
(#611)
([bc2d673](bc2d673))
* **deps:** update dependency defenseunicorns/uds-common to v0.9.0
([#592](#592))
([44ea2d7](44ea2d7))
* **deps:** update dependency weaveworks/eksctl to v0.187.0
([#539](#539))
([9002a94](9002a94))
* **deps:** update githubactions
([#553](#553))
([2a9e29a](2a9e29a))
* **deps:** update grafana curl image to v8.9.0
([#596](#596))
([64f9408](64f9408))
* **deps:** update grafana helm chart to v8.3.6
([#594](#594))
([1f2005b](1f2005b))
* **deps:** update istio to v1.22.3
([#580](#580))
([7aba89e](7aba89e))
* **deps:** update lula to v0.4.4
([#615](#615))
([b02b305](b02b305))
* **deps:** update neuvector-updater/curl to v8.9.0
([#597](#597))
([b4bd660](b4bd660))
* **deps:** update promtail configmap-reload to v0.13.1
([#608](#608))
([d98bbae](d98bbae))
* **deps:** update promtail helm chart to v6.16.4
([#574](#574))
([bf9f65c](bf9f65c))
* **deps:** update to identity-config 0.5.2
([#635](#635))
([6474d16](6474d16))
* **deps:** update uds cli to v0.13.1
([#569](#569))
([4339c89](4339c89))
* **deps:** update zarf to v0.36.1
([#562](#562))
([058cfb3](058cfb3))
* disable telemetry/analytics for loki/grafana
([#601](#601))
([ad785bc](ad785bc))
* update zarf to new repo location, 0.37.0
([#631](#631))
([29f9fd0](29f9fd0))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjnagel mjnagel mjnagel approved these changes

@zachariahmiller zachariahmiller Awaiting requested review from zachariahmiller

@UnicornChance UnicornChance Awaiting requested review from UnicornChance

@rjferguson21 rjferguson21 Awaiting requested review from rjferguson21

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ability to configure UDS Core to accept TLS 1.2
2 participants
@jamestexas @mjnagel