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

Remove panic on imports for TF Plugin Framework #1317

Closed
wants to merge 6 commits into from
Closed

Remove panic on imports for TF Plugin Framework #1317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4fd5348
Return nil instead of panic from block schema
t0yv0 Jul 31, 2023
3675d22
resolve merge conflict
guineveresaenger Aug 2, 2023
2ccf5c2
fix linting
guineveresaenger Aug 2, 2023
aa0e4d4
Remove panic from Type schema
guineveresaenger Aug 2, 2023
b7c2bae
Return a custom error message and log it during getDefaultValue
guineveresaenger Aug 2, 2023
4427237
Create global err and fix logic. Return explicit error if error is no…
guineveresaenger Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pf/internal/schemashim/attr_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package schemashim

import (
"fmt"
bridge "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"

pfattr "github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/types"
Expand Down Expand Up @@ -58,7 +59,7 @@ func (*attrSchema) DefaultFunc() shim.SchemaDefaultFunc {
func (*attrSchema) DefaultValue() (interface{}, error) {
// DefaultValue() should not be called by tfgen, but it currently may be called by ExtractInputsFromOutputs, so
// returning nil is better than a panic.
return nil, nil
return nil, bridge.SchemaDefaultValueErr
}

func (s *attrSchema) Description() string {
Expand Down
5 changes: 4 additions & 1 deletion pf/internal/schemashim/block_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package schemashim

import (
"fmt"
bridge "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"

"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
Expand Down Expand Up @@ -120,7 +121,9 @@ func (*blockSchema) DefaultFunc() shim.SchemaDefaultFunc {
}

func (*blockSchema) DefaultValue() (interface{}, error) {
panic("DefaultValue() should not be called during schema generation")
// DefaultValue() should not be called by tfgen, but it currently may be called by ExtractInputsFromOutputs, so
// returning nil is better than a panic.
return nil, bridge.SchemaDefaultValueErr
}

func (*blockSchema) ExactlyOneOf() []string {
Expand Down
6 changes: 4 additions & 2 deletions pf/internal/schemashim/type_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ package schemashim
import (
pfattr "github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/types"

"github.com/pulumi/pulumi-terraform-bridge/pf/internal/pfutils"
bridge "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
)
Expand Down Expand Up @@ -93,7 +93,9 @@ func (*typeSchema) DefaultFunc() shim.SchemaDefaultFunc {
}

func (*typeSchema) DefaultValue() (interface{}, error) {
panic("DefaultValue() should not be called during schema generation")
// DefaultValue() should not be called by tfgen, but it currently may be called by ExtractInputsFromOutputs, so
// returning nil is better than a panic.
return nil, bridge.SchemaDefaultValueErr
}

func (*typeSchema) Description() string {
Expand Down
3 changes: 2 additions & 1 deletion pf/tests/internal/testprovider/cmd/pulumi-resource-testbridge/bridge-metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"resources": {
"testbridge:index/testnest:Testnest": "testbridge_testnest",
"testbridge:index/testres:AutoNameRes": "testbridge_autoname_res",
"testbridge:index/testnestattr:Testnestattr": "testbridge_testnestattr",
"testbridge:index/testres:Privst": "testbridge_privst",
"testbridge:index/testres:TestConfigRes": "testbridge_testconfigres",
"testbridge:index/testres:TestDefaultInfoRes": "testbridge_test_default_info_res",
Expand Down Expand Up @@ -65,4 +66,4 @@
"stringDefaultinfoConfigProp": "string_defaultinfo_config_prop"
}
}
}
}
132 changes: 132 additions & 0 deletions pf/tests/internal/testprovider/cmd/pulumi-resource-testbridge/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,84 @@
},
"type": "object"
},
"testbridge:index/TestnestService:TestnestService": {
"properties": {
"intport": {
"type": "integer"
},
"ports": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestServicePort:TestnestServicePort"
}
},
"protocol": {
"type": "string"
}
},
"type": "object",
"required": [
"intport",
"ports",
"protocol"
]
},
"testbridge:index/TestnestServicePort:TestnestServicePort": {
"properties": {
"handlers": {
"type": "array",
"items": {
"type": "string"
}
},
"port": {
"type": "integer"
}
},
"type": "object",
"required": [
"port"
]
},
"testbridge:index/TestnestattrService:TestnestattrService": {
"properties": {
"intport": {
"type": "integer"
},
"ports": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestattrServicePort:TestnestattrServicePort"
}
},
"protocol": {
"type": "string"
}
},
"type": "object",
"required": [
"intport",
"ports",
"protocol"
]
},
"testbridge:index/TestnestattrServicePort:TestnestattrServicePort": {
"properties": {
"handlers": {
"type": "array",
"items": {
"type": "string"
}
},
"port": {
"type": "integer"
}
},
"type": "object",
"required": [
"port"
]
},
"testbridge:index/TestresService:TestresService": {
"properties": {
"intport": {
Expand Down Expand Up @@ -187,6 +265,13 @@
"$ref": "#/types/testbridge:index/TestnestRule:TestnestRule"
},
"description": "List of rules to apply to the ruleset.\n"
},
"services": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestService:TestnestService"
},
"description": "services: tests ListNestedAttributes support\n"
}
},
"inputProperties": {
Expand All @@ -196,6 +281,13 @@
"$ref": "#/types/testbridge:index/TestnestRule:TestnestRule"
},
"description": "List of rules to apply to the ruleset.\n"
},
"services": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestService:TestnestService"
},
"description": "services: tests ListNestedAttributes support\n"
}
},
"stateInputs": {
Expand All @@ -207,6 +299,46 @@
"$ref": "#/types/testbridge:index/TestnestRule:TestnestRule"
},
"description": "List of rules to apply to the ruleset.\n"
},
"services": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestService:TestnestService"
},
"description": "services: tests ListNestedAttributes support\n"
}
},
"type": "object"
}
},
"testbridge:index/testnestattr:Testnestattr": {
"properties": {
"services": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestattrService:TestnestattrService"
},
"description": "services: tests ListNestedAttributes support\n"
}
},
"inputProperties": {
"services": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestattrService:TestnestattrService"
},
"description": "services: tests ListNestedAttributes support\n"
}
},
"stateInputs": {
"description": "Input properties used for looking up and filtering Testnestattr resources.\n",
"properties": {
"services": {
"type": "array",
"items": {
"$ref": "#/types/testbridge:index/TestnestattrService:TestnestattrService"
},
"description": "services: tests ListNestedAttributes support\n"
}
},
"type": "object"
Expand Down
4 changes: 4 additions & 0 deletions pf/tests/internal/testprovider/testbridge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ func SyntheticTestBridgeProvider() tfbridge.ProviderInfo {
},
},
},
"testbridge_testnestattr": {
Tok: "testbridge:index/testnestattr:Testnestattr",
},
"testbridge_testcompres": {Tok: "testbridge:index/testres:Testcompres"},
"testbridge_testconfigres": {Tok: "testbridge:index/testres:TestConfigRes"},

