haskell
haskell copied to clipboard
kubernetes-client
Support GHC 9 (with both Aeson 1 and Aeson 2)
I started this PR with the goal of supporting aeson 2, which is used on the Stackage resolvers starting from the first week of February 2022. As it turned out, the changes for aeson 2 were pretty minimal, but the changes to support hoauth2 were more extensive, since hoauth2 went through a series of minor breaking changes at 2.0, 2.2, and 2.3. I've added separate CPP sections to support each of these.
So, this version should support both Aeson 1 and 2, as well as all the hoauth2 versions. I added a couple stack.yaml files to test these configurations. (I'd suggest setting up a Github Actions pipeline to build all of these? I could open a PR with that if you'd like.)
One thing to call out is that the oauth2RedirectUri option became required in hoauth2-2.3.0. I don't really understand why this is required for the OIDC authentication flow used by this library, since it wasn't provided in the past. But I added it to the OIDCAuth type, again protected by CPP.
I broke things up by commit to make it easier to review. Thanks!
Hi @thomasjm. Thanks for your PR.
I'm waiting for a kubernetes-client member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
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 kubernetes/test-infra repository.
Everything under the /Kubernetes folder is code-generated by https://github.com/OpenAPITools/openapi-generator . We re-gen this folder whenever the kubernetes API code needs to be re-generated e.g. for new kubernetes versions, which will overwrite it's contents. When we need to make changes to this folder beyond a simple re-gen, those changes first have to be upstreamed to https://github.com/OpenAPITools/openapi-generator , and then we have to update this repo to target the new openapi-generator, and then run the re-gen process.
Fortunately, the haskell-http-client code in https://github.com/OpenAPITools/openapi-generator has been updated recently for ghc9 and aeson2 (It just switches to aeson2 instead of attempting to support both) https://github.com/OpenAPITools/openapi-generator/pull/12309 .
Firstly, I would just switch everything over to aeson2 and exclude aeson1 to be consistent with the above.
Secondly, we need to include in this PR changing the targeted openapi-generator commit and then re-gen the /kubernetes folder with the latest generator code. After that the only manual update we make to /Kubernetes is to touch up and increment the package versions.
The process to regen is:
- clone https://github.com/kubernetes-client/gen (for example to
../kubernetes-client-gen - update the value of
OPENAPI_GENERATOR_COMMITinsettingsto the latest openapi-generator commit - run
./kubernetes-client-gen/openapi/haskell.sh kubernetes settings
Okay, so if I understand correctly the openapi-generator has not been updated to deal with later versions of hoauth2, right? So if we regenerate right now, the result won't build with the recent Stackage resolvers that use those versions. What do you think of me opening a PR to upstream the changes I made here to Kubernetes.Client.Auth.OIDC?
If we're upstreaming things to openapi-generator, I kind of want to also upstream the CPP for switching between Aeson 1 and 2. It's actually easy to support both, and it seems like the right thing to do since the ecosystem is still transitioning. What do you think?
Okay, so if I understand correctly the
openapi-generatorhas not been updated to deal with later versions ofhoauth2, right? So if we regenerate right now, the result won't build with the recent Stackage resolvers that use those versions. What do you think of me opening a PR to upstream the changes I made here toKubernetes.Client.Auth.OIDC?
The hoauth2 changes are limited to /kubernetes-client and so has no interaction with openapi-generator, and so those changes can be made directly to this repo.
/kubernetesis code-generated./kubernetes-clientis not code-generated, and can be update directly in this repo.
If we're upstreaming things to
openapi-generator, I kind of want to also upstream theCPPfor switching between Aeson 1 and 2. It's actually easy to support both, and it seems like the right thing to do since the ecosystem is still transitioning. What do you think?
Yes, we could upstream the CPP for switching between Aeson 1 and 2. It is only a little more work because there are more files in the generator and sample code that would need CPP, but nothing too bad ( https://github.com/OpenAPITools/openapi-generator/pull/12309/files ). I'd probably just like to do that myself since i'm more familiar with how to update that project
@thomasjm i added a commit to a copy of ghc9 https://github.com/jonschoning/kubernetes-client-haskell/tree/ghc9 with the updated generator and re-gen'd code if you want to review+pull the commit for your pr here: https://github.com/jonschoning/kubernetes-client-haskell/commit/a4a39efdd426d2b78077c78b0e793a90291986a8
@thomasjm: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| kubernetes-clients-haskell-unit-tests | c04d857e8828fed5230387d7efbf8d88e473a8e9 | link | false | /test kubernetes-clients-haskell-unit-tests |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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 kubernetes/test-infra repository. I understand the commands that are listed here.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: thomasjm To complete the pull request process, please ask for approval from jonschoning after the PR has been reviewed.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Thanks for updating the generator! I built it locally with both Aeson 1 and 2 and it looks good.
Trying to update the Travis config now. Have you considered switching to GitHub Actions? In my experience there tends to be less boilerplate with the GitHub Haskell action, so it's easier to update. For example: https://github.com/codedownio/sandwich/blob/master/.github/workflows/sandwich.yml
Actually I think we may have already moved away from travis to prow.
It looks like this is the failing job for the kubernetes prow test runner: https://prow.k8s.io/job-history/gs/kubernetes-jenkins/pr-logs/directory/kubernetes-clients-haskell-unit-tests
This may be file which is controlling the prow job: https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-client/haskell/client-haskell-presubmits.yaml
it looks like a much simpler configuration so we probably just need to udpate the docker image for ghc9 for the default build configuration in the test-infra repo.
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale - Mark this issue or PR as rotten with
/lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
@jonschoning maybe it's time to give up on https://github.com/kubernetes/test-infra/pull/26444 ? Is it possible to merge this anyway?
This PR works great, it would be nice to have it merged. Thanks @thomasjm !
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closed
You can:
- Reopen this PR with
/reopen - Mark this PR as fresh with
/remove-lifecycle rotten - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close
@k8s-triage-robot: Closed this PR.
In response to this:
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied- After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied- After 30d of inactivity since
lifecycle/rottenwas applied, the PR is closedYou can:
- Reopen this PR with
/reopen- Mark this PR as fresh with
/remove-lifecycle rotten- Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/close
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 kubernetes/test-infra repository.
@thomasjm: Reopened this PR.
In response to this:
/reopen /remove-lifecycle rotten
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 kubernetes/test-infra repository.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: thomasjm
Once this PR has been reviewed and has the lgtm label, please ask for approval from jonschoning by writing /assign @jonschoning in a comment. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@thomasjm , here is a GitHub Action based on this branch ghc9, ~perhaps it can be included in your PR~, please review: https://github.com/codedownio/kubernetes-client-haskell/pull/1
@peterbecich switching to GitHub Actions sounds great to me. In fact I suggested it earlier in this thread. But I was told that Kubernetes projects use Prow, and then we got mired in the attempt to do that using the Kubernetes test infra stuff.
FWIW I think the Prow approach is inferior anyway, because it uses a single Dockerfile with a baked-in version of GHC. Whereas GitHub Actions allows us to test all supported GHC versions.
What do you think @jonschoning? We could merge this PR, and then separately merge one containing the GH Actions config. Then we'll have everything working and CI running in this repo, with no dependency on test-infra.
@thomasjm I agree we should try switching to GitHub Actions. I think some of the other projects may be doing something similar now as well.
Okay, what do you think about the plan of merging this now and then doing a separate PR to get CI working? And once that's all green we can get a release on Hackage. Or would you rather do those in a different order?
Also, do we need another approver from the kubernetes-client project to merge?
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: jonschoning, thomasjm
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [jonschoning]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment