Feature Request: support markdown in documentation (variables/outputs/etc)

[freakinhippie]

The terraform-docs tool supports markdown in description fields with various output types which make use of it. It would be great if terrareg had support for markdown to allow for more detailed documentation to accompany modules.

[MatthewJohn]

This sounds great - we certainly do markdown-to-html in various places already, so don't imagine this would be too difficult.

My only concern might be what type of markdown to support... my mind immediately goes to a table within a table, which would be horrendous :D At least to start with, what kind of support would be useful to you (e.g. bold, italics etc, links/images?)

[MatthewJohn]

Created gitlab issue: https://gitlab.dockstudios.co.uk/pub/terrareg/-/issues/527 gitlab-issue-id:527

[freakinhippie]

This sounds great - we certainly do markdown-to-html in various places already, so don't imagine this would be too difficult.

My only concern might be what type of markdown to support... my mind immediately goes to a table within a table, which would be horrendous :D At least to start with, what kind of support would be useful to you (e.g. bold, italics etc, links/images?)

My recommendation would be to either start with plain markdown, github flavored markdown, or whatever terraform-docs support since those seem to be the most common. There are certainly extensions that would certainly be nice, but I would rather have something now and/or soon than everything that I want sometime later.

[MatthewJohn]

Well, I guess my point is sort of, I don't really want to support tables and actively disallow them (because I know it would render absolutely horrendously).. I'll take a look to see what the "allowed" types are for what we currently use and maybe just drop tables for now :)

[freakinhippie]

Ah, I see. I use tables quite frequently within my markdown descriptions. However, I don't nest tables within tables.

With that said, I would still be happy to get support for markdown sans tables.

[MatthewJohn]

Ah, I see. I use tables quite frequently within my markdown descriptions. However, I don't nest tables within tables. With that said, I would still be happy to get support for markdown sans tables.

that is interesting! :) Would you be able to provide an example of a description with a table - I'd be interesed in testing it to see how it looks in the UI within the outputs/variables table! :)

[freakinhippie]

Here are a couple examples, with names changes to protect the innocent, as well as simplified to respond to the request directly.

variable "owner_team" {
  description = <<-EOD
    The stack owner's lowercase team name, with non-alpha characters replaced with hyphens. It must be one of the following:

    |                     |                         |                         |                  |
    |---------------------|-------------------------|-------------------------|------------------|
    | infrastructure      | cloud-ops               | devops                  | software-dev     |
    | data-analytics      | machine-learning        | ai-research             | cyber-security   |
    | backend-services    | frontend-development    | qa-testing              | site-reliability |
    | network-engineering | it-support              | tech-operations         | data-engineering |
    | product-management  | user-experience         | api-development         | integration-team |
    | platform-team       | enterprise-architecture | database-admin          | it-compliance    |
    | automation          | big-data                | storage-solutions       | mobile-dev       |
    | system-architecture | cloud-security          | software-quality        | dev-tools        |
    | support-services    | system-integration      | cloud-migration         | network-security |
    | tech-infrastructure | devsecops               | performance-engineering | app-development  |
  EOD
  type        = string

  # validation which limits input to one of those values...
}

variable "pci_context" {
  description = <<-EOD
    The context of PCI scope.

    | Context                                         | Applicability                                                                                                                                                                                                                      

                                                                                                                                                                                                                                                  |
    |-------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
    | `"cat1a"` Card Data Environment (CDE) Resources | System component stores, processes, or transmits card holder data (CHD)                                                                                                                                                            

                                                                                                                                                                                                                                                  |
    | `"cat1b"` Card Data Environment (CDE) Resources | System component is on the same network segment—for example, in the same subnet or VLAN as system(s) that store, process, or transmit CHD                                                                                          

                                                                                                                                                                                                                                                  |
    | `"cat2a"` Connected to CDE or Resources         | System component is on a different network (or subnet or VLAN) but can connect to or access the CDE (e.g., via internal network connectivity)<br/>**OR**<br/>System component can connect to or access the CDE via another system—for example, via connection to a jump server that provides access to the CDE)<br/>**OR**<b>System component provides security services to the CDE—for example, network traffic filtering, patch distribution, or authentication management<br/>**OR**<br/>System component supports PCI DSS requirements, such as time servers and audit log storage servers<br/>**OR**<br/>System component provides segmentation of the CDE from out-of-scope systems and networks—for example, firewalls configured to block traffic from untrusted networks |
    | `"cat2b"` Security Impacting Systems            | System component can impact configuration or security of the CDE, or how CHD/SAD is handled— for example, a web redirection server or name resolution server                                                                       

                                                                                                                                                                                                                                                  |
    | `"cat3"`  Out of PCI Scope                      | Systems that do not meet criteria described above for CAT1/CAT2                                                                                                                                                                    

                                                                                                                                                                                                                                                  |
  EOD
  type        = string

  # validation which limits input to one of those values...
}

[MatthewJohn]

Hey @freakinhippie ,

I've implemented the API changes for this in https://gitlab.dockstudios.co.uk/pub/terrareg/-/merge_requests/406. I've got some draft changes locally for the front-end - how does this look?

Matt

[MatthewJohn]

with names changes to protect the innocent

I don't think you've done this well enough - I'm scarred from those definitions of catX :rofl: :rofl:

[freakinhippie]

That looks pretty good. Although I guess you've nested the markdown inside the existing table of variables.

