Greenfield: Implement direct controller, E2E fixtures, and fuzzer for ContactCenterInsightsConversation - #12432
Conversation
ada-coder-bot
added
overseer
greenfield
Real GCP Testing Verification (Audit Logs)Below are the Google Cloud audit logs retrieved using insertId: 1kdmqr4e1i3as
logName: projects/cnrm-barni-4/logs/cloudaudit.googleapis.com%2Factivity
operation:
first: true
id: operations/noop.DONE_OPERATION
last: true
producer: serviceusage.googleapis.com
protoPayload:
'@type': type.googleapis.com/google.cloud.audit.AuditLog
authenticationInfo:
principalEmail: overseer-kcc-tester@cnrm-barni-4.iam.gserviceaccount.com
authorizationInfo:
- granted: true
permission: serviceusage.services.enable
permissionType: ADMIN_WRITE
resource: projectnumbers/600845353393/services/contactcenterinsights.googleapis.com
methodName: google.api.serviceusage.v1.ServiceUsage.EnableService
request:
'@type': type.googleapis.com/google.api.serviceusage.v1.EnableServiceRequest
name: projects/cnrm-barni-4/services/contactcenterinsights.googleapis.com
response:
'@type': type.googleapis.com/google.api.serviceusage.v1.EnableServiceResponse
service:
config:
name: contactcenterinsights.googleapis.com
title: Contact Center AI Insights API
state: ENABLED
serviceName: serviceusage.googleapis.com
status: {}
severity: NOTICE
timestamp: '2026-08-16T03:16:25.352157Z' |
|
[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. |
beb3d79 to
e3482ae
Compare
Investigating fuzz-roundtrippers-2-of-8 failureRun: 31926827593 (https://github.com/GoogleCloudPlatform/k8s-config-connector/actions/runs/31926827593/job/95115607775) Investigating presubmit-gatekeeper failureRun: 31926827593 (https://github.com/GoogleCloudPlatform/k8s-config-connector/actions/runs/31926827593/job/95117002057) (This report was generated by overseer) |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
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: Fail - KRM status update is skipped on no-op updates when returning early on line 226.
- LRO Wait: Pass (not applicable)
- Test Coverage: Pass
Detailed Findings / Actions Required:
- In
ccinsightsconversation_controller.goon line 226, returningnildirectly when there is no diff skips the status update. The controller must always update status duringUpdatereconciliation even when no-op, to ensure the latest observed state and external references are properly reflected in the KRM object status.
|
|
||
| if !diffs.HasDiff() { | ||
| log.V(2).Info("no field needs update", "id", a.id) | ||
| return nil |
There was a problem hiding this comment.
Returning nil directly here when there is no diff skips the status update.
To comply with Rule 3 (Mandatory Status Updates) of the greenfield direct controller instructions, you should always invoke a.updateStatus(ctx, updateOp, a.actual) instead of returning nil. This ensures that status fields (e.g., observed state, conditions, external references) are correctly synchronized even during no-op reconciliations.
|
🤖 AI Factory started addressing review feedback for this pull request. |
…erInsightsConversation This PR implements the direct controller for ContactCenterInsightsConversation, registers it as the default reconciler in static_config.go, adds the fuzzer registration, and includes both minimal and maximal E2E fixtures. Fix: 11414
e3482ae to
ba051a4
Compare
|
Hi @reviewbot-robot, Thank you for the review! I have addressed the feedback:
I have updated the commit and force-pushed the changes to this branch. (This comment was generated by overseer) |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
reviewbot-robot
left a comment
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 (Fields mapped using
common.DiffForTopLevelFields) - Structured Reporting: Pass (
structuredreporting.ReportDiff(ctx, diffs)called before update) - KRM Status Updates: Pass (
updateStatusinvoked at the end of bothCreateandUpdate) - LRO Wait: Pass (No LROs returned by the
CreateorUpdateendpoints of this resource) - Test Coverage: Pass (Minimal and maximal test cases are provided, but with hardcoded project ID bucket dependencies)
The Greenfield controller implementation for CCInsightsConversation looks solid, structured, and compliant with KCC direct controller patterns. However, there are a few issues that need to be addressed before merging, particularly the hardcoded project ID in the bucket paths of the test fixtures which will break test execution/recording on other projects.
| medium: CHAT | ||
| dataSource: | ||
| gcsSource: | ||
| transcriptURI: gs://cnrm-barni-4-ccinsights-test-bucket-2026/transcript.json |
There was a problem hiding this comment.
The GCS bucket URI is hardcoded to a specific project (cnrm-barni-4-ccinsights-test-bucket-2026). Please use the parameterized project variable ${projectId} instead (e.g., gs://${projectId}-ccinsights-test-bucket-2026/transcript.json) to allow tests to run cleanly on other GCP projects without permission/existence errors.
| medium: CHAT | ||
| dataSource: | ||
| gcsSource: | ||
| transcriptURI: gs://cnrm-barni-4-ccinsights-test-bucket-2026/transcript.json |
There was a problem hiding this comment.
The GCS bucket URI is hardcoded to a specific project (cnrm-barni-4-ccinsights-test-bucket-2026). Please use the parameterized project variable ${projectId} instead (e.g., gs://${projectId}-ccinsights-test-bucket-2026/transcript.json) to allow tests to run cleanly on other GCP projects without permission/existence errors.
| medium: CHAT | ||
| dataSource: | ||
| gcsSource: | ||
| transcriptURI: gs://cnrm-barni-4-ccinsights-test-bucket-2026/transcript.json |
There was a problem hiding this comment.
The GCS bucket URI is hardcoded to a specific project (cnrm-barni-4-ccinsights-test-bucket-2026). Please use the parameterized project variable ${projectId} instead (e.g., gs://${projectId}-ccinsights-test-bucket-2026/transcript.json) to allow tests to run cleanly on other GCP projects without permission/existence errors.
| clonedDesired.Expiration = maskedActual.Expiration | ||
| } | ||
|
|
||
| // The following fields are immutable on GCP, so we must copy them from actual to desired |
There was a problem hiding this comment.
Silently copying user-configurable immutable fields (ObfuscatedUserId, DataSource, Medium, Metadata) from actual to desired during updates avoids update mask additions, but silently ignores user modifications to these fields, resulting in a permanent drift in the KRM Spec that is never corrected or reported as an error.
Consider checking if any of these immutable fields have changed between clonedDesired and maskedActual before copying them, and returning an explicit error (e.g., fmt.Errorf("cannot update immutable field ...")) to the user to maintain KRM configuration integrity.
|
🤖 AI Factory started addressing review feedback for this pull request. |
|
🤖 AI Factory started reviewing this pull request in a sandbox. |
4 participants
Greenfield: Implement direct controller, E2E fixtures, and fuzzer for ContactCenterInsightsConversation
This Pull Request implements the direct controller, E2E fixtures (both minimal and maximal), and the fuzzer for
ContactCenterInsightsConversation(CCInsightsConversationv1alpha1).Fixes #11414
Implementation Details
pkg/controller/direct/contactcenterinsights/ccinsightsconversation/.teamempty and populatesteamsarray instead, which our compiled Go library does not model yet). Handled by normalizingTeamwhen they match.ObfuscatedUserId,DataSource,Medium, andMetadata) from the actual server state to the desired state during comparisons to prevent false update mask additions.Verification & Testing
record-gcpwas run successfully against real GCP on projectcnrm-barni-4to capture traffic and golden files for both the minimal and maximal fixtures.go test -v ./pkg/fuzztesting/fuzztests/... -run TestSomeMappers.go test ./pkg/crd/template/...andgo test -v ./tests/apichecks/... -run TestCRDFieldPresenceInTestsForAlpha. All checks passed.This PR was generated by the overseer,overseer,greenfield,step/controller agent (powered by the gemini-3.5-flash model).