vmware / terraform-provider-tanzu-mission-control

Terraform provider to manage resources of Tanzu Mission Control
Mozilla Public License 2.0
38 stars 31 forks source link

Make meta data converter mapping generic and populate provisioner meta info #380

Closed shobha2626 closed 6 months ago

shobha2626 commented 6 months ago
  1. What this PR does / why we need it:

Make meta data converter mapping generic and populate provisioner meta info

Meta converter was not supporting the array format as that of a provisioner resource. As a result, meta was not getting populated for provisioner because the model path was not including the entire path. In Provisioner resource scenario the meta should be provisioner.meta.

Made changes to accomodate this change not just for provisioner but any resource that follows this structure

  1. Which issue(s) this PR fixes

{ "version": 4, "terraform_version": "1.6.4", "serial": 1, "lineage": "2dcd02f2-3dcc-d8a3-42b1-f05b03f9e9fe", "outputs": {}, "resources": [ { "mode": "data", "type": "tanzu-mission-control_provisioner", "name": "read_provisioner", "provider": "provider[\"vmware/devd/tanzu-mission-control\"]", "instances": [ { "schema_version": 0, "attributes": { "id": "prvn:01HQ2DV4SAY0C5G832WHZ4TBST", "provisioners": [ { "management_cluster": "eks", "meta": [ { "annotations": {}, "description": "Create provisioner through terraform", "labels": { "key1": "value1", "key2": "value2" }, "resource_version": "4965", "uid": "prvn:01HQ2DV4SAY0C5G832WHZ4TBST" } ], "name": "mshobha-test", "org_id": "" } ] }, "sensitive_attributes": [] } ] } ], "check_results": null }

    (optional, in `fixes #<issue number>` format, will close the issue(s) when PR gets merged):

    Fixes #
  1. Additional information

  2. Special notes for your reviewer

codecov-commenter commented 6 months ago

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (aa47e00) 25.36% compared to head (c178b75) 25.36%.

Files Patch % Lines
internal/resources/common/objectmeta_schema.go 0.00% 6 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #380 +/- ## ======================================= Coverage 25.36% 25.36% ======================================= Files 195 195 Lines 16810 16810 ======================================= Hits 4264 4264 Misses 12328 12328 Partials 218 218 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ramya-bangera commented 6 months ago

@shobha2626 - Does the state file dump given in description captures the state file info with LIST or GET datasource? If it's just GET can you add the LIST too? As there are changes to common meta converter code have you validated the automation against your change?

shobha2626 commented 6 months ago

@shobha2626 - Does the state file dump given in description captures the state file info with LIST or GET datasource? If it's just GET can you add the LIST too? As there are changes to common meta converter code have you validated the automation against your change?

@ramya-bangera I have captured only GET because the result for LIST is huge. I have verified the LIST as well, if needed I can run again and share the same. I have validate this change against other resource like namespace and workspace manually and there seems to be no issue. It is optional parameter and shouldn't cause any.

ramya-bangera commented 6 months ago

@shobha2626 - Does the state file dump given in description captures the state file info with LIST or GET datasource? If it's just GET can you add the LIST too? As there are changes to common meta converter code have you validated the automation against your change?

@ramya-bangera I have captured only GET because the result for LIST is huge. I have verified the LIST as well, if needed I can run again and share the same. I have validate this change against other resource like namespace and workspace manually and there seems to be no issue. It is optional parameter and shouldn't cause any.

Thanks for confirming , since these details was missing I just wanted to be sure that it is validated.