Skip to content

Commit

Permalink
Support for conditionally enabling of alpha-level CRDs (#462)
Browse files Browse the repository at this point in the history 
* Support for enabling/disabling of alpha-level CRDs

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Fixed crd template

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Groups are matched with correct file descriptors now

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Added 'Grandfathered' field to Group model

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Fixing test failures

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Tests are passing

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Added more tests

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Test fixes

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Added a changelog entry

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Respect 'RenderManifests' Group field

* Moved changelog to v0.31.3

* Regenerated code

Signed-off-by: Dmitri Dolguikh <[email protected]>

* De-flaked a test

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Added support for skipping conditional rendering of CRDs for grandfathered groups

Signed-off-by: Dmitri Dolguikh <[email protected]>

* De-flaked a test

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Made ordering of crds in a group deterministic

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Responded to feedback

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Fixed an issue in template rendering

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Responded to feedback

Signed-off-by: Dmitri Dolguikh <[email protected]>

* changed changelog entry to 'BREAKING_CHANGE'

Signed-off-by: Dmitri Dolguikh <[email protected]>

* cleaned up crd.CustomResourceDefinitions

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Made configurable the name for the flag that contains the list of enabled alpha API

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Added validation that CRD templates can be rendered

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Fixed an issue with crd template rendering

Signed-off-by: Dmitri Dolguikh <[email protected]>

* Adding changelog file to new location

* Deleting changelog file from old location

* Updated version to v0.33.0

Signed-off-by: Dmitri Dolguikh <[email protected]>

---------

Signed-off-by: Dmitri Dolguikh <[email protected]>
Co-authored-by: changelog-bot <changelog-bot>
  • Loading branch information
@dmitri-d
dmitri-d authored Aug 1, 2023
1 parent 0d5c2a5 commit ac641d9
Show file tree
Hide file tree
Showing 22 changed files with 1,518 additions and 223 deletions.
18 changes: 10 additions & 8 deletions api/multicluster/v1alpha1/group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,16 @@ var Group = model.Group{
Name: "KubernetesClusterStatus",
},
},
Stored: true,
},
},
RenderManifests: true,
RenderValidationSchemas: true,
RenderController: true,
RenderClients: true,
RenderTypes: true,
MockgenDirective: true,
ApiRoot: "pkg/api",
CustomTemplates: contrib.AllGroupCustomTemplates,
RenderManifests: true,
RenderValidationSchemas: true,
RenderController: true,
RenderClients: true,
RenderTypes: true,
MockgenDirective: true,
ApiRoot: "pkg/api",
CustomTemplates: contrib.AllGroupCustomTemplates,
SkipConditionalCRDLoading: true,
}
5 changes: 5 additions & 0 deletions changelog/v0.33.0/conditional_crd_rendering.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
changelog:
- type: BREAKING_CHANGE
issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/9019
description: >
Add support for conditional rendering of CRDs.
64 changes: 39 additions & 25 deletions codegen/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ type Command struct {

// context of the command
ctx context.Context

// the name of the flag to pass the list of enabled alpha-level crds
// used in codegen/templates/manifests/crd.yamltmpl
EnabledAlphaApiFlagName string
}

