tracking issue: teach controllerbuilder tooling about multiple resources

GoogleCloudPlatform / k8s-config-connector

GCP Config Connector, a Kubernetes add-on for managing GCP resources

https://cloud.google.com/config-connector/docs/overview

Apache License 2.0

902 stars 235 forks source link

tracking issue: teach controllerbuilder tooling about multiple resources #2802

Open acpana opened 2 months ago

acpana commented 2 months ago

I am working on another resource (Listing) in the same api group (BigQueryAnalyticsHub). I'm going to write down papercuts and changes that are needed for our tooling to support more than one resource per api group easily.

[x] controller name should have the resource kind in it: ✅ https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/2791
[ ] generate-type cmd needs to place types in their own folder: https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/2809
[ ] generate-mapper cmd needs to place types in their own folder
[x] add cmd needs to place types in their own folder: ✅ https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/2851
[ ] refactor "old" / previous direct resource to follow the new folder structure

acpana commented 2 months ago

generate-type cmd needs to place types in their own folder

 go run main.go generate-types \                               
     --service google.cloud.bigquery.analyticshub.v1  \
     --proto-source-path ../proto-to-mapper/build/googleapis.pb \
     --output-api $REPO_ROOT/apis \
     --kind BigQueryAnalyticsHubDataExchangeListing \
     --proto-resource Listing \
     --api-version "bigqueryanalyticshub.cnrm.cloud.google.com/v1alpha1"

effectively replaces the types.generated.go file which we probably want to have live on a per resource folder level. The final folder structure can look something like:

serviceName
 ├─resourceA
 └─resourceB
    ├─version1
    │  └─types.generated.go
    └─version2

jingyih commented 2 months ago

Alternative approach: rather than running "generate-types" for each individual resource, we could modify the command to handle multiple resources in a single run. This would result in the generated types for all resources (in the same API group) being combined into one "types.generated.go" file.

jingyih commented 2 months ago

After thinking more about this issue, my preference is to use a single "types.generated.go" for a specific API and version. (e.g. google.cloud.bigquery.analyticshub.v1)

It prevents generating duplicated dependent resources shared between resource A and B.
It makes some special cases easier to handle, such as the case where resource A is a nested message of resource B

It better aligns with the structure of the API's proto

serviceName
├─version1
└─version2
├─resourceA_types.go
├─resourceB_types.go
└─types.generated.go

acpana commented 2 months ago

hey @jingyih thanks for engaging with this! I think we're on the same page modulo the folder structure. Here's the PR for what I had in mind:

https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/2809

The work for generate-type cmd needs to place types in their own folder is mostly orthogonal to

we could modify the command to handle multiple resources in a single run.

We can add that as a follow on.

jasonvigil commented 2 months ago

I just created https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/2819, which bundles the types for WorkstationCluster and WorkstationConfig into the same package. Did this manually for now. Seems they do share a type, which I placed into shared_types.go. Any reason we would prefer to split into separate packages @acpana? Seems sort of nice to be able to share types between the resources in the same service.