[1.17] Warn on missing TLS secret

.github/workflows/nightly-tests.yaml

@@ -50,7 +50,7 @@ jobs:
     name: End-to-End (branch=${{ github.ref_name }}, cluster=${{ matrix.test.cluster-name }}, version=${{ matrix.version-files.label }} )
     if: ${{ github.event_name == 'workflow_dispatch' && inputs.run-kubernetes-end-to-end && inputs.branch == 'workflow_initiating_branch' }}
     runs-on: ubuntu-22.04
-    timeout-minutes: 150
+    timeout-minutes: 180
     strategy:
       # Since we are running these on a schedule, there is no value in failing fast
       # In fact, we want to ensure that all tests run, so that we have a clearer picture of which tests are prone to flaking
@@ -60,7 +60,7 @@ jobs:
           # When running the tests at night, there is no value in splitting the tests across multiple clusters and running them in parallel.
           # As a result, we increase the threshold for the tests, since they all run serially on a single cluster
           - cluster-name: 'cluster-one'
-            go-test-args: '-v -timeout=120m'
+            go-test-args: '-v -timeout=150m'
             go-test-run-regex: ${{ inputs.kubernetes-end-to-end-run-regex }}
         # In our nightly tests, we run the suite of tests using the lower and upper ends of versions that we claim to support
         # The versions should mirror: https://docs.solo.io/gloo-edge/latest/reference/support/
@@ -110,7 +110,7 @@ jobs:
     name: End-to-End (branch=main, cluster=${{ matrix.test.cluster-name }}, version=${{ matrix.version-files.label }} )
     if: ${{ (github.event_name == 'workflow_dispatch' && inputs.run-kubernetes-end-to-end && inputs.branch == 'main') || github.event.schedule == '0 5 * * 1-5' }}
     runs-on: ubuntu-22.04
-    timeout-minutes: 150
+    timeout-minutes: 180
     strategy:
       # Since we are running these on a schedule, there is no value in failing fast
       # In fact, we want to ensure that all tests run, so that we have a clearer picture of which tests are prone to flaking
@@ -120,7 +120,7 @@ jobs:
           # When running the tests at night, there is no value in splitting the tests across multiple clusters and running them in parallel.
           # As a result, we increase the threshold for the tests, since they all run serially on a single cluster
           - cluster-name: 'cluster-one'
-            go-test-args: '-v -timeout=120m'
+            go-test-args: '-v -timeout=150m'
             go-test-run-regex: ""
         # In our nightly tests, we run the suite of tests using the lower and upper ends of versions that we claim to support
         # The versions should mirror: https://docs.solo.io/gloo-edge/latest/reference/support/

changelog/v1.17.5/missing-tls-secret.yaml

@@ -1,13 +1,13 @@
 changelog:
-  - type: BREAKING_CHANGE
+  - type: FIX
     issueLink: https://github.com/solo-io/gloo/issues/6957
     resolvesIssue: false
     description: >-
       Fix for issue where a missing TLS secret was treated by validation as an error,
       potentially bringing down the entire HTTPS gateway if the gloo pod restarts while 
       in this bad state. This is a breaking change in the default behavior of validation.
 
-      To disable this behavior, use the helm setting `gateway.validation.warnMissingTlsSecret=false`
+      To enable this behavior, use the helm setting `gateway.validation.warnMissingTlsSecret=true`
       or the same field on the Settings CR. This field has no effect if allowWarnings is false or 
       acceptAllResources is true.
   - type: HELM
@@ -16,5 +16,5 @@ changelog:
     description: >-
       New field gateway.validation.warnMissingTlsSecret controls whether missing TLS secrets referenced
       in SslConfig and UpstreamSslConfig will be treated as a warning instead of an error during validation.
-      Defaults to true. This field has no effect if allowWarnings is false or acceptAllResources is true.
+      Defaults to false. This field has no effect if allowWarnings is false or acceptAllResources is true.
 

docs/content/reference/values.txt

@@ -482,7 +482,7 @@
 |gateway.validation.enabled|bool|true|enable Gloo Edge API Gateway validation hook (default true)|
 |gateway.validation.alwaysAcceptResources|bool|true|unless this is set this to false in order to ensure validation webhook rejects invalid resources. by default, validation webhook will only log and report metrics for invalid resource admission without rejecting them outright.|
 |gateway.validation.allowWarnings|bool|true|set this to false in order to ensure validation webhook rejects resources that would have warning status or rejected status, rather than just rejected.|
