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

Optimize GetModuleContent performance #148

Merged
merged 1 commit into from
Mar 26, 2022
Merged

Optimize GetModuleContent performance #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 23 additions & 0 deletions plugin/fromproto/fromproto.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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