Skip to content

Commit 7bfe71f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#596 from jpbetz/apigroups
Move modelPackage to group level, provide opt-out
2 parents b7f5293 + d3ae681 commit 7bfe71f

13 files changed

Lines changed: 164 additions & 45 deletions

File tree

‎pkg/generators/apidefinitions/loader.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,12 @@ const (
3333
// in the source tree like we do for everything else.
3434
schemeGroupVersion = "apidefinitions.k8s.io/v1alpha1"
3535
kindAPIVersion = "APIVersion"
36+
kindAPIGroup = "APIGroup"
3637

3738
// We have a naming convention for the files used to define
3839
// APIs in the source tree.
3940
apiVersionFile = "apiversion.yaml"
41+
apiGroupFile = "apigroup.yaml"
4042
)
4143

4244
// LoadAPIVersion reads an apiversion.yaml file, returning nil if absent.
@@ -58,6 +60,22 @@ func LoadAPIVersion(dir string) (*APIVersion, error) {
5860
return av, nil
5961
}
6062

63+
// LoadAPIGroup reads an apigroup.yaml file, returning nil if absent.
64+
func LoadAPIGroup(dir string) (*APIGroup, error) {
65+
data, err := readManifest(dir, apiGroupFile)
66+
if err != nil || data == nil {
67+
return nil, err
68+
}
69+
g := &APIGroup{}
70+
if err := yaml.Unmarshal(data, g); err != nil {
71+
return nil, fmt.Errorf("%s: %w", filepath.Join(dir, apiGroupFile), err)
72+
}
73+
if err := validateTypeMeta(g.APIVersion, g.Kind, kindAPIGroup); err != nil {
74+
return nil, fmt.Errorf("%s: %w", filepath.Join(dir, apiGroupFile), err)
75+
}
76+
return g, nil
77+
}
78+
6179
func readManifest(dir, filename string) ([]byte, error) {
6280
data, err := os.ReadFile(filepath.Join(dir, filename))
6381
if errors.Is(err, fs.ErrNotExist) {

‎pkg/generators/apidefinitions/loader_test.go‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@ func TestLoadAPIVersion(t *testing.T) {
2626
const manifest = `apiVersion: apidefinitions.k8s.io/v1alpha1
2727
kind: APIVersion
2828
metadata:
29-
name: test.apidefinitions.k8s.io/v1
30-
spec:
31-
modelPackage: io.k8s.api.apps.v1
29+
name: apps/v1
3230
`
3331
dir := t.TempDir()
3432
if err := os.WriteFile(filepath.Join(dir, "apiversion.yaml"), []byte(manifest), 0644); err != nil {
@@ -42,11 +40,11 @@ spec:
4240
if av == nil {
4341
t.Fatal("expected APIVersion, got nil")
4442
}
45-
if got, want := av.Metadata.Name, "test.apidefinitions.k8s.io/v1"; got != want {
43+
if got, want := av.Metadata.Name, "apps/v1"; got != want {
4644
t.Errorf("metadata.name = %q, want %q", got, want)
4745
}
48-
if got, want := av.Spec.ModelPackage, "io.k8s.api.apps.v1"; got != want {
49-
t.Errorf("spec.modelPackage = %q, want %q", got, want)
46+
if got, want := av.VersionFromName(), "v1"; got != want {
47+
t.Errorf("VersionFromName() = %q, want %q", got, want)
5048
}
5149
}
5250

@@ -60,6 +58,47 @@ func TestLoadAPIVersion_Missing(t *testing.T) {
6058
}
6159
}
6260

61+
func TestLoadAPIGroup(t *testing.T) {
62+
const manifest = `apiVersion: apidefinitions.k8s.io/v1alpha1
63+
kind: APIGroup
64+
metadata:
65+
name: apps
66+
spec:
67+
modelPackage: io.k8s.api.apps
68+
`
69+
dir := t.TempDir()
70+
if err := os.WriteFile(filepath.Join(dir, "apigroup.yaml"), []byte(manifest), 0644); err != nil {
71+
t.Fatal(err)
72+
}
73+
74+
g, err := LoadAPIGroup(dir)
75+
if err != nil {
76+
t.Fatalf("unexpected error: %v", err)
77+
}
78+
if g == nil {
79+
t.Fatal("expected APIGroup, got nil")
80+
}
81+
if got, want := g.Metadata.Name, "apps"; got != want {
82+
t.Errorf("metadata.name = %q, want %q", got, want)
83+
}
84+
if got, want := g.Spec.ModelPackage, "io.k8s.api.apps"; got != want {
85+
t.Errorf("spec.modelPackage = %q, want %q", got, want)
86+
}
87+
if got, want := g.ModelPackageFor("v1"), "io.k8s.api.apps.v1"; got != want {
88+
t.Errorf("ModelPackageFor(v1) = %q, want %q", got, want)
89+
}
90+
}
91+
92+
func TestLoadAPIGroup_Missing(t *testing.T) {
93+
g, err := LoadAPIGroup(t.TempDir())
94+
if err != nil {
95+
t.Fatalf("unexpected error: %v", err)
96+
}
97+
if g != nil {
98+
t.Errorf("expected nil APIGroup, got %+v", g)
99+
}
100+
}
101+
63102
func TestValidateName(t *testing.T) {
64103
tests := []struct {
65104
name string

‎pkg/generators/apidefinitions/types.go‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,35 @@ type APIVersion struct {
3232
}
3333

3434
// APIVersionSpec provides a specification for an APIVersion.
35-
type APIVersionSpec struct {
36-
// ModelPackage is the OpenAPI model package name for this group/version
37-
// (e.g. "io.k8s.api.apps.v1").
35+
type APIVersionSpec struct{}
36+
37+
// VersionFromName returns the version part of the metadata.name.
38+
func (av *APIVersion) VersionFromName() string {
39+
_, v, _ := splitGroupVersion(av.Metadata.Name)
40+
return v
41+
}
42+
43+
// APIGroup declares an external versioned API group.
44+
type APIGroup struct {
45+
APIVersion string `json:"apiVersion"`
46+
Kind string `json:"kind"`
47+
Metadata Metadata `json:"metadata"`
48+
Spec APIGroupSpec `json:"spec"`
49+
}
50+
51+
// APIGroupSpec provides a specification of an APIGroup.
52+
type APIGroupSpec struct {
53+
// ModelPackage is the OpenAPI model package prefix shared by every
54+
// version in this group. The per-version model package is
55+
// "<ModelPackage>.<version>" (e.g. "io.k8s.api.apps" + "v1" yields
56+
// "io.k8s.api.apps.v1").
3857
ModelPackage string `json:"modelPackage,omitempty"`
3958
}
59+
60+
// ModelPackageFor returns the OpenAPI model package for the given version.
61+
func (g *APIGroup) ModelPackageFor(version string) string {
62+
if g.Spec.ModelPackage == "" {
63+
return ""
64+
}
65+
return g.Spec.ModelPackage + "." + version
66+
}

‎pkg/generators/model_names.go‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package generators
1919
import (
2020
"fmt"
2121
"io"
22+
"path/filepath"
2223
"strings"
2324

2425
"k8s.io/gengo/v2"
@@ -42,24 +43,48 @@ func extractOpenAPISchemaNamePackage(comments []string) (string, error) {
4243
}
4344

4445
// resolvePackageModelPackage returns the OpenAPI model package for pkg.
45-
// The model package may be specified via apiversion.yaml's spec.modelPackage
46-
// or the +k8s:openapi-model-package tag. If both are present, they must agree.
46+
//
47+
// The package's model package is derived from two yaml files:
48+
// - apiversion.yaml in pkg's directory provides the version,
49+
// - apigroup.yaml in the parent directory provides spec.modelPackage,
50+
//
51+
// joined as "<spec.modelPackage>.<version>" (e.g. "io.k8s.api.apps" + "v1"
52+
// → "io.k8s.api.apps.v1").
53+
//
54+
// The legacy +k8s:openapi-model-package= tag, if present, must agree with
55+
// the derived value; an empty result means the package opts out of openapi
56+
// model naming.
4757
func resolvePackageModelPackage(pkg *types.Package) (string, error) {
58+
tagPackage, err := extractOpenAPISchemaNamePackage(pkg.Comments)
59+
if err != nil {
60+
return "", err
61+
}
4862
apiVersion, err := apidefinitions.LoadAPIVersion(pkg.Dir)
4963
if err != nil {
5064
return "", err
5165
}
52-
tagPackage, err := extractOpenAPISchemaNamePackage(pkg.Comments)
66+
if apiVersion == nil {
67+
// No apiversion.yaml: fall back to whatever the tag says (possibly "").
68+
return tagPackage, nil
69+
}
70+
parentDir := filepath.Dir(pkg.Dir)
71+
apiGroup, err := apidefinitions.LoadAPIGroup(parentDir)
5372
if err != nil {
5473
return "", err
5574
}
56-
if apiVersion != nil {
57-
if len(tagPackage) > 0 && tagPackage != apiVersion.Spec.ModelPackage {
58-
return "", fmt.Errorf("in package %s: apiversion.yaml spec.modelPackage (%q) and k8s:openapi-model-package tag (%q) disagree. Please make them match or remove one.", pkg.Dir, apiVersion.Spec.ModelPackage, tagPackage)
75+
if apiGroup == nil {
76+
if tagPackage != "" {
77+
return "", fmt.Errorf("in package %s: %s declares +k8s:openapi-model-package=%q but no apigroup.yaml is present in the parent directory %s to derive a model package",
78+
pkg.Dir, filepath.Join(pkg.Dir, "doc.go"), tagPackage, parentDir)
5979
}
60-
return apiVersion.Spec.ModelPackage, nil
80+
return "", nil
81+
}
82+
derived := apiGroup.ModelPackageFor(apiVersion.VersionFromName())
83+
if tagPackage != "" && tagPackage != derived {
84+
return "", fmt.Errorf("in package %s: parent apigroup.yaml derives model package %q for version %q but +k8s:openapi-model-package tag is %q. Please make them match or remove the tag.",
85+
pkg.Dir, derived, apiVersion.VersionFromName(), tagPackage)
6186
}
62-
return tagPackage, nil
87+
return derived, nil
6388
}
6489

6590
func singularTag(tagName string, comments []string) (*gengo.Tag, error) {

‎pkg/generators/openapi.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,21 +120,26 @@ func apiTypeFilterFunc(c *generator.Context, t *types.Type) bool {
120120
}
121121

122122
// isOpenAPIEnabledForPackage reports whether openapi generation is
123-
// requested for pkg. apiversion.yaml is authoritative when present;
124-
// the legacy +k8s:openapi-gen=true tag is consulted only when the yaml
125-
// is absent.
123+
// requested for pkg. apigroup.yaml and apiversion.yaml are authoritative when present.
124+
// The +k8s:openapi-gen=true tag is checked only when the API definition files
125+
// are absent.
126126
func isOpenAPIEnabledForPackage(pkg *types.Package) bool {
127127
if pkg == nil {
128128
return false
129129
}
130+
hasFalse := hasOpenAPITagValue(pkg.Comments, tagValueFalse)
131+
hasTrue := hasOpenAPITagValue(pkg.Comments, tagValueTrue)
132+
if hasFalse && !hasTrue { // For backward compatability
133+
return false
134+
}
130135
av, err := apidefinitions.LoadAPIVersion(pkg.Dir)
131136
if err != nil {
132137
klog.Fatalf("Package %v: %v", pkg.Path, err)
133138
}
134139
if av != nil {
135140
return true
136141
}
137-
return hasOpenAPITagValue(pkg.Comments, tagValueTrue)
142+
return hasTrue
138143
}
139144

140145
const (

‎test/integration/integration_suite_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,12 @@ var _ = BeforeSuite(func() {
124124

125125
By("'apidefinitions' running openapi-gen")
126126
args = append([]string{
127-
"--output-dir", tempDir + "/apidefinitions",
128-
"--output-pkg", outputPkg + "/apidefinitions",
127+
"--output-dir", tempDir + "/apidefinitions/v1",
128+
"--output-pkg", outputPkg + "/apidefinitions/v1",
129129
"--output-file", generatedCodeFileName,
130130
"--output-model-name-file", generatedSchemaNameCodeFileName,
131131
"--go-header-file", headerFilePath,
132-
}, path.Join(testPkgRoot, "apidefinitions"))
132+
}, path.Join(testPkgRoot, "apidefinitions", "v1"))
133133
command = exec.Command(openAPIGenPath, args...)
134134
command.Dir = workingDirectory
135135
session, err = gexec.Start(command, GinkgoWriter, GinkgoWriter)
@@ -213,8 +213,8 @@ var _ = Describe("Open API Definitions Generation", func() {
213213
It("'apidefinitions' Generated code should match golden files", func() {
214214
command := exec.Command(
215215
"diff", "-u",
216-
"pkg/generated/apidefinitions/"+generatedCodeFileName,
217-
generatedFile("apidefinitions/"+generatedCodeFileName),
216+
"pkg/generated/apidefinitions/v1/"+generatedCodeFileName,
217+
generatedFile("apidefinitions/v1/"+generatedCodeFileName),
218218
)
219219
command.Dir = workingDirectory
220220
session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter)

test/integration/pkg/generated/apidefinitions/openapi_generated.go renamed to test/integration/pkg/generated/apidefinitions/v1/openapi_generated.go

Lines changed: 8 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integration/testdata/apidefinitions/apiversion.yaml renamed to test/integration/testdata/apidefinitions/apigroup.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
apiVersion: apidefinitions.k8s.io/v1alpha1
2-
kind: APIVersion
2+
kind: APIGroup
33
metadata:
4-
name: test.apidefinitions.k8s.io/v1
4+
name: apidefinitions.test.k8s.io
55
spec:
66
modelPackage: io.k8s.kube-openapi.test.integration.testdata.apidefinitions

‎test/integration/testdata/apidefinitions/doc.go‎

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎test/integration/testdata/apidefinitions/v1/apiversion.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
apiVersion: apidefinitions.k8s.io/v1alpha1
2+
kind: APIVersion
3+
metadata:
4+
name: apidefinitions.test.k8s.io/v1

0 commit comments

Comments
 (0)