Skip to content

Commit

Permalink
guard against empty rendered manifests
Browse files Browse the repository at this point in the history
Signed-off-by: Kent <[email protected]>
  • Loading branch information
krancour committed Sep 22, 2023
1 parent 82c6835 commit 1a94105
Show file tree
Hide file tree
Showing 8 changed files with 323 additions and 12 deletions.
4 changes: 4 additions & 0 deletions action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ inputs:
targetBranch:
description: The environment branch for which you want to render manifests
required: true
allowEmpty:
description: Whether to allow rendered manifests to be empty. If false this is disallowed as a safeguard against scenarios where a bug of any kind might otherwise cause Bookkeeper to wipe out the contents of the target branch in error.
required: false
default: 'false'
runs:
using: docker
image: docker://ghcr.io/akuity/bookkeeper:v0.1.0-rc.20
Expand Down
1 change: 1 addition & 0 deletions cmd/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
)

const (
flagAllowEmpty = "allow-empty"
flagCommitMessage = "commit-message"
flagDebug = "debug"
flagImage = "image"
Expand Down
11 changes: 11 additions & 0 deletions cmd/cli/render_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ func newRenderCommand() (*cobra.Command, error) {
"",
"the environment-specific branch to render manifests into (required)",
)
cmd.Flags().Bool(
flagAllowEmpty,
false,
"allow the rendered manifests to be empty; if false this is disallowed as "+
"a safeguard against scenarios where a bug of any kind might otherwise "+
"cause Bookkeeper to wipe out the contents of the target branch in error",
)
if err := cmd.MarkFlagRequired(flagTargetBranch); err != nil {
return nil, err
}
Expand Down Expand Up @@ -118,6 +125,10 @@ func runRenderCmd(cmd *cobra.Command, _ []string) error {
if err != nil {
return err
}
req.AllowEmpty, err = cmd.Flags().GetBool(flagAllowEmpty)
if err != nil {
return err
}

logLevel := bookkeeper.LogLevelError
var debug bool
Expand Down
32 changes: 22 additions & 10 deletions rendering.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import (
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"

"github.com/akuity/bookkeeper/internal/helm"
"github.com/akuity/bookkeeper/internal/kustomize"
"github.com/akuity/bookkeeper/internal/strings"
"github.com/akuity/bookkeeper/internal/ytt"
)

var lastMileKustomizationBytes = []byte(
Expand All @@ -24,9 +22,10 @@ resources:
`,
)

func preRender(
func (s *service) preRender(
ctx context.Context,
rc renderRequestContext,
repoDir string,
) (map[string][]byte, error) {
logger := rc.logger
manifests := map[string][]byte{}
Expand All @@ -49,12 +48,12 @@ func preRender(
"chartPath": chartPath,
"valuesPaths": valuesPaths,
})
chartPath = filepath.Join(rc.repo.WorkingDir(), chartPath)
chartPath = filepath.Join(repoDir, chartPath)
absValuesPaths := make([]string, len(valuesPaths))
for i, valuesPath := range valuesPaths {
absValuesPaths[i] = filepath.Join(rc.repo.WorkingDir(), valuesPath)
absValuesPaths[i] = filepath.Join(repoDir, valuesPath)
}
manifests[appName], err = helm.Render(
manifests[appName], err = s.helmRenderFn(
ctx,
appConfig.ConfigManagement.Helm.ReleaseName,
chartPath,
Expand All @@ -71,9 +70,9 @@ func preRender(
})
absPaths := make([]string, len(paths))
for i, path := range paths {
absPaths[i] = filepath.Join(rc.repo.WorkingDir(), path)
absPaths[i] = filepath.Join(repoDir, path)
}
manifests[appName], err = ytt.Render(ctx, absPaths)
manifests[appName], err = s.yttRenderFn(ctx, absPaths)
} else {
path := appConfig.ConfigManagement.Kustomize.Path
if path == "" {
Expand All @@ -83,8 +82,8 @@ func preRender(
"configManagement": "kustomize",
"path": path,
})
path = filepath.Join(rc.repo.WorkingDir(), path)
manifests[appName], err = kustomize.Render(
path = filepath.Join(repoDir, path)
manifests[appName], err = s.kustomizeRenderFn(
ctx,
path,
nil,
Expand All @@ -96,6 +95,19 @@ func preRender(
}
appLogger.Debug("completed manifest pre-rendering")
}

if !rc.request.AllowEmpty {
// This is a sanity check. Argo CD does this also.
for appName := range rc.target.branchConfig.AppConfigs {
if manifests, ok := manifests[appName]; !ok || len(manifests) == 0 {
return nil, errors.Errorf(
"pre-rendered manifests for app %q contain 0 bytes; this looks "+
"like a mistake and allowEmpty is not set; refusing to proceed",
appName,
)
}
}
}
return manifests, nil
}

Expand Down
244 changes: 244 additions & 0 deletions rendering_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
package bookkeeper

import (
"context"
"testing"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"

"github.com/akuity/bookkeeper/internal/helm"
"github.com/akuity/bookkeeper/internal/kustomize"
"github.com/akuity/bookkeeper/internal/ytt"
)

func TestPreRender(t *testing.T) {
const testAppName = "test-app"
fakeManifest := []byte("fake-manifest")
testCases := []struct {
name string
rc renderRequestContext
service *service
assertions func(manifests map[string][]byte, err error)
}{
{
name: "error pre-rendering with helm",
rc: renderRequestContext{
target: targetContext{
branchConfig: branchConfig{
AppConfigs: map[string]appConfig{
testAppName: {
ConfigManagement: configManagementConfig{
Helm: &helm.Config{},
},
},
},
},
},
},
service: &service{
helmRenderFn: func(
context.Context,
string,
string,
[]string,
) ([]byte, error) {
return nil, errors.New("something went wrong")
},
},
assertions: func(_ map[string][]byte, err error) {
require.Error(t, err)
require.Equal(t, "something went wrong", err.Error())
},
},
{
name: "success pre-rendering with helm",
rc: renderRequestContext{
target: targetContext{
branchConfig: branchConfig{
AppConfigs: map[string]appConfig{
testAppName: {
ConfigManagement: configManagementConfig{
Helm: &helm.Config{},
},
},
},
},
},
},
service: &service{
helmRenderFn: func(
context.Context,
string,
string,
[]string,
) ([]byte, error) {
return fakeManifest, nil
},
},
assertions: func(manifests map[string][]byte, err error) {
require.NoError(t, err)
require.Equal(t, fakeManifest, manifests[testAppName])
},
},
{
name: "error pre-rendering with ytt",
rc: renderRequestContext{
target: targetContext{
branchConfig: branchConfig{
AppConfigs: map[string]appConfig{
"test-app": {
ConfigManagement: configManagementConfig{
Ytt: &ytt.Config{},
},
},
},
},
},
},
service: &service{
yttRenderFn: func(context.Context, []string) ([]byte, error) {
return nil, errors.New("something went wrong")
},
},
assertions: func(_ map[string][]byte, err error) {
require.Error(t, err)
require.Equal(t, "something went wrong", err.Error())
},
},
{
name: "success pre-rendering with ytt",
rc: renderRequestContext{
target: targetContext{
branchConfig: branchConfig{
AppConfigs: map[string]appConfig{
"test-app": {
ConfigManagement: configManagementConfig{
Ytt: &ytt.Config{},
},
},
},
},
},
},
service: &service{
yttRenderFn: func(context.Context, []string) ([]byte, error) {
return fakeManifest, nil
},
},
assertions: func(manifests map[string][]byte, err error) {
require.NoError(t, err)
require.Equal(t, fakeManifest, manifests[testAppName])
},
},
{
name: "error pre-rendering with kustomize",
rc: renderRequestContext{
target: targetContext{
branchConfig: branchConfig{
AppConfigs: map[string]appConfig{
"test-app": {
ConfigManagement: configManagementConfig{
Kustomize: &kustomize.Config{},
},
},
},
},
},
},
service: &service{
kustomizeRenderFn: func(
context.Context,
string,
[]string,
bool,
) ([]byte, error) {
return nil, errors.New("something went wrong")
},
},
assertions: func(manifests map[string][]byte, err error) {
require.Error(t, err)
require.Equal(t, "something went wrong", err.Error())
},
},
{
name: "success pre-rendering with kustomize",
rc: renderRequestContext{
target: targetContext{
branchConfig: branchConfig{
AppConfigs: map[string]appConfig{
"test-app": {
ConfigManagement: configManagementConfig{
Kustomize: &kustomize.Config{},
},
},
},
},
},
},
service: &service{
kustomizeRenderFn: func(
context.Context,
string,
[]string,
bool,
) ([]byte, error) {
return fakeManifest, nil
},
},
assertions: func(manifests map[string][]byte, err error) {
require.NoError(t, err)
require.Equal(t, fakeManifest, manifests[testAppName])
},
},
{
name: "safeguards against empty manifests",
rc: renderRequestContext{
target: targetContext{
branchConfig: branchConfig{
AppConfigs: map[string]appConfig{
"test-app": {
ConfigManagement: configManagementConfig{
Kustomize: &kustomize.Config{},
},
},
},
},
},
},
service: &service{
kustomizeRenderFn: func(
context.Context,
string,
[]string,
bool,
) ([]byte, error) {
return []byte{}, nil // This is probably a mistake!
},
},
assertions: func(manifests map[string][]byte, err error) {
require.Error(t, err)
require.Contains(
t,
err.Error(),
"contain 0 bytes; this looks like a mistake",
)
},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
testCase.rc.logger = &logrus.Entry{
Logger: logrus.New(),
}
testCase.assertions(
testCase.service.preRender(
context.Background(),
testCase.rc,
"fake/repo/path",
),
)
})
}
}
Loading

0 comments on commit 1a94105

Please sign in to comment.