From e26a66307d4184c3871ab2f61ddbdb44d79b73ac Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Thu, 21 Sep 2023 10:10:46 -0700 Subject: [PATCH] Actionable error message for a missing repo (#1388) Fixes https://github.com/pulumi/pulumi-terraform-bridge/issues/1365 The current warning for failing to find the source repo (always due to misconfigured `ProviderInfo`) looks like this (1 line per resource/datasoruce): ``` $ pulumi-acme/bin/pulumi-tfgen-acme schema --out provider/cmd/pulumi-resource-acme warning: could not find docs for resource 'acme_certificate'. Override the Docs property in the resource mapping. See type tfbridge.DocInfo for details. warning: could not find docs for resource 'acme_registration'. Override the Docs property in the resource mapping. See type tfbridge.DocInfo for details. ``` This PR changes the error message to look like this: ``` warning: Unable to find the upstream provider's documentation: The upstream repository is expected to be at "github.com/vancluever/terraform-provider-acme". If the expected path is not correct, you should check the values of these fields (current values shown): tfbridge.ProviderInfo{ GitHubHost: "github.com", GitHubOrg: "vancluever", Name: "acme", TFProviderModuleVersion: "", } The original error is: error running 'go mod download -json github.com/vancluever/terraform-provider-acme' in "/Users/ianwahbe/go/src/github.com/pulumiverse/pulumi-acme/provider" dir for module: exit status 1 Output: { "Path": "github.com/vancluever/terraform-provider-acme", "Error": "module github.com/vancluever/terraform-provider-acme: not a known dependency" } ``` This error message will appear once, regardless of how many resources/datasources are in the provider. --- For review: - e568cc4a1f346485ba7234be67321c75d0b629cb creates the new error. - 1ae3dd68e14a359c272a4068873ee50b4a489d8a catches the new error and displays the warning. --- pkg/tfgen/generate.go | 54 +++++++++++++++++++++++++++++++++++++++--- pkg/tfgen/source.go | 55 ++++++++++++++++++++++++++++++------------- 2 files changed, 90 insertions(+), 19 deletions(-) diff --git a/pkg/tfgen/generate.go b/pkg/tfgen/generate.go index f98978cae..710eb79d9 100644 --- a/pkg/tfgen/generate.go +++ b/pkg/tfgen/generate.go @@ -75,6 +75,10 @@ type Generator struct { editRules editRules convertedCode map[string][]byte + + // Set if we can't find the docs repo and we have already printed a warning + // message. + noDocsRepo bool } type Language string @@ -1207,10 +1211,11 @@ func (g *Generator) gatherResource(rawname string, if !isProvider { source := NewGitRepoDocsSource(g) pd, err := getDocsForResource(g, source, ResourceDocs, rawname, info) - if err != nil { + if err == nil { + entityDocs = pd + } else if !g.checkNoDocsError(err) { return nil, err } - entityDocs = pd } else { entityDocs.Description = fmt.Sprintf( "The provider type for the %s package. By default, resources use package-wide configuration\n"+ @@ -1400,7 +1405,7 @@ func (g *Generator) gatherDataSource(rawname string, // Collect documentation information for this data source. source := NewGitRepoDocsSource(g) entityDocs, err := getDocsForResource(g, source, DataSourceDocs, rawname, info) - if err != nil { + if err != nil && !g.checkNoDocsError(err) { return nil, err } @@ -1537,6 +1542,49 @@ func (g *Generator) gatherOverlays() (moduleMap, error) { return modules, nil } +func (g *Generator) checkNoDocsError(err error) bool { + var e GetRepoPathErr + if !errors.As(err, &e) { + // Not the right kind of error + return false + } + + // If we have already warned, we can just discard the message + if !g.noDocsRepo { + g.logMissingRepoPath(e) + } + g.noDocsRepo = true + return true +} + +func (g *Generator) logMissingRepoPath(err GetRepoPathErr) { + msg := `Unable to find the upstream provider's documentation: +The upstream repository is expected to be at %q. +%s +The original error is: %s` + + var correction string + if g.info.UpstreamRepoPath != "" { + correction = fmt.Sprintf(` +The upstream repository path has been overridden, but the specified path is invalid. +You should check the value of: +tfbridge.ProviderInfo{ + UpstreamRepoPath: %q, +}`, g.info.UpstreamRepoPath) + } else { + correction = fmt.Sprintf(` +If the expected path is not correct, you should check the values of these fields (current values shown): +tfbridge.ProviderInfo{ + GitHubHost: %q, + GitHubOrg: %q, + Name: %q, + TFProviderModuleVersion: %q, +}`, g.info.GetGitHubHost(), g.info.GetGitHubOrg(), g.info.Name, g.info.GetProviderModuleVersion()) + } + + g.sink.Warningf(&diag.Diag{Message: msg}, err.Expected, correction, err.Underlying) +} + // emitProjectMetadata emits the Pulumi.yaml project file into the package's root directory. func (g *Generator) emitProjectMetadata(name tokens.Package, language Language) error { w, err := newGenWriter(tfgen, g.root, "Pulumi.yaml") diff --git a/pkg/tfgen/source.go b/pkg/tfgen/source.go index 83bed4c7e..bd0326c97 100644 --- a/pkg/tfgen/source.go +++ b/pkg/tfgen/source.go @@ -16,14 +16,13 @@ package tfgen import ( "encoding/json" + "errors" "fmt" "os" "os/exec" "path/filepath" "sync" - "github.com/pulumi/pulumi/sdk/v3/go/common/diag" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" ) @@ -43,7 +42,6 @@ type DocFile struct { func NewGitRepoDocsSource(g *Generator) DocsSource { return &gitRepoSource{ - sink: g.sink, docRules: g.info.DocRules, upstreamRepoPath: g.info.UpstreamRepoPath, org: g.info.GetGitHubOrg(), @@ -55,7 +53,6 @@ func NewGitRepoDocsSource(g *Generator) DocsSource { } type gitRepoSource struct { - sink diag.Sink docRules *tfbridge.DocRuleInfo upstreamRepoPath string org string @@ -86,9 +83,7 @@ func (gh *gitRepoSource) getFile( var err error repoPath, err = getRepoPath(gh.githost, gh.org, gh.provider, gh.providerModuleVersion) if err != nil { - msg := "Skip getMarkdownDetails(rawname=%q) because getRepoPath(%q, %q, %q, %q) failed: %v" - gh.sink.Debugf(&diag.Diag{Message: msg}, rawname, gh.githost, gh.org, gh.provider, gh.providerModuleVersion, err) - return nil, nil + return nil, fmt.Errorf("repo for token %q: %w", rawname, err) } } @@ -98,22 +93,41 @@ func (gh *gitRepoSource) getFile( possibleMarkdownNames = append(possibleMarkdownNames, info.Source) } - markdownBytes, markdownFileName, found := readMarkdown(repoPath, kind, possibleMarkdownNames) - if !found { - return nil, nil - } + return readMarkdown(repoPath, kind, possibleMarkdownNames) +} - return &DocFile{Content: markdownBytes, FileName: markdownFileName}, nil +// An error that represents a missing repo path directory. +type GetRepoPathErr struct { + Expected string + Underlying error +} + +func (e GetRepoPathErr) Error() string { + return fmt.Sprintf("Unable to access repository at %s: %s", e.Expected, e.Underlying.Error()) +} + +func (e GetRepoPathErr) Unwrap() error { + return e.Underlying } var repoPaths sync.Map -func getRepoPath(gitHost string, org string, provider string, version string) (string, error) { +func getRepoPath(gitHost string, org string, provider string, version string) (_ string, err error) { moduleCoordinates := fmt.Sprintf("%s/%s/terraform-provider-%s", gitHost, org, provider) if version != "" { moduleCoordinates = fmt.Sprintf("%s/%s", moduleCoordinates, version) } + defer func() { + if err == nil { + return + } + err = GetRepoPathErr{ + Expected: moduleCoordinates, + Underlying: err, + } + }() + if path, ok := repoPaths.Load(moduleCoordinates); ok { return path.(string), nil } @@ -178,15 +192,24 @@ func getMarkdownNames(packagePrefix, rawName string, globalInfo *tfbridge.DocRul } // readMarkdown searches all possible locations for the markdown content -func readMarkdown(repo string, kind DocKind, possibleLocations []string) ([]byte, string, bool) { +func readMarkdown(repo string, kind DocKind, possibleLocations []string) (*DocFile, error) { locationPrefix := getDocsPath(repo, kind) for _, name := range possibleLocations { location := filepath.Join(locationPrefix, name) markdownBytes, err := os.ReadFile(location) if err == nil { - return markdownBytes, name, true + return &DocFile{markdownBytes, name}, nil + } else if !os.IsNotExist(err) && !errors.Is(err, &os.PathError{}) { + // Missing doc files are expected and OK. + // + // If the file we expect is actually a directory (PathError), that + // is also OK. + // + // Other errors (such as permission errors) indicate a problem + // with the host system, and should be reported. + return nil, fmt.Errorf("%s: %w", location, err) } } - return nil, "", false + return nil, nil }