Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/activator/net/revision_backends.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type revisionDestsUpdate struct {
const (
probeTimeout time.Duration = 300 * time.Millisecond
probeFrequency time.Duration = 200 * time.Millisecond
probePath = "/_internal/knative/activator/probe"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some random path for activator to probe. It can be changed as @tcnghia has mentioned here

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to be constant here, which is fine.
It needs to be documented as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though it seems quite long for a random probe path.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can follow up to extract this to a configurable option in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please fine an issue, so it's not forgotten

)

// revisionWatcher watches the podIPs and ClusterIP of the service for a revision. It implements the logic
Expand Down Expand Up @@ -135,6 +136,7 @@ func (rw *revisionWatcher) probe(ctx context.Context, dest string) (bool, error)
httpDest := url.URL{
Scheme: "http",
Host: dest,
Path: probePath,
}
// NOTE: changes below may require changes to testing/roundtripper.go to make unit tests passing.
return prober.Do(ctx, rw.transport, httpDest.String(),
Expand Down
5 changes: 5 additions & 0 deletions test/config/100-namespace.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,8 @@ apiVersion: v1
kind: Namespace
metadata:
name: serving-tests-alt
---
apiVersion: v1
kind: Namespace
metadata:
name: serving-tests-security
16 changes: 16 additions & 0 deletions test/config/mtls/policy.yaml → test/config/security/policy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,19 @@ spec:
peers:
- mtls:
mode: PERMISSIVE
---
apiVersion: authentication.istio.io/v1alpha1
kind: Policy
metadata:
name: default
namespace: serving-tests-security
spec:
origins:
- jwt:
issuer: testing@secure.istio.io
jwks: "{ \"keys\":[ {\"e\":\"AQAB\",\"kid\":\"DHFbpoIUqrY8t2zpA2qXfCmr5VO5ZEr4RzHU_-envvQ\",\"kty\":\"RSA\",\"n\":\"xAE7eB6qugXyCAG3yhh7pkDkT65pHymX-P7KfIupjf59vsdo91bSP9C8H07pSAGQO1MV_xFj9VswgsCg4R6otmg5PV2He95lZdHtOcU5DXIg_pbhLdKXbi66GlVeK6ABZOUW3WYtnNHD-91gVuoeJT_DwtGGcp4ignkgXfkiEm4sw-4sfb4qdt5oLbyVpmW6x9cfa7vs2WTfURiCrBoUqgBo_-4WTiULmmHSGZHOjzwa8WtrtOQGsAFjIbno85jp6MnGGGZPYZbDAa_b3y5u-YpW7ypZrvD8BgtKVjgtQgZhLAGezMt0ua3DRrWnKqTZ0BJ_EyxOGuHJrLsn00fnMQ\"}]} "
triggerRules:
- excludedPaths:
- prefix: /metrics
- prefix: /_internal/knative/activator/probe
principalBinding: USE_ORIGIN
7 changes: 5 additions & 2 deletions test/e2e-common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,8 @@ function test_setup() {
if (( MESH )); then
kubectl label namespace serving-tests istio-injection=enabled
kubectl label namespace serving-tests-alt istio-injection=enabled
ko apply ${KO_FLAGS} -f test/config/mtls/ || return 1
kubectl label namespace serving-tests-security istio-injection=enabled
ko apply ${KO_FLAGS} -f test/config/security/ || return 1
fi

echo ">> Uploading test images..."
Expand Down Expand Up @@ -491,13 +492,15 @@ function test_teardown() {
echo ">> Removing test resources (test/config/)"
ko delete --ignore-not-found=true --now -f test/config/
if (( MESH )); then
ko delete --ignore-not-found=true --now -f test/config/mtls/
ko delete --ignore-not-found=true --now -f test/config/security/
fi
echo ">> Ensuring test namespaces are clean"
kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests
kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests
kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests-alt
kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests-alt
kubectl delete all --all --ignore-not-found --now --timeout 60s -n serving-tests-security
kubectl delete --ignore-not-found --now --timeout 60s namespace serving-tests-security
}

# Dump more information when test fails.
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/e2e.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ func SetupAlternativeNamespace(t *testing.T) *test.Clients {
return SetupWithNamespace(t, test.AlternativeServingNamespace)
}

//SetupServingNamespaceforSecurityTesting creates the client objects needed in e2e tests
// under the security testing namespace.
func SetupServingNamespaceforSecurityTesting(t *testing.T) *test.Clients {
return SetupWithNamespace(t, test.ServingNamespaceforSecurityTesting)
}

// SetupWithNamespace creates the client objects needed in the e2e tests under the specified namespace.
func SetupWithNamespace(t *testing.T, namespace string) *test.Clients {
pkgTest.SetupLoggingFlags()
Expand Down
93 changes: 93 additions & 0 deletions test/e2e/probe_whitelist_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// +build e2e

/*
Copyright 2020 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.
*/

package e2e

import (
"net/http"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
pkgTest "knative.dev/pkg/test"
"knative.dev/pkg/test/logstream"
"knative.dev/serving/test"
v1a1test "knative.dev/serving/test/v1alpha1"
)

//This test checks if the activator can probe
//the service when istio end user auth policy is
//applied on the service.
//This test needs istio side car injected and
//istio policy check enabled.
//policy is present in test/config/security/policy.yaml
//apply policy before running this test
func TestProbeWhitelist(t *testing.T) {
t.Parallel()
cancel := logstream.Start(t)
defer cancel()

clients := SetupServingNamespaceforSecurityTesting(t)

names := test.ResourceNames{
Service: test.ObjectNameForTest(t),
Image: "helloworld",
}

if test.ServingFlags.Https {
// Save the current Gateway to restore it after the test
oldGateway, err := clients.IstioClient.NetworkingV1alpha3().Gateways(v1a1test.Namespace).Get(v1a1test.GatewayName, metav1.GetOptions{})
if err != nil {
t.Fatalf("Failed to get Gateway %s/%s", v1a1test.Namespace, v1a1test.GatewayName)
}
test.CleanupOnInterrupt(func() { v1a1test.RestoreGateway(t, clients, *oldGateway) })
defer v1a1test.RestoreGateway(t, clients, *oldGateway)
}

test.CleanupOnInterrupt(func() { test.TearDown(clients, names) })
defer test.TearDown(clients, names)

t.Log("Creating a new Service")
resources, httpsTransportOption, err := v1a1test.CreateRunLatestServiceReady(t, clients, &names, test.ServingFlags.Https)
if err != nil {
t.Fatalf("Failed to create initial Service: %v: %v", names.Service, err)
}

url := resources.Route.Status.URL.URL()
var opt interface{}
if test.ServingFlags.Https {
url.Scheme = "https"
if httpsTransportOption == nil {
t.Fatalf("HTTPS transport option is nil")
}
opt = *httpsTransportOption
}
if _, err := pkgTest.WaitForEndpointState(
clients.KubeClient,
t.Logf,
url,
v1a1test.RetryingRouteInconsistency(pkgTest.MatchesAllOf(pkgTest.IsOneOfStatusCodes(http.StatusUnauthorized))),
"HelloWorldServesAuthFailed",
test.ServingFlags.ResolvableDomain,
opt); err != nil {
// check if side car is injected before reporting error
if _, err := getContainer(clients.KubeClient, resources.Service.Name, "istio-proxy", resources.Service.Namespace); err != nil {
t.Skip("Side car not enabled, skipping test")
}
t.Fatalf("The endpoint %s for Route %s didn't serve the expected status %d: %v", url, names.Route, http.StatusUnauthorized, err)
}
}
2 changes: 2 additions & 0 deletions test/e2e_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ const (
// namespace tests in.
AlternativeServingNamespace = "serving-tests-alt"

// ServingNamespaceforSecurityTesting is the namespace for security tests.
ServingNamespaceforSecurityTesting = "serving-tests-security"
// Environment propagation conformance test objects

// ConformanceConfigMap is the name of the configmap to propagate env variables from
Expand Down