// function to execute skv2 code gen from another repository
Expand Down Expand Up @@ -144,17 +148,16 @@ func (c Command) Execute() error {
var groups []*model.Group
for _, group := range c.Groups {
group := group // pike
c.initGroup(&group, descriptors)
groups = append(groups, &group)
}
for i, group := range groups {
group := group // pike
c.initGroup(group, descriptors)

if err := c.generateGroup(*group, protoOpts, c.GroupOptions); err != nil {
return err
}
if err := c.generateGroups(groups, protoOpts, c.GroupOptions); err != nil {
return err
}

// replace group in Groups array with the group including generated fields
// replace group in Groups array with the group including generated fields
for i, group := range groups {
c.Groups[i] = *group
}

Expand Down Expand Up @@ -239,8 +242,8 @@ func (c Command) renderProtos() ([]*collector.DescriptorWithPath, error) {
return descriptors, nil
}

func (c Command) generateGroup(
grp model.Group,
func (c Command) generateGroups(
grps []*model.Group,
protoOpts proto.Options,
groupOptions model.GroupOptions,
) error {
Expand All @@ -250,25 +253,27 @@ func (c Command) generateGroup(
Header: c.GeneratedHeader,
}

protoTypes, err := render.RenderProtoTypes(grp)
if err != nil {
return err
}
for _, grp := range grps {
protoTypes, err := render.RenderProtoTypes(*grp)
if err != nil {
return err
}

if err := fileWriter.WriteFiles(protoTypes); err != nil {
return err
}
if err := fileWriter.WriteFiles(protoTypes); err != nil {
return err
}

apiTypes, err := render.RenderApiTypes(grp)
if err != nil {
return err
}
apiTypes, err := render.RenderApiTypes(*grp)
if err != nil {
return err
}

if err := fileWriter.WriteFiles(apiTypes); err != nil {
return err
if err := fileWriter.WriteFiles(apiTypes); err != nil {
return err
}
}

manifests, err := render.RenderManifests(c.AppName, c.ManifestRoot, c.ProtoDir, protoOpts, groupOptions, grp)
manifests, err := render.RenderManifests(c.AppName, c.ManifestRoot, c.ProtoDir, c.EnabledAlphaApiFlagName, protoOpts, groupOptions, grps)
if err != nil {
return err
}
Expand All @@ -277,8 +282,10 @@ func (c Command) generateGroup(
return err
}

if err := render.KubeCodegen(grp); err != nil {
return err
for _, grp := range grps {
if err := render.KubeCodegen(*grp); err != nil {
return err
}
}

return nil
Expand Down Expand Up @@ -335,6 +342,13 @@ func (c Command) initGroup(
// default to the resource API Group name
matchingProtoPackage = resource.Group.Group
}
// TODO (dmitri-d): This assumes we only ever have a single message with a given name, and breaks when we
// have multiple versions of the same proto message.
// `go_package`` is not a reliable way to determine the package version, as it is not always set.
// protobuf path is not reliable either, as it is not always contains the version (see test/test_api.proto).
// A common approach is to include the version in the proto package name, perhaps we could adopt that?
// For example, workspace.proto package now is "admin.gloo.solo.io", it would change to
// "admin.gloo.solo.io.v2" with such an approach.
if fileDescriptor.GetPackage() == matchingProtoPackage {
if message := fileDescriptor.GetMessage(fieldType.Name); message != nil {
fieldType.Message = message
Expand Down
30 changes: 26 additions & 4 deletions codegen/cmd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os/exec"
"path/filepath"
"reflect"
"strings"

goyaml "gopkg.in/yaml.v3"
rbacv1 "k8s.io/api/rbac/v1"
Expand Down Expand Up @@ -54,11 +55,13 @@ var _ = Describe("Cmd", func() {
Kind: "Paint",
Spec: Field{Type: Type{Name: "PaintSpec"}},
Status: &Field{Type: Type{Name: "PaintStatus"}},
Stored: true,
},
{
Kind: "ClusterResource",
Spec: Field{Type: Type{Name: "ClusterResourceSpec"}},
ClusterScoped: true,
Stored: true,
},
},
RenderManifests: true,
Expand Down Expand Up @@ -133,6 +136,7 @@ var _ = Describe("Cmd", func() {
Name: "KubernetesCluster",
GoPackage: "github.com/solo-io/skv2/pkg/api/multicluster.solo.io/v1alpha1",
}},
Stored: true,
},
},
RenderManifests: true,
Expand Down Expand Up @@ -171,11 +175,13 @@ var _ = Describe("Cmd", func() {
Kind: "Paint",
Spec: Field{Type: Type{Name: "PaintSpec"}},
Status: &Field{Type: Type{Name: "PaintStatus"}},
Stored: true,
},
{
Kind: "ClusterResource",
Spec: Field{Type: Type{Name: "ClusterResourceSpec"}},
ClusterScoped: true,
Stored: true,
},
},
RenderManifests: true,
Expand Down Expand Up @@ -256,11 +262,13 @@ var _ = Describe("Cmd", func() {
Kind: "Paint",
Spec: Field{Type: Type{Name: "PaintSpec"}},
Status: &Field{Type: Type{Name: "PaintStatus"}},
Stored: true,
},
{
Kind: "ClusterResource",
Spec: Field{Type: Type{Name: "ClusterResourceSpec"}},
ClusterScoped: true,
Stored: true,
},
},
RenderManifests: true,
Expand Down Expand Up @@ -375,11 +383,13 @@ var _ = Describe("Cmd", func() {
Kind: "Paint",
Spec: Field{Type: Type{Name: "PaintSpec"}},
Status: &Field{Type: Type{Name: "PaintStatus"}},
Stored: true,
},
{
Kind: "ClusterResource",
Spec: Field{Type: Type{Name: "ClusterResourceSpec"}},
ClusterScoped: true,
Stored: true,
},
},
RenderManifests: true,
Expand Down Expand Up @@ -1195,11 +1205,13 @@ var _ = Describe("Cmd", func() {
Kind: "Paint",
Spec: Field{Type: Type{Name: "PaintSpec"}},
Status: &Field{Type: Type{Name: "PaintStatus"}},
Stored: true,
},
{
Kind: "ClusterResource",
Spec: Field{Type: Type{Name: "ClusterResourceSpec"}},
ClusterScoped: true,
Stored: true,
},
},
RenderManifests: true,
Expand Down Expand Up @@ -1315,7 +1327,7 @@ var _ = Describe("Cmd", func() {
})

It("can include field descriptions", func() {
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_v1_crds.yaml")
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_crds.yaml")

err := cmd.Execute()
Expect(err).NotTo(HaveOccurred())
Expand All @@ -1325,16 +1337,25 @@ var _ = Describe("Cmd", func() {
Expect(string(bytes)).To(ContainSubstring("description: OpenAPI gen test for recursive fields"))
})

// TODO (dmitri-d): kube_crud_test and kube_multicluster_test depend on crds in this suite.
It("generates google.protobuf.Value with no type", func() {
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_v1_crds.yaml")
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_crds.yaml")

err := cmd.Execute()
Expect(err).NotTo(HaveOccurred())

bytes, err := ioutil.ReadFile(crdFilePath)
Expect(err).NotTo(HaveOccurred())
paintCrdYaml := ""
for _, crd := range strings.Split(string(bytes), "---") {
if strings.Contains(crd, "kind: Paint") {
paintCrdYaml = crd
}
}
Expect(paintCrdYaml).ToNot(BeEmpty())

generatedCrd := &v12.CustomResourceDefinition{}
Expect(yaml.Unmarshal(bytes, generatedCrd)).NotTo(HaveOccurred())
Expect(yaml.Unmarshal([]byte(paintCrdYaml), &generatedCrd)).NotTo(HaveOccurred())
protobufValueField := generatedCrd.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["spec"].Properties["recursiveType"].Properties["protobufValue"]
// access the field to make sure it's not nil
Expect(protobufValueField.XPreserveUnknownFields).ToNot(BeNil())
Expand All @@ -1346,7 +1367,7 @@ var _ = Describe("Cmd", func() {
// write this manifest to a different dir to avoid modifying the crd file from the
// above test, which other tests seem to depend on
cmd.ManifestRoot = "codegen/test/chart-no-desc"
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_v1_crds.yaml")
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_crds.yaml")

cmd.Groups[0].SkipSchemaDescriptions = true

Expand Down Expand Up @@ -1514,6 +1535,7 @@ func helmTemplate(path string, values interface{}) []byte {
path,
"--values", helmValuesFile.Name(),
).CombinedOutput()

ExpectWithOffset(1, err).NotTo(HaveOccurred(), string(out))
return out
}
Expand Down
Loading

0 comments on commit ac641d9

Please sign in to comment.