Merge pull request emissary-ingress#4519 from emissary-ingress/ci/rep…

…atriate/from-v2.4-to-v2.5

[v3.2] Repatriate from v2.4

.github/actions/collect-logs/action.yml

@@ -34,13 +34,17 @@ runs:
         if test -f ~/.kube/config; then
           make tools/bin/kubectl
           mkdir /tmp/test-logs/cluster
+          tools/bin/kubectl get hosts --all-namespaces -o yaml >/tmp/test-logs/cluster/all-hosts.yaml || true
           tools/bin/kubectl get pods --all-namespaces      >/tmp/test-logs/cluster/all-pods.txt || true
           tools/bin/kubectl describe pods --all-namespaces >/tmp/test-logs/cluster/all-pods-described.txt || true
 
           tools/bin/kubectl get pods --all-namespaces -ocustom-columns="name:.metadata.name,namespace:.metadata.namespace" --no-headers | while read -r name namespace; do
             tools/bin/kubectl --namespace="$namespace" logs "$name" >"/tmp/test-logs/cluster/pod.${namespace}.${name}.log" || true
           done
+
+          tools/bin/kubectl cp xfpredirect:/tmp/ambassador/snapshots /tmp/test-logs/cluster/xfpredirect.snapshots || true
         fi
+        cp /tmp/*.yaml /tmp/test-logs || true
     - name: "Upload Logs"
       uses: actions/upload-artifact@v2
       with:

CHANGELOG.md

@@ -132,15 +132,29 @@ it will be removed; but as it won't be user-visible this isn't considered a brea
   the specified non-negative window period in seconds before doing an Envoy reconfiguration. Default
   is "1" if not set.
 
-- Bugfix: If a `Host` or `TLSContext` contained a hostname with a `:` then when using the 
+- Bugfix: If a `Host` or `TLSContext` contained a hostname with a `:` then when using the
   diagnostics endpoints `ambassador/v0/diagd` then an error would be thrown due to the parsing logic
-  not  being able to handle the extra colon. This has been fixed and Emissary-ingress will not throw
+  not being able to handle the extra colon. This has been fixed and Emissary-ingress will not throw
   an error when parsing envoy metrics for the diagnostics user interface.
 
 - Feature: It is now possible to set `custom_tags` in the `TracingService`. Trace tags can be set
   based on literal values, environment variables, or request headers. (Thanks to <a
   href="https://github.com/psalaberria002">Paul</a>!) ([#4181])
 
+- Bugfix: Emissary-ingress 2.0.0 introduced a bug where a `TCPMapping` that uses SNI, instead of
+  using the hostname glob in the `TCPMapping`, uses the hostname glob in the `Host` that the TLS
+  termination configuration comes from.
+
+- Bugfix: Emissary-ingress 2.0.0 introduced a bug where a `TCPMapping` that terminates TLS must have
+  a corresponding `Host` that it can take the TLS configuration from. This was semi-intentional, but
+  didn't make much sense.  You can now use a `TLSContext` without a `Host`as in Emissary-ingress 1.y
+  releases, or a `Host` with or without a `TLSContext` as in prior 2.y releases.
+
+- Bugfix: Prior releases of Emissary-ingress had the arbitrary limitation that a `TCPMapping` cannot
+  be used on the same port that HTTP is served on, even if TLS+SNI would make this possible. 
+  Emissary-ingress now allows `TCPMappings` to be used on the same `Listener` port as HTTP `Hosts`,
+  as long as that `Listener` terminates TLS.
+
 [#4354]: https://github.com/emissary-ingress/emissary/issues/4354
 [#4181]: https://github.com/emissary-ingress/emissary/pull/4181
 
@@ -315,6 +329,20 @@ it will be removed; but as it won't be user-visible this isn't considered a brea
   the specified non-negative window period in seconds before doing an Envoy reconfiguration. Default
   is "1" if not set.
 
+- Bugfix: Emissary-ingress 2.0.0 introduced a bug where a `TCPMapping` that uses SNI, instead of
+  using the hostname glob in the `TCPMapping`, uses the hostname glob in the `Host` that the TLS
+  termination configuration comes from.
+
+- Bugfix: Emissary-ingress 2.0.0 introduced a bug where a `TCPMapping` that terminates TLS must have
+  a corresponding `Host` that it can take the TLS configuration from. This was semi-intentional, but
+  didn't make much sense.  You can now use a `TLSContext` without a `Host`as in Emissary-ingress 1.y
+  releases, or a `Host` with or without a `TLSContext` as in prior 2.y releases.
+
+- Bugfix: Prior releases of Emissary-ingress had the arbitrary limitation that a `TCPMapping` cannot
+  be used on the same port that HTTP is served on, even if TLS+SNI would make this possible. 
+  Emissary-ingress now allows `TCPMappings` to be used on the same `Listener` port as HTTP `Hosts`,
+  as long as that `Listener` terminates TLS.
+
 ## [1.14.5] TBD
 [1.14.5]: https://github.com/emissary-ingress/emissary/compare/v2.3.2...v1.14.5
 

build-aux/.gitignore

@@ -33,6 +33,10 @@
 # Remove the tail of this list when the commit making the change gets
 # far enough in to the past.
 
+# whenever-we-stop-working-on-2.x
+/pytest-kat-envoy2.txt
+/pytest-kat-envoy3.txt
+
 # 2019-07-01
 /teleproxy
 /kubeapply

builder/builder.mk

@@ -321,7 +321,7 @@ pytest-kat-envoy3: push-pytest-images # doing this all at once is too much for C
 	$(MAKE) pytest KAT_RUN_MODE=envoy PYTEST_ARGS="$$PYTEST_ARGS python/tests/kat"
 # ... so we have a separate rule to run things split up
 build-aux/.pytest-kat.txt.stamp: $(OSS_HOME)/venv push-pytest-images FORCE
-	. venv/bin/activate && set -o pipefail && pytest --collect-only python/tests/kat 2>&1 | sed -En 's/.*<Function (.*)>/\1/p' | sed 's/[].].*//' | sort -u > $@
+	. venv/bin/activate && set -o pipefail && pytest --collect-only python/tests/kat 2>&1 | sed -En 's/.*<Function (.*)>/\1/p' | cut -d. -f1 | sort -u > $@
 build-aux/pytest-kat.txt: build-aux/%: build-aux/.%.stamp $(tools/copy-ifchanged)
 	$(tools/copy-ifchanged) $< $@
 clean: build-aux/.pytest-kat.txt.stamp.rm build-aux/pytest-kat.txt.rm

