From 7ee77c937f62be1a5564cf8db727af57211e1185 Mon Sep 17 00:00:00 2001 From: Wanxian Yang <79273084+Lou1415926@users.noreply.github.com> Date: Tue, 6 Dec 2022 14:57:40 -0800 Subject: [PATCH 1/2] ci: compute size diff between PR and last release (#4240) instead of mainline vs. last release By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- .github/workflows/ci_size_computer.yml | 97 ++++++++++++++++ .github/workflows/ci_size_writer.yml | 110 ++++++++++++++++++ .github/workflows/ci_writer.yml | 152 ------------------------- 3 files changed, 207 insertions(+), 152 deletions(-) create mode 100644 .github/workflows/ci_size_computer.yml create mode 100644 .github/workflows/ci_size_writer.yml delete mode 100644 .github/workflows/ci_writer.yml diff --git a/.github/workflows/ci_size_computer.yml b/.github/workflows/ci_size_computer.yml new file mode 100644 index 00000000000..b432ec0346b --- /dev/null +++ b/.github/workflows/ci_size_computer.yml @@ -0,0 +1,97 @@ +name: Compute bin sizes + +on: + pull_request: + types: + - opened + - edited + - synchronize + +jobs: + binsize: + runs-on: ubuntu-latest + steps: + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: 1.18 + + - name: Set up Node + uses: actions/setup-node@v1 + with: + node-version: 16.x + + - name: Get latest release with tag + id: latestrelease + run: | + echo "VERSION_TAG=$(curl -s https://api.github.com/repos/aws/copilot-cli/releases/latest | jq '.tag_name' | sed 's/\"//g')" >> $GITHUB_OUTPUT + + - name: Check out latest release tag + uses: actions/checkout@v3 + with: + ref: ${{ steps.latestrelease.outputs.VERSION_TAG }} + repository: aws/copilot-cli + + - name: Compute old release binary sizes + id: old-bins + run: | + make release + echo "MAC_AMD_KIB=$(du -k ./bin/local/copilot-darwin-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "MAC_ARM_KIB=$(du -k ./bin/local/copilot-darwin-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "LINUX_AMD_KIB=$(du -k ./bin/local/copilot-linux-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "LINUX_ARM_KIB=$(du -k ./bin/local/copilot-linux-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "WINDOWS_AMD_KIB=$(du -k ./bin/local/copilot.exe | awk '{ print $1}')" >> $GITHUB_OUTPUT + + - name: Check out PR commit + uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Compute new release binary sizes + id: new-bins + run: | + make release + echo "MAC_AMD_KIB=$(du -k ./bin/local/copilot-darwin-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "MAC_ARM_KIB=$(du -k ./bin/local/copilot-darwin-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "LINUX_AMD_KIB=$(du -k ./bin/local/copilot-linux-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "LINUX_ARM_KIB=$(du -k ./bin/local/copilot-linux-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT + echo "WINDOWS_AMD_KIB=$(du -k ./bin/local/copilot.exe | awk '{ print $1}')" >> $GITHUB_OUTPUT + + - name: Save sizes and PR number + run: | + mkdir artifacts + sizes_json="{ + \"macOS\": { + \"amd\": { + \"old\": ${{ steps.old-bins.outputs.MAC_AMD_KIB }}, + \"cur\": ${{ steps.new-bins.outputs.MAC_AMD_KIB }} + }, + \"arm\": { + \"old\": ${{ steps.old-bins.outputs.MAC_ARM_KIB }}, + \"cur\": ${{ steps.new-bins.outputs.MAC_ARM_KIB }} + } + }, + \"linux\": { + \"amd\": { + \"old\": ${{ steps.old-bins.outputs.LINUX_AMD_KIB }}, + \"cur\": ${{ steps.new-bins.outputs.LINUX_AMD_KIB }} + }, + \"arm\": { + \"old\": ${{ steps.old-bins.outputs.LINUX_ARM_KIB }}, + \"cur\": ${{ steps.new-bins.outputs.LINUX_ARM_KIB }} + } + }, + \"windows\": { + \"amd\": { + \"old\": ${{ steps.old-bins.outputs.WINDOWS_AMD_KIB }}, + \"cur\": ${{ steps.new-bins.outputs.WINDOWS_AMD_KIB }} + } + } + }" + echo ${{ github.event.number }} > artifacts/pr_number + echo ${sizes_json} > artifacts/sizes.json + + - uses: actions/upload-artifact@v3 + with: + name: pr_number_and_bin_sizes + path: artifacts/ diff --git a/.github/workflows/ci_size_writer.yml b/.github/workflows/ci_size_writer.yml new file mode 100644 index 00000000000..c83d9c6332f --- /dev/null +++ b/.github/workflows/ci_size_writer.yml @@ -0,0 +1,110 @@ +name: Comment PR with bin sizes + +on: + workflow_run: + workflows: ["Compute bin sizes"] + types: + - completed + +jobs: + comment_bin_size: + runs-on: ubuntu-latest + steps: + - name: Download artifacts + uses: actions/github-script@v6 + with: + script: | + let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.payload.workflow_run.id, + }); + let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => { + return artifact.name == "pr_number_and_bin_sizes" + })[0]; + let download = await github.rest.actions.downloadArtifact({ + owner: context.repo.owner, + repo: context.repo.repo, + artifact_id: matchArtifact.id, + archive_format: 'zip', + }); + let fs = require('fs'); + fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/artifacts.zip`, Buffer.from(download.data)); + + - name: Unzip artifacts + run: unzip artifacts.zip + + - name: Calculate size change + id: diff + uses: actions/github-script@v6 + with: + result-encoding: string + script: | + let fs = require('fs'); + let sizes_raw = fs.readFileSync('sizes.json'); + let sizes = JSON.parse(sizes_raw) + + const diff = (old, cur) => { + const res = (cur-old)/old * 100; + return res.toFixed(2); // 2 decimal places. + }; + + const lastRelease = '${{ steps.latestrelease.outputs.VERSION_TAG }}'; + let msg = ` + 🍕 Here are the new binary sizes! + + | Name | New size (kiB) | ${lastRelease} size (kiB) | Delta (%) | + |:-- |:--- |:--- |:-- | + `; + + let shouldAcknowledgeSize = false; + for (const os of Object.keys(sizes)) { + for (const arch of Object.keys(sizes[os])) { + const delta = diff(sizes[os][arch].old, sizes[os][arch].cur); + if (delta > 10) { + shouldAcknowledgeSize = true; + } + + let symbol = "❤️ "; + if (delta > 5) { + symbol = "😭 +"; + } else if (delta > 1) { + symbol = "🥺 +" + } else if (delta > 0) { + symbol = "+"; + } + msg += `| ${os} (${arch}) | ${sizes[os][arch].cur} | ${sizes[os][arch].old} | ${symbol}${delta} |` + "\n"; + } + } + + if (shouldAcknowledgeSize) { + msg += "**The binary size increased more than expected! Applying the `do-not-merge` label.**\n" + github.rest.issues.addLabels({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + labels: ['do-not-merge'] + }); + } + return msg; + + - name: Find PR number + id: find-pr-number + run: | + echo "PR_NUMBER=$(cat pr_number)" >> $GITHUB_OUTPUT + + - name: Find comment + uses: peter-evans/find-comment@v2 + id: fc + with: + issue-number: ${{ steps.find-pr-number.outputs.PR_NUMBER }} + comment-author: 'github-actions[bot]' + body-includes: 'Here are the new binary sizes' + + - name: Create or update comment + uses: peter-evans/create-or-update-comment@v2 + with: + comment-id: ${{ steps.fc.outputs.comment-id }} + issue-number: ${{ steps.find-pr-number.outputs.PR_NUMBER }} + body: ${{ steps.diff.outputs.result }} + edit-mode: replace \ No newline at end of file diff --git a/.github/workflows/ci_writer.yml b/.github/workflows/ci_writer.yml deleted file mode 100644 index 27140a2df0b..00000000000 --- a/.github/workflows/ci_writer.yml +++ /dev/null @@ -1,152 +0,0 @@ -name: ci annotate - -on: - pull_request_target: - types: - - opened - - edited - - synchronize - -jobs: - binsize: - runs-on: ubuntu-latest - steps: - - name: Set up Go - uses: actions/setup-go@v2 - with: - go-version: 1.18 - - - name: Set up Node - uses: actions/setup-node@v1 - with: - node-version: 16.x - - - name: Get latest release with tag - id: latestrelease - run: | - echo "VERSION_TAG=$(curl -s https://api.github.com/repos/aws/copilot-cli/releases/latest | jq '.tag_name' | sed 's/\"//g')" >> $GITHUB_OUTPUT - - - name: Check out latest release tag - uses: actions/checkout@v3 - with: - ref: ${{ steps.latestrelease.outputs.VERSION_TAG }} - repository: aws/copilot-cli - - - name: Compute old release binary sizes - id: old-bins - run: | - make release - echo "MAC_AMD_KIB=$(du -k ./bin/local/copilot-darwin-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "MAC_ARM_KIB=$(du -k ./bin/local/copilot-darwin-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "LINUX_AMD_KIB=$(du -k ./bin/local/copilot-linux-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "LINUX_ARM_KIB=$(du -k ./bin/local/copilot-linux-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "WINDOWS_AMD_KIB=$(du -k ./bin/local/copilot.exe | awk '{ print $1}')" >> $GITHUB_OUTPUT - - - name: Check out PR commit - uses: actions/checkout@v3 - - - name: Compute new release binary sizes - id: new-bins - run: | - make release - echo "MAC_AMD_KIB=$(du -k ./bin/local/copilot-darwin-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "MAC_ARM_KIB=$(du -k ./bin/local/copilot-darwin-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "LINUX_AMD_KIB=$(du -k ./bin/local/copilot-linux-amd64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "LINUX_ARM_KIB=$(du -k ./bin/local/copilot-linux-arm64 | awk '{ print $1}')" >> $GITHUB_OUTPUT - echo "WINDOWS_AMD_KIB=$(du -k ./bin/local/copilot.exe | awk '{ print $1}')" >> $GITHUB_OUTPUT - - - name: Calculate size change - id: diff - uses: actions/github-script@v6 - with: - result-encoding: string - script: | - const diff = (old, cur) => { - const res = (cur-old)/old * 100; - return res.toFixed(2); // 2 decimal places. - }; - - const dat = { - 'macOS': { - 'amd': { - 'old': parseFloat(${{ steps.old-bins.outputs.MAC_AMD_KIB }}), - 'cur': parseFloat(${{ steps.new-bins.outputs.MAC_AMD_KIB }}), - }, - 'arm': { - 'old': parseFloat(${{ steps.old-bins.outputs.MAC_ARM_KIB }}), - 'cur': parseFloat(${{ steps.new-bins.outputs.MAC_AMD_KIB }}), - }, - }, - 'linux': { - 'amd': { - 'old': parseFloat(${{ steps.old-bins.outputs.LINUX_AMD_KIB }}), - 'cur': parseFloat(${{ steps.new-bins.outputs.LINUX_AMD_KIB }}), - }, - 'arm': { - 'old': parseFloat(${{ steps.old-bins.outputs.LINUX_ARM_KIB }}), - 'cur': parseFloat(${{ steps.new-bins.outputs.LINUX_AMD_KIB }}), - }, - }, - 'windows': { - 'amd': { - 'old': parseFloat(${{ steps.old-bins.outputs.WINDOWS_AMD_KIB }}), - 'cur': parseFloat(${{ steps.new-bins.outputs.WINDOWS_AMD_KIB }}), - }, - }, - }; - - const lastRelease = '${{ steps.latestrelease.outputs.VERSION_TAG }}'; - let msg = ` - 🍕 Here are the new binary sizes! - - | Name | New size (kiB) | ${lastRelease} size (kiB) | Delta (%) | - |:-- |:--- |:--- |:-- | - `; - - let shouldAcknowledgeSize = false; - for (const os of Object.keys(dat)) { - for (const arch of Object.keys(dat[os])) { - const delta = diff(dat[os][arch].old, dat[os][arch].cur); - if (delta > 10) { - shouldAcknowledgeSize = true; - } - - let symbol = "❤️ "; - if (delta > 5) { - symbol = "😭 +"; - } else if (delta > 1) { - symbol = "🥺 +" - } else if (delta > 0) { - symbol = "+"; - } - msg += `| ${os} (${arch}) | ${dat[os][arch].cur} | ${dat[os][arch].old} | ${symbol}${delta} |` + "\n"; - } - } - - if (shouldAcknowledgeSize) { - msg += "**The binary size increased more than expected! Applying the `do-not-merge` label.**\n" - github.rest.issues.addLabels({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - labels: ['do-not-merge'] - }); - } - - return msg; - - - name: Find comment - uses: peter-evans/find-comment@v2 - id: fc - with: - issue-number: ${{ github.event.pull_request.number }} - comment-author: 'github-actions[bot]' - body-includes: 'Here are the new binary sizes' - - - name: Create or update comment - uses: peter-evans/create-or-update-comment@v2 - with: - comment-id: ${{ steps.fc.outputs.comment-id }} - issue-number: ${{ github.event.pull_request.number }} - body: ${{steps.diff.outputs.result}} - edit-mode: replace \ No newline at end of file From 2d3a3f8036941d4f366fc0888b03c7f8eb573f3b Mon Sep 17 00:00:00 2001 From: Wanxian Yang <79273084+Lou1415926@users.noreply.github.com> Date: Tue, 6 Dec 2022 16:58:50 -0800 Subject: [PATCH 2/2] chore: parse and package environment addons (#4221) This PR enable parsing of environment addons, as long as customized parameters and packaging. Related: #4219 By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License. --- internal/pkg/addon/addons.go | 175 ++++++--- internal/pkg/addon/addons_test.go | 360 ++++++++++++++++-- internal/pkg/addon/errors.go | 5 +- internal/pkg/addon/mocks/mock_addons.go | 65 ++++ internal/pkg/addon/package.go | 20 +- internal/pkg/addon/package_test.go | 101 ++++- internal/pkg/cli/deploy/svc.go | 4 +- .../lb_grpc_web_service_integration_test.go | 2 +- ...lb_network_web_service_integration_test.go | 2 +- .../stack/lb_web_service_integration_test.go | 2 +- .../stack/rd_web_svc_integration_test.go | 2 +- .../stack/scheduled_job_integration_test.go | 2 +- .../stack/stack_local_integration_test.go | 2 +- .../validate_template_integration_test.go | 4 +- ...windows_lb_web_service_integration_test.go | 2 +- .../stack/worker_service_integration_test.go | 2 +- .../pkg/template/artifactpath/artifactpath.go | 7 + internal/pkg/template/template_functions.go | 5 +- 18 files changed, 652 insertions(+), 110 deletions(-) diff --git a/internal/pkg/addon/addons.go b/internal/pkg/addon/addons.go index 67037178f62..e9b584e5a39 100644 --- a/internal/pkg/addon/addons.go +++ b/internal/pkg/addon/addons.go @@ -7,6 +7,7 @@ package addon import ( "fmt" "path/filepath" + "strconv" "strings" "github.com/dustin/go-humanize/english" @@ -18,6 +19,11 @@ const ( StackName = "AddonsStack" ) +var ( + wkldAddonsParameterReservedKeys = []string{"App", "Env", "Name"} + envAddonsParameterReservedKeys = []string{"App", "Name"} +) + var ( yamlExtensions = []string{".yaml", ".yml"} parameterFileNames = func() []string { @@ -33,49 +39,86 @@ var ( type workspaceReader interface { WorkloadAddonsPath(name string) string WorkloadAddonFilePath(wkldName, fName string) string + EnvAddonsPath() string + EnvAddonFilePath(fName string) string ListFiles(dirPath string) ([]string, error) ReadFile(fPath string) ([]byte, error) } -// Stack represents a CloudFormation stack. -type Stack struct { - template *cfnTemplate - parameters yaml.Node +// WorkloadStack represents a CloudFormation stack for workload addons. +type WorkloadStack struct { + stack workloadName string } -// Parse parses the 'addon/' directory for the given workload +// EnvironmentStack represents a CloudFormation stack for environment addons. +type EnvironmentStack struct { + stack +} + +type stack struct { + template *cfnTemplate + parameters yaml.Node +} + +type parser struct { + ws workspaceReader + addonsDirPath func() string + addonsFilePath func(fName string) string + validateParameters func(params yaml.Node) error +} + +// ParseFromWorkload parses the 'addon/' directory for the given workload // and returns a Stack created by merging the CloudFormation templates -// files found there. If no addons are found, Parse returns a nil +// files found there. If no addons are found, ParseFromWorkload returns a nil // Stack and ErrAddonsNotFound. -func Parse(workloadName string, ws workspaceReader) (*Stack, error) { - fNames, err := ws.ListFiles(ws.WorkloadAddonsPath(workloadName)) - if err != nil { - return nil, &ErrAddonsNotFound{ - WlName: workloadName, - ParentErr: err, - } +func ParseFromWorkload(workloadName string, ws workspaceReader) (*WorkloadStack, error) { + parser := parser{ + ws: ws, + addonsDirPath: func() string { + return ws.WorkloadAddonsPath(workloadName) + }, + addonsFilePath: func(fName string) string { + return ws.WorkloadAddonFilePath(workloadName, fName) + }, + validateParameters: func(params yaml.Node) error { + return validateReservedParameters(params, wkldAddonsParameterReservedKeys) + }, } - - template, err := parseTemplate(fNames, workloadName, ws) + stack, err := parser.stack() if err != nil { return nil, err } + return &WorkloadStack{ + stack: *stack, + workloadName: workloadName, + }, nil +} - params, err := parseParameters(fNames, workloadName, ws) +// ParseFromEnv parses the 'addon/' directory for environments +// and returns a Stack created by merging the CloudFormation templates +// files found there. If no addons are found, ParseFromWorkload returns a nil +// Stack and ErrAddonsNotFound. +func ParseFromEnv(ws workspaceReader) (*EnvironmentStack, error) { + parser := parser{ + ws: ws, + addonsDirPath: ws.EnvAddonsPath, + addonsFilePath: ws.EnvAddonFilePath, + validateParameters: func(params yaml.Node) error { + return validateReservedParameters(params, envAddonsParameterReservedKeys) + }, + } + stack, err := parser.stack() if err != nil { return nil, err } - - return &Stack{ - template: template, - parameters: params, - workloadName: workloadName, + return &EnvironmentStack{ + stack: *stack, }, nil } // Template returns Stack's CloudFormation template as a yaml string. -func (s *Stack) Template() (string, error) { +func (s *stack) Template() (string, error) { if s.template == nil { return "", nil } @@ -84,7 +127,7 @@ func (s *Stack) Template() (string, error) { } // Parameters returns Stack's CloudFormation parameters as a yaml string. -func (s *Stack) Parameters() (string, error) { +func (s *stack) Parameters() (string, error) { if s.parameters.IsZero() { return "", nil } @@ -93,7 +136,7 @@ func (s *Stack) Parameters() (string, error) { } // encode encodes v as a yaml string indented with 2 spaces. -func (s *Stack) encode(v any) (string, error) { +func (s *stack) encode(v any) (string, error) { str := &strings.Builder{} enc := yaml.NewEncoder(str) enc.SetIndent(2) @@ -105,30 +148,50 @@ func (s *Stack) encode(v any) (string, error) { return str.String(), nil } -// parseTemplate merges CloudFormation templates under the "addons/" directory of a workload -// into a single CloudFormation template and returns it. +func (p *parser) stack() (*stack, error) { + path := p.addonsDirPath() + fNames, err := p.ws.ListFiles(path) + if err != nil { + return nil, fmt.Errorf("list addons under path %s: %w", path, &ErrAddonsNotFound{ + ParentErr: err, + }) + } + template, err := p.parseTemplate(fNames) + if err != nil { + return nil, err + } + params, err := p.parseParameters(fNames) + if err != nil { + return nil, err + } + return &stack{ + template: template, + parameters: params, + }, nil +} + +// parseTemplate merges CloudFormation templates under the "addons/" directory into a single CloudFormation +// template and returns it. // // If the addons directory doesn't exist or no yaml files are found in // the addons directory, it returns the empty string and // ErrAddonsNotFound. -func parseTemplate(fNames []string, workloadName string, ws workspaceReader) (*cfnTemplate, error) { +func (p *parser) parseTemplate(fNames []string) (*cfnTemplate, error) { templateFiles := filterFiles(fNames, yamlMatcher, nonParamsMatcher) if len(templateFiles) == 0 { - return nil, &ErrAddonsNotFound{ - WlName: workloadName, - } + return nil, &ErrAddonsNotFound{} } mergedTemplate := newCFNTemplate("merged") for _, fname := range templateFiles { - path := ws.WorkloadAddonFilePath(workloadName, fname) - out, err := ws.ReadFile(path) + path := p.addonsFilePath(fname) + out, err := p.ws.ReadFile(path) if err != nil { - return nil, fmt.Errorf("read addon %s under %s: %w", fname, workloadName, err) + return nil, fmt.Errorf("read addons file %q under path %s: %w", fname, path, err) } tpl := newCFNTemplate(fname) if err := yaml.Unmarshal(out, tpl); err != nil { - return nil, fmt.Errorf("unmarshal addon %s under %s: %w", fname, workloadName, err) + return nil, fmt.Errorf("unmarshal addon %s under path %s: %w", fname, path, err) } if err := mergedTemplate.merge(tpl); err != nil { return nil, err @@ -143,49 +206,44 @@ func parseTemplate(fNames []string, workloadName string, ws workspaceReader) (*c // If there are addons but no parameters file defined, then returns "" and nil for error. // If there are multiple parameters files, then returns "" and cannot define multiple parameter files error. // If the addons parameters use the reserved parameter names, then returns "" and a reserved parameter error. -func parseParameters(fNames []string, workloadName string, ws workspaceReader) (yaml.Node, error) { +func (p *parser) parseParameters(fNames []string) (yaml.Node, error) { paramFiles := filterFiles(fNames, paramsMatcher) if len(paramFiles) == 0 { return yaml.Node{}, nil } if len(paramFiles) > 1 { - return yaml.Node{}, fmt.Errorf("defining %s is not allowed under %s addons/", english.WordSeries(parameterFileNames, "and"), workloadName) + return yaml.Node{}, fmt.Errorf("defining %s is not allowed under addons/", english.WordSeries(parameterFileNames, "and")) } paramFile := paramFiles[0] - path := ws.WorkloadAddonFilePath(workloadName, paramFile) - raw, err := ws.ReadFile(path) + path := p.addonsFilePath(paramFile) + raw, err := p.ws.ReadFile(path) if err != nil { - return yaml.Node{}, fmt.Errorf("read parameter file %s under %s addons/: %w", paramFile, workloadName, err) + return yaml.Node{}, fmt.Errorf("read parameter file %s under path %s: %w", paramFile, path, err) } content := struct { Parameters yaml.Node `yaml:"Parameters"` }{} if err := yaml.Unmarshal(raw, &content); err != nil { - return yaml.Node{}, fmt.Errorf("unmarshal 'Parameters' in file %s under %s addons/: %w", paramFile, workloadName, err) + return yaml.Node{}, fmt.Errorf("unmarshal 'Parameters' in file %s: %w", paramFile, err) } if content.Parameters.IsZero() { - return yaml.Node{}, fmt.Errorf("must define field 'Parameters' in file %s under %s addons/", paramFile, workloadName) + return yaml.Node{}, fmt.Errorf("must define field 'Parameters' in file %s under path %s", paramFile, path) } - if err := validateReservedParameters(content.Parameters, paramFile, workloadName); err != nil { + if err := p.validateParameters(content.Parameters); err != nil { return yaml.Node{}, err } return content.Parameters, nil } -func validateReservedParameters(params yaml.Node, fname, workloadName string) error { - content := struct { - App yaml.Node `yaml:"App"` - Env yaml.Node `yaml:"Env"` - Name yaml.Node `yaml:"Name"` - }{} +func validateReservedParameters(params yaml.Node, reservedKeys []string) error { + content := make(map[string]yaml.Node, len(reservedKeys)) if err := params.Decode(&content); err != nil { - return fmt.Errorf("decode content of parameters file %s under %s addons/", fname, workloadName) + return fmt.Errorf("decode \"Parameters\" section of the parameters file: %w", err) } - - for _, field := range []yaml.Node{content.App, content.Env, content.Name} { - if !field.IsZero() { - return fmt.Errorf("reserved parameters 'App', 'Env', and 'Name' cannot be declared in %s under %s addons/", fname, workloadName) + for _, key := range reservedKeys { + if _, ok := content[key]; ok { + return fmt.Errorf("reserved parameters %s cannot be declared", english.WordSeries(quoteSlice(reservedKeys), "and")) } } return nil @@ -228,3 +286,14 @@ func contains(arr []string, el string) bool { } return false } + +func quoteSlice(elems []string) []string { + if len(elems) == 0 { + return nil + } + quotedElems := make([]string, len(elems)) + for i, el := range elems { + quotedElems[i] = strconv.Quote(el) + } + return quotedElems +} diff --git a/internal/pkg/addon/addons_test.go b/internal/pkg/addon/addons_test.go index 0c2c16a1296..711bc3baad2 100644 --- a/internal/pkg/addon/addons_test.go +++ b/internal/pkg/addon/addons_test.go @@ -5,6 +5,7 @@ package addon import ( "errors" + "fmt" "os" "path/filepath" "testing" @@ -14,7 +15,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestTemplate(t *testing.T) { +func TestWorkload_Template(t *testing.T) { const ( testSvcName = "mysvc" testJobName = "resizer" @@ -24,8 +25,9 @@ func TestTemplate(t *testing.T) { workloadName string setupMocks func(m addonMocks) - wantedTemplate string - wantedErr error + wantedTemplate string + wantedErr error + wantedAddonsNotFoundError bool }{ "return ErrAddonsNotFound if addons doesn't exist in a service": { workloadName: testSvcName, @@ -33,10 +35,9 @@ func TestTemplate(t *testing.T) { m.ws.EXPECT().WorkloadAddonsPath(testSvcName).Return("mockPath") m.ws.EXPECT().ListFiles("mockPath").Return(nil, testErr) }, - wantedErr: &ErrAddonsNotFound{ - WlName: testSvcName, + wantedErr: fmt.Errorf("list addons under path mockPath: %w", &ErrAddonsNotFound{ ParentErr: testErr, - }, + }), }, "return ErrAddonsNotFound if addons doesn't exist in a job": { workloadName: testJobName, @@ -44,10 +45,9 @@ func TestTemplate(t *testing.T) { m.ws.EXPECT().WorkloadAddonsPath(testJobName).Return("mockPath") m.ws.EXPECT().ListFiles("mockPath").Return(nil, testErr) }, - wantedErr: &ErrAddonsNotFound{ - WlName: testJobName, + wantedErr: fmt.Errorf("list addons under path mockPath: %w", &ErrAddonsNotFound{ ParentErr: testErr, - }, + }), }, "return ErrAddonsNotFound if addons directory is empty in a service": { workloadName: testSvcName, @@ -56,7 +56,6 @@ func TestTemplate(t *testing.T) { m.ws.EXPECT().ListFiles("mockPath").Return([]string{}, nil) }, wantedErr: &ErrAddonsNotFound{ - WlName: testSvcName, ParentErr: nil, }, }, @@ -67,7 +66,6 @@ func TestTemplate(t *testing.T) { m.ws.EXPECT().ListFiles("mockPath").Return([]string{"gitkeep"}, nil) }, wantedErr: &ErrAddonsNotFound{ - WlName: testSvcName, ParentErr: nil, }, }, @@ -78,18 +76,9 @@ func TestTemplate(t *testing.T) { m.ws.EXPECT().ListFiles("mockPath").Return([]string{"addons.parameters.yml", "addons.parameters.yaml"}, nil) }, wantedErr: &ErrAddonsNotFound{ - WlName: testSvcName, ParentErr: nil, }, }, - "print correct error message for ErrAddonsNotFound": { - workloadName: testJobName, - setupMocks: func(m addonMocks) { - m.ws.EXPECT().WorkloadAddonsPath(testJobName).Return("mockPath") - m.ws.EXPECT().ListFiles("mockPath").Return(nil, testErr) - }, - wantedErr: errors.New("read addons directory for resizer: some error"), - }, "return err on invalid Metadata fields": { workloadName: testSvcName, setupMocks: func(m addonMocks) { @@ -221,12 +210,13 @@ func TestTemplate(t *testing.T) { } // WHEN - stack, err := Parse(tc.workloadName, mocks.ws) + stack, err := ParseFromWorkload(tc.workloadName, mocks.ws) if tc.wantedErr != nil { require.EqualError(t, err, tc.wantedErr.Error()) return } require.NoError(t, err) + require.Equal(t, tc.workloadName, stack.workloadName) template, err := stack.Template() require.NoError(t, err) @@ -235,22 +225,21 @@ func TestTemplate(t *testing.T) { } } -func TestParameters(t *testing.T) { +func TestWorkload_Parameters(t *testing.T) { testCases := map[string]struct { setupMocks func(m addonMocks) wantedParams string - wantedErr string + wantedErr error }{ "returns ErrAddonsNotFound if there is no addons/ directory defined": { setupMocks: func(m addonMocks) { m.ws.EXPECT().WorkloadAddonsPath("api").Return("mockPath") m.ws.EXPECT().ListFiles("mockPath").Return(nil, errors.New("some error")) }, - wantedErr: (&ErrAddonsNotFound{ - WlName: "api", + wantedErr: fmt.Errorf(`list addons under path mockPath: %w`, &ErrAddonsNotFound{ ParentErr: errors.New("some error"), - }).Error(), + }), }, "returns empty string and nil if there are no parameter files under addons/": { setupMocks: func(m addonMocks) { @@ -267,7 +256,7 @@ func TestParameters(t *testing.T) { m.ws.EXPECT().WorkloadAddonFilePath("api", "database.yml").Return("mockPath") m.ws.EXPECT().ReadFile("mockPath").Return(nil, nil) }, - wantedErr: "defining addons.parameters.yaml and addons.parameters.yml is not allowed under api addons/", + wantedErr: errors.New("defining addons.parameters.yaml and addons.parameters.yml is not allowed under addons/"), }, "returns an error if cannot read parameter file under addons/": { setupMocks: func(m addonMocks) { @@ -278,7 +267,7 @@ func TestParameters(t *testing.T) { m.ws.EXPECT().WorkloadAddonFilePath("api", "addons.parameters.yml").Return("mockPath") m.ws.EXPECT().ReadFile("mockPath").Return(nil, errors.New("some error")) }, - wantedErr: "read parameter file addons.parameters.yml under api addons/: some error", + wantedErr: errors.New("read parameter file addons.parameters.yml under path mockPath: some error"), }, "returns an error if there are no 'Parameters' field defined in a parameters file": { setupMocks: func(m addonMocks) { @@ -289,7 +278,7 @@ func TestParameters(t *testing.T) { m.ws.EXPECT().WorkloadAddonFilePath("api", "addons.parameters.yml").Return("mockPath") m.ws.EXPECT().ReadFile("mockPath").Return([]byte(""), nil) }, - wantedErr: "must define field 'Parameters' in file addons.parameters.yml under api addons/", + wantedErr: errors.New("must define field 'Parameters' in file addons.parameters.yml under path mockPath"), }, "returns an error if reserved parameter fields is redefined in a parameters file": { setupMocks: func(m addonMocks) { @@ -308,7 +297,7 @@ Parameters: DiscoveryServiceArn: !GetAtt DiscoveryService.Arn `), nil) }, - wantedErr: "reserved parameters 'App', 'Env', and 'Name' cannot be declared in addons.parameters.yml under api addons/", + wantedErr: errors.New(`reserved parameters "App", "Env" and "Name" cannot be declared`), }, "returns the content of Parameters on success": { setupMocks: func(m addonMocks) { @@ -350,9 +339,314 @@ DiscoveryServiceArn: !GetAtt DiscoveryService.Arn } // WHEN - stack, err := Parse("api", mocks.ws) - if tc.wantedErr != "" { - require.EqualError(t, err, tc.wantedErr) + stack, err := ParseFromWorkload("api", mocks.ws) + if tc.wantedErr != nil { + require.EqualError(t, err, tc.wantedErr.Error()) + return + } + require.NoError(t, err) + require.Equal(t, "api", stack.workloadName) + + params, err := stack.Parameters() + require.NoError(t, err) + require.Equal(t, tc.wantedParams, params) + }) + } +} + +func TestEnv_Template(t *testing.T) { + testErr := errors.New("some error") + testCases := map[string]struct { + setupMocks func(m addonMocks) + + wantedTemplate string + wantedErr error + }{ + "return ErrAddonsNotFound if addons doesn't exist in a service": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return(nil, testErr) + }, + wantedErr: fmt.Errorf("list addons under path mockPath: %w", &ErrAddonsNotFound{ + ParentErr: testErr, + }), + }, + "return ErrAddonsNotFound if addons directory is empty in a service": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{}, nil) + }, + wantedErr: &ErrAddonsNotFound{}, + }, + "return ErrAddonsNotFound if addons directory does not contain yaml files in a service": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"gitkeep"}, nil) + }, + wantedErr: &ErrAddonsNotFound{}, + }, + "ignore addons.parameters.yml files": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"addons.parameters.yml", "addons.parameters.yaml"}, nil) + }, + wantedErr: &ErrAddonsNotFound{}, + }, + "return err on invalid Metadata fields": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"first.yaml", "invalid-metadata.yaml"}, nil) + + first, _ := os.ReadFile(filepath.Join("testdata", "merge", "first.yaml")) + m.ws.EXPECT().EnvAddonFilePath("first.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(first, nil) + + second, _ := os.ReadFile(filepath.Join("testdata", "merge", "invalid-metadata.yaml")) + m.ws.EXPECT().EnvAddonFilePath("invalid-metadata.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(second, nil) + }, + wantedErr: errors.New(`metadata key "Services" defined in "first.yaml" at Ln 4, Col 7 is different than in "invalid-metadata.yaml" at Ln 3, Col 5`), + }, + "returns err on invalid Parameters fields": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"first.yaml", "invalid-parameters.yaml"}, nil) + + first, _ := os.ReadFile(filepath.Join("testdata", "merge", "first.yaml")) + m.ws.EXPECT().EnvAddonFilePath("first.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(first, nil) + + second, _ := os.ReadFile(filepath.Join("testdata", "merge", "invalid-parameters.yaml")) + m.ws.EXPECT().EnvAddonFilePath("invalid-parameters.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(second, nil) + }, + wantedErr: errors.New(`parameter logical ID "Name" defined in "first.yaml" at Ln 15, Col 9 is different than in "invalid-parameters.yaml" at Ln 3, Col 7`), + }, + "returns err on invalid Mappings fields": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"first.yaml", "invalid-mappings.yaml"}, nil) + + first, _ := os.ReadFile(filepath.Join("testdata", "merge", "first.yaml")) + m.ws.EXPECT().EnvAddonFilePath("first.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(first, nil) + + second, _ := os.ReadFile(filepath.Join("testdata", "merge", "invalid-mappings.yaml")) + m.ws.EXPECT().EnvAddonFilePath("invalid-mappings.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(second, nil) + }, + wantedErr: errors.New(`mapping "MyTableDynamoDBSettings.test" defined in "first.yaml" at Ln 21, Col 13 is different than in "invalid-mappings.yaml" at Ln 4, Col 7`), + }, + "returns err on invalid Conditions fields": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"first.yaml", "invalid-conditions.yaml"}, nil) + + first, _ := os.ReadFile(filepath.Join("testdata", "merge", "first.yaml")) + m.ws.EXPECT().EnvAddonFilePath("first.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(first, nil) + + second, _ := os.ReadFile(filepath.Join("testdata", "merge", "invalid-conditions.yaml")) + m.ws.EXPECT().EnvAddonFilePath("invalid-conditions.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(second, nil) + }, + wantedErr: errors.New(`condition "IsProd" defined in "first.yaml" at Ln 28, Col 13 is different than in "invalid-conditions.yaml" at Ln 2, Col 13`), + }, + "returns err on invalid Resources fields": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"first.yaml", "invalid-resources.yaml"}, nil) + + first, _ := os.ReadFile(filepath.Join("testdata", "merge", "first.yaml")) + m.ws.EXPECT().EnvAddonFilePath("first.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(first, nil) + + second, _ := os.ReadFile(filepath.Join("testdata", "merge", "invalid-resources.yaml")) + m.ws.EXPECT().EnvAddonFilePath("invalid-resources.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(second, nil) + }, + wantedErr: errors.New(`resource "MyTable" defined in "first.yaml" at Ln 34, Col 9 is different than in "invalid-resources.yaml" at Ln 3, Col 5`), + }, + "returns err on invalid Outputs fields": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"first.yaml", "invalid-outputs.yaml"}, nil) + + first, _ := os.ReadFile(filepath.Join("testdata", "merge", "first.yaml")) + m.ws.EXPECT().EnvAddonFilePath("first.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(first, nil) + + second, _ := os.ReadFile(filepath.Join("testdata", "merge", "invalid-outputs.yaml")) + m.ws.EXPECT().EnvAddonFilePath("invalid-outputs.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(second, nil) + }, + wantedErr: errors.New(`output "MyTableAccessPolicy" defined in "first.yaml" at Ln 85, Col 9 is different than in "invalid-outputs.yaml" at Ln 3, Col 5`), + }, + "merge fields successfully": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"first.yaml", "second.yaml"}, nil) + + first, _ := os.ReadFile(filepath.Join("testdata", "merge", "first.yaml")) + m.ws.EXPECT().EnvAddonFilePath("first.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(first, nil) + + second, _ := os.ReadFile(filepath.Join("testdata", "merge", "second.yaml")) + m.ws.EXPECT().EnvAddonFilePath("second.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(second, nil) + }, + wantedTemplate: func() string { + wanted, _ := os.ReadFile(filepath.Join("testdata", "merge", "wanted.yaml")) + return string(wanted) + }(), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := addonMocks{ + ws: mocks.NewMockworkspaceReader(ctrl), + } + if tc.setupMocks != nil { + tc.setupMocks(m) + } + + // WHEN + stack, err := ParseFromEnv(m.ws) + if tc.wantedErr != nil { + require.EqualError(t, err, tc.wantedErr.Error()) + return + } + require.NoError(t, err) + + template, err := stack.Template() + require.NoError(t, err) + require.Equal(t, tc.wantedTemplate, template) + }) + } +} + +func TestEnv_Parameters(t *testing.T) { + testCases := map[string]struct { + setupMocks func(m addonMocks) + + wantedParams string + wantedErr error + }{ + "returns ErrAddonsNotFound if there is no addons/ directory defined": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return(nil, errors.New("some error")) + }, + wantedErr: fmt.Errorf("list addons under path mockPath: %w", &ErrAddonsNotFound{ + ParentErr: errors.New("some error"), + }), + }, + "returns empty string and nil if there are no parameter files under addons/": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"database.yaml"}, nil) + m.ws.EXPECT().EnvAddonFilePath("database.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(nil, nil) + }, + }, + "returns an error if there are multiple parameter files defined under addons/": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"database.yml", "addons.parameters.yml", "addons.parameters.yaml"}, nil) + m.ws.EXPECT().EnvAddonFilePath("database.yml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(nil, nil) + }, + wantedErr: errors.New("defining addons.parameters.yaml and addons.parameters.yml is not allowed under addons/"), + }, + "returns an error if cannot read parameter file under addons/": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"template.yml", "addons.parameters.yml"}, nil) + m.ws.EXPECT().EnvAddonFilePath("template.yml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(nil, nil) + m.ws.EXPECT().EnvAddonFilePath("addons.parameters.yml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(nil, errors.New("some error")) + }, + wantedErr: errors.New("read parameter file addons.parameters.yml under path mockPath: some error"), + }, + "returns an error if there are no 'Parameters' field defined in a parameters file": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"template.yaml", "addons.parameters.yml"}, nil) + m.ws.EXPECT().EnvAddonFilePath("template.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(nil, nil) + m.ws.EXPECT().EnvAddonFilePath("addons.parameters.yml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return([]byte(""), nil) + }, + wantedErr: errors.New("must define field 'Parameters' in file addons.parameters.yml under path mockPath"), + }, + "returns an error if reserved parameter fields is redefined in a parameters file": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"template.yaml", "addons.parameters.yml"}, nil) + m.ws.EXPECT().EnvAddonFilePath("template.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(nil, nil) + m.ws.EXPECT().EnvAddonFilePath("addons.parameters.yml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return([]byte(` +Parameters: + App: !Ref AppName + Env: !Ref EnvName + Name: !Ref WorkloadName + EventsQueue: + !Ref EventsQueue + DiscoveryServiceArn: !GetAtt DiscoveryService.Arn +`), nil) + }, + wantedErr: errors.New(`reserved parameters "App" and "Name" cannot be declared`), + }, + "returns the content of Parameters on success": { + setupMocks: func(m addonMocks) { + m.ws.EXPECT().EnvAddonsPath().Return("mockPath") + m.ws.EXPECT().ListFiles("mockPath").Return([]string{"template.yaml", "addons.parameters.yaml"}, nil) + m.ws.EXPECT().EnvAddonFilePath("template.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return(nil, nil) + m.ws.EXPECT().EnvAddonFilePath("addons.parameters.yaml").Return("mockPath") + m.ws.EXPECT().ReadFile("mockPath").Return([]byte(` +Parameters: + EventsQueue: + !Ref EventsQueue + ServiceName: !Ref Service + SecurityGroupId: + Fn::GetAtt: [ServiceSecurityGroup, Id] + DiscoveryServiceArn: !GetAtt DiscoveryService.Arn +`), nil) + }, + wantedParams: `EventsQueue: !Ref EventsQueue +ServiceName: !Ref Service +SecurityGroupId: + Fn::GetAtt: [ServiceSecurityGroup, Id] +DiscoveryServiceArn: !GetAtt DiscoveryService.Arn +`, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mocks := addonMocks{ + ws: mocks.NewMockworkspaceReader(ctrl), + } + if tc.setupMocks != nil { + tc.setupMocks(mocks) + } + + // WHEN + stack, err := ParseFromEnv(mocks.ws) + if tc.wantedErr != nil { + require.EqualError(t, err, tc.wantedErr.Error()) return } require.NoError(t, err) diff --git a/internal/pkg/addon/errors.go b/internal/pkg/addon/errors.go index 3cfcebd9060..599d971005d 100644 --- a/internal/pkg/addon/errors.go +++ b/internal/pkg/addon/errors.go @@ -12,15 +12,14 @@ import ( // ErrAddonsNotFound occurs when an addons directory for a workload is either not found or empty. type ErrAddonsNotFound struct { - WlName string ParentErr error } func (e *ErrAddonsNotFound) Error() string { if e.ParentErr != nil { - return fmt.Sprintf("read addons directory for %s: %v", e.WlName, e.ParentErr) + return fmt.Sprintf("no addons found: %v", e.ParentErr) } - return fmt.Sprintf("read addons directory for %s: no addons found", e.WlName) + return "no addons found" } type errKeyAlreadyExists struct { diff --git a/internal/pkg/addon/mocks/mock_addons.go b/internal/pkg/addon/mocks/mock_addons.go index a582c273a3d..0824bb6f4e5 100644 --- a/internal/pkg/addon/mocks/mock_addons.go +++ b/internal/pkg/addon/mocks/mock_addons.go @@ -33,6 +33,34 @@ func (m *MockworkspaceReader) EXPECT() *MockworkspaceReaderMockRecorder { return m.recorder } +// EnvAddonFilePath mocks base method. +func (m *MockworkspaceReader) EnvAddonFilePath(fName string) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "EnvAddonFilePath", fName) + ret0, _ := ret[0].(string) + return ret0 +} + +// EnvAddonFilePath indicates an expected call of EnvAddonFilePath. +func (mr *MockworkspaceReaderMockRecorder) EnvAddonFilePath(fName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnvAddonFilePath", reflect.TypeOf((*MockworkspaceReader)(nil).EnvAddonFilePath), fName) +} + +// EnvAddonsPath mocks base method. +func (m *MockworkspaceReader) EnvAddonsPath() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "EnvAddonsPath") + ret0, _ := ret[0].(string) + return ret0 +} + +// EnvAddonsPath indicates an expected call of EnvAddonsPath. +func (mr *MockworkspaceReaderMockRecorder) EnvAddonsPath() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnvAddonsPath", reflect.TypeOf((*MockworkspaceReader)(nil).EnvAddonsPath)) +} + // ListFiles mocks base method. func (m *MockworkspaceReader) ListFiles(dirPath string) ([]string, error) { m.ctrl.T.Helper() @@ -90,3 +118,40 @@ func (mr *MockworkspaceReaderMockRecorder) WorkloadAddonsPath(name interface{}) mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WorkloadAddonsPath", reflect.TypeOf((*MockworkspaceReader)(nil).WorkloadAddonsPath), name) } + +// MockcontentNode is a mock of contentNode interface. +type MockcontentNode struct { + ctrl *gomock.Controller + recorder *MockcontentNodeMockRecorder +} + +// MockcontentNodeMockRecorder is the mock recorder for MockcontentNode. +type MockcontentNodeMockRecorder struct { + mock *MockcontentNode +} + +// NewMockcontentNode creates a new mock instance. +func NewMockcontentNode(ctrl *gomock.Controller) *MockcontentNode { + mock := &MockcontentNode{ctrl: ctrl} + mock.recorder = &MockcontentNodeMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockcontentNode) EXPECT() *MockcontentNodeMockRecorder { + return m.recorder +} + +// Marshaled mocks base method. +func (m *MockcontentNode) Marshaled() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Marshaled") + ret0, _ := ret[0].(bool) + return ret0 +} + +// Marshaled indicates an expected call of Marshaled. +func (mr *MockcontentNodeMockRecorder) Marshaled() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Marshaled", reflect.TypeOf((*MockcontentNode)(nil).Marshaled)) +} diff --git a/internal/pkg/addon/package.go b/internal/pkg/addon/package.go index a213e508e32..c203fb17c27 100644 --- a/internal/pkg/addon/package.go +++ b/internal/pkg/addon/package.go @@ -197,14 +197,26 @@ type PackageConfig struct { WorkspacePath string FS afero.Fs - workloadName string + s3Path func(hash string) string } // Package finds references to local files in Stack's template, uploads // the files to S3, and replaces the file path with the S3 location. -func (s *Stack) Package(cfg PackageConfig) error { - cfg.workloadName = s.workloadName +func (s *EnvironmentStack) Package(cfg PackageConfig) error { + cfg.s3Path = artifactpath.EnvironmentAddonAsset + return s.packageAssets(cfg) +} + +// Package finds references to local files in Stack's template, uploads +// the files to S3, and replaces the file path with the S3 location. +func (s *WorkloadStack) Package(cfg PackageConfig) error { + cfg.s3Path = func(hash string) string { + return artifactpath.AddonAsset(s.workloadName, hash) + } + return s.packageAssets(cfg) +} +func (s *stack) packageAssets(cfg PackageConfig) error { err := cfg.packageIncludeTransforms(&s.template.Metadata, &s.template.Mappings, &s.template.Conditions, &s.template.Transform, &s.template.Resources, &s.template.Outputs) if err != nil { return fmt.Errorf("package transforms: %w", err) @@ -363,7 +375,7 @@ func (p *PackageConfig) uploadAddonAsset(assetPath string, forceZip bool) (templ return template.S3ObjectLocation{}, fmt.Errorf("create asset: %w", err) } - s3Path := artifactpath.AddonAsset(p.workloadName, asset.hash) + s3Path := p.s3Path(asset.hash) url, err := p.Uploader.Upload(p.Bucket, s3Path, asset.data) if err != nil { return template.S3ObjectLocation{}, fmt.Errorf("upload %s to s3 bucket %s: %w", assetPath, p.Bucket, err) diff --git a/internal/pkg/addon/package_test.go b/internal/pkg/addon/package_test.go index 769cb520fbd..59e5f823b89 100644 --- a/internal/pkg/addon/package_test.go +++ b/internal/pkg/addon/package_test.go @@ -396,9 +396,11 @@ Resources: tc.setupMocks(mocks) } - stack := &Stack{ + stack := &WorkloadStack{ workloadName: wlName, - template: newCFNTemplate("merged"), + stack: stack{ + template: newCFNTemplate("merged"), + }, } require.NoError(t, yaml.Unmarshal([]byte(tc.inTemplate), stack.template)) @@ -422,4 +424,99 @@ Resources: require.Equal(t, strings.TrimSpace(tc.outTemplate), strings.TrimSpace(tmpl)) }) } + +} + +func TestEnvironmentAddonStack_PackagePackage(t *testing.T) { + const ( + wsPath = "/" + bucket = "mockBucket" + ) + + lambdaZipHash := sha256.New() + indexZipHash := sha256.New() + indexFileHash := sha256.New() + + // fs has the following structure: + // . + // ├─ lambda + // │ ├─ index.js (contains lambda function) + // ┴ └─ test.js (empty) + fs := afero.NewMemMapFs() + fs.Mkdir("/lambda", 0644) + + f, _ := fs.Create("/lambda/index.js") + defer f.Close() + info, _ := f.Stat() + io.MultiWriter(lambdaZipHash, indexZipHash).Write([]byte("index.js " + info.Mode().String())) + io.MultiWriter(f, lambdaZipHash, indexZipHash, indexFileHash).Write([]byte(`exports.handler = function(event, context) {}`)) + + f2, _ := fs.Create("/lambda/test.js") + info, _ = f2.Stat() + lambdaZipHash.Write([]byte("test.js " + info.Mode().String())) + + indexZipS3PathForEnvironmentAddon := fmt.Sprintf("manual/addons/environments/assets/%s", hex.EncodeToString(indexZipHash.Sum(nil))) + t.Run("package zipped AWS::Lambda::Function for environment addons", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // Set up mocks. + m := addonMocks{ + uploader: mocks.NewMockuploader(ctrl), + ws: mocks.NewMockworkspaceReader(ctrl), + } + m.uploader.EXPECT().Upload(bucket, indexZipS3PathForEnvironmentAddon, gomock.Any()).Return(s3.URL("us-west-2", bucket, "asdf"), nil) + + // WHEN. + inTemplate := ` +Resources: + Test: + Metadata: + "testKey": "testValue" + Type: AWS::Lambda::Function + Properties: + Code: lambda/index.js + Handler: "index.handler" + Timeout: 900 + MemorySize: 512 + Role: !GetAtt "TestRole.Arn" + Runtime: nodejs16.x +` + stack := &EnvironmentStack{ + stack: stack{ + template: newCFNTemplate("merged"), + }, + } + require.NoError(t, yaml.Unmarshal([]byte(inTemplate), stack.template)) + config := PackageConfig{ + Bucket: bucket, + WorkspacePath: wsPath, + Uploader: m.uploader, + FS: fs, + } + err := stack.Package(config) + + // Expect. + outTemplate := ` +Resources: + Test: + Metadata: + "testKey": "testValue" + Type: AWS::Lambda::Function + Properties: + Code: + S3Bucket: mockBucket + S3Key: asdf + Handler: "index.handler" + Timeout: 900 + MemorySize: 512 + Role: !GetAtt "TestRole.Arn" + Runtime: nodejs16.x +` + require.NoError(t, err) + tmpl, err := stack.Template() + require.NoError(t, err) + require.Equal(t, strings.TrimSpace(outTemplate), strings.TrimSpace(tmpl)) + }) + } diff --git a/internal/pkg/cli/deploy/svc.go b/internal/pkg/cli/deploy/svc.go index 6a9959dce2f..f916df66d47 100644 --- a/internal/pkg/cli/deploy/svc.go +++ b/internal/pkg/cli/deploy/svc.go @@ -217,11 +217,11 @@ func newWorkloadDeployer(in *WorkloadDeployerInput) (*workloadDeployer, error) { } var addons stackBuilder - addons, err = addon.Parse(in.Name, ws) + addons, err = addon.ParseFromWorkload(in.Name, ws) if err != nil { var notFoundErr *addon.ErrAddonsNotFound if !errors.As(err, ¬FoundErr) { - return nil, fmt.Errorf("parse addons stack: %w", err) + return nil, fmt.Errorf("parse addons stack for workload %s: %w", in.Name, err) } addons = nil // so that we can check for no addons with nil comparison } diff --git a/internal/pkg/deploy/cloudformation/stack/lb_grpc_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_grpc_web_service_integration_test.go index 5fd8eec3728..6db1074b787 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_grpc_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_grpc_web_service_integration_test.go @@ -70,7 +70,7 @@ func TestGrpcLoadBalancedWebService_Template(t *testing.T) { require.NoError(t, err) ws, err := workspace.Use(fs) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go index 23a664112ac..7b4278660b4 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go @@ -91,7 +91,7 @@ func TestNetworkLoadBalancedWebService_Template(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go index 81354ab305a..b3b98e91a95 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go @@ -91,7 +91,7 @@ func TestLoadBalancedWebService_TemplateInteg(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/rd_web_svc_integration_test.go b/internal/pkg/deploy/cloudformation/stack/rd_web_svc_integration_test.go index f8a07ee215b..ca9e30e0f47 100644 --- a/internal/pkg/deploy/cloudformation/stack/rd_web_svc_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/rd_web_svc_integration_test.go @@ -67,7 +67,7 @@ func TestRDWS_Template(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/scheduled_job_integration_test.go b/internal/pkg/deploy/cloudformation/stack/scheduled_job_integration_test.go index 1bc94d59b60..9f82a648473 100644 --- a/internal/pkg/deploy/cloudformation/stack/scheduled_job_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/scheduled_job_integration_test.go @@ -58,7 +58,7 @@ func TestScheduledJob_Template(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go b/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go index 3f3e5068475..abee52f661c 100644 --- a/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/stack_local_integration_test.go @@ -59,7 +59,7 @@ func Test_Stack_Local_Integration(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go b/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go index 8eadded0651..6bb5f1e9b05 100644 --- a/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/validate_template_integration_test.go @@ -43,7 +43,7 @@ func TestAutoscalingIntegration_Validate(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) @@ -104,7 +104,7 @@ func TestScheduledJob_Validate(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/windows_lb_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/windows_lb_web_service_integration_test.go index 6212fd64156..f9e80643abb 100644 --- a/internal/pkg/deploy/cloudformation/stack/windows_lb_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/windows_lb_web_service_integration_test.go @@ -58,7 +58,7 @@ func TestWindowsLoadBalancedWebService_Template(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/deploy/cloudformation/stack/worker_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/worker_service_integration_test.go index 7d79807af34..6ececde1622 100644 --- a/internal/pkg/deploy/cloudformation/stack/worker_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/worker_service_integration_test.go @@ -58,7 +58,7 @@ func TestWorkerService_Template(t *testing.T) { ws, err := workspace.Use(fs) require.NoError(t, err) - _, err = addon.Parse(aws.StringValue(v.Name), ws) + _, err = addon.ParseFromWorkload(aws.StringValue(v.Name), ws) var notFound *addon.ErrAddonsNotFound require.ErrorAs(t, err, ¬Found) diff --git a/internal/pkg/template/artifactpath/artifactpath.go b/internal/pkg/template/artifactpath/artifactpath.go index 70e26a0148c..39d702d4b3a 100644 --- a/internal/pkg/template/artifactpath/artifactpath.go +++ b/internal/pkg/template/artifactpath/artifactpath.go @@ -19,6 +19,7 @@ const ( s3ArtifactEnvFilesDirName = "env-files" s3ScriptsDirName = "scripts" s3CustomResourcesDirName = "custom-resources" + s3EnvironmentsAddonsDirName = "environments" ) // MkdirSHA256 prefixes the key with the SHA256 hash of the contents of "manual//key". @@ -38,6 +39,12 @@ func AddonAsset(workloadName, hash string) string { return path.Join(s3ArtifactDirName, s3ArtifactAddonsDirName, workloadName, s3ArtifactAddonAssetDirName, hash) } +// EnvironmentAddonAsset returns the path to store an addon asset file for an environment addon. +// Example: manual/addons/environments/assets/668e2b73ac. +func EnvironmentAddonAsset(hash string) string { + return path.Join(s3ArtifactDirName, s3ArtifactAddonsDirName, s3EnvironmentsAddonsDirName, s3ArtifactAddonAssetDirName, hash) +} + // CFNTemplate returns the path to store cloudformation templates with sha256 of the content. // Example: manual/templates/key/sha.yml. func CFNTemplate(key string, content []byte) string { diff --git a/internal/pkg/template/template_functions.go b/internal/pkg/template/template_functions.go index f9ae7454389..9c37ea6b566 100644 --- a/internal/pkg/template/template_functions.go +++ b/internal/pkg/template/template_functions.go @@ -104,11 +104,10 @@ func FmtSliceFunc(elems []string) string { // QuoteSliceFunc places quotation marks around all elements of a go string slice. func QuoteSliceFunc(elems []string) []string { - var quotedElems []string if len(elems) == 0 { - return quotedElems + return nil } - quotedElems = make([]string, len(elems)) + quotedElems := make([]string, len(elems)) for i, el := range elems { quotedElems[i] = strconv.Quote(el) }