I was thinking more along the lines of how terraform-docs outputs a <h3> for each variable (or output) and adds the description in a <p> tag. One of the benefits of this pattern is that the description field is not artificially constrained to the width remaining after name, type, and whether it is required consume some fraction of the available horizontal real estate. This would be especially problematic with some variable type definitions that are complex objects, rather than simple inputs.

With all that said, I am very appreciative of your incredibly rapid turn around on these requested changes. The demo above is certainly an improvement and I don't want this message to come across as overly critical. So, thank you for all your effort!

[MatthewJohn]

Hey @freakinhippie,

Not at all - was hoping for feedback (good or bad!) Do you mean something like this?

(I realise this could be full-width, but at least, without more wrangling, it's a constraint for all of the tabs - obviously easy to mock up, probably a little harder to nicely achieve as a committable change)

Matt

[freakinhippie]

This is pretty great. I would suggest leaving a small border and a bit of cell padding in the tables so that they aren't so bunched together, but otherwise that is pretty close to what I had envisioned.

The only other point I would add is that you might want to consider placing the type definition on a separate line from the "Type:" label, particularly for when the type is complex. For example:

variable "lifecycle_rules" {
  type = list(object({
    id      = string
    enabled = bool
    prefix  = string
    tags    = map(string)
    transition = list(object({
      days          = number
      date          = string
      storage_class = string
    }))
    expiration = list(object({
      days                         = number
      date                         = string
      expired_object_delete_marker = bool
    }))
    noncurrent_version_transition = list(object({
      days          = number
      storage_class = string
    }))
    noncurrent_version_expiration = list(object({
      days = number
    }))
    abort_incomplete_multipart_upload_days = number
  }))
}

[MatthewJohn]

After a little bit more of a play, though I'm not keen on the padding in the mulit-line types/comments, this might be an improvement

Matt

[freakinhippie]

That's fantastic! I have no further critiques as far as I can see.

Thank you again!

[freakinhippie]

After a little bit more of a play, though I'm not keen on the padding in the mulit-line types/comments, this might be an improvement

Matt

I don't think padding for the type or default definition blocks is necessary, They're shown with a different coloured background and they don't really have anything adjacent to them that would be confused for part of the block.

[mrmattyboy1]

We have modules with 20+ variables and I almost feel like it would become impossible to see them concisely - I wonder if a table of contents would help

[freakinhippie]

We have modules with 20+ variables and I almost feel like it would become impossible to see them concisely - I wonder if a table of contents would help

I may be wrong but I would guess this display format would be an optional feature rather than the default. Although I wouldn't object to a TOC.

[MatthewJohn]

I may be wrong but I would guess this display format would be an optional feature rather than the default. Although I wouldn't object to a TOC.

Do you mean a feature to toggle between the table and the version in the screenshot above?

[freakinhippie]

I may be wrong but I would guess this display format would be an optional feature rather than the default. Although I wouldn't object to a TOC.

Do you mean a feature to toggle between the table and the version in the screenshot above?

Yes. Much like how terraform-docs defaults to the table output, but if you pass the markdown output type it results in something similar to the screenshot above.

Obviously, I prefer the markdown layout that I'm requesting here, but I won't suggest that is what everyone would prefer.

[MatthewJohn]

Hey @freakinhippie,

I've added support for both and updated the "outputs" to do the same.

The change is currently running in a PR environment here: https://terrareg-527-feature-request-support-markdown-in-docume.gitlab-pr.dockstudios.co.uk/modules/demo/ecs/aws#inputs

Let me know what you think :) BTW, the selection of table vs details is stored in local storage, so is persistent between pages etc. However, currently, when you change it (say in the inputs tab), it's not immediately reflected in the other tabs (e.g. output tab)

Matt

[freakinhippie]

This is great! I especially like how you've implemented the table vs expanded view as local storage so that the user can decide for themselves what they prefer rather than a site-wide selection. Although, it might be nice to be able to set the site-wide default to one or the other, but that would just be a bonus. I'm more than pleased with this already!

Thank you again!

[MatthewJohn]

Hey @freakinhippie,

I've added a global configuration for this in 61a1fce2. CI will soon deploy the changes to that demo environment - there's a couple of other fixes are layout that can be seen. I've hopefully tackled all of the tests, but time will tell for that (since the CI takes ~30 mins to run)

Let me know if you have any additional comments on it :)

Matt

[freakinhippie]

I see you cleaned up the overlap issue with the TOC and the tab body. It looks good.

I do have one further suggestion (at least)... In the [Usage] section, the terraform snippet has syntax highlighting. However, in the README section, there is a block of terraform but it does not have syntax highlighting. It would be great to use the same highlighting for fenced code blocks. This might be more appropriate for a separate feature request.

[mrmattyboy1]

I'll see if the syntax highlighter library that I'm using is able to mangle it :D

[MatthewJohn]

I see you cleaned up the overlap issue with the TOC and the tab body. It looks good.

I do have one further suggestion (at least)... In the [Usage] section, the terraform snippet has syntax highlighting. However, in the README section, there is a block of terraform but it does not have syntax highlighting. It would be great to use the same highlighting for fenced code blocks. This might be more appropriate for a separate feature request.

@freakinhippie Sorry, I'd misread and assumed that you meant the 'default' and 'description' sections of inputs/outputs! For the README it's a little trickier. I wonder if we could open a new issue to discuss that there?

[freakinhippie]

Of course. I'll open another feature request for that after these changes get published, unless you'd prefer that I do it immediately.

[MatthewJohn]

This is all merged :) Released in https://github.com/MatthewJohn/terrareg/releases/tag/v3.6.0

Feel free to raise the other ticket whenever :)

Matt