kubernetes / kube-openapi

Kubernetes OpenAPI spec generation & serving
Apache License 2.0
319 stars 208 forks source link

Update go-json-experiment to latest version #374

Closed Jefftree closed 1 year ago

Jefftree commented 1 year ago

Update go-json-experiment to latest version. Also create a hack/update-json-library.sh script for updating this in the future.

This also drops support for go 1.18

/cc @apelisse @alexzielenski @liggitt

apelisse commented 1 year ago

That seems expensive, and the CI will break anytime they update something, so we would depend on something outside our control?

alexzielenski commented 1 year ago

At the very least I mean something to ensure this dir doesn't accidentally get changed. I dont mean we should track HEAD.

apelisse commented 1 year ago

Sounds like we could use the hash saved in the file to check that the dir maps to the given hash.

Also, we could have a daily/weekly script run and try the most recent of repo to see if it still works.

Jefftree commented 1 year ago

it looks like go-json-experiment now has a dependency on go 1.19 (https://github.com/go-json-experiment/json/commit/ef074b63cd94c0c0f523f4312bc4140a749f0e85) so the go 1.18 CI tests are failing.

This PR also drops support for go 1.18 since Kubernetes v1.23+ are all on go 1.19.

liggitt commented 1 year ago

This PR also drops support for go 1.18 since Kubernetes v1.23+ are all on go 1.19.

they are at HEAD of release branches, but still pass unit and integration tests on their original go versions (go1.17 for v1.23, go1.18 for v1.24), and we wouldn't want to backport a change to k8s release branches that break people using kubernetes libraries with their original go version on a patch release.

That said, if we had to backport a kube-openapi change to 1.23 (unlikely, since 1.23 hits EOL in a couple weeks) or 1.24, we could make a kube-openapi branch prior to this commit, so go1.19+ support here seems fine

apelisse commented 1 year ago

That said, if we had to backport a kube-openapi change to 1.23 (unlikely, since 1.23 hits EOL in a couple weeks) or 1.24, we could make a kube-openapi branch prior to this commit, so go1.19+ support here seems fine

I don't think we would be able to backport kube-openapi that far since it would carry a lot of unwanted changes? We've branched in the past, notably because of smd version changing in backwards incompatible ways, I don't think that would be a problem.

apelisse commented 1 year ago

OK I'm fine with it /approve

@liggitt Want to LGTM?

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apelisse, Jefftree

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes/kube-openapi/blob/master/OWNERS)~~ [Jefftree,apelisse] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
liggitt commented 1 year ago

mobile, last round looked good to me if dsnet indicated HEAD is good to grab now

apelisse commented 1 year ago

@dsnet, would you recommend using the current HEAD or a specific hash for this?

apelisse commented 1 year ago

/lgtm

We can make a new PR if the hash is no bueno.