Expand Down Expand Up @@ -199,6 +202,7 @@ func (p *syntheticProvider) Resources(context.Context) []func() resource.Resourc
return []func() resource.Resource{
newTestres,
newTestnest,
newTestnestattr,
newTestCompRes,
newTestConfigRes,
newTestDefaultInfoRes,
Expand Down
69 changes: 69 additions & 0 deletions pf/tests/internal/testprovider/testbridge_resource_testnest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,66 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/types"
)

type testnest struct{}

var _ resource.Resource = &testnest{}
var _ resource.ResourceWithImportState = &testnest{}

func newTestnest() resource.Resource {
return &testnest{}
}

func (*testnest) schema() rschema.Schema {
return rschema.Schema{
Attributes: map[string]rschema.Attribute{
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding as requirement for testing import against this resource.

"id": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
// Example borrowed from https://github.com/fly-apps/terraform-provider-fly/blob/28438713f2bdf08dbd0aa2fae9d74baaca9845f1/internal/provider/machine_resource.go#L176
"services": schema.ListNestedAttribute{
MarkdownDescription: "services: tests ListNestedAttributes support",
Optional: true,
NestedObject: schema.NestedAttributeObject{
Attributes: map[string]schema.Attribute{
"ports": schema.ListNestedAttribute{
MarkdownDescription: "External ports and handlers",
Required: true,
NestedObject: schema.NestedAttributeObject{
Attributes: map[string]schema.Attribute{
"port": schema.Int64Attribute{
MarkdownDescription: "External port",
Required: true,
},
"handlers": schema.ListAttribute{
ElementType: types.StringType,
MarkdownDescription: "How the edge should process requests",
Optional: true,
},
},
},
},
"protocol": schema.StringAttribute{
MarkdownDescription: "network protocol",
Required: true,
},
// TODO internal_port gets mangled to internalPort by Pulumi renaming and does
// not work end-to-end yet.
"intport": schema.Int64Attribute{
MarkdownDescription: "Port application listens on internally",
Required: true,
},
},
},
},
},
Blocks: map[string]schema.Block{
"rules": schema.ListNestedBlock{
MarkdownDescription: "List of rules to apply to the ruleset.",
Expand Down Expand Up @@ -98,3 +146,24 @@ func (e *testnest) Update(ctx context.Context, req resource.UpdateRequest, resp
func (e *testnest) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
panic("unimplemented")
}

// ImportState is called when the provider must import the state of a resource instance. This method must return enough
// state so the Read method can properly refresh the full resource.
//
// If setting an attribute with the import identifier, it is recommended to use the ImportStatePassthroughID() call in
// this method.
func (e *testnest) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
type ruleModel struct{}

type model struct {
ID types.String `tfsdk:"id"`
Rules []ruleModel `tfsdk:"rules"`
Services []ServiceModel `tfsdk:"services"`
}

resp.Diagnostics = resp.State.Set(ctx, &model{
ID: types.StringValue(req.ID),
Rules: []ruleModel{},
Services: []ServiceModel{},
})
}
Loading
Loading