search

gravitational / teleport-plugins

Set of plugins for Teleport
Apache License 2.0
83 stars 78 forks source link

terraform: calls to `schemav1.CopyLoginRuleToTerraform` copy a lock by value #845

Closed espadolini closed 1 year ago

espadolini commented 1 year ago

I'm not sure if this problem is more widespread than go vet can detect, or if the gogoproto-generated types are just more resilient to this, but this is the output of go vet ./terraform/... from go 1.20.4 on the current tip of master (e4b2e509c5554b93e506cb3639a27071d11a3e49):

# github.com/gravitational/teleport-plugins/terraform/tfschema/loginrule/v1
terraform/tfschema/loginrule/v1/loginrule_terraform.go:346:56: CopyLoginRuleToTerraform passes lock by value: github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1.LoginRule contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
# github.com/gravitational/teleport-plugins/terraform/provider
terraform/provider/data_source_teleport_login_rule.go:69:49: call of schemav1.CopyLoginRuleToTerraform copies lock value: github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1.LoginRule contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
terraform/provider/resource_teleport_login_rule.go:134:49: call of schemav1.CopyLoginRuleToTerraform copies lock value: github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1.LoginRule contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
terraform/provider/resource_teleport_login_rule.go:177:49: call of schemav1.CopyLoginRuleToTerraform copies lock value: github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1.LoginRule contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
terraform/provider/resource_teleport_login_rule.go:254:49: call of schemav1.CopyLoginRuleToTerraform copies lock value: github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1.LoginRule contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
terraform/provider/resource_teleport_login_rule.go:303:49: call of schemav1.CopyLoginRuleToTerraform copies lock value: github.com/gravitational/teleport/api/gen/proto/go/teleport/loginrule/v1.LoginRule contains google.golang.org/protobuf/internal/impl.MessageState contains sync.Mutex
espadolini commented 1 year ago

@nklaassen I've assigned you just because this problem only seems to appear for LoginRule, but the code does a shallow copy by dereference for all protobuf messages, which I believe is not really correct - and passing protobuf messages by value is a pretty big smell to begin with.