From aa973fe1c3b4386871d2eb2b4f6a60a9704596dd Mon Sep 17 00:00:00 2001 From: Easton Crupper <65553218+ecrupper@users.noreply.github.com> Date: Tue, 3 Dec 2024 08:47:44 -0600 Subject: [PATCH] enhance(compiler): cache templates per compilation (#1224) --- compiler/native/compile.go | 14 +- compiler/native/expand.go | 15 ++- compiler/native/expand_test.go | 231 +++++++++++++++++++++++++++++++++ compiler/native/native.go | 4 + compiler/native/native_test.go | 7 +- 5 files changed, 263 insertions(+), 8 deletions(-) diff --git a/compiler/native/compile.go b/compiler/native/compile.go index 13ad8eba7..9e7c638df 100644 --- a/compiler/native/compile.go +++ b/compiler/native/compile.go @@ -220,9 +220,17 @@ func (c *client) compileInline(ctx context.Context, p *yaml.Build, depth int) (* } for _, template := range p.Templates { - bytes, err := c.getTemplate(ctx, template, template.Name) - if err != nil { - return nil, err + var ( + bytes []byte + found bool + err error + ) + + if bytes, found = c.TemplateCache[template.Source]; !found { + bytes, err = c.getTemplate(ctx, template, template.Name) + if err != nil { + return nil, err + } } format := template.Format diff --git a/compiler/native/expand.go b/compiler/native/expand.go index 0420338c0..aae868693 100644 --- a/compiler/native/expand.go +++ b/compiler/native/expand.go @@ -118,9 +118,16 @@ func (c *client) ExpandSteps(ctx context.Context, s *yaml.Build, tmpls map[strin return s, err } - bytes, err := c.getTemplate(ctx, tmpl, step.Template.Name) - if err != nil { - return s, err + var ( + bytes []byte + found bool + ) + + if bytes, found = c.TemplateCache[tmpl.Source]; !found { + bytes, err = c.getTemplate(ctx, tmpl, step.Template.Name) + if err != nil { + return s, err + } } // initialize variable map if not parsed from config @@ -341,6 +348,8 @@ func (c *client) getTemplate(ctx context.Context, tmpl *yaml.Template, name stri return bytes, fmt.Errorf("unsupported template type: %v", tmpl.Type) } + c.TemplateCache[tmpl.Source] = bytes + return bytes, nil } diff --git a/compiler/native/expand_test.go b/compiler/native/expand_test.go index 8702f9c8b..8c56d77f8 100644 --- a/compiler/native/expand_test.go +++ b/compiler/native/expand_test.go @@ -954,6 +954,237 @@ func TestNative_ExpandSteps_TemplateCallTemplate(t *testing.T) { } } +func TestNative_ExpandStepsDuplicateCalls(t *testing.T) { + // setup context + gin.SetMode(gin.TestMode) + + resp := httptest.NewRecorder() + _, engine := gin.CreateTestContext(resp) + + testCallsMap := make(map[string]bool) + + // setup mock server + engine.GET("/api/v3/repos/:org/:repo/contents/:path", func(c *gin.Context) { + testCallKey := c.Param("path") + + if refQuery, exists := c.GetQuery("ref"); exists { + testCallKey += refQuery + } + + // this is the real test + if testCallsMap[testCallKey] { + t.Errorf("ExpandSteps() called the same template %s twice", c.Param("path")) + } + + testCallsMap[c.Param("path")] = true + body, err := convertFileToGithubResponse(c.Param("path")) + if err != nil { + t.Error(err) + } + c.JSON(http.StatusOK, body) + }) + + s := httptest.NewServer(engine) + defer s.Close() + + // setup types + set := flag.NewFlagSet("test", 0) + set.Bool("github-driver", true, "doc") + set.String("github-url", s.URL, "doc") + set.String("github-token", "", "doc") + set.Int("max-template-depth", 5, "doc") + set.String("clone-image", defaultCloneImage, "doc") + c := cli.NewContext(nil, set, nil) + + testRepo := new(api.Repo) + + testRepo.SetID(1) + testRepo.SetOrg("foo") + testRepo.SetName("bar") + + tests := []struct { + name string + tmpls map[string]*yaml.Template + }{ + { + name: "GitHub", + tmpls: map[string]*yaml.Template{ + "gradle": { + Name: "gradle", + Source: "github.example.com/foo/bar/long_template.yml", + Type: "github", + }, + }, + }, + } + + steps := yaml.StepSlice{ + &yaml.Step{ + Name: "sample", + Template: yaml.StepTemplate{ + Name: "gradle", + Variables: map[string]interface{}{ + "image": "openjdk:latest", + "environment": "{ GRADLE_USER_HOME: .gradle, GRADLE_OPTS: -Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false }", + "pull_policy": "pull: true", + }, + }, + }, + &yaml.Step{ + Name: "sample-dup", + Template: yaml.StepTemplate{ + Name: "gradle", + Variables: map[string]interface{}{ + "image": "openjdk:latest", + "environment": "{ GRADLE_USER_HOME: .gradle, GRADLE_OPTS: -Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false }", + "pull_policy": "pull: true", + }, + }, + }, + } + + globalEnvironment := raw.StringSliceMap{ + "foo": "test1", + "bar": "test2", + } + + wantSteps := yaml.StepSlice{ + &yaml.Step{ + Commands: []string{"./gradlew downloadDependencies"}, + Environment: raw.StringSliceMap{ + "GRADLE_OPTS": "-Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false", + "GRADLE_USER_HOME": ".gradle", + }, + Image: "openjdk:latest", + Name: "sample_install", + Pull: "always", + }, + &yaml.Step{ + Commands: []string{"./gradlew check"}, + Environment: raw.StringSliceMap{ + "GRADLE_OPTS": "-Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false", + "GRADLE_USER_HOME": ".gradle", + }, + Image: "openjdk:latest", + Name: "sample_test", + Pull: "always", + }, + &yaml.Step{ + Commands: []string{"./gradlew build", "echo gradle"}, + Environment: raw.StringSliceMap{ + "GRADLE_OPTS": "-Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false", + "GRADLE_USER_HOME": ".gradle", + }, + Image: "openjdk:latest", + Name: "sample_build", + Pull: "always", + }, + &yaml.Step{ + Commands: []string{"./gradlew downloadDependencies"}, + Environment: raw.StringSliceMap{ + "GRADLE_OPTS": "-Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false", + "GRADLE_USER_HOME": ".gradle", + }, + Image: "openjdk:latest", + Name: "sample-dup_install", + Pull: "always", + }, + &yaml.Step{ + Commands: []string{"./gradlew check"}, + Environment: raw.StringSliceMap{ + "GRADLE_OPTS": "-Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false", + "GRADLE_USER_HOME": ".gradle", + }, + Image: "openjdk:latest", + Name: "sample-dup_test", + Pull: "always", + }, + &yaml.Step{ + Commands: []string{"./gradlew build", "echo gradle"}, + Environment: raw.StringSliceMap{ + "GRADLE_OPTS": "-Dorg.gradle.daemon=false -Dorg.gradle.workers.max=1 -Dorg.gradle.parallel=false", + "GRADLE_USER_HOME": ".gradle", + }, + Image: "openjdk:latest", + Name: "sample-dup_build", + Pull: "always", + }, + } + + wantSecrets := yaml.SecretSlice{ + &yaml.Secret{ + Name: "docker_username", + Key: "org/repo/foo/bar", + Engine: "native", + Type: "repo", + Origin: yaml.Origin{}, + Pull: "build_start", + }, + &yaml.Secret{ + Name: "foo_password", + Key: "org/repo/foo/password", + Engine: "vault", + Type: "repo", + Origin: yaml.Origin{}, + Pull: "build_start", + }, + } + + wantServices := yaml.ServiceSlice{ + &yaml.Service{ + Image: "postgres:12", + Name: "postgres", + Pull: "not_present", + }, + } + + wantEnvironment := raw.StringSliceMap{ + "foo": "test1", + "bar": "test2", + "star": "test3", + } + + // run test + compiler, err := FromCLIContext(c) + if err != nil { + t.Errorf("Creating new compiler returned err: %v", err) + } + + compiler.WithCommit("123abc456def").WithRepo(testRepo) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + build, err := compiler.ExpandSteps( + context.Background(), + &yaml.Build{ + Steps: steps, + Services: yaml.ServiceSlice{}, + Environment: globalEnvironment, + }, + test.tmpls, new(pipeline.RuleData), compiler.GetTemplateDepth()) + if err != nil { + t.Errorf("ExpandSteps_Type%s returned err: %v", test.name, err) + } + + if diff := cmp.Diff(build.Steps, wantSteps); diff != "" { + t.Errorf("ExpandSteps()_Type%s mismatch (-want +got):\n%s", test.name, diff) + } + + if diff := cmp.Diff(build.Secrets, wantSecrets); diff != "" { + t.Errorf("ExpandSteps()_Type%s mismatch (-want +got):\n%s", test.name, diff) + } + + if diff := cmp.Diff(build.Services, wantServices); diff != "" { + t.Errorf("ExpandSteps()_Type%s mismatch (-want +got):\n%s", test.name, diff) + } + + if diff := cmp.Diff(build.Environment, wantEnvironment); diff != "" { + t.Errorf("ExpandSteps()_Type%s mismatch (-want +got):\n%s", test.name, diff) + } + }) + } +} + func TestNative_ExpandSteps_TemplateCallTemplate_CircularFail(t *testing.T) { // setup context gin.SetMode(gin.TestMode) diff --git a/compiler/native/native.go b/compiler/native/native.go index ec6d78366..1b36e39fb 100644 --- a/compiler/native/native.go +++ b/compiler/native/native.go @@ -31,6 +31,7 @@ type client struct { PrivateGithub registry.Service UsePrivateGithub bool ModificationService ModificationConfig + TemplateCache map[string][]byte settings.Compiler @@ -102,6 +103,8 @@ func FromCLIContext(ctx *cli.Context) (*client, error) { c.UsePrivateGithub = true } + c.TemplateCache = make(map[string][]byte) + return c, nil } @@ -131,6 +134,7 @@ func (c *client) Duplicate() compiler.Engine { cc.CloneImage = c.CloneImage cc.TemplateDepth = c.TemplateDepth cc.StarlarkExecLimit = c.StarlarkExecLimit + cc.TemplateCache = c.TemplateCache return cc } diff --git a/compiler/native/native_test.go b/compiler/native/native_test.go index 86b143dbe..ef3ddb202 100644 --- a/compiler/native/native_test.go +++ b/compiler/native/native_test.go @@ -23,8 +23,9 @@ func TestNative_New(t *testing.T) { c := cli.NewContext(nil, set, nil) public, _ := github.New(context.Background(), "", "") want := &client{ - Github: public, - Compiler: settings.CompilerMockEmpty(), + Github: public, + Compiler: settings.CompilerMockEmpty(), + TemplateCache: make(map[string][]byte), } want.SetCloneImage(defaultCloneImage) @@ -56,6 +57,7 @@ func TestNative_New_PrivateGithub(t *testing.T) { Github: public, PrivateGithub: private, UsePrivateGithub: true, + TemplateCache: make(map[string][]byte), Compiler: settings.CompilerMockEmpty(), } want.SetCloneImage(defaultCloneImage) @@ -88,6 +90,7 @@ func TestNative_DuplicateRetainSettings(t *testing.T) { Github: public, PrivateGithub: private, UsePrivateGithub: true, + TemplateCache: make(map[string][]byte), Compiler: settings.CompilerMockEmpty(), } want.SetCloneImage(defaultCloneImage)