Skip to content
This repository was archived by the owner on Mar 24, 2023. It is now read-only.

Commit 57fe3a3

Browse files
dexhorthydexhorthy
authored
Merge pull request #827 from dexhorthy/nonnullplanner-renderroot
GitHub assets should respect render root (fixes #814)
2 parents b9fc82c + 7560ad9 commit 57fe3a3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+116
-225
lines changed

‎integration/init/github-no-proxy-gogetter/metadata.yaml

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
upstream: "input/ship.yaml"
22
make_absolute: true
3-
args: ["--prefer-git"]
3+
args: ["--prefer-git"]
4+
skip_cleanup: false

‎integration/init/jaeger-cassandra/expected/.ship/state.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"lists": [
1111
{
1212
"apiVersion": "v1",
13-
"path": "installer/base/cassandra.yml",
13+
"path": "base/cassandra.yml",
1414
"items": [
1515
{
1616
"kind": "Service",
@@ -37,7 +37,7 @@
3737
},
3838
{
3939
"apiVersion": "v1",
40-
"path": "installer/base/jaeger-production-template.yml",
40+
"path": "base/jaeger-production-template.yml",
4141
"items": [
4242
{
4343
"kind": "Deployment",

‎integration/init/jaeger-cassandra/expected/installer/base/DaemonSet-jaeger-agent.yaml ‎integration/init/jaeger-cassandra/expected/base/DaemonSet-jaeger-agent.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/Deployment-jaeger-collector.yaml ‎integration/init/jaeger-cassandra/expected/base/Deployment-jaeger-collector.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/Deployment-jaeger-query.yaml ‎integration/init/jaeger-cassandra/expected/base/Deployment-jaeger-query.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/Job-jaeger-cassandra-schema-job.yaml ‎integration/init/jaeger-cassandra/expected/base/Job-jaeger-cassandra-schema-job.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/Service-cassandra.yaml ‎integration/init/jaeger-cassandra/expected/base/Service-cassandra.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/Service-jaeger-collector.yaml ‎integration/init/jaeger-cassandra/expected/base/Service-jaeger-collector.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/Service-jaeger-query.yaml ‎integration/init/jaeger-cassandra/expected/base/Service-jaeger-query.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/Service-zipkin.yaml ‎integration/init/jaeger-cassandra/expected/base/Service-zipkin.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/StatefulSet-cassandra.yaml ‎integration/init/jaeger-cassandra/expected/base/StatefulSet-cassandra.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/base/kustomization.yaml ‎integration/init/jaeger-cassandra/expected/base/kustomization.yaml

File renamed without changes.

‎integration/init/jaeger-cassandra/expected/installer/.gitkeep

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
workaround

‎integration/init/jaeger-cassandra/expected/overlays/ship/kustomization.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
kind: ""
22
apiversion: ""
33
bases:
4-
- ../../installer/base
4+
- ../../base

‎integration/init/jaeger-cassandra/input/ship.yaml

+5-1
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,14 @@ assets:
1313
path: jaeger-production-template.yml
1414
proxy: false
1515
strip_path: true
16+
- inline:
17+
dest: installer/.gitkeep
18+
contents:
19+
workaround
1620
lifecycle:
1721
v1:
1822
- render:
1923
root: .
2024
- kustomize:
21-
base: installer/base
25+
base: base
2226
dest: rendered.yaml

‎integration/init/kustomize-overlay/input/ship.yaml

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
11
assets:
22
v1:
33
- github:
4-
dest: ../base/common
4+
dest: base/common
55
repo: kubernetes/ingress-nginx
66
path: deploy/mandatory.yaml
77
ref: nginx-0.22.0
88
proxy: false
99
strip_path: true
1010

1111
- github:
12-
dest: ../base
12+
dest: base
1313
repo: kubernetes/ingress-nginx
1414
path: deploy/provider/aws/service-l4.yaml
1515
ref: nginx-0.22.0
1616
proxy: false
1717
strip_path: true
1818

1919
- github:
20-
dest: ../overlays/cloud
20+
dest: overlays/cloud
2121
repo: kubernetes/ingress-nginx
2222
path: deploy/provider/aws/patch-configmap-l4.yaml
2323
ref: nginx-0.22.0

‎pkg/lifecycle/kustomize/patch_test.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,11 @@ import (
44
"path"
55
"testing"
66

7+
"github.com/ghodss/yaml"
8+
"github.com/go-kit/kit/log"
79
"github.com/replicatedhq/ship/pkg/api"
810
"github.com/replicatedhq/ship/pkg/constants"
911
"github.com/replicatedhq/ship/pkg/state"
10-
11-
"github.com/ghodss/yaml"
12-
"github.com/go-kit/kit/log"
1312
"github.com/spf13/afero"
1413
"github.com/spf13/viper"
1514
"github.com/stretchr/testify/require"

‎pkg/lifecycle/kustomize/post_kustomize.go

-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package kustomize
33
import (
44
"github.com/go-kit/kit/log"
55
"github.com/go-kit/kit/log/level"
6-
76
"github.com/replicatedhq/ship/pkg/api"
87
"github.com/replicatedhq/ship/pkg/util"
98
)

‎pkg/lifecycle/render/backup.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ import (
44
"github.com/replicatedhq/ship/pkg/util"
55
)
66

7-
func (r *renderer) backupIfPresent(basePath string) error {
7+
func (r *headlessrenderer) backupIfPresent(basePath string) error {
88
return util.BackupIfPresent(r.Fs, basePath, r.Logger, r.UI)
99
}

‎pkg/lifecycle/render/factory.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,21 @@ import (
1515
"github.com/spf13/afero"
1616
)
1717

18-
// Factory gets a *renderer and implements lifecycle.Renderer
19-
type Factory func() *renderer
18+
// Factory gets a *headlessrenderer and implements lifecycle.Renderer
19+
type Factory func() *headlessrenderer
2020

2121
// factory implements lifecycle.Renderer
22-
var _ lifecycle.Renderer = Factory(func() *renderer { return nil })
22+
var _ lifecycle.Renderer = Factory(func() *headlessrenderer { return nil })
2323

2424
func (f Factory) Execute(ctx context.Context, release *api.Release, step *api.Render) error {
2525
r := f()
2626
return r.Execute(ctx, release, step)
2727
}
2828

2929
func (f Factory) WithPlanner(plannerFactory pkgplanner.Planner) lifecycle.Renderer {
30-
return Factory(func() *renderer {
30+
return Factory(func() *headlessrenderer {
3131
r := f()
32-
return &renderer{
32+
return &headlessrenderer{
3333
Logger: r.Logger,
3434
ConfigResolver: r.ConfigResolver,
3535
Planner: plannerFactory,
@@ -43,9 +43,9 @@ func (f Factory) WithPlanner(plannerFactory pkgplanner.Planner) lifecycle.Render
4343
}
4444

4545
func (f Factory) WithStatusReceiver(receiver daemontypes.StatusReceiver) lifecycle.Renderer {
46-
return Factory(func() *renderer {
46+
return Factory(func() *headlessrenderer {
4747
r := f()
48-
return &renderer{
48+
return &headlessrenderer{
4949
Logger: r.Logger,
5050
ConfigResolver: r.ConfigResolver,
5151
Planner: r.Planner,
@@ -67,8 +67,8 @@ func NewFactory(
6767
resolver config.Resolver,
6868
status daemontypes.StatusReceiver,
6969
) lifecycle.Renderer {
70-
return Factory(func() *renderer {
71-
return &renderer{
70+
return Factory(func() *headlessrenderer {
71+
return &headlessrenderer{
7272
Logger: logger,
7373
ConfigResolver: resolver,
7474
Planner: planner,

‎pkg/lifecycle/render/github/render.go

+11-5
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ type Renderer interface {
3131
rootFs root.Fs,
3232
asset api.GitHubAsset,
3333
configGroups []libyaml.ConfigGroup,
34+
renderRoot string,
3435
meta api.ReleaseMetadata,
3536
templateContext map[string]interface{},
3637
) func(ctx context.Context) error
@@ -69,6 +70,7 @@ func (r *LocalRenderer) Execute(
6970
rootFs root.Fs,
7071
asset api.GitHubAsset,
7172
configGroups []libyaml.ConfigGroup,
73+
renderRoot string,
7274
meta api.ReleaseMetadata,
7375
templateContext map[string]interface{},
7476
) func(ctx context.Context) error {
@@ -96,7 +98,7 @@ func (r *LocalRenderer) Execute(
9698

9799
if asset.Source == "public" || !asset.Proxy {
98100
debug.Log("event", "resolveNoProxyGithubAssets")
99-
err := r.resolveNoProxyGithubAssets(asset, builder)
101+
err := r.resolveNoProxyGithubAssets(asset, builder, renderRoot)
100102
if err != nil {
101103
return errors.Wrap(err, "resolveNoProxyGithubAssets")
102104
}
@@ -175,7 +177,7 @@ func (r *LocalRenderer) resolveProxyGithubAssets(asset api.GitHubAsset, builder
175177
return nil
176178
}
177179

178-
func (r *LocalRenderer) resolveNoProxyGithubAssets(asset api.GitHubAsset, builder *templates.Builder) error {
180+
func (r *LocalRenderer) resolveNoProxyGithubAssets(asset api.GitHubAsset, builder *templates.Builder, renderRoot string) error {
179181
debug := level.Debug(log.With(r.Logger, "step.type", "render", "render.phase", "execute", "asset.type", "github", "dest", asset.Dest, "description", asset.Description))
180182
debug.Log("event", "createUpstream")
181183
upstream, err := createUpstreamURL(asset, builder)
@@ -200,7 +202,7 @@ func (r *LocalRenderer) resolveNoProxyGithubAssets(asset api.GitHubAsset, builde
200202
}
201203

202204
debug.Log("event", "getDestPath")
203-
dest, err := getDestPathNoProxy(asset, builder)
205+
dest, err := r.getDestPathNoProxy(asset, builder, renderRoot)
204206
if err != nil {
205207
return errors.Wrap(err, "get dest path")
206208
}
@@ -259,7 +261,9 @@ func getDestPath(githubPath string, asset api.GitHubAsset, builder *templates.Bu
259261
return filepath.Join(destDir, githubPath), nil
260262
}
261263

262-
func getDestPathNoProxy(asset api.GitHubAsset, builder *templates.Builder) (string, error) {
264+
func (r *LocalRenderer) getDestPathNoProxy(asset api.GitHubAsset, builder *templates.Builder, renderRoot string) (string, error) {
265+
266+
debug := level.Debug(log.With(r.Logger, "method", "getdestPathNoProxy", "asset.type", "github", "dest", asset.Dest, "renderRoot", renderRoot, "proxy", false))
263267
assetPath := asset.Path
264268
stripPath, err := builder.Bool(asset.StripPath, false)
265269
if err != nil {
@@ -279,7 +283,9 @@ func getDestPathNoProxy(asset api.GitHubAsset, builder *templates.Builder) (stri
279283
}
280284
}
281285

282-
return filepath.Join(constants.InstallerPrefixPath, destDir, assetPath), nil
286+
destPath := filepath.Join(renderRoot, destDir, assetPath)
287+
debug.Log("event", "destPath.resolve", "path", destPath)
288+
return destPath, nil
283289
}
284290

285291
func createUpstreamURL(asset api.GitHubAsset, builder *templates.Builder) (string, error) {

‎pkg/lifecycle/render/github/render_test.go

+36-5
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/replicatedhq/libyaml"
99
"github.com/replicatedhq/ship/pkg/api"
10+
"github.com/replicatedhq/ship/pkg/constants"
1011
"github.com/replicatedhq/ship/pkg/templates"
1112
"github.com/replicatedhq/ship/pkg/test-mocks/state"
1213
"github.com/replicatedhq/ship/pkg/testing/logger"
@@ -231,10 +232,11 @@ func Test_getDestPath(t *testing.T) {
231232

232233
func Test_getDestPathNoProxy(t *testing.T) {
233234
tests := []struct {
234-
name string
235-
asset api.GitHubAsset
236-
want string
237-
wantErr bool
235+
name string
236+
asset api.GitHubAsset
237+
want string
238+
wantErr bool
239+
renderRoot string
238240
}{
239241
{
240242
name: "basic file",
@@ -248,6 +250,32 @@ func Test_getDestPathNoProxy(t *testing.T) {
248250
want: "installer/README.md",
249251
wantErr: false,
250252
},
253+
{
254+
name: "basic file, no installer prefix",
255+
renderRoot: "./",
256+
asset: api.GitHubAsset{
257+
Path: "README.md",
258+
StripPath: "",
259+
AssetShared: api.AssetShared{
260+
Dest: "./",
261+
},
262+
},
263+
want: "README.md",
264+
wantErr: false,
265+
},
266+
{
267+
name: "basic file, customer installer prefix",
268+
renderRoot: "my-installer",
269+
asset: api.GitHubAsset{
270+
Path: "README.md",
271+
StripPath: "",
272+
AssetShared: api.AssetShared{
273+
Dest: "./",
274+
},
275+
},
276+
want: "my-installer/README.md",
277+
wantErr: false,
278+
},
251279
{
252280
name: "file in subdir",
253281
asset: api.GitHubAsset{
@@ -379,7 +407,10 @@ func Test_getDestPathNoProxy(t *testing.T) {
379407
builder, err := bb.FullBuilder(api.ReleaseMetadata{}, []libyaml.ConfigGroup{}, map[string]interface{}{})
380408
req.NoError(err)
381409

382-
got, err := getDestPathNoProxy(tt.asset, builder)
410+
if tt.renderRoot == "" {
411+
tt.renderRoot = constants.InstallerPrefixPath
412+
}
413+
got, err := (&LocalRenderer{Logger: testLogger}).getDestPathNoProxy(tt.asset, builder, tt.renderRoot)
383414
if !tt.wantErr {
384415
req.NoError(err)
385416
} else {

‎pkg/lifecycle/render/helm/fetch.go

+3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ type ChartFetcher interface {
2222
FetchChart(
2323
ctx context.Context,
2424
asset api.HelmAsset,
25+
renderRoot string,
2526
meta api.ReleaseMetadata,
2627
configGroups []libyaml.ConfigGroup,
2728
templateContext map[string]interface{},
@@ -39,6 +40,7 @@ type ClientFetcher struct {
3940
func (f *ClientFetcher) FetchChart(
4041
ctx context.Context,
4142
asset api.HelmAsset,
43+
renderRoot string,
4244
meta api.ReleaseMetadata,
4345
configGroups []libyaml.ConfigGroup,
4446
templateContext map[string]interface{},
@@ -59,6 +61,7 @@ func (f *ClientFetcher) FetchChart(
5961
root.NewRootFS(f.FS, "."),
6062
*asset.GitHub,
6163
configGroups,
64+
renderRoot,
6265
meta,
6366
templateContext,
6467
)(ctx)

‎pkg/lifecycle/render/helm/fetch_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ func TestFetch(t *testing.T) {
7979
},
8080
},
8181
[]libyaml.ConfigGroup{},
82+
"",
8283
api.ReleaseMetadata{},
8384
map[string]interface{}{},
8485
).Return(func(ctx context.Context) error { return nil })
@@ -121,6 +122,7 @@ func TestFetch(t *testing.T) {
121122
},
122123
},
123124
[]libyaml.ConfigGroup{},
125+
"installer/",
124126
api.ReleaseMetadata{},
125127
map[string]interface{}{},
126128
).Return(func(ctx context.Context) error { return nil })
@@ -149,6 +151,7 @@ func TestFetch(t *testing.T) {
149151
dest, err := fetcher.FetchChart(
150152
context.Background(),
151153
test.asset,
154+
test.renderRoot,
152155
api.ReleaseMetadata{},
153156
[]libyaml.ConfigGroup{},
154157
map[string]interface{}{},

‎pkg/lifecycle/render/helm/render.go

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ func (r *LocalRenderer) Execute(
5151
chartLocation, err := r.Fetcher.FetchChart(
5252
ctx,
5353
asset,
54+
rootFs.RootPath,
5455
meta,
5556
configGroups,
5657
templateContext,

‎pkg/lifecycle/render/helm/render_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestRender(t *testing.T) {
6767
ctx := context.Background()
6868

6969
mockFetcher.EXPECT().
70-
FetchChart(ctx, asset, metadata, configGroups, templateContext).
70+
FetchChart(ctx, asset, "", metadata, configGroups, templateContext).
7171
Return(test.fetchPath, test.fetchErr)
7272

7373
if test.fetchErr == nil {

‎pkg/lifecycle/render/noconfig.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ func (r *noconfigrenderer) Execute(ctx context.Context, release *api.Release, st
5454
templateContext := previousState.CurrentConfig()
5555
r.StatusReceiver.SetProgress(ProgressRender)
5656

57+
// this should probably happen even higher up, like to the validation stage where we assign IDs to lifecycle steps, but moving it up here for now
58+
if step.Root == "" {
59+
step.Root = constants.InstallerPrefixPath
60+
}
61+
5762
debug.Log("event", "render.plan")
5863
pln, err := r.Planner.Build(step.Root, release.Spec.Assets.V1, release.Spec.Config.V1, release.Metadata, templateContext)
5964
if err != nil {
@@ -62,10 +67,6 @@ func (r *noconfigrenderer) Execute(ctx context.Context, release *api.Release, st
6267

6368
debug.Log("event", "backup.start")
6469

65-
if step.Root == "" {
66-
step.Root = constants.InstallerPrefixPath
67-
}
68-
6970
if step.Root != "." && step.Root != "./" {
7071
if r.Viper.GetBool("rm-asset-dest") {
7172
err := r.Fs.RemoveAll(step.Root)

‎pkg/lifecycle/render/planner/build.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func (p *CLIPlanner) Build(renderRoot string, assets []api.Asset, configGroups [
108108

109109
p.logAssetResolve(debug, evaluatedWhen, "github")
110110
if evaluatedWhen {
111-
plan = append(plan, p.githubStep(rootFs, *asset.GitHub, configGroups, meta, templateContext))
111+
plan = append(plan, p.githubStep(rootFs, *asset.GitHub, configGroups, renderRoot, meta, templateContext))
112112
}
113113
} else if asset.Terraform != nil {
114114
evaluatedWhen, err := p.evalAssetWhen(debug, *builder, asset, asset.Terraform.AssetShared.When)

‎pkg/lifecycle/render/planner/planner.go

+1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func (f Factory) WithStatusReceiver(receiver daemontypes.StatusReceiver) Planner
6767

6868
Inline: planner.Inline,
6969
Helm: planner.Helm,
70+
Local: planner.Local,
7071
Docker: planner.Docker,
7172
DockerLayer: planner.DockerLayer,
7273
GitHub: planner.GitHub,

0 commit comments

Comments
 (0)