builder/copy-gold.sh

@@ -116,5 +116,19 @@ copy_gold xfpredirect
 copy_gold empty empty-namespace
 copy_gold plain plain-namespace
 copy_gold tcpmappingtest tcp-namespace
+copy_gold tcpmappingbasictest
+copy_gold tcpmappingcrossnamespacetest
+copy_gold tcpmappingtlsoriginationbooltest
+copy_gold tcpmappingtlsoriginationv2schemetest
+copy_gold tcpmappingtlsoriginationcontexttest
+copy_gold tcpmappingtlsoriginationcontextwithdottest
+copy_gold tcpmappingtlsoriginationcontextcrossnamespacetest
+copy_gold tcpmappingtlsterminationbasictest
+copy_gold tcpmappingtlsterminationcrossnamespacetest
+copy_gold tcpmappingsnisharedcontexttest
+copy_gold tcpmappingsniseparatecontextstest
+copy_gold tcpmappingsniwithhttptest
+copy_gold tcpmappingaddresstest
+copy_gold tcpmappingweighttest
 
 printf "\n"

docs/releaseNotes.yml

@@ -102,12 +102,12 @@ items:
         body: >-
           The <code>AMBASSADOR_RECONFIG_MAX_DELAY</code> env var can be optionally set to batch changes for the specified
           non-negative window period in seconds before doing an Envoy reconfiguration. Default is "1" if not set.
-      
+
       - title: Diagnostics stats properly handles parsing envoy metrics with colons
         type: bugfix
         body: >-
