Skip to content

Commit dabf09d

Browse files
committed
Fix controller-tools doesn't support single files as input
#837
1 parent 3f5bd8e commit dabf09d

22 files changed

+1242
-71
lines changed
+108
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package crd_test
18+
19+
import (
20+
"bytes"
21+
"os"
22+
"path/filepath"
23+
24+
"github.com/google/go-cmp/cmp"
25+
. "github.com/onsi/ginkgo"
26+
. "github.com/onsi/gomega"
27+
28+
"sigs.k8s.io/controller-tools/pkg/crd"
29+
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
30+
"sigs.k8s.io/controller-tools/pkg/genall"
31+
"sigs.k8s.io/controller-tools/pkg/loader"
32+
"sigs.k8s.io/controller-tools/pkg/markers"
33+
)
34+
35+
var _ = Describe("CRD Generation golang files", func() {
36+
var (
37+
ctx, ctx2 *genall.GenerationContext
38+
out *outputRule
39+
40+
genDir = filepath.Join("testdata", "multiple_files")
41+
)
42+
43+
BeforeEach(func() {
44+
By("switching into testdata to appease go modules")
45+
cwd, err := os.Getwd()
46+
Expect(err).NotTo(HaveOccurred())
47+
Expect(os.Chdir(genDir)).To(Succeed()) // go modules are directory-sensitive
48+
defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()
49+
50+
By("loading the roots")
51+
pkgs, err := loader.LoadRoots("file_one.go")
52+
Expect(err).NotTo(HaveOccurred())
53+
Expect(pkgs).To(HaveLen(1))
54+
Expect(pkgs[0].GoFiles).To(HaveLen(1))
55+
pkgs2, err := loader.LoadRoots("file_two.go", "file_two_reference.go")
56+
Expect(err).NotTo(HaveOccurred())
57+
Expect(pkgs2).To(HaveLen(1))
58+
Expect(pkgs2[0].GoFiles).To(HaveLen(2))
59+
60+
By("setup up the context")
61+
reg := &markers.Registry{}
62+
Expect(crdmarkers.Register(reg)).To(Succeed())
63+
out = &outputRule{
64+
buf: &bytes.Buffer{},
65+
}
66+
ctx = &genall.GenerationContext{
67+
Collector: &markers.Collector{Registry: reg},
68+
Roots: pkgs,
69+
Checker: &loader.TypeChecker{},
70+
OutputRule: out,
71+
}
72+
ctx2 = &genall.GenerationContext{
73+
Collector: &markers.Collector{Registry: reg},
74+
Roots: pkgs2,
75+
Checker: &loader.TypeChecker{},
76+
OutputRule: out,
77+
}
78+
})
79+
80+
It("should have deterministic output for single golang file", func() {
81+
By("calling Generate on single golang file")
82+
gen := &crd.Generator{
83+
CRDVersions: []string{"v1"},
84+
}
85+
Expect(gen.Generate(ctx)).NotTo(HaveOccurred())
86+
87+
By("loading the desired YAML")
88+
expectedFileOne, err := os.ReadFile(filepath.Join(genDir, "one.example.com.yaml"))
89+
Expect(err).NotTo(HaveOccurred())
90+
expectedFileOne = fixAnnotations(expectedFileOne)
91+
expectedOut := string(expectedFileOne)
92+
Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut))
93+
})
94+
It("should have deterministic output for multiple golang files referencing other types", func() {
95+
By("calling Generate on two golang files")
96+
gen := &crd.Generator{
97+
CRDVersions: []string{"v1"},
98+
}
99+
Expect(gen.Generate(ctx2)).NotTo(HaveOccurred())
100+
101+
By("loading the desired YAML file")
102+
expectedFileTwo, err := os.ReadFile(filepath.Join(genDir, "two.example.com.yaml"))
103+
Expect(err).NotTo(HaveOccurred())
104+
expectedFileTwo = fixAnnotations(expectedFileTwo)
105+
expectedOut := string(expectedFileTwo)
106+
Expect(out.buf.String()).To(Equal(expectedOut), cmp.Diff(out.buf.String(), expectedOut))
107+
})
108+
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
*/
15+
16+
// +groupName=testdata.kubebuilder.io
17+
package multiplefiles
18+
19+
import (
20+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21+
multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
22+
)
23+
24+
type OneResourceSpec struct {
25+
Struct multiver.OuterStruct `json:"struct,omitempty"`
26+
}
27+
28+
// +kubebuilder:object:root=true
29+
// +kubebuilder:subresource:status
30+
// +kubebuilder:resource:singular=oneresource
31+
32+
type OneResource struct {
33+
metav1.TypeMeta `json:",inline"`
34+
metav1.ObjectMeta `json:"metadata,omitempty"`
35+
36+
Spec OneResourceSpec `json:"spec"`
37+
}
38+
39+
// +kubebuilder:object:root=true
40+
41+
type OneResourceList struct {
42+
metav1.TypeMeta `json:",inline"`
43+
metav1.ListMeta `json:"metadata,omitempty"`
44+
Items []OneResource `json:"items"`
45+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
*/
15+
16+
// +groupName=testdata.kubebuilder.io
17+
package multiplefiles
18+
19+
import (
20+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21+
)
22+
23+
// +kubebuilder:object:root=true
24+
// +kubebuilder:subresource:status
25+
// +kubebuilder:resource:singular=tworesource
26+
27+
type TwoResource struct {
28+
metav1.TypeMeta `json:",inline"`
29+
metav1.ObjectMeta `json:"metadata,omitempty"`
30+
31+
Spec TwoResourceSpec `json:"spec"`
32+
}
33+
34+
// +kubebuilder:object:root=true
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
7+
http://www.apache.org/licenses/LICENSE-2.0
8+
9+
Unless required by applicable law or agreed to in writing, software
10+
distributed under the License is distributed on an "AS IS" BASIS,
11+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
See the License for the specific language governing permissions and
13+
limitations under the License.
14+
*/
15+
16+
package multiplefiles
17+
18+
import (
19+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
20+
multiver "testdata.kubebuilder.io/cronjob/multiple_versions"
21+
)
22+
23+
type TwoResourceSpec struct {
24+
Struct multiver.OuterStruct `json:"struct,omitempty"`
25+
}
26+
27+
type TwoResourceList struct {
28+
metav1.TypeMeta `json:",inline"`
29+
metav1.ListMeta `json:"metadata,omitempty"`
30+
Items []TwoResource `json:"items"`
31+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
apiVersion: apiextensions.k8s.io/v1
3+
kind: CustomResourceDefinition
4+
metadata:
5+
annotations:
6+
controller-gen.kubebuilder.io/version: (unknown)
7+
name: oneresources.testdata.kubebuilder.io
8+
spec:
9+
group: testdata.kubebuilder.io
10+
names:
11+
kind: OneResource
12+
listKind: OneResourceList
13+
plural: oneresources
14+
singular: oneresource
15+
scope: Namespaced
16+
versions:
17+
- name: multiplefiles
18+
schema:
19+
openAPIV3Schema:
20+
properties:
21+
apiVersion:
22+
description: 'APIVersion defines the versioned schema of this representation
23+
of an object. Servers should convert recognized schemas to the latest
24+
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
25+
type: string
26+
kind:
27+
description: 'Kind is a string value representing the REST resource this
28+
object represents. Servers may infer this from the endpoint the client
29+
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
30+
type: string
31+
metadata:
32+
type: object
33+
spec:
34+
properties:
35+
struct:
36+
properties:
37+
struct:
38+
properties:
39+
foo:
40+
type: string
41+
type: object
42+
x-kubernetes-map-type: atomic
43+
type: object
44+
type: object
45+
required:
46+
- spec
47+
type: object
48+
served: true
49+
storage: true
50+
subresources:
51+
status: {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
apiVersion: apiextensions.k8s.io/v1
3+
kind: CustomResourceDefinition
4+
metadata:
5+
annotations:
6+
controller-gen.kubebuilder.io/version: (unknown)
7+
name: tworesources.testdata.kubebuilder.io
8+
spec:
9+
group: testdata.kubebuilder.io
10+
names:
11+
kind: TwoResource
12+
listKind: TwoResourceList
13+
plural: tworesources
14+
singular: tworesource
15+
scope: Namespaced
16+
versions:
17+
- name: multiplefiles
18+
schema:
19+
openAPIV3Schema:
20+
properties:
21+
apiVersion:
22+
description: 'APIVersion defines the versioned schema of this representation
23+
of an object. Servers should convert recognized schemas to the latest
24+
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
25+
type: string
26+
kind:
27+
description: 'Kind is a string value representing the REST resource this
28+
object represents. Servers may infer this from the endpoint the client
29+
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
30+
type: string
31+
metadata:
32+
type: object
33+
spec:
34+
properties:
35+
struct:
36+
properties:
37+
struct:
38+
properties:
39+
foo:
40+
type: string
41+
type: object
42+
x-kubernetes-map-type: atomic
43+
type: object
44+
type: object
45+
required:
46+
- spec
47+
type: object
48+
served: true
49+
storage: true
50+
subresources:
51+
status: {}

pkg/loader/loader.go

+6-14
Original file line numberDiff line numberDiff line change
@@ -389,12 +389,6 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
389389
}
390390
}()
391391

392-
// uniquePkgIDs is used to keep track of the discovered packages to be nice
393-
// and try and prevent packages from showing up twice when nested module
394-
// support is enabled. there is not harm that comes from this per se, but
395-
// it makes testing easier when a known number of modules can be asserted
396-
uniquePkgIDs := sets.Set[string]{}
397-
398392
// loadPackages returns the Go packages for the provided roots
399393
//
400394
// if validatePkgFn is nil, a package will be returned in the slice,
@@ -412,10 +406,7 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
412406
var pkgs []*Package
413407
for _, rp := range rawPkgs {
414408
p := l.packageFor(rp)
415-
if !uniquePkgIDs.Has(p.ID) {
416-
pkgs = append(pkgs, p)
417-
uniquePkgIDs.Insert(p.ID)
418-
}
409+
pkgs = append(pkgs, p)
419410
}
420411
return pkgs, nil
421412
}
@@ -568,13 +559,14 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
568559
for _, r := range fspRoots {
569560
b, d := filepath.Base(r), filepath.Dir(r)
570561

571-
// we want the base part of the path to be either "..." or ".", except
572-
// Go's filepath utilities clean paths during manipulation, removing the
573-
// ".". thus, if not "...", let's update the path components so that:
562+
// we want the base part of the path to be either "..." or ".", except Go's
563+
// filepath utilities clean paths during manipulation or go file path,
564+
// removing the ".". thus, if not "..." or go file, let's update the path
565+
// components so that:
574566
//
575567
// d = r
576568
// b = "."
577-
if b != "..." {
569+
if b != "..." && filepath.Ext(b) != ".go" {
578570
d = r
579571
b = "."
580572
}

0 commit comments

Comments
 (0)