Skip to content

Implement Direct Controller for NetworkServices LBRouteExtension#6957

Merged
maqiuyujoyce merged 8 commits into
GoogleCloudPlatform:masterfrom
katrielt:networkservices-lbrouteextensions
Apr 9, 2026
Merged

Implement Direct Controller for NetworkServices LBRouteExtension#6957
maqiuyujoyce merged 8 commits into
GoogleCloudPlatform:masterfrom
katrielt:networkservices-lbrouteextensions

Conversation

@katrielt

@katrielt katrielt commented Mar 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Change description

Implemented the direct controller, identity logic, and fixture tests for the NetworkServicesLBRouteExtension resource.

Addresses #6513

WHY do we need this change?

To support NetworkServicesLBRouteExtension using the modern "direct" controller approach, replacing the need for Terraform-based reconciliation for this resource.

Special notes for your reviewer:

  • Manual Mappers: Implemented manual ToProto and FromProto mappers in lbrouteextension_mapper.go to handle the polymorphic service field (which can reference either a BackendService or a WasmPlugin).
  • Reference Normalization: Updated the controller to ensure both ForwardingRuleRef and BackendServiceRef are normalized to full HTTPS URLs before reconciliation.
  • Re-reconciliation Stability: Added logic to the controller to normalize GCP-returned Project Numbers back to Project IDs in URLs. This was necessary to match the KRM "desired" state and prevent infinite re-reconciliation
    loops.
  • Deterministic FieldMask: Explicitly sorted FieldMask paths in the Update method to ensure deterministic updateMask parameters in the recorded HTTP logs.

Tests you have done

  • Verified CRUD cycle against real GCP using hack/record-gcp.
  • Verified parity and stability against MockGCP using hack/compare-mock.
  • All fixture tests under pkg/test/resourcefixture/testdata/basic/networkservices/v1alpha1/networkserviceslbrouteextension/ are
  • Run make ready-pr to ensure this PR is ready for review.
  • Perform necessary E2E testing for changed resources.

@katrielt katrielt requested a review from maqiuyujoyce March 10, 2026 10:20
maqiuyujoyce
maqiuyujoyce reviewed Mar 14, 2026
View reviewed changes
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go Outdated
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go Outdated
Comment thread mockgcp/mocknetworkservices/normalize.go Outdated
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go
@katrielt
Address review comments for LBRouteExtension direct controller
4856c0a 
- Refactor LBRouteExtensionAdapter to defer reference resolution and proto mapping to Create/Update phase, ensuring safe deletion when dependencies are gone.
- Add nil pointer safety checks for ForwardingRuleRefs to prevent controller panics.
- Update the Update() method to always refresh status (externalRef and observedState), enabling correct behavior for resource acquisition.
- Remove redundant updateMask normalization in mock service, as the direct controller now handles deterministic field sorting.
maqiuyujoyce
maqiuyujoyce previously approved these changes Mar 20, 2026
View reviewed changes

@maqiuyujoyce maqiuyujoyce left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/lgtm

The comments are not blockers and feel free to address the important ones in v1beta1 promotion.

Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go Outdated
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go Outdated
@google-oss-prow google-oss-prow Bot removed the lgtm label Mar 22, 2026
@katrielt

katrielt commented Mar 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

I would rather address all nits and comments now, so we get to beta in a better state (Also, I would for sure forget to implement your nits 😄 )

@codebot-robot

codebot-robot commented Mar 24, 2026

Copy link
Copy Markdown
Collaborator

I have created a task to address recent feedback on this PR.

codebot-robot
codebot-robot reviewed Mar 24, 2026
View reviewed changes

@codebot-robot codebot-robot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @katrielt for addressing the feedback and adding the project equivalency check! The recent commits look good and fully resolve the nits and comments. I have verified the changes.

(This comment was generated by Overseer)

maqiuyujoyce
maqiuyujoyce reviewed Mar 25, 2026
View reviewed changes
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go Outdated
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go Outdated
Comment thread pkg/controller/direct/networkservices/lbrouteextension_controller.go
katrielt added 2 commits March 25, 2026 08:31
@katrielt
networkservices: address review feedback for LBRouteExtension
2f5ce3f 
- Move project ID extraction to common utility with unit tests.
- Relocate GCP label handling to AdapterForObject for consistency.
- Strengthen project ID validation for resource references.
- Store desiredProto in the adapter to streamline reconciliation logic.
@katrielt
run make fmt
5f551bd
maqiuyujoyce
maqiuyujoyce approved these changes Apr 9, 2026
View reviewed changes

@maqiuyujoyce maqiuyujoyce left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/lgtm

@maqiuyujoyce maqiuyujoyce added this pull request to the merge queue Apr 9, 2026
@google-oss-prow google-oss-prow Bot added the lgtm label Apr 9, 2026
@google-oss-prow

google-oss-prow Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maqiuyujoyce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Merged via the queue into GoogleCloudPlatform:master with commit 90723ea Apr 9, 2026
166 checks passed
@katrielt katrielt deleted the networkservices-lbrouteextensions branch April 9, 2026 06:47
@cheftako cheftako added this to the 1.149 milestone Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maqiuyujoyce maqiuyujoyce maqiuyujoyce approved these changes

+1 more reviewer

@codebot-robot codebot-robot codebot-robot left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@maqiuyujoyce maqiuyujoyce

Labels

approved lgtm overseer

Projects

None yet

Milestone

1.149

Development

Successfully merging this pull request may close these issues.

4 participants

@katrielt @codebot-robot @maqiuyujoyce @cheftako