-          If a <code>Host</code> or <code>TLSContext</code> contained a hostname with a <code>:</code> then when using the 
-          diagnostics endpoints <code>ambassador/v0/diagd</code> then an error would be thrown due to the parsing logic not 
+          If a <code>Host</code> or <code>TLSContext</code> contained a hostname with a <code>:</code> then when using the
+          diagnostics endpoints <code>ambassador/v0/diagd</code> then an error would be thrown due to the parsing logic not
           being able to handle the extra colon. This has been fixed and $productName$ will not throw an error when parsing
           envoy metrics for the diagnostics user interface.
           
@@ -122,6 +122,31 @@ items:
         - title: "#4181"
           link: https://github.com/emissary-ingress/emissary/pull/4181
 
+      - title: TCPMappings use correct SNI configuration
+        type: bugfix
+        body: >-
+          $productName$ 2.0.0 introduced a bug where a <code>TCPMapping</code> that uses SNI,
+          instead of using the hostname glob in the <code>TCPMapping</code>, uses the hostname glob
+          in the <code>Host</code> that the TLS termination configuration comes from.
+
+      - title: TCPMappings configure TLS termination without a Host resource
+        type: bugfix
+        body: >-
+          $productName$ 2.0.0 introduced a bug where a <code>TCPMapping</code> that terminates TLS
+          must have a corresponding <code>Host</code> that it can take the TLS configuration from.
+          This was semi-intentional, but didn't make much sense.  You can now use a
+          <code>TLSContext</code> without a <code>Host</code>as in $productName$ 1.y releases, or a
+          <code>Host</code> with or without a <code>TLSContext</code> as in prior 2.y releases.
+
+      - title: TCPMappings and HTTP Hosts can coexist on Listeners that terminate TLS
+        type: bugfix
+        body: >-
+          Prior releases of $productName$ had the arbitrary limitation that a
+          <code>TCPMapping</code> cannot be used on the same port that HTTP is served on, even if
+          TLS+SNI would make this possible.  $productName$ now allows <code>TCPMappings</code> to be
+          used on the same <code>Listener</code> port as HTTP <code>Hosts</code>, as long as that
+          <code>Listener</code> terminates TLS.
+
   - version: 3.1.1
     prevVersion: 3.1.0
     date: 'TBD'
@@ -352,6 +377,31 @@ items:
           The <code>AMBASSADOR_RECONFIG_MAX_DELAY</code> env var can be optionally set to batch changes for the specified
           non-negative window period in seconds before doing an Envoy reconfiguration. Default is "1" if not set.
 
+      - title: TCPMappings use correct SNI configuration
+        type: bugfix
+        body: >-
+          $productName$ 2.0.0 introduced a bug where a <code>TCPMapping</code> that uses SNI,
+          instead of using the hostname glob in the <code>TCPMapping</code>, uses the hostname glob
+          in the <code>Host</code> that the TLS termination configuration comes from.
+
+      - title: TCPMappings configure TLS termination without a Host resource
+        type: bugfix
+        body: >-
+          $productName$ 2.0.0 introduced a bug where a <code>TCPMapping</code> that terminates TLS
+          must have a corresponding <code>Host</code> that it can take the TLS configuration from.
+          This was semi-intentional, but didn't make much sense.  You can now use a
+          <code>TLSContext</code> without a <code>Host</code>as in $productName$ 1.y releases, or a
+          <code>Host</code> with or without a <code>TLSContext</code> as in prior 2.y releases.
+
+      - title: TCPMappings and HTTP Hosts can coexist on Listeners that terminate TLS
+        type: bugfix
+        body: >-
+          Prior releases of $productName$ had the arbitrary limitation that a
+          <code>TCPMapping</code> cannot be used on the same port that HTTP is served on, even if
+          TLS+SNI would make this possible.  $productName$ now allows <code>TCPMappings</code> to be
+          used on the same <code>Listener</code> port as HTTP <code>Hosts</code>, as long as that
+          <code>Listener</code> terminates TLS.
+
   - version: 1.14.5
     date: 'TBD'
     notes: