Skip to content

fix: remove cross ns policy attachment status#8901

Merged
zhaohuabing merged 2 commits into
envoyproxy:mainfrom
zhaohuabing:remove-cross-ns-policy-attachment-status
May 6, 2026
Merged

fix: remove cross ns policy attachment status#8901
zhaohuabing merged 2 commits into
envoyproxy:mainfrom
zhaohuabing:remove-cross-ns-policy-attachment-status

Conversation

@zhaohuabing
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing commented May 1, 2026

This PR removes the warning condition for cross-ns policy attachment missing RefrenceGrants.
This is a follow-up of #8676
fixes: #8894

This reverts commit 5d88fbb.

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing requested a review from a team as a code owner May 1, 2026 01:22
@netlify
Copy link
Copy Markdown

netlify Bot commented May 1, 2026

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit b5acf8b
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69f40d828540560008dd45fd
😎 Deploy Preview https://deploy-preview-8901--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@zhaohuabing
Copy link
Copy Markdown
Member Author

@codex review

@zhaohuabing zhaohuabing marked this pull request as draft May 1, 2026 01:23
@zhaohuabing zhaohuabing force-pushed the remove-cross-ns-policy-attachment-status branch from 602633c to a01c1e6 Compare May 1, 2026 01:25
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 602633c804

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/gatewayapi/helpers.go
Comment thread api/v1alpha1/shared_types.go
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.68%. Comparing base (3a0306e) to head (b5acf8b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8901      +/-   ##
==========================================
+ Coverage   74.66%   74.68%   +0.01%     
==========================================
  Files         251      251              
  Lines       40257    40125     -132     
==========================================
- Hits        30059    29968      -91     
+ Misses       8130     8096      -34     
+ Partials     2068     2061       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zhaohuabing zhaohuabing force-pushed the remove-cross-ns-policy-attachment-status branch from a01c1e6 to f1e9355 Compare May 1, 2026 01:59
@zhaohuabing zhaohuabing added this to the v1.8.0 Release milestone May 1, 2026
…renceGrants

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing force-pushed the remove-cross-ns-policy-attachment-status branch from f1e9355 to b5acf8b Compare May 1, 2026 02:18
@zhaohuabing zhaohuabing marked this pull request as ready for review May 1, 2026 02:18
@zhaohuabing zhaohuabing requested review from arkodg and guydc May 1, 2026 02:20
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b5acf8b5b2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +298 to 305
targetRefs := resolvePolicyTargets(
currPolicy.Spec.PolicyTargetReferences,
routes,
resources.ReferenceGrants,
egv1a1.GroupName,
egv1a1.KindBackendTrafficPolicy,
currPolicy.Namespace,
t.GetNamespace)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Emit status when cross-namespace selector targets are denied

This change stops tracking denied selector matches, so a policy whose cross-namespace targetSelectors lose their ReferenceGrant can drop out of the translated policy set entirely and never publish a replacement status. Because policy delete updates are ignored by the Kubernetes status subscribers (internal/provider/kubernetes/status.go skips update.Delete), the old Accepted ancestor condition can persist indefinitely even though the policy no longer applies. Fresh evidence in this commit is the removal of denied-target handling from policy processors (including this one), which makes the stale-status path reproducible during normal runtime (e.g., revoking an existing ReferenceGrant), not just during upgrade transitions.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

non blocking - will a ReferenceGrant, not trigger a Reconcile which will trigger a update in gateway-api layer to update status post translation ?

Copy link
Copy Markdown
Member Author

@zhaohuabing zhaohuabing May 6, 2026

Choose a reason for hiding this comment

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

I think the cross-namespace policy attachment feature does not change the pre-existing status behavior here. The same behavior already existed for selector targets before cross-namespace attachment, and this feature just makes a ReferenceGrant change another way to reach the same path.

Copy link
Copy Markdown
Member Author

@zhaohuabing zhaohuabing May 6, 2026

Choose a reason for hiding this comment

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

I created #8926 and #8927 to track the stale status issue separately.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

non blocking - will a ReferenceGrant, not trigger a Reconcile which will trigger a update in gateway-api layer to update status post translation

A ReferenceGrant will trigger a Reconcile, but stale status won't get updated for non-matching/missing ReferenceGrant due to #8927 .

@zhaohuabing
Copy link
Copy Markdown
Member Author

/retest

@arkodg arkodg requested review from a team May 3, 2026 23:24
@kkk777-7
Copy link
Copy Markdown
Member

kkk777-7 commented May 6, 2026

LGTM, thanks!

@zhaohuabing zhaohuabing merged commit 69b1dde into envoyproxy:main May 6, 2026
65 of 68 checks passed
@zhaohuabing zhaohuabing deleted the remove-cross-ns-policy-attachment-status branch May 6, 2026 03:43
jukie added a commit that referenced this pull request May 13, 2026
* fix(api): increase RateLimitSelectCondition.headers MaxItems from 16 to 64 (#8906)

* fix(api): increase RateLimitSelectCondition.headers MaxItems from 16 to 64

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* feat: policy field owner (#8538)

* feat: policy field owner

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* skip invalid listener first in IR (#8577)

* skip invalid listener

Signed-off-by: zirain <zirain2009@gmail.com>

* fix specValid

Signed-off-by: zirain <zirain2009@gmail.com>

* nit

Signed-off-by: zirain <zirain2009@gmail.com>

* fix

Signed-off-by: zirain <zirain2009@gmail.com>

* MUST NOT pick one conflicting Listener as the winner

Signed-off-by: zirain <zirain2009@gmail.com>

* update

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
Co-authored-by: Isaac Wilson <isaac.wilson514@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: remove cross ns policy attachment status (#8901)

* Revert "add warning for partially accepted targets"

This reverts commit 5d88fbb.

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

* remove warning condition for cross-ns policy attachments without referenceGrants

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* feat: add runner event metrics (#8802)

* add metrics for runner

Signed-off-by: zirain <zirain2009@gmail.com>

* rename

Signed-off-by: zirain <zirain2009@gmail.com>

* rename

Signed-off-by: zirain <zirain2009@gmail.com>

* reuse

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: respect backend endpoint hostname for health checks (#8929)

* fix: respect backend endpoint hostname for health checks

### Summary
- Keep BackendTrafficPolicy HTTP health check hostnames as explicit cluster-level hosts, and leave route-derived host fallback to xDS cluster translation.
- Preserve Backend endpoint hostnames as per-endpoint overrides via Endpoint.HealthCheckConfig.hostname, ahead of the route fallback.
- Update gatewayapi/xDS fixtures, release notes, and generated API docs/CRDs for the host selection order.

### Test plan
- go test ./internal/ir
- go test ./internal/xds/translator
- go test ./internal/gatewayapi -run TestTranslate/backendtrafficpolicy
- go test ./internal/gatewayapi -run TestTranslate/(clienttrafficpolicy-http-health-check|envoyextensionpolicy-with-extproc-with-retries|envoyextensionpolicy-with-extproc-with-traffic-features|envoyproxy-accesslog-with-traffic|envoyproxy-tracing-backend-uds|envoyproxy-tracing-backend|securitypolicy-with-jwt-backendcluster|securitypolicy-with-jwt-backendsettings)
- make generate
- make manifests
- git diff --check

Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Co-authored-by: Codex <noreply@openai.com>

* fix gen

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix(helm): propagate commonLabels to RBAC resources (#8818)

* feat(helm): propagate commonLabels to RBAC resources

Issue #8817 reported that 'helm template ... --set
commonLabels.custom-label=custom-value' left ClusterRole,
ClusterRoleBinding, Role, and RoleBinding resources unlabelled.
The other resources in the chart already include 'eg.labels' in
their metadata - which picks up 'commonLabels' via the helper
at _helpers.tpl:43 - but envoy-gateway-rbac.yaml didn't set any
labels block.

Add 'labels: {{- include "eg.labels" . | nindent 4 }}' on every
Role / RoleBinding / ClusterRole / ClusterRoleBinding declared in
envoy-gateway-rbac.yaml. Matches the existing labels pattern used
in certgen-rbac.yaml and envoy-gateway-deployment.yaml. Scopes are
'$' inside the watched-namespaces 'range' and '.' at the template
root, same rule the helper block inside the file already used.

Verified locally with:
  helm dependency update charts/gateway-helm
  envsubst < charts/gateway-helm/values.tmpl.yaml > \
    charts/gateway-helm/values.yaml
  helm template eg charts/gateway-helm \
    --set commonLabels.custom-label=custom-value | yq ...

All four RBAC resources now emit 'custom-label: custom-value' in
their metadata.labels, matching the issue's repro steps. Cert-gen
RBAC resources already carried it; this PR brings the core
envoy-gateway RBAC set into parity.

Fixes #8817

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

* chore: regenerate helm-template snapshots for RBAC labels

Run 'make helm-template.gateway-helm' to regenerate the snapshot fixtures
after the envoy-gateway-rbac.yaml labels change. Adds the 'labels:' block
to the RBAC resources in all 27 test cases.

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

* fix gen

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

* add release note

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef (#8871)

* fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef

When a ListenerSet listener has an unresolvable TLS certificate reference
(InvalidCertificateRef or RefNotPermitted), Accepted: False was incorrectly
set on both the listener and ListenerSet object. The Gateway API spec places
InvalidCertificateRef exclusively under ResolvedRefs, not Accepted — a missing
certificate is a reference resolution concern, not a structural one.

Fixes #8870

Signed-off-by: apkatsikas <apkatsikas@gmail.com>

* chore: fix gofumpt formatting in validateListenerConditions

Signed-off-by: apkatsikas <apkatsikas@gmail.com>

* fix(translator): separate RefNotPermitted from InvalidCertificateRef handling

Unlike InvalidCertificateRef, RefNotPermitted should not set Accepted:True.
Update unit test fixtures to match.

Signed-off-by: apkatsikas <apkatsikas@gmail.com>

* fix gen

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: apkatsikas <apkatsikas@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: do not downgrade ALPN for only hostnames-overlapping listeners (#8934)

fix: do not downgrade ALPN for overlapping hostnames withoug SANs overlapping

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* feat: enableDeferredCreationStats by default (#8937)

* feat: enableDeferredCreationStats by default

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: restore last transition time in merge status conditions (#8962)

* fix: restore last transition time in merge status conditions

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* add release note

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

---------

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* [release/v1.8] v1.8.0 release notes

Cherry-picked release-notes/v1.8.0.yaml and VERSION bump from #8942.

Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

---------

Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Signed-off-by: apkatsikas <apkatsikas@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Co-authored-by: wucm667 <109257021+wucm667@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Kota Kimura <86363983+kkk777-7@users.noreply.github.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Matt Van Horn <mvanhorn@users.noreply.github.com>
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Andrew Katsikas <apkatsikas@gmail.com>
Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify cross-namespace policy attachment semantics with namespaceSelector and ReferenceGrant

3 participants