From 13356425e357116da77bb809c699a10053dd104d Mon Sep 17 00:00:00 2001 From: Serverless QE Robot Date: Tue, 24 Oct 2023 20:42:11 -0400 Subject: [PATCH] :fire: Apply carried patches. --- .ko.yaml | 3 +- config/core/100-namespace.yaml | 21 -------- .../200-roles/config-map-view-downstream.yaml | 25 +++++++++ config/core/deployments/activator-hpa.yaml | 2 +- config/core/deployments/webhook-hpa.yaml | 2 +- openshift/release/artifacts/serving-core.yaml | 52 ++++++++++--------- pkg/apis/serving/v1/revision_defaults.go | 15 ++---- pkg/apis/serving/v1/revision_defaults_test.go | 21 +++----- pkg/reconciler/revision/resources/queue.go | 3 -- test/e2e/grpc_test.go | 24 ++------- .../secure_pod_defaults_test.go | 3 -- .../pkg/controller/stats_reporter.go | 4 +- vendor/knative.dev/pkg/test/helpers/name.go | 2 +- .../pkg/test/spoof/openshift_checks.go | 40 ++++++++++++++ vendor/knative.dev/pkg/test/spoof/spoof.go | 8 ++- .../knative.dev/pkg/webhook/stats_reporter.go | 5 +- 16 files changed, 121 insertions(+), 109 deletions(-) delete mode 100644 config/core/100-namespace.yaml create mode 100644 config/core/200-roles/config-map-view-downstream.yaml create mode 100644 vendor/knative.dev/pkg/test/spoof/openshift_checks.go diff --git a/.ko.yaml b/.ko.yaml index 14afa53a5541..bceaa1320402 100644 --- a/.ko.yaml +++ b/.ko.yaml @@ -1,4 +1,5 @@ # Use :nonroot base image for all containers -defaultBaseImage: gcr.io/distroless/static:nonroot +defaultBaseImage: registry.access.redhat.com/ubi8/ubi-minimal:latest baseImageOverrides: + knative.dev/serving/test/test_images/runtime: gcr.io/distroless/static:nonroot knative.dev/serving/vendor/github.com/tsenart/vegeta/v12: ubuntu:latest diff --git a/config/core/100-namespace.yaml b/config/core/100-namespace.yaml deleted file mode 100644 index cce6c212b727..000000000000 --- a/config/core/100-namespace.yaml +++ /dev/null @@ -1,21 +0,0 @@ -# Copyright 2018 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: Namespace -metadata: - name: knative-serving - labels: - app.kubernetes.io/name: knative-serving - app.kubernetes.io/version: devel diff --git a/config/core/200-roles/config-map-view-downstream.yaml b/config/core/200-roles/config-map-view-downstream.yaml new file mode 100644 index 000000000000..91e329703f95 --- /dev/null +++ b/config/core/200-roles/config-map-view-downstream.yaml @@ -0,0 +1,25 @@ +# Extra role for downstream, so that users can get the autoscaling CM to fetch defaults. +kind: Role +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + namespace: knative-serving + name: openshift-serverless-view-serving-configmaps +rules: + - apiGroups: [""] + resources: ["configmaps"] + resourceNames: ["config-autoscaler"] + verbs: ["get", "list", "watch"] +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: openshift-serverless-view-serving-configmaps + namespace: knative-serving +subjects: + - kind: Group + name: system:authenticated + apiGroup: rbac.authorization.k8s.io +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: openshift-serverless-view-serving-configmaps diff --git a/config/core/deployments/activator-hpa.yaml b/config/core/deployments/activator-hpa.yaml index 85ddd54df771..a4e0fefa55ed 100644 --- a/config/core/deployments/activator-hpa.yaml +++ b/config/core/deployments/activator-hpa.yaml @@ -50,7 +50,7 @@ metadata: app.kubernetes.io/name: knative-serving app.kubernetes.io/version: devel spec: - minAvailable: 80% + minAvailable: 1 selector: matchLabels: app: activator diff --git a/config/core/deployments/webhook-hpa.yaml b/config/core/deployments/webhook-hpa.yaml index a3afec4d6097..15db14912e1c 100644 --- a/config/core/deployments/webhook-hpa.yaml +++ b/config/core/deployments/webhook-hpa.yaml @@ -48,7 +48,7 @@ metadata: app.kubernetes.io/name: knative-serving app.kubernetes.io/version: devel spec: - minAvailable: 80% + minAvailable: 1 selector: matchLabels: app: webhook diff --git a/openshift/release/artifacts/serving-core.yaml b/openshift/release/artifacts/serving-core.yaml index bba3ea8c18ea..ef570ec6eec6 100644 --- a/openshift/release/artifacts/serving-core.yaml +++ b/openshift/release/artifacts/serving-core.yaml @@ -1,26 +1,4 @@ --- -# Copyright 2018 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: Namespace -metadata: - name: knative-serving - labels: - app.kubernetes.io/name: knative-serving - app.kubernetes.io/version: "release-v1.12" ---- # Copyright 2023 The Knative Authors # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -235,6 +213,32 @@ rules: resources: ["images"] verbs: ["get", "list", "create", "update", "delete", "patch", "watch"] --- +# Extra role for downstream, so that users can get the autoscaling CM to fetch defaults. +kind: Role +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + namespace: knative-serving + name: openshift-serverless-view-serving-configmaps +rules: + - apiGroups: [""] + resources: ["configmaps"] + resourceNames: ["config-autoscaler"] + verbs: ["get", "list", "watch"] +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: openshift-serverless-view-serving-configmaps + namespace: knative-serving +subjects: + - kind: Group + name: system:authenticated + apiGroup: rbac.authorization.k8s.io +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: openshift-serverless-view-serving-configmaps +--- # Copyright 2019 The Knative Authors # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -6147,7 +6151,7 @@ metadata: app.kubernetes.io/name: knative-serving app.kubernetes.io/version: "release-v1.12" spec: - minAvailable: 80% + minAvailable: 1 selector: matchLabels: app: activator @@ -6640,7 +6644,7 @@ metadata: app.kubernetes.io/name: knative-serving app.kubernetes.io/version: "release-v1.12" spec: - minAvailable: 80% + minAvailable: 1 selector: matchLabels: app: webhook diff --git a/pkg/apis/serving/v1/revision_defaults.go b/pkg/apis/serving/v1/revision_defaults.go index 2b3f5f2f29da..b0960e6c311a 100644 --- a/pkg/apis/serving/v1/revision_defaults.go +++ b/pkg/apis/serving/v1/revision_defaults.go @@ -189,21 +189,14 @@ func (rs *RevisionSpec) defaultSecurityContext(psc *corev1.PodSecurityContext, c if updatedSC.AllowPrivilegeEscalation == nil { updatedSC.AllowPrivilegeEscalation = ptr.Bool(false) } - if psc.SeccompProfile == nil || psc.SeccompProfile.Type == "" { - if updatedSC.SeccompProfile == nil { - updatedSC.SeccompProfile = &corev1.SeccompProfile{} - } - if updatedSC.SeccompProfile.Type == "" { - updatedSC.SeccompProfile.Type = corev1.SeccompProfileTypeRuntimeDefault - } - } + if updatedSC.Capabilities == nil { updatedSC.Capabilities = &corev1.Capabilities{} updatedSC.Capabilities.Drop = []corev1.Capability{"ALL"} // Default in NET_BIND_SERVICE to allow binding to low-numbered ports. needsLowPort := false for _, p := range container.Ports { - if p.ContainerPort < 1024 { + if p.ContainerPort > 0 && p.ContainerPort < 1024 { needsLowPort = true break } @@ -212,11 +205,9 @@ func (rs *RevisionSpec) defaultSecurityContext(psc *corev1.PodSecurityContext, c updatedSC.Capabilities.Add = []corev1.Capability{"NET_BIND_SERVICE"} } } - - if psc.RunAsNonRoot == nil { + if psc.RunAsNonRoot == nil && updatedSC.RunAsNonRoot == nil { updatedSC.RunAsNonRoot = ptr.Bool(true) } - if *updatedSC != (corev1.SecurityContext{}) { container.SecurityContext = updatedSC } diff --git a/pkg/apis/serving/v1/revision_defaults_test.go b/pkg/apis/serving/v1/revision_defaults_test.go index 0fe5e65079b7..401cac325fa9 100644 --- a/pkg/apis/serving/v1/revision_defaults_test.go +++ b/pkg/apis/serving/v1/revision_defaults_test.go @@ -900,11 +900,8 @@ func TestRevisionDefaulting(t *testing.T) { ReadinessProbe: defaultProbe, Resources: defaultResources, SecurityContext: &corev1.SecurityContext{ - RunAsNonRoot: ptr.Bool(true), AllowPrivilegeEscalation: ptr.Bool(false), - SeccompProfile: &corev1.SeccompProfile{ - Type: corev1.SeccompProfileTypeRuntimeDefault, - }, + RunAsNonRoot: ptr.Bool(true), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, Add: []corev1.Capability{"NET_BIND_SERVICE"}, @@ -914,11 +911,8 @@ func TestRevisionDefaulting(t *testing.T) { Name: "sidecar", Resources: defaultResources, SecurityContext: &corev1.SecurityContext{ - RunAsNonRoot: ptr.Bool(true), AllowPrivilegeEscalation: ptr.Bool(false), - SeccompProfile: &corev1.SeccompProfile{ - Type: corev1.SeccompProfileTypeRuntimeDefault, - }, + RunAsNonRoot: ptr.Bool(true), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, @@ -927,11 +921,8 @@ func TestRevisionDefaulting(t *testing.T) { Name: "special-sidecar", Resources: defaultResources, SecurityContext: &corev1.SecurityContext{ - RunAsNonRoot: ptr.Bool(true), AllowPrivilegeEscalation: ptr.Bool(true), - SeccompProfile: &corev1.SeccompProfile{ - Type: corev1.SeccompProfileTypeRuntimeDefault, - }, + RunAsNonRoot: ptr.Bool(true), Capabilities: &corev1.Capabilities{ Add: []corev1.Capability{"NET_ADMIN"}, Drop: []corev1.Capability{}, @@ -941,12 +932,12 @@ func TestRevisionDefaulting(t *testing.T) { InitContainers: []corev1.Container{{ Name: "special-init", SecurityContext: &corev1.SecurityContext{ - RunAsNonRoot: ptr.Bool(true), AllowPrivilegeEscalation: ptr.Bool(true), SeccompProfile: &corev1.SeccompProfile{ Type: corev1.SeccompProfileTypeLocalhost, LocalhostProfile: ptr.String("special"), }, + RunAsNonRoot: ptr.Bool(true), Capabilities: &corev1.Capabilities{ Add: []corev1.Capability{"NET_ADMIN"}, }, @@ -1004,8 +995,8 @@ func TestRevisionDefaulting(t *testing.T) { ReadinessProbe: defaultProbe, Resources: defaultResources, SecurityContext: &corev1.SecurityContext{ - RunAsNonRoot: ptr.Bool(true), AllowPrivilegeEscalation: ptr.Bool(false), + RunAsNonRoot: ptr.Bool(true), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, @@ -1014,8 +1005,8 @@ func TestRevisionDefaulting(t *testing.T) { InitContainers: []corev1.Container{{ Name: "init", SecurityContext: &corev1.SecurityContext{ - RunAsNonRoot: ptr.Bool(true), AllowPrivilegeEscalation: ptr.Bool(false), + RunAsNonRoot: ptr.Bool(true), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, diff --git a/pkg/reconciler/revision/resources/queue.go b/pkg/reconciler/revision/resources/queue.go index 1fb964a53dab..b8cd617efe15 100644 --- a/pkg/reconciler/revision/resources/queue.go +++ b/pkg/reconciler/revision/resources/queue.go @@ -86,9 +86,6 @@ var ( Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, }, - SeccompProfile: &corev1.SeccompProfile{ - Type: corev1.SeccompProfileTypeRuntimeDefault, - }, } ) diff --git a/test/e2e/grpc_test.go b/test/e2e/grpc_test.go index bd67d62ef026..404819ed4760 100644 --- a/test/e2e/grpc_test.go +++ b/test/e2e/grpc_test.go @@ -34,7 +34,6 @@ import ( "golang.org/x/sync/errgroup" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" corev1 "k8s.io/api/core/v1" @@ -68,9 +67,6 @@ func hasPort(u string) bool { func dial(ctx *TestContext, host, domain string) (*grpc.ClientConn, error) { defaultPort := "80" - if test.ServingFlags.HTTPS { - defaultPort = "443" - } if !hasPort(host) { host = net.JoinHostPort(host, defaultPort) } @@ -83,12 +79,6 @@ func dial(ctx *TestContext, host, domain string) (*grpc.ClientConn, error) { } creds := insecure.NewCredentials() - if test.ServingFlags.HTTPS { - tlsConfig := test.TLSClientConfig(context.Background(), ctx.t.Logf, ctx.clients) - // Set ServerName for pseudo hostname with TLS. - tlsConfig.ServerName = domain - creds = credentials.NewTLS(tlsConfig) - } return grpc.Dial( host, @@ -321,11 +311,6 @@ func streamTest(tc *TestContext, host, domain string) { func testGRPC(t *testing.T, f grpcTest, fopts ...rtesting.ServiceOption) { t.Helper() - // TODO: https option with parallel leads to flakes. - // https://github.com/knative/serving/issues/11387 - if !test.ServingFlags.HTTPS { - t.Parallel() - } // Setup clients := Setup(t) @@ -366,16 +351,13 @@ func testGRPC(t *testing.T, f grpcTest, fopts ...rtesting.ServiceOption) { } host := url.Host - if !test.ServingFlags.ResolvableDomain { + if true { addr, mapper, err := ingress.GetIngressEndpoint(context.Background(), clients.KubeClient, pkgTest.Flags.IngressEndpoint) if err != nil { t.Fatal("Could not get service endpoint:", err) } - if test.ServingFlags.HTTPS { - host = net.JoinHostPort(addr, mapper("443")) - } else { - host = net.JoinHostPort(addr, mapper("80")) - } + + host = net.JoinHostPort(addr, mapper("80")) } f(&TestContext{ diff --git a/test/e2e/securedefaults/secure_pod_defaults_test.go b/test/e2e/securedefaults/secure_pod_defaults_test.go index af1498deede8..96e4839a91e6 100644 --- a/test/e2e/securedefaults/secure_pod_defaults_test.go +++ b/test/e2e/securedefaults/secure_pod_defaults_test.go @@ -60,9 +60,6 @@ func TestSecureDefaults(t *testing.T) { if revisionSC.AllowPrivilegeEscalation == nil || *revisionSC.AllowPrivilegeEscalation { t.Errorf("Expected allowPrivilegeEscalation: false, got %v", revisionSC.AllowPrivilegeEscalation) } - if revisionSC.SeccompProfile == nil || revisionSC.SeccompProfile.Type != v1.SeccompProfileTypeRuntimeDefault { - t.Errorf("Expected seccompProfile to be RuntimeDefault, got: %v", revisionSC.SeccompProfile) - } } func TestUnsafePermitted(t *testing.T) { diff --git a/vendor/knative.dev/pkg/controller/stats_reporter.go b/vendor/knative.dev/pkg/controller/stats_reporter.go index 6735285db478..67ec3d6a123f 100644 --- a/vendor/knative.dev/pkg/controller/stats_reporter.go +++ b/vendor/knative.dev/pkg/controller/stats_reporter.go @@ -199,7 +199,7 @@ func (r *reporter) ReportReconcile(duration time.Duration, success string, key t return err } - metrics.RecordBatch(ctx, reconcileCountStat.M(1), - reconcileLatencyStat.M(duration.Milliseconds())) + // TODO skonto: fix latency histograms + metrics.Record(ctx, reconcileCountStat.M(1)) return nil } diff --git a/vendor/knative.dev/pkg/test/helpers/name.go b/vendor/knative.dev/pkg/test/helpers/name.go index 0ceaed594304..fd55ec5b0a32 100644 --- a/vendor/knative.dev/pkg/test/helpers/name.go +++ b/vendor/knative.dev/pkg/test/helpers/name.go @@ -26,7 +26,7 @@ import ( const ( letterBytes = "abcdefghijklmnopqrstuvwxyz" randSuffixLen = 8 - nameLengthLimit = 50 + nameLengthLimit = 40 sep = '-' sepS = "-" testNamePrefix = "Test" diff --git a/vendor/knative.dev/pkg/test/spoof/openshift_checks.go b/vendor/knative.dev/pkg/test/spoof/openshift_checks.go new file mode 100644 index 000000000000..acaebe95bcf1 --- /dev/null +++ b/vendor/knative.dev/pkg/test/spoof/openshift_checks.go @@ -0,0 +1,40 @@ +package spoof + +import ( + "fmt" + "net/http" + "strings" +) + +// isUnknownAuthority checks if the error contains "certificate signed by unknown authority". +// This error happens when OpenShift Route starts/changes to use passthrough mode. It takes a little bit time to be synced. +func isUnknownAuthority(err error) bool { + return err != nil && strings.Contains(err.Error(), "certificate signed by unknown authority") +} + +// RetryingRouteInconsistency retries common requests seen when creating a new route +// - 503 to account for Openshift route inconsistency (https://jira.coreos.com/browse/SRVKS-157) +func RouteInconsistencyRetryChecker(resp *Response) (bool, error) { + if resp.StatusCode == http.StatusServiceUnavailable { + return true, fmt.Errorf("retrying route inconsistency request: %s", resp) + } + return false, nil +} + +// RouteInconsistencyMultiRetryChecker retries common requests seen when creating a new route +// - 503 to account for Openshift route inconsistency (https://jira.coreos.com/browse/SRVKS-157) +func RouteInconsistencyMultiRetryChecker() ResponseChecker { + const neededSuccesses = 32 + var successes int + return func(resp *Response) (bool, error) { + if resp.StatusCode == http.StatusServiceUnavailable { + successes = 0 + return true, fmt.Errorf("retrying route inconsistency request: %s", resp) + } + successes++ + if successes < neededSuccesses { + return true, fmt.Errorf("successful requests: %d, required: %d", successes, neededSuccesses) + } + return false, nil + } +} diff --git a/vendor/knative.dev/pkg/test/spoof/spoof.go b/vendor/knative.dev/pkg/test/spoof/spoof.go index 319e2ead86f3..c9a66749b020 100644 --- a/vendor/knative.dev/pkg/test/spoof/spoof.go +++ b/vendor/knative.dev/pkg/test/spoof/spoof.go @@ -163,7 +163,7 @@ func (sc *SpoofingClient) Do(req *http.Request, errorRetryCheckers ...interface{ // If no retry checkers are specified `DefaultErrorRetryChecker` will be used. func (sc *SpoofingClient) Poll(req *http.Request, inState ResponseChecker, checkers ...interface{}) (*Response, error) { if len(checkers) == 0 { - checkers = []interface{}{ErrorRetryChecker(DefaultErrorRetryChecker), ResponseRetryChecker(DefaultResponseRetryChecker)} + checkers = []interface{}{ErrorRetryChecker(DefaultErrorRetryChecker), ResponseRetryChecker(DefaultResponseRetryChecker), ResponseRetryChecker(RouteInconsistencyRetryChecker)} } var resp *Response @@ -251,6 +251,9 @@ func DefaultErrorRetryChecker(err error) (bool, error) { if isNoRouteToHostError(err) { return true, fmt.Errorf("retrying for 'no route to host' error: %w", err) } + if isUnknownAuthority(err) { + return true, fmt.Errorf("retrying for certificate signed by unknown authority: %w", err) + } return false, err } @@ -327,6 +330,9 @@ func (sc *SpoofingClient) endpointState( } func (sc *SpoofingClient) Check(req *http.Request, inState ResponseChecker, checkers ...interface{}) (*Response, error) { + if len(checkers) == 0 { + checkers = []interface{}{ErrorRetryChecker(DefaultErrorRetryChecker), ResponseRetryChecker(DefaultResponseRetryChecker), ResponseRetryChecker(RouteInconsistencyMultiRetryChecker())} + } resp, err := sc.Do(req, checkers...) if err != nil { return nil, err diff --git a/vendor/knative.dev/pkg/webhook/stats_reporter.go b/vendor/knative.dev/pkg/webhook/stats_reporter.go index 1fe30e8af16d..e0115024ca69 100644 --- a/vendor/knative.dev/pkg/webhook/stats_reporter.go +++ b/vendor/knative.dev/pkg/webhook/stats_reporter.go @@ -106,9 +106,8 @@ func (r *reporter) ReportAdmissionRequest(req *admissionv1.AdmissionRequest, res return err } - metrics.RecordBatch(ctx, requestCountM.M(1), - // Convert time.Duration in nanoseconds to milliseconds - responseTimeInMsecM.M(float64(d.Milliseconds()))) + // TODO skonto: fix latency histograms + metrics.Record(ctx, requestCountM.M(1)) return nil }