-|gateway.validation.warnMissingTlsSecret|bool|true|set this to false in order to treat missing tls secret references as errors, causing validation to fail.|
+|gateway.validation.warnMissingTlsSecret|bool|false|set this to true in order to treat missing tls secret references as warnings, causing validation to allow this state to support eventually consistent workflows.|
 |gateway.validation.serverEnabled|bool|true|By providing the validation field (parent of this object) the user is implicitly opting into validation. This field allows the user to opt out of the validation server, while still configuring pre-existing fields such as warn_route_short_circuiting and disable_transformation_validation.|
 |gateway.validation.disableTransformationValidation|bool|false|set this to true to disable transformation validation. This may bring signifigant performance benefits if using many transformations, at the cost of possibly incorrect transformations being sent to Envoy. When using this value make sure to pre-validate transformations.|
 |gateway.validation.warnRouteShortCircuiting|bool|false|Write a warning to route resources if validation produced a route ordering warning (defaults to false). By setting to true, this means that Gloo Edge will start assigning warnings to resources that would result in route short-circuiting within a virtual host.|

install/helm/gloo/generate/values.go

@@ -460,7 +460,7 @@ type GatewayValidation struct {
 	Enabled                          *bool    `json:"enabled,omitempty" desc:"enable Gloo Edge API Gateway validation hook (default true)"`
 	AlwaysAcceptResources            *bool    `json:"alwaysAcceptResources,omitempty" desc:"unless this is set this to false in order to ensure validation webhook rejects invalid resources. by default, validation webhook will only log and report metrics for invalid resource admission without rejecting them outright."`
 	AllowWarnings                    *bool    `json:"allowWarnings,omitempty" desc:"set this to false in order to ensure validation webhook rejects resources that would have warning status or rejected status, rather than just rejected."`
-	WarnMissingTlsSecret             *bool    `json:"warnMissingTlsSecret,omitempty" desc:"set this to false in order to treat missing tls secret references as errors, causing validation to fail."`
+	WarnMissingTlsSecret             *bool    `json:"warnMissingTlsSecret,omitempty" desc:"set this to true in order to treat missing tls secret references as warnings, causing validation to allow this state to support eventually consistent workflows."`
 	ServerEnabled                    *bool    `json:"serverEnabled,omitempty" desc:"By providing the validation field (parent of this object) the user is implicitly opting into validation. This field allows the user to opt out of the validation server, while still configuring pre-existing fields such as warn_route_short_circuiting and disable_transformation_validation."`
 	DisableTransformationValidation  *bool    `json:"disableTransformationValidation,omitempty" desc:"set this to true to disable transformation validation. This may bring signifigant performance benefits if using many transformations, at the cost of possibly incorrect transformations being sent to Envoy. When using this value make sure to pre-validate transformations."`
 	WarnRouteShortCircuiting         *bool    `json:"warnRouteShortCircuiting,omitempty" desc:"Write a warning to route resources if validation produced a route ordering warning (defaults to false). By setting to true, this means that Gloo Edge will start assigning warnings to resources that would result in route short-circuiting within a virtual host."`

install/helm/gloo/values-template.yaml

@@ -124,7 +124,9 @@ gateway:
     secretName: gateway-validation-certs
     alwaysAcceptResources: true
     allowWarnings: true
-    warnMissingTlsSecret: true
+    # Explicitly setting to false in order to emphasize the opt-in nature of this 
+    # behavior in backports.
+    warnMissingTlsSecret: false
     serverEnabled: true
     disableTransformationValidation: false
     warnRouteShortCircuiting: false

install/test/fixtures/settings/consul_config_upstream_discovery.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/consul_config_values.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/disable_kubernetes_destinations.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/disable_proxy_garbage_collection.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/disabled_gateway.yaml

@@ -17,7 +17,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/enable_default_credentials.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/enable_rest_eds.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/enable_rest_eds_and_gloo_mtls.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/gateway_circuit_breakers.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/gateway_settings.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/gateway_validation.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: false
+      warnMissingTlsSecret: true
       serverEnabled: true
       disableTransformationValidation: true
       warnRouteShortCircuiting: true

install/test/fixtures/settings/graphql_fds_disabled.yaml

@@ -18,7 +18,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/isolate_virtual_hosts_by_ssl_config.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/ratelimit_descriptors.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/ratelimit_server.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/set_regex_max_program_size.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/set_regex_max_program_size_default.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/set_secretSettings_in_settings.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/sts_discovery.yaml

@@ -16,7 +16,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/uds_disabled.yaml

@@ -18,7 +18,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/fixtures/settings/watched_discovery_labels.yaml

@@ -19,7 +19,7 @@ spec:
     validation:
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

install/test/helm_test.go

@@ -4170,7 +4170,7 @@ spec:
 							ValuesArgs: []string{
 								"gateway.validation.disableTransformationValidation=true",
 								"gateway.validation.warnRouteShortCircuiting=true",
-								"gateway.validation.warnMissingTlsSecret=false",
+								"gateway.validation.warnMissingTlsSecret=true",
 							},
 						})
 						testManifest.ExpectUnstructured(settings.GetKind(), settings.GetNamespace(), settings.GetName()).To(BeEquivalentTo(settings))
