Skip to content

Commit dc73f48

Browse files
yuehaiizirain
andauthored
fix: prevent GatewayNamespace mode from hijacking pre-existing ServiceAccounts and ConfigMaps (#9215)
* check ownership before apply Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * both the generic managed-by label and the owning-gateway identity labels must match Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * if supplies a custom SA name via EnvoyServiceAccount.Name, the SA is pre-existing and user-managed. we must not block the apply or add an ownerReference to it. Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * add test cases Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * add e2e test case and release note. Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * fix lint format warning Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * use an uncached/APIReader-style live read for this conflict check before applying Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * add same fix and test cases for Deployment, DaemonSet, optional PDB, optional HPA, and Service Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * add missing release note Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * sync the release notes with main Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * restore previous added content in current release Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * rename the release file name to lower cases Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * solve the issues detected by codex Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * adjust the test cases and remove the gate. fix lint format warning. Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * replace with GatewayNamespaceMode gate Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * fix previous dirty context caused HTTPRouteMixedBackends test case breaking Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * Revert "use an uncached/APIReader-style live read for this conflict check before applying" This reverts commit 999a349. Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * review and adjust code suggested by copilot and codex. Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * refactor code to make it more readable. Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> * refactor code to be more readable. Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> --------- Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com> Signed-off-by: Hai <20416005+yuehaii@users.noreply.github.com> Co-authored-by: zirain <zirain2009@gmail.com>
1 parent 29cb580 commit dc73f48

5 files changed

Lines changed: 569 additions & 9 deletions

File tree

internal/infrastructure/kubernetes/infra_resource.go

Lines changed: 86 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,21 @@ import (
2020
"k8s.io/apimachinery/pkg/types"
2121
"sigs.k8s.io/controller-runtime/pkg/client"
2222

23+
"github.com/envoyproxy/gateway/internal/gatewayapi"
2324
"github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/proxy"
2425
"github.com/envoyproxy/gateway/internal/metrics"
2526
labelsutil "github.com/envoyproxy/gateway/internal/utils/labels"
2627
)
2728

29+
// applyIfOwned skips the apply when the resource would hijack an unrelated,
30+
// pre-existing resource in GatewayNamespace mode (see checkOwnership).
31+
func (i *Infra) applyIfOwned(ctx context.Context, obj client.Object) error {
32+
if err := i.checkOwnership(ctx, obj); err != nil {
33+
return err
34+
}
35+
return i.Client.ServerSideApply(ctx, obj)
36+
}
37+
2838
// createOrUpdateServiceAccount creates a ServiceAccount in the kube api server based on the
2939
// provided ResourceRender, if it doesn't exist and updates it if it does.
3040
func (i *Infra) createOrUpdateServiceAccount(ctx context.Context, r ResourceRender) (err error) {
@@ -66,7 +76,7 @@ func (i *Infra) createOrUpdateServiceAccount(ctx context.Context, r ResourceRend
6676
return err
6777
}
6878

69-
return i.Client.ServerSideApply(ctx, sa)
79+
return i.applyIfOwned(ctx, sa)
7080
}
7181

7282
// createOrUpdateConfigMap creates a ConfigMap in the Kube api server based on the provided
@@ -105,7 +115,7 @@ func (i *Infra) createOrUpdateConfigMap(ctx context.Context, r ResourceRender) (
105115
}
106116
}()
107117

108-
return i.Client.ServerSideApply(ctx, cm)
118+
return i.applyIfOwned(ctx, cm)
109119
}
110120

111121
// createOrUpdateDeployment creates a Deployment in the kube api server based on the provided
@@ -158,7 +168,7 @@ func (i *Infra) createOrUpdateDeployment(ctx context.Context, r ResourceRender)
158168
if err != nil {
159169
if apierrors.IsNotFound(err) {
160170
// It's the deployment creation.
161-
return i.Client.ServerSideApply(ctx, deployment)
171+
return i.applyIfOwned(ctx, deployment)
162172
}
163173
return err
164174
}
@@ -192,7 +202,7 @@ func (i *Infra) createOrUpdateDeployment(ctx context.Context, r ResourceRender)
192202
}
193203
}
194204

195-
return i.Client.ServerSideApply(ctx, deployment)
205+
return i.applyIfOwned(ctx, deployment)
196206
}
197207

198208
// createOrUpdateDaemonSet creates a DaemonSet in the kube api server based on the provided
@@ -245,7 +255,7 @@ func (i *Infra) createOrUpdateDaemonSet(ctx context.Context, r ResourceRender) (
245255
if err != nil {
246256
if apierrors.IsNotFound(err) {
247257
// It's the daemonset creation.
248-
return i.Client.ServerSideApply(ctx, daemonSet)
258+
return i.applyIfOwned(ctx, daemonSet)
249259
}
250260
return err
251261
}
@@ -278,7 +288,7 @@ func (i *Infra) createOrUpdateDaemonSet(ctx context.Context, r ResourceRender) (
278288
}
279289
}
280290

281-
return i.Client.ServerSideApply(ctx, daemonSet)
291+
return i.applyIfOwned(ctx, daemonSet)
282292
}
283293

284294
func (i *Infra) createOrUpdatePodDisruptionBudget(ctx context.Context, r ResourceRender) (err error) {
@@ -324,7 +334,7 @@ func (i *Infra) createOrUpdatePodDisruptionBudget(ctx context.Context, r Resourc
324334
}
325335
}()
326336

327-
return i.Client.ServerSideApply(ctx, pdb)
337+
return i.applyIfOwned(ctx, pdb)
328338
}
329339

330340
// createOrUpdateHPA creates HorizontalPodAutoscaler object in the kube api server based on
@@ -373,7 +383,7 @@ func (i *Infra) createOrUpdateHPA(ctx context.Context, r ResourceRender) (err er
373383
}
374384
}()
375385

376-
return i.Client.ServerSideApply(ctx, hpa)
386+
return i.applyIfOwned(ctx, hpa)
377387
}
378388

379389
// createOrUpdateRateLimitService creates a Service in the kube api server based on the provided ResourceRender,
@@ -414,7 +424,7 @@ func (i *Infra) createOrUpdateService(ctx context.Context, r ResourceRender) (er
414424
}
415425
}()
416426

417-
return i.Client.ServerSideApply(ctx, svc)
427+
return i.applyIfOwned(ctx, svc)
418428
}
419429

420430
// deleteServiceAccount deletes the ServiceAccount in the kube api server, if it exists.
@@ -721,3 +731,70 @@ func (i *Infra) getEnvoyGatewayCA(ctx context.Context) string {
721731
}
722732
return string(secret.Data[proxy.XdsTLSCaFileName])
723733
}
734+
735+
// noGatewayIdentityLabels reports whether labels carry none of the three
736+
// gateway ownership identity labels. Resources in GatewayNamespace mode
737+
// with no identity labels (such as ratelimit resources) are controller-scoped
738+
// and need no collision guard.
739+
func noGatewayIdentityLabels(labels map[string]string) bool {
740+
return labels[gatewayapi.OwningGatewayNameLabel] == "" &&
741+
labels[gatewayapi.OwningGatewayNamespaceLabel] == "" &&
742+
labels[gatewayapi.OwningGatewayClassLabel] == ""
743+
}
744+
745+
// checkOwnership guards against adopting a pre-existing resource that shares a
746+
// name with a Gateway-owned resource in GatewayNamespace mode. If a resource
747+
// with the same name/namespace already exists and is not labeled as owned by
748+
// this Gateway, the apply is skipped to avoid hijacking (and later garbage-
749+
// collecting) an unrelated resource.
750+
//
751+
// This uses the cached client, so it is best-effort: there is a small window
752+
// where a same-named resource created just before this reconcile may not yet be
753+
// reflected in the cache and could be adopted. This is considered an acceptable
754+
// edge case — it does not cause EG to fail, and using the cache avoids extra
755+
// uncached API reads on every reconcile (see #8764).
756+
func (i *Infra) checkOwnership(ctx context.Context, obj client.Object) error {
757+
if !i.EnvoyGateway.GatewayNamespaceMode() || noGatewayIdentityLabels(obj.GetLabels()) {
758+
return nil
759+
}
760+
761+
existing := obj.DeepCopyObject().(client.Object)
762+
err := i.Client.Get(ctx, types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}, existing)
763+
if err != nil {
764+
if apierrors.IsNotFound(err) {
765+
return nil
766+
}
767+
return err
768+
}
769+
770+
if !ownedByGateway(existing.GetLabels(), obj.GetLabels()) {
771+
ownershipErr := fmt.Errorf("%s %s/%s already exists and is not owned by this Gateway",
772+
obj.GetObjectKind().GroupVersionKind().Kind, obj.GetNamespace(), obj.GetName())
773+
i.logger.Error(ownershipErr, "resource already exists and is not owned by this Gateway, skipping",
774+
"kind", obj.GetObjectKind().GroupVersionKind().Kind,
775+
"name", obj.GetName(), "namespace", obj.GetNamespace())
776+
return ownershipErr
777+
}
778+
return nil
779+
}
780+
781+
func ownedByGateway(existingLabels, desiredLabels map[string]string) bool {
782+
if existingLabels["app.kubernetes.io/managed-by"] != "envoy-gateway" {
783+
return false
784+
}
785+
786+
gwName := desiredLabels[gatewayapi.OwningGatewayNameLabel]
787+
gwNS := desiredLabels[gatewayapi.OwningGatewayNamespaceLabel]
788+
if gwName != "" || gwNS != "" {
789+
return existingLabels[gatewayapi.OwningGatewayNameLabel] == gwName &&
790+
existingLabels[gatewayapi.OwningGatewayNamespaceLabel] == gwNS
791+
}
792+
793+
gwClass := desiredLabels[gatewayapi.OwningGatewayClassLabel]
794+
if gwClass != "" {
795+
return existingLabels[gatewayapi.OwningGatewayClassLabel] == gwClass
796+
}
797+
798+
// no identity labels present.
799+
return false
800+
}

0 commit comments

Comments
 (0)