Skip to content

Greenfield: Implement direct KRM types, identity, and generate.sh for SaaSServiceMgmtTenant - #13623

Open
lovelace-coder-bot wants to merge 1 commit into
GoogleCloudPlatform:masterfrom
lovelace-coder-bot:issue-13604-1790893875
Open

lovelace-coder-bot wants to merge 1 commit into
GoogleCloudPlatform:masterfrom
lovelace-coder-bot:issue-13604-1790893875

Conversation

@lovelace-coder-bot

Copy link
Copy Markdown
Collaborator

Description

This PR implements direct KRM types, identity, reference, and generate.sh for the SaaSServiceMgmtTenant greenfield resource (saasservicemgmt.cnrm.cloud.google.com/v1alpha1).

Key Changes

  1. Types & References:
    • Added KRM types for SaaSServiceMgmtTenantSpec and SaaSServiceMgmtTenantObservedState in apis/saasservicemgmt/v1alpha1/saasservicemgmttenant_types.go.
    • Added external-only reference type SaaSServiceMgmtSaaSRef in apis/saasservicemgmt/v1alpha1/saasservicemgmtsaas_reference.go and identity SaaSServiceMgmtSaaSIdentity in apis/saasservicemgmt/v1alpha1/saasservicemgmtsaas_identity.go.
  2. Identity & Reference:
    • Implemented SaaSServiceMgmtTenantIdentity matching the CAIS URL format projects/{project}/locations/{location}/tenants/{tenant} in apis/saasservicemgmt/v1alpha1/saasservicemgmttenant_identity.go.
    • Implemented SaaSServiceMgmtTenantRef in apis/saasservicemgmt/v1alpha1/saasservicemgmttenant_reference.go.
    • Added comprehensive unit tests in saasservicemgmttenant_identity_test.go and saasservicemgmtsaas_identity_test.go.
  3. Scaffolding & CRD Generation:
    • Added --resource SaaSServiceMgmtTenant:Tenant to apis/saasservicemgmt/generate.sh.
    • Generated CRD manifest, deepcopy code, and proto mappers.
    • Updated naming_violations.txt and alpha-missingfields.txt.

Fixes #13604

NONE

This PR was generated by the overseer,step/gen-types,greenfield,overseer/review agent (powered by the gemini-3.7-flash model).

@lovelace-coder-bot lovelace-coder-bot added overseer step/gen-types greenfield Indicates implementation of a new resource (vs migration) overseer/review labels Oct 2, 2026
@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign barney-s for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@argus-watcher-bot

Copy link
Copy Markdown
Collaborator

🤖 AI Factory started reviewing this pull request in a sandbox.

@reviewbot-robot reviewbot-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.

KCC Auto-Review Results

  • Trigger criteria matched: Yes
  • API Version Check: Pass - (CRD and Go types implemented under apis/saasservicemgmt/v1alpha1 with version v1alpha1)
  • Go Type Pointers: Pass - (All scalar primitives in SaaSServiceMgmtTenantSpec, SaaSServiceMgmtTenantObservedState, and Location are pointers; non-primitives and slices follow KCC conventions)
  • Completeness & Heuristics: Pass - (100% field coverage of the google.cloud.saasplatform.saasservicemgmt.v1beta1.Tenant proto definition, proper split between Spec and ObservedState)
  • 1:1 Kind to Proto Mapping: Pass - (SaaSServiceMgmtTenant maps 1:1 to proto Tenant)
  • References/Identity: Pass - (SaaSServiceMgmtTenantIdentity, SaaSServiceMgmtTenantRef, and SaaSServiceMgmtSaaSRef implemented with canonical format templates and validations)
  • Exception Files: Fail - (tests/apichecks/testdata/exceptions/naming_violations.txt modified)

Detailed Findings / Actions Required:

  1. tests/apichecks/testdata/exceptions/naming_violations.txt: Greenfield PRs must not modify exception files other than tests/apichecks/testdata/exceptions/alpha-missingfields.txt.
  2. apis/saasservicemgmt/v1alpha1/saasservicemgmtsaas_reference.go: Consider implementing ParseExternalToIdentity() (identity.Identity, error) on SaaSServiceMgmtSaaSRef to satisfy refs.ExternalRef.
[naming_violation] file=apis/refs/dataprocmetastoreservice_reference_test.go prefix=dataprocmetastoreservice (expected a valid resource kind prefix)
[naming_violation] file=apis/refs/organization_reference.go prefix=organization (expected a valid resource kind prefix)
[naming_violation] file=apis/run/v1beta1/revision_reference.go prefix=revision (expected a valid resource kind prefix)
[naming_violation] file=apis/saasservicemgmt/v1alpha1/saasservicemgmtsaas_identity.go prefix=saasservicemgmtsaas (expected a valid resource kind prefix)

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.

Greenfield PRs must not add exceptions to any exception files other than tests/apichecks/testdata/exceptions/alpha-missingfields.txt. Please verify if naming_violations.txt changes can be avoided or if the naming test needs adjustment.

var _ refsv1beta1.Ref = &SaaSServiceMgmtSaaSRef{}
var SaaSServiceMgmtSaaSGVK = GroupVersion.WithKind("SaaSServiceMgmtSaaS")

