CNTRLPLANE-2449: feat: implement OpenAPIModelNamer interface#2856
Open
ingvagabund wants to merge 6 commits into
Open
CNTRLPLANE-2449: feat: implement OpenAPIModelNamer interface#2856ingvagabund wants to merge 6 commits into
ingvagabund wants to merge 6 commits into
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Hello @ingvagabund! Some important instructions when contributing to openshift/api: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (50)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds +k8s:openapi-model-package annotations to many API package doc.go files, updates go.mod dependency versions, and introduces a new tools/codegen "modelname" generator (CLI command, config types, generator implementation, helper, and wiring into the default generator pipeline). Suggested reviewers
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
openshift-ci
Bot
added
the
size/XXL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform/v1alpha1/doc.go`:
- Line 4: Remove the conflicting code-gen directive or reconcile the package
comment: either delete the
"+k8s:openapi-model-package=com.github.openshift.api.platform.v1alpha1"
directive from the deprecated platform/v1alpha1 doc.go so the file matches the
existing "kept here for historical reference only" comment, or if you intend
this API to participate in code generation, update the deprecation text in
doc.go to remove/adjust language that says it "will not be used to generate
code" so it accurately reflects that code-gen is enabled.
In `@tools/codegen/pkg/modelname/modelname.go`:
- Line 17: The generateModelNames function currently ignores the verify bool;
modify generateModelNames(globalParser *parser.Parser, universe types.Universe,
packagePath, outputFileName, headerFilePath string, verify bool) to first
generate the model-name output into a memory buffer/string (using the existing
generation logic that uses globalParser/universe/packagePath/headerFilePath),
and then: if verify is true, read the existing outputFileName from disk and
compare it to the generated buffer and return a non-nil error when they differ
(so CI/--verify fails) without writing any files; if verify is false, proceed to
write headerFilePath/outputFileName as before. Ensure the compare treats missing
file as a mismatch and include clear error messages.
- Around line 39-43: The helper myTargets should not call klog.Fatalf; instead
propagate the error: change myTargets from func(context *gengenerator.Context)
[]gengenerator.Target to func(context *gengenerator.Context)
([]gengenerator.Target, error), remove klog.Fatalf("Failed loading boilerplate:
%v", err) and return nil, fmt.Errorf("loading boilerplate: %w", err) (or the
preferred error type), and update any callers to handle the returned error and
aggregate/report it through the generator's normal error path; keep the
gengo.GoBoilerplate invocation and error check but return the error instead of
exiting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2ecb0b5a-773b-457b-9d2b-0f401b72230c
⛔ Files ignored due to path filters (245)
apiextensions/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*apiserver/v1/zz_generated.model_name.gois excluded by!**/zz_generated*apps/v1/zz_generated.model_name.gois excluded by!**/zz_generated*authorization/v1/zz_generated.model_name.gois excluded by!**/zz_generated*build/v1/zz_generated.model_name.gois excluded by!**/zz_generated*cloudnetwork/v1/zz_generated.model_name.gois excluded by!**/zz_generated*config/v1/zz_generated.model_name.gois excluded by!**/zz_generated*config/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*config/v1alpha2/zz_generated.model_name.gois excluded by!**/zz_generated*console/v1/zz_generated.model_name.gois excluded by!**/zz_generated*etcd/v1/zz_generated.model_name.gois excluded by!**/zz_generated*etcd/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*example/v1/zz_generated.model_name.gois excluded by!**/zz_generated*example/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*go.sumis excluded by!**/*.sumhelm/v1beta1/zz_generated.model_name.gois excluded by!**/zz_generated*image/v1/zz_generated.model_name.gois excluded by!**/zz_generated*imageregistry/v1/zz_generated.model_name.gois excluded by!**/zz_generated*insights/v1/zz_generated.model_name.gois excluded by!**/zz_generated*insights/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*insights/v1alpha2/zz_generated.model_name.gois excluded by!**/zz_generated*kubecontrolplane/v1/zz_generated.model_name.gois excluded by!**/zz_generated*legacyconfig/v1/zz_generated.model_name.gois excluded by!**/zz_generated*machine/v1/zz_generated.model_name.gois excluded by!**/zz_generated*machine/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*machine/v1beta1/zz_generated.model_name.gois excluded by!**/zz_generated*machineconfiguration/v1/zz_generated.model_name.gois excluded by!**/zz_generated*machineconfiguration/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*monitoring/v1/zz_generated.model_name.gois excluded by!**/zz_generated*network/v1/zz_generated.model_name.gois excluded by!**/zz_generated*network/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*networkoperator/v1/zz_generated.model_name.gois excluded by!**/zz_generated*oauth/v1/zz_generated.model_name.gois excluded by!**/zz_generated*openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*openshiftcontrolplane/v1/zz_generated.model_name.gois excluded by!**/zz_generated*operator/v1/zz_generated.model_name.gois excluded by!**/zz_generated*operator/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*operatorcontrolplane/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*operatoringress/v1/zz_generated.model_name.gois excluded by!**/zz_generated*osin/v1/zz_generated.model_name.gois excluded by!**/zz_generated*project/v1/zz_generated.model_name.gois excluded by!**/zz_generated*quota/v1/zz_generated.model_name.gois excluded by!**/zz_generated*route/v1/zz_generated.model_name.gois excluded by!**/zz_generated*samples/v1/zz_generated.model_name.gois excluded by!**/zz_generated*security/v1/zz_generated.model_name.gois excluded by!**/zz_generated*securityinternal/v1/zz_generated.model_name.gois excluded by!**/zz_generated*servicecertsigner/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*sharedresource/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*template/v1/zz_generated.model_name.gois excluded by!**/zz_generated*user/v1/zz_generated.model_name.gois excluded by!**/zz_generated*vendor/github.com/emicklei/go-restful/v3/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/CHANGES.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/compress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/curly.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/custom_verb.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/entity_accessors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/jsoniter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/jsr311.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/emicklei/go-restful/v3/route.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/.codecov.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/.mockery.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/SECURITY.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/cmd_utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/cmdutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/convert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/convert_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/format.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/sizeof.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv/type_constraints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/conv_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/convert.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/convert_types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils/path.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/fileutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/go.workis excluded by!**/*.work,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/go.work.sumis excluded by!**/*.sum,!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/initialism_index.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname/name_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonname_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/ifaces/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/ifaces/ifaces.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/ifaces/registry_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/adapter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/lexer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/ordered_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/pool.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/adapters/stdlib/json/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/concat.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils/ordered_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/jsonutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/json.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/loading.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading/yaml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/loading_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/BENCHMARK.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/initialism_index.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/name_lexem.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/name_mangler.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/pools.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/split.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/string_bytes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/mangling_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/name_lexem.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/net.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils/net.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/netutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/split.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/collection_formats.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/stringutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/typeutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yaml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/ordered_map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils/yaml.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/go-openapi/swag/yamlutils_iface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/josharian/intern/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/josharian/intern/intern.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/josharian/intern/license.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/buffer/pool.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/jlexer/bytestostr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/jlexer/bytestostr_nounsafe.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/jlexer/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/jlexer/lexer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/mailru/easyjson/jwriter/writer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/spf13/pflag/flag.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/mod/semver/semver.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc9218.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/errgroup/errgroup.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/inspector.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/iter.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/visit.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/objectpath/objectpath.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/typeutil/callee.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/typeutil/map.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/aliases/aliases.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/aliases/aliases_go122.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/core/event.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/core/export.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/keys/keys.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/label/label.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/bimport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iexport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/ureader_yes.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/import.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/stdlib.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typeparams/free.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typeparams/normalize.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/classify_call.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/element.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/fx.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/isnamed.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/qualifier.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/varkind.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/varkind_go124.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/zerovalue.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/versions/features.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/apic.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/decode.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/emitterc.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/encode.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/parserc.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/readerc.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/resolve.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/scannerc.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/sorter.gois excluded by!**/vendor/**,!vendor/**vendor/gopkg.in/yaml.v3/writerc.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (55)
apiextensions/v1alpha1/doc.goapiserver/v1/doc.goapps/v1/doc.goauthorization/v1/doc.gobuild/v1/doc.gocloudnetwork/v1/doc.goconfig/v1/doc.goconfig/v1alpha1/doc.goconfig/v1alpha2/doc.goconsole/v1/doc.goetcd/v1/doc.goetcd/v1alpha1/doc.goexample/v1/doc.goexample/v1alpha1/doc.gogo.modhelm/v1beta1/doc.goimage/v1/doc.goimageregistry/v1/doc.goinsights/v1/doc.goinsights/v1alpha1/doc.goinsights/v1alpha2/doc.gokubecontrolplane/v1/doc.golegacyconfig/v1/doc.gomachine/v1/doc.gomachine/v1alpha1/doc.gomachine/v1beta1/doc.gomachineconfiguration/v1/doc.gomachineconfiguration/v1alpha1/doc.gomonitoring/v1/doc.gonetwork/v1/doc.gonetwork/v1alpha1/doc.gonetworkoperator/v1/doc.gooauth/v1/doc.goopenshiftcontrolplane/v1/doc.gooperator/v1/doc.gooperator/v1alpha1/doc.gooperatorcontrolplane/v1alpha1/doc.gooperatoringress/v1/doc.goosin/v1/doc.goplatform/v1alpha1/doc.goproject/v1/doc.goquota/v1/doc.goroute/v1/doc.gosamples/v1/doc.gosecurity/v1/doc.gosecurityinternal/v1/doc.goservicecertsigner/v1alpha1/doc.gosharedresource/v1alpha1/doc.gotemplate/v1/doc.gotools/codegen/cmd/modelname.gotools/codegen/cmd/root.gotools/codegen/pkg/generation/types.gotools/codegen/pkg/modelname/generator.gotools/codegen/pkg/modelname/modelname.gouser/v1/doc.go
| // +k8s:deepcopy-gen=package,register | ||
| // +k8s:defaulter-gen=TypeMeta | ||
| // +k8s:openapi-gen=true | ||
| // +k8s:openapi-model-package=com.github.openshift.api.platform.v1alpha1 |
There was a problem hiding this comment.
Inconsistency: Adding code-generation directive to deprecated API.
The existing comment at lines 10-16 explicitly states that this API "will not be used to generate code by code-gen" and is "kept here for historical reference only." Adding the +k8s:openapi-model-package directive contradicts this statement.
Either remove this directive from the deprecated platform/v1alpha1 package, or update the comment if the API should now participate in code generation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform/v1alpha1/doc.go` at line 4, Remove the conflicting code-gen
directive or reconcile the package comment: either delete the
"+k8s:openapi-model-package=com.github.openshift.api.platform.v1alpha1"
directive from the deprecated platform/v1alpha1 doc.go so the file matches the
existing "kept here for historical reference only" comment, or if you intend
this API to participate in code generation, update the deprecation text in
doc.go to remove/adjust language that says it "will not be used to generate
code" so it accurately reflects that code-gen is enabled.
| ) | ||
|
|
||
| // generateModelNames generates the model name functions for the given API package path. | ||
| func generateModelNames(globalParser *parser.Parser, universe types.Universe, packagePath, outputFileName, headerFilePath string, verify bool) error { |
There was a problem hiding this comment.
verify mode is currently ignored in model-name generation.
Line 17 accepts verify, but the function never uses it. That makes verify and generate execution effectively identical for this generator, which can break expected --verify behavior.
Also applies to: 24-35, 48-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/codegen/pkg/modelname/modelname.go` at line 17, The generateModelNames
function currently ignores the verify bool; modify
generateModelNames(globalParser *parser.Parser, universe types.Universe,
packagePath, outputFileName, headerFilePath string, verify bool) to first
generate the model-name output into a memory buffer/string (using the existing
generation logic that uses globalParser/universe/packagePath/headerFilePath),
and then: if verify is true, read the existing outputFileName from disk and
compare it to the generated buffer and return a non-nil error when they differ
(so CI/--verify fails) without writing any files; if verify is false, proceed to
write headerFilePath/outputFileName as before. Ensure the compare treats missing
file as a mismatch and include clear error messages.
Comment on lines
+39
to
+43
| myTargets := func(context *gengenerator.Context) []gengenerator.Target { | ||
| boilerplate, err := gengo.GoBoilerplate(arguments.GoHeaderFile, gengo.StdBuildTag, gengo.StdGeneratedBy) | ||
| if err != nil { | ||
| klog.Fatalf("Failed loading boilerplate: %v", err) | ||
| } |
There was a problem hiding this comment.
Avoid terminating the whole process from this helper.
Line 42 uses klog.Fatalf, which exits immediately and bypasses the normal per-generator error aggregation path. Return an error instead so failures are reported consistently.
Proposed fix
- myTargets := func(context *gengenerator.Context) []gengenerator.Target {
- boilerplate, err := gengo.GoBoilerplate(arguments.GoHeaderFile, gengo.StdBuildTag, gengo.StdGeneratedBy)
- if err != nil {
- klog.Fatalf("Failed loading boilerplate: %v", err)
- }
-
- return generators.GetModelNameTargets(context, arguments, boilerplate)
- }
+ boilerplate, err := gengo.GoBoilerplate(arguments.GoHeaderFile, gengo.StdBuildTag, gengo.StdGeneratedBy)
+ if err != nil {
+ return fmt.Errorf("failed loading boilerplate: %w", err)
+ }
+ myTargets := func(context *gengenerator.Context) []gengenerator.Target {
+ return generators.GetModelNameTargets(context, arguments, boilerplate)
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/codegen/pkg/modelname/modelname.go` around lines 39 - 43, The helper
myTargets should not call klog.Fatalf; instead propagate the error: change
myTargets from func(context *gengenerator.Context) []gengenerator.Target to
func(context *gengenerator.Context) ([]gengenerator.Target, error), remove
klog.Fatalf("Failed loading boilerplate: %v", err) and return nil,
fmt.Errorf("loading boilerplate: %w", err) (or the preferred error type), and
update any callers to handle the returned error and aggregate/report it through
the generator's normal error path; keep the gengo.GoBoilerplate invocation and
error check but return the error instead of exiting.
Member
Author
|
Checking the job definition under openshift/release the script replaces o/api dep in o/client-go. Is the ci job expected to be overridden in case the API definition changes? |
ingvagabund
force-pushed
the
implement-OpenAPIModelNamer-interface
branch
from
cfd52d6 to
7d170fd
Compare
Member
Author
|
/retest-required |
ingvagabund
force-pushed
the
implement-OpenAPIModelNamer-interface
branch
from
7d170fd to
2cac22d
Compare
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ingvagabund
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@ingvagabund: This pull request references CNTRLPLANE-2449 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
needs-rebase
ingvagabund
force-pushed
the
implement-OpenAPIModelNamer-interface
branch
from
2cac22d to
14f7da2
Compare
openshift-ci
Bot
removed
the
needs-rebase
openshift-ci
Bot
added
the
needs-rebase
ingvagabund
force-pushed
the
implement-OpenAPIModelNamer-interface
branch
from
14f7da2 to
8a7f7b4
Compare
openshift-ci
Bot
removed
the
needs-rebase
… are already reversed via setting +k8s:openapi-model-package.
ingvagabund
force-pushed
the
implement-OpenAPIModelNamer-interface
branch
from
8a7f7b4 to
03828e0
Compare
ingvagabund
changed the title
Contributor
|
@ingvagabund: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TBD
Consequence of kubernetes/kubernetes#131755
Important note: the current
openapi/openapi.jsonis generated with the openshift type names incom.github.openshift.api.<group>.<version>.<TypeName>order. Implementing the new interface results in the openshift types names generated in<TypeName>.<version>.<group>.api.openshift.github.com. Which completely reshuffles the keys inopenapi/openapi.json. Which makes it hard to follow the changes. The kubernetes types already follow this new naming schema. E.g.APIGroup.v1.meta.apis.pkg.apimachinery.k8s.io.After the PR merged if consumers rely on the generated openapi.json they need to adjust their steps by running extra
GENERATOR=modelname hack/update-codegen.shbeforemake update-openapi. E.g. https://github.com/openshift/cluster-kube-descheduler-operator/blob/main/hack/update-codegen.sh#L27-L32