Skip to content

Commit

Permalink
Switch pulumi-terraform-provider to use cobra for args parsing
Browse files Browse the repository at this point in the history
This gets us out of the business of writing flag parsing. This does change the format of
options. Before, they looked like:

```console
$ pulumi package get-schema registry.opentofu.org/owner/provider version fullDocs=true
```

They now look like:

```console
$ pulumi package get-schema -- registry.opentofu.org/owner/provider version --fullDocs
```
  • Loading branch information
iwahbe committed Dec 13, 2024
1 parent 4f8782f commit 316be51
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 77 deletions.
2 changes: 1 addition & 1 deletion dynamic/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ require (
github.com/skeema/knownhosts v1.2.2 // indirect
github.com/spf13/afero v1.9.5
github.com/spf13/cast v1.5.0 // indirect
github.com/spf13/cobra v1.8.0 // indirect
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5 // indirect
github.com/texttheater/golang-levenshtein v1.0.1 // indirect
github.com/uber/jaeger-client-go v2.30.0+incompatible // indirect
Expand Down
2 changes: 1 addition & 1 deletion dynamic/internal/shim/run/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func NamedProvider(ctx context.Context, key, version string) (Provider, error) {

v, err := getproviders.ParseVersionConstraints(version)
if err != nil {
return nil, err
return nil, fmt.Errorf("could not parse version constraint %q: %w", version, err)
}

return getProviderServer(ctx, p, v, disco.New())
Expand Down
2 changes: 1 addition & 1 deletion dynamic/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func initialSetup() (info.Provider, pfbridge.ProviderMetadata, func() error) {
args = value.IntoArgs()
case *plugin.ParameterizeArgs:
var err error
args, err = parameterize.ParseArgs(params.Args)
args, err = parameterize.ParseArgs(ctx, params.Args)
if err != nil {
return plugin.ParameterizeResponse{}, err
}
Expand Down
126 changes: 77 additions & 49 deletions dynamic/parameterize/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,16 @@
package parameterize

import (
"bytes"
"context"
"errors"
"fmt"
"strings"

"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
"github.com/spf13/cobra"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
)

// Args represents a parsed CLI argument from a parameterize call.
Expand All @@ -31,6 +39,7 @@ type RemoteArgs struct {
Name string
// Version is the (possibly empty) version constraint on the provider.
Version string

// Docs indicates if full schema documentation should be generated.
Docs bool
}
Expand All @@ -39,69 +48,88 @@ type RemoteArgs struct {
type LocalArgs struct {
// Path is the path to the provider binary. It can be relative or absolute.
Path string

// UpstreamRepoPath (if provided) is the local path to the dynamically bridged Terraform provider's repo.
//
// If set, full documentation will be generated for the provider.
// If not set, only documentation from the TF provider's schema will be used.
UpstreamRepoPath string
}

func ParseArgs(args []string) (Args, error) {
// Check for a leading '.' or '/' to indicate a path
if len(args) >= 1 &&
(strings.HasPrefix(args[0], "./") || strings.HasPrefix(args[0], "/")) {
func ParseArgs(ctx context.Context, a []string) (Args, error) {
var args Args
var fullDocs bool
var upstreamRepoPath string
cmd := cobra.Command{
Use: "./local | remote version",
RunE: func(cmd *cobra.Command, a []string) error {
var err error
args, err = parseArgs(cmd.Context(), a, fullDocs, upstreamRepoPath)
return err
},
Args: cobra.RangeArgs(1, 2),
}
cmd.Flags().BoolVar(&fullDocs, "fullDocs", false,
"Generate a schema with full docs, at the expense of speed")
cmd.Flags().StringVar(&upstreamRepoPath, "upstreamRepoPath", "",
"Specify a local file path to the root of the Git repository of the provider being dynamically bridged")

// We hide docs flags since they are not intended for end users, and they may not be stable.
contract.AssertNoErrorf(
errors.Join(
cmd.Flags().MarkHidden("fullDocs"),
cmd.Flags().MarkHidden("upstreamRepoPath"),
),
"impossible - these are static values and should never fail",
)

cmd.SetArgs(a)

var out bytes.Buffer
cmd.SetOut(&out)
cmd.SetErr(&out)
defer func() {
if out.Len() == 0 {
return
}
tfbridge.GetLogger(ctx).Info(out.String())
}()

return args, cmd.ExecuteContext(ctx)
}

func parseArgs(_ context.Context, args []string, fullDocs bool, upstreamRepoPath string) (Args, error) {
// If we see a local prefix (starts with '.' or '/'), parse args for a local provider
if strings.HasPrefix(args[0], ".") || strings.HasPrefix(args[0], "/") {
if len(args) > 1 {
docsArg := args[1]
upstreamRepoPath, found := strings.CutPrefix(docsArg, "upstreamRepoPath=")
if !found {
return Args{}, fmt.Errorf(
"path based providers are only parameterized by 2 arguments: <path> " +
"[upstreamRepoPath=<path/to/files>]",
)
}
return Args{}, fmt.Errorf("local providers only accept one argument, found %d", len(args))
}
if fullDocs {
msg := "fullDocs only applies to remote providers"
if upstreamRepoPath == "" {
return Args{}, fmt.Errorf(
"upstreamRepoPath must be set to a non-empty value: " +
"upstreamRepoPath=path/to/files",
)
msg += ", consider specifying upstreamRepoPath instead"
}
return Args{Local: &LocalArgs{Path: args[0], UpstreamRepoPath: upstreamRepoPath}}, nil
return Args{}, errors.New(msg)
}
return Args{Local: &LocalArgs{Path: args[0]}}, nil
return Args{Local: &LocalArgs{Path: args[0], UpstreamRepoPath: upstreamRepoPath}}, nil
}

// This is a registry based provider
var remote RemoteArgs
switch len(args) {
// The third argument, if any, is the full docs option for when we need to generate docs
case 3:
docsArg := args[2]
errMsg := "expected third parameterized argument to be 'fullDocs=<true|false>' or be empty"

fullDocs, found := strings.CutPrefix(docsArg, "fullDocs=")
if !found {
return Args{}, fmt.Errorf("%s", errMsg)
}

switch fullDocs {
case "true":
remote.Docs = true
case "false":
// Do nothing
default:
return Args{}, fmt.Errorf("%s", errMsg)
if upstreamRepoPath != "" {
msg := "upstreamRepoPath only applies to local providers"
if upstreamRepoPath == "" {
msg += ", consider specifying fullDocs instead"
}
return Args{}, errors.New(msg)
}

fallthrough
// The second argument, if any is the version
case 2:
remote.Version = args[1]
fallthrough
// The first argument is the provider name
case 1:
remote.Name = args[0]
return Args{Remote: &remote}, nil
default:
return Args{}, fmt.Errorf("expected to be parameterized by 1-3 arguments: <name> [version] [fullDocs=<true|false>]")
var version string
if len(args) > 1 {
version = args[1]
}

return Args{Remote: &RemoteArgs{
Name: args[0],
Version: version,
Docs: fullDocs,
}}, nil
}
44 changes: 19 additions & 25 deletions dynamic/parameterize/args_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@
package parameterize

import (
"context"
"testing"

"github.com/hexops/autogold/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/bridgetest"
)

func TestParseArgs(t *testing.T) {
Expand All @@ -36,29 +39,20 @@ func TestParseArgs(t *testing.T) {
expect: Args{Local: &LocalArgs{Path: "./my-provider"}},
},
{
name: "local too many args",
args: []string{"./my-provider", "nonsense"},
errMsg: autogold.Expect(
"path based providers are only parameterized by 2 arguments: <path> [upstreamRepoPath=<path/to/files>]",
),
name: "local too many args",
args: []string{"./my-provider", "nonsense"},
errMsg: autogold.Expect(`local providers only accept one argument, found 2`),
},
{
name: "local with docs location",
args: []string{"./my-provider", "upstreamRepoPath=./my-provider"},
args: []string{"./my-provider", "--upstreamRepoPath=./my-provider"},
expect: Args{
Local: &LocalArgs{
Path: "./my-provider",
UpstreamRepoPath: "./my-provider",
},
},
},
{
name: "local empty upstreamRepoPath",
args: []string{"./my-provider", "upstreamRepoPath="},
errMsg: autogold.Expect(
"upstreamRepoPath must be set to a non-empty value: upstreamRepoPath=path/to/files",
),
},
{
name: "remote",
args: []string{"my-registry.io/typ"},
Expand All @@ -75,19 +69,17 @@ func TestParseArgs(t *testing.T) {
{
name: "no args",
args: []string{},
errMsg: autogold.Expect("expected to be parameterized by 1-3 arguments: <name> [version] [fullDocs=<true|false>]"),
errMsg: autogold.Expect("accepts between 1 and 2 arg(s), received 0"),
},
{
name: "too many args",
args: []string{"arg1", "arg2", "arg3", "arg4"},
errMsg: autogold.Expect("expected to be parameterized by 1-3 arguments: <name> [version] [fullDocs=<true|false>]"),
errMsg: autogold.Expect("accepts between 1 and 2 arg(s), received 4"),
},
{
name: "invalid third arg",
args: []string{"arg1", "arg2", "arg3"},
errMsg: autogold.Expect(
"expected third parameterized argument to be 'fullDocs=<true|false>' or be empty",
),
name: "invalid third arg",
args: []string{"arg1", "arg2", "arg3"},
errMsg: autogold.Expect(`accepts between 1 and 2 arg(s), received 3`),
},
{
name: "empty third arg",
Expand All @@ -100,7 +92,7 @@ func TestParseArgs(t *testing.T) {
},
{
name: "valid third arg true",
args: []string{"my-registry.io/typ", "1.2.3", "fullDocs=true"},
args: []string{"my-registry.io/typ", "1.2.3", "--fullDocs=true"},
expect: Args{Remote: &RemoteArgs{
Name: "my-registry.io/typ",
Version: "1.2.3",
Expand All @@ -109,7 +101,7 @@ func TestParseArgs(t *testing.T) {
},
{
name: "valid third arg false",
args: []string{"my-registry.io/typ", "1.2.3", "fullDocs=false"},
args: []string{"my-registry.io/typ", "1.2.3", "--fullDocs=false"},
expect: Args{Remote: &RemoteArgs{
Name: "my-registry.io/typ",
Version: "1.2.3",
Expand All @@ -118,17 +110,19 @@ func TestParseArgs(t *testing.T) {
},
{
name: "third arg invalid input",
args: []string{"my-registry.io/typ", "1.2.3", "fullDocs=invalid-input"},
args: []string{"my-registry.io/typ", "1.2.3", "--fullDocs=invalid-input"},
//nolint:lll
errMsg: autogold.Expect(
"expected third parameterized argument to be 'fullDocs=<true|false>' or be empty",
`invalid argument "invalid-input" for "--fullDocs" flag: strconv.ParseBool: parsing "invalid-input": invalid syntax`,
),
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
actual, err := ParseArgs(tt.args)
ctx := bridgetest.InitLogging(t, context.Background(), nil)
actual, err := ParseArgs(ctx, tt.args)
if tt.errMsg == nil {
require.NoError(t, err)
assert.Equal(t, tt.expect, actual)
Expand Down

0 comments on commit 316be51

Please sign in to comment.