// SaaSServiceMgmtSaaSRef is a reference to a GCP SaaSServiceMgmtSaaS.

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.

Consider implementing ParseExternalToIdentity() (identity.Identity, error) on SaaSServiceMgmtSaaSRef so that it satisfies the refs.ExternalRef interface, matching SaaSServiceMgmtTenantRef and standard reference implementations across KCC.

@argus-watcher-bot

Copy link
Copy Markdown
Collaborator

🤖 AI Factory started addressing review feedback for this pull request.

… SaaSServiceMgmtTenant

Implement direct KRM types, identity, reference, and generate.sh for SaaSServiceMgmtTenant
(saasservicemgmt.cnrm.cloud.google.com/v1alpha1).

Issue: GoogleCloudPlatform#13604
@lovelace-coder-bot

Copy link
Copy Markdown
Collaborator Author

Thanks for the review! I have addressed all the feedback:

  1. Reverted naming_violations.txt: Moved SaaSServiceMgmtSaaSRef and SaaSServiceMgmtSaaSIdentity into apis/saasservicemgmt/v1alpha1/saasservicemgmttenant_reference.go and removed the standalone saasservicemgmtsaas_* files. This eliminated the naming violations and allowed restoring tests/apichecks/testdata/exceptions/naming_violations.txt to its original state.
  2. Implemented ParseExternalToIdentity(): Added ParseExternalToIdentity() (identity.Identity, error) on SaaSServiceMgmtSaaSRef so that it fully satisfies the refs.ExternalRef interface.
  3. Updated unit tests in saasservicemgmttenant_identity_test.go to test SaaSServiceMgmtSaaSIdentity and SaaSServiceMgmtSaaSRef.ParseExternalToIdentity().

(This comment was generated by overseer)

@argus-watcher-bot

Copy link
Copy Markdown
Collaborator

🤖 AI Factory started investigating CI check failures for this pull request.

Note: We recommend waiting for the 'ready-for-human' label before leaving review comments. Comments added while the system is actively working may be associated with outdated commits once a new commit is pushed, causing them to be ignored.

@lovelace-coder-bot

Copy link
Copy Markdown
Collaborator Author

Investigating tests-e2e-direct-iam failure

Run: 36957375530
Name: tests-e2e-direct-iam
Cause: Infrastructure
Details: The job failed with unable to start control plane itself: failed to start the controlplane. retried 5 times: fork/exec .../.build/envtest-bin/etcd: text file busy during TestAllInSeries/fixtures/pkg/test/resourcefixture/testdata/basic/iam/v1beta1/iamserviceaccountkey/iamserviceaccountkey. This is an environmental issue where the etcd binary was temporarily locked by another process on the CI runner, unrelated to the PR changes which only affect saasservicemgmt.
Action Taken: Rerun triggered

Investigating tests-e2e-fixtures-networksecurity failure

Run: 36957375530
Name: tests-e2e-fixtures-networksecurity
Cause: Flake
Details: The job failed in networksecuritysecurityprofile/securityprofile-customintercept due to an HTTP log mismatch (mismatched number of HTTP calls: want 4, got 3 on GetInterceptEndpointGroup) during resource deletion cleanup. This is an asynchronous polling timing flake in mockgcp and is unrelated to the PR changes which only affect saasservicemgmt.
Action Taken: Rerun triggered

(This report was generated by overseer)

@argus-watcher-bot

Copy link
Copy Markdown
Collaborator

🤖 AI Factory started reviewing this pull request in a sandbox.

@reviewbot-robot reviewbot-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.

KCC Auto-Review Results

  • Trigger criteria matched: Yes
  • API Version Check: Pass - (apis/saasservicemgmt/v1alpha1/, CRD version v1alpha1)
  • Go Type Pointers: Pass - (All primitive scalar fields including Location use pointer types)
  • Completeness & Heuristics: Pass - (All proto fields for Tenant are mapped to spec, status.observedState, or KRM metadata)
  • 1:1 Kind to Proto Mapping: Pass - (SaaSServiceMgmtTenant strictly maps 1:1 to google.cloud.saasplatform.saasservicemgmt.v1beta1.Tenant)
  • References/Identity: Pass - (SaasRef, ProjectRef, SaaSServiceMgmtTenantIdentity, and SaaSServiceMgmtSaaSIdentity follow gcpurls.Template pattern and use refs.Normalize)
  • Exception Files: Pass - (Only tests/apichecks/testdata/exceptions/alpha-missingfields.txt was updated)

Detailed Findings / Actions Required:

None. The implementation strictly adheres to all greenfield type conventions and validation checks.

@argus-watcher-bot

Copy link
Copy Markdown
Collaborator

🤖 AI Factory started addressing review feedback for this pull request.

@lovelace-coder-bot

Copy link
Copy Markdown
Collaborator Author

Thank you for the review! All review checks have passed and no additional changes are required. Unit tests and API checks have been verified.

(This comment was generated by overseer)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

greenfield Indicates implementation of a new resource (vs migration) overseer/ready-for-human overseer step/gen-types

3 participants