Greenfield: Implement direct controller, E2E fixtures, and fuzzer for VertexAICustomJob - #12431
Greenfield: Implement direct controller, E2E fixtures, and fuzzer for VertexAICustomJob#12431ada-coder-bot wants to merge 2 commits into
Conversation
Live GCP Testing Audit LogsHere are the retrieved Google Cloud audit logs proving that our direct controller successfully hit the real GCP endpoints during end-to-end testing of ---
insertId: b5ewcde5u7vk
logName: projects/cnrm-barni-4/logs/cloudaudit.googleapis.com%2Factivity
protoPayload:
'@type': type.googleapis.com/google.cloud.audit.AuditLog
authenticationInfo:
principalEmail: overseer-kcc-tester@cnrm-barni-4.iam.gserviceaccount.com
authorizationInfo:
- granted: true
permission: aiplatform.customJobs.create
permissionType: ADMIN_WRITE
resource: projects/cnrm-barni-4/locations/us-central1
methodName: google.cloud.aiplatform.v1beta1.JobService.CreateCustomJob
request:
'@type': type.googleapis.com/google.cloud.aiplatform.v1beta1.CreateCustomJobRequest
customJob:
jobSpec:
workerPoolSpecs:
- containerSpec:
command:
- sh
- -c
- echo hello world
imageUri: alpine:latest
machineSpec:
machineType: n1-standard-4
replicaCount: '1'
parent: projects/cnrm-barni-4/locations/us-central1
response:
'@type': type.googleapis.com/google.cloud.aiplatform.v1beta1.CustomJob
name: projects/600845353393/locations/us-central1/customJobs/3877150412133892096
state: JOB_STATE_PENDING
serviceName: aiplatform.googleapis.com
status: {}
severity: NOTICE
timestamp: '2026-08-16T03:24:34.959831610Z' |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
🤖 AI Factory started investigating CI check failures for this pull request. |
e44ee05 to
eafd008
Compare
Investigating test-fuzz-random-mappers failureRun: 31926482399 Investigating fuzz-roundtrippers-2-of-8 failureRun: 31926482399 Investigating fuzz-roundtrippers-3-of-8 failureRun: 31926482399 Investigating presubmit-gatekeeper failureRun: 31926482399 Investigating unit-tests-2-of-4 failureRun: 31926482399 Summary of Fixes Applied:
All fixes have been validated locally and successfully passed fuzzing verification tests. The branch has been force-pushed to origin. (This report was generated by overseer) |
|
🤖 AI Factory started investigating CI check failures for this pull request. |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
There was a problem hiding this comment.
KCC Auto-Review Results
- Trigger criteria matched: Yes
- Proto Diffs & Update Mask: Pass (VertexAICustomJob is immutable and correctly rejects updates when differences are detected)
- Structured Reporting: Pass (Not applicable as updates are blocked due to immutability)
- KRM Status Updates: Pass (Status updates are correctly invoked at the end of both create and update operations)
- LRO Wait: Pass (Deletion correctly waits for the Long Running Operation to finish; creation is synchronous)
- Test Coverage: Pass (Both minimal and maximal test cases are provided)
Detailed Findings / Actions Required:
- Critical Bug in Identity Mapping: Because
VertexAICustomJobnames/IDs are completely server-assigned on GCP and cannot be user-specified, settingstatus.ExternalRefto the user-specified IDa.id.String()causes subsequentFindcalls to query GCP with an invalid name format, resulting in400 Bad Request. Sincedirectbase_controller.gocurrently ignores non-dependency errors returned fromFind, this creates a silent failure loop where the controller thinks the resource does not exist and callsCreateon every single reconciliation loop. This can lead to massive resource duplication and billing in production. - GetIdentity Correction: To support server-assigned IDs,
GetIdentityinapis/vertexai/v1alpha1/vertexaicustomjob_identity.gomust be updated to return the parsedstatusIdentityas the authoritative identity whenStatus.ExternalRefis set, rather than failing on the mismatch between the spec name and the server-assigned ID.
| if mapCtx.Err() != nil { | ||
| return mapCtx.Err() | ||
| } | ||
| status.ExternalRef = direct.LazyPtr(a.id.String()) |
There was a problem hiding this comment.
Vertex AI Custom Jobs use system-generated numeric IDs rather than user-specified resource IDs. Thus, a.id.String() refers to an invalid/non-existent name format on GCP.
Set status.ExternalRef to the actual server-assigned name from GCP, i.e., direct.LazyPtr(created.Name).
Additionally, since apis/vertexai/v1alpha1/vertexaicustomjob_identity.go currently checks that statusIdentity.String() == specIdentity.String(), you must update GetIdentity in that file to only validate mutable parts (like project/location) and return statusIdentity as the authoritative identity when Status.ExternalRef is present. Otherwise, subsequent reconciliations and deletions will fail.
| if mapCtx.Err() != nil { | ||
| return mapCtx.Err() | ||
| } | ||
| status.ExternalRef = direct.LazyPtr(a.id.String()) |
There was a problem hiding this comment.
Ensure status.ExternalRef is set to the true server-assigned GCP resource name using direct.LazyPtr(latest.Name) instead of a.id.String().
|
🤖 AI Factory started addressing review feedback for this pull request. |
Summary of Fixes AppliedI have successfully addressed the review feedback by correcting the identity mapping and external reference handling for
All local formatting, |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
There was a problem hiding this comment.
KCC Auto-Review Results
- Trigger criteria matched: Yes
- Proto Diffs & Update Mask: Pass - Diffs are calculated using
common.DiffForTopLevelFields(the canonical modern replacement for the deprecatedCompareProtoMessage). Since the entire resource is immutable on GCP, any detected diff correctly results in an immutability error and rejects the update. - Structured Reporting: Pass - No actual update requests are sent to GCP due to immutability, so
ReportDiffis not applicable. - KRM Status Updates: Pass - Status updates are performed at the end of both
CreateandUpdateoperations. - LRO Wait: Pass - Deletion properly waits for the LRO returned by
DeleteCustomJobto complete. - Test Coverage: Pass - Both minimal (
vertexaicustomjob-minimal) and maximal (vertexaicustomjob-maximal) test fixtures are present with complete HTTP logs and object status golden files recorded.
Detailed Findings / Actions Required:
The implementation is highly complete, idiomatic, and adheres to all Config Connector greenfield direct controller design guidelines. No bugs or issues were identified.
|
🤖 AI Factory started addressing review feedback for this pull request. |
1d549c3 to
221acf7
Compare
|
I have successfully addressed all of the review comments and verified the implementation:
(This comment was generated by overseer) |
|
🤖 AI Factory started investigating CI check failures for this pull request. |
Investigating tests-e2e-fixtures-networkconnectivity failureRun: 31963898566 This caused subsequent failures starting the control plane with Action Taken: Rerun triggered. Investigating presubmit-gatekeeper failureRun: 31963898566 (This report was generated by overseer) |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
There was a problem hiding this comment.
KCC Auto-Review Results
- Trigger criteria matched: Yes
- Proto Diffs & Update Mask: Pass -
VertexAICustomJobis an immutable resource, meaning updates to its spec are not supported. The adapter correctly computes the diff usingcommon.DiffForTopLevelFieldsand safely enforces immutability. - Structured Reporting: Fail - The controller is missing a call to
structuredreporting.ReportDiffwhen rejecting updates due to mutations on immutable fields. - KRM Status Updates: Pass - The controller successfully ensures that KRM status (including
ObservedState,ExternalRef, etc.) is updated duringCreateand at the end ofUpdate. - LRO Wait: Pass - Create/Update operations do not return LROs on Vertex AI. Delete operation correctly waits for the returned LRO using
op.Wait(ctx). - Test Coverage: Pass - Highly robust test coverage with both minimal and maximal basic test fixtures, a hand-written roundtrip fuzzer, and dedicated identity mapping unit tests.
Detailed Findings / Actions Required:
- In
pkg/controller/direct/vertexai/vertexaicustomjob_controller.go(line 183): When detecting speculative updates on immutable fields, the adapter should associate the diff with the unstructured object and report it viastructuredreporting.ReportDiffbefore returning the immutability error.
|
🤖 AI Factory started addressing review feedback for this pull request. |
… VertexAICustomJob This PR implements the direct reconciliation controller, roundtrip fuzzer tests, and live recorded E2E fixtures for the VertexAICustomJob resource. Changes: - Created the direct controller vertexaicustomjob_controller.go under pkg/controller/direct/vertexai/ to support CRUD operations on CustomJob using the modern, isolated direct controller pattern and REST client. - Hand-wrote FuzzVertexAICustomJob and registered it to run during package roundtrip verification, ensuring perfect lossless conversion between KRM and protobuf. - Created vertexaicustomjob_fuzzer.go registering Spec/Status fields for KRM fuzz testing. - Registered VertexAICustomJob as a direct controller in static_config.go. - Added unversioned mapper wrappers to mappers.go. - Scaffolded both minimal and maximal E2E test fixtures, and recorded live HTTP traffic and object golden files against real GCP. Fixes GoogleCloudPlatform#11866
221acf7 to
b00c45a
Compare
|
I have successfully addressed the review feedback by reporting speculative/unsupported updates on immutable fields using the structured diff reporting utility:
(This comment was generated by overseer) |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
There was a problem hiding this comment.
KCC Auto-Review Results
- Trigger criteria matched: Yes
- Proto Diffs & Update Mask: Pass (Diff is correctly calculated and used to enforce immutability; updates are rejected if diffs are found, as Vertex AI CustomJobs are immutable)
- Structured Reporting: Pass (ReportDiff is correctly called before immutability rejection)
- KRM Status Updates: Pass (Status updates are correctly performed at the end of both Create and Update)
- LRO Wait: Pass (Delete is awaited correctly; Create returns the completed resource immediately rather than an LRO)
- Test Coverage: Pass (Both minimal and maximal live-recorded test cases are provided)
Overall, the implementation is solid, follows the correct Greenfield direct controller patterns, and the test suite has excellent coverage. The only finding is to ensure that any IAM service account reference is explicitly resolved in the adapter, as standard normalization skips it.
|
🤖 AI Factory started addressing review feedback for this pull request. |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
There was a problem hiding this comment.
KCC Auto-Review Results
- Trigger criteria matched: Yes
- Proto Diffs & Update Mask: Pass -
VertexAICustomJobis immutable. The controller compares desired and actual states usingcommon.DiffForTopLevelFields, which is the canonical and modern method for structured proto message diffing. - Structured Reporting: Pass - Speculative updates to the immutable custom job correctly leverage
structuredreporting.ReportDiff(ctx, diffs)to communicate the spec difference to the user before returning an immutable field error. - KRM Status Updates: Pass - KRM status (including ObservedState and ExternalRef) is fully updated during both Create and Update reconciled states.
- LRO Wait: Pass - Deletion of custom jobs returns a Long Running Operation (LRO) and is properly awaited using
op.Wait(ctx). - Test Coverage: Pass - Both minimal and maximal E2E test fixtures exist and have recorded golden traffic. Fuzz testing and round-trip verification are fully integrated and run successfully.
Detailed Findings / Actions Required:
No major issues, bugs, or regressions were found. The direct controller implementation is exceptionally clean, fully compliant withConfig Connector's design patterns, and follows the latest engineering and architecture standards.
|
🤖 AI Factory started addressing review feedback for this pull request. |
|
I have reviewed the feedback and confirmed that:
(This comment was generated by overseer) |
|
🤖 AI Factory started investigating CI check failures for this pull request. |
| if diffs.HasDiff() { | ||
| diffs.Object = updateOp.GetUnstructured() | ||
| structuredreporting.ReportDiff(ctx, diffs) | ||
| return fmt.Errorf("VertexAICustomJob %s is immutable and cannot be updated", a.id) |
There was a problem hiding this comment.
Remove this line. Don't return error. Reporting error is good enough.
|
🤖 AI Factory started addressing review feedback for this pull request. |
5206ef1 to
1777e62
Compare
|
I have successfully addressed the review feedback by removing the error return when a diff is detected on immutable fields for All formatting, (This comment was generated by overseer) |
|
🤖 AI Factory started investigating CI check failures for this pull request. |
Investigating golangci-lint failureRun: 32332491587 Investigating presubmit-gatekeeper failureRun: 32332491587 (This report was generated by overseer) |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
There was a problem hiding this comment.
KCC Auto-Review Results
- Trigger criteria matched: Yes
- Proto Diffs & Update Mask: Pass
- Structured Reporting: Pass
- KRM Status Updates: Pass
- LRO Wait: Pass
- Test Coverage: Pass
Detailed Findings / Actions Required:
No actionable findings, bugs, performance concerns, or security issues were found in the implementation. The direct controller, tests, fuzzer, and E2E fixtures fully conform to Config Connector development standards.
|
🤖 AI Factory started addressing review feedback for this pull request. |
|
I have successfully addressed the review feedback:
(This comment was generated by overseer) |
This PR implements the direct reconciliation controller, roundtrip fuzzer tests, and live recorded E2E fixtures for the
VertexAICustomJobresource.Changes
pkg/controller/direct/vertexai/vertexaicustomjob_controller.gousing the modern, isolated direct controller pattern and REST client.FuzzVertexAICustomJobinpkg/controller/direct/vertexai/roundtrip_test.goand verified correct KRM <=> Protobuf conversion.pkg/controller/direct/vertexai/vertexaicustomjob_fuzzer.go.VertexAICustomJobinpkg/controller/resourceconfig/static_config.go.pkg/controller/direct/vertexai/mappers.go.GCP Recording Details
record-gcpwas successfully run against real GCP.cnrm-barni-4./hack/record-gcp "fixtures/^vertexaicustomjob-minimal$"./hack/record-gcp "fixtures/^vertexaicustomjob-maximal$"Fixes #11866
This PR was generated by the overseer,overseer,greenfield,step/controller agent (powered by the gemini-3.5-flash model).