Skip to content

Commit

Permalink
Revert "Support for conditionally enabling of alpha-level CRDs (#462)"
Browse files Browse the repository at this point in the history
This reverts commit ac641d9.
  • Loading branch information
chunter0 committed Aug 7, 2023
1 parent 91f2377 commit 21353c8
Show file tree
Hide file tree
Showing 21 changed files with 223 additions and 1,509 deletions.
18 changes: 8 additions & 10 deletions api/multicluster/v1alpha1/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,14 @@ 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,
SkipConditionalCRDLoading: true,
RenderManifests: true,
RenderValidationSchemas: true,
RenderController: true,
RenderClients: true,
RenderTypes: true,
MockgenDirective: true,
ApiRoot: "pkg/api",
CustomTemplates: contrib.AllGroupCustomTemplates,
}
5 changes: 0 additions & 5 deletions changelog/v0.33.0/conditional_crd_rendering.yaml

This file was deleted.

64 changes: 25 additions & 39 deletions codegen/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ 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 @@ -148,16 +144,17 @@ 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.generateGroups(groups, protoOpts, c.GroupOptions); err != nil {
return err
}
if err := c.generateGroup(*group, protoOpts, c.GroupOptions); err != nil {
return err
}

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

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

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

for _, grp := range grps {
protoTypes, err := render.RenderProtoTypes(*grp)
if err != nil {
return err
}
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, c.EnabledAlphaApiFlagName, protoOpts, groupOptions, grps)
manifests, err := render.RenderManifests(c.AppName, c.ManifestRoot, c.ProtoDir, protoOpts, groupOptions, grp)
if err != nil {
return err
}
Expand All @@ -282,10 +277,8 @@ func (c Command) generateGroups(
return err
}

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

return nil
Expand Down Expand Up @@ -342,13 +335,6 @@ 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: 4 additions & 26 deletions codegen/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"os/exec"
"path/filepath"
"reflect"
"strings"

goyaml "gopkg.in/yaml.v3"
rbacv1 "k8s.io/api/rbac/v1"
Expand Down Expand Up @@ -166,13 +165,11 @@ 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 @@ -247,7 +244,6 @@ 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 @@ -286,13 +282,11 @@ 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 @@ -373,13 +367,11 @@ 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 @@ -494,13 +486,11 @@ 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 @@ -1507,13 +1497,11 @@ roleRef:
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 @@ -1629,7 +1617,7 @@ roleRef:
})

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

err := cmd.Execute()
Expect(err).NotTo(HaveOccurred())
Expand All @@ -1639,25 +1627,16 @@ roleRef:
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_crds.yaml")
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_v1_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([]byte(paintCrdYaml), &generatedCrd)).NotTo(HaveOccurred())
Expect(yaml.Unmarshal(bytes, 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 @@ -1669,7 +1648,7 @@ roleRef:
// 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_crds.yaml")
crdFilePath := filepath.Join(util.GetModuleRoot(), cmd.ManifestRoot, "/crds/things.test.io_v1_crds.yaml")

cmd.Groups[0].SkipSchemaDescriptions = true

Expand Down Expand Up @@ -1837,7 +1816,6 @@ 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 21353c8

Please sign in to comment.