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

Lack of SchemaDescriptionBuilder equivalent (from sdkv2) #625

Open
jacobbednarz opened this issue Jan 16, 2023 · 4 comments
Open

Lack of SchemaDescriptionBuilder equivalent (from sdkv2) #625

jacobbednarz opened this issue Jan 16, 2023 · 4 comments
Labels
enhancement New feature or request sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it.

Comments

@jacobbednarz
Copy link

jacobbednarz commented Jan 16, 2023

Module version

github.com/hashicorp/terraform-plugin-framework v1.1.1

Overview

Within terraform-provider-cloudflare, we heavily rely on terraform-plugin-docs to generate our documentation. As part of this, we have an init method that handles some formatting that we specifically want to all field attributes. I.e. if changing a schema attribute will force a replacement, we call that out in the description.

https://github.com/cloudflare/terraform-provider-cloudflare/blob/20222d155d457736d71eaa41a63bb87853cfaf29/internal/provider/provider.go#L19-L76

From the code snippet, you can see that it relies on SchemaDescriptionBuilder and a function that modifies the descriptions to match our format.

I've had a dig around in the framework codebase but I'm unable to find anything that resembles any similar (even in the provider Configure). Have I overlooked this? Or is this not supported in its current form?

References

n/a

@jacobbednarz jacobbednarz added the bug Something isn't working label Jan 16, 2023
jacobbednarz added a commit to cloudflare/terraform-provider-cloudflare that referenced this issue Jan 19, 2023
Right now, there isn't a way to do automatic description decoration based on the
schema attributes[1]. This is a problem because to mux two provider types they
must return identical schemas.

To work around this, I've added conditionals to the generator to ignore the
schema fields in the providers where we may experience differences between the
providers but it remains for all resources and data sources. For now, this
strikes the balance of unblocking us and keeping the documentation nicely
decorated.

[1]: hashicorp/terraform-plugin-framework#625
jacobbednarz added a commit to cloudflare/terraform-provider-cloudflare that referenced this issue Jan 19, 2023
Right now, there isn't a way to do automatic description decoration based on the
schema attributes[1]. This is a problem because to mux two provider types they
must return identical schemas.

To work around this, I've added conditionals to the generator to ignore the
schema fields in the providers where we may experience differences between the
providers but it remains for all resources and data sources. For now, this
strikes the balance of unblocking us and keeping the documentation nicely
decorated.

[1]: hashicorp/terraform-plugin-framework#625
@bflad bflad added enhancement New feature or request sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it. and removed bug Something isn't working labels Feb 9, 2023
@bflad
Copy link
Contributor

bflad commented Feb 9, 2023

Hi @jacobbednarz 👋 Thank you for raising this and apologies for the delayed response. I have relabeled this issue as a feature request because this is functionality that does not yet exist in the framework, rather than something not working.

The current solution for what to do now is manually adjust attribute descriptions to include all desired text for documentation generation.

Longer term, this type of functionality will likely take a different form than what was provided by terraform-plugin-sdk. A rough edge with the SchemaDescriptionBuilder functionality is that it is overloading a singular textual description for an attribute. That single description is then ingested by all other downstream tooling via the terraform providers schema -json command or potentially the Terraform CLI itself if it ever offered textual documentation. Any text formatting, potential verbiage changes for different tooling, or desired syntax handling (Markdown or plaintext) is not flexible since the Terraform plugin protocol only offers one field for the information.

One of the design considerations that has been baked into various extension points of the framework, such as schema-based validators, is that each of those interfaces requires separate Description and MarkdownDescription methods. The next step would be moving away from reliance on the Terraform plugin protocol for surfacing information from framework providers. Instead, the framework would support a methodology for outputting richer schema information directly by executing the plugin binary or potentially as a separate RPC. This might take the form of a JSON (very roughly for illustrative purposes) like:

{
  "resources": [
    {
      "description": "...",
      "markdown_description": "...",
      "schema": {
        "attributes": [
          {
            "name": "example_attribute",
            "description": "...",
            "markdown_description": "...",
            "validators": [
              {
                "description": "...",
                "markdown_description": "..."
              }
            ]
          }
        ]
      }
    }
  ]
}

