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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Fix controller-tools doesn't support single files as input #864

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
108 changes: 108 additions & 0 deletions pkg/crd/gen_single_file_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
Copyright 2019 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package crd_test

import (
"bytes"
"os"
"path/filepath"

"github.com/google/go-cmp/cmp"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"sigs.k8s.io/controller-tools/pkg/crd"
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
"sigs.k8s.io/controller-tools/pkg/genall"
"sigs.k8s.io/controller-tools/pkg/loader"
"sigs.k8s.io/controller-tools/pkg/markers"
)

var _ = Describe("CRD Generation golang files", func() {
var (
ctx, ctx2 *genall.GenerationContext
out *outputRule

genDir = filepath.Join("testdata", "multiple_files")
)

BeforeEach(func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expect(err).NotTo(HaveOccurred())
Expect(os.Chdir(genDir)).To(Succeed()) // go modules are directory-sensitive
defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()

By("loading the roots")
pkgs, err := loader.LoadRoots("file_one.go")
Expect(err).NotTo(HaveOccurred())
Expect(pkgs).To(HaveLen(1))
Expect(pkgs[0].GoFiles).To(HaveLen(1))
pkgs2, err := loader.LoadRoots("file_two.go", "file_two_reference.go")
Expect(err).NotTo(HaveOccurred())
Expect(pkgs2).To(HaveLen(1))
Expect(pkgs2[0].GoFiles).To(HaveLen(2))

By("setup up the context")
reg := &markers.Registry{}
Expect(crdmarkers.Register(reg)).To(Succeed())
out = &outputRule{
buf: &bytes.Buffer{},
}
ctx = &genall.GenerationContext{
Collector: &markers.Collector{Registry: reg},
Roots: pkgs,
Checker: &loader.TypeChecker{},
OutputRule: out,
}
ctx2 = &genall.GenerationContext{
Collector: &markers.Collector{Registry: reg},
Roots: pkgs2,
Checker: &loader.TypeChecker{},
OutputRule: out,
}
})

It("should have deterministic output for single golang file", func() {
By("calling Generate on single golang file")
gen := &crd.Generator{
CRDVersions: []string{"v1"},
}
Expect(gen.Generate(ctx)).NotTo(HaveOccurred())

By("loading the desired YAML")
expectedFileOne, err := os.ReadFile(filepath.Join(genDir, "one.example.com.yaml"))
Expect(err).NotTo(HaveOccurred())
expectedFileOne = fixAnnotations(expectedFileOne)
expectedOut := string(expectedFileOne)
Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut))
})
It("should have deterministic output for multiple golang files referencing other types", func() {
By("calling Generate on two golang files")
gen := &crd.Generator{
CRDVersions: []string{"v1"},
}
Expect(gen.Generate(ctx2)).NotTo(HaveOccurred())

By("loading the desired YAML file")
expectedFileTwo, err := os.ReadFile(filepath.Join(genDir, "two.example.com.yaml"))
Expect(err).NotTo(HaveOccurred())
expectedFileTwo = fixAnnotations(expectedFileTwo)
expectedOut := string(expectedFileTwo)
Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut))
})
})
45 changes: 45 additions & 0 deletions pkg/crd/testdata/multiple_files/file_one.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// +groupName=testdata.kubebuilder.io
package multiplefiles

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
)

type OneResourceSpec struct {
Struct multiver.OuterStruct `json:"struct,omitempty"`
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:resource:singular=oneresource

type OneResource struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec OneResourceSpec `json:"spec"`
}

// +kubebuilder:object:root=true

type OneResourceList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []OneResource `json:"items"`
}
34 changes: 34 additions & 0 deletions pkg/crd/testdata/multiple_files/file_two.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// +groupName=testdata.kubebuilder.io
package multiplefiles

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:resource:singular=tworesource

type TwoResource struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec TwoResourceSpec `json:"spec"`
}

// +kubebuilder:object:root=true
31 changes: 31 additions & 0 deletions pkg/crd/testdata/multiple_files/file_two_reference.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package multiplefiles

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
)

type TwoResourceSpec struct {
Struct multiver.OuterStruct `json:"struct,omitempty"`
}

type TwoResourceList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []TwoResource `json:"items"`
}
51 changes: 51 additions & 0 deletions pkg/crd/testdata/multiple_files/one.example.com.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: (unknown)
name: oneresources.testdata.kubebuilder.io
spec:
group: testdata.kubebuilder.io
names:
kind: OneResource
listKind: OneResourceList
plural: oneresources
singular: oneresource
scope: Namespaced
versions:
- name: multiplefiles
schema:
openAPIV3Schema:
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
spec:
properties:
struct:
properties:
struct:
properties:
foo:
type: string
type: object
x-kubernetes-map-type: atomic
type: object
type: object
required:
- spec
type: object
served: true
storage: true
subresources:
status: {}
51 changes: 51 additions & 0 deletions pkg/crd/testdata/multiple_files/two.example.com.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: (unknown)
name: tworesources.testdata.kubebuilder.io
spec:
group: testdata.kubebuilder.io
names:
kind: TwoResource
listKind: TwoResourceList
plural: tworesources
singular: tworesource
scope: Namespaced
versions:
- name: multiplefiles
schema:
openAPIV3Schema:
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
spec:
properties:
struct:
properties:
struct:
properties:
foo:
type: string
type: object
x-kubernetes-map-type: atomic
type: object
type: object
required:
- spec
type: object
served: true
storage: true
subresources:
status: {}
20 changes: 6 additions & 14 deletions pkg/loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,12 +389,6 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
}
}()

// uniquePkgIDs is used to keep track of the discovered packages to be nice
// and try and prevent packages from showing up twice when nested module
// support is enabled. there is not harm that comes from this per se, but
// it makes testing easier when a known number of modules can be asserted
uniquePkgIDs := sets.Set[string]{}

// loadPackages returns the Go packages for the provided roots
//
// if validatePkgFn is nil, a package will be returned in the slice,
Expand All @@ -412,10 +406,7 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
var pkgs []*Package
for _, rp := range rawPkgs {
p := l.packageFor(rp)
if !uniquePkgIDs.Has(p.ID) {
pkgs = append(pkgs, p)
uniquePkgIDs.Insert(p.ID)
}
pkgs = append(pkgs, p)
}
return pkgs, nil
}
Expand Down Expand Up @@ -568,13 +559,14 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
for _, r := range fspRoots {
b, d := filepath.Base(r), filepath.Dir(r)

// we want the base part of the path to be either "..." or ".", except
// Go's filepath utilities clean paths during manipulation, removing the
// ".". thus, if not "...", let's update the path components so that:
// we want the base part of the path to be either "..." or ".", except Go's
// filepath utilities clean paths during manipulation or go file path,
// removing the ".". thus, if not "..." or go file, let's update the path
// components so that:
//
// d = r
// b = "."
if b != "..." {
if b != "..." && filepath.Ext(b) != ".go" {
d = r
b = "."
}
Expand Down
Loading