From a9605bd7be87f96ac9ee6a4f9c3a16552b8f4f5f Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 11 Jan 2022 20:30:45 +0900 Subject: [PATCH 01/29] TLS between Ingress and activator --- .github/workflows/kind-e2e.yaml | 5 +++ cmd/activator/main.go | 20 +++++++++ config/core/deployments/activator.yaml | 13 ++++++ pkg/networking/constants.go | 3 ++ pkg/reconciler/route/resources/ingress.go | 28 ++++++++---- .../route/resources/ingress_test.go | 9 ++-- .../serverlessservice/resources/services.go | 45 ++++++++++++++++--- test/config/tls/config-network.yaml | 26 +++++++++++ test/e2e-common.sh | 3 +- test/generate-cert.sh | 44 ++++++++++++++++++ 10 files changed, 176 insertions(+), 20 deletions(-) create mode 100644 test/config/tls/config-network.yaml create mode 100755 test/generate-cert.sh diff --git a/.github/workflows/kind-e2e.yaml b/.github/workflows/kind-e2e.yaml index 6f18ae1d8acb..48e2ac42af2b 100644 --- a/.github/workflows/kind-e2e.yaml +++ b/.github/workflows/kind-e2e.yaml @@ -97,6 +97,7 @@ jobs: ingress: - kourier + - kourier-tls - istio - contour # Disabled due to consistent failures @@ -132,6 +133,9 @@ jobs: - ingress: istio namespace-resources: virtualservices + - ingress: kourier-tls + enable-tls: 1 + - test-suite: runtime test-path: ./test/conformance/runtime/... @@ -144,6 +148,7 @@ jobs: env: KIND: 1 INGRESS_CLASS: ${{ matrix.ingress-class || matrix.ingress }}.ingress.networking.knative.dev + ENABLE_TLS: ${{ matrix.enable-tls || 0 }} steps: - name: Set up Go 1.17.x diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 1ae3c9c640d2..63089416954a 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -68,6 +68,10 @@ const ( // The port on which autoscaler WebSocket server listens. autoscalerPort = ":8080" + + certDirectory = "/var/lib/knative/certs" + certPath = certDirectory + "/tls.crt" + keyPath = certDirectory + "/tls.key" ) type config struct { @@ -241,6 +245,22 @@ func main() { }(name, server) } + // TODO: Use configmap to enable tls mode. + if true { + tlsServers := map[string]*http.Server{ + "https": pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah), + } + + for name, server := range tlsServers { + go func(name string, s *http.Server) { + // Don't forward ErrServerClosed as that indicates we're already shutting down. + if err := s.ListenAndServeTLS(certPath, keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) { + errCh <- fmt.Errorf("%s server failed: %w", name, err) + } + }(name, server) + } + } + // Wait for the signal to drain. select { case <-sigCtx.Done(): diff --git a/config/core/deployments/activator.yaml b/config/core/deployments/activator.yaml index 0a5a5b0dbcd2..989c2b26b83b 100644 --- a/config/core/deployments/activator.yaml +++ b/config/core/deployments/activator.yaml @@ -54,6 +54,11 @@ spec: cpu: 1000m memory: 600Mi + volumeMounts: + - name: server-certs + mountPath: /var/lib/knative/certs + readOnly: true + env: # Run Activator with GC collection when newly generated memory is 500%. - name: GOGC @@ -123,6 +128,11 @@ spec: # connections. terminationGracePeriodSeconds: 600 + volumes: + - name: server-certs + secret: + secretName: server-certs + --- apiVersion: v1 kind: Service @@ -148,6 +158,9 @@ spec: - name: http port: 80 targetPort: 8012 + - name: https + port: 443 + targetPort: 8112 - name: http2 port: 81 targetPort: 8013 diff --git a/pkg/networking/constants.go b/pkg/networking/constants.go index e85118bf939b..0fe1e5ea1048 100644 --- a/pkg/networking/constants.go +++ b/pkg/networking/constants.go @@ -26,6 +26,9 @@ const ( // BackendHTTP2Port is the backend, i.e. `targetPort` that we setup for HTTP/2 services. BackendHTTP2Port = 8013 + // BackendHTTPSPort is the backend. i.e. `targetPort` that we setup for HTTPS services. + BackendHTTPSPort = 8112 + // QueueAdminPort specifies the port number for // health check and lifecycle hooks for queue-proxy. QueueAdminPort = 8022 diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go index 13c272654d56..3952bf42f48f 100644 --- a/pkg/reconciler/route/resources/ingress.go +++ b/pkg/reconciler/route/resources/ingress.go @@ -135,6 +135,11 @@ func makeIngressSpec( rules := make([]netv1alpha1.IngressRule, 0, len(names)) featuresConfig := config.FromContextOrDefaults(ctx).Features + // Use ConfigMap to enable internalTLS. + internalTLS := false + if true { + internalTLS = true + } for _, name := range names { visibilities := []netv1alpha1.IngressVisibility{netv1alpha1.IngressVisibilityClusterLocal} @@ -148,7 +153,7 @@ func makeIngressSpec( return netv1alpha1.IngressSpec{}, err } rule := makeIngressRule(domains, r.Namespace, - visibility, tc.Targets[name], ro.RolloutsByTag(name)) + visibility, tc.Targets[name], ro.RolloutsByTag(name), internalTLS) if featuresConfig.TagHeaderBasedRouting == apicfg.Enabled { if rule.HTTP.Paths[0].AppendHeaders == nil { rule.HTTP.Paths[0].AppendHeaders = make(map[string]string, 1) @@ -170,7 +175,7 @@ func makeIngressSpec( // Since names are sorted `DefaultTarget == ""` is the first one, // so just pass the subslice. rule.HTTP.Paths = append( - makeTagBasedRoutingIngressPaths(r.Namespace, tc, ro, names[1:]), rule.HTTP.Paths...) + makeTagBasedRoutingIngressPaths(r.Namespace, tc, ro, internalTLS, names[1:]), rule.HTTP.Paths...) } else { // If a request is routed by a tag-attached hostname instead of the tag header, // the request may not have the tag header "Knative-Serving-Tag", @@ -263,24 +268,25 @@ func MakeACMEIngressPaths(acmeChallenges []netv1alpha1.HTTP01Challenge, domains func makeIngressRule(domains []string, ns string, visibility netv1alpha1.IngressVisibility, targets traffic.RevisionTargets, - roCfgs []*traffic.ConfigurationRollout) netv1alpha1.IngressRule { + roCfgs []*traffic.ConfigurationRollout, + internalTLS bool) netv1alpha1.IngressRule { return netv1alpha1.IngressRule{ Hosts: domains, Visibility: visibility, HTTP: &netv1alpha1.HTTPIngressRuleValue{ Paths: []netv1alpha1.HTTPIngressPath{ - *makeBaseIngressPath(ns, targets, roCfgs), + *makeBaseIngressPath(ns, targets, roCfgs, internalTLS), }, }, } } // `names` must not include `""` — the DefaultTarget. -func makeTagBasedRoutingIngressPaths(ns string, tc *traffic.Config, ro *traffic.Rollout, names []string) []netv1alpha1.HTTPIngressPath { +func makeTagBasedRoutingIngressPaths(ns string, tc *traffic.Config, ro *traffic.Rollout, internalTLS bool, names []string) []netv1alpha1.HTTPIngressPath { paths := make([]netv1alpha1.HTTPIngressPath, 0, len(names)) for _, name := range names { - path := makeBaseIngressPath(ns, tc.Targets[name], ro.RolloutsByTag(name)) + path := makeBaseIngressPath(ns, tc.Targets[name], ro.RolloutsByTag(name), internalTLS) path.Headers = map[string]netv1alpha1.HeaderMatch{network.TagHeaderName: {Exact: name}} paths = append(paths, *path) } @@ -300,7 +306,7 @@ func rolloutConfig(cfgName string, ros []*traffic.ConfigurationRollout) *traffic } func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, - roCfgs []*traffic.ConfigurationRollout) *netv1alpha1.HTTPIngressPath { + roCfgs []*traffic.ConfigurationRollout, internalTLS bool) *netv1alpha1.HTTPIngressPath { // Optimistically allocate |targets| elements. splits := make([]netv1alpha1.IngressBackendSplit, 0, len(targets)) for _, t := range targets { @@ -312,6 +318,10 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, if t.LatestRevision != nil && *t.LatestRevision { cfg = rolloutConfig(t.ConfigurationName, roCfgs) } + servicePort := intstr.FromInt(networking.ServicePort(t.Protocol)) + if internalTLS { + servicePort = intstr.FromInt(443) + } if cfg == nil || len(cfg.Revisions) < 2 { // No rollout in progress. splits = append(splits, netv1alpha1.IngressBackendSplit{ @@ -320,7 +330,7 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, ServiceName: t.RevisionName, // Port on the public service must match port on the activator. // Otherwise, the serverless services can't guarantee seamless positive handoff. - ServicePort: intstr.FromInt(networking.ServicePort(t.Protocol)), + ServicePort: servicePort, }, Percent: int(*t.Percent), AppendHeaders: map[string]string{ @@ -337,7 +347,7 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, ServiceName: rev.RevisionName, // Port on the public service must match port on the activator. // Otherwise, the serverless services can't guarantee seamless positive handoff. - ServicePort: intstr.FromInt(networking.ServicePort(t.Protocol)), + ServicePort: servicePort, }, Percent: rev.Percent, AppendHeaders: map[string]string{ diff --git a/pkg/reconciler/route/resources/ingress_test.go b/pkg/reconciler/route/resources/ingress_test.go index f955e0b08f56..fd4a65f1676a 100644 --- a/pkg/reconciler/route/resources/ingress_test.go +++ b/pkg/reconciler/route/resources/ingress_test.go @@ -866,8 +866,9 @@ func TestMakeIngressRuleVanilla(t *testing.T) { }, } ro := tc.BuildRollout() + internalTLS := false rule := makeIngressRule(domains, ns, - netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget)) + netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), internalTLS) expected := netv1alpha1.IngressRule{ Hosts: []string{ "a.com", @@ -919,8 +920,9 @@ func TestMakeIngressRuleZeroPercentTarget(t *testing.T) { }, } ro := tc.BuildRollout() + internalTLS := false rule := makeIngressRule(domains, ns, - netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget)) + netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), internalTLS) expected := netv1alpha1.IngressRule{ Hosts: []string{"test.org"}, HTTP: &netv1alpha1.HTTPIngressRuleValue{ @@ -968,9 +970,10 @@ func TestMakeIngressRuleTwoTargets(t *testing.T) { }, } ro := tc.BuildRollout() + internalTLS := false domains := []string{"test.org"} rule := makeIngressRule(domains, ns, netv1alpha1.IngressVisibilityExternalIP, - targets, ro.RolloutsByTag("a-tag")) + targets, ro.RolloutsByTag("a-tag"), internalTLS) expected := netv1alpha1.IngressRule{ Hosts: []string{"test.org"}, HTTP: &netv1alpha1.HTTPIngressRuleValue{ diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index a6f51f032986..1c07531b72c8 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -53,16 +53,32 @@ func MakePublicService(sks *v1alpha1.ServerlessService) *corev1.Service { OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(sks)}, }, Spec: corev1.ServiceSpec{ - Ports: []corev1.ServicePort{{ - Name: pkgnet.ServicePortName(sks.Spec.ProtocolType), - Protocol: corev1.ProtocolTCP, - Port: int32(pkgnet.ServicePort(sks.Spec.ProtocolType)), - TargetPort: targetPort(sks), - }}, + Ports: makePublicServicePorts(sks), }, } } +func makePublicServicePorts(sks *v1alpha1.ServerlessService) []corev1.ServicePort { + ports := []corev1.ServicePort{{ + Name: pkgnet.ServicePortName(sks.Spec.ProtocolType), + Protocol: corev1.ProtocolTCP, + Port: int32(pkgnet.ServicePort(sks.Spec.ProtocolType)), + TargetPort: targetPort(sks), + }} + + // TODO: Use annotation or sks.spec whether add HTTPS port or not. + if true { + p := corev1.ServicePort{ + Name: pkgnet.ServicePortNameHTTPS, + Protocol: corev1.ProtocolTCP, + Port: pkgnet.ServiceHTTPSPort, + TargetPort: intstr.FromInt(networking.BackendHTTPSPort), + } + ports = append(ports, p) + } + return ports +} + // MakePublicEndpoints constructs a K8s Endpoints that is not backed a selector // and will be manually reconciled by the SKS controller. func MakePublicEndpoints(sks *v1alpha1.ServerlessService, src *corev1.Endpoints) *corev1.Endpoints { @@ -99,11 +115,20 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core ret := make([]corev1.EndpointSubset, len(subsets)) for i, sss := range subsets { sst := sss.DeepCopy() + ssts := sss.DeepCopy() // Find the port we care about and remove all others. for j, p := range sst.Ports { if p.Port == targetPort { sst.Ports = sst.Ports[j : j+1] - break + } + } + + // TODO: Use annotation or configmap to add HTTPS port. + if true { + for j, p := range ssts.Ports { + if p.Port == networking.BackendHTTPSPort { + sst.Ports = append(sst.Ports, ssts.Ports[j:j+1]...) + } } } ret[i] = *sst @@ -134,6 +159,12 @@ func MakePrivateService(sks *v1alpha1.ServerlessService, selector map[string]str // This one is matching the public one, since this is the // port queue-proxy listens on. TargetPort: targetPort(sks), + }, { + // TODO: Add https port only when tls mode is enabled? + Name: pkgnet.ServicePortNameHTTPS, + Protocol: corev1.ProtocolTCP, + Port: pkgnet.ServiceHTTPSPort, + TargetPort: intstr.FromInt(networking.BackendHTTPSPort), }, { Name: servingv1.AutoscalingQueueMetricsPortName, Protocol: corev1.ProtocolTCP, diff --git a/test/config/tls/config-network.yaml b/test/config/tls/config-network.yaml new file mode 100644 index 000000000000..51c849bfcad3 --- /dev/null +++ b/test/config/tls/config-network.yaml @@ -0,0 +1,26 @@ +# Copyright 2022 The Knative 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 +# +# https://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. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-network + namespace: knative-serving + labels: + app.kubernetes.io/name: knative-serving + app.kubernetes.io/version: devel + serving.knative.dev/release: devel +data: + activator-ca: "serving-ca" + activator-san: "knative" diff --git a/test/e2e-common.sh b/test/e2e-common.sh index 9aafaa795088..82e595443896 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -50,7 +50,8 @@ export TMP_DIR="${TMP_DIR:-$(mktemp -d -t ci-$(date +%Y-%m-%d-%H-%M-%S)-XXXXXXXX readonly E2E_YAML_DIR="${TMP_DIR}/e2e-yaml" # This the namespace used to install Knative Serving. Use generated UUID as namespace. -export SYSTEM_NAMESPACE="${SYSTEM_NAMESPACE:-$(uuidgen | tr 'A-Z' 'a-z')}" +#export SYSTEM_NAMESPACE="${SYSTEM_NAMESPACE:-$(uuidgen | tr 'A-Z' 'a-z')}" +export SYSTEM_NAMESPACE="knative-serving" # Keep this in sync with test/ha/ha.go readonly REPLICAS=3 diff --git a/test/generate-cert.sh b/test/generate-cert.sh new file mode 100755 index 000000000000..b08bc422b72c --- /dev/null +++ b/test/generate-cert.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash + +# Copyright 2022 The Knative 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. + +SERVING_SYSTEM_NAMESPACE=knative-serving +TEST_NAMESPACE=serving-tests +out_dir="$(mktemp -d /tmp/certs-XXX)" +san="knative" + +kubectl create ns $SERVING_SYSTEM_NAMESPACE + +# Generate Root key and cert. +openssl req -x509 -sha256 -nodes -days 365 -newkey rsa:2048 -subj '/O=Example/CN=Example' -keyout "${out_dir}"/root.key -out "${out_dir}"/root.crt + +# Create server key +openssl req -out "${out_dir}"/tls.csr -newkey rsa:2048 -nodes -keyout "${out_dir}"/tls.key -subj "/CN=Example/O=Example" -addext "subjectAltName = DNS:$san" + +# Create server certs +openssl x509 -req -extfile <(printf "subjectAltName=DNS:$san") -days 365 -in "${out_dir}"/tls.csr -CA "${out_dir}"/root.crt -CAkey "${out_dir}"/root.key -CAcreateserial -out "${out_dir}"/tls.crt + +# Create secret +kubectl create -n ${SERVING_SYSTEM_NAMESPACE} secret generic serving-ca \ + --from-file=ca.crt="${out_dir}"/root.crt --dry-run=client -o yaml | kubectl apply -f - + +kubectl create -n ${SERVING_SYSTEM_NAMESPACE} secret tls server-certs \ + --key="${out_dir}"/tls.key \ + --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - + +#kubectl create ns ${TEST_NAMESPACE} +#kubectl create -n ${TEST_NAMESPACE} secret tls server-certs \ +# --key="${out_dir}"/tls.key \ +# --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - From 608b17bb6900361196b509a0c3c8afb1b32f8c14 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 30 Mar 2022 14:34:33 +0900 Subject: [PATCH 02/29] Separate github action --- .github/workflows/kind-e2e-tls.yaml | 189 ++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 .github/workflows/kind-e2e-tls.yaml diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml new file mode 100644 index 000000000000..7c0d5dfb08ac --- /dev/null +++ b/.github/workflows/kind-e2e-tls.yaml @@ -0,0 +1,189 @@ +name: KinD e2e tests - tls + +on: + pull_request: + branches: [ 'main', 'release-*' ] + +defaults: + run: + shell: bash + working-directory: ./src/knative.dev/serving + +jobs: + + e2e-tests: + name: e2e tests + runs-on: ubuntu-latest + strategy: + fail-fast: false # Keep running if one leg fails. + matrix: + cluster: + - v1.21.1/kourier + + test-suite: + - ./test/conformance/runtime + - ./test/conformance/api/... + - ./test/e2e + + include: + # Map between K8s and KinD versions. + # This is attempting to make it a bit clearer what's being tested. + # See: https://github.com/kubernetes-sigs/kind/releases + - cluster: v1.21.1/kourier + k8s-version: v1.21.1 + kind-version: v0.11.1 + kind-image-sha: sha256:69860bda5563ac81e3c0057d654b5253219618a22ec3a346306239bba8cfa1a6 + kingress: kourier + test-flags: "--enable-alpha --enable-beta" + cluster-suffix: c${{ github.run_id }}.local + + env: + GOPATH: ${{ github.workspace }} + GO111MODULE: on + GOFLAGS: -tags=nostackdriver + # https://github.com/google/go-containerregistry/pull/125 allows insecure registry for + # '*.local' hostnames. This works both for `ko` and our own tag-to-digest resolution logic, + # thus allowing us to test without bypassing tag-to-digest resolution. + REGISTRY_NAME: registry.local + REGISTRY_PORT: 5000 + KO_DOCKER_REPO: registry.local:5000/knative + + steps: + - name: Set up Go 1.17.x + uses: actions/setup-go@v2 + with: + go-version: 1.17.x + + - name: Install Dependencies + working-directory: ./ + run: | + echo '::group:: install ko' + curl -L https://github.com/google/ko/releases/download/v0.6.0/ko_0.6.0_Linux_x86_64.tar.gz | tar xzf - ko + chmod +x ./ko + sudo mv ko /usr/local/bin + echo '::endgroup::' + + - name: Check out code onto GOPATH + uses: actions/checkout@v2 + with: + path: ./src/knative.dev/serving + + - name: Install KinD + run: | + set -x + + # Disable swap otherwise memory enforcement doesn't work + # See: https://kubernetes.slack.com/archives/CEKK1KTN2/p1600009955324200 + sudo swapoff -a + sudo rm -f /swapfile + + # Use in-memory storage to avoid etcd server timeouts. + # https://kubernetes.slack.com/archives/CEKK1KTN2/p1615134111016300 + # https://github.com/kubernetes-sigs/kind/issues/845 + sudo mkdir -p /tmp/etcd + sudo mount -t tmpfs tmpfs /tmp/etcd + + curl -Lo ./kind https://github.com/kubernetes-sigs/kind/releases/download/${{ matrix.kind-version }}/kind-$(uname)-amd64 + chmod +x ./kind + sudo mv kind /usr/local/bin + + - name: Configure KinD Cluster + working-directory: ./src/knative.dev/serving + run: | + set -x + + # KinD configuration. + cat > kind.yaml < 127.0.0.1, to tell `ko` to publish to + # local reigstry, even when pushing $REGISTRY_NAME:$REGISTRY_PORT/some/image + sudo echo "127.0.0.1 $REGISTRY_NAME" | sudo tee -a /etc/hosts + + - name: Install Serving & Ingress + working-directory: ./src/knative.dev/serving + run: | + source ./test/e2e-common.sh + + # Generate TLS cert + bash ./test/generate-cert.sh + + KIND=1 + INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" + CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" + + knative_setup + test_setup + + kubectl apply -f ./test/config/tls + + echo "INGRESS_CLASS=$INGRESS_CLASS" >> $GITHUB_ENV + echo "CLUSTER_DOMAIN=$CLUSTER_DOMAIN" >> $GITHUB_ENV + echo "SYSTEM_NAMESPACE=$SYSTEM_NAMESPACE" >> $GITHUB_ENV + echo "GATEWAY_OVERRIDE=$GATEWAY_OVERRIDE" >> $GITHUB_ENV + echo "GATEWAY_NAMESPACE_OVERRIDE=$GATEWAY_NAMESPACE_OVERRIDE" >> $GITHUB_ENV + + - name: Run Test + working-directory: ./src/knative.dev/serving + run: | + set -x + + source ./test/e2e-common.sh + + # Exclude the control-plane node, which doesn't seem to expose the nodeport service. + IPS=( $(kubectl get nodes -lkubernetes.io/hostname!=kind-control-plane -ojsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') ) + # Run the tests tagged as e2e on the KinD cluster. + # TODO: Remove feature toogle when emptyDir is enabled by default + toggle_feature kubernetes.podspec-volumes-emptydir Enabled + go test -race -count=1 -parallel=1 -timeout=30m -tags=e2e ${{ matrix.test-suite }} \ + --ingressendpoint="${IPS[0]}" \ + ${{ matrix.test-flags }} + toggle_feature kubernetes.podspec-volumes-emptydir Disabled From 888d38e818579a3a2d6c456e6df9b5b1b241deb1 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 30 Mar 2022 14:40:46 +0900 Subject: [PATCH 03/29] Use fileExists func --- cmd/activator/main.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 63089416954a..b51749f162c6 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -245,8 +245,8 @@ func main() { }(name, server) } - // TODO: Use configmap to enable tls mode. - if true { + // Enable TLS server when activator server certs are mounted. + if fileExists(certPath) && fileExists(keyPath) { tlsServers := map[string]*http.Server{ "https": pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah), } @@ -282,6 +282,11 @@ func main() { logger.Info("Servers shutdown.") } +func fileExists(filename string) bool { + _, err := os.Stat(filename) + return err == nil +} + func newHealthCheck(sigCtx context.Context, logger *zap.SugaredLogger, statSink *websocket.ManagedConnection) func() error { once := sync.Once{} return func() error { From 6f4efa10ed0b392ecdc7d912dd0167b5c12776d2 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 30 Mar 2022 15:14:19 +0900 Subject: [PATCH 04/29] Use patch --- .github/workflows/kind-e2e-tls.yaml | 3 +-- config/core/deployments/activator.yaml | 13 ------------- test/config/tls/service-patch.yaml | 5 +++++ test/config/tls/volume-patch.yaml | 13 +++++++++++++ test/e2e-common.sh | 9 +++++++++ 5 files changed, 28 insertions(+), 15 deletions(-) create mode 100644 test/config/tls/service-patch.yaml create mode 100644 test/config/tls/volume-patch.yaml diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml index 7c0d5dfb08ac..f072bf77d5f0 100644 --- a/.github/workflows/kind-e2e-tls.yaml +++ b/.github/workflows/kind-e2e-tls.yaml @@ -159,12 +159,11 @@ jobs: KIND=1 INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" + ENABLE_TLS=1 knative_setup test_setup - kubectl apply -f ./test/config/tls - echo "INGRESS_CLASS=$INGRESS_CLASS" >> $GITHUB_ENV echo "CLUSTER_DOMAIN=$CLUSTER_DOMAIN" >> $GITHUB_ENV echo "SYSTEM_NAMESPACE=$SYSTEM_NAMESPACE" >> $GITHUB_ENV diff --git a/config/core/deployments/activator.yaml b/config/core/deployments/activator.yaml index 989c2b26b83b..0a5a5b0dbcd2 100644 --- a/config/core/deployments/activator.yaml +++ b/config/core/deployments/activator.yaml @@ -54,11 +54,6 @@ spec: cpu: 1000m memory: 600Mi - volumeMounts: - - name: server-certs - mountPath: /var/lib/knative/certs - readOnly: true - env: # Run Activator with GC collection when newly generated memory is 500%. - name: GOGC @@ -128,11 +123,6 @@ spec: # connections. terminationGracePeriodSeconds: 600 - volumes: - - name: server-certs - secret: - secretName: server-certs - --- apiVersion: v1 kind: Service @@ -158,9 +148,6 @@ spec: - name: http port: 80 targetPort: 8012 - - name: https - port: 443 - targetPort: 8112 - name: http2 port: 81 targetPort: 8013 diff --git a/test/config/tls/service-patch.yaml b/test/config/tls/service-patch.yaml new file mode 100644 index 000000000000..3dbc93d869ac --- /dev/null +++ b/test/config/tls/service-patch.yaml @@ -0,0 +1,5 @@ +spec: + ports: + - name: https + port: 443 + targetPort: 8112 diff --git a/test/config/tls/volume-patch.yaml b/test/config/tls/volume-patch.yaml new file mode 100644 index 000000000000..fdb715dc8510 --- /dev/null +++ b/test/config/tls/volume-patch.yaml @@ -0,0 +1,13 @@ +spec: + template: + spec: + volumes: + - name: server-certs + secret: + secretName: server-certs + containers: + - name: activator + volumeMounts: + - name: server-certs + mountPath: /var/lib/knative/certs + readOnly: true diff --git a/test/e2e-common.sh b/test/e2e-common.sh index 82e595443896..aca06a8dbd5e 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -34,6 +34,7 @@ export RUN_HTTP01_AUTO_TLS_TESTS=0 export HTTPS=0 export SHORT=0 export ENABLE_HA=0 +export ENABLE_TLS=0 export MESH=0 export PERF=0 export KIND=${KIND:-0} @@ -357,6 +358,14 @@ function install() { # kubectl -n ${SYSTEM_NAMESPACE} delete leases --all wait_for_leader_controller || return 1 fi + + if (( ENABLE_TLS )); then + echo "Patch to activator to serve TLS" + kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/volume-patch.yaml)" + kubectl patch service -n ${SYSTEM_NAMESPACE} activator-service --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/service-patch.yaml)" + kubectl apply -n ${SYSTEM_NAMESPACE} -f ${REPO_ROOT_DIR}/test/config/tls/config-network.yaml + fi + } # Check if we should use --resolvabledomain. In case the ingress only has From 7097b034f98eda272135fe81ad3452ae61ff1b93 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 30 Mar 2022 23:07:24 +0900 Subject: [PATCH 05/29] Drop todo --- pkg/reconciler/route/resources/ingress.go | 4 ++-- .../serverlessservice/resources/services.go | 17 +++++------------ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go index 3952bf42f48f..c790d1a47a1e 100644 --- a/pkg/reconciler/route/resources/ingress.go +++ b/pkg/reconciler/route/resources/ingress.go @@ -135,9 +135,9 @@ func makeIngressSpec( rules := make([]netv1alpha1.IngressRule, 0, len(names)) featuresConfig := config.FromContextOrDefaults(ctx).Features - // Use ConfigMap to enable internalTLS. + networkConfig := config.FromContextOrDefaults(ctx).Network internalTLS := false - if true { + if activatorCA := networkConfig.ActivatorCA; activatorCA != "" { internalTLS = true } diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 1c07531b72c8..f7637059901c 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -64,18 +64,12 @@ func makePublicServicePorts(sks *v1alpha1.ServerlessService) []corev1.ServicePor Protocol: corev1.ProtocolTCP, Port: int32(pkgnet.ServicePort(sks.Spec.ProtocolType)), TargetPort: targetPort(sks), + }, { + Name: pkgnet.ServicePortNameHTTPS, + Protocol: corev1.ProtocolTCP, + Port: pkgnet.ServiceHTTPSPort, + TargetPort: intstr.FromInt(networking.BackendHTTPSPort), }} - - // TODO: Use annotation or sks.spec whether add HTTPS port or not. - if true { - p := corev1.ServicePort{ - Name: pkgnet.ServicePortNameHTTPS, - Protocol: corev1.ProtocolTCP, - Port: pkgnet.ServiceHTTPSPort, - TargetPort: intstr.FromInt(networking.BackendHTTPSPort), - } - ports = append(ports, p) - } return ports } @@ -160,7 +154,6 @@ func MakePrivateService(sks *v1alpha1.ServerlessService, selector map[string]str // port queue-proxy listens on. TargetPort: targetPort(sks), }, { - // TODO: Add https port only when tls mode is enabled? Name: pkgnet.ServicePortNameHTTPS, Protocol: corev1.ProtocolTCP, Port: pkgnet.ServiceHTTPSPort, From 3551a842b0b4fa6872d6abf8578f1cbb94db9307 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 31 Mar 2022 09:57:27 +0900 Subject: [PATCH 06/29] Clean up --- .github/workflows/kind-e2e-tls.yaml | 3 --- .../serverlessservice/resources/services.go | 17 +++++--------- .../resources/services_test.go | 22 ++++++++++++++++++- .../serverlessservice_test.go | 6 ++--- test/e2e-common.sh | 3 +++ 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml index f072bf77d5f0..837fab3d0710 100644 --- a/.github/workflows/kind-e2e-tls.yaml +++ b/.github/workflows/kind-e2e-tls.yaml @@ -153,9 +153,6 @@ jobs: run: | source ./test/e2e-common.sh - # Generate TLS cert - bash ./test/generate-cert.sh - KIND=1 INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index f7637059901c..f9ec28bc7d9b 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -108,23 +108,18 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core } ret := make([]corev1.EndpointSubset, len(subsets)) for i, sss := range subsets { + org := sss.DeepCopy() sst := sss.DeepCopy() - ssts := sss.DeepCopy() // Find the port we care about and remove all others. - for j, p := range sst.Ports { + for j, p := range org.Ports { if p.Port == targetPort { - sst.Ports = sst.Ports[j : j+1] + sst.Ports = org.Ports[j : j+1] } - } - - // TODO: Use annotation or configmap to add HTTPS port. - if true { - for j, p := range ssts.Ports { - if p.Port == networking.BackendHTTPSPort { - sst.Ports = append(sst.Ports, ssts.Ports[j:j+1]...) - } + if p.Port == networking.BackendHTTPSPort { + sst.Ports = append(sst.Ports, org.Ports[j:j+1]...) } } + ret[i] = *sst } return ret diff --git a/pkg/reconciler/serverlessservice/resources/services_test.go b/pkg/reconciler/serverlessservice/resources/services_test.go index d24dabb6c5af..8c78fc71d5eb 100644 --- a/pkg/reconciler/serverlessservice/resources/services_test.go +++ b/pkg/reconciler/serverlessservice/resources/services_test.go @@ -112,6 +112,11 @@ func svc(t networking.ServiceType, mods ...func(*corev1.Service)) *corev1.Servic Protocol: corev1.ProtocolTCP, Port: pkgnet.ServiceHTTPPort, TargetPort: intstr.FromInt(networking.BackendHTTPPort), + }, { + Name: pkgnet.ServicePortNameHTTPS, + Protocol: corev1.ProtocolTCP, + Port: pkgnet.ServiceHTTPSPort, + TargetPort: intstr.FromInt(networking.BackendHTTPSPort), }}, }, } @@ -181,6 +186,11 @@ func TestMakePublicService(t *testing.T) { Protocol: corev1.ProtocolTCP, Port: pkgnet.ServiceHTTP2Port, TargetPort: intstr.FromInt(networking.BackendHTTP2Port), + }, { + Name: pkgnet.ServicePortNameHTTPS, + Protocol: corev1.ProtocolTCP, + Port: pkgnet.ServiceHTTPSPort, + TargetPort: intstr.FromInt(networking.BackendHTTPSPort), }} s.Annotations = map[string]string{"cherub": "rock"} s.OwnerReferences[0].UID = "1988" @@ -196,6 +206,11 @@ func TestMakePublicService(t *testing.T) { Protocol: corev1.ProtocolTCP, Port: pkgnet.ServiceHTTP2Port, TargetPort: intstr.FromInt(networking.BackendHTTP2Port), + }, { + Name: pkgnet.ServicePortNameHTTPS, + Protocol: corev1.ProtocolTCP, + Port: pkgnet.ServiceHTTPSPort, + TargetPort: intstr.FromInt(networking.BackendHTTPSPort), }} }), }, { @@ -211,6 +226,11 @@ func TestMakePublicService(t *testing.T) { Protocol: corev1.ProtocolTCP, Port: pkgnet.ServiceHTTP2Port, TargetPort: intstr.FromInt(networking.BackendHTTP2Port), + }, { + Name: pkgnet.ServicePortNameHTTPS, + Protocol: corev1.ProtocolTCP, + Port: pkgnet.ServiceHTTPSPort, + TargetPort: intstr.FromInt(networking.BackendHTTPSPort), }} s.Labels["infinite"] = "sadness" }), @@ -419,7 +439,7 @@ func TestMakePrivateService(t *testing.T) { Port: pkgnet.ServiceHTTPPort, TargetPort: intstr.FromInt(networking.BackendHTTP2Port), } - s.Spec.Ports[4] = corev1.ServicePort{ + s.Spec.Ports[5] = corev1.ServicePort{ Name: pkgnet.ServicePortNameH2C + "-istio", Protocol: corev1.ProtocolTCP, Port: networking.BackendHTTP2Port, diff --git a/pkg/reconciler/serverlessservice/serverlessservice_test.go b/pkg/reconciler/serverlessservice/serverlessservice_test.go index a20317c74b24..90e32b9bd1cb 100644 --- a/pkg/reconciler/serverlessservice/serverlessservice_test.go +++ b/pkg/reconciler/serverlessservice/serverlessservice_test.go @@ -769,9 +769,9 @@ func withHTTP2Priv(svc *corev1.Service) { svc.Spec.Ports[0].Name = "http2" svc.Spec.Ports[0].TargetPort = intstr.FromInt(networking.BackendHTTP2Port) - svc.Spec.Ports[4].Name = "http2-istio" - svc.Spec.Ports[4].Port = networking.BackendHTTP2Port - svc.Spec.Ports[4].TargetPort = intstr.FromInt(networking.BackendHTTP2Port) + svc.Spec.Ports[5].Name = "http2-istio" + svc.Spec.Ports[5].Port = networking.BackendHTTP2Port + svc.Spec.Ports[5].TargetPort = intstr.FromInt(networking.BackendHTTP2Port) } func withHTTP2(svc *corev1.Service) { diff --git a/test/e2e-common.sh b/test/e2e-common.sh index aca06a8dbd5e..86af653a894a 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -360,6 +360,9 @@ function install() { fi if (( ENABLE_TLS )); then + echo "Generate certificates" + bash ${REPO_ROOT_DIR}/test/generate-cert.sh + echo "Patch to activator to serve TLS" kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/volume-patch.yaml)" kubectl patch service -n ${SYSTEM_NAMESPACE} activator-service --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/service-patch.yaml)" From 9608d7c129d4b36037499c5daca19acedc4a555f Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 31 Mar 2022 11:21:41 +0900 Subject: [PATCH 07/29] Drop kind for tls --- .github/workflows/kind-e2e-tls.yaml | 185 ---------------------------- 1 file changed, 185 deletions(-) delete mode 100644 .github/workflows/kind-e2e-tls.yaml diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml deleted file mode 100644 index 837fab3d0710..000000000000 --- a/.github/workflows/kind-e2e-tls.yaml +++ /dev/null @@ -1,185 +0,0 @@ -name: KinD e2e tests - tls - -on: - pull_request: - branches: [ 'main', 'release-*' ] - -defaults: - run: - shell: bash - working-directory: ./src/knative.dev/serving - -jobs: - - e2e-tests: - name: e2e tests - runs-on: ubuntu-latest - strategy: - fail-fast: false # Keep running if one leg fails. - matrix: - cluster: - - v1.21.1/kourier - - test-suite: - - ./test/conformance/runtime - - ./test/conformance/api/... - - ./test/e2e - - include: - # Map between K8s and KinD versions. - # This is attempting to make it a bit clearer what's being tested. - # See: https://github.com/kubernetes-sigs/kind/releases - - cluster: v1.21.1/kourier - k8s-version: v1.21.1 - kind-version: v0.11.1 - kind-image-sha: sha256:69860bda5563ac81e3c0057d654b5253219618a22ec3a346306239bba8cfa1a6 - kingress: kourier - test-flags: "--enable-alpha --enable-beta" - cluster-suffix: c${{ github.run_id }}.local - - env: - GOPATH: ${{ github.workspace }} - GO111MODULE: on - GOFLAGS: -tags=nostackdriver - # https://github.com/google/go-containerregistry/pull/125 allows insecure registry for - # '*.local' hostnames. This works both for `ko` and our own tag-to-digest resolution logic, - # thus allowing us to test without bypassing tag-to-digest resolution. - REGISTRY_NAME: registry.local - REGISTRY_PORT: 5000 - KO_DOCKER_REPO: registry.local:5000/knative - - steps: - - name: Set up Go 1.17.x - uses: actions/setup-go@v2 - with: - go-version: 1.17.x - - - name: Install Dependencies - working-directory: ./ - run: | - echo '::group:: install ko' - curl -L https://github.com/google/ko/releases/download/v0.6.0/ko_0.6.0_Linux_x86_64.tar.gz | tar xzf - ko - chmod +x ./ko - sudo mv ko /usr/local/bin - echo '::endgroup::' - - - name: Check out code onto GOPATH - uses: actions/checkout@v2 - with: - path: ./src/knative.dev/serving - - - name: Install KinD - run: | - set -x - - # Disable swap otherwise memory enforcement doesn't work - # See: https://kubernetes.slack.com/archives/CEKK1KTN2/p1600009955324200 - sudo swapoff -a - sudo rm -f /swapfile - - # Use in-memory storage to avoid etcd server timeouts. - # https://kubernetes.slack.com/archives/CEKK1KTN2/p1615134111016300 - # https://github.com/kubernetes-sigs/kind/issues/845 - sudo mkdir -p /tmp/etcd - sudo mount -t tmpfs tmpfs /tmp/etcd - - curl -Lo ./kind https://github.com/kubernetes-sigs/kind/releases/download/${{ matrix.kind-version }}/kind-$(uname)-amd64 - chmod +x ./kind - sudo mv kind /usr/local/bin - - - name: Configure KinD Cluster - working-directory: ./src/knative.dev/serving - run: | - set -x - - # KinD configuration. - cat > kind.yaml < 127.0.0.1, to tell `ko` to publish to - # local reigstry, even when pushing $REGISTRY_NAME:$REGISTRY_PORT/some/image - sudo echo "127.0.0.1 $REGISTRY_NAME" | sudo tee -a /etc/hosts - - - name: Install Serving & Ingress - working-directory: ./src/knative.dev/serving - run: | - source ./test/e2e-common.sh - - KIND=1 - INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" - CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" - ENABLE_TLS=1 - - knative_setup - test_setup - - echo "INGRESS_CLASS=$INGRESS_CLASS" >> $GITHUB_ENV - echo "CLUSTER_DOMAIN=$CLUSTER_DOMAIN" >> $GITHUB_ENV - echo "SYSTEM_NAMESPACE=$SYSTEM_NAMESPACE" >> $GITHUB_ENV - echo "GATEWAY_OVERRIDE=$GATEWAY_OVERRIDE" >> $GITHUB_ENV - echo "GATEWAY_NAMESPACE_OVERRIDE=$GATEWAY_NAMESPACE_OVERRIDE" >> $GITHUB_ENV - - - name: Run Test - working-directory: ./src/knative.dev/serving - run: | - set -x - - source ./test/e2e-common.sh - - # Exclude the control-plane node, which doesn't seem to expose the nodeport service. - IPS=( $(kubectl get nodes -lkubernetes.io/hostname!=kind-control-plane -ojsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') ) - # Run the tests tagged as e2e on the KinD cluster. - # TODO: Remove feature toogle when emptyDir is enabled by default - toggle_feature kubernetes.podspec-volumes-emptydir Enabled - go test -race -count=1 -parallel=1 -timeout=30m -tags=e2e ${{ matrix.test-suite }} \ - --ingressendpoint="${IPS[0]}" \ - ${{ matrix.test-flags }} - toggle_feature kubernetes.podspec-volumes-emptydir Disabled From 35c74dbc128a42b5fde5d441a5b6c0e1ab7072fd Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 31 Mar 2022 12:08:30 +0900 Subject: [PATCH 08/29] Fix namespace --- test/config/tls/config-network.yaml | 1 - test/e2e-common.sh | 3 +-- test/generate-cert.sh | 7 ++----- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/test/config/tls/config-network.yaml b/test/config/tls/config-network.yaml index 51c849bfcad3..c939db8f9553 100644 --- a/test/config/tls/config-network.yaml +++ b/test/config/tls/config-network.yaml @@ -16,7 +16,6 @@ apiVersion: v1 kind: ConfigMap metadata: name: config-network - namespace: knative-serving labels: app.kubernetes.io/name: knative-serving app.kubernetes.io/version: devel diff --git a/test/e2e-common.sh b/test/e2e-common.sh index 86af653a894a..faafd964111e 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -51,8 +51,7 @@ export TMP_DIR="${TMP_DIR:-$(mktemp -d -t ci-$(date +%Y-%m-%d-%H-%M-%S)-XXXXXXXX readonly E2E_YAML_DIR="${TMP_DIR}/e2e-yaml" # This the namespace used to install Knative Serving. Use generated UUID as namespace. -#export SYSTEM_NAMESPACE="${SYSTEM_NAMESPACE:-$(uuidgen | tr 'A-Z' 'a-z')}" -export SYSTEM_NAMESPACE="knative-serving" +export SYSTEM_NAMESPACE="${SYSTEM_NAMESPACE:-$(uuidgen | tr 'A-Z' 'a-z')}" # Keep this in sync with test/ha/ha.go readonly REPLICAS=3 diff --git a/test/generate-cert.sh b/test/generate-cert.sh index b08bc422b72c..5055e551f8e2 100755 --- a/test/generate-cert.sh +++ b/test/generate-cert.sh @@ -14,13 +14,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -SERVING_SYSTEM_NAMESPACE=knative-serving TEST_NAMESPACE=serving-tests out_dir="$(mktemp -d /tmp/certs-XXX)" san="knative" -kubectl create ns $SERVING_SYSTEM_NAMESPACE - # Generate Root key and cert. openssl req -x509 -sha256 -nodes -days 365 -newkey rsa:2048 -subj '/O=Example/CN=Example' -keyout "${out_dir}"/root.key -out "${out_dir}"/root.crt @@ -31,10 +28,10 @@ openssl req -out "${out_dir}"/tls.csr -newkey rsa:2048 -nodes -keyout "${out_dir openssl x509 -req -extfile <(printf "subjectAltName=DNS:$san") -days 365 -in "${out_dir}"/tls.csr -CA "${out_dir}"/root.crt -CAkey "${out_dir}"/root.key -CAcreateserial -out "${out_dir}"/tls.crt # Create secret -kubectl create -n ${SERVING_SYSTEM_NAMESPACE} secret generic serving-ca \ +kubectl create -n ${SYSTEM_NAMESPACE} secret generic serving-ca \ --from-file=ca.crt="${out_dir}"/root.crt --dry-run=client -o yaml | kubectl apply -f - -kubectl create -n ${SERVING_SYSTEM_NAMESPACE} secret tls server-certs \ +kubectl create -n ${SYSTEM_NAMESPACE} secret tls server-certs \ --key="${out_dir}"/tls.key \ --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - From 861af78103d524b88c5385accd328ef383616a75 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 4 Apr 2022 08:51:29 +0900 Subject: [PATCH 09/29] Revert "Drop kind for tls" This reverts commit 02560110a5e2a3387b1e365f6b15df74a70c814b. --- .github/workflows/kind-e2e-tls.yaml | 185 ++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 .github/workflows/kind-e2e-tls.yaml diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml new file mode 100644 index 000000000000..837fab3d0710 --- /dev/null +++ b/.github/workflows/kind-e2e-tls.yaml @@ -0,0 +1,185 @@ +name: KinD e2e tests - tls + +on: + pull_request: + branches: [ 'main', 'release-*' ] + +defaults: + run: + shell: bash + working-directory: ./src/knative.dev/serving + +jobs: + + e2e-tests: + name: e2e tests + runs-on: ubuntu-latest + strategy: + fail-fast: false # Keep running if one leg fails. + matrix: + cluster: + - v1.21.1/kourier + + test-suite: + - ./test/conformance/runtime + - ./test/conformance/api/... + - ./test/e2e + + include: + # Map between K8s and KinD versions. + # This is attempting to make it a bit clearer what's being tested. + # See: https://github.com/kubernetes-sigs/kind/releases + - cluster: v1.21.1/kourier + k8s-version: v1.21.1 + kind-version: v0.11.1 + kind-image-sha: sha256:69860bda5563ac81e3c0057d654b5253219618a22ec3a346306239bba8cfa1a6 + kingress: kourier + test-flags: "--enable-alpha --enable-beta" + cluster-suffix: c${{ github.run_id }}.local + + env: + GOPATH: ${{ github.workspace }} + GO111MODULE: on + GOFLAGS: -tags=nostackdriver + # https://github.com/google/go-containerregistry/pull/125 allows insecure registry for + # '*.local' hostnames. This works both for `ko` and our own tag-to-digest resolution logic, + # thus allowing us to test without bypassing tag-to-digest resolution. + REGISTRY_NAME: registry.local + REGISTRY_PORT: 5000 + KO_DOCKER_REPO: registry.local:5000/knative + + steps: + - name: Set up Go 1.17.x + uses: actions/setup-go@v2 + with: + go-version: 1.17.x + + - name: Install Dependencies + working-directory: ./ + run: | + echo '::group:: install ko' + curl -L https://github.com/google/ko/releases/download/v0.6.0/ko_0.6.0_Linux_x86_64.tar.gz | tar xzf - ko + chmod +x ./ko + sudo mv ko /usr/local/bin + echo '::endgroup::' + + - name: Check out code onto GOPATH + uses: actions/checkout@v2 + with: + path: ./src/knative.dev/serving + + - name: Install KinD + run: | + set -x + + # Disable swap otherwise memory enforcement doesn't work + # See: https://kubernetes.slack.com/archives/CEKK1KTN2/p1600009955324200 + sudo swapoff -a + sudo rm -f /swapfile + + # Use in-memory storage to avoid etcd server timeouts. + # https://kubernetes.slack.com/archives/CEKK1KTN2/p1615134111016300 + # https://github.com/kubernetes-sigs/kind/issues/845 + sudo mkdir -p /tmp/etcd + sudo mount -t tmpfs tmpfs /tmp/etcd + + curl -Lo ./kind https://github.com/kubernetes-sigs/kind/releases/download/${{ matrix.kind-version }}/kind-$(uname)-amd64 + chmod +x ./kind + sudo mv kind /usr/local/bin + + - name: Configure KinD Cluster + working-directory: ./src/knative.dev/serving + run: | + set -x + + # KinD configuration. + cat > kind.yaml < 127.0.0.1, to tell `ko` to publish to + # local reigstry, even when pushing $REGISTRY_NAME:$REGISTRY_PORT/some/image + sudo echo "127.0.0.1 $REGISTRY_NAME" | sudo tee -a /etc/hosts + + - name: Install Serving & Ingress + working-directory: ./src/knative.dev/serving + run: | + source ./test/e2e-common.sh + + KIND=1 + INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" + CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" + ENABLE_TLS=1 + + knative_setup + test_setup + + echo "INGRESS_CLASS=$INGRESS_CLASS" >> $GITHUB_ENV + echo "CLUSTER_DOMAIN=$CLUSTER_DOMAIN" >> $GITHUB_ENV + echo "SYSTEM_NAMESPACE=$SYSTEM_NAMESPACE" >> $GITHUB_ENV + echo "GATEWAY_OVERRIDE=$GATEWAY_OVERRIDE" >> $GITHUB_ENV + echo "GATEWAY_NAMESPACE_OVERRIDE=$GATEWAY_NAMESPACE_OVERRIDE" >> $GITHUB_ENV + + - name: Run Test + working-directory: ./src/knative.dev/serving + run: | + set -x + + source ./test/e2e-common.sh + + # Exclude the control-plane node, which doesn't seem to expose the nodeport service. + IPS=( $(kubectl get nodes -lkubernetes.io/hostname!=kind-control-plane -ojsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') ) + # Run the tests tagged as e2e on the KinD cluster. + # TODO: Remove feature toogle when emptyDir is enabled by default + toggle_feature kubernetes.podspec-volumes-emptydir Enabled + go test -race -count=1 -parallel=1 -timeout=30m -tags=e2e ${{ matrix.test-suite }} \ + --ingressendpoint="${IPS[0]}" \ + ${{ matrix.test-flags }} + toggle_feature kubernetes.podspec-volumes-emptydir Disabled From 3c8ec24c7b63640b5f778d57448f4391b2d46af8 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 31 Mar 2022 11:21:41 +0900 Subject: [PATCH 10/29] Fix filterSubsetPorts --- .github/workflows/kind-e2e-tls.yaml | 25 ++++++++++--- cmd/activator/main.go | 7 ++-- .../serverlessservice/resources/services.go | 13 +++---- .../resources/services_test.go | 36 +++++++++++++++++++ test/e2e/autoscale_test.go | 12 +++++++ test/generate-cert.sh | 1 + 6 files changed, 81 insertions(+), 13 deletions(-) diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml index 837fab3d0710..55a1ace2df8b 100644 --- a/.github/workflows/kind-e2e-tls.yaml +++ b/.github/workflows/kind-e2e-tls.yaml @@ -1,4 +1,4 @@ -name: KinD e2e tests - tls +name: KinD e2e tests -tls on: pull_request: @@ -18,7 +18,7 @@ jobs: fail-fast: false # Keep running if one leg fails. matrix: cluster: - - v1.21.1/kourier + - v1.23.0/kourier test-suite: - ./test/conformance/runtime @@ -29,10 +29,10 @@ jobs: # Map between K8s and KinD versions. # This is attempting to make it a bit clearer what's being tested. # See: https://github.com/kubernetes-sigs/kind/releases - - cluster: v1.21.1/kourier - k8s-version: v1.21.1 + - cluster: v1.23.0/kourier + k8s-version: v1.23.0 kind-version: v0.11.1 - kind-image-sha: sha256:69860bda5563ac81e3c0057d654b5253219618a22ec3a346306239bba8cfa1a6 + kind-image-sha: sha256:49824ab1727c04e56a21a5d8372a402fcd32ea51ac96a2706a12af38934f81ac kingress: kourier test-flags: "--enable-alpha --enable-beta" cluster-suffix: c${{ github.run_id }}.local @@ -128,6 +128,21 @@ jobs: image: kindest/node:${{ matrix.k8s-version }}@${{ matrix.kind-image-sha }} EOF + - name: Add Workers to KinD Cluster (Istio, Gateway API) + working-directory: ./src/knative.dev/serving + if: matrix.kingress == 'istio' || matrix.kingress == 'gateway-api' + run: | + set -x + + cat >> kind.yaml < Date: Mon, 4 Apr 2022 14:39:55 +0900 Subject: [PATCH 11/29] Use deepcopy --- .../serverlessservice/resources/services.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 970cbe350261..9df5bc2b0b1f 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -108,20 +108,23 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core } ret := make([]corev1.EndpointSubset, len(subsets)) for i, sss := range subsets { - sst := sss - sst.Ports = nil + sst := sss.DeepCopy() + sst2 := sss.DeepCopy() // Find the port we care about and remove all others. - for j, p := range sss.Ports { + for j, p := range sst.Ports { if p.Port == targetPort { sst.Ports = sss.Ports[j : j+1] + break } + } + for j, p := range sst2.Ports { if p.Port == networking.BackendHTTPSPort { - port := sss.Ports[j : j+1] + port := sst2.Ports[j : j+1] sst.Ports = append(sst.Ports, port...) + break } } - - ret[i] = sst + ret[i] = *sst } return ret } From 600b7a1be86e37807d4657088158211886467a53 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 4 Apr 2022 15:17:21 +0900 Subject: [PATCH 12/29] Do not use deepcopy --- .../serverlessservice/resources/services.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 9df5bc2b0b1f..0105c05e19de 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -108,23 +108,23 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core } ret := make([]corev1.EndpointSubset, len(subsets)) for i, sss := range subsets { - sst := sss.DeepCopy() - sst2 := sss.DeepCopy() + sst := sss + sst.Ports = nil // Find the port we care about and remove all others. - for j, p := range sst.Ports { + for j, p := range sss.Ports { if p.Port == targetPort { sst.Ports = sss.Ports[j : j+1] break } } - for j, p := range sst2.Ports { + for j, p := range sss.Ports { if p.Port == networking.BackendHTTPSPort { - port := sst2.Ports[j : j+1] + port := sss.Ports[j : j+1] sst.Ports = append(sst.Ports, port...) break } } - ret[i] = *sst + ret[i] = sst } return ret } From d9e4646a2d0da51d37df00addbfaf55510901642 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 4 Apr 2022 16:01:18 +0900 Subject: [PATCH 13/29] fix --- pkg/reconciler/serverlessservice/resources/services.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 0105c05e19de..27ee44b36d60 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -119,8 +119,7 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core } for j, p := range sss.Ports { if p.Port == networking.BackendHTTPSPort { - port := sss.Ports[j : j+1] - sst.Ports = append(sst.Ports, port...) + sst.Ports = append(sst.Ports, sss.Ports[j:j+1]...) break } } From 1979dd90d40f0f64293164cc03c531b8f60a0b5f Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 4 Apr 2022 16:43:57 +0900 Subject: [PATCH 14/29] Fix review comments --- .github/workflows/kind-e2e-tls.yaml | 2 +- cmd/activator/main.go | 2 ++ pkg/reconciler/route/resources/ingress.go | 2 +- pkg/reconciler/serverlessservice/resources/services.go | 3 +++ pkg/reconciler/serverlessservice/resources/services_test.go | 6 +++--- test/generate-cert.sh | 5 ----- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml index 55a1ace2df8b..a8d976d31ed4 100644 --- a/.github/workflows/kind-e2e-tls.yaml +++ b/.github/workflows/kind-e2e-tls.yaml @@ -1,4 +1,4 @@ -name: KinD e2e tests -tls +name: KinD e2e tests - tls on: pull_request: diff --git a/cmd/activator/main.go b/cmd/activator/main.go index a475c608bf62..30ac75eb9679 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -246,6 +246,8 @@ func main() { } // Enable TLS server when activator server certs are mounted. + // At this moment activator with TLS does not disable HTTP. + // See also https://github.com/knative/serving/issues/12808. if exists(logger, certPath) && exists(logger, keyPath) { tlsServers := map[string]*http.Server{ "https": pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah), diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go index c790d1a47a1e..0ca15cb8c8b5 100644 --- a/pkg/reconciler/route/resources/ingress.go +++ b/pkg/reconciler/route/resources/ingress.go @@ -320,7 +320,7 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, } servicePort := intstr.FromInt(networking.ServicePort(t.Protocol)) if internalTLS { - servicePort = intstr.FromInt(443) + servicePort = intstr.FromInt(networking.ServiceHTTPSPort) } if cfg == nil || len(cfg.Revisions) < 2 { // No rollout in progress. diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 27ee44b36d60..5d4f54e5d78c 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -65,6 +65,9 @@ func makePublicServicePorts(sks *v1alpha1.ServerlessService) []corev1.ServicePor Port: int32(pkgnet.ServicePort(sks.Spec.ProtocolType)), TargetPort: targetPort(sks), }, { + // The HTTPS port is used when activator-ca is enabled. + // Although it is not used by default, we put it here as it should be harmless + // and makes the code simple. Name: pkgnet.ServicePortNameHTTPS, Protocol: corev1.ProtocolTCP, Port: pkgnet.ServiceHTTPSPort, diff --git a/pkg/reconciler/serverlessservice/resources/services_test.go b/pkg/reconciler/serverlessservice/resources/services_test.go index 97743b558b02..61f7bad027ad 100644 --- a/pkg/reconciler/serverlessservice/resources/services_test.go +++ b/pkg/reconciler/serverlessservice/resources/services_test.go @@ -388,7 +388,7 @@ func TestFilterSubsetPorts(t *testing.T) { }}, }}, }, { - name: "target pors and https port, keep middle and https", + name: "four ports including https ports, keep target and https port", port: 2006, subsets: []corev1.EndpointSubset{{ Ports: []corev1.EndpointPort{{ @@ -405,7 +405,7 @@ func TestFilterSubsetPorts(t *testing.T) { Protocol: "TCP", }, { Name: "https", - Port: 8112, + Port: networking.BackendHTTPSPort, Protocol: "TCP", }}, }}, @@ -418,7 +418,7 @@ func TestFilterSubsetPorts(t *testing.T) { }, { Name: "https", - Port: 8112, + Port: networking.BackendHTTPSPort, Protocol: "TCP", }, }, diff --git a/test/generate-cert.sh b/test/generate-cert.sh index 24c5b4828d9f..00d9ce5d7b51 100755 --- a/test/generate-cert.sh +++ b/test/generate-cert.sh @@ -35,8 +35,3 @@ kubectl create -n ${SYSTEM_NAMESPACE} secret generic serving-ca \ kubectl create -n ${SYSTEM_NAMESPACE} secret tls server-certs \ --key="${out_dir}"/tls.key \ --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - - -#kubectl create ns ${TEST_NAMESPACE} -#kubectl create -n ${TEST_NAMESPACE} secret tls server-certs \ -# --key="${out_dir}"/tls.key \ -# --cert="${out_dir}"/tls.crt --dry-run=client -o yaml | kubectl apply -f - From 859ce6ca0cb76e0364b4d39696ce234ec236829c Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 4 Apr 2022 17:32:45 +0900 Subject: [PATCH 15/29] Use activatorCA string --- pkg/reconciler/route/resources/ingress.go | 20 ++++++++----------- .../route/resources/ingress_test.go | 9 +++------ 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go index 0ca15cb8c8b5..9edef5f8dccf 100644 --- a/pkg/reconciler/route/resources/ingress.go +++ b/pkg/reconciler/route/resources/ingress.go @@ -136,10 +136,6 @@ func makeIngressSpec( featuresConfig := config.FromContextOrDefaults(ctx).Features networkConfig := config.FromContextOrDefaults(ctx).Network - internalTLS := false - if activatorCA := networkConfig.ActivatorCA; activatorCA != "" { - internalTLS = true - } for _, name := range names { visibilities := []netv1alpha1.IngressVisibility{netv1alpha1.IngressVisibilityClusterLocal} @@ -153,7 +149,7 @@ func makeIngressSpec( return netv1alpha1.IngressSpec{}, err } rule := makeIngressRule(domains, r.Namespace, - visibility, tc.Targets[name], ro.RolloutsByTag(name), internalTLS) + visibility, tc.Targets[name], ro.RolloutsByTag(name), networkConfig.ActivatorCA) if featuresConfig.TagHeaderBasedRouting == apicfg.Enabled { if rule.HTTP.Paths[0].AppendHeaders == nil { rule.HTTP.Paths[0].AppendHeaders = make(map[string]string, 1) @@ -175,7 +171,7 @@ func makeIngressSpec( // Since names are sorted `DefaultTarget == ""` is the first one, // so just pass the subslice. rule.HTTP.Paths = append( - makeTagBasedRoutingIngressPaths(r.Namespace, tc, ro, internalTLS, names[1:]), rule.HTTP.Paths...) + makeTagBasedRoutingIngressPaths(r.Namespace, tc, ro, networkConfig.ActivatorCA, names[1:]), rule.HTTP.Paths...) } else { // If a request is routed by a tag-attached hostname instead of the tag header, // the request may not have the tag header "Knative-Serving-Tag", @@ -269,24 +265,24 @@ func makeIngressRule(domains []string, ns string, visibility netv1alpha1.IngressVisibility, targets traffic.RevisionTargets, roCfgs []*traffic.ConfigurationRollout, - internalTLS bool) netv1alpha1.IngressRule { + activatorCA string) netv1alpha1.IngressRule { return netv1alpha1.IngressRule{ Hosts: domains, Visibility: visibility, HTTP: &netv1alpha1.HTTPIngressRuleValue{ Paths: []netv1alpha1.HTTPIngressPath{ - *makeBaseIngressPath(ns, targets, roCfgs, internalTLS), + *makeBaseIngressPath(ns, targets, roCfgs, activatorCA), }, }, } } // `names` must not include `""` — the DefaultTarget. -func makeTagBasedRoutingIngressPaths(ns string, tc *traffic.Config, ro *traffic.Rollout, internalTLS bool, names []string) []netv1alpha1.HTTPIngressPath { +func makeTagBasedRoutingIngressPaths(ns string, tc *traffic.Config, ro *traffic.Rollout, activatorCA string, names []string) []netv1alpha1.HTTPIngressPath { paths := make([]netv1alpha1.HTTPIngressPath, 0, len(names)) for _, name := range names { - path := makeBaseIngressPath(ns, tc.Targets[name], ro.RolloutsByTag(name), internalTLS) + path := makeBaseIngressPath(ns, tc.Targets[name], ro.RolloutsByTag(name), activatorCA) path.Headers = map[string]netv1alpha1.HeaderMatch{network.TagHeaderName: {Exact: name}} paths = append(paths, *path) } @@ -306,7 +302,7 @@ func rolloutConfig(cfgName string, ros []*traffic.ConfigurationRollout) *traffic } func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, - roCfgs []*traffic.ConfigurationRollout, internalTLS bool) *netv1alpha1.HTTPIngressPath { + roCfgs []*traffic.ConfigurationRollout, activatorCA string) *netv1alpha1.HTTPIngressPath { // Optimistically allocate |targets| elements. splits := make([]netv1alpha1.IngressBackendSplit, 0, len(targets)) for _, t := range targets { @@ -319,7 +315,7 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, cfg = rolloutConfig(t.ConfigurationName, roCfgs) } servicePort := intstr.FromInt(networking.ServicePort(t.Protocol)) - if internalTLS { + if len(activatorCA) != 0 { servicePort = intstr.FromInt(networking.ServiceHTTPSPort) } if cfg == nil || len(cfg.Revisions) < 2 { diff --git a/pkg/reconciler/route/resources/ingress_test.go b/pkg/reconciler/route/resources/ingress_test.go index fd4a65f1676a..9f6bb47b9b88 100644 --- a/pkg/reconciler/route/resources/ingress_test.go +++ b/pkg/reconciler/route/resources/ingress_test.go @@ -866,9 +866,8 @@ func TestMakeIngressRuleVanilla(t *testing.T) { }, } ro := tc.BuildRollout() - internalTLS := false rule := makeIngressRule(domains, ns, - netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), internalTLS) + netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), "" /* activatorCA */) expected := netv1alpha1.IngressRule{ Hosts: []string{ "a.com", @@ -920,9 +919,8 @@ func TestMakeIngressRuleZeroPercentTarget(t *testing.T) { }, } ro := tc.BuildRollout() - internalTLS := false rule := makeIngressRule(domains, ns, - netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), internalTLS) + netv1alpha1.IngressVisibilityExternalIP, targets, ro.RolloutsByTag(traffic.DefaultTarget), "" /* activatorCA */) expected := netv1alpha1.IngressRule{ Hosts: []string{"test.org"}, HTTP: &netv1alpha1.HTTPIngressRuleValue{ @@ -970,10 +968,9 @@ func TestMakeIngressRuleTwoTargets(t *testing.T) { }, } ro := tc.BuildRollout() - internalTLS := false domains := []string{"test.org"} rule := makeIngressRule(domains, ns, netv1alpha1.IngressVisibilityExternalIP, - targets, ro.RolloutsByTag("a-tag"), internalTLS) + targets, ro.RolloutsByTag("a-tag"), "" /* activatorCA */) expected := netv1alpha1.IngressRule{ Hosts: []string{"test.org"}, HTTP: &netv1alpha1.HTTPIngressRuleValue{ From 66f094b698e72f5973838ed44fe92e423fa223d0 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 4 Apr 2022 18:06:58 +0900 Subject: [PATCH 16/29] Add unit test for makeIngress --- .../route/resources/ingress_test.go | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/pkg/reconciler/route/resources/ingress_test.go b/pkg/reconciler/route/resources/ingress_test.go index 9f6bb47b9b88..27063d4612de 100644 --- a/pkg/reconciler/route/resources/ingress_test.go +++ b/pkg/reconciler/route/resources/ingress_test.go @@ -1089,6 +1089,128 @@ func TestMakeIngressWithHTTPOption(t *testing.T) { } } +func TestMakeIngressWithActivatorCA(t *testing.T) { + targets := map[string]traffic.RevisionTargets{ + traffic.DefaultTarget: {{ + TrafficTarget: v1.TrafficTarget{ + ConfigurationName: "config", + RevisionName: "v2", + Percent: ptr.Int64(100), + }, + }}, + "v1": {{ + TrafficTarget: v1.TrafficTarget{ + ConfigurationName: "config", + RevisionName: "v1", + Percent: ptr.Int64(100), + }, + }}, + } + + r := Route(ns, "test-route", WithURL) + + expected := []netv1alpha1.IngressRule{{ + Hosts: []string{ + "test-route." + ns, + "test-route." + ns + ".svc", + pkgnet.GetServiceHostname("test-route", ns), + }, + HTTP: &netv1alpha1.HTTPIngressRuleValue{ + Paths: []netv1alpha1.HTTPIngressPath{{ + Splits: []netv1alpha1.IngressBackendSplit{{ + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: ns, + ServiceName: "v2", + ServicePort: intstr.FromInt(443), + }, + Percent: 100, + AppendHeaders: map[string]string{ + "Knative-Serving-Revision": "v2", + "Knative-Serving-Namespace": ns, + }, + }}, + }}, + }, + Visibility: netv1alpha1.IngressVisibilityClusterLocal, + }, { + Hosts: []string{ + "test-route." + ns + ".example.com", + }, + HTTP: &netv1alpha1.HTTPIngressRuleValue{ + Paths: []netv1alpha1.HTTPIngressPath{{ + Splits: []netv1alpha1.IngressBackendSplit{{ + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: ns, + ServiceName: "v2", + ServicePort: intstr.FromInt(443), + }, + Percent: 100, + AppendHeaders: map[string]string{ + "Knative-Serving-Revision": "v2", + "Knative-Serving-Namespace": ns, + }, + }}, + }}, + }, + Visibility: netv1alpha1.IngressVisibilityExternalIP, + }, { + Hosts: []string{ + "v1-test-route." + ns, + "v1-test-route." + ns + ".svc", + pkgnet.GetServiceHostname("v1-test-route", ns), + }, + HTTP: &netv1alpha1.HTTPIngressRuleValue{ + Paths: []netv1alpha1.HTTPIngressPath{{ + Splits: []netv1alpha1.IngressBackendSplit{{ + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: ns, + ServiceName: "v1", + ServicePort: intstr.FromInt(443), + }, + Percent: 100, + AppendHeaders: map[string]string{ + "Knative-Serving-Revision": "v1", + "Knative-Serving-Namespace": ns, + }, + }}, + }}, + }, + Visibility: netv1alpha1.IngressVisibilityClusterLocal, + }, { + Hosts: []string{ + "v1-test-route." + ns + ".example.com", + }, + HTTP: &netv1alpha1.HTTPIngressRuleValue{ + Paths: []netv1alpha1.HTTPIngressPath{{ + Splits: []netv1alpha1.IngressBackendSplit{{ + IngressBackend: netv1alpha1.IngressBackend{ + ServiceNamespace: ns, + ServiceName: "v1", + ServicePort: intstr.FromInt(443), + }, + Percent: 100, + AppendHeaders: map[string]string{ + "Knative-Serving-Revision": "v1", + "Knative-Serving-Namespace": ns, + }, + }}, + }}, + }, + Visibility: netv1alpha1.IngressVisibilityExternalIP, + }} + + tc := &traffic.Config{Targets: targets} + ro := tc.BuildRollout() + ci, err := makeIngressSpec(testContextWithActivatorCA(), r, nil /*tls*/, tc, ro) + if err != nil { + t.Error("Unexpected error", err) + } + + if !cmp.Equal(expected, ci.Rules) { + t.Error("Unexpected rules (-want, +got):", cmp.Diff(expected, ci.Rules)) + } +} + func TestMakeIngressTLS(t *testing.T) { cert := &netv1alpha1.Certificate{ ObjectMeta: metav1.ObjectMeta{ @@ -1300,3 +1422,10 @@ func testContextWithHTTPOption() context.Context { cfg.Network.HTTPProtocol = network.HTTPRedirected return config.ToContext(context.Background(), cfg) } + +func testContextWithActivatorCA() context.Context { + cfg := testConfig() + cfg.Network.ActivatorCA = "ca-ame" + cfg.Network.ActivatorSAN = "san-name" + return config.ToContext(context.Background(), cfg) +} From 9f4825585ff32efef13b3ef2aafef50575f1a750 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 4 Apr 2022 18:52:59 +0900 Subject: [PATCH 17/29] Use all match for tls --- .github/workflows/kind-e2e-tls.yaml | 200 ---------------------------- 1 file changed, 200 deletions(-) delete mode 100644 .github/workflows/kind-e2e-tls.yaml diff --git a/.github/workflows/kind-e2e-tls.yaml b/.github/workflows/kind-e2e-tls.yaml deleted file mode 100644 index a8d976d31ed4..000000000000 --- a/.github/workflows/kind-e2e-tls.yaml +++ /dev/null @@ -1,200 +0,0 @@ -name: KinD e2e tests - tls - -on: - pull_request: - branches: [ 'main', 'release-*' ] - -defaults: - run: - shell: bash - working-directory: ./src/knative.dev/serving - -jobs: - - e2e-tests: - name: e2e tests - runs-on: ubuntu-latest - strategy: - fail-fast: false # Keep running if one leg fails. - matrix: - cluster: - - v1.23.0/kourier - - test-suite: - - ./test/conformance/runtime - - ./test/conformance/api/... - - ./test/e2e - - include: - # Map between K8s and KinD versions. - # This is attempting to make it a bit clearer what's being tested. - # See: https://github.com/kubernetes-sigs/kind/releases - - cluster: v1.23.0/kourier - k8s-version: v1.23.0 - kind-version: v0.11.1 - kind-image-sha: sha256:49824ab1727c04e56a21a5d8372a402fcd32ea51ac96a2706a12af38934f81ac - kingress: kourier - test-flags: "--enable-alpha --enable-beta" - cluster-suffix: c${{ github.run_id }}.local - - env: - GOPATH: ${{ github.workspace }} - GO111MODULE: on - GOFLAGS: -tags=nostackdriver - # https://github.com/google/go-containerregistry/pull/125 allows insecure registry for - # '*.local' hostnames. This works both for `ko` and our own tag-to-digest resolution logic, - # thus allowing us to test without bypassing tag-to-digest resolution. - REGISTRY_NAME: registry.local - REGISTRY_PORT: 5000 - KO_DOCKER_REPO: registry.local:5000/knative - - steps: - - name: Set up Go 1.17.x - uses: actions/setup-go@v2 - with: - go-version: 1.17.x - - - name: Install Dependencies - working-directory: ./ - run: | - echo '::group:: install ko' - curl -L https://github.com/google/ko/releases/download/v0.6.0/ko_0.6.0_Linux_x86_64.tar.gz | tar xzf - ko - chmod +x ./ko - sudo mv ko /usr/local/bin - echo '::endgroup::' - - - name: Check out code onto GOPATH - uses: actions/checkout@v2 - with: - path: ./src/knative.dev/serving - - - name: Install KinD - run: | - set -x - - # Disable swap otherwise memory enforcement doesn't work - # See: https://kubernetes.slack.com/archives/CEKK1KTN2/p1600009955324200 - sudo swapoff -a - sudo rm -f /swapfile - - # Use in-memory storage to avoid etcd server timeouts. - # https://kubernetes.slack.com/archives/CEKK1KTN2/p1615134111016300 - # https://github.com/kubernetes-sigs/kind/issues/845 - sudo mkdir -p /tmp/etcd - sudo mount -t tmpfs tmpfs /tmp/etcd - - curl -Lo ./kind https://github.com/kubernetes-sigs/kind/releases/download/${{ matrix.kind-version }}/kind-$(uname)-amd64 - chmod +x ./kind - sudo mv kind /usr/local/bin - - - name: Configure KinD Cluster - working-directory: ./src/knative.dev/serving - run: | - set -x - - # KinD configuration. - cat > kind.yaml <> kind.yaml < 127.0.0.1, to tell `ko` to publish to - # local reigstry, even when pushing $REGISTRY_NAME:$REGISTRY_PORT/some/image - sudo echo "127.0.0.1 $REGISTRY_NAME" | sudo tee -a /etc/hosts - - - name: Install Serving & Ingress - working-directory: ./src/knative.dev/serving - run: | - source ./test/e2e-common.sh - - KIND=1 - INGRESS_CLASS="${{ matrix.kingress }}.ingress.networking.knative.dev" - CLUSTER_DOMAIN="${{ matrix.cluster-suffix }}" - ENABLE_TLS=1 - - knative_setup - test_setup - - echo "INGRESS_CLASS=$INGRESS_CLASS" >> $GITHUB_ENV - echo "CLUSTER_DOMAIN=$CLUSTER_DOMAIN" >> $GITHUB_ENV - echo "SYSTEM_NAMESPACE=$SYSTEM_NAMESPACE" >> $GITHUB_ENV - echo "GATEWAY_OVERRIDE=$GATEWAY_OVERRIDE" >> $GITHUB_ENV - echo "GATEWAY_NAMESPACE_OVERRIDE=$GATEWAY_NAMESPACE_OVERRIDE" >> $GITHUB_ENV - - - name: Run Test - working-directory: ./src/knative.dev/serving - run: | - set -x - - source ./test/e2e-common.sh - - # Exclude the control-plane node, which doesn't seem to expose the nodeport service. - IPS=( $(kubectl get nodes -lkubernetes.io/hostname!=kind-control-plane -ojsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') ) - # Run the tests tagged as e2e on the KinD cluster. - # TODO: Remove feature toogle when emptyDir is enabled by default - toggle_feature kubernetes.podspec-volumes-emptydir Enabled - go test -race -count=1 -parallel=1 -timeout=30m -tags=e2e ${{ matrix.test-suite }} \ - --ingressendpoint="${IPS[0]}" \ - ${{ matrix.test-flags }} - toggle_feature kubernetes.podspec-volumes-emptydir Disabled From 59101229890fe6473b0e15b317e54cc36d5122ce Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 6 Apr 2022 08:47:35 +0900 Subject: [PATCH 18/29] Drop map --- cmd/activator/main.go | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 30ac75eb9679..182f34127e6b 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -249,18 +249,13 @@ func main() { // At this moment activator with TLS does not disable HTTP. // See also https://github.com/knative/serving/issues/12808. if exists(logger, certPath) && exists(logger, keyPath) { - tlsServers := map[string]*http.Server{ - "https": pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah), - } - - for name, server := range tlsServers { - go func(name string, s *http.Server) { - // Don't forward ErrServerClosed as that indicates we're already shutting down. - if err := s.ListenAndServeTLS(certPath, keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) { - errCh <- fmt.Errorf("%s server failed: %w", name, err) - } - }(name, server) - } + name, server := "https", pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah) + go func(name string, s *http.Server) { + // Don't forward ErrServerClosed as that indicates we're already shutting down. + if err := s.ListenAndServeTLS(certPath, keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) { + errCh <- fmt.Errorf("%s server failed: %w", name, err) + } + }(name, server) } // Wait for the signal to drain. From a7209942a33049848c6bf47d15bbb8a417e8096f Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 6 Apr 2022 11:26:26 +0900 Subject: [PATCH 19/29] Use secret to specify activator server cert --- cmd/activator/main.go | 29 +++++++++++++------------- config/core/deployments/activator.yaml | 3 +++ test/config/tls/config-network.yaml | 1 + test/config/tls/service-patch.yaml | 5 ----- test/config/tls/volume-patch.yaml | 13 ------------ test/e2e-common.sh | 3 +-- 6 files changed, 19 insertions(+), 35 deletions(-) delete mode 100644 test/config/tls/service-patch.yaml delete mode 100644 test/config/tls/volume-patch.yaml diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 182f34127e6b..1c00580d4156 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -18,6 +18,7 @@ package main import ( "context" + "crypto/tls" "errors" "fmt" "log" @@ -68,10 +69,6 @@ const ( // The port on which autoscaler WebSocket server listens. autoscalerPort = ":8080" - - certDirectory = "/var/lib/knative/certs" - certPath = certDirectory + "/tls.crt" - keyPath = certDirectory + "/tls.key" ) type config struct { @@ -245,14 +242,24 @@ func main() { }(name, server) } - // Enable TLS server when activator server certs are mounted. + // Enable TLS server when activator-server-cert is specified. // At this moment activator with TLS does not disable HTTP. // See also https://github.com/knative/serving/issues/12808. - if exists(logger, certPath) && exists(logger, keyPath) { + if networkConfig.ActivatorServerCert != "" { + secret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, networkConfig.ActivatorServerCert, metav1.GetOptions{}) + if err != nil { + logger.Fatalw("failed to get secret", zap.Error(err)) + } + cert, err := tls.X509KeyPair(secret.Data["tls.crt"], secret.Data["tls.key"]) + if err != nil { + logger.Fatalw("failed to load certs", zap.Error(err)) + } + name, server := "https", pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah) go func(name string, s *http.Server) { + s.TLSConfig = &tls.Config{Certificates: []tls.Certificate{cert}, MinVersion: tls.VersionTLS12} // Don't forward ErrServerClosed as that indicates we're already shutting down. - if err := s.ListenAndServeTLS(certPath, keyPath); err != nil && !errors.Is(err, http.ErrServerClosed) { + if err := s.ListenAndServeTLS("", ""); err != nil && !errors.Is(err, http.ErrServerClosed) { errCh <- fmt.Errorf("%s server failed: %w", name, err) } }(name, server) @@ -279,14 +286,6 @@ func main() { logger.Info("Servers shutdown.") } -func exists(logger *zap.SugaredLogger, filename string) bool { - _, err := os.Stat(filename) - if err != nil && !os.IsNotExist(err) { - logger.Fatalw(fmt.Sprintf("Failed to verify the file path %q", filename), zap.Error(err)) - } - return err == nil -} - func newHealthCheck(sigCtx context.Context, logger *zap.SugaredLogger, statSink *websocket.ManagedConnection) func() error { once := sync.Once{} return func() error { diff --git a/config/core/deployments/activator.yaml b/config/core/deployments/activator.yaml index 0a5a5b0dbcd2..0f69a980848a 100644 --- a/config/core/deployments/activator.yaml +++ b/config/core/deployments/activator.yaml @@ -151,4 +151,7 @@ spec: - name: http2 port: 81 targetPort: 8013 + - name: https + port: 443 + targetPort: 8112 type: ClusterIP diff --git a/test/config/tls/config-network.yaml b/test/config/tls/config-network.yaml index c939db8f9553..8c3f63773819 100644 --- a/test/config/tls/config-network.yaml +++ b/test/config/tls/config-network.yaml @@ -23,3 +23,4 @@ metadata: data: activator-ca: "serving-ca" activator-san: "knative" + activator-server-cert: "server-certs" diff --git a/test/config/tls/service-patch.yaml b/test/config/tls/service-patch.yaml deleted file mode 100644 index 3dbc93d869ac..000000000000 --- a/test/config/tls/service-patch.yaml +++ /dev/null @@ -1,5 +0,0 @@ -spec: - ports: - - name: https - port: 443 - targetPort: 8112 diff --git a/test/config/tls/volume-patch.yaml b/test/config/tls/volume-patch.yaml deleted file mode 100644 index fdb715dc8510..000000000000 --- a/test/config/tls/volume-patch.yaml +++ /dev/null @@ -1,13 +0,0 @@ -spec: - template: - spec: - volumes: - - name: server-certs - secret: - secretName: server-certs - containers: - - name: activator - volumeMounts: - - name: server-certs - mountPath: /var/lib/knative/certs - readOnly: true diff --git a/test/e2e-common.sh b/test/e2e-common.sh index faafd964111e..5644cb63e3d0 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -363,9 +363,8 @@ function install() { bash ${REPO_ROOT_DIR}/test/generate-cert.sh echo "Patch to activator to serve TLS" - kubectl patch deploy -n ${SYSTEM_NAMESPACE} activator --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/volume-patch.yaml)" - kubectl patch service -n ${SYSTEM_NAMESPACE} activator-service --patch "$(cat ${REPO_ROOT_DIR}/test/config/tls/service-patch.yaml)" kubectl apply -n ${SYSTEM_NAMESPACE} -f ${REPO_ROOT_DIR}/test/config/tls/config-network.yaml + kubectl delete pod -n ${SYSTEM_NAMESPACE} -l app=activator fi } From cfccf816f2cc7ace570c7677f1198740f6091376 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 6 Apr 2022 16:27:20 +0900 Subject: [PATCH 20/29] Update comments --- test/config/tls/config-network.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/config/tls/config-network.yaml b/test/config/tls/config-network.yaml index 8c3f63773819..eee9fe00c50e 100644 --- a/test/config/tls/config-network.yaml +++ b/test/config/tls/config-network.yaml @@ -23,4 +23,4 @@ metadata: data: activator-ca: "serving-ca" activator-san: "knative" - activator-server-cert: "server-certs" + activator-server-certs: "server-certs" From 7accf89c1868ad3c0f41b221bcdeef08e211dd45 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 7 Apr 2022 11:55:50 +0900 Subject: [PATCH 21/29] Fix review comment --- pkg/reconciler/route/resources/ingress.go | 4 +++- pkg/reconciler/route/resources/ingress_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go index 9edef5f8dccf..edf9a817a204 100644 --- a/pkg/reconciler/route/resources/ingress.go +++ b/pkg/reconciler/route/resources/ingress.go @@ -314,9 +314,11 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets, if t.LatestRevision != nil && *t.LatestRevision { cfg = rolloutConfig(t.ConfigurationName, roCfgs) } - servicePort := intstr.FromInt(networking.ServicePort(t.Protocol)) + var servicePort intstr.IntOrString if len(activatorCA) != 0 { servicePort = intstr.FromInt(networking.ServiceHTTPSPort) + } else { + servicePort = intstr.FromInt(networking.ServicePort(t.Protocol)) } if cfg == nil || len(cfg.Revisions) < 2 { // No rollout in progress. diff --git a/pkg/reconciler/route/resources/ingress_test.go b/pkg/reconciler/route/resources/ingress_test.go index 27063d4612de..c87816c3a92d 100644 --- a/pkg/reconciler/route/resources/ingress_test.go +++ b/pkg/reconciler/route/resources/ingress_test.go @@ -1121,7 +1121,7 @@ func TestMakeIngressWithActivatorCA(t *testing.T) { IngressBackend: netv1alpha1.IngressBackend{ ServiceNamespace: ns, ServiceName: "v2", - ServicePort: intstr.FromInt(443), + ServicePort: intstr.FromInt(networking.ServiceHTTPSPort), }, Percent: 100, AppendHeaders: map[string]string{ @@ -1142,7 +1142,7 @@ func TestMakeIngressWithActivatorCA(t *testing.T) { IngressBackend: netv1alpha1.IngressBackend{ ServiceNamespace: ns, ServiceName: "v2", - ServicePort: intstr.FromInt(443), + ServicePort: intstr.FromInt(networking.ServiceHTTPSPort), }, Percent: 100, AppendHeaders: map[string]string{ @@ -1165,7 +1165,7 @@ func TestMakeIngressWithActivatorCA(t *testing.T) { IngressBackend: netv1alpha1.IngressBackend{ ServiceNamespace: ns, ServiceName: "v1", - ServicePort: intstr.FromInt(443), + ServicePort: intstr.FromInt(networking.ServiceHTTPSPort), }, Percent: 100, AppendHeaders: map[string]string{ @@ -1186,7 +1186,7 @@ func TestMakeIngressWithActivatorCA(t *testing.T) { IngressBackend: netv1alpha1.IngressBackend{ ServiceNamespace: ns, ServiceName: "v1", - ServicePort: intstr.FromInt(443), + ServicePort: intstr.FromInt(networking.ServiceHTTPSPort), }, Percent: 100, AppendHeaders: map[string]string{ From 21bf5e0f4f3e4a2e772a2473ecc8e3e8e4a6641a Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 7 Apr 2022 11:59:58 +0900 Subject: [PATCH 22/29] wip --- pkg/reconciler/serverlessservice/resources/services.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 5d4f54e5d78c..d4106bfcbc69 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -120,6 +120,7 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core break } } + // Add HTTPS port if exists. for j, p := range sss.Ports { if p.Port == networking.BackendHTTPSPort { sst.Ports = append(sst.Ports, sss.Ports[j:j+1]...) From 4d94c8c4a05c7f625fb4a7532de4e6331a21bbc4 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Thu, 7 Apr 2022 12:07:59 +0900 Subject: [PATCH 23/29] Add comment --- cmd/activator/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/activator/main.go b/cmd/activator/main.go index 1c00580d4156..e842f2689e08 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -255,6 +255,8 @@ func main() { logger.Fatalw("failed to load certs", zap.Error(err)) } + // TODO: Implement the secret (certificate) rotation like knative.dev/pkg/webhook/certificates/. + // Also, the current activator must be restarted when updating the secret. name, server := "https", pkgnet.NewServer(":"+strconv.Itoa(networking.BackendHTTPSPort), ah) go func(name string, s *http.Server) { s.TLSConfig = &tls.Config{Certificates: []tls.Certificate{cert}, MinVersion: tls.VersionTLS12} From 6cc276d421cd58e5a29f24b34ab3c98899049a7b Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 11 Apr 2022 16:34:04 +0900 Subject: [PATCH 24/29] Add append to both --- pkg/reconciler/serverlessservice/resources/services.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index d4106bfcbc69..64d4c754f16d 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -116,15 +116,11 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core // Find the port we care about and remove all others. for j, p := range sss.Ports { if p.Port == targetPort { - sst.Ports = sss.Ports[j : j+1] - break + sst.Ports = append(sst.Ports, sss.Ports[j:j+1]...) } - } - // Add HTTPS port if exists. - for j, p := range sss.Ports { + // Add HTTPS port if exists. if p.Port == networking.BackendHTTPSPort { sst.Ports = append(sst.Ports, sss.Ports[j:j+1]...) - break } } ret[i] = sst From 38085fa8019816df2ac7f6628f399b1ee4e25020 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 11 Apr 2022 16:35:46 +0900 Subject: [PATCH 25/29] Use sss.Ports[j] --- pkg/reconciler/serverlessservice/resources/services.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 64d4c754f16d..580a57a46d89 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -116,11 +116,11 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core // Find the port we care about and remove all others. for j, p := range sss.Ports { if p.Port == targetPort { - sst.Ports = append(sst.Ports, sss.Ports[j:j+1]...) + sst.Ports = append(sst.Ports, sss.Ports[j]) } // Add HTTPS port if exists. if p.Port == networking.BackendHTTPSPort { - sst.Ports = append(sst.Ports, sss.Ports[j:j+1]...) + sst.Ports = append(sst.Ports, sss.Ports[j]) } } ret[i] = sst From 2010099454677da3e2ddba66af2fbb882a4b1fef Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Mon, 11 Apr 2022 17:30:35 +0900 Subject: [PATCH 26/29] Fix script --- .github/workflows/kind-e2e.yaml | 1 + test/e2e-common.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/kind-e2e.yaml b/.github/workflows/kind-e2e.yaml index 48e2ac42af2b..3154421b6afc 100644 --- a/.github/workflows/kind-e2e.yaml +++ b/.github/workflows/kind-e2e.yaml @@ -134,6 +134,7 @@ jobs: namespace-resources: virtualservices - ingress: kourier-tls + ingress-class: kourier enable-tls: 1 - test-suite: runtime diff --git a/test/e2e-common.sh b/test/e2e-common.sh index 5644cb63e3d0..aabb0adff055 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -34,7 +34,7 @@ export RUN_HTTP01_AUTO_TLS_TESTS=0 export HTTPS=0 export SHORT=0 export ENABLE_HA=0 -export ENABLE_TLS=0 +export ENABLE_TLS=${ENABLE_TLS:-0} export MESH=0 export PERF=0 export KIND=${KIND:-0} From 32ea07b95ff1aab73ab39504b979d29c29563b53 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 12 Apr 2022 08:30:55 +0900 Subject: [PATCH 27/29] Use switch --- pkg/reconciler/serverlessservice/resources/services.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/reconciler/serverlessservice/resources/services.go b/pkg/reconciler/serverlessservice/resources/services.go index 580a57a46d89..7ad61ad674ed 100644 --- a/pkg/reconciler/serverlessservice/resources/services.go +++ b/pkg/reconciler/serverlessservice/resources/services.go @@ -115,11 +115,10 @@ func filterSubsetPorts(targetPort int32, subsets []corev1.EndpointSubset) []core sst.Ports = nil // Find the port we care about and remove all others. for j, p := range sss.Ports { - if p.Port == targetPort { - sst.Ports = append(sst.Ports, sss.Ports[j]) - } - // Add HTTPS port if exists. - if p.Port == networking.BackendHTTPSPort { + switch p.Port { + case networking.BackendHTTPSPort: + fallthrough + case targetPort: sst.Ports = append(sst.Ports, sss.Ports[j]) } } From 22cf5bd600066b2d85c2e5fee0b099df64633b57 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 12 Apr 2022 08:33:14 +0900 Subject: [PATCH 28/29] Fix format more readable --- test/e2e/autoscale_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e/autoscale_test.go b/test/e2e/autoscale_test.go index 7beb0339c74d..a4a76c12f8ec 100644 --- a/test/e2e/autoscale_test.go +++ b/test/e2e/autoscale_test.go @@ -138,8 +138,7 @@ func TestTargetBurstCapacity(t *testing.T) { if err != nil { t.Fatal("Fail to get ConfigMap config-network:", err) } - activatorCA := cm.Data[netpkg.ActivatorCAKey] - if len(activatorCA) != 0 { + if cm.Data[netpkg.ActivatorCAKey] != "" { // TODO: Remove this when https://github.com/knative/serving/issues/12797 was done. t.Skip("Skipping TestTargetBurstCapacity as activator-ca is specified. See issue/12797.") } From 248eb283ec7f8e67daa1f034cdb7b5b73e38c8e3 Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Tue, 12 Apr 2022 17:04:42 +0900 Subject: [PATCH 29/29] Update variable names --- cmd/activator/main.go | 4 ++-- test/config/tls/config-network.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/activator/main.go b/cmd/activator/main.go index e842f2689e08..3d2633023d38 100644 --- a/cmd/activator/main.go +++ b/cmd/activator/main.go @@ -245,8 +245,8 @@ func main() { // Enable TLS server when activator-server-cert is specified. // At this moment activator with TLS does not disable HTTP. // See also https://github.com/knative/serving/issues/12808. - if networkConfig.ActivatorServerCert != "" { - secret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, networkConfig.ActivatorServerCert, metav1.GetOptions{}) + if networkConfig.ActivatorCertSecret != "" { + secret, err := kubeClient.CoreV1().Secrets(system.Namespace()).Get(ctx, networkConfig.ActivatorCertSecret, metav1.GetOptions{}) if err != nil { logger.Fatalw("failed to get secret", zap.Error(err)) } diff --git a/test/config/tls/config-network.yaml b/test/config/tls/config-network.yaml index eee9fe00c50e..6a3b909b8a48 100644 --- a/test/config/tls/config-network.yaml +++ b/test/config/tls/config-network.yaml @@ -23,4 +23,4 @@ metadata: data: activator-ca: "serving-ca" activator-san: "knative" - activator-server-certs: "server-certs" + activator-cert-secret: "server-certs"