-
Notifications
You must be signed in to change notification settings - Fork 611
conformance: TLSRoute with Terminate mode
#4137
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
base: main
Are you sure you want to change the base?
Changes from all commits
dfe5581
bf18884
2e8847d
0ea5d63
25443f5
0531cee
1e374ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| /* | ||
| Copyright 2025 The Kubernetes Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package tests | ||
|
|
||
| import ( | ||
| "crypto/tls" | ||
| "crypto/x509" | ||
| "fmt" | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "k8s.io/apimachinery/pkg/types" | ||
|
|
||
| "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" | ||
| "sigs.k8s.io/gateway-api/conformance/utils/suite" | ||
| "sigs.k8s.io/gateway-api/pkg/features" | ||
|
|
||
| mqtt "github.com/eclipse/paho.mqtt.golang" | ||
| ) | ||
|
|
||
| func init() { | ||
| ConformanceTests = append(ConformanceTests, TLSRouteTerminateSimpleSameNamespace) | ||
| } | ||
|
|
||
| var TLSRouteTerminateSimpleSameNamespace = suite.ConformanceTest{ | ||
| ShortName: "TLSRouteTerminateSimpleSameNamespace", | ||
| Description: "A single TLSRoute in the gateway-conformance-infra namespace attaches to a Gateway using Terminate mode in the same namespace", | ||
| Features: []features.FeatureName{ | ||
| features.SupportGateway, | ||
| features.SupportTLSRoute, | ||
| features.SupportTLSRouteModeTerminate, | ||
| }, | ||
| Provisional: true, | ||
| Manifests: []string{"tests/tlsroute-terminate-simple-same-namespace.yaml"}, | ||
| Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { | ||
| ns := "gateway-conformance-infra" | ||
| routeNN := types.NamespacedName{Name: "gateway-conformance-mqtt-test", Namespace: ns} | ||
| gwNN := types.NamespacedName{Name: "gateway-tlsroute-terminate", Namespace: ns} | ||
| caCertNN := types.NamespacedName{Name: "tls-checks-ca-certificate", Namespace: ns} | ||
|
|
||
| kubernetes.NamespacesMustBeReady(t, suite.Client, suite.TimeoutConfig, []string{ns}) | ||
|
|
||
| gwAddr, hostnames := kubernetes.GatewayAndTLSRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) | ||
| if len(hostnames) != 1 { | ||
| t.Fatalf("unexpected error in test configuration, found %d hostnames", len(hostnames)) | ||
| } | ||
| serverStr := string(hostnames[0]) | ||
|
|
||
| caConfigMap, err := kubernetes.GetConfigMapData(suite.Client, caCertNN) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error finding TLS secret: %v", err) | ||
| } | ||
| caString, ok := caConfigMap["ca.crt"] | ||
| if !ok { | ||
| t.Fatalf("ca.crt not found in configmap: %s/%s", caCertNN.Namespace, caCertNN.Name) | ||
| } | ||
|
|
||
| t.Run("Simple MQTT TLS request matching TLSRoute should reach mqtt-backend", func(t *testing.T) { | ||
| t.Logf("Establishing MQTT connection to host %s via %s", serverStr, gwAddr) | ||
|
|
||
| certpool := x509.NewCertPool() | ||
| if !certpool.AppendCertsFromPEM([]byte(caString)) { | ||
| t.Fatal("Failed to append CA certificate") | ||
| } | ||
|
|
||
| opts := mqtt.NewClientOptions() | ||
| opts.AddBroker(fmt.Sprintf("tls://%s", gwAddr)) | ||
| opts.SetTLSConfig(&tls.Config{ | ||
| RootCAs: certpool, | ||
| ServerName: serverStr, | ||
| MinVersion: tls.VersionTLS13, | ||
| }) | ||
| opts.SetConnectRetry(true) | ||
|
|
||
| var wg sync.WaitGroup | ||
| wg.Add(1) | ||
|
|
||
| topic := "test/tlsroute-terminate" | ||
| opts.OnConnect = func(c mqtt.Client) { | ||
| t.Log("Connected to MQTT broker") | ||
|
|
||
| if token := c.Subscribe(topic, 0, func(_ mqtt.Client, msg mqtt.Message) { | ||
| t.Logf("Received message: %s\n", string(msg.Payload())) | ||
| wg.Done() | ||
| }); token.Wait() && token.Error() != nil { | ||
| t.Fatalf("Failed to subscribe: %v", token.Error()) | ||
| } | ||
|
|
||
| t.Log("Subscribed, publishing test message...") | ||
| if token := c.Publish(topic, 0, false, "Hello TLSRoute Terminate MQTT!"); token.Wait() && token.Error() != nil { | ||
| t.Fatalf("Failed to publish: %v", token.Error()) | ||
| } | ||
| } | ||
|
|
||
| client := mqtt.NewClient(opts) | ||
| if token := client.Connect(); token.Wait() && token.Error() != nil { | ||
| t.Fatalf("Connection failed: %v", token.Error()) | ||
| } | ||
|
|
||
| waitCh := make(chan struct{}) | ||
| go func() { | ||
| wg.Wait() | ||
| close(waitCh) | ||
| }() | ||
|
|
||
| select { | ||
| case <-waitCh: | ||
| t.Log("Round-trip test succeeded") | ||
| case <-time.After(5 * time.Second): | ||
| t.Fatal("Timed out waiting for message") | ||
| } | ||
| }) | ||
| }, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| apiVersion: gateway.networking.k8s.io/v1alpha3 | ||
| kind: TLSRoute | ||
| metadata: | ||
| name: gateway-conformance-mqtt-test | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| parentRefs: | ||
| - name: gateway-tlsroute-terminate | ||
| namespace: gateway-conformance-infra | ||
| hostnames: | ||
| - tls.example.com | ||
| rules: | ||
| - backendRefs: | ||
| - name: mqtt-backend | ||
| port: 1883 | ||
| --- | ||
| apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: Gateway | ||
| metadata: | ||
| name: gateway-tlsroute-terminate | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| gatewayClassName: "{GATEWAY_CLASS_NAME}" | ||
| listeners: | ||
| - name: mqtt | ||
| port: 8883 | ||
| protocol: TLS | ||
| hostname: tls.example.com | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: Same | ||
| kinds: | ||
| - kind: TLSRoute | ||
| tls: | ||
| mode: Terminate | ||
| certificateRefs: | ||
| - name: tls-terminate-checks-certificate | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: mqtt-backend | ||
| namespace: gateway-conformance-infra | ||
| spec: | ||
| selector: | ||
| app: mqtt-backend | ||
| ports: | ||
| - protocol: TCP | ||
| port: 1883 | ||
| targetPort: 1883 | ||
| --- | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: mqtt-backend | ||
| namespace: gateway-conformance-infra | ||
| labels: | ||
| app: mqtt-backend | ||
| spec: | ||
| replicas: 1 | ||
| selector: | ||
| matchLabels: | ||
| app: mqtt-backend | ||
| template: | ||
| metadata: | ||
| labels: | ||
| app: mqtt-backend | ||
| spec: | ||
| containers: | ||
| - name: mqtt-backend | ||
| # https://hub.docker.com/_/eclipse-mosquitto | ||
| image: eclipse-mosquitto:2 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am slightly concerned about using an image on docker hub, mostly because of rate limits and other situations that may impact the tests. @robscott do we have a way to push this image to our staging repo? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another approach would be simply extend https://github.com/kubernetes-sigs/gateway-api/blob/main/conformance/echo-basic/echo-basic.go to add a "TCP echo server" so you don't need any mqtt, etc, just bare simple TCP client that would connect from one side using TLS, and from gateway to backend without it and echo the request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (or disregard all of it, if mqtt works fine for everyone let's move with it!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| volumeMounts: | ||
| - name: config | ||
| mountPath: /mosquitto/config/mosquitto.conf | ||
| subPath: mosquitto.conf | ||
| ports: | ||
| - containerPort: 1883 | ||
| resources: | ||
| requests: | ||
| cpu: 10m | ||
| volumes: | ||
| - name: config | ||
| configMap: | ||
| name: mosquitto-config | ||
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: mosquitto-config | ||
| namespace: gateway-conformance-infra | ||
| data: | ||
| mosquitto.conf: | | ||
| listener 1883 | ||
| allow_anonymous true | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,10 @@ var ( | |
| features.SupportReferenceGrant, | ||
| features.SupportTLSRoute, | ||
| ), | ||
| ExtendedFeatures: features.SetsToNamesSet(features.GatewayExtendedFeatures), | ||
| ExtendedFeatures: features.SetsToNamesSet( | ||
| features.GatewayExtendedFeatures, | ||
| features.TLSRouteExtendedFeatures, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could be convinced otherwise, but I don't think TLSRouteExtendedFeatures belongs grouped with GatewayTLSConformanceProfile. We should create a new conformance profile for TLSRoute. Unless we add TLSRouteCoreFeatures to the GatewayTLSConformanceProfile as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't it the case of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, please disregard. |
||
| ), | ||
| } | ||
|
|
||
| // GatewayGRPCConformanceProfile is a ConformanceProfile that covers testing GRPC | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ module sigs.k8s.io/gateway-api | |
| go 1.24.0 | ||
|
|
||
| require ( | ||
| github.com/eclipse/paho.mqtt.golang v1.5.1 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. side note (no action from you @phuhung273 ) @robscott one more reason we should move fast removing conformance from main go.mod :D |
||
| github.com/miekg/dns v1.1.68 | ||
| github.com/stretchr/testify v1.11.1 | ||
| golang.org/x/net v0.46.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,16 +25,33 @@ import "k8s.io/apimachinery/pkg/util/sets" | |||||||||||||||||||||||||||||||||||||
| const ( | ||||||||||||||||||||||||||||||||||||||
| // This option indicates support for TLSRoute | ||||||||||||||||||||||||||||||||||||||
| SupportTLSRoute FeatureName = "TLSRoute" | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // This option indicates support for TLSRoute mode Terminate (extended conformance) | ||||||||||||||||||||||||||||||||||||||
| SupportTLSRouteModeTerminate FeatureName = "TLSRouteModeTerminate" | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // TLSRouteFeature contains metadata for the TLSRoute feature. | ||||||||||||||||||||||||||||||||||||||
| var TLSRouteFeature = Feature{ | ||||||||||||||||||||||||||||||||||||||
| Name: SupportTLSRoute, | ||||||||||||||||||||||||||||||||||||||
| Channel: FeatureChannelExperimental, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| var ( | ||||||||||||||||||||||||||||||||||||||
| // TLSRouteFeature contains metadata for the TLSRoute feature. | ||||||||||||||||||||||||||||||||||||||
| TLSRouteFeature = Feature{ | ||||||||||||||||||||||||||||||||||||||
| Name: SupportTLSRoute, | ||||||||||||||||||||||||||||||||||||||
| Channel: FeatureChannelExperimental, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| // TLSRouteModeTerminate contains metadata for the TLSRouteModeTerminate feature. | ||||||||||||||||||||||||||||||||||||||
| TLSRouteModeTerminateFeature = Feature{ | ||||||||||||||||||||||||||||||||||||||
| Name: SupportTLSRouteModeTerminate, | ||||||||||||||||||||||||||||||||||||||
| Channel: FeatureChannelExperimental, | ||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does TLSRouteModeTerminateFeature remain in the experimental channel after TLSRouteFeature moves to the standard channel? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given GEP-2643: TLSRoute (although not finalized yet) stating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make sure it is marked as Extended in the CRD. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not really understand your point @candita, should I make any change in CRD ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @candita since this has a separate feature name in conformance, it's inherently extended. We could optionally also label this as extended in the relevant part of the Gateway spec, but the only ways I can think of modifying this would likely be more confusing than helpful: gateway-api/apis/v1/gateway_types.go Lines 551 to 568 in c204adf
IMO, this is one case where documentation + conformance tests is likely sufficient (not requiring changes to API spec). |
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // TLSCoreFeatures includes all the supported features for the TLSRoute API at | ||||||||||||||||||||||||||||||||||||||
| // a Core level of support. | ||||||||||||||||||||||||||||||||||||||
| var TLSRouteCoreFeatures = sets.New( | ||||||||||||||||||||||||||||||||||||||
| TLSRouteFeature, | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // TLSRouteExtendedFeatures includes all extended features for TLSRoute | ||||||||||||||||||||||||||||||||||||||
| // conformance and can be used to opt-in to run all TLSRoute extended features tests. | ||||||||||||||||||||||||||||||||||||||
| // This does not include any Core Features. | ||||||||||||||||||||||||||||||||||||||
| var TLSRouteExtendedFeatures = sets.New( | ||||||||||||||||||||||||||||||||||||||
| TLSRouteModeTerminateFeature, | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not an MQTT expert, but don't you have to subscribe and publish a message to test the transport? Doing a Connect doesn't seem enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no worries, I thought connect is enough. Please check latest update for the full round-trip test. Thanks for the feedback.