Meaning that individual downstream tooling can appropriately decide how they want to surface the information. For example, terraform-plugin-docs could offer to template validator descriptions as a Markdown list under the list item that includes the attribute description. This also would enable functionality such as #160 and much more.

I apologize that this might not be the answer you are looking for here, but hopefully this tease of the potential future here helps justify why this particular functionality was not directly copied into the new framework.

@jacobbednarz
Copy link
Author

Thanks for the response @bflad. That definitely makes sense given the way terraform-plugin-framework is designed vs the SDKv2.

The only bit I'd like to counter is on where the modifications can happen. I think for majority of use cases, a downstream tool is the answer however, there are times when how we want to provide downstreams a generated value (whether that be plaintext or markdown). Take for instance, using terraform-plugin-docs. There may be support for Default/ForceNew values to render one way however, in our descriptions we'd like to emphasis or display addon information based on other factors. This is a rather trivial example but it wouldn't be feasible today with the framework to manually apply each of these. The only solution we'd have would be to maintain our own fork of the downstream tool to achieve it at which case, we're maintaining another part of the ecosystem we don't really want to be doing given it is such a small variation.

For me, something like the PlanModifiers concept would be a good middle ground (no idea on naming though) that allowed this functionality and not leave all of the onus on downstream tooling. Where we needed to vary, we would implement one of those and the downstream tooling would inherit it as oppose to putting more in the downstream tooling. Food for thought I guess.

@bflad
Copy link
Contributor

bflad commented Feb 10, 2023

Really appreciate that feedback, @jacobbednarz 👍 Could you maybe walk me through what you are thinking in this regard just to ensure we have a shared understanding?

Maybe I am mistaken, but what it sounds like the proposal here is something like:

// Illustrative code only. Naming and interface design is not thought out.

type DescriptorRequest struct {
  // Target is set to the tool name that requested the description
  Target string
}

// Debatable to leave with multiple fields for formatting
type DescriptorResponse struct {
  // Plaintext should contain the plaintext formatted description
  Plaintext string

  // Markdown should contain the Markdown formatted description
  Markdown string
}

schema.StringAttribute{
  // ... other fields ...
  Descriptor: func(ctx context.Context, req DescriptorRequest, resp *DescriptorResponse) {
    switch req.Target {
    case "terraform-plugin-docs":
      resp.Markdown = "..."
      resp.Plaintext = "..."
    default:
      resp.Markdown = "..."
      resp.Plaintext = "..."
  }
}

And potentially updating the existing validator, plan modifier, etc. interfaces with a similar request/response setup. For existing Description/MarkdownDescription fields, Descriptor would supersede them and maybe a quick helper function introduced into the framework that takes plaintext and markdown string parameters to fill in the response:

func GenericDescriptions(plaintext string, markdown string) func(context.Context, DescriptorRequest, *DescriptorResponse) {
  return func(_ context.Context, req DescriptorRequest, resp *DescriptorResponse) {
    resp.Plaintext = plaintext
    resp.Markdown = markdown
  }
}

This would certainly enable a lot more flexibility over the single plaintext/markdown descriptions, but I would be curious if the additional complexity is worth it for most developers.

@jacobbednarz
Copy link
Author

jacobbednarz commented Feb 12, 2023

I probably wouldn't go as far as to design it with specific tools in mind but I think we're roughly on the same page. In my uses, I don't particularly need to know that is consuming the information but I do need to ensure that I can pass them a modified value if needed as opposed to them having all the logic built in. To provide a concrete example: Whenever an attribute description has the string "wirefilter syntax" and doesn't match [wirefilter syntax](https://developers.cloudflare.com/ruleset-engine/rules-language/) we want to replace it. Internally, we want this for consistency in our documentation language but it's not something that any downstream tool should be accounting for.

Initially, I was thinking the provider level Configure method is where it would make sense as Resources/Datasources already exist (as any) but we could potentially do it like you've outlined above too (just with less specificity)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it.
Projects
None yet
Development

No branches or pull requests

2 participants