Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tracking issue: teach controllerbuilder tooling about multiple resources #2802

Open
2 of 5 tasks
acpana opened this issue Sep 26, 2024 · 5 comments
Open
2 of 5 tasks

Comments

@acpana
Copy link
Collaborator

acpana commented Sep 26, 2024

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.

@acpana
Copy link
Collaborator Author

acpana commented Sep 26, 2024

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
Copy link
Collaborator

jingyih commented Sep 26, 2024

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
Copy link
Collaborator

jingyih commented Sep 27, 2024

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
Copy link
Collaborator Author

acpana commented Sep 27, 2024

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:

#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
Copy link
Collaborator

I just created #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants