chore: Support nested attributes with same name avoiding generation of duplicate model names #3303

AgustinBettati · 2025-04-25T12:35:22Z

Description

Link to any related issue(s): CLOUDP-315376

Problem: In the case of Stream Instance and Cluster resource we encountered compilation errors when generating resources. This is due to the schemas having nested attributes with same name present in different nested objects.
Example: Cluster resource generated TFComputeModel twice as compute nested attribute is defined both in auto_scaling and analytics_auto_scaling.

Solution: All nested models have the parent model name as part of their name, ensuring no duplicate models. For case of Compute model we now have TFReplicationSpecsRegionConfigsAnalyticsAutoScalingComputeModel and TFReplicationSpecsRegionConfigsAutoScalingComputeModel.

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

github-actions · 2025-04-25T12:53:11Z

APIx bot: a message has been sent to Docs Slack channel

EspenAlbert

Are these the same objects in the API Spec?
Why not re-use?

EspenAlbert · 2025-04-25T13:02:59Z

internal/service/clusterapi/resource_schema.go

+}
+type TFReplicationSpecsRegionConfigsAutoScalingComputeModel struct {
+	Enabled           types.Bool   `tfsdk:"enabled"`
+	MaxInstanceSize   types.String `tfsdk:"max_instance_size" autogen:"omitjson"`


Why is this omitjson?

Yes this is a bug for sure, either inconsistency in the API Spec or bug in our API Spec parsing logic. Will try to dive deeper next week to see the root cause.

EspenAlbert · 2025-04-25T13:04:10Z

internal/service/streaminstanceapi/resource_schema.go

+				MarkdownDescription: "Label that identifies the cloud service provider where MongoDB Cloud performs stream processing. Currently, this parameter only supports AWS and AZURE.",
+			},
+			"connections": schema.ListNestedAttribute{
+				Computed:            true,


Computed only?

Yes, this one is actually correct in terms of how the API Spec is defined. connections cannot be provided in requests of Stream Instance CRUD however it is present in the responses as read only.

As for our manual resources we decided to not include connections as there are separate resource and data sources for handling connections.

lantoli · 2025-04-25T13:32:15Z

internal/service/clusterapi/resource_schema.go

+			},
+			"mongo_dbmajor_version": schema.StringAttribute{
+				Optional:            true,
+				MarkdownDescription: "MongoDB major version of the cluster. Set to the binary major version. \n\nOn creation: Choose from the available versions of MongoDB, or leave unspecified for the current recommended default in the MongoDB Cloud platform. The recommended version is a recent Long Term Support version. The default is not guaranteed to be the most recently released version throughout the entire release cycle. For versions available in a specific project, see the linked documentation or use the API endpoint for [project LTS versions endpoint](#tag/Projects/operation/getProjectLtsVersions).\n\n On update: Increase version only by 1 major version at a time. If the cluster is pinned to a MongoDB feature compatibility version exactly one major version below the current MongoDB version, the MongoDB version can be downgraded to the previous major version.",


nit: we might want to limit description size

Yes this might be a hard criteria to determine. Will copy generated docs into personal provider so they can be viewed directly in terraform registry, hopefully we can get good feedback from product in case current schema docs require significant changes.

tools/codegen/gofilegen/schema/typed_model.go

lantoli

LGTM, nice what a small change is needed

AgustinBettati · 2025-04-25T13:49:09Z

@EspenAlbert responding here

Are these the same objects in the API Spec?

Yes

Why not re-use?

Conserving the original references to common models, both in the intermediate model and in code generation logic, adds complexity. As of now I cant see significant drawbacks of having multiple models with same content, mainly as all code is fully generated. We can always revisit if we find additional drawbacks.

lantoli · 2025-04-25T14:31:34Z

@AgustinBettati @EspenAlbert I think it's ok as it is now, we might consider the improvement of keeping a set of models already generated so we only generate links, etc. once, if they really have the same content always. then we can go back to no need the ancestor prefix.

edit: or maybe reconsider this PR and do it here directly keeping the name without ancestors but generating it only once keeping the set of already generated models

AgustinBettati · 2025-04-25T15:32:16Z

we might consider the improvement of keeping a set of models already generated so we only generate links once

Yes this could be an alternative. We have to keep in mind that additional logic in generation using common models also can imply additional logic in conversion logic to find the correct nested model (currently we don't need this, but the use case may come in the future).

My take is that at the moment the drawback is small (potential models with same content), as we continue covering more cases we can assess if there are more drawbacks to this approach which keeps model and code generation simple (KISS principle).

* chore: Defining helper function for making untyped API calls (#3215) * adopting adjusted branch and doing successful GET * remove project example calls * formatting * linting * include struct for passing parameters * inline api call * chore: Define template for resource operations file (#3227) * create directories if not present when generating new resource * initial template * small adjustments to resource template * include package and resource name * add generated resource * fix golden files * leave comment clarifying meaning of file permission parameter * adjust permission of generated files * chore: Define conversion function for setting API response values into TF Model (non-nested) (#3230) * Create Unmarshal and basic test * fix linter * unmarshal strings * bools * numbers * fix linter * error tests * doc * fix linter * unsupported tests * typo * chore: Support code generation for making POST/GET/DELETE API calls (#3242) * adjusting code generation logic with hardcoded values * refactor case sensitive info * passing unit tests * allow version header not being defined in config and obtained from API Spec * define unit test for resource file generation * fix ordering * adjust template * renaming functions * extract version header to a variable * chore: Define conversion function for generating API request body from Resource Model in create (non-nested) (#3239) * create Marshal * marshal string * make marshal and umarshal func naming more consistent * TestMarshalUnsupported and TestMarshalPanic * panic messages * failing test for omitjson * typo * implement omitjson * createonly * adjust comment * simplify const * rename creatonly tag to omitjsonupdate * document Marshal * fix tfsdk tags * chore: Adjust model generation to include tags relevant for conversion logic of marshalling (#3244) * initial adjustment of model and golden file adjustment * handle generation * support parsing of req body usage * updating pushbasedlogexport * adding comment in merge attributes * small doc adjustment * renaming createonly to omitjsonupdate * add omit all to timeouts attribute * define var group * chore: Adjust code generation to call conversion functions (#3249) * read * read resource * fix Read * Create * update golden file * typo message * chore: Fix string case in resulting json of Marshal call (#3251) * fix string case for json marshalling * configure pble * remove from provider * chore: Configure database_user_api autogeneration (#3254) * detect if generate resource is not up-to-date * custom_db_role_api config * custom_db_role_api files * database_user_api * comment custom_db_role * add generate-resource-check depedency * chore: Support processing PATCH request properties and inferring OmitUpdateBody (#3256) * TDD :) adding failing test * implementing logic to parse request properties in PATCH request * adjust test resource API spec * fix case of using parent computability when merging nested attributes * fix case where post and patch request have different computability * avoid processing properties from request is they are read only * clean up of comments * moving order * refactor to existingAttr, found := * refactor addOrUpdate into smaller function, also simplified logic in mergeNestedAttributes * small comment adjustment * chore: Support list nested attributes in conversion logic for generating API request from Resource Model (#3255) * marshal list of strings * types.Object * renames * types.Set * types.Map * doc * refactor getAttr logic * fix JSON key names * TestMarshalNestedMultiLevel * chore: Add code generation for update operation (#3258) * adjust template * adjust test golden file * adding configuration for http method * chore: Move generate resource Github action to its own file (#3264) * extract generate-resource-check * TEMPORARY: check GHA * Revert "TEMPORARY: check GHA" This reverts commit b1d43df. * add api resources * TEMPORARY change api file * Revert "TEMPORARY change api file" This reverts commit b29dabe. * TEMPORARY change not in api * Revert "TEMPORARY change not in api" This reverts commit 14cc52c. * Reapply "TEMPORARY: check GHA" This reverts commit cc1a392. * Revert "Reapply "TEMPORARY: check GHA"" This reverts commit 92e6a3a. * chore: Support import operation in generated resources (#3265) * add import function with unit testing * add import function with unit testing * add import function with unit testing * adjust code generation to include call to generic import function * add more specific error message * renaming req body usage enums * remove linter exception * renaming of construct to build * close response body after reading * small renaming * extract common error string * rename template variable to ImportIDAttributes * adding seperate error message of read of body fails * chore: Function `UntypedAPICall` return errors when API responds with status code > 300 (#3267) * Return errors when API responds with status code > 300 * remove registering resources * add todo comment on go mod * chore: Remove generation of ObjType models in resource generation (#3274) * add limitation comment * avoid generating obj types in generation * change order * order alphabetically * chore: Support nested attributes in conversion logic for generating Resource Model from API response (#3261) * failing TestUnmarshalNestedAllTypes * keepKeyCase doc * marshal bool * rename jsonResp * simple case test pass for case map[string]any * rename model struct * skip attributes that are not in the model * test case: model attr types in objects must match JSON types * pass test case: model attr types in objects must match JSON types * add float and bool to modelTest * unkown and null attributes in model to get unmarshalled into * compare model instead of individual attrs * support null object as model source * convertUnknownToNull * refactor getObjAttrsAndTypes and getNullAttr * setObjAttrModel * test case: JSON maps not supported yet * more test cases in TestUnmarshalErrors * extract tom setAttrModel and setObjAttrTfModel * getNullAttr for objects * Unmarshal recursive support for objects * refactor setObjAttrModel * support list and set of strings at root level * failing test for object list * getTfAttr * recursive logic in getCollectionElements * fix use of newValue * set example * small doc change * Map comment * split file for marshal and unmarshal * don't convert unknown to null in this PR * recursive lists and sets * chore: Rename autogeneration to autogen (#3278) * rename package * rename autogen struct tag * chore: Convert unknown values to null values (#3281) * fill unkowns in first-level * rename to PrepareResponseModel * recursive objects * recursive tests * fix golden files * move zero-len collection logic to Unmarshal * reuse elements in collections * rename to ResolveUnknowns * update doc * chore: Add CI acc tests for custom_db_role_api and database_user_api (#3279) * acc test config * basic database_user_api test * install tools for goimports * TestAutogenResourcesNotEnabled * fix attributes * custom db role basic test * remove test * comment TestAutogenResourcesNotEnabled * fix database_user_api test * fix customdb_role_api test * chore: Simplify Unmarshal (#3287) * remove duplicated Unmarshal logic * add autogen folder to trigger acc tests * show attribute name in errors * improve error messages * chore: Add CI acc tests for mongodbatlas_push_based_log_export_api and mongodbatlas_search_deployment_api (#3291) * add mongodbatlas_push_based_log_export_api basic test to CI * generate search_deployment_api * add mongodbatlas_search_deployment_api basic test to CI * aws vars in CI acc tests * fix tests bc not running ops * wait in Delete * add reference to CLOUDP-314960 * change search_deployment version so delete works with 404 error * increate wait time for search_deployment delete * chore: Adjust make command to support generating documentation for autogenerated resources (#3297) * adjust docs generation script to support generated autogenerated docs * adjust provide test to support _api_v* fromat as well * add search deployment docs * adjust comment * add comment clarification * adjusting naming * move comments within phony definition * add comment about limitation * include enable-autogen in generate resources workflow * chore: Create template model for wait functionality (#3296) * hardcode values in the template model directly * move values to operations * search_deployment state attr * add return after errors * adjust timeouts * update golden test files * sleep in WaitForChanges * pass params to WaitForChangesReq * also return http.Response so it can be used to update the response state * doc WaitForChanges return params * HandleCreate * rename TestAccCustomDBRoleAPI_basic * HandleRead * HandleUpdate * rename GenericImportOperation to HandleImport * HandleDelete * remove unneeded consts * request structs for operation handlers * pass Wait info to Create and Update * remove test limitations * handleWait * callAPIWithBody and callAPIWithoutBody * don't consider delete wait errors * adjust config.yml * improve error messages * chore: Allow configuring alias for path parameters when names do not match properties in req/resp bodies (#3300) * adjust config and fix for aliases in path params * adjust test * add project and resource policy generation * small change to cluster config * formatting * add new resource in make file * adding generated docs * clarify limitation * avoid provider file causing false positive when finding differences in generated code and docs of generated resources * chore: Support nested attributes with same name avoiding generation of duplicate model names (#3303) * support multiple nested modles with same parent property * formatting * new code generation * add docs * change parentName to ancestorsName * map (#3306) * chore: Implement full wait functionality for long-running operations (#3305) * wip * update templates * state attribute with JSON naming * improve not found logic * delete TODO comment * revert timeout * typo * error if no pending states * chore: Adding CI tests for working project and resource policy generated resources (#3307) * adding test for project api * add resource policy tests * add project and resource policy in PR acceptance test runner for autogen group * config fixes * adjust template to use state for path param values in patch * removal of tags is not yet supported * adjust golden files * use state instead of plan for wait call params in update * add todo in comment * chore: Define long lasting operation information in configuration file (#3313) * rename state attribute to state property * move hardcoded wait values over to config file * adding DELETE in config * adding comment * clean up * adding comment on state property * supporting map type model with null value (#3321) * use untyped client funcs * fix linter error: QF1003: could use tagged switch on attr.ReqBodyUsage * run: make generate-resources * run: make tools enable-autogen generate-resources * move autogen resources to internal/serviceapi * TEMPORARY check catching api resources * Revert "TEMPORARY check catching api resources" This reverts commit 4f6a0d0. * remove autogen resource docs * don't check autogen resource docs * TEMPORARILY: change generated code * Revert "TEMPORARILY: change generated code" This reverts commit ce13e84. * adjust doc * no need to register autogen resources to check if they're generated * removed outdate comment * feedback about autogen resources without acc tests --------- Co-authored-by: Agustin Bettati <bettatiagustin@gmail.com>

AgustinBettati added 4 commits April 25, 2025 14:23

support multiple nested modles with same parent property

2fbcd85

formatting

4e5d0ed

new code generation

c73a651

add docs

bdbe60f

AgustinBettati marked this pull request as ready for review April 25, 2025 12:52

AgustinBettati requested review from a team as code owners April 25, 2025 12:52

EspenAlbert reviewed Apr 25, 2025

View reviewed changes

lantoli reviewed Apr 25, 2025

View reviewed changes

tools/codegen/gofilegen/schema/typed_model.go Outdated Show resolved Hide resolved

lantoli approved these changes Apr 25, 2025

View reviewed changes

change parentName to ancestorsName

d0fa229

AgustinBettati merged commit 8c6f417 into CLOUDP-301808-poc-autogeneration Apr 25, 2025
42 checks passed

AgustinBettati deleted the CLOUDP-315376-rebased branch April 25, 2025 15:38

chore: Support nested attributes with same name avoiding generation of duplicate model names #3303

chore: Support nested attributes with same name avoiding generation of duplicate model names #3303

Uh oh!

Conversation

AgustinBettati commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change:

Required Checklist:

Further comments

Uh oh!

github-actions bot commented Apr 25, 2025

Uh oh!

EspenAlbert left a comment

Choose a reason for hiding this comment

Uh oh!

EspenAlbert Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

AgustinBettati Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

EspenAlbert Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

AgustinBettati Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lantoli Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

AgustinBettati Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lantoli left a comment

Choose a reason for hiding this comment

Uh oh!

AgustinBettati commented Apr 25, 2025

Uh oh!

lantoli commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AgustinBettati commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AgustinBettati commented Apr 25, 2025 •

edited

Loading

AgustinBettati Apr 25, 2025 •

edited

Loading

lantoli commented Apr 25, 2025 •

edited

Loading

AgustinBettati commented Apr 25, 2025 •

edited

Loading