Changes to ancestrymanager logic to parse "parent" field for project, folder and organization asset types.#1450
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
/gcbrun |
|
/gcbrun |
melinath
left a comment
There was a problem hiding this comment.
this seems reasonable; could you add some test cases to the unit tests for each file?
| return folderID.(string), ok | ||
| } | ||
| // folder name is store in display_name for google_folder | ||
| folderID, ok = tfData.GetOk("display_name") |
There was a problem hiding this comment.
There are a number of "display_name" fields on different resources that generally hold "human-friendly" names for resources, so we can't rely on it to hold a folder identifier. Even for folder - I am pretty sure the display_name isn't used in the ancestry. The value returned from this function will be used to cache the ancestry of the folder, so we need to be able to rely on it to be in the standard format - which is folders/XXXXX.
Looking at the resource - you'll probably need to either:
- use
folder.idand check with a regex that it exactly matches the expected format (because every terraform resource has anidfield - use
folder_idbut add thefolders/prefix back in - because the folder resource strips it out.
No matter which you choose, you'll need to make sure (with new test cases) that folder will return a valid ancestry even if folder_id is present on the resource with just a number in it. (Currently, this function assumes that folder_id contains something like folders/XXXX because on most resources, that's correct.)
There was a problem hiding this comment.
I have removed the code fetching {display_name} as an ancestor for google_folder. As discussed in offline conversation, ancestry path will be determined by {parent} field when the {folder_id} for google_folder is not yet set.
The PR handles ancestryManager module for the following changes: