upstream/kubernetes
1
0
Fork 0
mirror of https://github.com/kubernetes/kubernetes.git synced 2026-06-08 16:30:57 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Improve error messages and documentation

Browse source
This commit is contained in:
Joe Betz 2026-05-08 18:50:16 -04:00
parent c45b9a8940
commit e6bc813ac3
No known key found for this signature in database
GPG key ID: 1E2BA7FEB91911CB
10 changed files with 108 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
staging/src/k8s.io/code-generator/cmd/applyconfiguration-gen/generators/targets.go
View file

@ -17,7 +17,6 @@ limitations under the License.
package generators
import (
"errors"
"fmt"
"path"
"path/filepath"
@ -294,11 +293,11 @@ func groupVersion(p *types.Package) (gv clientgentypes.GroupVersion, err error)
// If there's a comment of the form "// +groupName=somegroup" or
// "// +groupName=somegroup.foo.bar.io", use the first field (somegroup) as the name of the
// group when generating.
override, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil && !errors.Is(err, apidefinitions.ErrGroupUndeclared) {
override, ok, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil {
return gv, err
}
if err == nil {
if ok {
gv.Group = clientgentypes.Group(override)
}

7
staging/src/k8s.io/code-generator/cmd/client-gen/generators/client_generator.go
View file

@ -18,7 +18,6 @@ limitations under the License.
package generators
import (
"errors"
"fmt"
"path"
"path/filepath"
@ -281,11 +280,11 @@ func applyGroupOverrides(universe types.Universe, args *args.Args) error {
changes := make(map[clientgentypes.GroupVersion]clientgentypes.GroupVersion)
for gv, inputDir := range args.GroupVersionPackages() {
p := universe.Package(inputDir)
override, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil && !errors.Is(err, apidefinitions.ErrGroupUndeclared) {
override, ok, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil {
return err
}
if err == nil {
if ok {
newGV := clientgentypes.GroupVersion{
Group: clientgentypes.Group(override),
Version: gv.Version,

7
staging/src/k8s.io/code-generator/cmd/client-gen/generators/generator_for_group.go
View file

@ -17,7 +17,6 @@ limitations under the License.
package generators
import (
"errors"
"io"
"path"
@ -73,11 +72,11 @@ func (g *genGroup) GenerateType(c *generator.Context, t *types.Type, w io.Writer
// allow user to define a group name that's different from the one parsed from the directory.
p := c.Universe.Package(g.inputPackage)
groupName := g.group
override, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil && !errors.Is(err, apidefinitions.ErrGroupUndeclared) {
override, ok, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil {
return err
}
if err == nil {
if ok {
groupName = override
}

7
staging/src/k8s.io/code-generator/cmd/informer-gen/generators/targets.go
View file

@ -17,7 +17,6 @@ limitations under the License.
package generators
import (
"errors"
"fmt"
"path"
"path/filepath"
@ -159,11 +158,11 @@ func GetTargets(context *generator.Context, args *args.Args) []generator.Target
// If there's a comment of the form "// +groupName=somegroup" or
// "// +groupName=somegroup.foo.bar.io", use the first field (somegroup) as the name of the
// group when generating.
override, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil && !errors.Is(err, apidefinitions.ErrGroupUndeclared) {
override, ok, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil {
klog.Fatalf("error resolving group name: %v", err)
}
if err == nil {
if ok {
gv.Group = clientgentypes.Group(override)
}

7
staging/src/k8s.io/code-generator/cmd/lister-gen/generators/lister.go
View file

@ -17,7 +17,6 @@ limitations under the License.
package generators
import (
"errors"
"fmt"
"io"
"path"
@ -116,11 +115,11 @@ func GetTargets(context *generator.Context, args *args.Args) []generator.Target
// If there's a comment of the form "// +groupName=somegroup" or
// "// +groupName=somegroup.foo.bar.io", use the first field (somegroup) as the name of the
// group when generating.
override, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil && !errors.Is(err, apidefinitions.ErrGroupUndeclared) {
override, ok, err := apidefinitions.GroupNameForPackage(p.Comments)
if err != nil {
klog.Fatalf("error resolving group name: %v", err)
}
if err == nil {
if ok {
gv.Group = clientgentypes.Group(strings.SplitN(override, ".", 2)[0])
}

7
staging/src/k8s.io/code-generator/cmd/register-gen/generators/targets.go
View file

@ -17,7 +17,6 @@ limitations under the License.
package generators
import (
"errors"
"fmt"
"os"
"path"
@ -93,11 +92,11 @@ func GetTargets(context *generator.Context, args *args.Args) []generator.Target
// if there is a comment of the form "// +groupName=somegroup" or "// +groupName=somegroup.foo.bar.io",
// extract the fully qualified API group name from it and overwrite the group inferred from the package path
override, err := apidefinitions.GroupNameForPackage(pkg.Comments)
if err != nil && !errors.Is(err, apidefinitions.ErrGroupUndeclared) {
override, ok, err := apidefinitions.GroupNameForPackage(pkg.Comments)
if err != nil {
klog.Fatalf("error resolving group name: %v", err)
}
if err == nil {
if ok {
klog.V(5).Infof("overriding the group name with = %s", override)
gv.Group = clientgentypes.Group(override)
}

7
staging/src/k8s.io/code-generator/pkg/apidefinitions/activation_test.go
View file

@ -108,14 +108,13 @@ func TestIdentify(t *testing.T) {
comments: []string{"+k8s:prerelease-lifecycle-gen=false"},
},
{
name: "=false mixed with peer pkg passes through",
name: "=false mixed with peer pkg is rejected",
spec: Conversion,
comments: []string{
"+k8s:conversion-gen=k8s.io/api/extensions/v1beta1",
"+k8s:conversion-gen=false",
},
wantShouldGenerate: true,
wantValues: []string{"k8s.io/api/extensions/v1beta1", "false"},
wantErr: `cannot mix "false"`,
},
{
name: "repeated identical peer is allowed and preserved",
@ -148,7 +147,7 @@ func TestIdentify(t *testing.T) {
comments: []string{
"+k8s:conversion-x-gen=k8s.io/api/foo",
},
wantErr: "+k8s:conversion--gen",
wantErr: "+k8s:conversion-x-gen",
},
{
name: "non-generator +k8s: tag passes validation untouched",

72
staging/src/k8s.io/code-generator/pkg/apidefinitions/lint.go
View file

@ -17,6 +17,7 @@ limitations under the License.
package apidefinitions
import (
"errors"
"fmt"
"path"
"reflect"
@ -146,18 +147,41 @@ func checkExplicitDisablement(pkg *types.Package) error {
return nil
}
tagged := codetags.Extract("+", pkg.Comments)
var missing []string
var missing []Spec
for _, spec := range expected {
if _, ok := tagged[spec.ActivationTag]; !ok {
missing = append(missing, "+"+spec.ActivationTag)
missing = append(missing, spec)
}
}
if len(missing) == 0 {
return nil
}
sort.Strings(missing)
return fmt.Errorf("%s/doc.go: package classifies as %s but lacks explicit +k8s:<gen>=value or =false for: %s",
pkg.Dir, roles, strings.Join(missing, ", "))
sort.Slice(missing, func(i, j int) bool { return missing[i].ActivationTag < missing[j].ActivationTag })
var sb strings.Builder
fmt.Fprintf(&sb, "%s/doc.go: %s.\n", pkg.Dir, roles.describePackage(pkg.Dir))
sb.WriteString("Set explicit configuration or disablement for these generators in doc.go:")
for _, spec := range missing {
fmt.Fprintf(&sb, "\n +%s=<%s>", spec.ActivationTag, valueHint(spec))
}
return errors.New(sb.String())
}
// valueHint describes the accepted shape of an ActivationTag value, for
// inclusion in user-facing errors.
func valueHint(spec Spec) string {
switch spec.ValueMode {
case ConversionPeerList:
return "peer-package or false"
case TypeFilterList:
return "TypeMeta or false"
case Package:
return "package or false"
case Boolean:
return "true or false"
default:
return "value or false"
}
}
func expectedGenerators(roles packageRoles) []Spec {
@ -172,31 +196,47 @@ func expectedGenerators(roles packageRoles) []Spec {
}
type packageRoles struct {
// Not all packages have a single role. Combinations are allowed.
isExternalGroup bool
isInternalGroup bool
// isExternalGroup identifies an API group package containing
// versioned API types (e.g. staging/src/k8s.io/api/apps).
isExternalGroup bool
// isInternalGroup identifies an API group package containing
// API type internals (e.g. pkg/apis/apps). The "internal type" (aka Hub type)
// often lives here.
isInternalGroup bool
// isExternalVersion identifies a package containing versioned API types
// (e.g. staging/src/k8s.io/api/apps/v1).
isExternalVersion bool
// isInternalVersion identifies a per-version internals package containing
// conversion / defaulting / validation for hub types in the parent
// directory (e.g. pkg/apis/apps/v1).
isInternalVersion bool
}
func (r packageRoles) String() string {
func (r packageRoles) describePackage(pkgDir string) string {
var parts []string
if r.isExternalGroup {
parts = append(parts, "external-group")
parts = append(parts, "API group package with versioned API types in subpackages")
}
if r.isInternalGroup {
parts = append(parts, "internal-group")
parts = append(parts, "API group package containing hub API types")
}
if r.isExternalVersion {
parts = append(parts, "external-version")
parts = append(parts, "package containing versioned API types")
}
if r.isInternalVersion {
parts = append(parts, "internal-version")
desc := "package containing per-version internals (conversion, defaulting, validation)"
if parent := path.Dir(pkgDir); parent != "" && parent != "." && parent != "/" {
desc += " for hub types at " + parent
}
parts = append(parts, desc)
}
if len(parts) == 0 {
return "not-api-package"
return "package has no recognized API role"
}
return strings.Join(parts, ",")
return "this package looks like: " + strings.Join(parts, "; ")
}
// classifyPackage looks for evidence of package roles.
@ -236,7 +276,7 @@ func classifyPackage(pkg *types.Package) packageRoles {
}
func looksLikeAPIPackage(pkg *types.Package) bool {
if _, err := GroupNameForPackage(pkg.Comments); err == nil {
if _, ok, _ := GroupNameForPackage(pkg.Comments); ok {
return true
}
for name := range codetags.Extract("+", pkg.Comments) {

38
staging/src/k8s.io/code-generator/pkg/apidefinitions/tags.go
View file

@ -17,7 +17,7 @@ limitations under the License.
package apidefinitions
import (
"errors"
"fmt"
"k8s.io/gengo/v2/codetags"
)
@ -25,8 +25,9 @@ import (
const tagGroupName = "groupName"
// extractGeneratorTag returns the values of "+<tagName>" in comments.
// A sole "false" value sets optedOut and clears values; any other value
// (including "false" mixed with other values) is returned as-is.
// If any value is "false", every value must be "false" (duplicate
// disablement is allowed but mixing disablement with any other value
// is an error); optedOut is set when so.
func extractGeneratorTag(comments []string, tagName string) (values []string, optedOut bool, err error) {
if tagName == "" {
return nil, false, nil
@ -35,7 +36,18 @@ func extractGeneratorTag(comments []string, tagName string) (values []string, op
if err != nil {
return nil, false, err
}
if len(values) == 1 && values[0] == "false" {
hasFalse, hasOther := false, false
for _, v := range values {
if v == "false" {
hasFalse = true
} else {
hasOther = true
}
}
if hasFalse && hasOther {
return nil, false, fmt.Errorf("+%s: cannot mix \"false\" with other values; got %v", tagName, values)
}
if hasFalse {
return nil, true, nil
}
return values, false, nil
@ -71,19 +83,15 @@ func extractTags(lines []string, names ...string) (map[string][]codetags.Tag, er
return out, nil
}
// ErrGroupUndeclared indicates that no +groupName= tag declares a group
// for the package.
var ErrGroupUndeclared = errors.New("no group declared for package")
// GroupNameForPackage returns the +groupName= value from comments, or
// ErrGroupUndeclared if no such tag is present.
func GroupNameForPackage(comments []string) (string, error) {
// GroupNameForPackage returns the value of the +groupName= tag from
// comments. ok is false when the tag is absent.
func GroupNameForPackage(comments []string) (group string, ok bool, err error) {
values, err := tagValues(comments, tagGroupName)
if err != nil {
return "", err
return "", false, err
}
if len(values) > 0 {
return values[0], nil
if len(values) == 0 {
return "", false, nil
}
return "", ErrGroupUndeclared
return values[0], true, nil
}

20
staging/src/k8s.io/code-generator/pkg/apidefinitions/tags_test.go
View file

@ -17,7 +17,6 @@ limitations under the License.
package apidefinitions
import (
"errors"
"testing"
)
@ -26,24 +25,21 @@ func TestGroupNameForPackage(t *testing.T) {
name string
comments []string
want string
wantErr error
wantOK bool
}{
{name: "groupName tag", comments: []string{"+groupName=apps"}, want: "apps"},
{name: "qualified groupName", comments: []string{"+groupName=foo.example.com"}, want: "foo.example.com"},
{name: "no tag", wantErr: ErrGroupUndeclared},
{name: "groupName tag", comments: []string{"+groupName=apps"}, want: "apps", wantOK: true},
{name: "qualified groupName", comments: []string{"+groupName=foo.example.com"}, want: "foo.example.com", wantOK: true},
{name: "no tag"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := GroupNameForPackage(tc.comments)
if tc.wantErr != nil {
if !errors.Is(err, tc.wantErr) {
t.Errorf("err = %v, want %v", err, tc.wantErr)
}
return
}
got, ok, err := GroupNameForPackage(tc.comments)
if err != nil {
t.Fatalf("err = %v", err)
}
if ok != tc.wantOK {
t.Errorf("ok = %v, want %v", ok, tc.wantOK)
}
if got != tc.want {
t.Errorf("got %q, want %q", got, tc.want)
}