[DO NOT MERGE] Add one-pager for Terraform registry scrapers and registry metadata #2956
Conversation
enhanced Terrajet codegen pipelines - Fixes crossplane/terrajet/issues/203 Signed-off-by: Alper Rifat Ulucinar <[email protected]>
ulucinar
force-pushed
the
fix-terrajet-203
branch
from
7c9f0a5 to
99a70b1
Compare
turkenh
requested review from
turkenh,
ezgidemirel,
muvaf,
sergenyalcin and
AaronME
| implementations to be able to fetch metadata from different sources but the | ||
| Terrajet pipelines will always be working on a well defined format regardless | ||
| of how those metadata are scraped. | ||
| - We would like to have the scrapers run as needed, produce their output in the |
There was a problem hiding this comment.
I think this specific item is leaking the proposal detail into the goal section. IMO, the goals section as a whole should be a bit higher level and allow alternative proposal to achieve the same goals.
There was a problem hiding this comment.
Thanks @muvaf for the detailed review.
I have restructured the Goals section and removed the proposal hints from it.
| run each time with a `make generate`, just like the existing codegen pipelines | ||
| we have. This would allow us to separate the lifecycles of metadata-scraping | ||
| and code generation. | ||
|
|
There was a problem hiding this comment.
A Proposal section could be helpful for readers here to understand the whole solution before jumping to the metadata format which is one of the implementation details of the solution.
There was a problem hiding this comment.
Listing what's available in TF registry as metadata information would also be very valuable for having readers think about the future expansions.
There was a problem hiding this comment.
I have added a top-level proposal section giving a high-level overview and made the existing proposal sections subsections of this top-level section.
I have also extended the discussion on available metadata in the Terraform registry.
|
|
||
| ### Metadata Format | ||
| The proposed syntax for scraped metadata documents is YAML as we would also like | ||
| the metadata to be human readable, searchable and maintainable, if needed. A |
There was a problem hiding this comment.
I have added example maintenance tasks for the scraped metadata in the document.
| `azurerm_analysis_services_server` of the native Terraform provider | ||
| [terraform-provider-azurerm] could be as follows: | ||
|
|
||
| ```yaml |
There was a problem hiding this comment.
Having an example definitely helps to understand what goes where but I wonder if we can have an API reference before the examples to show the schema of the YAML.
There was a problem hiding this comment.
I think using a full example (an example that contains all keys) with comments instead of a formal notation (like BNF or OpenAPI schema) serves our purposes better here. This is not intended to be a formal specification but rather the purpose is to convey the idea.
| importStatements: | ||
| - terraform import azurerm_analysis_services_server.server /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resourcegroup1/providers/Microsoft.AnalysisServices/servers/server1 | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Could you elaborate on why the chosen method is better than these alternatives? This would help us reason about them more deliberately when we want to make changes to the pipeline.
There was a problem hiding this comment.
Added clarifications on the choices we made here.
| from a pointed directory in the local filesystem, which is specified as a | ||
| command-line argument, for instance. | ||
|
|
||
| As already indicated, if it turns out that a common registry scraper |
| none or some are not available in the corresponding metadata document. | ||
|
|
||
| Metadata is valuable; the scrapers should capture as much metadata as possible | ||
| and store them in the common format, even for future use cases we do not yet |
There was a problem hiding this comment.
I'm a bit on the fence about storing everything we can because we have to have a format for each of those information that we don't use and once we do use them, we may want to change the format, i.e. where it lives in the YAML and also since we provide manual input ability, the format become one of our public APIs. So, I wonder if we should, for example, just keep the examples since that's all we use today and then once we use other information, we could add them.
There was a problem hiding this comment.
I have added example use cases for the proposed fields. In fact (apart from documentation) most have been utilized in provider-jet-azure. I think this also serves as an opportunity to show the potential we have here.
| Metadata is valuable; the scrapers should capture as much metadata as possible | ||
| and store them in the common format, even for future use cases we do not yet | ||
| envision. New Terrajet pipelines can be added, or existing ones can be enhanced | ||
| to support advanced use cases. One such proposal could be to extend the CRD |
There was a problem hiding this comment.
A Future Considerations section could work well to list all the use cases you have in mind at the moment.
There was a problem hiding this comment.
Added a Future Considerations section.
| providers `provider-jet-aws`, `provider-jet-gcp` and `provider-jet-azure` in the | ||
| context of the corresponding [Terrajet issue #48], we have seen utility in | ||
| extracting such metadata from the Terraform registry and use it to generate | ||
| example manifests and documentation. In this document, we would like to propose: |
There was a problem hiding this comment.
I think Background section could be limited to the problem statement in general and the actual proposal could be kept for its own section.
There was a problem hiding this comment.
Removed proposal hints from the Background section as suggested.
| @@ -0,0 +1,272 @@ | |||
| # Metadata Extraction from Terraform Registry for Terrajet-based providers | |||
There was a problem hiding this comment.
| # Metadata Extraction from Terraform Registry for Terrajet-based providers | |
| # Metadata Extraction from Terraform Registry in Terrajet |
Since we're removing the difference between Terrajet vs SDK calls at repository level, I think removing Terrajet-based provider or Terrajet-based repository terms would be more future-proof for readers.
|
Would it make sense to move this design (and perhaps the Terrajet design doc too?) to a new |
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
move existing proposal sections as subsections of this top-level section. Signed-off-by: Alper Rifat Ulucinar <[email protected]>
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
monolithic one. Signed-off-by: Alper Rifat Ulucinar <[email protected]>
from getting lost as scrapers are rerun, and extend the discussion on when the proposed scrapers are run. Signed-off-by: Alper Rifat Ulucinar <[email protected]>
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
|
Hi @negz, |
ulucinar
changed the title
I think having all design docs under |
I don't feel super strongly, but it does seem a little odd to me that a design pertaining completely to Terrajet is not in the Terrajet repo. We can just stick with everything going in the design folder here if folks feel differently. |
|
Closing this PR as we would like to keep code generation in terrajet only scoped to generating CRDs. |
Description of your changes
Fixes crossplane/terrajet/issues/203
With this one-pager, we would like to discuss how metadata about Terraform native resources can be scraped and processed from the Terraform registry and used in Terrajet codegen pipelines to produce example manifests or CRD documentation and in various other use cases.
We believe such metadata offers large potential to improve the user experience with the Crossplane providers generated using Terrajet as well as the quality of them (e.g., auto-generated CRD documentation).
We aim to gather community feedback and stir up some discussions to discover new use cases with such metadata in Terrajet-based providers.
I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
N.A.
NOTE: We are considering to move this proposal to https://github.com/crossplane/terrajet after the review process is completed.