Bugs from recent changes - go.mod version invalid for go 1.20 and new interface is missing input -i

[vsoch]

I attempting using master here as of the changes today, and ran into two issues. First, the go.mod specifies 1.21.3, which is invalid for older verisons of go (possibly OK for newer but we are using 1.20):

17.96 /tmp/kube-openapi/go.mod:3: invalid go version '1.21.3': must match format 1.23
17.96 make: *** [Makefile:350: /workspace/bin/openapi-gen] Error 1
------

When I tweak that, the binary that builds is missing the -i for input file parameter, a new change in the UI that doesn't seem to be documented. Here is the UI before (two days ago):

Note the -i for input dirs.

And now after - there is no -i

Noted here:

There aren't releases (that I can pin to use) or even recent tags (the most recent with release- is from 3 years ago) so I don't have any choice but to pin the commit from two days ago (which works). It would be great if there were guidance on either of the above. We can't just update the entire version of go (at least right now) and pinning a commit is probably not best.

Ping @thockin for guidance who I think made the recent changes. Thank you!

[thockin]

Hihi.

We considered whether we should make a "v2" of this and decided not to, but the implication of that is that downstreams who use kube-openapi directly are exposed to this breaking change.

First, the X.Y vs. X.Y.Z change. Unfortunately, Go is doing this by default. We could manually reset it to "1.21" for now but eventually we won't want to keep patching that by hand. @alexzielenski or @jpbetz or @liggitt may have an opinion, but I am OK with that.

Second the CLI changes. Yeah. To make k/k use Go workspaces, I needed to reboot gengo. Fixing that required all the tools which use gengo to be changed, and it is just not completely compatible. As such, we took that moment to fix some long-standing pain points.

This could be better communicated - my fault.

1. The -i is not needed any more - just pass the arguments on the commandline.
2. The arguments need to be distinct, not comma-separated. E.g. what was -i ./foo,./bar,./bat is now ./foo ./bar ./bat.
3. The arguments need to be valid Go "patterns". E.g. example.com/foo or ./foo or ./..., but not "foo" or "/home/vsoch/stuff/foo".
4. The --output-base flag is now --output-dir and is the directory whereinto files will be written (e.g. "./generated").
5. There's a new --output-pkg flag which is required; it is the Go package name (e.g. "example.com/stuff/generated").
6. The --output-file-base flag is now --output-file and should include the ".go" suffix.

As a full example, you can look at https://github.com/kubernetes/kubernetes/blob/238c6dc5be3c016b8aaf207cc6d6918aac77121d/staging/src/k8s.io/code-generator/kube_codegen.sh#L358-L368

        "${gobin}/openapi-gen" \
            -v "${v}" \
            --output-file zz_generated.openapi.go \
            --go-header-file "${boilerplate}" \
            --output-dir "${out_dir}" \
            --output-pkg "${out_pkg}" \
            --report-filename "${new_report}" \
            "k8s.io/apimachinery/pkg/apis/meta/v1" \
            "k8s.io/apimachinery/pkg/runtime" \
            "k8s.io/apimachinery/pkg/version" \
            "${input_pkgs[@]}"

[thockin]

/assign

[thockin]

#462 has landed -- can you please confirm that at least the Go version problem is resolved?

[vsoch]

Yep can test now - just getting out of a meeting.

[vsoch]

Woot! That issue is resolved! Let me give a shot at updating my generation command now for the syntax you shared...

[vsoch]

I'm probably going to just pin it to the old version - I got a variant working:

.PHONY: generate
generate: controller-gen openapi-gen
	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
	rm -rf ./api/${API_VERSION}/zz_generated.openapi.go
	${OPENAPI_GEN} --logtostderr=true  --output-file zz_generated.openapi.go --output-pkg ${API_VERSION} --output-dir ./api/${API_VERSION}/ --go-header-file ./hack/boilerplate.go.txt -r "-" ./api/${API_VERSION}/

but it's making a ton of new files (I suspect for the subsequent swagger which I'm not sure is compatible anymore).

Too much change/ bugs to warrant the update now. Thanks for the help though!

[thockin]

Hmm, the output should be the same (maybe module the "generated by" comment format. I'd be happy to understand if that's not true.

[thockin]

When I run the Makefile patch you listed above, I see a bunch of diffs but they are all (arguably) correct:

@@ -13,8 +13,6 @@ SPDX-License-Identifier: Apache-2.0
 
 // Code generated by openapi-gen. DO NOT EDIT.
 
-// This file was autogenerated by openapi-gen. Do not edit it manually!
-
 package v1alpha1
 
 import (

As predicted.

@@ -24,35 +22,35 @@ import (

 func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition {
        return map[string]common.OpenAPIDefinition{
-               "./api/v1alpha1/.BurstedCluster":            schema__api_v1alpha1__BurstedCluster(ref),
-               "./api/v1alpha1/.Bursting":                  schema__api_v1alpha1__Bursting(ref),
-               "./api/v1alpha1/.Commands":                  schema__api_v1alpha1__Commands(ref),
-               "./api/v1alpha1/.ContainerResources":        schema__api_v1alpha1__ContainerResources(ref),
-               "./api/v1alpha1/.ContainerVolume":           schema__api_v1alpha1__ContainerVolume(ref),
-               "./api/v1alpha1/.FluxBroker":                schema__api_v1alpha1__FluxBroker(ref),
-               "./api/v1alpha1/.FluxRestful":               schema__api_v1alpha1__FluxRestful(ref),
-               "./api/v1alpha1/.FluxScheduler":             schema__api_v1alpha1__FluxScheduler(ref),
-               "./api/v1alpha1/.FluxSpec":                  schema__api_v1alpha1__FluxSpec(ref),
-               "./api/v1alpha1/.FluxUser":                  schema__api_v1alpha1__FluxUser(ref),
-               "./api/v1alpha1/.LifeCycle":                 schema__api_v1alpha1__LifeCycle(ref),
-               "./api/v1alpha1/.LoggingSpec":               schema__api_v1alpha1__LoggingSpec(ref),
-               "./api/v1alpha1/.MiniCluster":               schema__api_v1alpha1__MiniCluster(ref),
-               "./api/v1alpha1/.MiniClusterArchive":        schema__api_v1alpha1__MiniClusterArchive(ref),
-               "./api/v1alpha1/.MiniClusterContainer":      schema__api_v1alpha1__MiniClusterContainer(ref),
-               "./api/v1alpha1/.MiniClusterExistingVolume": schema__api_v1alpha1__MiniClusterExistingVolume(ref),
-               "./api/v1alpha1/.MiniClusterList":           schema__api_v1alpha1__MiniClusterList(ref),
-               "./api/v1alpha1/.MiniClusterSpec":           schema__api_v1alpha1__MiniClusterSpec(ref),
-               "./api/v1alpha1/.MiniClusterStatus":         schema__api_v1alpha1__MiniClusterStatus(ref),
-               "./api/v1alpha1/.MiniClusterUser":           schema__api_v1alpha1__MiniClusterUser(ref),
-               "./api/v1alpha1/.MiniClusterVolume":         schema__api_v1alpha1__MiniClusterVolume(ref),
-               "./api/v1alpha1/.Network":                   schema__api_v1alpha1__Network(ref),
-               "./api/v1alpha1/.PodSpec":                   schema__api_v1alpha1__PodSpec(ref),
-               "./api/v1alpha1/.Secret":                    schema__api_v1alpha1__Secret(ref),
-               "./api/v1alpha1/.SecurityContext":           schema__api_v1alpha1__SecurityContext(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.BurstedCluster":            schema_flux_framework_flux_operator_api_v1alpha1_BurstedCluster(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.Bursting":                  schema_flux_framework_flux_operator_api_v1alpha1_Bursting(ref),                  
+               "github.com/flux-framework/flux-operator/api/v1alpha1.Commands":                  schema_flux_framework_flux_operator_api_v1alpha1_Commands(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.ContainerResources":        schema_flux_framework_flux_operator_api_v1alpha1_ContainerResources(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.ContainerVolume":           schema_flux_framework_flux_operator_api_v1alpha1_ContainerVolume(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.FluxBroker":                schema_flux_framework_flux_operator_api_v1alpha1_FluxBroker(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.FluxRestful":               schema_flux_framework_flux_operator_api_v1alpha1_FluxRestful(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.FluxScheduler":             schema_flux_framework_flux_operator_api_v1alpha1_FluxScheduler(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.FluxSpec":                  schema_flux_framework_flux_operator_api_v1alpha1_FluxSpec(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.FluxUser":                  schema_flux_framework_flux_operator_api_v1alpha1_FluxUser(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.LifeCycle":                 schema_flux_framework_flux_operator_api_v1alpha1_LifeCycle(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.LoggingSpec":               schema_flux_framework_flux_operator_api_v1alpha1_LoggingSpec(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniCluster":               schema_flux_framework_flux_operator_api_v1alpha1_MiniCluster(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterArchive":        schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterArchive(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterContainer":      schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterContainer(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterExistingVolume": schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterExistingVolume(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterList":           schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterList(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterSpec":           schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterSpec(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterStatus":         schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterStatus(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterUser":           schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterUser(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.MiniClusterVolume":         schema_flux_framework_flux_operator_api_v1alpha1_MiniClusterVolume(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.Network":                   schema_flux_framework_flux_operator_api_v1alpha1_Network(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.PodSpec":                   schema_flux_framework_flux_operator_api_v1alpha1_PodSpec(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.Secret":                    schema_flux_framework_flux_operator_api_v1alpha1_Secret(ref),
+               "github.com/flux-framework/flux-operator/api/v1alpha1.SecurityContext":           schema_flux_framework_flux_operator_api_v1alpha1_SecurityContext(ref),
        }
 }

It looks like openapi-gen was retaining valid-but-unexpected input into code-generation. The pkg name is not properly "./api/v1alpha1/" but "github.com/flux-framework/flux-operator/api/v1alpha1". Both the leading "./" and the trailing "/" are now fixed.

@@ -79,7 +77,7 @@ func schema__api_v1alpha1__BurstedCluster(ref common.ReferenceCallback) common.O
        }
 }
 
-func schema__api_v1alpha1__Bursting(ref common.ReferenceCallback) common.OpenAPIDefinition {
+func schema_flux_framework_flux_operator_api_v1alpha1_Bursting(ref common.ReferenceCallback) common.OpenAPIDefinition {
        return common.OpenAPIDefinition{
                Schema: spec.Schema{
                        SchemaProps: spec.SchemaProps{
@@ -90,7 +88,7 @@ func schema__api_v1alpha1__Bursting(ref common.ReferenceCallback) common.OpenAPI
                                                SchemaProps: spec.SchemaProps{
                                                        Description: "The lead broker ip address to join to. E.g., if we burst to cluster 2, this is the address to connect to cluster 1 For the first cluster, this should not be defined",
                                                        Default:     map[string]interface{}{},
-                                                       Ref:         ref("./api/v1alpha1/.FluxBroker"),
+                                                       Ref:         ref("github.com/flux-framework/flux-operator/api/v1alpha1.FluxBroker"),
                                                },
                                        },
                                        "hostlist": {

Function names and refs are also fixed.

[thockin]

Should we keep this open? I'm not sure there's really a bug here (except maybe docs)

[vsoch]

Perhaps it's an issue with using it to generate the python files? I see:

And the generate (swagger):

.PHONY: api
api: generate api
	go run hack/python-sdk/main.go ${API_VERSION} > ${SWAGGER_API_JSON}
	rm -rf ./sdk/python/${API_VERSION}/fluxoperator/model/*
	rm -rf ./sdk/python/${API_VERSION}/fluxoperator/test/test_*.py
	java -jar ${SWAGGER_JAR} generate -i ${SWAGGER_API_JSON} -g python-legacy -o ./sdk/python/${API_VERSION} -c ./hack/python-sdk/swagger_config.json --git-repo-id flux-operator --git-user-id flux-framework

I ran into the same issue last year of not having good example for combining these two: #375

Ah yes, that's it. There is a slight change in the naming that goes into my custom script hack/python-sdk/main.go and when I fix that, the name is correctly replaced and the files are good! Apologies for missing that before - thanks for the help @thockin ! Sorry I think I just rubber ducked you in this comment. 😆

[thockin]

Happy to be the rubber duck :)