From f1552e55b10832c13e90c4b4cc5025c0a087d203 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 6 Mar 2025 17:41:37 -0800 Subject: [PATCH 01/10] load BackendTLSPolicy in standalone mode (#5431) Signed-off-by: Arko Dasgupta (cherry picked from commit 4d914ae47a30b5e5b2b6817e4ed13d8c4ad094eb) Signed-off-by: Guy Daich --- .../translate/in/backend-endpoint.yaml | 13 +++++++++ .../translate/out/backend-endpoint.all.yaml | 27 +++++++++++++++++++ internal/gatewayapi/resource/load.go | 14 ++++++++++ release-notes/current.yaml | 2 +- 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml b/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml index d2aa0f78f0..0f1f3ab69f 100644 --- a/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml +++ b/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml @@ -44,3 +44,16 @@ spec: - ip: address: 0.0.0.0 port: 3000 +--- +apiVersion: gateway.networking.k8s.io/v1alpha3 +kind: BackendTLSPolicy +metadata: + name: example-com-tls-policy +spec: + targetRefs: + - group: 'gateway.envoyproxy.io' + kind: Backend + name: backend + validation: + wellKnownCACertificates: "System" + hostname: www.example.com diff --git a/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml b/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml index 0841f84877..41a84b3d83 100644 --- a/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml +++ b/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml @@ -1,3 +1,30 @@ +backendTLSPolicies: +- kind: BackendTLSPolicy + metadata: + creationTimestamp: null + name: example-com-tls-policy + namespace: envoy-gateway-system + spec: + targetRefs: + - group: gateway.envoyproxy.io + kind: Backend + name: backend + validation: + hostname: www.example.com + wellKnownCACertificates: System + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: eg + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller backends: - kind: Backend metadata: diff --git a/internal/gatewayapi/resource/load.go b/internal/gatewayapi/resource/load.go index e3357b0b27..b3d1a523a3 100644 --- a/internal/gatewayapi/resource/load.go +++ b/internal/gatewayapi/resource/load.go @@ -21,6 +21,7 @@ import ( utilyaml "k8s.io/apimachinery/pkg/util/yaml" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + gwapiv1a3 "sigs.k8s.io/gateway-api/apis/v1alpha3" "sigs.k8s.io/yaml" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -341,6 +342,19 @@ func loadKubernetesYAMLToResources(input []byte, addMissingResources bool) (*Res Data: typedData.(map[string]string), } resources.ConfigMaps = append(resources.ConfigMaps, configMap) + case KindBackendTLSPolicy: + typedSpec := spec.Interface() + backendTLSPolicy := &gwapiv1a3.BackendTLSPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: KindBackendTLSPolicy, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: typedSpec.(gwapiv1a3.BackendTLSPolicySpec), + } + resources.BackendTLSPolicies = append(resources.BackendTLSPolicies, backendTLSPolicy) } return nil diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 288804f88b..d2821d1834 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -10,6 +10,7 @@ security updates: | new features: | bug fixes: | + Added support for BackendTLSPolicy parsing in Standalone mode. # Enhancements that improve performance. performance improvements: | @@ -19,4 +20,3 @@ deprecations: | # Other notable changes not covered by the above sections. Other changes: | - From 2a3e1cd27b57c6348f68408c9f457bbf0e35bdeb Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Sat, 8 Mar 2025 06:42:51 +0800 Subject: [PATCH 02/10] Wasm: cache Wasm OCI image permission check results (#5358) * add TTL for wasm permission check Signed-off-by: Huabing (Robin) Zhao * fix test Signed-off-by: Huabing (Robin) Zhao * change Signed-off-by: Huabing (Robin) Zhao * refresh the cache Signed-off-by: Huabing (Robin) Zhao * purge the cache Signed-off-by: Huabing (Robin) Zhao * refactor Signed-off-by: Huabing (Robin) Zhao * on retry on retriable errors Signed-off-by: Huabing (Robin) Zhao * add release note Signed-off-by: Huabing (Robin) Zhao --------- Signed-off-by: Huabing (Robin) Zhao (cherry picked from commit 672de8adcdfd351eb1f7243e62f3f611abc7bd8c) Signed-off-by: Guy Daich --- go.mod | 1 + go.sum | 2 + internal/wasm/cache.go | 49 +++-- internal/wasm/premissioncache.go | 266 ++++++++++++++++++++++++ internal/wasm/premissioncache_test.go | 282 ++++++++++++++++++++++++++ release-notes/current.yaml | 2 + test/e2e/tests/wasm_oci.go | 4 +- 7 files changed, 588 insertions(+), 18 deletions(-) create mode 100644 internal/wasm/premissioncache.go create mode 100644 internal/wasm/premissioncache_test.go diff --git a/go.mod b/go.mod index bbb1ff92c2..ff70a2a484 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( fortio.org/log v1.17.1 github.com/Masterminds/semver/v3 v3.3.1 github.com/andybalholm/brotli v1.1.1 + github.com/avast/retry-go v3.0.0+incompatible github.com/cenkalti/backoff/v4 v4.3.0 github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc diff --git a/go.sum b/go.sum index ffb75d6685..d23202d252 100644 --- a/go.sum +++ b/go.sum @@ -99,6 +99,8 @@ github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:l github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= +github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHSxpiH9JdtuBj0= +github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= diff --git a/internal/wasm/cache.go b/internal/wasm/cache.go index 785d8d3701..7a045038f0 100644 --- a/internal/wasm/cache.go +++ b/internal/wasm/cache.go @@ -67,11 +67,16 @@ type localFileCache struct { // option sets for configuring the cache. CacheOptions + // permissionCheckCache is the cache for permission check for private OCI images. + // The permission check is run periodically by a background goroutine and the result is cached. + permissionCheckCache *permissionCache + // logger logger logging.Logger } func (c *localFileCache) Start(ctx context.Context) { + c.permissionCheckCache.Start(ctx) go c.purge(ctx) } @@ -131,9 +136,12 @@ type cacheEntry struct { func newLocalFileCache(options CacheOptions, logger logging.Logger) *localFileCache { options = options.sanitize() cache := &localFileCache{ - httpFetcher: NewHTTPFetcher(options.HTTPRequestTimeout, options.HTTPRequestMaxRetries, logger), - modules: make(map[moduleKey]*cacheEntry), - checksums: make(map[string]*checksumEntry), + httpFetcher: NewHTTPFetcher(options.HTTPRequestTimeout, options.HTTPRequestMaxRetries, logger), + modules: make(map[moduleKey]*cacheEntry), + checksums: make(map[string]*checksumEntry), + permissionCheckCache: newPermissionCache( + permissionCacheOptions{}, + logger), CacheOptions: options, logger: logger, } @@ -220,7 +228,7 @@ func (c *localFileCache) getOrFetch(key cacheKey, opts GetOptions) (*cacheEntry, if ce != nil { // We still need to check if the pull secret is correct if it is a private OCI image. if u.Scheme == "oci" && ce.isPrivate { - if err = c.checkPermission(ctx, u, insecure, opts); err != nil { + if _, err := c.permissionCheckCache.IsAllowed(ctx, u, opts.PullSecret, insecure); err != nil { return nil, err } } @@ -250,7 +258,24 @@ func (c *localFileCache) getOrFetch(key cacheKey, opts GetOptions) (*cacheEntry, if len(opts.PullSecret) > 0 { isPrivate = true } - if imageBinaryFetcher, dChecksum, err = c.prepareFetch(ctx, u, insecure, opts); err != nil { + + imageBinaryFetcher, dChecksum, err = c.prepareFetch(ctx, u, insecure, opts.PullSecret) + + if isPrivate { + e := &permissionCacheEntry{ + image: u, + fetcherOption: &ImageFetcherOption{ + Insecure: insecure, + PullSecret: opts.PullSecret, + }, + lastCheck: time.Now(), + lastAccess: time.Now(), + checkError: err, + } + c.permissionCheckCache.Put(e) + } + + if err != nil { wasmRemoteFetchTotal.WithFailure(reasonManifestError).Increment() return nil, fmt.Errorf("could not fetch Wasm OCI image: %w", err) } @@ -287,24 +312,16 @@ func (c *localFileCache) getOrFetch(key cacheKey, opts GetOptions) (*cacheEntry, return c.addEntry(key, b, isPrivate) } -func (c *localFileCache) checkPermission(ctx context.Context, u *url.URL, insecure bool, opts GetOptions) error { - // Try to get the image metadata to check if the pull secret is correct. - if _, _, err := c.prepareFetch(ctx, u, insecure, opts); err != nil { - return fmt.Errorf("failed to login to private registry: %w", err) - } - return nil -} - // prepareFetch won't fetch the binary, but it will prepare the binaryFetcher and actualDigest. func (c *localFileCache) prepareFetch( - ctx context.Context, url *url.URL, insecure bool, opts GetOptions) ( + ctx context.Context, url *url.URL, insecure bool, pullSecret []byte) ( binaryFetcher func() ([]byte, error), actualDigest string, err error, ) { imgFetcherOps := ImageFetcherOption{ Insecure: insecure, } - if len(opts.PullSecret) > 0 { - imgFetcherOps.PullSecret = opts.PullSecret + if len(pullSecret) > 0 { + imgFetcherOps.PullSecret = pullSecret } fetcher := NewImageFetcher(ctx, imgFetcherOps, c.logger) if binaryFetcher, actualDigest, err = fetcher.PrepareFetch(url.Host + url.Path); err != nil { diff --git a/internal/wasm/premissioncache.go b/internal/wasm/premissioncache.go new file mode 100644 index 0000000000..e680574ad9 --- /dev/null +++ b/internal/wasm/premissioncache.go @@ -0,0 +1,266 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +// Copyright Istio 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 wasm + +import ( + "context" + "encoding/hex" + "errors" + "fmt" + "net/http" + "net/url" + "sync" + "time" + + "github.com/avast/retry-go" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" + + "github.com/envoyproxy/gateway/internal/logging" +) + +type permissionCacheOptions struct { + // checkInterval is the interval to recheck the permission for the cached permission entries. + checkInterval time.Duration + + // permissionExpiry is the expiry time for permission cache entry. + // The permission cache entry will be updated by rechecking the OCI image permission against the pull secret. + permissionExpiry time.Duration + + // cacheExpiry is the expiry time for the permission cache. + // The permission cache will be removed if it is not accessed for the specified expiry time. + // This is used to purge the cache. + cacheExpiry time.Duration +} + +// sanitize validates and sets the default values for the permission cache options. +func (o *permissionCacheOptions) sanitize() { + if o.checkInterval == 0 { + o.checkInterval = 5 * time.Minute + } + if o.permissionExpiry == 0 { + o.permissionExpiry = 1 * time.Hour + } + if o.cacheExpiry == 0 { + o.cacheExpiry = 24 * time.Hour + } +} + +// permissionCache is a cache for permission check for private OCI images. +// After a new permission is put into the cache, it will be checked periodically by a background goroutine. +// It is used to avoid blocking the translator due to the permission check. +type permissionCache struct { + sync.Mutex + permissionCacheOptions + + cache map[string]*permissionCacheEntry + logger logging.Logger +} + +// permissionCacheEntry is an entry in the permission cache. +type permissionCacheEntry struct { + // The oci image URL. + image *url.URL + // fetcherOption contains the pull secret for the image. + fetcherOption *ImageFetcherOption + // The last time the pull secret is checked against the image. + lastCheck time.Time + // The error returned by the OCI registry when checking the permission. + // If error is not nil, the permission is not allowed. + // If it's a permission error, it's represented by a transport.Error with 401 or 403 HTTP status code. + // But it's not necessarily a permission error, it could be other errors like network error, non-exist image, etc. + // In this case, the permission is also not allowed. + checkError error + // The last time the cache entry is accessed. + lastAccess time.Time +} + +func (e *permissionCacheEntry) key() string { + return permissionCacheKey(e.image, e.fetcherOption.PullSecret) +} + +// isPermissionExpired returns true if the permission check is older +// than the specified expiry duration. If this is true, the entry +// should be rechecked. +func (e *permissionCacheEntry) isPermissionExpired(expiry time.Duration) bool { + return time.Now().After(e.lastCheck.Add(expiry)) +} + +// isCacheExpired returns true if the cache entry has not been accessed +// for the specified expiry duration. If this is true, the entry +// should be removed. +func (e *permissionCacheEntry) isCacheExpired(expiry time.Duration) bool { + return time.Now().After(e.lastAccess.Add(expiry)) +} + +// permissionCacheKey generates a key for a permission cache entry. +// The key is the hex encoded of concatenation of the image URL and the pull secret. +func permissionCacheKey(image *url.URL, pullSecret []byte) string { + b := make([]byte, len(image.String())+len(pullSecret)) + copy(b, image.String()) + copy(b[len(image.String()):], pullSecret) + return hex.EncodeToString(b) +} + +// newPermissionCache creates a new permission cache with a given TTL. +func newPermissionCache(options permissionCacheOptions, logger logging.Logger) *permissionCache { + options.sanitize() + return &permissionCache{ + cache: make(map[string]*permissionCacheEntry), + permissionCacheOptions: options, + logger: logger, + } +} + +// checkAndUpdatePermission checks the permission of the image against the pull secret and updates the cache entry. +func (p *permissionCache) checkAndUpdatePermission(ctx context.Context, e *permissionCacheEntry) error { + fetcher := NewImageFetcher(ctx, *e.fetcherOption, p.logger) + _, _, err := fetcher.PrepareFetch(e.image.Host + e.image.Path) + e.checkError = err + e.lastCheck = time.Now() + return err +} + +// start starts a background goroutine to periodically check the permission for the cached permission entries. +func (p *permissionCache) Start(ctx context.Context) { + go func() { + ticker := time.NewTicker(p.checkInterval) + defer ticker.Stop() + for { + select { + case <-ticker.C: + func() { + p.Lock() + defer p.Unlock() + for _, e := range p.cache { + if e.isCacheExpired(p.cacheExpiry) { + p.logger.Info("removing permission cache entry", "image", e.image.String()) + delete(p.cache, e.key()) + continue + } + if e.isPermissionExpired(p.permissionExpiry) { + const retryAttempts = 3 + const retryDelay = 1 * time.Second + p.logger.Info("rechecking permission for image", "image", e.image.String()) + err := retry.Do( + func() error { + err := p.checkAndUpdatePermission(ctx, e) + if err != nil && isRetriableError(err) { + p.logger.Error( + err, + "failed to check permission for image, will retry again", + "image", + e.image.String()) + return err + } + return nil + }, + retry.Attempts(retryAttempts), + retry.DelayType(retry.BackOffDelay), + retry.Delay(retryDelay), + retry.Context(ctx), + ) + if err != nil { + p.logger.Error( + err, + fmt.Sprintf("failed to recheck permission for image after %d attempts", retryAttempts), + "image", + e.image.String()) + } + } + } + }() + case <-ctx.Done(): + return + } + } + }() +} + +// isRetriableError checks if the error is retriable. +// If the error is a permission error, it's not retriable. For example, 401 and 403 HTTP status code. +func isRetriableError(err error) bool { + var terr *transport.Error + if errors.As(err, &terr) { + if terr.StatusCode == http.StatusUnauthorized || terr.StatusCode == http.StatusForbidden { + return false + } + } + return true +} + +// put puts a new permission cache entry into the cache. +func (p *permissionCache) Put(e *permissionCacheEntry) { + p.Lock() + defer p.Unlock() + e.lastAccess = time.Now() + e.lastCheck = time.Now() + p.cache[e.key()] = e +} + +// IsAllowed checks if the given image is allowed to be accessed with the provided pull secret. +// If the permission is not found in the cache, this method will block until the permission is checked and cached. +// This blocking won't be too long as it's only for the first time permission check and won't retry. Subsequent +// permission checks will be done in a background goroutine by the permission cache. +// +// If any error occurs, the permission is considered not allowed. +// The error can be a permission error or other errors like network error, non-exist image, etc. +func (p *permissionCache) IsAllowed(ctx context.Context, image *url.URL, pullSecret []byte, insecure bool) (bool, error) { + p.Lock() + defer p.Unlock() + key := permissionCacheKey(image, pullSecret) + if e, ok := p.cache[key]; ok { + e.lastAccess = time.Now() + return e.checkError == nil, e.checkError + } + + e := &permissionCacheEntry{ + image: image, + fetcherOption: &ImageFetcherOption{ + Insecure: insecure, + PullSecret: pullSecret, + }, + } + // Do not retry if the permission check fails because we don't want to block the translator for too long. + // The permission check will be retried in the background goroutine by the permission cache. + if err := p.checkAndUpdatePermission(ctx, e); err != nil { + p.logger.Error(err, "failed to check permission for image", "image", e.image.String()) + } + e.lastAccess = time.Now() + p.cache[key] = e + return e.checkError == nil, e.checkError +} + +// getForTest is a test helper to get a permission cache entry from the cache. +func (p *permissionCache) getForTest(key string) (permissionCacheEntry, bool) { + p.Lock() + defer p.Unlock() + entry, ok := p.cache[key] + if !ok { + return permissionCacheEntry{}, false + } + return *entry, true +} + +// deleteForTest is a test helper to delete a permission cache entry from the cache. +func (p *permissionCache) deleteForTest(key string) { + p.Lock() + defer p.Unlock() + delete(p.cache, key) +} diff --git a/internal/wasm/premissioncache_test.go b/internal/wasm/premissioncache_test.go new file mode 100644 index 0000000000..2fdb95ff18 --- /dev/null +++ b/internal/wasm/premissioncache_test.go @@ -0,0 +1,282 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +// Copyright Istio 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 wasm + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "sync" + "testing" + "time" + + "github.com/google/go-containerregistry/pkg/registry" + "github.com/stretchr/testify/require" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/logging" +) + +func TestPermissionCache(t *testing.T) { + lock := sync.Mutex{} + // Flag to control whether the permission check should fail. + failPermissionCheck := false + + // Set up a fake registry for OCI images. + reg := registry.New() + tos := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lock.Lock() + defer lock.Unlock() + if failPermissionCheck { + http.Error(w, "permission denied", http.StatusUnauthorized) + return + } + reg.ServeHTTP(w, r) + })) + defer tos.Close() + ou, err := url.Parse(tos.URL) + if err != nil { + t.Fatal(err) + } + _, _ = setupOCIRegistry(t, ou.Host) + ociURLWithTag := fmt.Sprintf("oci://%s/test/valid/docker:v0.1.0", ou.Host) + ociURLWithLatestTag := fmt.Sprintf("oci://%s/test/valid/docker:latest", ou.Host) + image, _ := url.Parse(ociURLWithTag) + latestImage, _ := url.Parse(ociURLWithLatestTag) + secret := []byte("") + + t.Run("Cached permission should be updated", func(t *testing.T) { + lock.Lock() + failPermissionCheck = false + lock.Unlock() + + ctx := context.Background() + defer ctx.Done() + cache, entry := setupTestPermissionCache( + permissionCacheOptions{ + checkInterval: 10 * time.Nanosecond, + permissionExpiry: 10 * time.Nanosecond, + }, + image, + latestImage, + secret) + cache.Start(ctx) + + lastAccessTime := entry.lastAccess + lastCheckTime := entry.lastCheck + + time.Sleep(1 * time.Millisecond) + allowed, err := cache.IsAllowed(context.Background(), image, secret, true) + require.True( + t, + allowed, + "permission should be rechecked and allowed after permission expired") + require.NoError( + t, + err, + "permission should be rechecked and allowed after permission expired") + + entry, ok := cache.getForTest(entry.key()) + require.True(t, ok, "cache entry should exist") + require.True(t, entry.lastAccess.After(lastAccessTime), "last access time should be updated") + require.True(t, entry.lastCheck.After(lastCheckTime), "last check time should be updated") + }) + + t.Run("Cached permission failed after recheck", func(t *testing.T) { + lock.Lock() + failPermissionCheck = true + lock.Unlock() + + ctx := context.Background() + defer ctx.Done() + cache, entry := setupTestPermissionCache( + permissionCacheOptions{ + checkInterval: 10 * time.Nanosecond, + permissionExpiry: 10 * time.Nanosecond, + }, + image, + latestImage, + secret) + cache.Start(ctx) + + lastAccessTime := entry.lastAccess + lastCheckTime := entry.lastCheck + + time.Sleep(1 * time.Millisecond) + allowed, err := cache.IsAllowed(context.Background(), image, secret, true) + require.False(t, isRetriableError(err), "permission check error should not be retriable") + require.False( + t, + allowed, + "permission should be rechecked and denied after permission expired and secret is invalid") + require.Error( + t, + err, + "permission should be rechecked and denied after permission expired and secret is invalid") + + entry, ok := cache.getForTest(entry.key()) + require.True(t, ok, "cache entry should exist") + require.True(t, entry.lastAccess.After(lastAccessTime), "last access time should be updated") + require.True(t, entry.lastCheck.After(lastCheckTime), "last check time should be updated") + }) + + t.Run("Cached permission should be removed after expiry", func(t *testing.T) { + lock.Lock() + failPermissionCheck = false + lock.Unlock() + + ctx := context.Background() + defer ctx.Done() + cache, entry := setupTestPermissionCache( + permissionCacheOptions{ + checkInterval: 10 * time.Nanosecond, + cacheExpiry: 10 * time.Nanosecond, + }, + image, + latestImage, + secret) + cache.Start(ctx) + + lastAccessTime := entry.lastAccess + lastCheckTime := entry.lastCheck + + time.Sleep(1 * time.Millisecond) + key := entry.key() + entry, ok := cache.getForTest(key) + require.False(t, ok, "cache entry should be removed after expiry") + allowed, err := cache.IsAllowed(context.Background(), image, secret, true) + require.True(t, + allowed, + "permission should be rechecked and allowed after cache removed") + require.NoError(t, + err, + "permission should be rechecked and allowed after cache removed") + entry, ok = cache.getForTest(key) + require.True(t, ok, "expired entry should be added after recheck") + require.True(t, entry.lastAccess.After(lastAccessTime), "last access time should be updated") + require.True(t, entry.lastCheck.After(lastCheckTime), "last check time should be updated") + }) + + t.Run("Non-exist permission should be checked and cached after first access for allowed permission", func(t *testing.T) { + lock.Lock() + failPermissionCheck = false + lock.Unlock() + + ctx := context.Background() + defer ctx.Done() + cache, entry := setupTestPermissionCache( + permissionCacheOptions{ + checkInterval: 10 * time.Nanosecond, + }, + image, + latestImage, + secret) + key := entry.key() + // remove the cache entry + cache.deleteForTest(key) + cache.Start(ctx) + + _, ok := cache.getForTest(key) + require.False(t, ok, "cache entry should not exist before access") + + now := time.Now() + allowed, err := cache.IsAllowed(context.Background(), image, secret, true) + require.True(t, + allowed, + "non-exist permission should be checked and allowed at first access") + require.NoError(t, + err, + "non-exist permission should be checked and allowed at first access") + + entry, ok = cache.getForTest(key) + require.True(t, ok, "non-exist permission should be added to the cache after first access ") + require.True(t, entry.lastAccess.After(now), "last access time should be updated after first access") + require.True(t, entry.lastCheck.After(now), "last check time should be updated after first access") + }) + + t.Run("Non-exist permission should be checked and cached after first access for denied permission", func(t *testing.T) { + lock.Lock() + failPermissionCheck = true + lock.Unlock() + + ctx := context.Background() + defer ctx.Done() + cache, entry := setupTestPermissionCache( + permissionCacheOptions{ + checkInterval: 10 * time.Nanosecond, + }, + image, + latestImage, + secret) + key := entry.key() + // remove the cache entry + cache.deleteForTest(key) + cache.Start(ctx) + + _, ok := cache.getForTest(key) + require.False(t, ok, "cache entry should not exist before access") + + now := time.Now() + allowed, err := cache.IsAllowed(context.Background(), image, secret, true) + require.False(t, + allowed, + "non-exist permission should be checked and denied at first access if secret is invalid") + require.Error(t, + err, + "non-exist permission should be checked and denied at first access if secret is invalid") + + entry, ok = cache.getForTest(key) + require.True(t, ok, "non-exist permission should be added to the cache after first access ") + require.True(t, entry.lastAccess.After(now), "last access time should be updated after first access") + require.True(t, entry.lastCheck.After(now), "last check time should be updated after first access") + }) +} + +// setupTestPermissionCache sets up a permission cache for testing. +func setupTestPermissionCache(options permissionCacheOptions, image, latestImage *url.URL, secret []byte) (*permissionCache, permissionCacheEntry) { + // Setup the permission cache. + cache := newPermissionCache( + options, + logging.DefaultLogger(egv1a1.LogLevelInfo)) + + entry := &permissionCacheEntry{ + image: image, + fetcherOption: &ImageFetcherOption{ + PullSecret: secret, + Insecure: true, + }, + lastCheck: time.Now(), + } + cache.Put(entry) + + // Add one more entry for the latest image to test the cache can handle multiple entries correctly. + cache.Put(&permissionCacheEntry{ + image: latestImage, + fetcherOption: &ImageFetcherOption{ + PullSecret: secret, + Insecure: true, + }, + lastCheck: time.Now(), + }) + + return cache, *entry +} diff --git a/release-notes/current.yaml b/release-notes/current.yaml index d2821d1834..b59001473b 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -14,6 +14,8 @@ bug fixes: | # Enhancements that improve performance. performance improvements: | + Added a cache for the Wasm OCI image permission checks and check the pullSecrets against the OCI image registry in + a background goroutine. # Deprecated features or APIs. deprecations: | diff --git a/test/e2e/tests/wasm_oci.go b/test/e2e/tests/wasm_oci.go index 514ab93735..f9f74cec21 100644 --- a/test/e2e/tests/wasm_oci.go +++ b/test/e2e/tests/wasm_oci.go @@ -197,7 +197,7 @@ var OCIWasmTest = suite.ConformanceTest{ t, suite.Client, types.NamespacedName{Name: testEEP, Namespace: testNS}, suite.ControllerName, - ancestorRef, "failed to login to private registry") + ancestorRef, "UNAUTHORIZED: authentication required") }) // Verify that the wasm module can't be loaded if the password is incorrect @@ -232,7 +232,7 @@ var OCIWasmTest = suite.ConformanceTest{ t, suite.Client, types.NamespacedName{Name: testEEP, Namespace: testNS}, suite.ControllerName, - ancestorRef, "failed to login to private registry") + ancestorRef, "UNAUTHORIZED: authentication required") }) }, } From 268175a2a86f3123f06fd51b8f4e662fad4838da Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 11 Mar 2025 18:19:06 -0700 Subject: [PATCH 03/10] Load EnvoyExtensionPolicy in standalone mode (#5460) * load EnvoyExtensionPolicy in standalone mode Signed-off-by: Takeshi Yoneda * more Signed-off-by: Takeshi Yoneda * release note Signed-off-by: Takeshi Yoneda * review: use a valid target name instead of myapp Signed-off-by: Takeshi Yoneda * gen Signed-off-by: Takeshi Yoneda --------- Signed-off-by: Takeshi Yoneda (cherry picked from commit 4be098d71a292da8629817a767b35974435c61ea) Signed-off-by: Guy Daich --- .../translate/in/backend-endpoint.yaml | 18 +++++++++ .../translate/out/backend-endpoint.all.yaml | 37 +++++++++++++++++++ internal/gatewayapi/resource/load.go | 15 +++++++- release-notes/current.yaml | 2 +- 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml b/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml index 0f1f3ab69f..f845c0941b 100644 --- a/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml +++ b/internal/cmd/egctl/testdata/translate/in/backend-endpoint.yaml @@ -57,3 +57,21 @@ spec: validation: wellKnownCACertificates: "System" hostname: www.example.com +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: EnvoyExtensionPolicy +metadata: + name: ext-proc-example +spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: backend + extProc: + - backendRefs: + - name: grpc-ext-proc + port: 9002 + processingMode: + request: {} + response: + body: Streamed diff --git a/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml b/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml index 41a84b3d83..1085a7aa20 100644 --- a/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml +++ b/internal/cmd/egctl/testdata/translate/out/backend-endpoint.all.yaml @@ -43,6 +43,43 @@ backends: reason: Accepted status: "True" type: Accepted +envoyExtensionPolicies: +- kind: EnvoyExtensionPolicy + metadata: + creationTimestamp: null + name: ext-proc-example + namespace: envoy-gateway-system + spec: + extProc: + - backendRefs: + - group: "" + kind: Service + name: grpc-ext-proc + port: 9002 + processingMode: + request: {} + response: + body: Streamed + targetRefs: + - group: gateway.networking.k8s.io + kind: HTTPRoute + name: backend + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: eg + namespace: envoy-gateway-system + conditions: + - lastTransitionTime: null + message: |- + Wasm: wasm cache is not initialized + ExtProc: service envoy-gateway-system/grpc-ext-proc not found. + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller gatewayClass: kind: GatewayClass metadata: diff --git a/internal/gatewayapi/resource/load.go b/internal/gatewayapi/resource/load.go index b3d1a523a3..23f64c96a6 100644 --- a/internal/gatewayapi/resource/load.go +++ b/internal/gatewayapi/resource/load.go @@ -46,9 +46,7 @@ func LoadResourcesFromYAMLBytes(yamlBytes []byte, addMissingResources bool) (*Re // loadKubernetesYAMLToResources converts a Kubernetes YAML string into GatewayAPI Resources. // TODO: add support for kind: -// - EnvoyExtensionPolicy (gateway.envoyproxy.io/v1alpha1) // - BackendLPPolicy (gateway.networking.k8s.io/v1alpha2) -// - BackendTLSPolicy (gateway.networking.k8s.io/v1alpha3) // - ReferenceGrant (gateway.networking.k8s.io/v1alpha2) func loadKubernetesYAMLToResources(input []byte, addMissingResources bool) (*Resources, error) { resources := NewResources() @@ -355,6 +353,19 @@ func loadKubernetesYAMLToResources(input []byte, addMissingResources bool) (*Res Spec: typedSpec.(gwapiv1a3.BackendTLSPolicySpec), } resources.BackendTLSPolicies = append(resources.BackendTLSPolicies, backendTLSPolicy) + case KindEnvoyExtensionPolicy: + typedSpec := spec.Interface() + envoyExtensionPolicy := &egv1a1.EnvoyExtensionPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: KindEnvoyExtensionPolicy, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: typedSpec.(egv1a1.EnvoyExtensionPolicySpec), + } + resources.EnvoyExtensionPolicies = append(resources.EnvoyExtensionPolicies, envoyExtensionPolicy) } return nil diff --git a/release-notes/current.yaml b/release-notes/current.yaml index b59001473b..6671818e39 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -10,7 +10,7 @@ security updates: | new features: | bug fixes: | - Added support for BackendTLSPolicy parsing in Standalone mode. + Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. # Enhancements that improve performance. performance improvements: | From 492af9cfef9081d74486b2207f9bad00dd6c9ee3 Mon Sep 17 00:00:00 2001 From: Mark Winter Date: Sat, 15 Mar 2025 02:13:18 +0000 Subject: [PATCH 04/10] fix: check for mirror backendRef in httproute index (#5497) * check for mirror backendRef Signed-off-by: mark winter (cherry picked from commit 72b72c48bbef314202903d8e1513657caf549c20) Signed-off-by: Guy Daich --- internal/provider/kubernetes/indexers.go | 16 +++++++ .../provider/kubernetes/predicates_test.go | 46 +++++++++++++++++++ release-notes/current.yaml | 1 + 3 files changed, 63 insertions(+) diff --git a/internal/provider/kubernetes/indexers.go b/internal/provider/kubernetes/indexers.go index 7afe712782..49196c66aa 100644 --- a/internal/provider/kubernetes/indexers.go +++ b/internal/provider/kubernetes/indexers.go @@ -120,6 +120,22 @@ func backendHTTPRouteIndexFunc(rawObj client.Object) []string { ) } } + + // Check for a RequestMirror filter to also include the backendRef from that filter + for _, filter := range rule.Filters { + if filter.Type != gwapiv1.HTTPRouteFilterRequestMirror { + continue + } + + mirrorBackendRef := filter.RequestMirror.BackendRef + + backendRefs = append(backendRefs, + types.NamespacedName{ + Namespace: gatewayapi.NamespaceDerefOr(mirrorBackendRef.Namespace, httproute.Namespace), + Name: string(mirrorBackendRef.Name), + }.String(), + ) + } } return backendRefs } diff --git a/internal/provider/kubernetes/predicates_test.go b/internal/provider/kubernetes/predicates_test.go index dc0d70cdce..4eb4e1203d 100644 --- a/internal/provider/kubernetes/predicates_test.go +++ b/internal/provider/kubernetes/predicates_test.go @@ -449,6 +449,52 @@ func TestValidateEndpointSliceForReconcile(t *testing.T) { endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "service"), expect: true, }, + { + name: "mirrored backend route exists", + configs: []client.Object{ + test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil), + sampleGateway, + &gwapiv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "httproute-test", + }, + Spec: gwapiv1.HTTPRouteSpec{ + CommonRouteSpec: gwapiv1.CommonRouteSpec{ + ParentRefs: []gwapiv1.ParentReference{ + {Name: gwapiv1.ObjectName("scheduled-status-test")}, + }, + }, + Rules: []gwapiv1.HTTPRouteRule{ + { + BackendRefs: []gwapiv1.HTTPBackendRef{ + { + BackendRef: gwapiv1.BackendRef{ + BackendObjectReference: gwapiv1.BackendObjectReference{ + Name: gwapiv1.ObjectName("service"), + Port: ptr.To(gwapiv1.PortNumber(80)), + }, + }, + }, + }, + Filters: []gwapiv1.HTTPRouteFilter{ + { + Type: gwapiv1.HTTPRouteFilterRequestMirror, + RequestMirror: &gwapiv1.HTTPRequestMirrorFilter{ + BackendRef: gwapiv1.BackendObjectReference{ + Name: "mirror-service", + Port: ptr.To(gwapiv1.PortNumber(80)), + }, + }, + }, + }, + }, + }, + }, + }, + }, + endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "mirror-service", false), + expect: true, + }, } // Create the reconciler. diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 6671818e39..e78a0ba440 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -11,6 +11,7 @@ new features: | bug fixes: | Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. + Fixed updates of endpoints when mirrored backend Pod IPs are changed. # Enhancements that improve performance. performance improvements: | From 888e0fd804e071677ee8f78abca7c1d78f6b09db Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Tue, 18 Mar 2025 06:07:42 -0700 Subject: [PATCH 05/10] fix: dont return an err when gatewayclass is not accepted (#5524) * bug: dont return an err when gatewayclass is not accepted this is a user generated error, we shouldnt log it as a system error, and return with an error Signed-off-by: Arko Dasgupta * release notes Signed-off-by: Arko Dasgupta --------- Signed-off-by: Arko Dasgupta (cherry picked from commit 51e87cabac6bd8dea727452b5877a7c266247387) Signed-off-by: Guy Daich --- internal/provider/kubernetes/controller.go | 3 +-- release-notes/current.yaml | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index bc7b5ba8c5..d655e8f088 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -215,8 +215,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques string(gwapiv1.GatewayClassReasonInvalidParameters), msg) r.resources.GatewayClassStatuses.Store(utils.NamespacedName(gc), &gc.Status) - r.log.Error(err, "failed to process parametersRef for gatewayclass", "name", managedGC.Name) - return reconcile.Result{}, err + return reconcile.Result{}, nil } } diff --git a/release-notes/current.yaml b/release-notes/current.yaml index e78a0ba440..0efd2b9d1d 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -12,6 +12,7 @@ new features: | bug fixes: | Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. Fixed updates of endpoints when mirrored backend Pod IPs are changed. + Fix not logging an error and returning it in the K8s Reconcile method when a GatewayClass is not accepted. # Enhancements that improve performance. performance improvements: | From 33dfe8d25d1778e748e5beb281f234b8ec319808 Mon Sep 17 00:00:00 2001 From: "Huabing (Robin) Zhao" Date: Fri, 21 Mar 2025 12:14:46 +0800 Subject: [PATCH 06/10] fix: host header should not be allowed to modify (#5533) * host header is not allowed to be modified Signed-off-by: Huabing (Robin) Zhao * address comment Signed-off-by: Huabing (Robin) Zhao --------- Signed-off-by: Huabing (Robin) Zhao (cherry picked from commit 54efa343998cfcc0b8305721c94f04c9dc29610c) Signed-off-by: Guy Daich --- internal/gatewayapi/filters.go | 436 ++++++------------ ...with-header-filter-invalid-headers.in.yaml | 58 ++- ...ith-header-filter-invalid-headers.out.yaml | 107 ++++- ...onse-header-filter-invalid-headers.in.yaml | 58 ++- ...nse-header-filter-invalid-headers.out.yaml | 105 ++++- release-notes/current.yaml | 1 + 6 files changed, 449 insertions(+), 316 deletions(-) diff --git a/internal/gatewayapi/filters.go b/internal/gatewayapi/filters.go index 06e1ce3cd1..655a133b73 100644 --- a/internal/gatewayapi/filters.go +++ b/internal/gatewayapi/filters.go @@ -192,15 +192,9 @@ func (t *Translator) processURLRewriteFilter( if filterContext.URLRewrite != nil { if hasMultipleCoreRewrites(rewrite, filterContext.URLRewrite) || hasConflictingCoreAndExtensionRewrites(rewrite, filterContext.URLRewrite) { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule", - ) + updateRouteStatusForFilter( + filterContext, + "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule") return } } @@ -213,15 +207,7 @@ func (t *Translator) processURLRewriteFilter( if rewrite.Hostname != nil { if err := t.validateHostname(string(*rewrite.Hostname)); err != nil { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - err.Error(), - ) + updateRouteStatusForFilter(filterContext, err.Error()) return } redirectHost := string(*rewrite.Hostname) @@ -234,29 +220,15 @@ func (t *Translator) processURLRewriteFilter( switch rewrite.Path.Type { case gwapiv1.FullPathHTTPPathModifier: if rewrite.Path.ReplacePrefixMatch != nil { - errMsg := "ReplacePrefixMatch cannot be set when rewrite path type is \"ReplaceFullPath\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplacePrefixMatch cannot be set when rewrite path type is \"ReplaceFullPath\"") return } if rewrite.Path.ReplaceFullPath == nil { - errMsg := "ReplaceFullPath must be set when rewrite path type is \"ReplaceFullPath\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceFullPath must be set when rewrite path type is \"ReplaceFullPath\"") return } if rewrite.Path.ReplaceFullPath != nil { @@ -268,29 +240,15 @@ func (t *Translator) processURLRewriteFilter( } case gwapiv1.PrefixMatchHTTPPathModifier: if rewrite.Path.ReplaceFullPath != nil { - errMsg := "ReplaceFullPath cannot be set when rewrite path type is \"ReplacePrefixMatch\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceFullPath cannot be set when rewrite path type is \"ReplacePrefixMatch\"") return } if rewrite.Path.ReplacePrefixMatch == nil { - errMsg := "ReplacePrefixMatch must be set when rewrite path type is \"ReplacePrefixMatch\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplacePrefixMatch must be set when rewrite path type is \"ReplacePrefixMatch\"") return } if rewrite.Path.ReplacePrefixMatch != nil { @@ -301,16 +259,11 @@ func (t *Translator) processURLRewriteFilter( } } default: - errMsg := fmt.Sprintf("Rewrite path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", rewrite.Path.Type) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Rewrite path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", + rewrite.Path.Type)) return } } @@ -324,15 +277,9 @@ func (t *Translator) processRedirectFilter( ) { // Can't have two redirects for the same route if filterContext.RedirectResponse != nil { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "Cannot configure multiple requestRedirect filters for a single HTTPRouteRule", - ) + updateRouteStatusForFilter( + filterContext, + "Cannot configure multiple requestRedirect filters for a single HTTPRouteRule") return } @@ -347,31 +294,16 @@ func (t *Translator) processRedirectFilter( if *redirect.Scheme == "http" || *redirect.Scheme == "https" { redir.Scheme = redirect.Scheme } else { - errMsg := fmt.Sprintf("Scheme: %s is unsupported, only 'https' and 'http' are supported", *redirect.Scheme) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf("Scheme: %s is unsupported, only 'https' and 'http' are supported", *redirect.Scheme)) return } } if redirect.Hostname != nil { if err := t.validateHostname(string(*redirect.Hostname)); err != nil { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - err.Error(), - ) + updateRouteStatusForFilter(filterContext, err.Error()) } else { redirectHost := string(*redirect.Hostname) redir.Hostname = &redirectHost @@ -393,16 +325,11 @@ func (t *Translator) processRedirectFilter( } } default: - errMsg := fmt.Sprintf("Redirect path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", redirect.Path.Type) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Redirect path type: %s is invalid, only \"ReplaceFullPath\" and \"ReplacePrefixMatch\" are supported", + redirect.Path.Type)) return } } @@ -414,15 +341,7 @@ func (t *Translator) processRedirectFilter( redir.StatusCode = &redirectCode } else { errMsg := fmt.Sprintf("Status code %d is invalid, only 302 and 301 are supported", redirectCode) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter(filterContext, errMsg) return } } @@ -453,28 +372,19 @@ func (t *Translator) processRequestHeaderModifierFilter( for _, addHeader := range headersToAdd { emptyFilterConfig = false if addHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "RequestHeaderModifier Filter cannot add a header with an empty name", - ) + updateRouteStatusForFilter( + filterContext, + "RequestHeaderModifier Filter cannot add a header with an empty name") // try to process the rest of the headers and produce a valid config. continue } - // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names - if strings.Contains(string(addHeader.Name), "/") || strings.Contains(string(addHeader.Name), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: %q", string(addHeader.Name)), + if !isModifiableHeader(string(addHeader.Name)) { + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Header: %q. The RequestHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them. To modify the Host header use the URLRewrite or the HTTPRouteFilter filter.", + string(addHeader.Name)), ) continue } @@ -510,27 +420,19 @@ func (t *Translator) processRequestHeaderModifierFilter( for _, setHeader := range headersToSet { if setHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "RequestHeaderModifier Filter cannot set a header with an empty name", - ) + updateRouteStatusForFilter( + filterContext, + "RequestHeaderModifier Filter cannot set a header with an empty name") continue } - // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names - if strings.Contains(string(setHeader.Name), "/") || strings.Contains(string(setHeader.Name), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("RequestHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: '%s'", string(setHeader.Name)), + + if !isModifiableHeader(string(setHeader.Name)) { + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Header: %q. The RequestHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them. To modify the Host header use the URLRewrite or the HTTPRouteFilter filter.", + string(setHeader.Name)), ) continue } @@ -566,14 +468,19 @@ func (t *Translator) processRequestHeaderModifierFilter( } for _, removedHeader := range headersToRemove { if removedHeader == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "RequestHeaderModifier Filter cannot remove a header with an empty name", + updateRouteStatusForFilter( + filterContext, + "RequestHeaderModifier Filter cannot remove a header with an empty name") + continue + } + + if !isModifiableHeader(removedHeader) { + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Header: %q. The RequestHeaderModifier filter cannot remove the Host header or headers with a '/' "+ + "or ':' character in them.", + removedHeader), ) continue } @@ -595,18 +502,30 @@ func (t *Translator) processRequestHeaderModifierFilter( // Update the status if the filter failed to configure any valid headers to add/remove if len(filterContext.AddRequestHeaders) == 0 && len(filterContext.RemoveRequestHeaders) == 0 && !emptyFilterConfig { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "RequestHeaderModifier Filter did not provide valid configuration to add/set/remove any headers", - ) + updateRouteStatusForFilter( + filterContext, + "RequestHeaderModifier Filter did not provide valid configuration to add/set/remove any headers") } } +func updateRouteStatusForFilter(filterContext *HTTPFiltersContext, message string) { + routeStatus := GetRouteStatus(filterContext.Route) + status.SetRouteStatusCondition(routeStatus, + filterContext.ParentRef.routeParentStatusIdx, + filterContext.Route.GetGeneration(), + gwapiv1.RouteConditionAccepted, + metav1.ConditionFalse, + gwapiv1.RouteReasonUnsupportedValue, + message, + ) +} + +func isModifiableHeader(headerName string) bool { + // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names + // And Envoy does not allow modification the pseudo headers and the host header + return !strings.Contains(headerName, "/") && !strings.Contains(headerName, ":") && !strings.EqualFold(headerName, "host") +} + func (t *Translator) processResponseHeaderModifierFilter( headerModifier *gwapiv1.HTTPHeaderFilter, filterContext *HTTPFiltersContext, @@ -625,29 +544,19 @@ func (t *Translator) processResponseHeaderModifierFilter( for _, addHeader := range headersToAdd { emptyFilterConfig = false if addHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "ResponseHeaderModifier Filter cannot add a header with an empty name", - ) + updateRouteStatusForFilter( + filterContext, + "ResponseHeaderModifier Filter cannot add a header with an empty name") // try to process the rest of the headers and produce a valid config. continue } - // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names - if strings.Contains(string(addHeader.Name), "/") || strings.Contains(string(addHeader.Name), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: %q", string(addHeader.Name)), - ) + if !isModifiableHeader(string(addHeader.Name)) { + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Header: %q. The ResponseHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them.", + string(addHeader.Name))) continue } // Check if the header is a duplicate @@ -682,28 +591,19 @@ func (t *Translator) processResponseHeaderModifierFilter( for _, setHeader := range headersToSet { if setHeader.Name == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "ResponseHeaderModifier Filter cannot set a header with an empty name", - ) + updateRouteStatusForFilter( + filterContext, + "ResponseHeaderModifier Filter cannot set a header with an empty name") continue } - // Per Gateway API specification on HTTPHeaderName, : and / are invalid characters in header names - if strings.Contains(string(setHeader.Name), "/") || strings.Contains(string(setHeader.Name), ":") { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - fmt.Sprintf("ResponseHeaderModifier Filter cannot set headers with a '/' or ':' character in them. Header: '%s'", string(setHeader.Name)), - ) + + if !isModifiableHeader(string(setHeader.Name)) { + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Header: %q. The ResponseHeaderModifier filter cannot set the Host header or headers with a '/' "+ + "or ':' character in them.", + string(setHeader.Name))) continue } @@ -738,15 +638,18 @@ func (t *Translator) processResponseHeaderModifierFilter( } for _, removedHeader := range headersToRemove { if removedHeader == "" { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "ResponseHeaderModifier Filter cannot remove a header with an empty name", - ) + updateRouteStatusForFilter( + filterContext, + "ResponseHeaderModifier Filter cannot remove a header with an empty name") + continue + } + if !isModifiableHeader(removedHeader) { + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf( + "Header: %q. The ResponseHeaderModifier filter cannot remove the Host header or headers with a '/' "+ + "or ':' character in them.", + removedHeader)) continue } @@ -768,15 +671,9 @@ func (t *Translator) processResponseHeaderModifierFilter( // Update the status if the filter failed to configure any valid headers to add/remove if len(filterContext.AddResponseHeaders) == 0 && len(filterContext.RemoveResponseHeaders) == 0 && !emptyFilterConfig { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "ResponseHeaderModifier Filter did not provide valid configuration to add/set/remove any headers", - ) + updateRouteStatusForFilter( + filterContext, + "ResponseHeaderModifier Filter did not provide valid configuration to add/set/remove any headers") } } @@ -798,15 +695,9 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec if filterContext.URLRewrite != nil { if hasMultipleExtensionRewrites(hrf.Spec.URLRewrite, filterContext.URLRewrite) || hasConflictingExtensionAndCoreRewrites(hrf.Spec.URLRewrite, filterContext.URLRewrite) { - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule", - ) + updateRouteStatusForFilter( + filterContext, + "Cannot configure multiple urlRewrite filters for a single HTTPRouteRule") return } } @@ -815,30 +706,16 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec if hrf.Spec.URLRewrite.Path.Type == egv1a1.RegexHTTPPathModifier { if hrf.Spec.URLRewrite.Path.ReplaceRegexMatch == nil || hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern == "" { - errMsg := "ReplaceRegexMatch Pattern must be set when rewrite path type is \"ReplaceRegexMatch\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceRegexMatch Pattern must be set when rewrite path type is \"ReplaceRegexMatch\"") return } else if _, err := regexp.Compile(hrf.Spec.URLRewrite.Path.ReplaceRegexMatch.Pattern); err != nil { // Avoid envoy NACKs due to invalid regex. // Golang's regexp is almost identical to RE2: https://pkg.go.dev/regexp/syntax - errMsg := "ReplaceRegexMatch must be a valid RE2 regular expression" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "ReplaceRegexMatch must be a valid RE2 regular expression") return } @@ -867,16 +744,9 @@ func (t *Translator) processExtensionRefHTTPFilter(extFilter *gwapiv1.LocalObjec var hm *ir.HTTPHostModifier if hrf.Spec.URLRewrite.Hostname.Type == egv1a1.HeaderHTTPHostnameModifier { if hrf.Spec.URLRewrite.Hostname.Header == nil { - errMsg := "Header must be set when rewrite path type is \"Header\"" - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + "Header must be set when rewrite path type is \"Header\"") return } hm = &ir.HTTPHostModifier{ @@ -1021,14 +891,7 @@ func (t *Translator) processUnresolvedHTTPFilter(errMsg string, filterContext *H gwapiv1.RouteReasonBackendNotFound, errMsg, ) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter(filterContext, errMsg) filterContext.DirectResponse = &ir.CustomResponse{ StatusCode: ptr.To(uint32(500)), } @@ -1036,31 +899,16 @@ func (t *Translator) processUnresolvedHTTPFilter(errMsg string, filterContext *H func (t *Translator) processUnsupportedHTTPFilter(filterType string, filterContext *HTTPFiltersContext) { errMsg := fmt.Sprintf("Unsupported filter type: %s", filterType) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter(filterContext, errMsg) filterContext.DirectResponse = &ir.CustomResponse{ StatusCode: ptr.To(uint32(500)), } } func (t *Translator) processInvalidHTTPFilter(filterType string, filterContext *HTTPFiltersContext, err error) { - errMsg := fmt.Sprintf("Invalid filter %s: %v", filterType, err) - routeStatus := GetRouteStatus(filterContext.Route) - status.SetRouteStatusCondition(routeStatus, - filterContext.ParentRef.routeParentStatusIdx, - filterContext.Route.GetGeneration(), - gwapiv1.RouteConditionAccepted, - metav1.ConditionFalse, - gwapiv1.RouteReasonUnsupportedValue, - errMsg, - ) + updateRouteStatusForFilter( + filterContext, + fmt.Sprintf("Invalid filter %s: %v", filterType, err)) filterContext.DirectResponse = &ir.CustomResponse{ StatusCode: ptr.To(uint32(500)), } diff --git a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml index aaf5bad87f..869f9122c2 100644 --- a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml +++ b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.in.yaml @@ -30,7 +30,7 @@ httpRoutes: rules: - matches: - path: - value: "/" + value: "/foo" backendRefs: - name: service-1 port: 8080 @@ -42,7 +42,57 @@ httpRoutes: value: "some-value" - name: "good-header" value: "some-value" - add: - - name: "example/2" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-2 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/bar" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: RequestHeaderModifier + requestHeaderModifier: + remove: + - "example/2" + set: + - name: "good-header" + value: "some-value" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-3 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/baz" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: RequestHeaderModifier + requestHeaderModifier: + set: + - name: "host" + value: "example.com" + - name: "good-header" value: "some-value" - diff --git a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml index 339e6a1f41..b1428fee39 100644 --- a/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-header-filter-invalid-headers.out.yaml @@ -17,7 +17,7 @@ gateways: protocol: HTTP status: listeners: - - attachedRoutes: 1 + - attachedRoutes: 3 conditions: - lastTransitionTime: null message: Sending translated listener configuration to the data plane @@ -60,9 +60,6 @@ httpRoutes: port: 8080 filters: - requestHeaderModifier: - add: - - name: example/2 - value: some-value set: - name: example:1 value: some-value @@ -71,13 +68,109 @@ httpRoutes: type: RequestHeaderModifier matches: - path: - value: / + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'Header: "example:1". The RequestHeaderModifier filter cannot set + the Host header or headers with a ''/'' or '':'' character in them. To modify + the Host header use the URLRewrite or the HTTPRouteFilter filter.' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - requestHeaderModifier: + remove: + - example/2 + set: + - name: good-header + value: some-value + type: RequestHeaderModifier + matches: + - path: + value: /bar + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'Header: "example/2". The RequestHeaderModifier filter cannot remove + the Host header or headers with a ''/'' or '':'' character in them.' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-3 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - requestHeaderModifier: + set: + - name: host + value: example.com + - name: good-header + value: some-value + type: RequestHeaderModifier + matches: + - path: + value: /baz status: parents: - conditions: - lastTransitionTime: null - message: 'RequestHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them. Header: ''example:1''' + message: 'Header: "host". The RequestHeaderModifier filter cannot set the + Host header or headers with a ''/'' or '':'' character in them. To modify + the Host header use the URLRewrite or the HTTPRouteFilter filter.' reason: UnsupportedValue status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml index 6e1e57425b..95da714409 100644 --- a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml +++ b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.in.yaml @@ -30,7 +30,7 @@ httpRoutes: rules: - matches: - path: - value: "/" + value: "/foo" backendRefs: - name: service-1 port: 8080 @@ -42,7 +42,57 @@ httpRoutes: value: "some-value" - name: "good-header" value: "some-value" - add: - - name: "example/2" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-2 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/bar" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + remove: + - "example/2" + set: + - name: "good-header" + value: "some-value" +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-3 + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - path: + value: "/baz" + backendRefs: + - name: service-1 + port: 8080 + filters: + - type: ResponseHeaderModifier + responseHeaderModifier: + set: + - name: "host" + value: "example.com" + - name: "good-header" value: "some-value" - diff --git a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml index 077e6e866e..3fee996d45 100644 --- a/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-response-header-filter-invalid-headers.out.yaml @@ -17,7 +17,7 @@ gateways: protocol: HTTP status: listeners: - - attachedRoutes: 1 + - attachedRoutes: 3 conditions: - lastTransitionTime: null message: Sending translated listener configuration to the data plane @@ -60,9 +60,6 @@ httpRoutes: port: 8080 filters: - responseHeaderModifier: - add: - - name: example/2 - value: some-value set: - name: example:1 value: some-value @@ -71,13 +68,107 @@ httpRoutes: type: ResponseHeaderModifier matches: - path: - value: / + value: /foo + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'Header: "example:1". The ResponseHeaderModifier filter cannot set + the Host header or headers with a ''/'' or '':'' character in them.' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - responseHeaderModifier: + remove: + - example/2 + set: + - name: good-header + value: some-value + type: ResponseHeaderModifier + matches: + - path: + value: /bar + status: + parents: + - conditions: + - lastTransitionTime: null + message: 'Header: "example/2". The ResponseHeaderModifier filter cannot remove + the Host header or headers with a ''/'' or '':'' character in them.' + reason: UnsupportedValue + status: "False" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-3 + namespace: default + spec: + hostnames: + - gateway.envoyproxy.io + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + filters: + - responseHeaderModifier: + set: + - name: host + value: example.com + - name: good-header + value: some-value + type: ResponseHeaderModifier + matches: + - path: + value: /baz status: parents: - conditions: - lastTransitionTime: null - message: 'ResponseHeaderModifier Filter cannot set headers with a ''/'' or - '':'' character in them. Header: ''example:1''' + message: 'Header: "host". The ResponseHeaderModifier filter cannot set the + Host header or headers with a ''/'' or '':'' character in them.' reason: UnsupportedValue status: "False" type: Accepted diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 0efd2b9d1d..42960fab94 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -13,6 +13,7 @@ bug fixes: | Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. Fixed updates of endpoints when mirrored backend Pod IPs are changed. Fix not logging an error and returning it in the K8s Reconcile method when a GatewayClass is not accepted. + Fixed validation of host header in RequestHeaderModifier filter. # Enhancements that improve performance. performance improvements: | From c787d96f3bbc56e62aae3205f8b79b4ebea5579c Mon Sep 17 00:00:00 2001 From: Teju Nareddy Date: Wed, 12 Mar 2025 21:25:45 -0500 Subject: [PATCH 07/10] fix: retrigger reconciliation when backendRef of type ServiceImport is updated (#5461) * fix: retrigger reconilation when backendRef of type ServiceImport is updated Signed-off-by: Teju Nareddy (cherry picked from commit e2f8978de56e3ede49eab21f714802f3a5a75af9) Signed-off-by: Guy Daich --- internal/provider/kubernetes/indexers.go | 2 +- .../provider/kubernetes/kubernetes_test.go | 6 +- .../provider/kubernetes/predicates_test.go | 76 ++++++++++++------- internal/provider/kubernetes/test/utils.go | 37 +++++++-- release-notes/current.yaml | 1 + 5 files changed, 86 insertions(+), 36 deletions(-) diff --git a/internal/provider/kubernetes/indexers.go b/internal/provider/kubernetes/indexers.go index 49196c66aa..9866f6d73a 100644 --- a/internal/provider/kubernetes/indexers.go +++ b/internal/provider/kubernetes/indexers.go @@ -111,7 +111,7 @@ func backendHTTPRouteIndexFunc(rawObj client.Object) []string { var backendRefs []string for _, rule := range httproute.Spec.Rules { for _, backend := range rule.BackendRefs { - if backend.Kind == nil || string(*backend.Kind) == resource.KindService || string(*backend.Kind) == egv1a1.KindBackend { + if backend.Kind == nil || string(*backend.Kind) == resource.KindService || string(*backend.Kind) == resource.KindServiceImport || string(*backend.Kind) == egv1a1.KindBackend { backendRefs = append(backendRefs, types.NamespacedName{ Namespace: gatewayapi.NamespaceDerefOr(backend.Namespace, httproute.Namespace), diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index 541237c2f6..4665f510e6 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -1429,20 +1429,22 @@ func TestNamespaceSelectorProvider(t *testing.T) { require.NoError(t, cli.Delete(ctx, nonWatchedSvc)) }() + watchedServiceBackendRef := test.GetServiceBackendRef(types.NamespacedName{Name: watchedSvc.Name}, 80) watchedHTTPRoute := test.GetHTTPRoute(types.NamespacedName{ Namespace: watchedNS.Name, Name: "watched-http-route", - }, watchedGateway.Name, types.NamespacedName{Name: watchedSvc.Name}, 80, "") + }, watchedGateway.Name, watchedServiceBackendRef, "") require.NoError(t, cli.Create(ctx, watchedHTTPRoute)) defer func() { require.NoError(t, cli.Delete(ctx, watchedHTTPRoute)) }() + nonWatchedServiceBackendRef := test.GetServiceBackendRef(types.NamespacedName{Name: nonWatchedSvc.Name}, 8001) nonWatchedHTTPRoute := test.GetHTTPRoute(types.NamespacedName{ Namespace: nonWatchedNS.Name, Name: "non-watched-http-route", - }, nonWatchedGateway.Name, types.NamespacedName{Name: nonWatchedSvc.Name}, 8001, "") + }, nonWatchedGateway.Name, nonWatchedServiceBackendRef, "") require.NoError(t, cli.Create(ctx, nonWatchedHTTPRoute)) defer func() { require.NoError(t, cli.Delete(ctx, nonWatchedHTTPRoute)) diff --git a/internal/provider/kubernetes/predicates_test.go b/internal/provider/kubernetes/predicates_test.go index 4eb4e1203d..5c9dbba1c0 100644 --- a/internal/provider/kubernetes/predicates_test.go +++ b/internal/provider/kubernetes/predicates_test.go @@ -123,11 +123,12 @@ func TestGatewayClassHasMatchingNamespaceLabels(t *testing.T) { Build(), } t.Run(tc.name, func(t *testing.T) { + sampleServiceBackendRef := test.GetServiceBackendRef(types.NamespacedName{Name: "service"}, 80) res := r.hasMatchingNamespaceLabels( test.GetHTTPRoute(types.NamespacedName{ Namespace: ns, Name: "httproute-test", - }, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, "")) + }, "scheduled-status-test", sampleServiceBackendRef, "")) require.Equal(t, tc.expect, res) }) } @@ -412,7 +413,10 @@ func TestValidateSecretForReconcile(t *testing.T) { // TestValidateEndpointSliceForReconcile tests the validateEndpointSliceForReconcile // predicate function. func TestValidateEndpointSliceForReconcile(t *testing.T) { + sampleGatewayClass := test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil) sampleGateway := test.GetGateway(types.NamespacedName{Namespace: "default", Name: "scheduled-status-test"}, "test-gc", 8080) + sampleServiceBackendRef := test.GetServiceBackendRef(types.NamespacedName{Name: "service"}, 80) + sampleServiceImportBackendRef := test.GetServiceImportBackendRef(types.NamespacedName{Name: "imported-service"}, 80) testCases := []struct { name string @@ -423,30 +427,40 @@ func TestValidateEndpointSliceForReconcile(t *testing.T) { { name: "route service but no routes exist", configs: []client.Object{ - test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil), + sampleGatewayClass, sampleGateway, }, - endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "service"), + endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "service", false), expect: false, }, { name: "http route service routes exist, but endpointslice is associated with another service", configs: []client.Object{ - test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil), + sampleGatewayClass, sampleGateway, - test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""), + test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", sampleServiceBackendRef, ""), }, - endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "other-service"), + endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "other-service", false), expect: false, }, { name: "http route service routes exist", configs: []client.Object{ - test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil), + sampleGatewayClass, sampleGateway, - test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""), + test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", sampleServiceBackendRef, ""), }, - endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "service"), + endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "service", false), + expect: true, + }, + { + name: "http route serviceimport routes exist", + configs: []client.Object{ + sampleGatewayClass, + sampleGateway, + test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", sampleServiceImportBackendRef, ""), + }, + endpointSlice: test.GetEndpointSlice(types.NamespacedName{Name: "endpointslice"}, "imported-service", true), expect: true, }, { @@ -597,6 +611,7 @@ func TestValidateServiceForReconcile(t *testing.T) { }, }, } + sampleServiceBackendRef := test.GetServiceBackendRef(types.NamespacedName{Name: "service"}, 80) testCases := []struct { name string @@ -660,7 +675,7 @@ func TestValidateServiceForReconcile(t *testing.T) { configs: []client.Object{ test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil), sampleGateway, - test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""), + test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", sampleServiceBackendRef, ""), }, service: test.GetService(types.NamespacedName{Name: "service"}, nil, nil), expect: true, @@ -672,7 +687,7 @@ func TestValidateServiceForReconcile(t *testing.T) { name: "route service routes exist but with non-existing gateway reference", configs: []client.Object{ test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil), - test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, ""), + test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", sampleServiceBackendRef, ""), }, service: test.GetService(types.NamespacedName{Name: "service"}, nil, nil), expect: true, @@ -1082,13 +1097,17 @@ func TestCheckObjectNamespaceLabels(t *testing.T) { }{ { name: "matching labels of namespace of the object is a subset of namespaceLabels", - object: test.GetHTTPRoute(types.NamespacedName{ - Name: "foo-route", - Namespace: "foo", - }, "eg", types.NamespacedName{ - Name: "foo-svc", - Namespace: "foo", - }, 8080, ""), + object: test.GetHTTPRoute( + types.NamespacedName{ + Name: "foo-route", + Namespace: "foo", + }, + "eg", + test.GetServiceBackendRef(types.NamespacedName{ + Name: "foo-svc", + Namespace: "foo", + }, 8080), + ""), ns: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -1102,13 +1121,17 @@ func TestCheckObjectNamespaceLabels(t *testing.T) { }, { name: "non-matching labels of namespace of the object is a subset of namespaceLabels", - object: test.GetHTTPRoute(types.NamespacedName{ - Name: "bar-route", - Namespace: "bar", - }, "eg", types.NamespacedName{ - Name: "bar-svc", - Namespace: "bar", - }, 8080, ""), + object: test.GetHTTPRoute( + types.NamespacedName{ + Name: "bar-route", + Namespace: "bar", + }, + "eg", + test.GetServiceBackendRef(types.NamespacedName{ + Name: "bar-svc", + Namespace: "bar", + }, 8080), + ""), ns: &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "bar", @@ -1262,6 +1285,7 @@ func TestValidateHTTPRouteFilerForReconcile(t *testing.T) { sampleGWC := test.GetGatewayClass("test-gc", egv1a1.GatewayControllerName, nil) sampleGateway := test.GetGateway(types.NamespacedName{Namespace: "default", Name: "scheduled-status-test"}, "test-gc", 8080) sampleService := test.GetService(types.NamespacedName{Name: "service"}, nil, nil) + sampleServiceBackendRef := test.GetServiceBackendRef(types.NamespacedName{Name: "service"}, 80) sampleHTTPRouteFilter := test.GetHTTPRouteFilter(types.NamespacedName{Name: "httproutefilter"}) testCases := []struct { @@ -1288,7 +1312,7 @@ func TestValidateHTTPRouteFilerForReconcile(t *testing.T) { sampleGateway, sampleService, sampleHTTPRouteFilter, - test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", types.NamespacedName{Name: "service"}, 80, "httproutefilter"), + test.GetHTTPRoute(types.NamespacedName{Name: "httproute-test"}, "scheduled-status-test", sampleServiceBackendRef, "httproutefilter"), }, httpRouteFilter: sampleHTTPRouteFilter, expect: true, diff --git a/internal/provider/kubernetes/test/utils.go b/internal/provider/kubernetes/test/utils.go index 77bc50c5e6..a6b2ef3256 100644 --- a/internal/provider/kubernetes/test/utils.go +++ b/internal/provider/kubernetes/test/utils.go @@ -15,8 +15,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + mcsapiv1a1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/gatewayapi" + "github.com/envoyproxy/gateway/internal/gatewayapi/resource" ) type GroupKindNamespacedName struct { @@ -107,8 +110,24 @@ func GetSecret(nsName types.NamespacedName) *corev1.Secret { } } +func GetServiceBackendRef(name types.NamespacedName, port int32) gwapiv1.BackendObjectReference { + return gwapiv1.BackendObjectReference{ + Name: gwapiv1.ObjectName(name.Name), + Port: ptr.To(gwapiv1.PortNumber(port)), + } +} + +func GetServiceImportBackendRef(name types.NamespacedName, port int32) gwapiv1.BackendObjectReference { + return gwapiv1.BackendObjectReference{ + Name: gwapiv1.ObjectName(name.Name), + Port: ptr.To(gwapiv1.PortNumber(port)), + Kind: gatewayapi.KindPtr(resource.KindServiceImport), + Group: gatewayapi.GroupPtr(mcsapiv1a1.GroupName), + } +} + // GetHTTPRoute returns a sample HTTPRoute with a parent reference. -func GetHTTPRoute(nsName types.NamespacedName, parent string, serviceName types.NamespacedName, port int32, httpRouteFilterName string) *gwapiv1.HTTPRoute { +func GetHTTPRoute(nsName types.NamespacedName, parent string, backendRef gwapiv1.BackendObjectReference, httpRouteFilterName string) *gwapiv1.HTTPRoute { httpRoute := &gwapiv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: nsName.Namespace, @@ -125,10 +144,7 @@ func GetHTTPRoute(nsName types.NamespacedName, parent string, serviceName types. BackendRefs: []gwapiv1.HTTPBackendRef{ { BackendRef: gwapiv1.BackendRef{ - BackendObjectReference: gwapiv1.BackendObjectReference{ - Name: gwapiv1.ObjectName(serviceName.Name), - Port: ptr.To(gwapiv1.PortNumber(port)), - }, + BackendObjectReference: backendRef, }, }, }, @@ -349,12 +365,19 @@ func GetService(nsName types.NamespacedName, labels map[string]string, ports map } // GetEndpointSlice returns a sample EndpointSlice. -func GetEndpointSlice(nsName types.NamespacedName, svcName string) *discoveryv1.EndpointSlice { +func GetEndpointSlice(nsName types.NamespacedName, svcName string, isServiceImport bool) *discoveryv1.EndpointSlice { + var labels map[string]string + if isServiceImport { + labels = map[string]string{mcsapiv1a1.LabelServiceName: svcName} + } else { + labels = map[string]string{discoveryv1.LabelServiceName: svcName} + } + return &discoveryv1.EndpointSlice{ ObjectMeta: metav1.ObjectMeta{ Name: nsName.Name, Namespace: nsName.Namespace, - Labels: map[string]string{discoveryv1.LabelServiceName: svcName}, + Labels: labels, }, Endpoints: []discoveryv1.Endpoint{ { diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 42960fab94..f8824d2393 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -14,6 +14,7 @@ bug fixes: | Fixed updates of endpoints when mirrored backend Pod IPs are changed. Fix not logging an error and returning it in the K8s Reconcile method when a GatewayClass is not accepted. Fixed validation of host header in RequestHeaderModifier filter. + Retrigger reconciliation when backendRef of type ServiceImport is updated or when EndpointSlice(s) for a ServiceImport are updated. # Enhancements that improve performance. performance improvements: | From ad65bc3182e173d42e12c2f02c5126ab07918746 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Fri, 21 Mar 2025 17:21:10 -0500 Subject: [PATCH 08/10] pin envoy and ratelimit Signed-off-by: Guy Daich --- api/v1alpha1/shared_types.go | 4 ++-- charts/gateway-helm/README.md | 2 +- charts/gateway-helm/values.tmpl.yaml | 2 +- .../kubernetes/proxy/testdata/daemonsets/component-level.yaml | 2 +- .../kubernetes/proxy/testdata/daemonsets/default.yaml | 2 +- .../proxy/testdata/daemonsets/disable-prometheus.yaml | 2 +- .../testdata/daemonsets/override-labels-and-annotations.yaml | 2 +- .../kubernetes/proxy/testdata/daemonsets/patch-daemonset.yaml | 2 +- .../proxy/testdata/daemonsets/shutdown-manager.yaml | 2 +- .../proxy/testdata/daemonsets/with-annotations.yaml | 2 +- .../proxy/testdata/daemonsets/with-concurrency.yaml | 2 +- .../kubernetes/proxy/testdata/daemonsets/with-extra-args.yaml | 2 +- .../proxy/testdata/daemonsets/with-image-pull-secrets.yaml | 2 +- .../kubernetes/proxy/testdata/daemonsets/with-name.yaml | 2 +- .../proxy/testdata/daemonsets/with-node-selector.yaml | 2 +- .../testdata/daemonsets/with-topology-spread-constraints.yaml | 2 +- .../kubernetes/proxy/testdata/deployments/bootstrap.yaml | 2 +- .../proxy/testdata/deployments/component-level.yaml | 2 +- .../kubernetes/proxy/testdata/deployments/default.yaml | 2 +- .../proxy/testdata/deployments/disable-prometheus.yaml | 2 +- .../kubernetes/proxy/testdata/deployments/dual-stack.yaml | 2 +- .../kubernetes/proxy/testdata/deployments/ipv6.yaml | 2 +- .../testdata/deployments/override-labels-and-annotations.yaml | 2 +- .../proxy/testdata/deployments/patch-deployment.yaml | 2 +- .../proxy/testdata/deployments/shutdown-manager.yaml | 2 +- .../proxy/testdata/deployments/with-annotations.yaml | 2 +- .../proxy/testdata/deployments/with-concurrency.yaml | 2 +- .../proxy/testdata/deployments/with-empty-memory-limits.yaml | 2 +- .../proxy/testdata/deployments/with-extra-args.yaml | 2 +- .../proxy/testdata/deployments/with-image-pull-secrets.yaml | 2 +- .../kubernetes/proxy/testdata/deployments/with-name.yaml | 2 +- .../proxy/testdata/deployments/with-node-selector.yaml | 2 +- .../deployments/with-topology-spread-constraints.yaml | 2 +- .../kubernetes/ratelimit/testdata/deployments/default.yaml | 2 +- .../ratelimit/testdata/deployments/disable-prometheus.yaml | 2 +- .../ratelimit/testdata/deployments/enable-tracing-custom.yaml | 2 +- .../ratelimit/testdata/deployments/enable-tracing.yaml | 2 +- .../ratelimit/testdata/deployments/merge-annotations.yaml | 2 +- .../ratelimit/testdata/deployments/merge-labels.yaml | 2 +- .../ratelimit/testdata/deployments/patch-deployment.yaml | 2 +- .../ratelimit/testdata/deployments/with-node-selector.yaml | 2 +- .../deployments/with-topology-spread-constraints.yaml | 2 +- release-notes/current.yaml | 2 ++ site/content/en/latest/install/gateway-helm-api.md | 2 +- site/content/zh/latest/install/gateway-helm-api.md | 2 +- test/helm/gateway-helm/certjen-custom-scheduling.out.yaml | 2 +- test/helm/gateway-helm/control-plane-with-pdb.out.yaml | 2 +- test/helm/gateway-helm/default-config.out.yaml | 2 +- test/helm/gateway-helm/deployment-custom-topology.out.yaml | 2 +- test/helm/gateway-helm/deployment-images-config.out.yaml | 2 +- test/helm/gateway-helm/deployment-priorityclass.out.yaml | 2 +- test/helm/gateway-helm/deployment-securitycontext.out.yaml | 2 +- test/helm/gateway-helm/service-annotations.out.yaml | 2 +- 53 files changed, 55 insertions(+), 53 deletions(-) diff --git a/api/v1alpha1/shared_types.go b/api/v1alpha1/shared_types.go index 21774dd387..b1ee8047e6 100644 --- a/api/v1alpha1/shared_types.go +++ b/api/v1alpha1/shared_types.go @@ -22,7 +22,7 @@ const ( // DefaultDeploymentMemoryResourceRequests for deployment memory resource DefaultDeploymentMemoryResourceRequests = "512Mi" // DefaultEnvoyProxyImage is the default image used by envoyproxy - DefaultEnvoyProxyImage = "docker.io/envoyproxy/envoy:distroless-v1.33.0" + DefaultEnvoyProxyImage = "docker.io/envoyproxy/envoy:distroless-v1.33.1" // DefaultShutdownManagerCPUResourceRequests for shutdown manager cpu resource DefaultShutdownManagerCPUResourceRequests = "10m" // DefaultShutdownManagerMemoryResourceRequests for shutdown manager memory resource @@ -30,7 +30,7 @@ const ( // DefaultShutdownManagerImage is the default image used for the shutdown manager. DefaultShutdownManagerImage = "docker.io/envoyproxy/gateway-dev:latest" // DefaultRateLimitImage is the default image used by ratelimit. - DefaultRateLimitImage = "docker.io/envoyproxy/ratelimit:ae4cee11" + DefaultRateLimitImage = "docker.io/envoyproxy/ratelimit:0141a24f" // HTTPProtocol is the common-used http protocol. HTTPProtocol = "http" // GRPCProtocol is the common-used grpc protocol. diff --git a/charts/gateway-helm/README.md b/charts/gateway-helm/README.md index 909b1d764e..b9ff059da0 100644 --- a/charts/gateway-helm/README.md +++ b/charts/gateway-helm/README.md @@ -102,7 +102,7 @@ To uninstall the chart: | global.images.envoyGateway.image | string | `nil` | | | global.images.envoyGateway.pullPolicy | string | `nil` | | | global.images.envoyGateway.pullSecrets | list | `[]` | | -| global.images.ratelimit.image | string | `"docker.io/envoyproxy/ratelimit:ae4cee11"` | | +| global.images.ratelimit.image | string | `"docker.io/envoyproxy/ratelimit:0141a24f"` | | | global.images.ratelimit.pullPolicy | string | `"IfNotPresent"` | | | global.images.ratelimit.pullSecrets | list | `[]` | | | kubernetesClusterDomain | string | `"cluster.local"` | | diff --git a/charts/gateway-helm/values.tmpl.yaml b/charts/gateway-helm/values.tmpl.yaml index a07560fc17..90ffd44925 100644 --- a/charts/gateway-helm/values.tmpl.yaml +++ b/charts/gateway-helm/values.tmpl.yaml @@ -12,7 +12,7 @@ global: pullSecrets: [] ratelimit: # This is the full image name including the hub, repo, and tag. - image: "docker.io/envoyproxy/ratelimit:ae4cee11" + image: "docker.io/envoyproxy/ratelimit:0141a24f" # Specify image pull policy if default behavior isn't desired. # Default behavior: latest images will be Always else IfNotPresent. pullPolicy: IfNotPresent diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/component-level.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/component-level.yaml index ae1a04b363..5be1c2ca27 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/component-level.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/component-level.yaml @@ -56,7 +56,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default.yaml index 5b7856ee51..6ece066ac6 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default.yaml @@ -227,7 +227,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/disable-prometheus.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/disable-prometheus.yaml index a42575b167..ca8b77c676 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/disable-prometheus.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/disable-prometheus.yaml @@ -198,7 +198,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/override-labels-and-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/override-labels-and-annotations.yaml index f1cf7e3b2e..7838100945 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/override-labels-and-annotations.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/override-labels-and-annotations.yaml @@ -236,7 +236,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/patch-daemonset.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/patch-daemonset.yaml index 5fdb5e11a8..068a6579b4 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/patch-daemonset.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/patch-daemonset.yaml @@ -227,7 +227,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/shutdown-manager.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/shutdown-manager.yaml index b96fe35ed5..aef27b2bb0 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/shutdown-manager.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/shutdown-manager.yaml @@ -227,7 +227,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-annotations.yaml index b5396e7296..b77221c265 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-annotations.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-annotations.yaml @@ -232,7 +232,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-concurrency.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-concurrency.yaml index 40dc270d4e..4f8f311d1a 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-concurrency.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-concurrency.yaml @@ -56,7 +56,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-extra-args.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-extra-args.yaml index 0c35ddf971..d9c734378d 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-extra-args.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-extra-args.yaml @@ -229,7 +229,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-image-pull-secrets.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-image-pull-secrets.yaml index 96f5adf008..c9769bc149 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-image-pull-secrets.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-image-pull-secrets.yaml @@ -227,7 +227,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-name.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-name.yaml index f004ff20a7..170705190c 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-name.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-name.yaml @@ -227,7 +227,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-node-selector.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-node-selector.yaml index 38747e0bcd..05e327e583 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-node-selector.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-node-selector.yaml @@ -227,7 +227,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-topology-spread-constraints.yaml b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-topology-spread-constraints.yaml index d5c9360142..d8e1ec5c00 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-topology-spread-constraints.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/daemonsets/with-topology-spread-constraints.yaml @@ -227,7 +227,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml index 13f4c8669c..08b968fcef 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/bootstrap.yaml @@ -59,7 +59,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml index 13008ce635..553420ca20 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/component-level.yaml @@ -60,7 +60,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml index 6d7bf9bade..6cff308366 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/default.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml index d847191228..452b3a95b8 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/disable-prometheus.yaml @@ -202,7 +202,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/dual-stack.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/dual-stack.yaml index 5802fc619c..999be154df 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/dual-stack.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/dual-stack.yaml @@ -232,7 +232,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/ipv6.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/ipv6.yaml index 15ec06ab50..128272808a 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/ipv6.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/ipv6.yaml @@ -232,7 +232,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml index c8b74296d7..4bc8d3f68e 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/override-labels-and-annotations.yaml @@ -240,7 +240,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml index bbf82b8fc9..03cc3d2843 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/patch-deployment.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml index 0f44f5ad4c..3e2778cd82 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/shutdown-manager.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml index 029b1a0ffd..dcd6873ed8 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-annotations.yaml @@ -236,7 +236,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml index f39bbdf469..0c787d2682 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-concurrency.yaml @@ -60,7 +60,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-empty-memory-limits.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-empty-memory-limits.yaml index b2082c4fe4..dce47dfd70 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-empty-memory-limits.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-empty-memory-limits.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml index 6447197647..262570814b 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-extra-args.yaml @@ -233,7 +233,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml index 97069c09b1..71920b7767 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-image-pull-secrets.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-name.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-name.yaml index 9b531fbb0d..d63face021 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-name.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-name.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml index 9354ff9437..fec95a0001 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-node-selector.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml index 9423247952..f8f2159cca 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/with-topology-spread-constraints.yaml @@ -231,7 +231,7 @@ spec: fieldRef: apiVersion: v1 fieldPath: metadata.name - image: docker.io/envoyproxy/envoy:distroless-v1.33.0 + image: docker.io/envoyproxy/envoy:distroless-v1.33.1 imagePullPolicy: IfNotPresent lifecycle: preStop: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml index 84af681d9e..6af6849e3b 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/default.yaml @@ -86,7 +86,7 @@ spec: value: :19001 - name: PROMETHEUS_MAPPER_YAML value: /etc/statsd-exporter/conf.yaml - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml index de4c83da5f..017b8154eb 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/disable-prometheus.yaml @@ -76,7 +76,7 @@ spec: value: tcp - name: REDIS_URL value: redis.redis.svc:6379 - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing-custom.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing-custom.yaml index 6971dc186e..b901fc37f1 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing-custom.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing-custom.yaml @@ -101,7 +101,7 @@ spec: value: "0.6" - name: OTEL_EXPORTER_OTLP_ENDPOINT value: http://trace-collector.envoy-gateway-system.svc.cluster.local:4317 - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing.yaml index b78ec434d6..386b108a32 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/enable-tracing.yaml @@ -101,7 +101,7 @@ spec: value: "1.0" - name: OTEL_EXPORTER_OTLP_ENDPOINT value: http://trace-collector.envoy-gateway-system.svc.cluster.local:4318 - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-annotations.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-annotations.yaml index 945e60e676..97ceb278ff 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-annotations.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-annotations.yaml @@ -88,7 +88,7 @@ spec: value: :19001 - name: PROMETHEUS_MAPPER_YAML value: /etc/statsd-exporter/conf.yaml - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-labels.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-labels.yaml index 9cd149a109..69a0153b6c 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-labels.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/merge-labels.yaml @@ -88,7 +88,7 @@ spec: value: :19001 - name: PROMETHEUS_MAPPER_YAML value: /etc/statsd-exporter/conf.yaml - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml index 21b6e9fbfd..c93d833bd9 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/patch-deployment.yaml @@ -86,7 +86,7 @@ spec: value: :19001 - name: PROMETHEUS_MAPPER_YAML value: /etc/statsd-exporter/conf.yaml - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml index 4700c613c3..8d979a8632 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-node-selector.yaml @@ -86,7 +86,7 @@ spec: value: :19001 - name: PROMETHEUS_MAPPER_YAML value: /etc/statsd-exporter/conf.yaml - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml index c784904d25..530bedf028 100644 --- a/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml +++ b/internal/infrastructure/kubernetes/ratelimit/testdata/deployments/with-topology-spread-constraints.yaml @@ -86,7 +86,7 @@ spec: value: :19001 - name: PROMETHEUS_MAPPER_YAML value: /etc/statsd-exporter/conf.yaml - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f imagePullPolicy: IfNotPresent name: envoy-ratelimit ports: diff --git a/release-notes/current.yaml b/release-notes/current.yaml index f8824d2393..c76cbad89f 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -26,3 +26,5 @@ deprecations: | # Other notable changes not covered by the above sections. Other changes: | + Bumped envoy to v1.33.1. + Bumpled ratelimit to 0141a24f. diff --git a/site/content/en/latest/install/gateway-helm-api.md b/site/content/en/latest/install/gateway-helm-api.md index 428286534e..146ab24df5 100644 --- a/site/content/en/latest/install/gateway-helm-api.md +++ b/site/content/en/latest/install/gateway-helm-api.md @@ -66,7 +66,7 @@ The Helm chart for Envoy Gateway | global.images.envoyGateway.image | string | `nil` | | | global.images.envoyGateway.pullPolicy | string | `nil` | | | global.images.envoyGateway.pullSecrets | list | `[]` | | -| global.images.ratelimit.image | string | `"docker.io/envoyproxy/ratelimit:ae4cee11"` | | +| global.images.ratelimit.image | string | `"docker.io/envoyproxy/ratelimit:0141a24f"` | | | global.images.ratelimit.pullPolicy | string | `"IfNotPresent"` | | | global.images.ratelimit.pullSecrets | list | `[]` | | | kubernetesClusterDomain | string | `"cluster.local"` | | diff --git a/site/content/zh/latest/install/gateway-helm-api.md b/site/content/zh/latest/install/gateway-helm-api.md index 428286534e..146ab24df5 100644 --- a/site/content/zh/latest/install/gateway-helm-api.md +++ b/site/content/zh/latest/install/gateway-helm-api.md @@ -66,7 +66,7 @@ The Helm chart for Envoy Gateway | global.images.envoyGateway.image | string | `nil` | | | global.images.envoyGateway.pullPolicy | string | `nil` | | | global.images.envoyGateway.pullSecrets | list | `[]` | | -| global.images.ratelimit.image | string | `"docker.io/envoyproxy/ratelimit:ae4cee11"` | | +| global.images.ratelimit.image | string | `"docker.io/envoyproxy/ratelimit:0141a24f"` | | | global.images.ratelimit.pullPolicy | string | `"IfNotPresent"` | | | global.images.ratelimit.pullSecrets | list | `[]` | | | kubernetesClusterDomain | string | `"cluster.local"` | | diff --git a/test/helm/gateway-helm/certjen-custom-scheduling.out.yaml b/test/helm/gateway-helm/certjen-custom-scheduling.out.yaml index e4541b8cd2..4bba4e1f50 100644 --- a/test/helm/gateway-helm/certjen-custom-scheduling.out.yaml +++ b/test/helm/gateway-helm/certjen-custom-scheduling.out.yaml @@ -37,7 +37,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: diff --git a/test/helm/gateway-helm/control-plane-with-pdb.out.yaml b/test/helm/gateway-helm/control-plane-with-pdb.out.yaml index 210af77b53..243833482c 100644 --- a/test/helm/gateway-helm/control-plane-with-pdb.out.yaml +++ b/test/helm/gateway-helm/control-plane-with-pdb.out.yaml @@ -52,7 +52,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: diff --git a/test/helm/gateway-helm/default-config.out.yaml b/test/helm/gateway-helm/default-config.out.yaml index ebd8d9db5c..dffdc90ac3 100644 --- a/test/helm/gateway-helm/default-config.out.yaml +++ b/test/helm/gateway-helm/default-config.out.yaml @@ -37,7 +37,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: diff --git a/test/helm/gateway-helm/deployment-custom-topology.out.yaml b/test/helm/gateway-helm/deployment-custom-topology.out.yaml index c78eb3d246..7d580d91fc 100644 --- a/test/helm/gateway-helm/deployment-custom-topology.out.yaml +++ b/test/helm/gateway-helm/deployment-custom-topology.out.yaml @@ -37,7 +37,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: diff --git a/test/helm/gateway-helm/deployment-images-config.out.yaml b/test/helm/gateway-helm/deployment-images-config.out.yaml index f09e89d191..c7d4d18e2c 100644 --- a/test/helm/gateway-helm/deployment-images-config.out.yaml +++ b/test/helm/gateway-helm/deployment-images-config.out.yaml @@ -37,7 +37,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: diff --git a/test/helm/gateway-helm/deployment-priorityclass.out.yaml b/test/helm/gateway-helm/deployment-priorityclass.out.yaml index 9063afac9d..7d3ed99c6b 100644 --- a/test/helm/gateway-helm/deployment-priorityclass.out.yaml +++ b/test/helm/gateway-helm/deployment-priorityclass.out.yaml @@ -37,7 +37,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: diff --git a/test/helm/gateway-helm/deployment-securitycontext.out.yaml b/test/helm/gateway-helm/deployment-securitycontext.out.yaml index cf23490af9..f0a5dff525 100644 --- a/test/helm/gateway-helm/deployment-securitycontext.out.yaml +++ b/test/helm/gateway-helm/deployment-securitycontext.out.yaml @@ -37,7 +37,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: diff --git a/test/helm/gateway-helm/service-annotations.out.yaml b/test/helm/gateway-helm/service-annotations.out.yaml index 83ea20ab6a..e15f842700 100644 --- a/test/helm/gateway-helm/service-annotations.out.yaml +++ b/test/helm/gateway-helm/service-annotations.out.yaml @@ -37,7 +37,7 @@ data: kubernetes: rateLimitDeployment: container: - image: docker.io/envoyproxy/ratelimit:ae4cee11 + image: docker.io/envoyproxy/ratelimit:0141a24f patch: type: StrategicMerge value: From 58f58e937fbff9e80d8dadf5e52d4f0321e7ed77 Mon Sep 17 00:00:00 2001 From: Tomi Juntunen Date: Sun, 23 Mar 2025 01:08:56 +0200 Subject: [PATCH 09/10] fix: otel sink json access logging without text field (#5498) * fix otel sink json access logging without text field Signed-off-by: Tomi Juntunen * use json format as default when format or type is not set Signed-off-by: Tomi Juntunen * set formatters only if the slice of formatters is not empty Signed-off-by: Tomi Juntunen --------- Signed-off-by: Tomi Juntunen Co-authored-by: Arko Dasgupta (cherry picked from commit cb3ffd2f9054bf7da2c90b2fc07b5d1d7b4156a4) Signed-off-by: Guy Daich --- internal/xds/translator/accesslog.go | 18 ++- release-notes/current.yaml | 1 + test/e2e/testdata/accesslog-otel-default.yaml | 61 ++++++++++ test/e2e/testdata/accesslog-otel-json.yaml | 79 ++++++++++++ test/e2e/tests/accesslog.go | 114 +++++++++++++++++- 5 files changed, 264 insertions(+), 9 deletions(-) create mode 100644 test/e2e/testdata/accesslog-otel-default.yaml create mode 100644 test/e2e/testdata/accesslog-otel-json.yaml diff --git a/internal/xds/translator/accesslog.go b/internal/xds/translator/accesslog.go index 991c8e342c..304f22da49 100644 --- a/internal/xds/translator/accesslog.go +++ b/internal/xds/translator/accesslog.go @@ -336,10 +336,10 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) ([ ResourceAttributes: convertToKeyValueList(otel.Resources, false), } - if otel.Text == nil { - return nil, errors.New("otel.Text is nil") + var format string + if otel.Text != nil && *otel.Text != "" { + format = *otel.Text } - format := *otel.Text if format != "" { al.Body = &otlpcommonv1.AnyValue{ @@ -349,9 +349,17 @@ func buildXdsAccessLog(al *ir.AccessLog, accessLogType ir.ProxyAccessLogType) ([ } } - al.Attributes = convertToKeyValueList(otel.Attributes, true) + var attrs map[string]string + if len(otel.Attributes) != 0 { + attrs = otel.Attributes + } else if len(otel.Attributes) == 0 && format == "" { + // if there are no attributes and text format is unset, use the default EnvoyJSONLogFields + attrs = EnvoyJSONLogFields + } + + al.Attributes = convertToKeyValueList(attrs, true) + formatters := accessLogOpenTelemetryFormatters(format, attrs) - formatters := accessLogOpenTelemetryFormatters(format, otel.Attributes) if len(formatters) != 0 { al.Formatters = formatters } diff --git a/release-notes/current.yaml b/release-notes/current.yaml index c76cbad89f..6c69e1673e 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -13,6 +13,7 @@ bug fixes: | Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. Fixed updates of endpoints when mirrored backend Pod IPs are changed. Fix not logging an error and returning it in the K8s Reconcile method when a GatewayClass is not accepted. + Fix allowing empty text field for opentelemetry sink when using JSON format. Fixed validation of host header in RequestHeaderModifier filter. Retrigger reconciliation when backendRef of type ServiceImport is updated or when EndpointSlice(s) for a ServiceImport are updated. diff --git a/test/e2e/testdata/accesslog-otel-default.yaml b/test/e2e/testdata/accesslog-otel-default.yaml new file mode 100644 index 0000000000..764e8e02e5 --- /dev/null +++ b/test/e2e/testdata/accesslog-otel-default.yaml @@ -0,0 +1,61 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: accesslog-gtw + namespace: gateway-conformance-infra +spec: + gatewayClassName: {GATEWAY_CLASS_NAME} + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same + infrastructure: + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: accesslog-otel +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: EnvoyProxy +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + ipFamily: IPv4 + telemetry: + accessLog: + settings: + - matches: + - "'x-envoy-logged' in request.headers" + sinks: + - type: OpenTelemetry + openTelemetry: + backendRefs: + - name: otel-collector + namespace: monitoring + port: 4317 + resources: + k8s.cluster.name: "envoy-gateway" + shutdown: + drainTimeout: 5s + minDrainDuration: 1s +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: accesslog-gtw + rules: + - matches: + - path: + type: PathPrefix + value: /otel + backendRefs: + - name: infra-backend-v2 + port: 8080 diff --git a/test/e2e/testdata/accesslog-otel-json.yaml b/test/e2e/testdata/accesslog-otel-json.yaml new file mode 100644 index 0000000000..219315112c --- /dev/null +++ b/test/e2e/testdata/accesslog-otel-json.yaml @@ -0,0 +1,79 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: accesslog-gtw + namespace: gateway-conformance-infra +spec: + gatewayClassName: {GATEWAY_CLASS_NAME} + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: Same + infrastructure: + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: accesslog-otel +--- +apiVersion: gateway.envoyproxy.io/v1alpha1 +kind: EnvoyProxy +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + ipFamily: IPv4 + telemetry: + accessLog: + settings: + - format: + type: JSON + json: + time: "%START_TIME%" + method: "%REQ(:METHOD)%" + metadata: "%METADATA(ROUTE:envoy-gateway:resources)%" + x-envoy-original-path: "%REQ(X-ENVOY-ORIGINAL-PATH?:PATH)%" + protocol: "%PROTOCOL%" + responseCode: "%RESPONSE_CODE%" + responseFlags: "%RESPONSE_FLAGS%" + bytesReceived: "%BYTES_RECEIVED%" + bytesSent: "%BYTES_SENT%" + duration: "%DURATION%" + x-forwarded-for: "%REQ(X-FORWARDED-FOR)%" + user-agent: "%REQ(USER-AGENT)%" + x-request-id: "%REQ(X-REQUEST-ID)%" + authority: "%REQ(:AUTHORITY)%" + upstreamHost: "%UPSTREAM_HOST%" + matches: + - "'x-envoy-logged' in request.headers" + sinks: + - type: OpenTelemetry + openTelemetry: + backendRefs: + - name: otel-collector + namespace: monitoring + port: 4317 + resources: + k8s.cluster.name: "envoy-gateway" + shutdown: + drainTimeout: 5s + minDrainDuration: 1s +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: accesslog-otel + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: accesslog-gtw + rules: + - matches: + - path: + type: PathPrefix + value: /otel + backendRefs: + - name: infra-backend-v2 + port: 8080 diff --git a/test/e2e/tests/accesslog.go b/test/e2e/tests/accesslog.go index 26b5caa4aa..f346228b13 100644 --- a/test/e2e/tests/accesslog.go +++ b/test/e2e/tests/accesslog.go @@ -21,7 +21,7 @@ import ( ) func init() { - ConformanceTests = append(ConformanceTests, FileAccessLogTest, OpenTelemetryTest, ALSTest) + ConformanceTests = append(ConformanceTests, FileAccessLogTest, OpenTelemetryTestText, OpenTelemetryTestJSON, ALSTest, OpenTelemetryTestJSONAsDefault) } var FileAccessLogTest = suite.ConformanceTest{ @@ -116,9 +116,9 @@ var FileAccessLogTest = suite.ConformanceTest{ }, } -var OpenTelemetryTest = suite.ConformanceTest{ - ShortName: "OpenTelemetryAccessLog", - Description: "Make sure OpenTelemetry access log is working", +var OpenTelemetryTestText = suite.ConformanceTest{ + ShortName: "OpenTelemetryTextAccessLog", + Description: "Make sure OpenTelemetry text access log is working", Manifests: []string{"testdata/accesslog-otel.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { labels := map[string]string{ @@ -169,6 +169,112 @@ var OpenTelemetryTest = suite.ConformanceTest{ }, } +var OpenTelemetryTestJSONAsDefault = suite.ConformanceTest{ + ShortName: "OpenTelemetryAccessLogJSONAsDefault", + Description: "Make sure OpenTelemetry JSON access log is working as default when no format or type is specified", + Manifests: []string{"testdata/accesslog-otel-default.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + labels := map[string]string{ + "k8s_namespace_name": "envoy-gateway-system", + "exporter": "OTLP", + } + + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "accesslog-otel", Namespace: ns} + gwNN := types.NamespacedName{Name: "accesslog-gtw", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + + t.Run("Positive", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + Headers: map[string]string{ + "x-envoy-logged": "1", + }, + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 1) + }) + + t.Run("Negative", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + // envoy will not log this request without the header x-envoy-logged + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 0) + }) + }, +} + +var OpenTelemetryTestJSON = suite.ConformanceTest{ + ShortName: "OpenTelemetryAccessLogJSON", + Description: "Make sure OpenTelemetry JSON access log is working with custom JSON attributes", + Manifests: []string{"testdata/accesslog-otel-json.yaml"}, + Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { + labels := map[string]string{ + "k8s_namespace_name": "envoy-gateway-system", + "exporter": "OTLP", + } + + ns := "gateway-conformance-infra" + routeNN := types.NamespacedName{Name: "accesslog-otel", Namespace: ns} + gwNN := types.NamespacedName{Name: "accesslog-gtw", Namespace: ns} + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) + + t.Run("Positive", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + Headers: map[string]string{ + "x-envoy-logged": "1", + }, + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 1) + }) + + t.Run("Negative", func(t *testing.T) { + expectedResponse := httputils.ExpectedResponse{ + Request: httputils.Request{ + Path: "/otel", + // envoy will not log this request without the header x-envoy-logged + }, + Response: httputils.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + // make sure listener is ready + httputils.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse) + + runLogTest(t, suite, gwAddr, expectedResponse, labels, "", 0) + }) + }, +} + var ALSTest = suite.ConformanceTest{ ShortName: "ALS", Description: "Make sure ALS access log is working", From bddf36c84ed0d32132817081f9fd77881ff76bc4 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 24 Mar 2025 06:54:10 -0500 Subject: [PATCH 10/10] [release/v1.3] v1.3.2 release notes (#5584) v1.3.2 release notes Signed-off-by: Guy Daich --- VERSION | 2 +- release-notes/v1.3.2.yaml | 19 +++++++++++++++ site/content/en/news/releases/notes/v1.3.2.md | 23 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 release-notes/v1.3.2.yaml create mode 100644 site/content/en/news/releases/notes/v1.3.2.md diff --git a/VERSION b/VERSION index 7574079828..968e750a6c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v1.3.1 +v1.3.2 diff --git a/release-notes/v1.3.2.yaml b/release-notes/v1.3.2.yaml new file mode 100644 index 0000000000..3422655b4e --- /dev/null +++ b/release-notes/v1.3.2.yaml @@ -0,0 +1,19 @@ +date: March 24, 2025 + +bug fixes: | + Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. + Fixed updates of endpoints when mirrored backend Pod IPs are changed. + Fix not logging an error and returning it in the K8s Reconcile method when a GatewayClass is not accepted. + Fix allowing empty text field for opentelemetry sink when using JSON format. + Fixed validation of host header in RequestHeaderModifier filter. + Retrigger reconciliation when backendRef of type ServiceImport is updated or when EndpointSlice(s) for a ServiceImport are updated. + +# Enhancements that improve performance. +performance improvements: | + Added a cache for the Wasm OCI image permission checks and check the pullSecrets against the OCI image registry in + a background goroutine. + +# Other notable changes not covered by the above sections. +Other changes: | + Bumped envoy to v1.33.1. + Bumped ratelimit to 0141a24f. diff --git a/site/content/en/news/releases/notes/v1.3.2.md b/site/content/en/news/releases/notes/v1.3.2.md new file mode 100644 index 0000000000..73760e606c --- /dev/null +++ b/site/content/en/news/releases/notes/v1.3.2.md @@ -0,0 +1,23 @@ +--- +title: "v1.3.2" +publishdate: 2025-03-24 +--- + +Date: March 24, 2025 + +## Bug fixes +- Added support for BackendTLSPolicy and EnvoyExtensionPolicy parsing in Standalone mode. +- Fixed updates of endpoints when mirrored backend Pod IPs are changed. +- Fix not logging an error and returning it in the K8s Reconcile method when a GatewayClass is not accepted. +- Fix allowing empty text field for opentelemetry sink when using JSON format. +- Fixed validation of host header in RequestHeaderModifier filter. +- Retrigger reconciliation when backendRef of type ServiceImport is updated or when EndpointSlice(s) for a ServiceImport are updated. + +## Performance improvements +- Added a cache for the Wasm OCI image permission checks and check the pullSecrets against the OCI image registry in +- a background goroutine. + +## Other changes +- Bumped envoy to v1.33.1. +- Bumped ratelimit to 0141a24f. +