From 2033b45ae44cd99db45058015d4da3a7b451ff40 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Wed, 3 Jul 2024 22:22:34 +0900 Subject: [PATCH 1/8] Impl lambda diff Signed-off-by: t-kikuc --- pkg/app/piped/platformprovider/lambda/diff.go | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 pkg/app/piped/platformprovider/lambda/diff.go diff --git a/pkg/app/piped/platformprovider/lambda/diff.go b/pkg/app/piped/platformprovider/lambda/diff.go new file mode 100644 index 0000000000..7b7eb49723 --- /dev/null +++ b/pkg/app/piped/platformprovider/lambda/diff.go @@ -0,0 +1,89 @@ +// Copyright 2024 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package lambda + +import ( + "fmt" + "strings" + + "github.com/pipe-cd/pipecd/pkg/diff" +) + +const ( + diffCommand = "diff" +) + +type DiffResult struct { + Diff *diff.Result + Old FunctionManifest + New FunctionManifest +} + +func (d *DiffResult) NoChange() bool { + return len(d.Diff.Nodes()) == 0 +} + +func Diff(old, new FunctionManifest, opts ...diff.Option) (*DiffResult, error) { + d, err := diff.DiffStructureds(old, new, opts...) + if err != nil { + return nil, err + } + + if !d.HasDiff() { + return &DiffResult{Diff: d}, nil + } + + ret := &DiffResult{ + Old: old, + New: new, + Diff: d, + } + return ret, nil +} + +type DiffRenderOptions struct { + // If true, use "diff" command to render. + UseDiffCommand bool +} + +func (d *DiffResult) Render(opt DiffRenderOptions) string { + var b strings.Builder + opts := []diff.RenderOption{ + diff.WithLeftPadding(1), + } + renderer := diff.NewRenderer(opts...) + if !opt.UseDiffCommand { + b.WriteString(renderer.Render(d.Diff.Nodes())) + } else { + d, err := renderByCommand(diffCommand, d.Old, d.New) + if err != nil { + b.WriteString(fmt.Sprintf("An error occurred while rendering diff (%v)", err)) + } else { + b.Write(d) + } + } + b.WriteString("\n") + + return b.String() +} + +func renderByCommand(command string, old, new FunctionManifest) ([]byte, error) { + diff, err := diff.RenderByCommand(command, old, new) + if err != nil { + return nil, err + } + + return diff, nil +} From 311cbc8f30493ce0445babcced25054c81b8b6b4 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Thu, 4 Jul 2024 09:46:42 +0900 Subject: [PATCH 2/8] Add diff_test Signed-off-by: t-kikuc --- .../platformprovider/lambda/diff_test.go | 91 +++++++++++++++++++ .../lambda/testdata/new_function.yaml | 12 +++ .../lambda/testdata/old_function.yaml | 12 +++ 3 files changed, 115 insertions(+) create mode 100644 pkg/app/piped/platformprovider/lambda/diff_test.go create mode 100644 pkg/app/piped/platformprovider/lambda/testdata/new_function.yaml create mode 100644 pkg/app/piped/platformprovider/lambda/testdata/old_function.yaml diff --git a/pkg/app/piped/platformprovider/lambda/diff_test.go b/pkg/app/piped/platformprovider/lambda/diff_test.go new file mode 100644 index 0000000000..477faecb4f --- /dev/null +++ b/pkg/app/piped/platformprovider/lambda/diff_test.go @@ -0,0 +1,91 @@ +// Copyright 2024 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package lambda + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDiff(t *testing.T) { + t.Parallel() + + old, err := loadFunctionManifest("testdata/old_function.yaml") + require.NoError(t, err) + + new, err := loadFunctionManifest("testdata/new_function.yaml") + require.NoError(t, err) + + // Expect to have change. + diff, err := Diff(old, new) + require.NoError(t, err) + noChange := diff.NoChange() + require.False(t, noChange) + + // Expect no change. + diff, err = Diff(old, old) + require.NoError(t, err) + noChange = diff.NoChange() + require.True(t, noChange) +} + +func TestDiffResult_Render(t *testing.T) { + old, err := loadFunctionManifest("testdata/old_function.yaml") + require.NoError(t, err) + + new, err := loadFunctionManifest("testdata/new_function.yaml") + require.NoError(t, err) + + result, err := Diff(old, new) + require.NoError(t, err) + + // Not using diff command + opt := DiffRenderOptions{} + actual := result.Render(opt) + expected := ` spec: + environments: + #spec.environments.FOO +- FOO: bar ++ FOO: bar2 + + #spec.image +- image: ecr.ap-northeast-1.amazonaws.com/lambda-test:v0.0.1 ++ image: ecr.ap-northeast-1.amazonaws.com/lambda-test:v0.0.2 + + +` + + require.Equal(t, expected, actual) + + // Use diff command + opt = DiffRenderOptions{UseDiffCommand: true} + actual = result.Render(opt) + expected = `@@ -2,9 +2,9 @@ + kind: LambdaFunction + spec: + environments: +- FOO: bar ++ FOO: bar2 + handler: "" +- image: ecr.ap-northeast-1.amazonaws.com/lambda-test:v0.0.1 ++ image: ecr.ap-northeast-1.amazonaws.com/lambda-test:v0.0.2 + memory: 512 + name: TestFunction + role: arn:aws:iam:region:account-id:role/lambda-role +` + + require.Equal(t, expected, actual) +} diff --git a/pkg/app/piped/platformprovider/lambda/testdata/new_function.yaml b/pkg/app/piped/platformprovider/lambda/testdata/new_function.yaml new file mode 100644 index 0000000000..32910a6b9f --- /dev/null +++ b/pkg/app/piped/platformprovider/lambda/testdata/new_function.yaml @@ -0,0 +1,12 @@ +apiVersion: pipecd.dev/v1beta1 +kind: LambdaFunction +spec: + name: TestFunction + role: arn:aws:iam:region:account-id:role/lambda-role + image: ecr.ap-northeast-1.amazonaws.com/lambda-test:v0.0.2 + memory: 512 + timeout: 30 + environments: + FOO: bar2 + tags: + app: test diff --git a/pkg/app/piped/platformprovider/lambda/testdata/old_function.yaml b/pkg/app/piped/platformprovider/lambda/testdata/old_function.yaml new file mode 100644 index 0000000000..1afe310599 --- /dev/null +++ b/pkg/app/piped/platformprovider/lambda/testdata/old_function.yaml @@ -0,0 +1,12 @@ +apiVersion: pipecd.dev/v1beta1 +kind: LambdaFunction +spec: + name: TestFunction + role: arn:aws:iam:region:account-id:role/lambda-role + image: ecr.ap-northeast-1.amazonaws.com/lambda-test:v0.0.1 + memory: 512 + timeout: 30 + environments: + FOO: bar + tags: + app: test From 46924b373fd602ad02dc50240c1c3af6080641e2 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Thu, 4 Jul 2024 11:43:07 +0900 Subject: [PATCH 3/8] add lambda cache Signed-off-by: t-kikuc --- .../piped/platformprovider/lambda/cache.go | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 pkg/app/piped/platformprovider/lambda/cache.go diff --git a/pkg/app/piped/platformprovider/lambda/cache.go b/pkg/app/piped/platformprovider/lambda/cache.go new file mode 100644 index 0000000000..f98cc544f4 --- /dev/null +++ b/pkg/app/piped/platformprovider/lambda/cache.go @@ -0,0 +1,68 @@ +// Copyright 2024 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package lambda + +import ( + "errors" + "fmt" + + "go.uber.org/zap" + + "github.com/pipe-cd/pipecd/pkg/cache" +) + +type FunctionManifestCache struct { + AppID string + Cache cache.Cache + Logger *zap.Logger +} + +func (c FunctionManifestCache) Get(commit string) (FunctionManifest, bool) { + key := manifestCacheKey(c.AppID, commit) + item, err := c.Cache.Get(key) + if err == nil { + return item.(FunctionManifest), true + } + + if errors.Is(err, cache.ErrNotFound) { + c.Logger.Info("function manifest wes not found in cache", + zap.String("app-id", c.AppID), + zap.String("commit-hash", commit), + ) + return FunctionManifest{}, false + } + + c.Logger.Error("failed while retrieving function manifest from cache", + zap.String("app-id", c.AppID), + zap.String("commit-hash", commit), + zap.Error(err), + ) + return FunctionManifest{}, false +} + +func (c FunctionManifestCache) Put(commit string, sm FunctionManifest) { + key := manifestCacheKey(c.AppID, commit) + if err := c.Cache.Put(key, sm); err != nil { + c.Logger.Error("failed while putting function manifest into cache", + zap.String("app-id", c.AppID), + zap.String("commit-hash", commit), + zap.Error(err), + ) + } +} + +func manifestCacheKey(appID, commit string) string { + return fmt.Sprintf("%s/%s", appID, commit) +} From a295d76074da1f16b2fcb3b4de9239de2605259e Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Thu, 4 Jul 2024 11:47:49 +0900 Subject: [PATCH 4/8] Add lambdadiff Signed-off-by: t-kikuc --- pkg/app/piped/planpreview/lambdadiff.go | 124 ++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 pkg/app/piped/planpreview/lambdadiff.go diff --git a/pkg/app/piped/planpreview/lambdadiff.go b/pkg/app/piped/planpreview/lambdadiff.go new file mode 100644 index 0000000000..82700f847f --- /dev/null +++ b/pkg/app/piped/planpreview/lambdadiff.go @@ -0,0 +1,124 @@ +// Copyright 2024 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package planpreview + +import ( + "bytes" + "context" + "fmt" + "io" + + "github.com/pipe-cd/pipecd/pkg/app/piped/deploysource" + provider "github.com/pipe-cd/pipecd/pkg/app/piped/platformprovider/lambda" + "github.com/pipe-cd/pipecd/pkg/diff" + "github.com/pipe-cd/pipecd/pkg/model" +) + +func (b *builder) lambdadiff( + ctx context.Context, + app *model.Application, + targetDSP deploysource.Provider, + lastCommit string, + buf *bytes.Buffer, +) (*diffResult, error) { + var ( + oldManifests, newManifests provider.FunctionManifest + err error + ) + + newManifests, err = b.loadFunctionManifest(ctx, *app, targetDSP) + if err != nil { + fmt.Fprintf(buf, "failed to load lambda manifests at the head commit (%v)\n", err) + return nil, err + } + + if lastCommit == "" { + fmt.Fprintf(buf, "failed to find the commit of the last successful deployment") + return nil, fmt.Errorf("cannot get the old manifests without the last successful deployment") + } + + runningDSP := deploysource.NewProvider( + b.workingDir, + deploysource.NewGitSourceCloner(b.gitClient, b.repoCfg, "running", lastCommit), + *app.GitPath, + b.secretDecrypter, + ) + + oldManifests, err = b.loadFunctionManifest(ctx, *app, runningDSP) + if err != nil { + fmt.Fprintf(buf, "failed to load lambda manifests at the running commit (%v)\n", err) + return nil, err + } + + result, err := provider.Diff( + oldManifests, + newManifests, + diff.WithEquateEmpty(), + diff.WithCompareNumberAndNumericString(), + ) + if err != nil { + fmt.Fprintf(buf, "failed to compare manifests (%v)\n", err) + return nil, err + } + + if result.NoChange() { + fmt.Fprintln(buf, "No changes were detected") + return &diffResult{ + summary: "No changes were detected", + noChange: true, + }, nil + } + + details := result.Render(provider.DiffRenderOptions{ + UseDiffCommand: true, + }) + fmt.Fprintf(buf, "--- Last Deploy\n+++ Head Commit\n\n%s\n", details) + + return &diffResult{ + summary: fmt.Sprintf("%d changes were detected", len(result.Diff.Nodes())), + }, nil +} + +func (b *builder) loadFunctionManifest(ctx context.Context, app model.Application, dsp deploysource.Provider) (provider.FunctionManifest, error) { + commit := dsp.Revision() + cache := provider.FunctionManifestCache{ + AppID: app.Id, + Cache: b.appManifestsCache, + Logger: b.logger, + } + + manifest, ok := cache.Get(commit) + if ok { + return manifest, nil + } + + ds, err := dsp.Get(ctx, io.Discard) + if err != nil { + return provider.FunctionManifest{}, err + } + + appCfg := ds.ApplicationConfig.LambdaApplicationSpec + if appCfg == nil { + return provider.FunctionManifest{}, fmt.Errorf("malformed application configuration file") + } + + manifest, err = provider.LoadFunctionManifest(ds.AppDir, appCfg.Input.FunctionManifestFile) + if err != nil { + return provider.FunctionManifest{}, err + } + + cache.Put(commit, manifest) + return manifest, nil +} From e640ef90f688d1028b05b57bb68ed7bb6b2bd0e8 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Thu, 4 Jul 2024 11:48:17 +0900 Subject: [PATCH 5/8] add lambda as a supported case Signed-off-by: t-kikuc --- pkg/app/piped/planpreview/builder.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/app/piped/planpreview/builder.go b/pkg/app/piped/planpreview/builder.go index 315e8f30a6..a36776a060 100644 --- a/pkg/app/piped/planpreview/builder.go +++ b/pkg/app/piped/planpreview/builder.go @@ -251,8 +251,9 @@ func (b *builder) buildApp(ctx context.Context, worker int, command string, app dr, err = b.cloudrundiff(ctx, app, targetDSP, preCommit, &buf) case model.ApplicationKind_ECS: dr, err = b.ecsdiff(ctx, app, targetDSP, preCommit, &buf) + case model.ApplicationKind_LAMBDA: + dr, err = b.lambdadiff(ctx, app, targetDSP, preCommit, &buf) default: - // TODO: Calculating planpreview's diff for other application kinds. dr = &diffResult{ summary: fmt.Sprintf("%s application is not implemented yet (coming soon)", app.Kind.String()), } From d264ef077613ef593c29a891cb0a61f74181ec5e Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Thu, 4 Jul 2024 11:54:19 +0900 Subject: [PATCH 6/8] add planpreview test data for Lambda Signed-off-by: t-kikuc --- pkg/app/pipectl/cmd/planpreview/planpreview_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/app/pipectl/cmd/planpreview/planpreview_test.go b/pkg/app/pipectl/cmd/planpreview/planpreview_test.go index 51530323ab..9360101110 100644 --- a/pkg/app/pipectl/cmd/planpreview/planpreview_test.go +++ b/pkg/app/pipectl/cmd/planpreview/planpreview_test.go @@ -172,6 +172,13 @@ NOTE: An error occurred while building plan-preview for applications of the foll ApplicationKind: model.ApplicationKind_ECS, Error: "wrong application configuration", }, + { + ApplicationId: "app-6", + ApplicationName: "app-6", + ApplicationUrl: "https://pipecd.dev/app-6", + ApplicationKind: model.ApplicationKind_LAMBDA, + Error: "wrong application configuration", + }, }, }, { @@ -203,7 +210,7 @@ changes-1 changes-2 ---DETAILS_END--- -NOTE: An error occurred while building plan-preview for the following 3 applications: +NOTE: An error occurred while building plan-preview for the following 4 applications: 1. app: app-3, env: env-3, kind: TERRAFORM reason: wrong application configuration @@ -214,6 +221,9 @@ NOTE: An error occurred while building plan-preview for the following 3 applicat 3. app: app-5, kind: ECS reason: wrong application configuration +4. app: app-6, kind: LAMBDA + reason: wrong application configuration + NOTE: An error occurred while building plan-preview for applications of the following 2 Pipeds: 1. piped: piped-name-1 (piped-1) From 64481e9f9cef79738e63ab0530dda1abcfa3b152 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Tue, 16 Jul 2024 17:31:32 +0900 Subject: [PATCH 7/8] fix typo: manifests->manifest Signed-off-by: t-kikuc --- pkg/app/piped/planpreview/lambdadiff.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/app/piped/planpreview/lambdadiff.go b/pkg/app/piped/planpreview/lambdadiff.go index 82700f847f..3d80fce576 100644 --- a/pkg/app/piped/planpreview/lambdadiff.go +++ b/pkg/app/piped/planpreview/lambdadiff.go @@ -34,19 +34,19 @@ func (b *builder) lambdadiff( buf *bytes.Buffer, ) (*diffResult, error) { var ( - oldManifests, newManifests provider.FunctionManifest - err error + oldManifest, newManifest provider.FunctionManifest + err error ) - newManifests, err = b.loadFunctionManifest(ctx, *app, targetDSP) + newManifest, err = b.loadFunctionManifest(ctx, *app, targetDSP) if err != nil { - fmt.Fprintf(buf, "failed to load lambda manifests at the head commit (%v)\n", err) + fmt.Fprintf(buf, "failed to load lambda manifest at the head commit (%v)\n", err) return nil, err } if lastCommit == "" { fmt.Fprintf(buf, "failed to find the commit of the last successful deployment") - return nil, fmt.Errorf("cannot get the old manifests without the last successful deployment") + return nil, fmt.Errorf("cannot get the old manifest without the last successful deployment") } runningDSP := deploysource.NewProvider( @@ -56,20 +56,20 @@ func (b *builder) lambdadiff( b.secretDecrypter, ) - oldManifests, err = b.loadFunctionManifest(ctx, *app, runningDSP) + oldManifest, err = b.loadFunctionManifest(ctx, *app, runningDSP) if err != nil { - fmt.Fprintf(buf, "failed to load lambda manifests at the running commit (%v)\n", err) + fmt.Fprintf(buf, "failed to load lambda manifest at the running commit (%v)\n", err) return nil, err } result, err := provider.Diff( - oldManifests, - newManifests, + oldManifest, + newManifest, diff.WithEquateEmpty(), diff.WithCompareNumberAndNumericString(), ) if err != nil { - fmt.Fprintf(buf, "failed to compare manifests (%v)\n", err) + fmt.Fprintf(buf, "failed to compare manifest (%v)\n", err) return nil, err } From 8d021abf8ccabcee64f3c4b9e3e5dd939fcff525 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Fri, 19 Jul 2024 14:55:17 +0900 Subject: [PATCH 8/8] fix nits: directly call RenderByCommand() Signed-off-by: t-kikuc --- pkg/app/piped/platformprovider/lambda/diff.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/pkg/app/piped/platformprovider/lambda/diff.go b/pkg/app/piped/platformprovider/lambda/diff.go index 7b7eb49723..81861da941 100644 --- a/pkg/app/piped/platformprovider/lambda/diff.go +++ b/pkg/app/piped/platformprovider/lambda/diff.go @@ -67,7 +67,7 @@ func (d *DiffResult) Render(opt DiffRenderOptions) string { if !opt.UseDiffCommand { b.WriteString(renderer.Render(d.Diff.Nodes())) } else { - d, err := renderByCommand(diffCommand, d.Old, d.New) + d, err := diff.RenderByCommand(diffCommand, d.Old, d.New) if err != nil { b.WriteString(fmt.Sprintf("An error occurred while rendering diff (%v)", err)) } else { @@ -78,12 +78,3 @@ func (d *DiffResult) Render(opt DiffRenderOptions) string { return b.String() } - -func renderByCommand(command string, old, new FunctionManifest) ([]byte, error) { - diff, err := diff.RenderByCommand(command, old, new) - if err != nil { - return nil, err - } - - return diff, nil -}