Skip to content

Commit

Permalink
Optimize GetModuleContent performance (#148)
Browse files Browse the repository at this point in the history
  • Loading branch information
@wata727
wata727 authored Mar 26, 2022
1 parent ff9c1b4 commit 1d4544f
Show file tree
Hide file tree
Showing 9 changed files with 574 additions and 414 deletions.
23 changes: 23 additions & 0 deletions plugin/fromproto/fromproto.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@ func Config(config *proto.ApplyGlobalConfig_Config) *tflint.Config {
return &tflint.Config{Rules: rules, DisabledByDefault: config.DisabledByDefault}
}

// GetModuleContentOption converts proto.GetModuleContent_Option to tflint.GetModuleContentOption
func GetModuleContentOption(opts *proto.GetModuleContent_Option) tflint.GetModuleContentOption {
if opts == nil {
return tflint.GetModuleContentOption{}
}

return tflint.GetModuleContentOption{
ModuleCtx: ModuleCtxType(opts.ModuleCtx),
Hint: GetModuleContentHint(opts.Hint),
}
}

// ModuleCtxType converts proto.ModuleCtxType to tflint.ModuleCtxType
func ModuleCtxType(ty proto.ModuleCtxType) tflint.ModuleCtxType {
switch ty {
Expand All @@ -197,6 +209,17 @@ func ModuleCtxType(ty proto.ModuleCtxType) tflint.ModuleCtxType {
}
}

// GetModuleContentHint converts proto.GetModuleContent_Hint to tflint.GetModuleContentHint
func GetModuleContentHint(hint *proto.GetModuleContent_Hint) tflint.GetModuleContentHint {
if hint == nil {
return tflint.GetModuleContentHint{}
}

return tflint.GetModuleContentHint{
ResourceType: hint.ResourceType,
}
}

// Error converts gRPC error status to wrapped error
func Error(err error) error {
if err == nil {
Expand Down
4 changes: 2 additions & 2 deletions plugin/host2plugin/host2plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,8 @@ func (s *mockServer) EmitIssue(rule tflint.Rule, message string, location hcl.Ra
return nil
}

func (s *mockServer) GetFiles(tflint.ModuleCtxType) map[string]*hcl.File {
return map[string]*hcl.File{}
func (s *mockServer) GetFiles(tflint.ModuleCtxType) map[string][]byte {
return map[string][]byte{}
}

func TestCheck(t *testing.T) {
Expand Down
7 changes: 6 additions & 1 deletion plugin/plugin2host/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ var _ tflint.Runner = &GRPCClient{}
// GetResourceContent gets the contents of resources based on the schema.
// This is shorthand of GetModuleContent for resources
func (c *GRPCClient) GetResourceContent(name string, inner *hclext.BodySchema, opts *tflint.GetModuleContentOption) (*hclext.BodyContent, error) {
if opts == nil {
opts = &tflint.GetModuleContentOption{}
}
opts.Hint.ResourceType = name

body, err := c.GetModuleContent(&hclext.BodySchema{
Blocks: []hclext.BlockSchema{
{Type: "resource", LabelNames: []string{"type", "name"}, Body: inner},
Expand Down Expand Up @@ -59,7 +64,7 @@ func (c *GRPCClient) GetModuleContent(schema *hclext.BodySchema, opts *tflint.Ge

req := &proto.GetModuleContent_Request{
Schema: toproto.BodySchema(schema),
Option: &proto.GetModuleContent_Option{ModuleCtx: toproto.ModuleCtxType(opts.ModuleCtx)},
Option: toproto.GetModuleContentOption(opts),
}
resp, err := c.Client.GetModuleContent(context.Background(), req)
if err != nil {
Expand Down
80 changes: 58 additions & 22 deletions plugin/plugin2host/plugin2host_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type mockServer struct {
type mockServerImpl struct {
getModuleContent func(*hclext.BodySchema, tflint.GetModuleContentOption) (*hclext.BodyContent, hcl.Diagnostics)
getFile func(string) (*hcl.File, error)
getFiles func() map[string]*hcl.File
getFiles func() map[string][]byte
getRuleConfigContent func(string, *hclext.BodySchema) (*hclext.BodyContent, *hcl.File, error)
evaluateExpr func(hcl.Expression, tflint.EvaluateExprOption) (cty.Value, error)
emitIssue func(tflint.Rule, string, hcl.Range) error
Expand All @@ -60,11 +60,11 @@ func (s *mockServer) GetFile(filename string) (*hcl.File, error) {
return nil, nil
}

func (s *mockServer) GetFiles(tflint.ModuleCtxType) map[string]*hcl.File {
func (s *mockServer) GetFiles(tflint.ModuleCtxType) map[string][]byte {
if s.impl.getFiles != nil {
return s.impl.getFiles()
}
return map[string]*hcl.File{}
return map[string][]byte{}
}

func (s *mockServer) GetRuleConfigContent(name string, schema *hclext.BodySchema) (*hclext.BodyContent, *hcl.File, error) {
Expand Down Expand Up @@ -96,8 +96,8 @@ func TestGetResourceContent(t *testing.T) {
neverHappend := func(err error) bool { return err != nil }

// default getFileImpl function
files := map[string]*hcl.File{}
fileExists := func() map[string]*hcl.File {
files := map[string][]byte{}
fileExists := func() map[string][]byte {
return files
}

Expand All @@ -107,15 +107,15 @@ func TestGetResourceContent(t *testing.T) {
if diags.HasErrors() {
panic(diags)
}
files[filename] = file
files[filename] = file.Bytes
return file
}
jsonFile := func(filename string, code string) *hcl.File {
file, diags := json.Parse([]byte(code), filename)
if diags.HasErrors() {
panic(diags)
}
files[filename] = file
files[filename] = file.Bytes
return file
}

Expand Down Expand Up @@ -215,6 +215,34 @@ resource "aws_instance" "foo" {
},
ErrCheck: neverHappend,
},
{
Name: "get content with options",
Args: func() (string, *hclext.BodySchema, *tflint.GetModuleContentOption) {
return "aws_instance", &hclext.BodySchema{}, &tflint.GetModuleContentOption{
ModuleCtx: tflint.RootModuleCtxType,
}
},
ServerImpl: func(schema *hclext.BodySchema, opts tflint.GetModuleContentOption) (*hclext.BodyContent, hcl.Diagnostics) {
if opts.ModuleCtx != tflint.RootModuleCtxType {
return &hclext.BodyContent{}, hcl.Diagnostics{
&hcl.Diagnostic{Severity: hcl.DiagError, Summary: "unexpected moduleCtx options"},
}
}
if opts.Hint.ResourceType != "aws_instance" {
return &hclext.BodyContent{}, hcl.Diagnostics{
&hcl.Diagnostic{Severity: hcl.DiagError, Summary: "unexpected hint options"},
}
}
return &hclext.BodyContent{}, hcl.Diagnostics{}
},
Want: func(resource string, schema *hclext.BodySchema, opts *tflint.GetModuleContentOption) (*hclext.BodyContent, hcl.Diagnostics) {
return &hclext.BodyContent{
Attributes: hclext.Attributes{},
Blocks: hclext.Blocks{},
}, hcl.Diagnostics{}
},
ErrCheck: neverHappend,
},
}

for _, test := range tests {
Expand Down Expand Up @@ -252,8 +280,8 @@ func TestGetModuleContent(t *testing.T) {
neverHappend := func(err error) bool { return err != nil }

// default getFileImpl function
files := map[string]*hcl.File{}
fileExists := func() map[string]*hcl.File {
files := map[string][]byte{}
fileExists := func() map[string][]byte {
return files
}

Expand All @@ -263,15 +291,15 @@ func TestGetModuleContent(t *testing.T) {
if diags.HasErrors() {
panic(diags)
}
files[filename] = file
files[filename] = file.Bytes
return file
}
jsonFile := func(filename string, code string) *hcl.File {
file, diags := json.Parse([]byte(code), filename)
if diags.HasErrors() {
panic(diags)
}
files[filename] = file
files[filename] = file.Bytes
return file
}

Expand Down Expand Up @@ -359,12 +387,20 @@ resource "aws_instance" "foo" {
{
Name: "get content with options",
Args: func() (*hclext.BodySchema, *tflint.GetModuleContentOption) {
return &hclext.BodySchema{}, &tflint.GetModuleContentOption{ModuleCtx: tflint.RootModuleCtxType}
return &hclext.BodySchema{}, &tflint.GetModuleContentOption{
ModuleCtx: tflint.RootModuleCtxType,
Hint: tflint.GetModuleContentHint{ResourceType: "aws_instance"},
}
},
ServerImpl: func(schema *hclext.BodySchema, opts tflint.GetModuleContentOption) (*hclext.BodyContent, hcl.Diagnostics) {
if opts.ModuleCtx != tflint.RootModuleCtxType {
return &hclext.BodyContent{}, hcl.Diagnostics{
&hcl.Diagnostic{Severity: hcl.DiagError, Summary: "unexpected options"},
&hcl.Diagnostic{Severity: hcl.DiagError, Summary: "unexpected moduleCtx options"},
}
}
if opts.Hint.ResourceType != "aws_instance" {
return &hclext.BodyContent{}, hcl.Diagnostics{
&hcl.Diagnostic{Severity: hcl.DiagError, Summary: "unexpected hint options"},
}
}
return &hclext.BodyContent{}, hcl.Diagnostics{}
Expand Down Expand Up @@ -608,19 +644,19 @@ func TestGetFiles(t *testing.T) {

tests := []struct {
Name string
ServerImpl func() map[string]*hcl.File
ServerImpl func() map[string][]byte
Want map[string]*hcl.File
ErrCheck func(error) bool
}{
{
Name: "HCL files",
ServerImpl: func() map[string]*hcl.File {
return map[string]*hcl.File{
"test1.tf": hclFile("test1.tf", `
ServerImpl: func() map[string][]byte {
return map[string][]byte{
"test1.tf": []byte(`
resource "aws_instance" "foo" {
instance_type = "t2.micro"
}`),
"test2.tf": hclFile("test2.tf", `
"test2.tf": []byte(`
resource "aws_s3_bucket" "bar" {
bucket = "baz"
}`),
Expand All @@ -640,9 +676,9 @@ resource "aws_s3_bucket" "bar" {
},
{
Name: "JSON files",
ServerImpl: func() map[string]*hcl.File {
return map[string]*hcl.File{
"test1.tf.json": jsonFile("test1.tf.json", `
ServerImpl: func() map[string][]byte {
return map[string][]byte{
"test1.tf.json": []byte(`
{
"resource": {
"aws_instance": {
Expand All @@ -652,7 +688,7 @@ resource "aws_s3_bucket" "bar" {
}
}
}`),
"test2.tf.json": jsonFile("test2.tf.json", `
"test2.tf.json": []byte(`
{
"resource": {
"aws_s3_bucket": {
Expand Down
21 changes: 6 additions & 15 deletions plugin/plugin2host/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ var _ proto.RunnerServer = &GRPCServer{}
type Server interface {
GetModuleContent(*hclext.BodySchema, tflint.GetModuleContentOption) (*hclext.BodyContent, hcl.Diagnostics)
GetFile(string) (*hcl.File, error)
GetFiles(tflint.ModuleCtxType) map[string]*hcl.File
// For performance, GetFiles returns map[string][]bytes instead of map[string]*hcl.File.
GetFiles(tflint.ModuleCtxType) map[string][]byte
GetRuleConfigContent(string, *hclext.BodySchema) (*hclext.BodyContent, *hcl.File, error)
EvaluateExpr(hcl.Expression, tflint.EvaluateExprOption) (cty.Value, error)
EmitIssue(rule tflint.Rule, message string, location hcl.Range) error
Expand All @@ -45,20 +46,16 @@ func (s *GRPCServer) GetModuleContent(ctx context.Context, req *proto.GetModuleC
return nil, status.Error(codes.InvalidArgument, "option should not be null")
}

moduleCtx := fromproto.ModuleCtxType(req.Option.ModuleCtx)
body, diags := s.Impl.GetModuleContent(fromproto.BodySchema(req.Schema), tflint.GetModuleContentOption{ModuleCtx: moduleCtx})
opts := fromproto.GetModuleContentOption(req.Option)
body, diags := s.Impl.GetModuleContent(fromproto.BodySchema(req.Schema), opts)
if diags.HasErrors() {
return nil, toproto.Error(codes.FailedPrecondition, diags)
}
if body == nil {
return nil, status.Error(codes.FailedPrecondition, "response body is empty")
}

sources := map[string][]byte{}
for name, file := range s.Impl.GetFiles(moduleCtx) {
sources[name] = file.Bytes
}
content := toproto.BodyContent(body, sources)
content := toproto.BodyContent(body, s.Impl.GetFiles(opts.ModuleCtx))

return &proto.GetModuleContent_Response{Content: content}, nil
}
Expand All @@ -80,13 +77,7 @@ func (s *GRPCServer) GetFile(ctx context.Context, req *proto.GetFile_Request) (*

// GetFiles returns bytes of hcl.File in the self module context.
func (s *GRPCServer) GetFiles(ctx context.Context, req *proto.GetFiles_Request) (*proto.GetFiles_Response, error) {
files := s.Impl.GetFiles(tflint.SelfModuleCtxType)

resp := map[string][]byte{}
for name, file := range files {
resp[name] = file.Bytes
}
return &proto.GetFiles_Response{Files: resp}, nil
return &proto.GetFiles_Response{Files: s.Impl.GetFiles(tflint.SelfModuleCtxType)}, nil
}

// GetRuleConfigContent returns BodyContent based on the rule name and config schema.
Expand Down
Loading

0 comments on commit 1d4544f

Please sign in to comment.