Skip to content

Commit

Permalink
Respect SkipCRDManifest and SkipTemplatedCRDManifest flags (#515)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jenshu
jenshu authored Nov 20, 2023
1 parent 4c86f64 commit 75d0d1f
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 8 deletions.
4 changes: 4 additions & 0 deletions changelog/v0.34.10/fix-skip-template.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/skv2/issues/514
description: Respect the SkipCRDManifest and SkipTemplatedCRDManifest flags on Groups.
13 changes: 8 additions & 5 deletions codegen/render/manifests_renderer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,22 @@ func (r ManifestsRenderer) RenderManifests(grps []*Group, protoOpts protoutil.Op
return nil, err
}

if !r.skipCrdsManifest {
// only render crd manifest if neither the options nor the group has the skip flag set to true
if !r.skipCrdsManifest && !shouldSkipCRDManifest[groupName] {
out, err := r.renderCRDManifest(r.AppName, groupName, crds)
if err != nil {
return nil, err
}
renderedFiles = append(renderedFiles, out)
}

out, err := r.renderTemplatedCRDManifest(r.AppName, groupName, crds, grandfatheredGroups)
if err != nil {
return nil, err
if !shouldSkipTemplatedCRDManifest[groupName] {
out, err := r.renderTemplatedCRDManifest(r.AppName, groupName, crds, grandfatheredGroups)
if err != nil {
return nil, err
}
renderedFiles = append(renderedFiles, out)
}
renderedFiles = append(renderedFiles, out)
}

return renderedFiles, nil
Expand Down
156 changes: 153 additions & 3 deletions codegen/render/manifests_renderer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,156 @@ var _ = Describe("ManifestsRenderer", func() {
Expect(obj).To(Equal(expectedObj))
})

Describe("should respect skip manifest flags", func() {
It("skips crd manifest if options.SkipCrdsManifest is true", func() {
grps := []*model.Group{{
GroupVersion: schema.GroupVersion{
Group: "things.test.io",
Version: "v1",
},
RenderManifests: true,
AddChartVersion: "1.0.0",
Resources: []model.Resource{
{
Kind: "kind",
Spec: model.Field{
Type: model.Type{
Name: "test",
Message: &v1.AcrylicType{},
},
},
Stored: false,
},
}},
}
for i := range grps {
grps[i].Init()
}

outFiles, err := render.RenderManifests(render.RenderOptions{
AppName: "appName",
ManifestRoot: "manifestDir",
ProtoDir: "protoDir",
Groups: grps,
SkipCrdsManifest: true,
})
Expect(err).NotTo(HaveOccurred())
Expect(outFiles).To(HaveLen(1)) // should only contain template manifest
Expect(outFiles[0].Path).To(Equal("manifestDir/templates/things.test.io_crds.yaml"))
})

It("skips crd manifest if group.SkipCRDManifest is true", func() {
grps := []*model.Group{{
SkipCRDManifest: true,
GroupVersion: schema.GroupVersion{
Group: "things.test.io",
Version: "v1",
},
RenderManifests: true,
AddChartVersion: "1.0.0",
Resources: []model.Resource{
{
Kind: "kind",
Spec: model.Field{
Type: model.Type{
Name: "test",
Message: &v1.AcrylicType{},
},
},
Stored: false,
},
}},
}
for i := range grps {
grps[i].Init()
}

outFiles, err := render.RenderManifests(render.RenderOptions{
AppName: "appName",
ManifestRoot: "manifestDir",
ProtoDir: "protoDir",
Groups: grps,
})
Expect(err).NotTo(HaveOccurred())
Expect(outFiles).To(HaveLen(1)) // should only contain template manifest
Expect(outFiles[0].Path).To(Equal("manifestDir/templates/things.test.io_crds.yaml"))
})

It("skips templated crd manifest if group.SkipTemplatedCRDManifest is true", func() {
grps := []*model.Group{{
SkipTemplatedCRDManifest: true,
GroupVersion: schema.GroupVersion{
Group: "things.test.io",
Version: "v1",
},
RenderManifests: true,
AddChartVersion: "1.0.0",
Resources: []model.Resource{
{
Kind: "kind",
Spec: model.Field{
Type: model.Type{
Name: "test",
Message: &v1.AcrylicType{},
},
},
Stored: false,
},
}},
}
for i := range grps {
grps[i].Init()
}

outFiles, err := render.RenderManifests(render.RenderOptions{
AppName: "appName",
ManifestRoot: "manifestDir",
ProtoDir: "protoDir",
Groups: grps,
})
Expect(err).NotTo(HaveOccurred())
Expect(outFiles).To(HaveLen(1)) // should only contain crd manifest
Expect(outFiles[0].Path).To(Equal("manifestDir/crds/things.test.io_crds.yaml"))
})

It("skips both manifests if both skip flags are true", func() {
grps := []*model.Group{{
SkipCRDManifest: true,
SkipTemplatedCRDManifest: true,
GroupVersion: schema.GroupVersion{
Group: "things.test.io",
Version: "v1",
},
RenderManifests: true,
AddChartVersion: "1.0.0",
Resources: []model.Resource{
{
Kind: "kind",
Spec: model.Field{
Type: model.Type{
Name: "test",
Message: &v1.AcrylicType{},
},
},
Stored: false,
},
}},
}
for i := range grps {
grps[i].Init()
}

outFiles, err := render.RenderManifests(render.RenderOptions{
AppName: "appName",
ManifestRoot: "manifestDir",
ProtoDir: "protoDir",
Groups: grps,
})
Expect(err).NotTo(HaveOccurred())
Expect(outFiles).To(HaveLen(0))
})
})

Describe("Generate non-alpha versioned CRD", func() {
var (
grps []*model.Group
Expand All @@ -81,7 +231,7 @@ var _ = Describe("ManifestsRenderer", func() {
grps[i].Init()
}
})
It("Renderse manifests with chart and spec hash", func() {
It("Renders manifests with chart and spec hash", func() {

// get api-level code gen options from descriptors
outFiles, err := render.RenderManifests(render.RenderOptions{
Expand Down Expand Up @@ -267,7 +417,7 @@ var _ = Describe("ManifestsRenderer", func() {
grps[i].Init()
}
})
It("Renderse manifests with chart and spec hash", func() {
It("Renders manifests with chart and spec hash", func() {

// get api-level code gen options from descriptors
outFiles, err := render.RenderManifests(render.RenderOptions{
Expand Down Expand Up @@ -356,7 +506,7 @@ var _ = Describe("ManifestsRenderer", func() {
grps[i].Init()
}
})
It("Renderse manifests with chart and spec hash", func() {
It("Renders manifests with chart and spec hash", func() {

// get api-level code gen options from descriptors
outFiles, err := render.RenderManifests(render.RenderOptions{
Expand Down

0 comments on commit 75d0d1f

Please sign in to comment.