@@ -4373,7 +4373,7 @@ spec:
       proxyValidationServerAddr: gloo:9988
       alwaysAccept: true
       allowWarnings: true
-      warnMissingTlsSecret: true
+      warnMissingTlsSecret: false
       serverEnabled: true
       disableTransformationValidation: false
       warnRouteShortCircuiting: false

test/kubernetes/testutils/assertions/curl.go

@@ -132,3 +132,60 @@ func (p *Provider) AssertEventuallyConsistentCurlResponse(
 		WithContext(ctx).
 		Should(Succeed())
 }
+
+// AssertEventualCurlError asserts that the response from a curl command is an error such as `Failed to connect`
+// as opposed to an http error from the server. This is useful when testing that a service is not reachable,
+// for example to validate that a delete operation has taken effect.
+func (p *Provider) AssertEventualCurlError(
+	ctx context.Context,
+	podOpts kubectl.PodExecOptions,
+	curlOptions []curl.Option,
+	expectedErrorCode int, // This is an application error code not an HTTP error code
+	timeout ...time.Duration,
+) {
+	// We rely on the curlPod to execute a curl, therefore we must assert that it actually exists
+	p.EventuallyObjectsExist(ctx, &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: podOpts.Name, Namespace: podOpts.Namespace,
+		},
+	})
+
+	pollTimeout := 5 * time.Second
+	pollInterval := 500 * time.Millisecond
+	if len(timeout) > 0 {
+		pollTimeout, pollInterval = helper.GetTimeouts(timeout...)
+	}
+
+	testMessage := fmt.Sprintf("Expected curl error %d", expectedErrorCode)
+
+	p.Gomega.Eventually(func(g Gomega) {
+		curlResponse, err := p.clusterContext.Cli.CurlFromPod(ctx, podOpts, curlOptions...)
+
+		if err == nil {
+			if curlResponse == nil { // This is not expected to happen, but adding for safety/future-proofing
+				fmt.Printf("wanted curl error, got no error and no response\n")
+				testMessage = fmt.Sprintf("Expected curl error %d, got no error and no response\n", expectedErrorCode)
+			} else {
+				fmt.Printf("wanted curl error, got response:\nstdout:\n%s\nstderr:%s\n", curlResponse.StdOut, curlResponse.StdErr)
+				curlHttpResponse := transforms.WithCurlResponse(curlResponse)
+				testMessage = fmt.Sprintf("failed to get a curl error, got response code: %d", curlHttpResponse.StatusCode)
+				curlHttpResponse.Body.Close()
+			}
+			g.Expect(err).To(HaveOccurred())
+		}
+
+		if expectedErrorCode > 0 {
+			expectedCurlError := fmt.Sprintf("exit status %d", expectedErrorCode)
+			fmt.Printf("wanted curl error: %s, got error: %s\n", expectedCurlError, err.Error())
+			testMessage = fmt.Sprintf("Expected curl error: %s, got: %s", expectedCurlError, err.Error())
+			g.Expect(err.Error()).To(Equal(expectedCurlError))
+		} else {
+			fmt.Printf("wanted any curl error, got error: %s\n", err.Error())
+		}
+
+	}).
+		WithTimeout(pollTimeout).
+		WithPolling(pollInterval).
+		WithContext(ctx).
+		Should(Succeed(), testMessage)
+}