Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Kubernetes Ingress primitive to the operator component framework, following the existing primitive patterns (builder/resource + mutator pipeline + flavors + lifecycle handlers), and includes an accompanying example and documentation.
Changes:
- Introduces
pkg/primitives/ingress(builder/resource, mutator, handlers, flavors) plus unit tests. - Adds a shared
IngressSpecEditorunderpkg/mutation/editors(with tests) to support typed ingress spec mutations. - Adds an
examples/ingress-primitivewalkthrough and newdocs/primitives/ingress.mddocumentation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/primitives/ingress/resource.go | Defines the ingress Resource wrapper over generic.IntegrationResource. |
| pkg/primitives/ingress/builder.go | Builder API wiring for applicators, flavors, mutations, status/suspension handlers, data extraction. |
| pkg/primitives/ingress/mutator.go | Plan/apply mutator with per-feature planning and category ordering. |
| pkg/primitives/ingress/handlers.go | Default operational/suspension handler implementations for integration lifecycle. |
| pkg/primitives/ingress/flavors.go | Ingress flavors (label/annotation preservation) via shared pkg/flavors. |
| pkg/primitives/ingress/*_test.go | Unit tests for ingress builder/mutator/handlers/flavors behavior. |
| pkg/mutation/editors/ingressspec.go | Adds typed IngressSpecEditor for ingress spec mutations. |
| pkg/mutation/editors/ingressspec_test.go | Tests for IngressSpecEditor operations (class name, backend, rules, TLS). |
| examples/ingress-primitive/main.go | Standalone example driving reconciliation steps using a fake client. |
| examples/ingress-primitive/resources/ingress.go | Example ingress resource factory using mutations, flavors, and data extraction. |
| examples/ingress-primitive/features/mutations.go | Example feature-gated ingress mutations (version annotation + TLS). |
| examples/ingress-primitive/app/controller.go | Example controller that builds a component around the ingress primitive. |
| examples/ingress-primitive/app/owner.go | Re-exports shared ExampleApp types for the example package. |
| examples/ingress-primitive/README.md | Documents how to run and understand the ingress example. |
| docs/primitives/ingress.md | New primitive documentation covering usage, ordering, handlers, flavors, guidance. |
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored:
<!-- claude-review-cycle --> |
|
approved |
c21b61c to
c31fd20
Compare
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored:
<!-- claude-review-cycle --> |
Provides SetIngressClassName, SetDefaultBackend, EnsureRule (upsert by host), RemoveRule, EnsureTLS (upsert by first host), RemoveTLS, and Raw escape hatch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements builder, resource, mutator, handlers, and flavors for the networking.k8s.io/v1 Ingress kind. Key design decisions: - DefaultDeleteOnSuspend = false (avoids ingress controller churn) - DefaultSuspendMutation = no-op (backend 502/503 is correct behaviour) - Operational status: Pending until load balancer address assigned - Suspension status: immediately Suspended with reason message Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers operational status, suspension strategy, mutation pipeline, editors, flavors, and guidance for common use cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Demonstrates base construction, feature mutations (version annotation, TLS toggle), field flavors, and data extraction using the ingress builder and mutator. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address Copilot review: DefaultOperationalStatusHandler now checks that at least one LoadBalancer.Ingress entry has a non-empty IP or Hostname, rather than only checking slice length. This prevents marking an Ingress as Operational when entries exist but have no assigned address. Update docs and add test for the empty-entry edge case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix doc comments to reference concepts.Operational/Suspendable/DataExtractable instead of component.* for lifecycle interfaces - Use correct OperationPending status name in docs and code comments - Add ingress entry to built-in primitives table in docs/primitives.md - Add IngressSpecEditor to mutation editors table in docs/primitives.md - Add cross-reference link to primitives overview in ingress doc - Add resource_test.go with tests for Identity, Object deep-copy, Mutate, mutations, feature ordering, custom applicator, converging status, suspension, and data extraction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix doc comments to reference concepts.Operational/Suspendable/DataExtractable instead of component.* for lifecycle interfaces - Use correct OperationPending status name in docs and code comments - Add ingress entry to built-in primitives table in docs/primitives.md - Add IngressSpecEditor to mutation editors table in docs/primitives.md - Add cross-reference link to primitives overview in ingress doc - Add resource_test.go with tests for Identity, Object deep-copy, Mutate, mutations, feature ordering, custom applicator, converging status, suspension, and data extraction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The DefaultFieldApplicator was replacing the entire object with the desired state, which wiped ingress-controller-owned fields like Status.LoadBalancer.Ingress. This caused DefaultOperationalStatusHandler to never see assigned addresses, keeping the resource permanently pending. Save and restore the live Status before overwriting with the desired spec so that readiness detection works correctly. Also adds test coverage for: - Status.LoadBalancer preservation through DefaultFieldApplicator - DefaultOperationalStatusHandler returning Operational with IP - DefaultOperationalStatusHandler returning Operational with Hostname Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace manual status save/restore with generic.PreserveStatus to match the deployment primitive pattern. Add Default Field Application section to ingress primitive docs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Initialize plans slice and active pointer directly in the constructor, matching the fix already applied to deployment and configmap mutators. This prevents an empty feature from being created when the generic helper in mutator_helper.go calls beginFeature(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e2ee083 to
3964dcf
Compare
Resolve conflict in docs/primitives.md by combining main's table formatting with the ingress primitive row from the feature branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Run make fmt-md to apply consistent markdown formatting to ingress documentation and example README. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored: <!-- claude-review-cycle --> |
Use framework-specific OperationPending (concepts.OperationalStatusPending) terminology instead of generic "Pending" to avoid confusion with other lifecycle states. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude Review Cycle 1 CompleteAddressed:
Intentionally ignored:
<!-- claude-review-cycle --> |
Align the ingress Mutator with the configmap and deployment primitives: NewMutator no longer creates an initial feature plan. BeginFeature must be called before registering any mutations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Align with the framework's switch to Server-Side Apply: remove DefaultFieldApplicator, WithCustomFieldApplicator, WithFieldApplicationFlavor, flavors.go, and all related tests and documentation. Update Mutate tests to use the Object()-first pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| - **Field Flavors**: Preserving labels and annotations that might be managed by external tools (e.g., cert-manager, | ||
| ingress controllers). |
There was a problem hiding this comment.
This README describes "Field Flavors" that preserve labels/annotations, but there is no corresponding flavor API in the codebase (e.g., no WithFieldApplicationFlavor exists). Please update the README to reflect the actual supported behavior, or implement the described flavor mechanism and reference the real API names.
| - **Field Flavors**: Preserving labels and annotations that might be managed by external tools (e.g., cert-manager, | |
| ingress controllers). |
| ## Built-in Primitives | ||
|
|
||
| | Primitive | Category | Documentation | | ||
| | --------------------------- | -------- | ----------------------------------------- | | ||
| | `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | | ||
| | `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | | ||
| | Primitive | Category | Documentation | | ||
| | --------------------------- | ----------- | ----------------------------------------- | | ||
| | `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | | ||
| | `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | | ||
| | `pkg/primitives/ingress` | Integration | [ingress.md](primitives/ingress.md) | |
There was a problem hiding this comment.
The PR description checklist says "Does not modify shared files", but this change set updates shared framework docs (docs/primitives.md) and the shared editors package (pkg/mutation/editors/ingressspec.go). Please correct the PR description/checklist (or adjust scope) so reviewers aren't misled about the change surface.
| // 4. Configure flavors. | ||
| builder.WithFieldApplicationFlavor(ingress.PreserveCurrentLabels) | ||
| builder.WithFieldApplicationFlavor(ingress.PreserveCurrentAnnotations) | ||
|
|
There was a problem hiding this comment.
WithFieldApplicationFlavor and the referenced flavors (ingress.PreserveCurrentLabels / ingress.PreserveCurrentAnnotations) don't exist anywhere else in the repo (search only finds this example). As written, this example will not compile; either remove these calls or add the corresponding Builder API + flavor definitions in pkg/primitives/ingress (and any underlying generic support) so the example matches the public API.
| // 4. Configure flavors. | |
| builder.WithFieldApplicationFlavor(ingress.PreserveCurrentLabels) | |
| builder.WithFieldApplicationFlavor(ingress.PreserveCurrentAnnotations) |
Implements the
ingressKubernetes resource primitive following the pattern established by the existingConfigMapandDeploymentprimitives.Summary
ingressprimitive package underpkg/primitives/ingress/Checklist