diff --git a/dynamic/go.mod b/dynamic/go.mod index 750036e8d..d8758cedd 100644 --- a/dynamic/go.mod +++ b/dynamic/go.mod @@ -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 diff --git a/dynamic/internal/shim/run/loader.go b/dynamic/internal/shim/run/loader.go index 5be199a9e..22398ce78 100644 --- a/dynamic/internal/shim/run/loader.go +++ b/dynamic/internal/shim/run/loader.go @@ -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()) diff --git a/dynamic/main.go b/dynamic/main.go index e7c108a64..b737cba8b 100644 --- a/dynamic/main.go +++ b/dynamic/main.go @@ -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 } diff --git a/dynamic/parameterize/args.go b/dynamic/parameterize/args.go index aa7352899..276719a0e 100644 --- a/dynamic/parameterize/args.go +++ b/dynamic/parameterize/args.go @@ -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. @@ -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 } @@ -39,6 +48,7 @@ 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. @@ -46,62 +56,80 @@ type LocalArgs struct { 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: " + - "[upstreamRepoPath=]", - ) - } + 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=' 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: [version] [fullDocs=]") + var version string + if len(args) > 1 { + version = args[1] } + + return Args{Remote: &RemoteArgs{ + Name: args[0], + Version: version, + Docs: fullDocs, + }}, nil } diff --git a/dynamic/parameterize/args_test.go b/dynamic/parameterize/args_test.go index f3530e36b..4e51c7218 100644 --- a/dynamic/parameterize/args_test.go +++ b/dynamic/parameterize/args_test.go @@ -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) { @@ -36,15 +39,13 @@ 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: [upstreamRepoPath=]", - ), + 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", @@ -52,13 +53,6 @@ func TestParseArgs(t *testing.T) { }, }, }, - { - 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"}, @@ -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: [version] [fullDocs=]"), + 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: [version] [fullDocs=]"), + 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=' 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", @@ -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", @@ -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", @@ -118,9 +110,10 @@ 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=' or be empty", + `invalid argument "invalid-input" for "--fullDocs" flag: strconv.ParseBool: parsing "invalid-input": invalid syntax`, ), }, } @@ -128,7 +121,8 @@ func TestParseArgs(t *testing.T) { 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)