Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for conditionally enabling of alpha-level CRDs #462

Merged
merged 30 commits into from
Aug 1, 2023
Merged
Changes from 17 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
407decd
Support for enabling/disabling of alpha-level CRDs
dmitri-d Jul 15, 2023
aba416a
Fixed crd template
dmitri-d Jul 18, 2023
ba8158d
Groups are matched with correct file descriptors now
dmitri-d Jul 18, 2023
804ea88
Added 'Grandfathered' field to Group model
dmitri-d Jul 20, 2023
322e66c
Merge remote-tracking branch 'origin/master' into dmitrid-crd-install
dmitri-d Jul 25, 2023
16c6794
Fixing test failures
dmitri-d Jul 26, 2023
fd01b15
Tests are passing
dmitri-d Jul 26, 2023
ff3b4c3
Added more tests
dmitri-d Jul 27, 2023
2ad44af
Test fixes
dmitri-d Jul 27, 2023
b947347
Added a changelog entry
dmitri-d Jul 27, 2023
08dfd42
Respect 'RenderManifests' Group field
dmitri-d Jul 27, 2023
e6a958c
Moved changelog to v0.31.3
dmitri-d Jul 27, 2023
77ee7d2
Regenerated code
dmitri-d Jul 27, 2023
13d41d9
De-flaked a test
dmitri-d Jul 27, 2023
ce5b6e4
Added support for skipping conditional rendering of CRDs for grandfat…
dmitri-d Jul 28, 2023
8ff87ad
De-flaked a test
dmitri-d Jul 28, 2023
acf693e
Made ordering of crds in a group deterministic
dmitri-d Jul 28, 2023
697c2af
Responded to feedback
dmitri-d Jul 28, 2023
49c4f85
Fixed an issue in template rendering
dmitri-d Jul 28, 2023
bf17709
Responded to feedback
dmitri-d Jul 28, 2023
7c87686
changed changelog entry to 'BREAKING_CHANGE'
dmitri-d Jul 28, 2023
774e479
cleaned up crd.CustomResourceDefinitions
dmitri-d Jul 28, 2023
458d999
Made configurable the name for the flag that contains the list of ena…
dmitri-d Jul 28, 2023
9892a1a
Added validation that CRD templates can be rendered
dmitri-d Jul 29, 2023
628ec34
Fixed an issue with crd template rendering
dmitri-d Jul 30, 2023
32b91d0
Adding changelog file to new location
Jul 31, 2023
9bc78c7
Deleting changelog file from old location
Jul 31, 2023
fb84047
Merge remote-tracking branch 'origin/master' into dmitrid-crd-install
dmitri-d Jul 31, 2023
c5ad04b
Updated version to v0.33.0
dmitri-d Jul 31, 2023
51e0190
Merge remote-tracking branch 'origin/dmitrid-crd-install' into dmitri…
dmitri-d Jul 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/multicluster/v1alpha1/group.go
Original file line number Diff line number Diff line change
@@ -26,6 +26,7 @@ var Group = model.Group{
Name: "KubernetesClusterStatus",
},
},
Stored: true,
},
},
RenderManifests: true,
5 changes: 5 additions & 0 deletions changelog/v0.31.3/conditional_crd_rendering.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
changelog:
- type: NON_USER_FACING
dmitri-d marked this conversation as resolved.
Show resolved Hide resolved
issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/9019
description: >
Add support for conditional rendering of CRDs.
58 changes: 34 additions & 24 deletions codegen/cmd.go
Original file line number Diff line number Diff line change
@@ -144,17 +144,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.generateGroup(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
}

@@ -240,7 +239,7 @@ func (c Command) renderProtos() ([]*collector.DescriptorWithPath, error) {
}

func (c Command) generateGroup(
dmitri-d marked this conversation as resolved.
Show resolved Hide resolved
grp model.Group,
grps []*model.Group,
protoOpts proto.Options,
groupOptions model.GroupOptions,
) error {
@@ -250,25 +249,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, protoOpts, groupOptions, grps)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we sohuld just move RenderManifests out rather than changing the whole function signature for this one line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly sure what you mean. RenderManifests needs all groups now, as it combines multiple versions of the same group into one manifest (and multiple versions of the same resource into a single crd).

if err != nil {
return err
}
@@ -277,8 +278,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
@@ -335,6 +338,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
30 changes: 26 additions & 4 deletions codegen/cmd_test.go
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import (
"os/exec"
"path/filepath"
"reflect"
"strings"

goyaml "gopkg.in/yaml.v3"
rbacv1 "k8s.io/api/rbac/v1"
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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())
@@ -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())
@@ -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

@@ -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
}
Loading