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.

Sign up for GitHub

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

Jump to bottom

Allowing single word resources to use templates #147

Merged
merged 5 commits into from Jun 10, 2022
Merged

Allowing single word resources to use templates #147

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b4b63ec
Allowing single word resources to use templates
bendbennett Jun 6, 2022
9377652
Updating CHANGELOG.md
bendbennett Jun 6, 2022
27bf553
Add unit test for resourceName
bendbennett Jun 6, 2022
b0001a4
Returning error if resource not found
bendbennett Jun 9, 2022
d6cc333
Adding field names to test structs
bendbennett Jun 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
@@ -1,12 +1,18 @@
# 0.9.1 (Unreleased)

BUG FIXES:

* cmd/tfplugindocs: Allow single word resources to use templates ([147](https://github.com/hashicorp/terraform-plugin-docs/pull/147)).

# 0.9.0 (June 1, 2022)

NEW FEATURES:

* cmd/tflugindocs: Additional CLI arguments `provider-name`, `rendered-provider-name`, `rendered-website-dir`, `examples-dir`, `website-temp-dir`, and `website-source-dir`. These allow to further customise generated doc ([#95](https://github.com/hashicorp/terraform-plugin-docs/pull/95)).
* cmd/tfplugindocs: Additional CLI arguments `provider-name`, `rendered-provider-name`, `rendered-website-dir`, `examples-dir`, `website-temp-dir`, and `website-source-dir`. These allow to further customise generated doc ([#95](https://github.com/hashicorp/terraform-plugin-docs/pull/95)).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those are definitely typos I made 🤦

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries. I like that we're keeping an eye out for each other 👍


ENHANCEMENTS:

* cmd/tflugindocs: Implemented usage output (i.e. `--help`) for `generate` and `validate` commands ([#95](https://github.com/hashicorp/terraform-plugin-docs/pull/95)).
* cmd/tfplugindocs: Implemented usage output (i.e. `--help`) for `generate` and `validate` commands ([#95](https://github.com/hashicorp/terraform-plugin-docs/pull/95)).

# 0.8.1 (May 10, 2022)

Expand Down
54 changes: 28 additions & 26 deletions internal/provider/generate.go
View file
Open in desktop
Expand Up @@ -414,35 +414,37 @@ func (g *generator) renderStaticWebsite(providerName string, providerSchema *tfj
g.infof("rendering %q", rel)
switch relDir {
case "data-sources/":
resName := shortName + "_" + removeAllExt(relFile)
resSchema, ok := providerSchema.DataSourceSchemas[resName]
if ok {
tmpl := resourceTemplate(tmplData)
render, err := tmpl.Render(resName, providerName, "Data Source", "", "", resSchema)
if err != nil {
return fmt.Errorf("unable to render data source template %q: %w", rel, err)
}
_, err = out.WriteString(render)
if err != nil {
return fmt.Errorf("unable to write rendered string: %w", err)
}
return nil
resSchema, resName := resourceSchema(providerSchema.DataSourceSchemas, shortName, relFile)
if resSchema == nil {
return fmt.Errorf("unable to find resource for provider (%s) and template (%s)", shortName, relFile)
}

tmpl := resourceTemplate(tmplData)
render, err := tmpl.Render(resName, providerName, "Data Source", "", "", resSchema)
if err != nil {
return fmt.Errorf("unable to render data source template %q: %w", rel, err)
}
_, err = out.WriteString(render)
if err != nil {
return fmt.Errorf("unable to write rendered string: %w", err)
}
return nil
case "resources/":
resName := shortName + "_" + removeAllExt(relFile)
resSchema, ok := providerSchema.ResourceSchemas[resName]
if ok {
tmpl := resourceTemplate(tmplData)
render, err := tmpl.Render(resName, providerName, "Resource", "", "", resSchema)
if err != nil {
return fmt.Errorf("unable to render resource template %q: %w", rel, err)
}
_, err = out.WriteString(render)
if err != nil {
return fmt.Errorf("unable to write regindered string: %w", err)
}
return nil
resSchema, resName := resourceSchema(providerSchema.ResourceSchemas, shortName, relFile)
if resSchema == nil {
return fmt.Errorf("unable to find resource for provider (%s) and template (%s)", shortName, relFile)
}

tmpl := resourceTemplate(tmplData)
render, err := tmpl.Render(resName, providerName, "Resource", "", "", resSchema)
if err != nil {
return fmt.Errorf("unable to render resource template %q: %w", rel, err)
}
_, err = out.WriteString(render)
if err != nil {
return fmt.Errorf("unable to write regindered string: %w", err)
}
return nil
case "": // provider
if relFile == "index.md.tmpl" {
tmpl := providerTemplate(tmplData)
Expand Down
1 change: 1 addition & 0 deletions internal/provider/template.go
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ import (
"text/template"

tfjson "github.com/hashicorp/terraform-json"

"github.com/hashicorp/terraform-plugin-docs/internal/mdplain"
"github.com/hashicorp/terraform-plugin-docs/internal/tmplfuncs"
"github.com/hashicorp/terraform-plugin-docs/schemamd"
Expand Down
19 changes: 19 additions & 0 deletions internal/provider/util.go
View file
Open in desktop
Expand Up @@ -9,6 +9,8 @@ import (
"os/exec"
"path/filepath"
"strings"

tfjson "github.com/hashicorp/terraform-json"
)

func providerShortName(n string) string {
Expand Down Expand Up @@ -52,6 +54,23 @@ func removeAllExt(file string) string {
}
}

// resourceSchema determines whether there is a schema in the supplied schemas map which
// has either the providerShortName or the providerShortName concatenated with the
// templateFileName (stripped of file extension.
func resourceSchema(schemas map[string]*tfjson.Schema, providerShortName, templateFileName string) (*tfjson.Schema, string) {
if schema, ok := schemas[providerShortName]; ok {
return schema, providerShortName
}

resName := providerShortName + "_" + removeAllExt(templateFileName)

if schema, ok := schemas[resName]; ok {
return schema, resName
}

return nil, ""
}

func writeFile(path string, data string) error {
dir, _ := filepath.Split(path)
err := os.MkdirAll(dir, 0755)
Expand Down
84 changes: 84 additions & 0 deletions internal/provider/util_test.go
View file
Open in desktop
@@ -0,0 +1,84 @@
package provider

import (
"testing"

"github.com/google/go-cmp/cmp"
tfjson "github.com/hashicorp/terraform-json"
)

func Test_resourceSchema(t *testing.T) {
cases := []struct {
name string
schemas map[string]*tfjson.Schema
providerShortName string
templateFileName string
expectedSchema *tfjson.Schema
expectedResourceName string
}{
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add another case where you have a resource named (just for kicks) https, using the same format used by the http provider?

I say this because the http provider is here not respecting the convention: <provider>_<resource> when exposed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@detro you mean along the lines of:

{
	"provider short name different from template file name",
	"https",
	"http.md.tmpl",
	"https_http",
},

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

{
	"provider short name different from template file name",
	"http",
	"https.md.tmpl",
	"https",
},

What I'm expecting is that this test would fail, because the provider name is http, but the resource name is https.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@detro I've refactored following discussion.

"provider short name matches schema name",
map[string]*tfjson.Schema{
"http": {},
},
"http",
"http.md.tmpl",
&tfjson.Schema{},
"http",
},
{
"provider short name does not match schema name",
map[string]*tfjson.Schema{
"http": {},
},
"tls",
"http.md.tmpl",
nil,
"",
},
{
"provider short name concatenated with template file name matches schema name",
map[string]*tfjson.Schema{
"tls_cert_request": {},
},
"tls",
"cert_request.md.tmpl",
&tfjson.Schema{},
"tls_cert_request",
},
{
"provider short name concatenated with template file name does not match schema name",
map[string]*tfjson.Schema{
"tls_cert_request": {},
},
"tls",
"not_found.md.tmpl",
nil,
"",
},
{
"provider short name concatenated with same template file name matches schema name",
map[string]*tfjson.Schema{
"tls_tls": {},
},
"tls",
"tls.md.tmpl",
&tfjson.Schema{},
"tls_tls",
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: While Go allows you to omit field names, I personally find it a little more difficult to read the structs once the struct definition is "beyond the fold" since its based on ordering. e.g. my brain is like what is the 3rd field defined as "tls" here? I have to scroll up, see providerShortName, scroll back down

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. I forget that GoLand is doing this for me:

image

Have added field names and refactored to use map[string]struct{} for the test cases.

}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
actualSchema, actualResourceName := resourceSchema(c.schemas, c.providerShortName, c.templateFileName)

if !cmp.Equal(c.expectedSchema, actualSchema) {
t.Errorf("expected: %+v, got: %+v", c.expectedSchema, actualSchema)
}

if !cmp.Equal(c.expectedResourceName, actualResourceName) {
t.Errorf("expected: %s, got: %s", c.expectedResourceName, actualResourceName)
}
})
}
}