chore: manage Gateway API resources with stable Helm chart - #2940
chore: manage Gateway API resources with stable Helm chart#2940joelmccoy wants to merge 4 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Greptile SummaryThis PR replaces the content-derived Gateway API manifest release with a stable local Helm chart and adds guarded migration of resources owned by the known legacy release.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the ownership checks address the previously reported takeover paths by migrating only resources annotated for the detected legacy release. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Deploy gateway-api-crds component] --> B{Matching legacy release exists?}
B -->|No| E[Install stable Helm chart]
B -->|Yes| C[Retag resources owned by legacy release]
C --> D{Any resources migrated?}
D -->|Yes| F[Delete legacy Helm storage records]
D -->|No| E
F --> E
E --> G[Validate stable ownership and absence of legacy release]
Reviews (5): Last reviewed commit: "chore: cleanup legacy release" | Re-trigger Greptile |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes upgrade/migration behavior for cluster-scoped Gateway API resources and Helm ownership metadata, which warrants a final human validation of the migration path in a real upgrade scenario.
Pull request overview
This PR migrates Gateway API resource management from Zarf manifests (which produced unstable/generated Helm release names) to a stable, vendored local Helm chart, and wires CI/tasks to keep the vendored install manifest in sync.
Changes:
- Add a pinned
GATEWAY_API_VERSIONand a new task to vendor/sync the Gateway API experimental install manifest into the repo. - Switch the Istio package’s Gateway API install from
manifeststo a stable local Helm chart release, including a one-time ownership metadata migration step. - Update the autogenerated-file CI check and codespell configuration to account for the newly vendored YAML.
File summaries
| File | Description |
|---|---|
src/istio/tasks.yaml |
Adds GATEWAY_API_VERSION and a task to download/sync the Gateway API install YAML; updates CRD type generation to use the pinned version. |
src/istio/gateway-api-crds/chart/Chart.yaml |
Introduces a local Helm chart wrapper for vendored Gateway API install resources. |
src/istio/common/zarf.yaml |
Switches Gateway API install to a stable local chart release and adds an ownership migration step for existing clusters. |
.github/actions/autogenerated-check/action.yaml |
Ensures CI regenerates and validates the vendored Gateway API YAML alongside other generated artifacts. |
.codespellrc |
Excludes the vendored Gateway API YAML directory from codespell checks. |
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@greptileai @codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@greptileai review |
|
@greptileai @codex review |
💡 Codex Reviewhttps://github.com/defenseunicorns/uds-core/blob/217ce021375b958363ce3eb95612924ee0ae7161/src/istio/gateway-api-crds/chart/templates/gateway-api-crds.yaml#L1 The newly vendored manifest has a copyright header without the current year, so it does not comply with the repository requirement that every modified file with such a header include 2026. Because AGENTS.md reference: AGENTS.md:L3-L3 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex @greptileai review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
8f28e00 to
53d3902
Compare
Description
When using manifests in zarf packages the helm chart name is generated based on the content causing the helm chart name to change. We should avoid using manifests directly as it can cause some ownership issues when using different resource managers. This swaps the gateway api resources to a local stable helm chart. This is needed for a clean migration to using cli next to deploy on top of cli legacy packages.
Instead of reference the manifests via a remote link, I updated our CI/tasks to pull down and sync them in the repo.
Type of change
Steps to Validate
uds run test-uds-core-upgrade --set FLAVOR=upstream --no-progressChecklist before merging