tfsdk: Introduce ServeOpts Address field, deprecate Name field (#296)

Reference: #295

.changelog/296.txt

@@ -0,0 +1,7 @@
+```release-note:note
+tfsdk: The `ServeOpts` type `Name` field has been deprecated in preference of the `Address` field. The `Name` field will be removed prior to version 1.0.0.
+```
+
+```release-note:enhancement
+tfsdk: Added `ServeOpts` type `Address` field, which should contain the full provider address in hostname/namespace/type format.
+```

tfsdk/serve.go

@@ -23,19 +23,6 @@ type server struct {
 	contextCancelsMu sync.Mutex
 }
 
-// ServeOpts are options for serving the provider.
-type ServeOpts struct {
-	// Name is the name of the provider, in full address form. For example:
-	// registry.terraform.io/hashicorp/random.
-	Name string
-
-	// Debug runs the provider in a mode acceptable for debugging and testing
-	// processes, such as delve, by managing the process lifecycle. Information
-	// needed for Terraform CLI to connect to the provider is output to stdout.
-	// os.Interrupt (Ctrl-c) can be used to stop the provider.
-	Debug bool
-}
-
 // NewProtocol6Server returns a tfprotov6.ProviderServer implementation based
 // on the passed Provider implementation.
 //
@@ -73,13 +60,19 @@ func NewProtocol6ProviderServerWithError(p Provider) (func() tfprotov6.ProviderS
 
 // Serve serves a provider, blocking until the context is canceled.
 func Serve(ctx context.Context, providerFunc func() Provider, opts ServeOpts) error {
+	err := opts.validate(ctx)
+
+	if err != nil {
+		return fmt.Errorf("unable to validate ServeOpts: %w", err)
+	}
+
 	var tf6serverOpts []tf6server.ServeOpt
 
 	if opts.Debug {
 		tf6serverOpts = append(tf6serverOpts, tf6server.WithManagedDebug())
 	}
 
-	return tf6server.Serve(opts.Name, func() tfprotov6.ProviderServer {
+	return tf6server.Serve(opts.address(ctx), func() tfprotov6.ProviderServer {
 		return &server{
 			p: providerFunc(),
 		}

tfsdk/serve_opts.go

@@ -0,0 +1,89 @@
+package tfsdk
+
+import (
+	"context"
+	"fmt"
+	"strings"
+)
+
+// ServeOpts are options for serving the provider.
+type ServeOpts struct {
+	// Address is the full address of the provider. Full address form has three
+	// parts separated by forward slashes (/): Hostname, namespace, and
+	// provider type ("name").
+	//
+	// For example: registry.terraform.io/hashicorp/random.
+	Address string
+
+	// Name is the name of the provider, in full address form. For example:
+	// registry.terraform.io/hashicorp/random.
+	//
+	// Deprecated: Use Address field instead.
+	Name string
+
+	// Debug runs the provider in a mode acceptable for debugging and testing
+	// processes, such as delve, by managing the process lifecycle. Information
+	// needed for Terraform CLI to connect to the provider is output to stdout.
+	// os.Interrupt (Ctrl-c) can be used to stop the provider.
+	Debug bool
+}
+
+// Get provider address, based on whether Address or Name is specified.
+//
+// Deprecated: Will be removed in preference of just using the Address field.
+func (opts ServeOpts) address(_ context.Context) string {
+	if opts.Address != "" {
+		return opts.Address
+	}
+
+	return opts.Name
+}
+
+// Validate a given provider address. This is only used for the Address field
+// to preserve backwards compatibility for the Name field.
+//
+// This logic is manually implemented over importing
+// github.com/hashicorp/terraform-registry-address as its functionality such as
+// ParseAndInferProviderSourceString and ParseRawProviderSourceString allow
+// shorter address formats, which would then require post-validation anyways.
+func (opts ServeOpts) validateAddress(_ context.Context) error {
+	addressParts := strings.Split(opts.Address, "/")
+	formatErr := fmt.Errorf("expected hostname/namespace/type format, got: %s", opts.Address)
+
+	if len(addressParts) != 3 {
+		return formatErr
+	}
+
+	if addressParts[0] == "" || addressParts[1] == "" || addressParts[2] == "" {
+		return formatErr
+	}
+
+	return nil
+}
+
+// Validation checks for provider defined ServeOpts.
+//
+// Current checks which return errors:
+//
+//    - If both Address and Name are set
+//    - If neither Address nor Name is set
+//    - If Address is set, it is a valid full provider address
+func (opts ServeOpts) validate(ctx context.Context) error {
+	if opts.Address == "" && opts.Name == "" {
+		return fmt.Errorf("either Address or Name must be provided")
+	}
+
+	if opts.Address != "" && opts.Name != "" {
+		return fmt.Errorf("only one of Address or Name should be provided")
+	}
+
+	if opts.Address != "" {
+		err := opts.validateAddress(ctx)
+
+		if err != nil {
+			return fmt.Errorf("unable to validate Address: %w", err)
+		}
+	}
+
+	return nil
+}

tfsdk/serve_opts_test.go

@@ -0,0 +1,169 @@
+package tfsdk
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func TestServeOptsAddress(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		serveOpts ServeOpts
+		expected  string
+	}{
+		"Address": {
+			serveOpts: ServeOpts{
+				Address: "registry.terraform.io/hashicorp/testing",
+			},
+			expected: "registry.terraform.io/hashicorp/testing",
+		},
+		"Address-and-Name-both": {
+			serveOpts: ServeOpts{
+				Address: "registry.terraform.io/hashicorp/testing",
+				Name:    "testing",
+			},
+			expected: "registry.terraform.io/hashicorp/testing",
+		},
+		"Name": {
+			serveOpts: ServeOpts{
+				Name: "testing",
+			},
+			expected: "testing",
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			got := testCase.serveOpts.address(context.Background())
+
+			if got != testCase.expected {
+				t.Fatalf("expected %q, got: %s", testCase.expected, got)
+			}
+		})
+	}
+}
+
+func TestServeOptsValidate(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		serveOpts     ServeOpts
+		expectedError error
+	}{
+		"Address": {
+			serveOpts: ServeOpts{
+				Address: "registry.terraform.io/hashicorp/testing",
+			},
+		},
+		"Address-and-Name-both": {
+			serveOpts: ServeOpts{
+				Address: "registry.terraform.io/hashicorp/testing",
+				Name:    "testing",
+			},
+			expectedError: fmt.Errorf("only one of Address or Name should be provided"),
+		},
+		"Address-and-Name-missing": {
+			serveOpts:     ServeOpts{},
+			expectedError: fmt.Errorf("either Address or Name must be provided"),
+		},
+		"Address-invalid-type-only": {
+			serveOpts: ServeOpts{
+				Address: "testing",
+			},
+			expectedError: fmt.Errorf("unable to validate Address: expected hostname/namespace/type format, got: testing"),
+		},
+		"Address-invalid-missing-hostname": {
+			serveOpts: ServeOpts{
+				Address: "hashicorp/testing",
+			},
+			expectedError: fmt.Errorf("unable to validate Address: expected hostname/namespace/type format, got: hashicorp/testing"),
+		},
+		"Name": {
+			serveOpts: ServeOpts{
+				Name: "testing",
+			},
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			err := testCase.serveOpts.validate(context.Background())
+
+			if err != nil {
+				if testCase.expectedError == nil {
+					t.Fatalf("expected no error, got: %s", err)
+				}
+
+				if !strings.Contains(err.Error(), testCase.expectedError.Error()) {
+					t.Fatalf("expected error %q, got: %s", testCase.expectedError, err)
+				}
+			}
+
+			if err == nil && testCase.expectedError != nil {
+				t.Fatalf("got no error, expected: %s", testCase.expectedError)
+			}
+		})
+	}
+}
+
+func TestServeOptsValidateAddress(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		serveOpts     ServeOpts
+		expectedError error
+	}{
+		"valid": {
+			serveOpts: ServeOpts{
+				Address: "registry.terraform.io/hashicorp/testing",
+			},
+		},
+		"invalid-type-only": {
+			serveOpts: ServeOpts{
+				Address: "testing",
+			},
+			expectedError: fmt.Errorf("expected hostname/namespace/type format, got: testing"),
+		},
+		"invalid-missing-hostname": {
+			serveOpts: ServeOpts{
+				Address: "hashicorp/testing",
+			},
+			expectedError: fmt.Errorf("expected hostname/namespace/type format, got: hashicorp/testing"),
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			err := testCase.serveOpts.validateAddress(context.Background())
+
+			if err != nil {
+				if testCase.expectedError == nil {
+					t.Fatalf("expected no error, got: %s", err)
+				}
+
+				if !strings.Contains(err.Error(), testCase.expectedError.Error()) {
+					t.Fatalf("expected error %q, got: %s", testCase.expectedError, err)
+				}
+			}
+
+			if err == nil && testCase.expectedError != nil {
+				t.Fatalf("got no error, expected: %s", testCase.expectedError)
+			}
+		})
+	}
+}