Deprecate the Endpoint resource#1019
Open
afritzler wants to merge 2 commits into
Open
Conversation
github-actions
Bot
added
api-change
documentation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThe PR marks the ChangesEndpoint deprecation contract
Documentation and compatibility support
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/crd/bases/metal.ironcore.dev_endpoints.yaml`:
- Around line 32-37: Keep the Endpoint deprecation text in the API definition in
endpoint_types.go, revert the direct edit to the generated CRD, and run make
manifests to regenerate the corresponding config/crd/bases artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb961332-52ba-4b4b-b893-3e5a55db5562
⛔ Files ignored due to path filters (1)
dist/chart/templates/crd/metal.ironcore.dev_endpoints.yamlis excluded by!**/dist/**
📒 Files selected for processing (10)
.golangci.ymlapi/v1alpha1/applyconfiguration/api/v1alpha1/endpoint.goapi/v1alpha1/endpoint_types.goconfig/crd/bases/metal.ironcore.dev_endpoints.yamldocs/api-reference/api.mddocs/concepts/endpoints.mddocs/concepts/servermaintenance.mddocs/concepts/servers.mddocs/usage/metalctl.mdinternal/webhook/v1alpha1/endpoint_webhook.go
Member
Author
|
@coderabbitai create an issue for remove the |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh issue create \
--repo ironcore-dev/metal-operator \
--title "Remove OwnerReference dependency between Endpoint and BMC" \
--assignee afritzler \
--body "## Summary
Remove the \`OwnerReference\` relationship currently established between \`Endpoint\` and \`BMC\` resources.
## Rationale
With the \`Endpoint\` resource now deprecated (see PR `#1019` and issue `#1018`) in favor of modeling endpoint information inline via \`BMC.spec.access\`, the existing \`OwnerReference\` from \`BMC\` to \`Endpoint\` (or vice versa) creates an unwanted side effect: Kubernetes garbage collection (GC) will delete the \`BMC\` object and all its dependent resources whenever the owning \`Endpoint\` is deleted. This is undesirable going forward since \`Endpoint\` is being phased out and should not be able to cascade-delete \`BMC\` and downstream objects.
## Affected areas
- Controllers/reconcilers that set the \`OwnerReference\` between \`Endpoint\` and \`BMC\` (likely in \`internal/controller\` for the BMC/Endpoint reconciliation logic).
- Any related webhook or validation logic in \`internal/webhook/v1alpha1\`.
## Acceptance criteria
- \`BMC\` objects no longer carry an \`OwnerReference\` pointing to their associated \`Endpoint\`.
- Deleting an \`Endpoint\` no longer triggers garbage collection of the associated \`BMC\` and its subsequent objects.
- Existing tests are updated/added to verify that deleting an \`Endpoint\` does not cascade-delete the \`BMC\`.
## References
- PR: https://github.com/ironcore-dev/metal-operator/pull/1019
- Comment: https://github.com/ironcore-dev/metal-operator/pull/1019#issuecomment (requested by `@afritzler`)
- Related issue: `#1018`
Requested by: `@afritzler`"Length of output: 1929
It captures the rationale (preventing GC from cascading-deleting |
Member
Author
|
@coderabbitai create an issue to remove the Endpoint resource/Ref in BMC and all related controller, webhook and unit tests. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh issue create \
--repo ironcore-dev/metal-operator \
--title "Remove Endpoint resource/Ref from BMC and related controller, webhook, and unit tests" \
--assignee afritzler \
--body "## Summary
Fully remove the \`Endpoint\` resource and its reference (\`endpointRef\`) from \`BMC\`, along with all related controller logic, webhook logic, and unit tests.
## Rationale
The \`Endpoint\` resource was deprecated in PR \`#1019\` (issue \`#1018\`) in favor of modeling endpoint information inline via \`BMC.spec.access\` (an \`InlineEndpoint\` carrying MAC address and IP). Now that the deprecation notice is in place, the next step is to complete the removal of the \`Endpoint\` type, its API, controller, webhook, and the \`endpointRef\` field/usage in \`BMC\`, along with associated unit tests, once consumers have migrated.
## Affected areas
- \`api/v1alpha1/endpoint_types.go\` and its generated apply configuration (\`api/v1alpha1/applyconfiguration/api/v1alpha1/endpoint.go\`)
- \`config/crd/bases/metal.ironcore.dev_endpoints.yaml\`
- \`internal/webhook/v1alpha1/endpoint_webhook.go\` and its unit tests
- Controllers referencing \`Endpoint\`/\`endpointRef\` in \`BMC\` reconciliation (likely under \`internal/controller\`)
- Any RBAC, sample manifests, and generated client code referencing \`Endpoint\`
- Documentation: \`docs/concepts/endpoints.md\`, \`docs/api-reference/api.md\`
- \`.golangci.yml\` staticcheck exclusion rule for \`Endpoint\` deprecation warnings (\`SA1019.*metalv1alpha1\.Endpoint\`), which should be removed once the type itself is removed
## Acceptance criteria
- The \`Endpoint\` CRD, its Go types, generated clients/apply configurations, and webhook are removed from the codebase.
- The \`endpointRef\`/reference to \`Endpoint\` is removed from the \`BMC\` spec/status and its controller logic.
- All related unit tests are removed or updated accordingly.
- Documentation is updated to remove references to the standalone \`Endpoint\` resource, pointing solely to \`BMC.spec.access\`.
- Related follow-up issue \`#1022\` (removing the \`OwnerReference\` dependency between \`Endpoint\` and \`BMC\`) is considered/closed as part of this cleanup if still applicable.
## References
- PR: https://github.com/ironcore-dev/metal-operator/pull/1019
- Related issue: `#1018`
- Related follow-up issue: `#1022`
Requested by: `@afritzler`"Length of output: 2595
It covers removing the |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/webhook/v1alpha1/bmc_webhook.go (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused package-level logger.
Once the webhook validation methods are updated to extract the logger from the context (as required by the coding guidelines), this package-level
bmclogvariable will be unused and should be removed.♻️ Proposed refactor
-// log is for logging in this package. -var bmclog = logf.Log.WithName("bmc-resource")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/webhook/v1alpha1/bmc_webhook.go` around lines 18 - 19, Remove the unused package-level bmclog logger declaration and any now-unneeded import supporting it, after webhook validation methods obtain logging from their context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/webhook/v1alpha1/bmc_webhook.go`:
- Around line 47-65: Update ValidateCreate, ValidateUpdate, and ValidateDelete
to obtain the logger from each method’s context via logf.FromContext(ctx)
instead of using the package-level bmclog, and log active-voice messages
beginning with “Validating BMC” while preserving the existing name field and
validation behavior.
---
Nitpick comments:
In `@internal/webhook/v1alpha1/bmc_webhook.go`:
- Around line 18-19: Remove the unused package-level bmclog logger declaration
and any now-unneeded import supporting it, after webhook validation methods
obtain logging from their context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8417d6e5-785b-4be8-9c55-05a408cb49d7
⛔ Files ignored due to path filters (2)
dist/chart/templates/crd/metal.ironcore.dev_bmcs.yamlis excluded by!**/dist/**dist/chart/templates/webhook/webhooks.yamlis excluded by!**/dist/**
📒 Files selected for processing (9)
.golangci.ymlapi/v1alpha1/applyconfiguration/api/v1alpha1/bmcspec.goapi/v1alpha1/bmc_types.gocmd/main.goconfig/crd/bases/metal.ironcore.dev_bmcs.yamlconfig/webhook/manifests.yamldocs/api-reference/api.mdinternal/webhook/v1alpha1/bmc_webhook.gointernal/webhook/v1alpha1/webhook_suite_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- .golangci.yml
afritzler
force-pushed
the
enh/endpoint-deprecation
branch
3 times, most recently
from
July 16, 2026 13:54
07911bc to
ec71469
Compare
Signed-off-by: Andreas Fritzler <andreas.fritzler@sap.com>
Signed-off-by: Andreas Fritzler <andreas.fritzler@sap.com>
afritzler
force-pushed
the
enh/endpoint-deprecation
branch
from
July 16, 2026 15:48
ec71469 to
97f01a2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
Deprecate the Endpoint resource.
Fixes #1018
Summary by CodeRabbit
Deprecation
Endpointresource andBMC.spec.endpointRefare now marked as deprecated (including CRD/API schema deprecation messaging).BMC.spec.access(using anInlineEndpointwith MAC address and IP).Endpointresources may surface an admission warning; existing deprecated resources remain supported for compatibility.Documentation
metalctl movedry-run instructions formatting.