Skip to content

Conversation

@puertomontt
Copy link
Contributor

@puertomontt puertomontt commented May 29, 2025

Description

Implement #10771.
Expose envoy SSL config options for clusters. Code is largely based on https://github.com/solo-io/gloo/blob/main/projects/gloo/pkg/utils/ssl.go.

Change Type

/kind new_feature

Changelog

backendconfigpolicy: add ssl config

Additional Notes

Copilot AI review requested due to automatic review settings May 29, 2025 22:05
@github-actions github-actions bot added do-not-merge/kind-invalid Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. labels May 29, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds SSL origination support to the BackendConfigPolicy API and its implementation.

  • Introduces a new SSLConfig field in the CRD, API types, and generated apply configurations.
  • Implements translateSSLConfig to convert SSLConfig into an Envoy UpstreamTlsContext and integrates it into the plugin.
  • Adds YAML test fixtures and comprehensive unit tests for SSL configuration translation.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
api/v1alpha1/backend_config_policy_types.go Adds SSLConfig, SSLFiles, SSLParameters, and related fields.
internal/kgateway/extensions2/plugins/backendconfigpolicy/ssl.go Implements translation of SSLConfig into Envoy TLS contexts.
internal/kgateway/extensions2/plugins/backendconfigpolicy/plugin.go Integrates SSL transport socket into the plugin IR and processing.
internal/kgateway/extensions2/plugins/backendconfigpolicy/ssl_test.go Adds unit tests for various SSLConfig scenarios.
install/helm/kgateway-crds/templates/gateway.kgateway.dev_backendconfigpolicies.yaml Extends CRD template with SSLConfig schema and validations.
Comments suppressed due to low confidence (2)

internal/kgateway/extensions2/plugins/backendconfigpolicy/plugin.go:227

  • Variable name 'ir' shadows the imported package 'ir'. Consider renaming the variable to 'policyIR' or similar for clarity.
ir := BackendConfigPolicyIR{

internal/kgateway/extensions2/plugins/backendconfigpolicy/ssl.go:217

  • [nitpick] Function name 'cleanedSslKeyPair' uses mixed-case for 'Ssl'; consider renaming to 'cleanedSSLKeyPair' to follow Go initialism conventions.
func cleanedSslKeyPair(certChain, privateKey, rootCa string) (cleanedChain string, err error) {

@github-actions github-actions bot added kind/feature Categorizes issue or PR as related to a new feature. release-note and removed do-not-merge/kind-invalid Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. labels May 29, 2025
Signed-off-by: omar <[email protected]>
Signed-off-by: omar <[email protected]>
@puertomontt puertomontt requested a review from Copilot June 3, 2025 00:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for SSL/TLS origination in BackendConfigPolicy by exposing Envoy upstream TLS options and wiring them through the plugin and IR. It also updates test fixtures, CRD schemas, and generated code to include the new SSLConfig types and increases timeouts in setup tests.

  • Introduce SSLConfig, SSLFiles, SSLParameters, and related deep-copy, apply-configuration, and CRD schema entries
  • Implement translateSSLConfig and integrate UpstreamTlsContext into the plugin and IR
  • Update test data and increase timeouts; add debug prints in secret lookup

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
api/v1alpha1/backend_config_policy_types.go Add SSLConfig, SSLFiles, SSLParameters to spec
internal/kgateway/extensions2/plugins/backendconfigpolicy/ssl.go Implement translation of SSLConfig into Envoy UpstreamTlsContext
internal/kgateway/extensions2/plugins/backendconfigpolicy/plugin.go Wire TransportSocket when SSLConfig is present
internal/kgateway/extensions2/pluginutils/secrets.go Leftover debug fmt.Println calls in secret retrieval
internal/kgateway/setup/setup_test.go Increased fixed Sleep and test timeout duration
internal/kgateway/setup/testdata/standard/backendconfigpolicy-ssl.yaml Add SSL settings to example Gateway/BackendConfigPolicy
internal/kgateway/setup/testdata/standard/backendconfigpolicy-ssl-out.yaml Updated expected XDS output including TLS config
install/helm/kgateway-crds/templates/gateway.kgateway.dev_backendconfigpolicies.yaml Add CRD schema for sslConfig and validation rule
api/applyconfiguration/{utils.go, internal/internal.go, api/v1alpha1/*.go} Extend apply-configuration for new SSLConfig types

SSLParameters *SSLParametersApplyConfiguration `json:"sslParameters,omitempty"`
AlpnProtocols []string `json:"alpnProtocols,omitempty"`
AllowRenegotiation *bool `json:"allowRenegotiation,omitempty"`
OneWayTLS *bool `json:"oneWayTLS,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we going to eventually support SDSConfig? https://github.com/solo-io/gloo/blob/32c9ffde5db865befcea872f60cac01384c8a703/projects/gloo/api/v1/ssl/ssl.proto#L16 Do we want to create a follow up issue for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@npolshakova npolshakova Jun 5, 2025

Choose a reason for hiding this comment

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

Do we want to add this as a todo and mark it as a good first issue once this goes in with the base API?

@jenshu jenshu added this pull request to the merge queue Jun 6, 2025
Merged via the queue into kgateway-dev:main with commit 8547287 Jun 6, 2025
32 of 34 checks passed
@puertomontt puertomontt deleted the ssl-config branch June 7, 2025 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Categorizes issue or PR as related to a new feature. release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants